Add fieldManagerDefault configuration option + set field manager on non-SSA operations
#258
Conversation
| @@ -1348,6 +1349,15 @@ public void setMasterUrl(String masterUrl) { | |||
| this.masterUrl = ensureEndsWithSlash(ensureHttps(masterUrl, this)); | |||
| } | |||
|
|
|||
| @JsonProperty("fieldManagerOverride") | |||
| @@ -204,6 +204,10 @@ public URL getResourceURLForWriteOperation(URL resourceURL) throws MalformedURLE | |||
| resourceURL = new URL( | |||
| URLUtils.join(resourceURL.toString(), "?fieldValidation=" + context.fieldValidation.parameterValue())); | |||
| } | |||
| if (config.getFieldManagerOverride() != null) { | |||
| resourceURL = new URLUtils.URLBuilder(resourceURL).addQueryParameter("fieldManager", config.getFieldManagerOverride()).build(); | |||
There was a problem hiding this comment.
This doesn't match how the URL is built above - is that intentional?
There was a problem hiding this comment.
My intention was to handle ?fieldValidation being present or absent. If it's not set, then we need to append a ? to the URL for the fieldManager parameter, but if it is set then we don't. But looking again at what URLUtils.join(...) does, it looks like it does actually handle this case, so I'll make the new logic consistent with that.
| @@ -228,6 +232,9 @@ public URL getResourceURLForPatchOperation(URL resourceUrl, PatchContext patchCo | |||
| if (fieldManager == null && patchContext.getPatchType() == PatchType.SERVER_SIDE_APPLY) { | |||
| fieldManager = "fabric8"; | |||
| } | |||
| if (fieldManager == null && patchContext.getPatchType() != PatchType.SERVER_SIDE_APPLY && config.getFieldManagerOverride() != null) { | |||
There was a problem hiding this comment.
patchContext.getPatchType() != PatchType.SERVER_SIDE_APPLY
Why do we want to preserve fabric8 as the field manager for server-side apply?
There was a problem hiding this comment.
My intention was to avoid changing how server-side apply works, since it has some existing logic for setting managers eg via the PatchContext param, so I was worried it would expand the scope of this change too much. But I think overriding the default here with our config does make sense, since it'll only be reached if the field manager is left unset by that existing path.
There was a problem hiding this comment.
9325c84 Uses the override config for server-side apply as well. In the case of no field manager override being set on the config, it falls back on the fabric8 default for server-side apply,.
…lable; preserve fabric8 default for serverside apply-only when no override provided
mbc1990
changed the title
fieldManagerOverride configuration option so we can set the field manager on non-SSA operationsfieldManagerDefault configuration option + set field manager on non-SSA operations
|
This is awaiting a parent pom canary opportunity |
Replaces #257.
For
PUToperations, the client currently doesn't set a?fieldManagerquery parameter at all, which causes the API server to infer one from the User-Agent header, which leaves resources created with kubectl-operators with all their fields owned bylocalhost-<pid>@<some macbook ID>. This causes conflicts later when tools such ask safe-applyare used on the same objects.To fix this we want to make kubectl-operators set the same field manager that
k safe-applyuses, and this PR updates the kubernetes client to make that configurable.In addition to
PUT, this uses the same field manger override value forPATCHrequests that are not of the server-side applyPatchType.The behavior of server-side apply, which already provides an API for configuring the field manager, is unchanged in the case of a user providing a field manager via the
.withFieldManagercall. When a user does not provide a field manager to a server-side apply operation, the client will use thefieldManagerDefaultif it is set, otherwise it will use the existing default offabric8.I tested this manually with https://git.hubteam.com/HubSpot/kube-operators-tools/compare/build-with-fabric8-client-branch?expand=1 and verified that the client now sets the field manager with our provided default.