From ca691363cb09049b5ffffd1898d529b2c6905063 Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Wed, 22 Apr 2026 22:43:38 -0600 Subject: [PATCH 01/12] Add server-side apply support --- internal/commands/apply.go | 5 + internal/commands/apply_test.go | 27 ++ .../server-side-apply/components/cm.yaml | 7 + .../projects/server-side-apply/qbec.yaml | 11 + internal/model/app.go | 8 + internal/model/swagger-schema.go | 8 + internal/model/swagger.yaml | 6 + internal/model/types.go | 10 + internal/remote/client.go | 65 +++++ internal/remote/client_ssa_test.go | 234 ++++++++++++++++++ site/content/reference/qbec-yaml.md | 4 + 11 files changed, 385 insertions(+) create mode 100644 internal/commands/testdata/projects/server-side-apply/components/cm.yaml create mode 100644 internal/commands/testdata/projects/server-side-apply/qbec.yaml create mode 100644 internal/remote/client_ssa_test.go diff --git a/internal/commands/apply.go b/internal/commands/apply.go index 278427f8..78cde6ea 100644 --- a/internal/commands/apply.go +++ b/internal/commands/apply.go @@ -118,6 +118,10 @@ func doApply(ctx context.Context, args []string, config applyCommandConfig) erro opts := config.syncOptions opts.DisableUpdateFn = newUpdatePolicy().disableUpdate + opts.ApplyStrategy = config.App().ApplyStrategy() + if opts.ForceConflicts && opts.ApplyStrategy != model.ApplyStrategyServer { + return cmd.NewUsageError("force-conflicts requires spec.applyStrategy: server") + } if !opts.DryRun && len(objects) > 0 { msg := fmt.Sprintf("will synchronize %d object(s)", len(objects)) @@ -285,6 +289,7 @@ func newApplyCommand(cp ctxProvider) *cobra.Command { c.Flags().BoolVar(&config.syncOptions.DisableCreate, "skip-create", false, "set to true to only update existing resources but not create new ones") c.Flags().BoolVarP(&config.syncOptions.DryRun, "dry-run", "n", false, "dry-run, do not create/ update resources but show what would happen") c.Flags().BoolVarP(&config.syncOptions.ShowSecrets, "show-secrets", "S", false, "do not obfuscate secret values in the output") + c.Flags().BoolVar(&config.syncOptions.ForceConflicts, "force-conflicts", false, "force field ownership conflicts when using server-side apply") c.Flags().BoolVar(&config.showDetails, "show-details", false, "show details for object operations") c.Flags().BoolVar(&config.gc, "gc", true, "garbage collect extra objects on the server") c.Flags().BoolVar(&config.wait, "wait", false, "wait for changed objects to be ready") diff --git a/internal/commands/apply_test.go b/internal/commands/apply_test.go index 6d84cad8..cb259f40 100644 --- a/internal/commands/apply_test.go +++ b/internal/commands/apply_test.go @@ -135,6 +135,33 @@ func TestApplyFlags(t *testing.T) { s.assertErrorLineMatch(regexp.MustCompile(`\*\* dry-run mode, nothing was actually changed \*\*`)) } +func TestApplyServerSideApply(t *testing.T) { + s := newCustomScaffold(t, "testdata/projects/server-side-apply") + defer s.reset() + first := true + var captured remote.SyncOptions + s.client.syncFunc = func(ctx context.Context, obj model.K8sLocalObject, opts remote.SyncOptions) (*remote.SyncResult, error) { + if first { + first = false + captured = opts + } + return &remote.SyncResult{Type: remote.SyncCreated}, nil + } + err := s.executeCommand("apply", "local", "--gc=false", "--force-conflicts") + require.NoError(t, err) + assert.Equal(t, model.ApplyStrategyServer, captured.ApplyStrategy) + assert.True(t, captured.ForceConflicts) +} + +func TestApplyForceConflictsRequiresServerSideApply(t *testing.T) { + s := newScaffold(t) + defer s.reset() + err := s.executeCommand("apply", "dev", "--gc=false", "--force-conflicts") + require.Error(t, err) + assert.True(t, cmd.IsUsageError(err)) + assert.Equal(t, "force-conflicts requires spec.applyStrategy: server", err.Error()) +} + func TestApplyNamespaceClusterFilters(t *testing.T) { tests := []struct { name string diff --git a/internal/commands/testdata/projects/server-side-apply/components/cm.yaml b/internal/commands/testdata/projects/server-side-apply/components/cm.yaml new file mode 100644 index 00000000..e27e306e --- /dev/null +++ b/internal/commands/testdata/projects/server-side-apply/components/cm.yaml @@ -0,0 +1,7 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ssa-config +data: + foo: bar diff --git a/internal/commands/testdata/projects/server-side-apply/qbec.yaml b/internal/commands/testdata/projects/server-side-apply/qbec.yaml new file mode 100644 index 00000000..b4df54fb --- /dev/null +++ b/internal/commands/testdata/projects/server-side-apply/qbec.yaml @@ -0,0 +1,11 @@ +--- +apiVersion: qbec.io/v1alpha1 +kind: App +metadata: + name: server-side-apply +spec: + applyStrategy: server + environments: + local: + context: kind-kind + defaultNamespace: default diff --git a/internal/model/app.go b/internal/model/app.go index b79194c0..050dc77c 100644 --- a/internal/model/app.go +++ b/internal/model/app.go @@ -285,6 +285,14 @@ func (a *App) AddComponentLabel() bool { return a.inner.Spec.AddComponentLabel } +// ApplyStrategy returns the strategy used for qbec apply. +func (a *App) ApplyStrategy() ApplyStrategy { + if a.inner.Spec.ApplyStrategy == "" { + return ApplyStrategyClient + } + return a.inner.Spec.ApplyStrategy +} + func (a *App) envObject(env string) (Environment, error) { envObj, ok := a.inner.Spec.Environments[env] if !ok { diff --git a/internal/model/swagger-schema.go b/internal/model/swagger-schema.go index 5f118d3b..a504dbcc 100644 --- a/internal/model/swagger-schema.go +++ b/internal/model/swagger-schema.go @@ -70,6 +70,14 @@ var swaggerJSON = ` "description": "add component name as label to Kubernetes objects", "type": "boolean" }, + "applyStrategy": { + "description": "strategy used for qbec apply", + "enum": [ + "client", + "server" + ], + "type": "string" + }, "baseProperties": { "description": "properties for the baseline environment", "type": "object" diff --git a/internal/model/swagger.yaml b/internal/model/swagger.yaml index 26d756ac..82a2e638 100644 --- a/internal/model/swagger.yaml +++ b/internal/model/swagger.yaml @@ -135,6 +135,12 @@ definitions: addComponentLabel: description: add component name as label to Kubernetes objects type: boolean + applyStrategy: + description: strategy used for qbec apply + type: string + enum: + - client + - server dataSources: description: a list of data sources to be defined for the qbec app. items: diff --git a/internal/model/types.go b/internal/model/types.go index d2952043..98d1126a 100644 --- a/internal/model/types.go +++ b/internal/model/types.go @@ -21,6 +21,14 @@ import ( //go:generate gen-qbec-swagger swagger.yaml swagger-schema.go +// ApplyStrategy controls how qbec updates objects on the cluster. +type ApplyStrategy string + +const ( + ApplyStrategyClient ApplyStrategy = "client" + ApplyStrategyServer ApplyStrategy = "server" +) + // Environment points to a specific destination and has its own set of runtime parameters. type Environment struct { DefaultNamespace string `json:"defaultNamespace"` // default namespace to set for k8s context @@ -115,6 +123,8 @@ type AppSpec struct { ClusterScopedLists bool `json:"clusterScopedLists,omitempty"` // add component name as label to Kubernetes objects, default to false AddComponentLabel bool `json:"addComponentLabel,omitempty"` + // strategy used for qbec apply, defaults to client + ApplyStrategy ApplyStrategy `json:"applyStrategy,omitempty"` } // QbecEnvironmentMapSpec is the spec for a QbecEnvironmentMap object. diff --git a/internal/remote/client.go b/internal/remote/client.go index 7ae53460..693e3ad6 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -18,6 +18,7 @@ import ( "context" "encoding/json" "fmt" + "reflect" "strings" "time" @@ -41,6 +42,7 @@ const ( identicalObjects = "objects are identical" opUpdate = "update object" opCreate = "create object" + ssaFieldManager = "qbec" ) // structured errors @@ -68,6 +70,8 @@ type SyncOptions struct { DisableUpdateFn ConditionFunc // do not update an existing object WaitOptions TypeWaitOptions // opts for waiting ShowSecrets bool // show secrets in patches and creations + ApplyStrategy model.ApplyStrategy + ForceConflicts bool } // DeleteOptions provides the caller with options for the delete operation. @@ -621,6 +625,9 @@ func (c *Client) maybeCreate(ctx context.Context, obj model.K8sLocalObject, opts SkipReason: "creation disabled due to user request", }, nil } + if opts.ApplyStrategy == model.ApplyStrategyServer && obj.GetName() != "" { + return c.serverSideApply(ctx, obj, nil, opts, opCreate) + } b, err := json.Marshal(obj) if err != nil { return nil, errors.Wrap(err, "json marshal") @@ -647,12 +654,70 @@ func (c *Client) maybeCreate(ctx context.Context, obj model.K8sLocalObject, opts return result, nil } +func comparableObject(obj *unstructured.Unstructured) map[string]interface{} { + if obj == nil { + return nil + } + ret := obj.DeepCopy() + unstructured.RemoveNestedField(ret.Object, "metadata", "managedFields") + unstructured.RemoveNestedField(ret.Object, "metadata", "resourceVersion") + unstructured.RemoveNestedField(ret.Object, "metadata", "uid") + unstructured.RemoveNestedField(ret.Object, "metadata", "generation") + unstructured.RemoveNestedField(ret.Object, "metadata", "creationTimestamp") + return ret.Object +} + +func sameObject(lhs, rhs *unstructured.Unstructured) bool { + return reflect.DeepEqual(comparableObject(lhs), comparableObject(rhs)) +} + +func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions, operation string) (*updateResult, error) { + b, err := json.Marshal(obj) + if err != nil { + return nil, errors.Wrap(err, "json marshal") + } + result := &updateResult{ + Operation: operation, + Source: "server-side apply", + Kind: apiTypes.ApplyPatchType, + patch: b, + } + if obj.GetName() == "" { + return nil, fmt.Errorf("server-side apply requires object name") + } + ri, err := c.resourceInterfaceWithDefaultNs(obj.GroupVersionKind(), obj.GetNamespace()) + if err != nil { + return nil, errors.Wrap(err, "get resource interface") + } + patchOpts := metav1.PatchOptions{ + FieldManager: ssaFieldManager, + } + if opts.DryRun { + patchOpts.DryRun = []string{metav1.DryRunAll} + } + if opts.ForceConflicts { + force := true + patchOpts.Force = &force + } + out, err := ri.Patch(ctx, obj.GetName(), apiTypes.ApplyPatchType, b, patchOpts) + if err != nil { + return nil, errors.Wrap(err, "server-side apply") + } + if remObj != nil && sameObject(remObj, out) { + return &updateResult{SkipReason: identicalObjects}, nil + } + return result, nil +} + func (c *Client) maybeUpdate(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions) (*updateResult, error) { if opts.DisableUpdateFn(model.NewK8sObject(remObj.Object)) { return &updateResult{ SkipReason: "update disabled due to user request", }, nil } + if opts.ApplyStrategy == model.ApplyStrategyServer { + return c.serverSideApply(ctx, obj, remObj, opts, opUpdate) + } res, err := c.schema.OpenAPIResources() if err != nil { sio.Warnln("get open API resources", err) diff --git a/internal/remote/client_ssa_test.go b/internal/remote/client_ssa_test.go new file mode 100644 index 00000000..cabfb381 --- /dev/null +++ b/internal/remote/client_ssa_test.go @@ -0,0 +1,234 @@ +// Copyright 2025 Splunk Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package remote + +import ( + "context" + "testing" + + "github.com/splunk/qbec/internal/model" + "github.com/splunk/qbec/internal/remote/k8smeta" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + apiTypes "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" +) + +type staticPool struct { + client dynamic.Interface +} + +func (s staticPool) clientForGroupVersionKind(kind schema.GroupVersionKind) (dynamic.Interface, error) { + return s.client, nil +} + +type testDisco struct { + groups *metav1.APIGroupList + lists map[string]*metav1.APIResourceList +} + +func (d testDisco) ServerGroups() (*metav1.APIGroupList, error) { + return d.groups, nil +} + +func (d testDisco) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { + return d.lists[groupVersion], nil +} + +type recorderDynamic struct { + resource *recorderResource +} + +func (r recorderDynamic) Resource(resource schema.GroupVersionResource) dynamic.NamespaceableResourceInterface { + r.resource.gvr = resource + return r.resource +} + +type recorderResource struct { + gvr schema.GroupVersionResource + namespace string + patchName string + patchType apiTypes.PatchType + patchData []byte + patchOptions metav1.PatchOptions + patchResponse *unstructured.Unstructured + createObject *unstructured.Unstructured +} + +func (r *recorderResource) Namespace(namespace string) dynamic.ResourceInterface { + r.namespace = namespace + return r +} + +func (r *recorderResource) Create(ctx context.Context, obj *unstructured.Unstructured, options metav1.CreateOptions, subresources ...string) (*unstructured.Unstructured, error) { + r.createObject = obj.DeepCopy() + return obj, nil +} + +func (r *recorderResource) Update(ctx context.Context, obj *unstructured.Unstructured, options metav1.UpdateOptions, subresources ...string) (*unstructured.Unstructured, error) { + panic("unexpected call to Update") +} + +func (r *recorderResource) UpdateStatus(ctx context.Context, obj *unstructured.Unstructured, options metav1.UpdateOptions) (*unstructured.Unstructured, error) { + panic("unexpected call to UpdateStatus") +} + +func (r *recorderResource) Delete(ctx context.Context, name string, options metav1.DeleteOptions, subresources ...string) error { + panic("unexpected call to Delete") +} + +func (r *recorderResource) DeleteCollection(ctx context.Context, options metav1.DeleteOptions, listOptions metav1.ListOptions) error { + panic("unexpected call to DeleteCollection") +} + +func (r *recorderResource) Get(ctx context.Context, name string, options metav1.GetOptions, subresources ...string) (*unstructured.Unstructured, error) { + panic("unexpected call to Get") +} + +func (r *recorderResource) List(ctx context.Context, opts metav1.ListOptions) (*unstructured.UnstructuredList, error) { + panic("unexpected call to List") +} + +func (r *recorderResource) Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { + panic("unexpected call to Watch") +} + +func (r *recorderResource) Patch(ctx context.Context, name string, pt apiTypes.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*unstructured.Unstructured, error) { + r.patchName = name + r.patchType = pt + r.patchData = append([]byte(nil), data...) + r.patchOptions = *options.DeepCopy() + if r.patchResponse != nil { + return r.patchResponse.DeepCopy(), nil + } + return &unstructured.Unstructured{}, nil +} + +func (r *recorderResource) Apply(ctx context.Context, name string, obj *unstructured.Unstructured, options metav1.ApplyOptions, subresources ...string) (*unstructured.Unstructured, error) { + panic("unexpected call to Apply") +} + +func (r *recorderResource) ApplyStatus(ctx context.Context, name string, obj *unstructured.Unstructured, options metav1.ApplyOptions) (*unstructured.Unstructured, error) { + panic("unexpected call to ApplyStatus") +} + +func newConfigMap(namespace, name string) model.K8sLocalObject { + return model.NewK8sLocalObject(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": name, + "namespace": namespace, + }, + "data": map[string]interface{}{ + "foo": "bar", + }, + }, model.LocalAttrs{App: "app", Component: "comp", Env: "env"}) +} + +func newGenerateNameConfigMap(namespace, generateName string) model.K8sLocalObject { + return model.NewK8sLocalObject(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "generateName": generateName, + "namespace": namespace, + }, + "data": map[string]interface{}{ + "foo": "bar", + }, + }, model.LocalAttrs{App: "app", Component: "comp", Env: "env"}) +} + +func newServerSideApplyClient(t *testing.T, response *unstructured.Unstructured) (*Client, *recorderResource) { + t.Helper() + recorder := &recorderResource{patchResponse: response} + resources, err := k8smeta.NewResources(testDisco{ + groups: &metav1.APIGroupList{ + Groups: []metav1.APIGroup{ + { + Name: "", + Versions: []metav1.GroupVersionForDiscovery{ + {GroupVersion: "v1", Version: "v1"}, + }, + PreferredVersion: metav1.GroupVersionForDiscovery{GroupVersion: "v1", Version: "v1"}, + }, + }, + }, + lists: map[string]*metav1.APIResourceList{ + "v1": { + GroupVersion: "v1", + APIResources: []metav1.APIResource{ + {Name: "configmaps", Kind: "ConfigMap", Namespaced: true, Verbs: metav1.Verbs([]string{"create", "delete", "get", "list", "patch"})}, + }, + }, + }, + }, k8smeta.ResourceOpts{}) + require.NoError(t, err) + return &Client{ + resources: resources, + pool: staticPool{client: recorderDynamic{resource: recorder}}, + defaultNs: "default", + }, recorder +} + +func TestMaybeCreateServerSideApplyUsesPatchOptions(t *testing.T) { + obj := newConfigMap("default", "ssa-config") + client, recorder := newServerSideApplyClient(t, obj.ToUnstructured()) + result, err := client.maybeCreate(context.Background(), obj, SyncOptions{ + DryRun: true, + ApplyStrategy: model.ApplyStrategyServer, + ForceConflicts: true, + }) + require.NoError(t, err) + assert.Equal(t, opCreate, result.Operation) + assert.Equal(t, apiTypes.ApplyPatchType, recorder.patchType) + assert.Equal(t, "ssa-config", recorder.patchName) + assert.Equal(t, ssaFieldManager, recorder.patchOptions.FieldManager) + assert.Equal(t, []string{metav1.DryRunAll}, recorder.patchOptions.DryRun) + require.NotNil(t, recorder.patchOptions.Force) + assert.True(t, *recorder.patchOptions.Force) + assert.Equal(t, schema.GroupVersionResource{Version: "v1", Resource: "configmaps"}, recorder.gvr) + assert.Equal(t, "default", recorder.namespace) +} + +func TestMaybeCreateServerSideApplyFallsBackForGenerateName(t *testing.T) { + client, recorder := newServerSideApplyClient(t, nil) + obj := newGenerateNameConfigMap("default", "ssa-config-") + result, err := client.maybeCreate(context.Background(), obj, SyncOptions{ + ApplyStrategy: model.ApplyStrategyServer, + }) + require.NoError(t, err) + assert.Equal(t, opCreate, result.Operation) + assert.Equal(t, "", recorder.patchName) + require.NotNil(t, recorder.createObject) + assert.Equal(t, "ssa-config-", recorder.createObject.GetGenerateName()) +} + +func TestMaybeUpdateServerSideApplyDetectsIdenticalObjects(t *testing.T) { + existing := newConfigMap("default", "ssa-config").ToUnstructured() + client, recorder := newServerSideApplyClient(t, existing.DeepCopy()) + result, err := client.maybeUpdate(context.Background(), newConfigMap("default", "ssa-config"), existing.DeepCopy(), SyncOptions{ + ApplyStrategy: model.ApplyStrategyServer, + DisableUpdateFn: func(model.K8sMeta) bool { return false }, + }) + require.NoError(t, err) + assert.Equal(t, identicalObjects, result.SkipReason) + assert.Equal(t, apiTypes.ApplyPatchType, recorder.patchType) +} diff --git a/site/content/reference/qbec-yaml.md b/site/content/reference/qbec-yaml.md index af6019cf..330e6a8a 100644 --- a/site/content/reference/qbec-yaml.md +++ b/site/content/reference/qbec-yaml.md @@ -116,6 +116,10 @@ spec: # if the following attribute is set to true, qbec will add component names also as labels to Kubernetes objects. addComponentLabel: true + + # choose how `qbec apply` updates existing resources. Defaults to `client`. + # set to `server` to use Kubernetes server-side apply. + applyStrategy: server ``` ### Environment files From 5b532f7335eb0e43d38b7df86ce38559662a3170 Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Thu, 23 Apr 2026 11:47:31 -0600 Subject: [PATCH 02/12] Preserve dry-run fallback for server-side apply creates --- internal/remote/client.go | 2 +- internal/remote/client_ssa_test.go | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/internal/remote/client.go b/internal/remote/client.go index 6ccb95cc..175d7189 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -625,7 +625,7 @@ func (c *Client) maybeCreate(ctx context.Context, obj model.K8sLocalObject, opts SkipReason: "creation disabled due to user request", }, nil } - if opts.ApplyStrategy == model.ApplyStrategyServer && obj.GetName() != "" { + if opts.ApplyStrategy == model.ApplyStrategyServer && obj.GetName() != "" && !opts.DryRun { return c.serverSideApply(ctx, obj, nil, opts, opCreate) } b, err := json.Marshal(obj) diff --git a/internal/remote/client_ssa_test.go b/internal/remote/client_ssa_test.go index cabfb381..3709a062 100644 --- a/internal/remote/client_ssa_test.go +++ b/internal/remote/client_ssa_test.go @@ -192,7 +192,6 @@ func TestMaybeCreateServerSideApplyUsesPatchOptions(t *testing.T) { obj := newConfigMap("default", "ssa-config") client, recorder := newServerSideApplyClient(t, obj.ToUnstructured()) result, err := client.maybeCreate(context.Background(), obj, SyncOptions{ - DryRun: true, ApplyStrategy: model.ApplyStrategyServer, ForceConflicts: true, }) @@ -201,7 +200,7 @@ func TestMaybeCreateServerSideApplyUsesPatchOptions(t *testing.T) { assert.Equal(t, apiTypes.ApplyPatchType, recorder.patchType) assert.Equal(t, "ssa-config", recorder.patchName) assert.Equal(t, ssaFieldManager, recorder.patchOptions.FieldManager) - assert.Equal(t, []string{metav1.DryRunAll}, recorder.patchOptions.DryRun) + assert.Nil(t, recorder.patchOptions.DryRun) require.NotNil(t, recorder.patchOptions.Force) assert.True(t, *recorder.patchOptions.Force) assert.Equal(t, schema.GroupVersionResource{Version: "v1", Resource: "configmaps"}, recorder.gvr) @@ -221,6 +220,21 @@ func TestMaybeCreateServerSideApplyFallsBackForGenerateName(t *testing.T) { assert.Equal(t, "ssa-config-", recorder.createObject.GetGenerateName()) } +func TestMaybeCreateServerSideApplyDryRunFallsBackToLocalCreate(t *testing.T) { + client, recorder := newServerSideApplyClient(t, nil) + obj := newConfigMap("default", "ssa-config") + result, err := client.maybeCreate(context.Background(), obj, SyncOptions{ + DryRun: true, + ApplyStrategy: model.ApplyStrategyServer, + }) + require.NoError(t, err) + assert.Equal(t, opCreate, result.Operation) + assert.Equal(t, "local", result.Source) + assert.Nil(t, recorder.createObject) + assert.Empty(t, recorder.patchName) + assert.Nil(t, recorder.patchData) +} + func TestMaybeUpdateServerSideApplyDetectsIdenticalObjects(t *testing.T) { existing := newConfigMap("default", "ssa-config").ToUnstructured() client, recorder := newServerSideApplyClient(t, existing.DeepCopy()) From 0f68fd9b364144791627df7aab9e60b29ded7cff Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Thu, 23 Apr 2026 12:31:15 -0600 Subject: [PATCH 03/12] fix: bypass SSA for secret dry-run --- internal/remote/client.go | 8 +++--- internal/remote/client_ssa_test.go | 43 +++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/internal/remote/client.go b/internal/remote/client.go index 175d7189..40b485f3 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -503,7 +503,7 @@ func (c *Client) doSync(ctx context.Context, original model.K8sLocalObject, opts c, _ := types.HideSensitiveInfo(remObj) remObj = c } - result, err = c.maybeUpdate(ctx, obj, remObj, opts) + result, err = c.maybeUpdate(ctx, obj, remObj, opts, internal) } if err != nil { return nil, err @@ -709,13 +709,15 @@ func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, return result, nil } -func (c *Client) maybeUpdate(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions) (*updateResult, error) { +func (c *Client) maybeUpdate(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions, internal internalSyncOptions) (*updateResult, error) { if opts.DisableUpdateFn(model.NewK8sObject(remObj.Object)) { return &updateResult{ SkipReason: "update disabled due to user request", }, nil } - if opts.ApplyStrategy == model.ApplyStrategyServer { + // Secret dry-runs redact data for safe display. They are not a real apply preflight and + // must not be sent to the apiserver via SSA, or redacted values can trigger bogus conflicts. + if opts.ApplyStrategy == model.ApplyStrategyServer && !internal.secretDryRun { return c.serverSideApply(ctx, obj, remObj, opts, opUpdate) } res, err := c.schema.OpenAPIResources() diff --git a/internal/remote/client_ssa_test.go b/internal/remote/client_ssa_test.go index 3709a062..73ed7f93 100644 --- a/internal/remote/client_ssa_test.go +++ b/internal/remote/client_ssa_test.go @@ -16,10 +16,13 @@ package remote import ( "context" + "fmt" "testing" + openapi_v2 "github.com/google/gnostic-models/openapiv2" "github.com/splunk/qbec/internal/model" "github.com/splunk/qbec/internal/remote/k8smeta" + qtypes "github.com/splunk/qbec/internal/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -51,6 +54,10 @@ func (d testDisco) ServerResourcesForGroupVersion(groupVersion string) (*metav1. return d.lists[groupVersion], nil } +func (d testDisco) OpenAPISchema() (*openapi_v2.Document, error) { + return nil, fmt.Errorf("not implemented") +} + type recorderDynamic struct { resource *recorderResource } @@ -156,6 +163,20 @@ func newGenerateNameConfigMap(namespace, generateName string) model.K8sLocalObje }, model.LocalAttrs{App: "app", Component: "comp", Env: "env"}) } +func newSecret(namespace, name, value string) model.K8sLocalObject { + return model.NewK8sLocalObject(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": name, + "namespace": namespace, + }, + "data": map[string]interface{}{ + "token": value, + }, + }, model.LocalAttrs{App: "app", Component: "comp", Env: "env"}) +} + func newServerSideApplyClient(t *testing.T, response *unstructured.Unstructured) (*Client, *recorderResource) { t.Helper() recorder := &recorderResource{patchResponse: response} @@ -176,6 +197,7 @@ func newServerSideApplyClient(t *testing.T, response *unstructured.Unstructured) GroupVersion: "v1", APIResources: []metav1.APIResource{ {Name: "configmaps", Kind: "ConfigMap", Namespaced: true, Verbs: metav1.Verbs([]string{"create", "delete", "get", "list", "patch"})}, + {Name: "secrets", Kind: "Secret", Namespaced: true, Verbs: metav1.Verbs([]string{"create", "delete", "get", "list", "patch"})}, }, }, }, @@ -183,6 +205,7 @@ func newServerSideApplyClient(t *testing.T, response *unstructured.Unstructured) require.NoError(t, err) return &Client{ resources: resources, + schema: k8smeta.NewServerSchema(testDisco{}), pool: staticPool{client: recorderDynamic{resource: recorder}}, defaultNs: "default", }, recorder @@ -241,8 +264,26 @@ func TestMaybeUpdateServerSideApplyDetectsIdenticalObjects(t *testing.T) { result, err := client.maybeUpdate(context.Background(), newConfigMap("default", "ssa-config"), existing.DeepCopy(), SyncOptions{ ApplyStrategy: model.ApplyStrategyServer, DisableUpdateFn: func(model.K8sMeta) bool { return false }, - }) + }, internalSyncOptions{}) require.NoError(t, err) assert.Equal(t, identicalObjects, result.SkipReason) assert.Equal(t, apiTypes.ApplyPatchType, recorder.patchType) } + +func TestMaybeUpdateSecretDryRunBypassesServerSideApply(t *testing.T) { + existing := newSecret("default", "ssa-secret", "dmFsdWU=").ToUnstructured() + client, recorder := newServerSideApplyClient(t, existing.DeepCopy()) + localObj, changed := qtypes.HideSensitiveLocalInfo(newSecret("default", "ssa-secret", "dmFsdWU=")) + require.True(t, changed) + result, err := client.maybeUpdate(context.Background(), localObj, existing.DeepCopy(), SyncOptions{ + DryRun: true, + ApplyStrategy: model.ApplyStrategyServer, + DisableUpdateFn: func(model.K8sMeta) bool { + return false + }, + }, internalSyncOptions{secretDryRun: true}) + require.NoError(t, err) + assert.NotEqual(t, apiTypes.ApplyPatchType, result.Kind) + assert.Empty(t, recorder.patchName) + assert.Nil(t, recorder.patchData) +} From 939b3c4610288e25950adcb12c53d3d3af201414 Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Wed, 22 Apr 2026 23:00:31 -0600 Subject: [PATCH 04/12] Move server-side apply to directives --- internal/commands/apply.go | 5 +--- internal/commands/apply_test.go | 26 +++++++------------ internal/commands/directives.go | 8 ++++++ internal/commands/directives_test.go | 10 +++++++ .../server-side-apply/components/cm.yaml | 9 +++++++ .../projects/server-side-apply/qbec.yaml | 1 - internal/model/app.go | 8 ------ internal/model/external-names.go | 18 +++++++------ internal/model/swagger-schema.go | 8 ------ internal/model/swagger.yaml | 6 ----- internal/model/types.go | 2 -- site/content/reference/directives.md | 10 ++++++- site/content/reference/qbec-yaml.md | 4 --- 13 files changed, 56 insertions(+), 59 deletions(-) diff --git a/internal/commands/apply.go b/internal/commands/apply.go index 78cde6ea..ab4e3279 100644 --- a/internal/commands/apply.go +++ b/internal/commands/apply.go @@ -118,10 +118,6 @@ func doApply(ctx context.Context, args []string, config applyCommandConfig) erro opts := config.syncOptions opts.DisableUpdateFn = newUpdatePolicy().disableUpdate - opts.ApplyStrategy = config.App().ApplyStrategy() - if opts.ForceConflicts && opts.ApplyStrategy != model.ApplyStrategyServer { - return cmd.NewUsageError("force-conflicts requires spec.applyStrategy: server") - } if !opts.DryRun && len(objects) > 0 { msg := fmt.Sprintf("will synchronize %d object(s)", len(objects)) @@ -182,6 +178,7 @@ func doApply(ctx context.Context, args []string, config applyCommandConfig) erro waitPolicy := newWaitPolicy() for _, ob := range objects { + opts.ApplyStrategy = applyStrategy(ob) name := client.DisplayName(ob) res, err := client.Sync(ctx, ob, opts) if res != nil && res.GeneratedName != "" { diff --git a/internal/commands/apply_test.go b/internal/commands/apply_test.go index cb259f40..8dea16b4 100644 --- a/internal/commands/apply_test.go +++ b/internal/commands/apply_test.go @@ -138,28 +138,20 @@ func TestApplyFlags(t *testing.T) { func TestApplyServerSideApply(t *testing.T) { s := newCustomScaffold(t, "testdata/projects/server-side-apply") defer s.reset() - first := true - var captured remote.SyncOptions + var captured []remote.SyncOptions s.client.syncFunc = func(ctx context.Context, obj model.K8sLocalObject, opts remote.SyncOptions) (*remote.SyncResult, error) { - if first { - first = false - captured = opts - } + captured = append(captured, opts) return &remote.SyncResult{Type: remote.SyncCreated}, nil } err := s.executeCommand("apply", "local", "--gc=false", "--force-conflicts") require.NoError(t, err) - assert.Equal(t, model.ApplyStrategyServer, captured.ApplyStrategy) - assert.True(t, captured.ForceConflicts) -} - -func TestApplyForceConflictsRequiresServerSideApply(t *testing.T) { - s := newScaffold(t) - defer s.reset() - err := s.executeCommand("apply", "dev", "--gc=false", "--force-conflicts") - require.Error(t, err) - assert.True(t, cmd.IsUsageError(err)) - assert.Equal(t, "force-conflicts requires spec.applyStrategy: server", err.Error()) + require.Len(t, captured, 2) + assert.ElementsMatch(t, + []model.ApplyStrategy{model.ApplyStrategyServer, model.ApplyStrategyClient}, + []model.ApplyStrategy{captured[0].ApplyStrategy, captured[1].ApplyStrategy}, + ) + assert.True(t, captured[0].ForceConflicts) + assert.True(t, captured[1].ForceConflicts) } func TestApplyNamespaceClusterFilters(t *testing.T) { diff --git a/internal/commands/directives.go b/internal/commands/directives.go index 9189be75..168bdc35 100644 --- a/internal/commands/directives.go +++ b/internal/commands/directives.go @@ -26,6 +26,7 @@ import ( const ( policyNever = "never" policyDefault = "default" + policyServer = "server" ) // isSet return true if the annotation name specified as directive is equal to the supplied value. @@ -58,6 +59,13 @@ func isSet(ob model.K8sMeta, directive, value string, otherAllowedValues []strin type updatePolicy struct{} +func applyStrategy(ob model.K8sMeta) model.ApplyStrategy { + if isSet(ob, model.QbecNames.Directives.ApplyStrategy, policyServer, []string{policyDefault}) { + return model.ApplyStrategyServer + } + return model.ApplyStrategyClient +} + func (u *updatePolicy) disableUpdate(ob model.K8sMeta) bool { return isSet(ob, model.QbecNames.Directives.UpdatePolicy, policyNever, []string{policyDefault}) } diff --git a/internal/commands/directives_test.go b/internal/commands/directives_test.go index d60b6a68..bfb6a3be 100644 --- a/internal/commands/directives_test.go +++ b/internal/commands/directives_test.go @@ -90,6 +90,16 @@ func TestDirectivesUpdatePolicy(t *testing.T) { a.True(ret) } +func TestDirectivesApplyStrategy(t *testing.T) { + a := assert.New(t) + ret := applyStrategy(k8sMetaWithAnnotations("ConfigMap", "foo", "bar", nil)) + a.Equal(model.ApplyStrategyClient, ret) + ret = applyStrategy(k8sMetaWithAnnotations("ConfigMap", "foo", "bar", map[string]interface{}{ + "directives.qbec.io/apply-strategy": "server", + })) + a.Equal(model.ApplyStrategyServer, ret) +} + func TestDirectivesDeletePolicy(t *testing.T) { dp := newDeletePolicy(func(gvk schema.GroupVersionKind) (bool, error) { return gvk.Kind == "ConfigMap", nil diff --git a/internal/commands/testdata/projects/server-side-apply/components/cm.yaml b/internal/commands/testdata/projects/server-side-apply/components/cm.yaml index e27e306e..10276423 100644 --- a/internal/commands/testdata/projects/server-side-apply/components/cm.yaml +++ b/internal/commands/testdata/projects/server-side-apply/components/cm.yaml @@ -3,5 +3,14 @@ apiVersion: v1 kind: ConfigMap metadata: name: ssa-config + annotations: + directives.qbec.io/apply-strategy: server data: foo: bar +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: client-config +data: + foo: baz diff --git a/internal/commands/testdata/projects/server-side-apply/qbec.yaml b/internal/commands/testdata/projects/server-side-apply/qbec.yaml index b4df54fb..5f26093e 100644 --- a/internal/commands/testdata/projects/server-side-apply/qbec.yaml +++ b/internal/commands/testdata/projects/server-side-apply/qbec.yaml @@ -4,7 +4,6 @@ kind: App metadata: name: server-side-apply spec: - applyStrategy: server environments: local: context: kind-kind diff --git a/internal/model/app.go b/internal/model/app.go index e7454b29..7b04aac0 100644 --- a/internal/model/app.go +++ b/internal/model/app.go @@ -285,14 +285,6 @@ func (a *App) AddComponentLabel() bool { return a.inner.Spec.AddComponentLabel } -// ApplyStrategy returns the strategy used for qbec apply. -func (a *App) ApplyStrategy() ApplyStrategy { - if a.inner.Spec.ApplyStrategy == "" { - return ApplyStrategyClient - } - return a.inner.Spec.ApplyStrategy -} - func (a *App) envObject(env string) (Environment, error) { envObj, ok := a.inner.Spec.Environments[env] if !ok { diff --git a/internal/model/external-names.go b/internal/model/external-names.go index b3e9a232..92ead12b 100644 --- a/internal/model/external-names.go +++ b/internal/model/external-names.go @@ -22,10 +22,11 @@ const QBECDirectivesNamespace = "directives.qbec.io/" // Directives is the list of directive names we support. type Directives struct { - ApplyOrder string // numeric apply order for object - DeletePolicy string // delete policy "default" | "never" - UpdatePolicy string // update policy "default" | "never" - WaitPolicy string // wait policy "default" | "never" + ApplyOrder string // numeric apply order for object + ApplyStrategy string // apply strategy "default" | "server" + DeletePolicy string // delete policy "default" | "never" + UpdatePolicy string // update policy "default" | "never" + WaitPolicy string // wait policy "default" | "never" } // QbecNames is the set of names used by Qbec. @@ -55,9 +56,10 @@ var QbecNames = struct { DefaultNsVarName: QBECMetadataPrefix + "defaultNs", CleanModeVarName: QBECMetadataPrefix + "cleanMode", Directives: Directives{ - ApplyOrder: QBECDirectivesNamespace + "apply-order", - DeletePolicy: QBECDirectivesNamespace + "delete-policy", - UpdatePolicy: QBECDirectivesNamespace + "update-policy", - WaitPolicy: QBECDirectivesNamespace + "wait-policy", + ApplyOrder: QBECDirectivesNamespace + "apply-order", + ApplyStrategy: QBECDirectivesNamespace + "apply-strategy", + DeletePolicy: QBECDirectivesNamespace + "delete-policy", + UpdatePolicy: QBECDirectivesNamespace + "update-policy", + WaitPolicy: QBECDirectivesNamespace + "wait-policy", }, } diff --git a/internal/model/swagger-schema.go b/internal/model/swagger-schema.go index a504dbcc..5f118d3b 100644 --- a/internal/model/swagger-schema.go +++ b/internal/model/swagger-schema.go @@ -70,14 +70,6 @@ var swaggerJSON = ` "description": "add component name as label to Kubernetes objects", "type": "boolean" }, - "applyStrategy": { - "description": "strategy used for qbec apply", - "enum": [ - "client", - "server" - ], - "type": "string" - }, "baseProperties": { "description": "properties for the baseline environment", "type": "object" diff --git a/internal/model/swagger.yaml b/internal/model/swagger.yaml index 82a2e638..26d756ac 100644 --- a/internal/model/swagger.yaml +++ b/internal/model/swagger.yaml @@ -135,12 +135,6 @@ definitions: addComponentLabel: description: add component name as label to Kubernetes objects type: boolean - applyStrategy: - description: strategy used for qbec apply - type: string - enum: - - client - - server dataSources: description: a list of data sources to be defined for the qbec app. items: diff --git a/internal/model/types.go b/internal/model/types.go index 98d1126a..135ee793 100644 --- a/internal/model/types.go +++ b/internal/model/types.go @@ -123,8 +123,6 @@ type AppSpec struct { ClusterScopedLists bool `json:"clusterScopedLists,omitempty"` // add component name as label to Kubernetes objects, default to false AddComponentLabel bool `json:"addComponentLabel,omitempty"` - // strategy used for qbec apply, defaults to client - ApplyStrategy ApplyStrategy `json:"applyStrategy,omitempty"` } // QbecEnvironmentMapSpec is the spec for a QbecEnvironmentMap object. diff --git a/site/content/reference/directives.md b/site/content/reference/directives.md index aeafa673..533a9e58 100644 --- a/site/content/reference/directives.md +++ b/site/content/reference/directives.md @@ -13,6 +13,15 @@ Annotations that you can use for your objects to control qbec behavior. controls the order in which objects are applied. This allows you, for example, to move updates of a custom resource to after all other objects have been processed. +#### `directives.qbec.io/apply-strategy` + +* Annotation source: local object +* Allowed values: `"default"`, `"server"` +* Default value: `"default"` + +when set to `"server"`, qbec uses Kubernetes server-side apply for that object during `qbec apply`. +Objects without this annotation continue to use the existing client-side apply behavior. + #### `directives.qbec.io/delete-policy` * Annotation source: in-cluster object @@ -41,4 +50,3 @@ object to remove this annotation will not work. when set to `"never"` for deployments or daemonsets, indicates that qbec should not wait for that object even when the `--wait` or `--wait-all` flags are set for the `apply` command. - diff --git a/site/content/reference/qbec-yaml.md b/site/content/reference/qbec-yaml.md index 330e6a8a..af6019cf 100644 --- a/site/content/reference/qbec-yaml.md +++ b/site/content/reference/qbec-yaml.md @@ -116,10 +116,6 @@ spec: # if the following attribute is set to true, qbec will add component names also as labels to Kubernetes objects. addComponentLabel: true - - # choose how `qbec apply` updates existing resources. Defaults to `client`. - # set to `server` to use Kubernetes server-side apply. - applyStrategy: server ``` ### Environment files From 719f567af1d7c5b8439b4701b31befc4d5032381 Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Thu, 23 Apr 2026 21:24:06 -0600 Subject: [PATCH 05/12] Strip pristine annotation from server-side apply --- internal/remote/client.go | 7 ++++++- internal/remote/client_ssa_test.go | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/internal/remote/client.go b/internal/remote/client.go index 40b485f3..7c23cada 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -672,7 +672,12 @@ func sameObject(lhs, rhs *unstructured.Unstructured) bool { } func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions, operation string) (*updateResult, error) { - b, err := json.Marshal(obj) + applyObj := obj.ToUnstructured().DeepCopy() + if ann := applyObj.GetAnnotations(); ann != nil { + delete(ann, model.QbecNames.PristineAnnotation) + applyObj.SetAnnotations(ann) + } + b, err := json.Marshal(applyObj.Object) if err != nil { return nil, errors.Wrap(err, "json marshal") } diff --git a/internal/remote/client_ssa_test.go b/internal/remote/client_ssa_test.go index 73ed7f93..e3102804 100644 --- a/internal/remote/client_ssa_test.go +++ b/internal/remote/client_ssa_test.go @@ -16,6 +16,7 @@ package remote import ( "context" + "encoding/json" "fmt" "testing" @@ -287,3 +288,24 @@ func TestMaybeUpdateSecretDryRunBypassesServerSideApply(t *testing.T) { assert.Empty(t, recorder.patchName) assert.Nil(t, recorder.patchData) } + +func TestServerSideApplyStripsPristineAnnotation(t *testing.T) { + existing := newConfigMap("default", "ssa-config").ToUnstructured() + annotated, err := qbecPristine{}.createFromPristine(newConfigMap("default", "ssa-config")) + require.NoError(t, err) + + client, recorder := newServerSideApplyClient(t, existing.DeepCopy()) + _, err = client.serverSideApply(context.Background(), annotated, existing.DeepCopy(), SyncOptions{}, opUpdate) + require.NoError(t, err) + + var patch map[string]interface{} + require.NoError(t, json.Unmarshal(recorder.patchData, &patch)) + + metadata, ok := patch["metadata"].(map[string]interface{}) + require.True(t, ok) + annotations, ok := metadata["annotations"].(map[string]interface{}) + if ok { + _, found := annotations[model.QbecNames.PristineAnnotation] + assert.False(t, found) + } +} From 079723b30d2664fecf7a9514dd13b9804a77da7f Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Thu, 23 Apr 2026 22:16:11 -0600 Subject: [PATCH 06/12] Support apply strategy transitions --- internal/remote/client.go | 203 +++++++++++++++++++++++++---- internal/remote/client_ssa_test.go | 88 +++++++++++-- internal/remote/pristine.go | 57 +++++++- internal/remote/pristine_test.go | 85 ++++++++++++ 4 files changed, 391 insertions(+), 42 deletions(-) diff --git a/internal/remote/client.go b/internal/remote/client.go index 7c23cada..ffc7be1d 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -18,7 +18,6 @@ import ( "context" "encoding/json" "fmt" - "reflect" "strings" "time" @@ -31,10 +30,13 @@ import ( apiErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" apiTypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" + "sigs.k8s.io/structured-merge-diff/v6/fieldpath" + "sigs.k8s.io/structured-merge-diff/v6/value" "sigs.k8s.io/yaml" ) @@ -667,17 +669,171 @@ func comparableObject(obj *unstructured.Unstructured) map[string]interface{} { return ret.Object } -func sameObject(lhs, rhs *unstructured.Unstructured) bool { - return reflect.DeepEqual(comparableObject(lhs), comparableObject(rhs)) +func cloneLocalObject(obj model.K8sLocalObject) model.K8sLocalObject { + return model.NewK8sLocalObject(runtime.DeepCopyJSON(obj.ToUnstructured().Object), model.LocalAttrs{ + App: obj.Application(), + Tag: obj.Tag(), + Component: obj.Component(), + Env: obj.Environment(), + }) } -func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions, operation string) (*updateResult, error) { - applyObj := obj.ToUnstructured().DeepCopy() - if ann := applyObj.GetAnnotations(); ann != nil { - delete(ann, model.QbecNames.PristineAnnotation) - applyObj.SetAnnotations(ann) +func stripApplyHistoryAnnotations(obj model.K8sLocalObject) model.K8sLocalObject { + ret := cloneLocalObject(obj) + annotations := ret.ToUnstructured().GetAnnotations() + if len(annotations) == 0 { + return ret + } + delete(annotations, model.QbecNames.PristineAnnotation) + delete(annotations, kubectlLastConfig) + ret.ToUnstructured().SetAnnotations(annotations) + return ret +} + +func managedFieldSet(obj *unstructured.Unstructured, fieldManager string) (*fieldpath.Set, error) { + set := fieldpath.NewSet() + if obj == nil { + return set, nil + } + for _, entry := range obj.GetManagedFields() { + if entry.Manager != fieldManager || entry.Operation != metav1.ManagedFieldsOperationApply || entry.Subresource != "" { + continue + } + if entry.FieldsV1 == nil || len(entry.FieldsV1.Raw) == 0 { + continue + } + entrySet := &fieldpath.Set{} + if err := entrySet.FromJSON(strings.NewReader(string(entry.FieldsV1.Raw))); err != nil { + return nil, errors.Wrap(err, "parse managed fields") + } + set = set.Union(entrySet.Leaves()) + } + return set, nil +} + +func projectedObject(obj *unstructured.Unstructured, fieldSet *fieldpath.Set) map[string]interface{} { + if obj == nil || fieldSet == nil || fieldSet.Empty() { + return nil + } + projected, ok := projectValue(obj.Object, fieldSet) + if !ok { + return nil + } + ret, ok := projected.(map[string]interface{}) + if !ok { + return nil + } + return comparableObject(&unstructured.Unstructured{Object: ret}) +} + +func projectValue(v interface{}, fieldSet *fieldpath.Set) (interface{}, bool) { + if fieldSet == nil || fieldSet.Empty() || v == nil { + return nil, false } - b, err := json.Marshal(applyObj.Object) + switch vv := v.(type) { + case map[string]interface{}: + out := map[string]interface{}{} + fieldSet.Members.Iterate(func(pe fieldpath.PathElement) { + if pe.FieldName == nil { + return + } + if child, ok := vv[*pe.FieldName]; ok { + out[*pe.FieldName] = runtime.DeepCopyJSONValue(child) + } + }) + fieldSet.Children.Iterate(func(pe fieldpath.PathElement) { + if pe.FieldName == nil { + return + } + childSet, ok := fieldSet.Children.Get(pe) + if !ok { + return + } + child, ok := vv[*pe.FieldName] + if !ok { + return + } + projected, ok := projectValue(child, childSet) + if ok { + out[*pe.FieldName] = projected + } + }) + return out, len(out) > 0 + case []interface{}: + out := []interface{}{} + fieldSet.Members.Iterate(func(pe fieldpath.PathElement) { + if child, ok := listItemForPathElement(vv, pe); ok { + out = append(out, runtime.DeepCopyJSONValue(child)) + } + }) + fieldSet.Children.Iterate(func(pe fieldpath.PathElement) { + childSet, ok := fieldSet.Children.Get(pe) + if !ok { + return + } + child, ok := listItemForPathElement(vv, pe) + if !ok { + return + } + projected, ok := projectValue(child, childSet) + if ok { + out = append(out, projected) + } + }) + return out, len(out) > 0 + default: + return runtime.DeepCopyJSONValue(v), true + } +} + +func listItemForPathElement(list []interface{}, pe fieldpath.PathElement) (interface{}, bool) { + for idx, item := range list { + if pathElementMatches(pe, idx, item) { + return item, true + } + } + return nil, false +} + +func pathElementMatches(pe fieldpath.PathElement, idx int, item interface{}) bool { + switch { + case pe.Index != nil: + return idx == *pe.Index + case pe.Value != nil: + return value.Equals(*pe.Value, value.NewValueInterface(item)) + case pe.Key != nil: + m, ok := item.(map[string]interface{}) + if !ok { + return false + } + for _, field := range *pe.Key { + v, ok := m[field.Name] + if !ok || !value.Equals(field.Value, value.NewValueInterface(v)) { + return false + } + } + return true + default: + return false + } +} + +func sameObject(lhs, rhs *unstructured.Unstructured, desired model.K8sLocalObject) (bool, error) { + fieldSet := fieldpath.SetFromValue(value.NewValueInterface(desired.ToUnstructured().Object)).Leaves() + managedSet, err := managedFieldSet(lhs, ssaFieldManager) + if err != nil { + return false, err + } + fieldSet = fieldSet.Union(managedSet) + return value.Equals( + value.NewValueInterface(projectedObject(lhs, fieldSet)), + value.NewValueInterface(projectedObject(rhs, fieldSet)), + ), nil +} + +func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions, operation string) (*updateResult, error) { + obj = stripApplyHistoryAnnotations(obj) + b, err := json.Marshal(obj) if err != nil { return nil, errors.Wrap(err, "json marshal") } @@ -708,8 +864,14 @@ func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, if err != nil { return nil, errors.Wrap(err, "server-side apply") } - if remObj != nil && sameObject(remObj, out) { - return &updateResult{SkipReason: identicalObjects}, nil + if remObj != nil { + same, err := sameObject(remObj, out, obj) + if err != nil { + return nil, err + } + if same { + return &updateResult{SkipReason: identicalObjects}, nil + } } return result, nil } @@ -735,23 +897,8 @@ func (c *Client) maybeUpdate(ctx context.Context, obj model.K8sLocalObject, remO } p := patcher{ - provider: c.resourceInterfaceWithDefaultNs, - cfgProvider: func(obj *unstructured.Unstructured) ([]byte, error) { - pristine, _ := getPristineVersion(obj, false) - if pristine == nil { - p := map[string]interface{}{ - "kind": obj.GetKind(), - "apiVersion": obj.GetAPIVersion(), - "metadata": map[string]interface{}{ - "name": obj.GetName(), - }, - } - pb, _ := json.Marshal(p) - return pb, nil - } - b, _ := json.Marshal(pristine) - return b, nil - }, + provider: c.resourceInterfaceWithDefaultNs, + cfgProvider: pristineBytesForClientSideApply, overwrite: true, backOff: clockwork.NewRealClock(), openAPILookup: lookup, diff --git a/internal/remote/client_ssa_test.go b/internal/remote/client_ssa_test.go index e3102804..bb2f79a1 100644 --- a/internal/remote/client_ssa_test.go +++ b/internal/remote/client_ssa_test.go @@ -150,6 +150,18 @@ func newConfigMap(namespace, name string) model.K8sLocalObject { }, model.LocalAttrs{App: "app", Component: "comp", Env: "env"}) } +func newConfigMapWithData(namespace, name string, data map[string]interface{}) model.K8sLocalObject { + return model.NewK8sLocalObject(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": name, + "namespace": namespace, + }, + "data": data, + }, model.LocalAttrs{App: "app", Component: "comp", Env: "env"}) +} + func newGenerateNameConfigMap(namespace, generateName string) model.K8sLocalObject { return model.NewK8sLocalObject(map[string]interface{}{ "apiVersion": "v1", @@ -289,23 +301,73 @@ func TestMaybeUpdateSecretDryRunBypassesServerSideApply(t *testing.T) { assert.Nil(t, recorder.patchData) } -func TestServerSideApplyStripsPristineAnnotation(t *testing.T) { +func TestMaybeUpdateServerSideApplyIgnoresControllerOwnedChanges(t *testing.T) { existing := newConfigMap("default", "ssa-config").ToUnstructured() - annotated, err := qbecPristine{}.createFromPristine(newConfigMap("default", "ssa-config")) + out := existing.DeepCopy() + require.NoError(t, unstructured.SetNestedField(out.Object, map[string]interface{}{"state": "ready"}, "status")) + annotations := out.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations["controller.example.com/hash"] = "12345" + out.SetAnnotations(annotations) + out.SetFinalizers([]string{"controller.example.com/finalizer"}) + + client, _ := newServerSideApplyClient(t, out) + result, err := client.maybeUpdate(context.Background(), newConfigMap("default", "ssa-config"), existing.DeepCopy(), SyncOptions{ + ApplyStrategy: model.ApplyStrategyServer, + DisableUpdateFn: func(model.K8sMeta) bool { return false }, + }, internalSyncOptions{}) require.NoError(t, err) + assert.Equal(t, identicalObjects, result.SkipReason) +} - client, recorder := newServerSideApplyClient(t, existing.DeepCopy()) - _, err = client.serverSideApply(context.Background(), annotated, existing.DeepCopy(), SyncOptions{}, opUpdate) +func TestMaybeUpdateServerSideApplyDetectsRemovedManagedFields(t *testing.T) { + existing := newConfigMapWithData("default", "ssa-config", map[string]interface{}{ + "foo": "bar", + "stale": "remove-me", + }).ToUnstructured() + existing.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:data":{"f:foo":{},"f:stale":{}}}`), + }, + }, + }) + out := newConfigMap("default", "ssa-config").ToUnstructured() + + client, recorder := newServerSideApplyClient(t, out) + result, err := client.maybeUpdate(context.Background(), newConfigMap("default", "ssa-config"), existing.DeepCopy(), SyncOptions{ + ApplyStrategy: model.ApplyStrategyServer, + DisableUpdateFn: func(model.K8sMeta) bool { return false }, + }, internalSyncOptions{}) require.NoError(t, err) + assert.Empty(t, result.SkipReason) + assert.Equal(t, opUpdate, result.Operation) + assert.Equal(t, apiTypes.ApplyPatchType, recorder.patchType) +} - var patch map[string]interface{} - require.NoError(t, json.Unmarshal(recorder.patchData, &patch)) +func TestServerSideApplyStripsApplyHistoryAnnotations(t *testing.T) { + obj := newConfigMap("default", "ssa-config") + annotated := cloneLocalObject(obj) + annotations := annotated.ToUnstructured().GetAnnotations() + annotations[model.QbecNames.PristineAnnotation] = "pristine" + annotations[kubectlLastConfig] = `{"apiVersion":"v1"}` + annotated.ToUnstructured().SetAnnotations(annotations) + + client, recorder := newServerSideApplyClient(t, annotated.ToUnstructured()) + _, err := client.serverSideApply(context.Background(), annotated, nil, SyncOptions{}, opUpdate) + require.NoError(t, err) - metadata, ok := patch["metadata"].(map[string]interface{}) - require.True(t, ok) - annotations, ok := metadata["annotations"].(map[string]interface{}) - if ok { - _, found := annotations[model.QbecNames.PristineAnnotation] - assert.False(t, found) - } + var payload map[string]interface{} + require.NoError(t, json.Unmarshal(recorder.patchData, &payload)) + gotAnnotations, found, err := unstructured.NestedStringMap(payload, "metadata", "annotations") + require.NoError(t, err) + require.True(t, found) + assert.NotContains(t, gotAnnotations, model.QbecNames.PristineAnnotation) + assert.NotContains(t, gotAnnotations, kubectlLastConfig) + assert.Equal(t, obj.Component(), gotAnnotations[model.QbecNames.ComponentAnnotation]) } diff --git a/internal/remote/pristine.go b/internal/remote/pristine.go index 95052d03..409a34a6 100644 --- a/internal/remote/pristine.go +++ b/internal/remote/pristine.go @@ -161,8 +161,63 @@ func (f fallbackPristine) getPristine(annotations map[string]string, orig *unstr return orig, "fallback - live object with some attributes removed" } +type managedFieldsPristine struct{} + +func objectIdentityBase(obj *unstructured.Unstructured) *unstructured.Unstructured { + base := &unstructured.Unstructured{Object: map[string]interface{}{ + "kind": obj.GetKind(), + "apiVersion": obj.GetAPIVersion(), + "metadata": map[string]interface{}{ + "name": obj.GetName(), + }, + }} + if ns := obj.GetNamespace(); ns != "" { + _ = unstructured.SetNestedField(base.Object, ns, "metadata", "namespace") + } + return base +} + +func pristineFromManagedFields(obj *unstructured.Unstructured, fieldManager string) (*unstructured.Unstructured, error) { + fieldSet, err := managedFieldSet(obj, fieldManager) + if err != nil { + return nil, err + } + if fieldSet.Empty() { + return nil, nil + } + projected := projectedObject(obj, fieldSet) + if projected == nil { + return nil, nil + } + base := objectIdentityBase(obj) + for k, v := range projected { + base.Object[k] = v + } + return base, nil +} + +func pristineBytesForClientSideApply(obj *unstructured.Unstructured) ([]byte, error) { + pristine, _ := getPristineVersion(obj, false) + if pristine == nil { + pristine = objectIdentityBase(obj) + } + return json.Marshal(pristine) +} + +func (m managedFieldsPristine) getPristine(_ map[string]string, obj *unstructured.Unstructured) (*unstructured.Unstructured, string) { + ret, err := pristineFromManagedFields(obj, ssaFieldManager) + if err != nil { + sio.Warnln("unable to read pristine from managed fields", err) + return nil, "" + } + if ret == nil { + return nil, "" + } + return ret, "managed fields" +} + func getPristineVersion(obj *unstructured.Unstructured, includeFallback bool) (*unstructured.Unstructured, string) { - pristineReaders := []pristineReader{qbecPristine{}, kubectlPristine{}} + pristineReaders := []pristineReader{qbecPristine{}, kubectlPristine{}, managedFieldsPristine{}} if includeFallback { pristineReaders = append(pristineReaders, fallbackPristine{}) } diff --git a/internal/remote/pristine_test.go b/internal/remote/pristine_test.go index 351e13c6..a10b6687 100644 --- a/internal/remote/pristine_test.go +++ b/internal/remote/pristine_test.go @@ -18,11 +18,13 @@ import ( "encoding/base64" "io/ioutil" "path/filepath" + "strings" "testing" "github.com/splunk/qbec/internal/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/yaml" ) @@ -222,3 +224,86 @@ func TestCreateFromPristine(t *testing.T) { require.Nil(t, err) a.EqualValues(un.Object, pObj) } + +func TestPristineReaderManagedFields(t *testing.T) { + obj := newConfigMapWithData("default", "ssa-config", map[string]interface{}{ + "foo": "bar", + "stale": "remove-me", + }).ToUnstructured() + obj.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:data":{"f:foo":{},"f:stale":{}}}`), + }, + }, + }) + + pristine, source := getPristineVersion(obj, false) + require.NotNil(t, pristine) + assert.Equal(t, "managed fields", source) + assert.Equal(t, "ssa-config", pristine.GetName()) + assert.Equal(t, "default", pristine.GetNamespace()) + data, found, err := unstructured.NestedStringMap(pristine.Object, "data") + require.NoError(t, err) + require.True(t, found) + assert.Equal(t, "bar", data["foo"]) + assert.Equal(t, "remove-me", data["stale"]) +} + +func TestClientSideApplyUsesManagedFieldsPristineForDeletion(t *testing.T) { + serverObj := newConfigMapWithData("default", "ssa-config", map[string]interface{}{ + "foo": "bar", + "stale": "remove-me", + }).ToUnstructured() + serverObj.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:data":{"f:foo":{},"f:stale":{}}}`), + }, + }, + }) + + p := patcher{cfgProvider: pristineBytesForClientSideApply} + result, err := p.getPatchContents(serverObj, newConfigMap("default", "ssa-config")) + require.NoError(t, err) + assert.Empty(t, result.SkipReason) + assert.Contains(t, string(result.patch), `"stale":null`) +} + +func TestClientServerClientRoundTripNoop(t *testing.T) { + desired := newConfigMap("default", "ssa-config") + clientApplied, err := qbecPristine{}.createFromPristine(desired) + require.NoError(t, err) + + serverObj := desired.ToUnstructured() + serverObj.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:data":{"f:foo":{}},"f:metadata":{"f:annotations":{"f:qbec.io~1component":{}},"f:labels":{"f:qbec.io~1application":{},"f:qbec.io~1environment":{}}}}`), + }, + }, + }) + + sanitized := stripApplyHistoryAnnotations(clientApplied) + assert.NotContains(t, sanitized.GetAnnotations(), model.QbecNames.PristineAnnotation) + assert.NotContains(t, sanitized.GetAnnotations(), kubectlLastConfig) + + p := patcher{cfgProvider: pristineBytesForClientSideApply} + result, err := p.getPatchContents(serverObj, desired) + require.NoError(t, err) + assert.Equal(t, identicalObjects, result.SkipReason) + + pristineBytes, err := pristineBytesForClientSideApply(serverObj) + require.NoError(t, err) + assert.NotContains(t, string(pristineBytes), model.QbecNames.PristineAnnotation) + assert.True(t, strings.Contains(string(pristineBytes), `"name":"ssa-config"`)) +} From 1e4ec24c69ba2256a5cd83533a26df0b1aa7570c Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Thu, 23 Apr 2026 22:39:51 -0600 Subject: [PATCH 07/12] Handle empty SSA collection updates --- internal/remote/client.go | 32 +++++++++++++++++++++++++++++- internal/remote/client_ssa_test.go | 18 +++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/internal/remote/client.go b/internal/remote/client.go index ffc7be1d..5b68316a 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -711,6 +711,35 @@ func managedFieldSet(obj *unstructured.Unstructured, fieldManager string) (*fiel return set, nil } +func emptyCollectionFieldSet(v interface{}) *fieldpath.Set { + set := fieldpath.NewSet() + collectEmptyCollectionPaths(v, nil, set) + return set +} + +func collectEmptyCollectionPaths(v interface{}, path fieldpath.Path, set *fieldpath.Set) { + switch vv := v.(type) { + case map[string]interface{}: + if len(vv) == 0 { + set.Insert(path) + return + } + for k, child := range vv { + fieldName := k + collectEmptyCollectionPaths(child, append(path, fieldpath.PathElement{FieldName: &fieldName}), set) + } + case []interface{}: + if len(vv) == 0 { + set.Insert(path) + return + } + for idx, child := range vv { + index := idx + collectEmptyCollectionPaths(child, append(path, fieldpath.PathElement{Index: &index}), set) + } + } +} + func projectedObject(obj *unstructured.Unstructured, fieldSet *fieldpath.Set) map[string]interface{} { if obj == nil || fieldSet == nil || fieldSet.Empty() { return nil @@ -819,7 +848,8 @@ func pathElementMatches(pe fieldpath.PathElement, idx int, item interface{}) boo } func sameObject(lhs, rhs *unstructured.Unstructured, desired model.K8sLocalObject) (bool, error) { - fieldSet := fieldpath.SetFromValue(value.NewValueInterface(desired.ToUnstructured().Object)).Leaves() + fieldSet := fieldpath.SetFromValue(value.NewValueInterface(desired.ToUnstructured().Object)) + fieldSet = fieldSet.Union(emptyCollectionFieldSet(desired.ToUnstructured().Object)) managedSet, err := managedFieldSet(lhs, ssaFieldManager) if err != nil { return false, err diff --git a/internal/remote/client_ssa_test.go b/internal/remote/client_ssa_test.go index bb2f79a1..92fe9e4f 100644 --- a/internal/remote/client_ssa_test.go +++ b/internal/remote/client_ssa_test.go @@ -350,6 +350,24 @@ func TestMaybeUpdateServerSideApplyDetectsRemovedManagedFields(t *testing.T) { assert.Equal(t, apiTypes.ApplyPatchType, recorder.patchType) } +func TestMaybeUpdateServerSideApplyTreatsEmptyDesiredCollectionAsUpdate(t *testing.T) { + existing := newConfigMapWithData("default", "ssa-config", map[string]interface{}{ + "foo": "bar", + }).ToUnstructured() + desired := newConfigMapWithData("default", "ssa-config", map[string]interface{}{}) + out := desired.ToUnstructured() + + client, recorder := newServerSideApplyClient(t, out) + result, err := client.maybeUpdate(context.Background(), desired, existing.DeepCopy(), SyncOptions{ + ApplyStrategy: model.ApplyStrategyServer, + DisableUpdateFn: func(model.K8sMeta) bool { return false }, + }, internalSyncOptions{}) + require.NoError(t, err) + assert.Empty(t, result.SkipReason) + assert.Equal(t, opUpdate, result.Operation) + assert.Equal(t, apiTypes.ApplyPatchType, recorder.patchType) +} + func TestServerSideApplyStripsApplyHistoryAnnotations(t *testing.T) { obj := newConfigMap("default", "ssa-config") annotated := cloneLocalObject(obj) From ca53881d03ddd4e17ebb972e2224b01e4ba14332 Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Thu, 23 Apr 2026 23:03:26 -0600 Subject: [PATCH 08/12] Preserve managed-field map keys in pristine projection --- internal/remote/client.go | 31 +++++++++++++++++++++---------- internal/remote/pristine_test.go | 31 +++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/internal/remote/client.go b/internal/remote/client.go index 5b68316a..b0722e1d 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -763,28 +763,23 @@ func projectValue(v interface{}, fieldSet *fieldpath.Set) (interface{}, bool) { case map[string]interface{}: out := map[string]interface{}{} fieldSet.Members.Iterate(func(pe fieldpath.PathElement) { - if pe.FieldName == nil { - return - } - if child, ok := vv[*pe.FieldName]; ok { - out[*pe.FieldName] = runtime.DeepCopyJSONValue(child) + fieldName, child, ok := mapItemForPathElement(vv, pe) + if ok { + out[fieldName] = runtime.DeepCopyJSONValue(child) } }) fieldSet.Children.Iterate(func(pe fieldpath.PathElement) { - if pe.FieldName == nil { - return - } childSet, ok := fieldSet.Children.Get(pe) if !ok { return } - child, ok := vv[*pe.FieldName] + fieldName, child, ok := mapItemForPathElement(vv, pe) if !ok { return } projected, ok := projectValue(child, childSet) if ok { - out[*pe.FieldName] = projected + out[fieldName] = projected } }) return out, len(out) > 0 @@ -815,6 +810,22 @@ func projectValue(v interface{}, fieldSet *fieldpath.Set) (interface{}, bool) { } } +func mapItemForPathElement(m map[string]interface{}, pe fieldpath.PathElement) (string, interface{}, bool) { + if pe.FieldName == nil { + return "", nil, false + } + if child, ok := m[*pe.FieldName]; ok { + return *pe.FieldName, child, true + } + unescaped := strings.NewReplacer("~1", "/", "~0", "~").Replace(*pe.FieldName) + if unescaped != *pe.FieldName { + if child, ok := m[unescaped]; ok { + return unescaped, child, true + } + } + return "", nil, false +} + func listItemForPathElement(list []interface{}, pe fieldpath.PathElement) (interface{}, bool) { for idx, item := range list { if pathElementMatches(pe, idx, item) { diff --git a/internal/remote/pristine_test.go b/internal/remote/pristine_test.go index a10b6687..76c8ea78 100644 --- a/internal/remote/pristine_test.go +++ b/internal/remote/pristine_test.go @@ -18,7 +18,6 @@ import ( "encoding/base64" "io/ioutil" "path/filepath" - "strings" "testing" "github.com/splunk/qbec/internal/model" @@ -276,6 +275,35 @@ func TestClientSideApplyUsesManagedFieldsPristineForDeletion(t *testing.T) { assert.Contains(t, string(result.patch), `"stale":null`) } +func TestClientSideApplyUsesManagedFieldsPristineForMetadataDeletion(t *testing.T) { + desired := newConfigMap("default", "ssa-config") + annotations := desired.ToUnstructured().GetAnnotations() + delete(annotations, model.QbecNames.ComponentAnnotation) + desired.ToUnstructured().SetAnnotations(annotations) + + serverObj := newConfigMap("default", "ssa-config").ToUnstructured() + serverObj.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:data":{"f:foo":{}},"f:metadata":{"f:annotations":{"f:qbec.io~1component":{}},"f:labels":{"f:qbec.io~1application":{},"f:qbec.io~1environment":{}}}}`), + }, + }, + }) + + p := patcher{cfgProvider: pristineBytesForClientSideApply} + result, err := p.getPatchContents(serverObj, desired) + require.NoError(t, err) + assert.Empty(t, result.SkipReason) + assert.Contains(t, string(result.patch), `"qbec.io/component":null`) + + pristineBytes, err := pristineBytesForClientSideApply(serverObj) + require.NoError(t, err) + assert.Contains(t, string(pristineBytes), `"qbec.io/component"`) +} + func TestClientServerClientRoundTripNoop(t *testing.T) { desired := newConfigMap("default", "ssa-config") clientApplied, err := qbecPristine{}.createFromPristine(desired) @@ -305,5 +333,4 @@ func TestClientServerClientRoundTripNoop(t *testing.T) { pristineBytes, err := pristineBytesForClientSideApply(serverObj) require.NoError(t, err) assert.NotContains(t, string(pristineBytes), model.QbecNames.PristineAnnotation) - assert.True(t, strings.Contains(string(pristineBytes), `"name":"ssa-config"`)) } From 85bb4c37fc73b5b00f55731778518e52aa829886 Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Fri, 24 Apr 2026 10:56:32 -0600 Subject: [PATCH 09/12] Fix SSA and client apply migration cleanup --- internal/remote/client.go | 39 +++++++++++++++++++++++++++++- internal/remote/client_ssa_test.go | 34 ++++++++++++++++++++++++++ internal/remote/pristine.go | 36 +++++++++++++++++++++++++-- internal/remote/pristine_test.go | 32 ++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 3 deletions(-) diff --git a/internal/remote/client.go b/internal/remote/client.go index b0722e1d..9f082083 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -690,6 +690,31 @@ func stripApplyHistoryAnnotations(obj model.K8sLocalObject) model.K8sLocalObject return ret } +func markLegacyApplyAnnotationsForDeletion(obj model.K8sLocalObject, remObj *unstructured.Unstructured) model.K8sLocalObject { + if !hasLegacyClientSideApplyState(remObj) { + return obj + } + ret := cloneLocalObject(obj) + metadata, found, err := unstructured.NestedMap(ret.ToUnstructured().Object, "metadata") + if err != nil || !found { + metadata = map[string]interface{}{} + } + annotations, found, err := unstructured.NestedMap(metadata, "annotations") + if err != nil || !found { + annotations = map[string]interface{}{} + } + remoteAnnotations := remObj.GetAnnotations() + if remoteAnnotations[model.QbecNames.PristineAnnotation] != "" { + annotations[model.QbecNames.PristineAnnotation] = nil + } + if remoteAnnotations[kubectlLastConfig] != "" { + annotations[kubectlLastConfig] = nil + } + metadata["annotations"] = annotations + ret.ToUnstructured().Object["metadata"] = metadata + return ret +} + func managedFieldSet(obj *unstructured.Unstructured, fieldManager string) (*fieldpath.Set, error) { set := fieldpath.NewSet() if obj == nil { @@ -872,8 +897,20 @@ func sameObject(lhs, rhs *unstructured.Unstructured, desired model.K8sLocalObjec ), nil } +func hasLegacyClientSideApplyState(obj *unstructured.Unstructured) bool { + if obj == nil { + return false + } + annotations := obj.GetAnnotations() + if len(annotations) == 0 { + return false + } + return annotations[model.QbecNames.PristineAnnotation] != "" || annotations[kubectlLastConfig] != "" +} + func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions, operation string) (*updateResult, error) { obj = stripApplyHistoryAnnotations(obj) + obj = markLegacyApplyAnnotationsForDeletion(obj, remObj) b, err := json.Marshal(obj) if err != nil { return nil, errors.Wrap(err, "json marshal") @@ -897,7 +934,7 @@ func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, if opts.DryRun { patchOpts.DryRun = []string{metav1.DryRunAll} } - if opts.ForceConflicts { + if opts.ForceConflicts || hasLegacyClientSideApplyState(remObj) { force := true patchOpts.Force = &force } diff --git a/internal/remote/client_ssa_test.go b/internal/remote/client_ssa_test.go index 92fe9e4f..78bfec12 100644 --- a/internal/remote/client_ssa_test.go +++ b/internal/remote/client_ssa_test.go @@ -350,6 +350,40 @@ func TestMaybeUpdateServerSideApplyDetectsRemovedManagedFields(t *testing.T) { assert.Equal(t, apiTypes.ApplyPatchType, recorder.patchType) } +func TestMaybeUpdateServerSideApplyForcesLegacyClientSideMigration(t *testing.T) { + existing := newConfigMap("default", "ssa-config").ToUnstructured() + annotations := existing.GetAnnotations() + annotations[model.QbecNames.PristineAnnotation] = "pristine" + existing.SetAnnotations(annotations) + + client, recorder := newServerSideApplyClient(t, existing.DeepCopy()) + result, err := client.maybeUpdate(context.Background(), newConfigMap("default", "ssa-config"), existing.DeepCopy(), SyncOptions{ + ApplyStrategy: model.ApplyStrategyServer, + DisableUpdateFn: func(model.K8sMeta) bool { return false }, + }, internalSyncOptions{}) + require.NoError(t, err) + assert.Equal(t, identicalObjects, result.SkipReason) + require.NotNil(t, recorder.patchOptions.Force) + assert.True(t, *recorder.patchOptions.Force) +} + +func TestMaybeUpdateServerSideApplyForcesKubectlMigration(t *testing.T) { + existing := newConfigMap("default", "ssa-config").ToUnstructured() + annotations := existing.GetAnnotations() + annotations[kubectlLastConfig] = `{"apiVersion":"v1"}` + existing.SetAnnotations(annotations) + + client, recorder := newServerSideApplyClient(t, existing.DeepCopy()) + result, err := client.maybeUpdate(context.Background(), newConfigMap("default", "ssa-config"), existing.DeepCopy(), SyncOptions{ + ApplyStrategy: model.ApplyStrategyServer, + DisableUpdateFn: func(model.K8sMeta) bool { return false }, + }, internalSyncOptions{}) + require.NoError(t, err) + assert.Equal(t, identicalObjects, result.SkipReason) + require.NotNil(t, recorder.patchOptions.Force) + assert.True(t, *recorder.patchOptions.Force) +} + func TestMaybeUpdateServerSideApplyTreatsEmptyDesiredCollectionAsUpdate(t *testing.T) { existing := newConfigMapWithData("default", "ssa-config", map[string]interface{}{ "foo": "bar", diff --git a/internal/remote/pristine.go b/internal/remote/pristine.go index 409a34a6..4bc56686 100644 --- a/internal/remote/pristine.go +++ b/internal/remote/pristine.go @@ -19,6 +19,7 @@ import ( "compress/gzip" "encoding/base64" "encoding/json" + "strings" "github.com/pkg/errors" "github.com/splunk/qbec/internal/model" @@ -196,11 +197,42 @@ func pristineFromManagedFields(obj *unstructured.Unstructured, fieldManager stri return base, nil } -func pristineBytesForClientSideApply(obj *unstructured.Unstructured) ([]byte, error) { - pristine, _ := getPristineVersion(obj, false) +func mergeQbecMetadata(pristine, obj *unstructured.Unstructured) *unstructured.Unstructured { if pristine == nil { pristine = objectIdentityBase(obj) } + + annotations := pristine.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + for k, v := range obj.GetAnnotations() { + if strings.HasPrefix(k, model.QBECMetadataPrefix) || strings.HasPrefix(k, model.QBECDirectivesNamespace) { + annotations[k] = v + } + } + if len(annotations) > 0 { + pristine.SetAnnotations(annotations) + } + + labels := pristine.GetLabels() + if labels == nil { + labels = map[string]string{} + } + for k, v := range obj.GetLabels() { + if strings.HasPrefix(k, model.QBECMetadataPrefix) { + labels[k] = v + } + } + if len(labels) > 0 { + pristine.SetLabels(labels) + } + return pristine +} + +func pristineBytesForClientSideApply(obj *unstructured.Unstructured) ([]byte, error) { + pristine, _ := getPristineVersion(obj, false) + pristine = mergeQbecMetadata(pristine, obj) return json.Marshal(pristine) } diff --git a/internal/remote/pristine_test.go b/internal/remote/pristine_test.go index 76c8ea78..dec03e8f 100644 --- a/internal/remote/pristine_test.go +++ b/internal/remote/pristine_test.go @@ -304,6 +304,38 @@ func TestClientSideApplyUsesManagedFieldsPristineForMetadataDeletion(t *testing. assert.Contains(t, string(pristineBytes), `"qbec.io/component"`) } +func TestClientSideApplyUsesManagedFieldsPristineForDirectiveDeletion(t *testing.T) { + desired := newConfigMap("default", "ssa-config") + annotations := desired.ToUnstructured().GetAnnotations() + delete(annotations, model.QbecNames.Directives.ApplyStrategy) + desired.ToUnstructured().SetAnnotations(annotations) + + serverObj := newConfigMap("default", "ssa-config").ToUnstructured() + annotations = serverObj.GetAnnotations() + annotations[model.QbecNames.Directives.ApplyStrategy] = string(model.ApplyStrategyServer) + serverObj.SetAnnotations(annotations) + serverObj.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:data":{"f:foo":{}},"f:metadata":{"f:annotations":{"f:qbec.io~1component":{}},"f:labels":{"f:qbec.io~1application":{},"f:qbec.io~1environment":{}}}}`), + }, + }, + }) + + p := patcher{cfgProvider: pristineBytesForClientSideApply} + result, err := p.getPatchContents(serverObj, desired) + require.NoError(t, err) + assert.Empty(t, result.SkipReason) + assert.Contains(t, string(result.patch), `"directives.qbec.io/apply-strategy":null`) + + pristineBytes, err := pristineBytesForClientSideApply(serverObj) + require.NoError(t, err) + assert.Contains(t, string(pristineBytes), `"directives.qbec.io/apply-strategy":"server"`) +} + func TestClientServerClientRoundTripNoop(t *testing.T) { desired := newConfigMap("default", "ssa-config") clientApplied, err := qbecPristine{}.createFromPristine(desired) From 65ea2242e3303e4fa5d3c50848f7413dc5b01aaa Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Fri, 24 Apr 2026 13:15:10 -0600 Subject: [PATCH 10/12] Fix client-side apply migration regressions --- internal/remote/client.go | 15 ++++++ internal/remote/pristine.go | 16 +++++- internal/remote/pristine_test.go | 93 ++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 1 deletion(-) diff --git a/internal/remote/client.go b/internal/remote/client.go index 9f082083..62d69fc2 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -826,6 +826,7 @@ func projectValue(v interface{}, fieldSet *fieldpath.Set) (interface{}, bool) { } projected, ok := projectValue(child, childSet) if ok { + projected = withListItemKeys(projected, pe) out = append(out, projected) } }) @@ -835,6 +836,20 @@ func projectValue(v interface{}, fieldSet *fieldpath.Set) (interface{}, bool) { } } +func withListItemKeys(projected interface{}, pe fieldpath.PathElement) interface{} { + if pe.Key == nil { + return projected + } + m, ok := projected.(map[string]interface{}) + if !ok { + return projected + } + for _, field := range *pe.Key { + m[field.Name] = runtime.DeepCopyJSONValue(field.Value.Unstructured()) + } + return m +} + func mapItemForPathElement(m map[string]interface{}, pe fieldpath.PathElement) (string, interface{}, bool) { if pe.FieldName == nil { return "", nil, false diff --git a/internal/remote/pristine.go b/internal/remote/pristine.go index 4bc56686..798cd968 100644 --- a/internal/remote/pristine.go +++ b/internal/remote/pristine.go @@ -197,6 +197,20 @@ func pristineFromManagedFields(obj *unstructured.Unstructured, fieldManager stri return base, nil } +func includeInClientSidePristineAnnotation(name string) bool { + if strings.HasPrefix(name, model.QBECMetadataPrefix) { + return true + } + switch name { + case model.QbecNames.Directives.ApplyOrder, + model.QbecNames.Directives.ApplyStrategy, + model.QbecNames.Directives.WaitPolicy: + return true + default: + return false + } +} + func mergeQbecMetadata(pristine, obj *unstructured.Unstructured) *unstructured.Unstructured { if pristine == nil { pristine = objectIdentityBase(obj) @@ -207,7 +221,7 @@ func mergeQbecMetadata(pristine, obj *unstructured.Unstructured) *unstructured.U annotations = map[string]string{} } for k, v := range obj.GetAnnotations() { - if strings.HasPrefix(k, model.QBECMetadataPrefix) || strings.HasPrefix(k, model.QBECDirectivesNamespace) { + if includeInClientSidePristineAnnotation(k) { annotations[k] = v } } diff --git a/internal/remote/pristine_test.go b/internal/remote/pristine_test.go index dec03e8f..35fe62e4 100644 --- a/internal/remote/pristine_test.go +++ b/internal/remote/pristine_test.go @@ -336,6 +336,66 @@ func TestClientSideApplyUsesManagedFieldsPristineForDirectiveDeletion(t *testing assert.Contains(t, string(pristineBytes), `"directives.qbec.io/apply-strategy":"server"`) } +func TestClientSideApplyPristineSkipsInClusterDirectives(t *testing.T) { + desired := newConfigMap("default", "ssa-config") + serverObj := desired.ToUnstructured().DeepCopy() + annotations := serverObj.GetAnnotations() + annotations[model.QbecNames.Directives.DeletePolicy] = "never" + annotations[model.QbecNames.Directives.UpdatePolicy] = "never" + serverObj.SetAnnotations(annotations) + serverObj.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:data":{"f:foo":{}}}`), + }, + }, + }) + + pristineBytes, err := pristineBytesForClientSideApply(serverObj) + require.NoError(t, err) + assert.NotContains(t, string(pristineBytes), model.QbecNames.Directives.DeletePolicy) + assert.NotContains(t, string(pristineBytes), model.QbecNames.Directives.UpdatePolicy) + + p := patcher{cfgProvider: pristineBytesForClientSideApply} + result, err := p.getPatchContents(serverObj, desired) + require.NoError(t, err) + assert.NotContains(t, string(result.patch), `"directives.qbec.io/delete-policy":null`) + assert.NotContains(t, string(result.patch), `"directives.qbec.io/update-policy":null`) +} + +func TestManagedFieldsPristinePreservesAssociativeListKeys(t *testing.T) { + desired := newDeployment("default", "ssa-deployment") + serverObj := desired.ToUnstructured().DeepCopy() + serverObj.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:spec":{"f:template":{"f:spec":{"f:containers":{"k:{\"name\":\"app\"}":{"f:image":{}}}}}}}`), + }, + }, + }) + + pristine, err := pristineFromManagedFields(serverObj, ssaFieldManager) + require.NoError(t, err) + containers, found, err := unstructured.NestedSlice(pristine.Object, "spec", "template", "spec", "containers") + require.NoError(t, err) + require.True(t, found) + require.Len(t, containers, 1) + container, ok := containers[0].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "app", container["name"]) + assert.Equal(t, "nginx", container["image"]) + + p := patcher{cfgProvider: pristineBytesForClientSideApply} + _, err = p.getPatchContents(serverObj, desired) + require.NoError(t, err) +} + func TestClientServerClientRoundTripNoop(t *testing.T) { desired := newConfigMap("default", "ssa-config") clientApplied, err := qbecPristine{}.createFromPristine(desired) @@ -366,3 +426,36 @@ func TestClientServerClientRoundTripNoop(t *testing.T) { require.NoError(t, err) assert.NotContains(t, string(pristineBytes), model.QbecNames.PristineAnnotation) } + +func newDeployment(namespace, name string) model.K8sLocalObject { + return model.NewK8sLocalObject(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": name, + "namespace": namespace, + }, + "spec": map[string]interface{}{ + "selector": map[string]interface{}{ + "matchLabels": map[string]interface{}{ + "app": "demo", + }, + }, + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "app": "demo", + }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "app", + "image": "nginx", + }, + }, + }, + }, + }, + }, model.LocalAttrs{App: "app", Component: "comp", Env: "env"}) +} From 955922fd809b5e4d90ce7970baf16a59abfe159d Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Fri, 24 Apr 2026 13:34:21 -0600 Subject: [PATCH 11/12] Fix apply migration deletion regressions --- internal/remote/client.go | 61 ++++++++++++++++++++++++++++- internal/remote/client_ssa_test.go | 62 ++++++++++++++++++++++++++++-- internal/remote/pristine.go | 6 +-- internal/remote/pristine_test.go | 48 ++++++++++++++++++++++- 4 files changed, 166 insertions(+), 11 deletions(-) diff --git a/internal/remote/client.go b/internal/remote/client.go index 62d69fc2..2ceccb72 100644 --- a/internal/remote/client.go +++ b/internal/remote/client.go @@ -715,6 +715,57 @@ func markLegacyApplyAnnotationsForDeletion(obj model.K8sLocalObject, remObj *uns return ret } +func preserveLegacyApplyAnnotations(obj model.K8sLocalObject, remObj *unstructured.Unstructured) model.K8sLocalObject { + if !hasLegacyClientSideApplyState(remObj) { + return obj + } + ret := cloneLocalObject(obj) + metadata, found, err := unstructured.NestedMap(ret.ToUnstructured().Object, "metadata") + if err != nil || !found { + metadata = map[string]interface{}{} + } + annotations, found, err := unstructured.NestedMap(metadata, "annotations") + if err != nil || !found { + annotations = map[string]interface{}{} + } + remoteAnnotations := remObj.GetAnnotations() + if v := remoteAnnotations[model.QbecNames.PristineAnnotation]; v != "" { + annotations[model.QbecNames.PristineAnnotation] = v + } + if v := remoteAnnotations[kubectlLastConfig]; v != "" { + annotations[kubectlLastConfig] = v + } + metadata["annotations"] = annotations + ret.ToUnstructured().Object["metadata"] = metadata + return ret +} + +func (c *Client) legacyClientSideApplyMigrationPatch(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions) (*updateResult, error) { + if !hasLegacyClientSideApplyState(remObj) { + return nil, nil + } + res, err := c.schema.OpenAPIResources() + if err != nil { + sio.Warnln("get open API resources", err) + } + var lookup openAPILookup + if res != nil { + lookup = res.LookupResource + } + + p := patcher{ + provider: c.resourceInterfaceWithDefaultNs, + cfgProvider: pristineBytesForClientSideApply, + overwrite: true, + backOff: clockwork.NewRealClock(), + openAPILookup: lookup, + } + if opts.DryRun { + return p.getPatchContents(remObj, obj) + } + return p.patch(ctx, remObj, obj) +} + func managedFieldSet(obj *unstructured.Unstructured, fieldManager string) (*fieldpath.Set, error) { set := fieldpath.NewSet() if obj == nil { @@ -925,6 +976,14 @@ func hasLegacyClientSideApplyState(obj *unstructured.Unstructured) bool { func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, remObj *unstructured.Unstructured, opts SyncOptions, operation string) (*updateResult, error) { obj = stripApplyHistoryAnnotations(obj) + migrationResult, err := c.legacyClientSideApplyMigrationPatch(ctx, preserveLegacyApplyAnnotations(obj, remObj), remObj, opts) + if err != nil { + return nil, errors.Wrap(err, "legacy client-side apply migration") + } + migrationChanged := migrationResult != nil && migrationResult.SkipReason == "" + if opts.DryRun && migrationChanged { + return migrationResult, nil + } obj = markLegacyApplyAnnotationsForDeletion(obj, remObj) b, err := json.Marshal(obj) if err != nil { @@ -962,7 +1021,7 @@ func (c *Client) serverSideApply(ctx context.Context, obj model.K8sLocalObject, if err != nil { return nil, err } - if same { + if same && !migrationChanged { return &updateResult{SkipReason: identicalObjects}, nil } } diff --git a/internal/remote/client_ssa_test.go b/internal/remote/client_ssa_test.go index 78bfec12..4cc02e14 100644 --- a/internal/remote/client_ssa_test.go +++ b/internal/remote/client_ssa_test.go @@ -68,6 +68,13 @@ func (r recorderDynamic) Resource(resource schema.GroupVersionResource) dynamic. return r.resource } +type recordedPatch struct { + name string + kind apiTypes.PatchType + data []byte + options metav1.PatchOptions +} + type recorderResource struct { gvr schema.GroupVersionResource namespace string @@ -75,6 +82,7 @@ type recorderResource struct { patchType apiTypes.PatchType patchData []byte patchOptions metav1.PatchOptions + patchCalls []recordedPatch patchResponse *unstructured.Unstructured createObject *unstructured.Unstructured } @@ -122,6 +130,12 @@ func (r *recorderResource) Patch(ctx context.Context, name string, pt apiTypes.P r.patchType = pt r.patchData = append([]byte(nil), data...) r.patchOptions = *options.DeepCopy() + r.patchCalls = append(r.patchCalls, recordedPatch{ + name: name, + kind: pt, + data: append([]byte(nil), data...), + options: *options.DeepCopy(), + }) if r.patchResponse != nil { return r.patchResponse.DeepCopy(), nil } @@ -162,6 +176,12 @@ func newConfigMapWithData(namespace, name string, data map[string]interface{}) m }, model.LocalAttrs{App: "app", Component: "comp", Env: "env"}) } +func newConfigMapWithoutNamespace(name string) model.K8sLocalObject { + obj := newConfigMap("", name) + unstructured.RemoveNestedField(obj.ToUnstructured().Object, "metadata", "namespace") + return obj +} + func newGenerateNameConfigMap(namespace, generateName string) model.K8sLocalObject { return model.NewK8sLocalObject(map[string]interface{}{ "apiVersion": "v1", @@ -356,13 +376,14 @@ func TestMaybeUpdateServerSideApplyForcesLegacyClientSideMigration(t *testing.T) annotations[model.QbecNames.PristineAnnotation] = "pristine" existing.SetAnnotations(annotations) - client, recorder := newServerSideApplyClient(t, existing.DeepCopy()) + out := newConfigMap("default", "ssa-config").ToUnstructured() + client, recorder := newServerSideApplyClient(t, out) result, err := client.maybeUpdate(context.Background(), newConfigMap("default", "ssa-config"), existing.DeepCopy(), SyncOptions{ ApplyStrategy: model.ApplyStrategyServer, DisableUpdateFn: func(model.K8sMeta) bool { return false }, }, internalSyncOptions{}) require.NoError(t, err) - assert.Equal(t, identicalObjects, result.SkipReason) + assert.Equal(t, opUpdate, result.Operation) require.NotNil(t, recorder.patchOptions.Force) assert.True(t, *recorder.patchOptions.Force) } @@ -373,17 +394,50 @@ func TestMaybeUpdateServerSideApplyForcesKubectlMigration(t *testing.T) { annotations[kubectlLastConfig] = `{"apiVersion":"v1"}` existing.SetAnnotations(annotations) - client, recorder := newServerSideApplyClient(t, existing.DeepCopy()) + out := newConfigMap("default", "ssa-config").ToUnstructured() + client, recorder := newServerSideApplyClient(t, out) result, err := client.maybeUpdate(context.Background(), newConfigMap("default", "ssa-config"), existing.DeepCopy(), SyncOptions{ ApplyStrategy: model.ApplyStrategyServer, DisableUpdateFn: func(model.K8sMeta) bool { return false }, }, internalSyncOptions{}) require.NoError(t, err) - assert.Equal(t, identicalObjects, result.SkipReason) + assert.Equal(t, opUpdate, result.Operation) require.NotNil(t, recorder.patchOptions.Force) assert.True(t, *recorder.patchOptions.Force) } +func TestMaybeUpdateServerSideApplyCleansLegacyOwnedFields(t *testing.T) { + legacy := newConfigMapWithData("default", "ssa-config", map[string]interface{}{ + "foo": "bar", + "stale": "remove-me", + }) + clientApplied, err := qbecPristine{}.createFromPristine(legacy) + require.NoError(t, err) + existing := legacy.ToUnstructured().DeepCopy() + existing.SetAnnotations(clientApplied.ToUnstructured().GetAnnotations()) + out := newConfigMap("default", "ssa-config").ToUnstructured() + + client, recorder := newServerSideApplyClient(t, out) + result, err := client.maybeUpdate(context.Background(), newConfigMap("default", "ssa-config"), existing.DeepCopy(), SyncOptions{ + ApplyStrategy: model.ApplyStrategyServer, + DisableUpdateFn: func(model.K8sMeta) bool { return false }, + }, internalSyncOptions{}) + require.NoError(t, err) + assert.Equal(t, opUpdate, result.Operation) + require.Len(t, recorder.patchCalls, 2) + + cleanup := recorder.patchCalls[0] + assert.NotEqual(t, apiTypes.ApplyPatchType, cleanup.kind) + assert.Contains(t, string(cleanup.data), `"stale":null`) + assert.NotContains(t, string(cleanup.data), `"`+model.QbecNames.PristineAnnotation+`":null`) + + ssa := recorder.patchCalls[1] + assert.Equal(t, apiTypes.ApplyPatchType, ssa.kind) + assert.Contains(t, string(ssa.data), `"`+model.QbecNames.PristineAnnotation+`":null`) + require.NotNil(t, ssa.options.Force) + assert.True(t, *ssa.options.Force) +} + func TestMaybeUpdateServerSideApplyTreatsEmptyDesiredCollectionAsUpdate(t *testing.T) { existing := newConfigMapWithData("default", "ssa-config", map[string]interface{}{ "foo": "bar", diff --git a/internal/remote/pristine.go b/internal/remote/pristine.go index 798cd968..552a5a4c 100644 --- a/internal/remote/pristine.go +++ b/internal/remote/pristine.go @@ -165,17 +165,13 @@ func (f fallbackPristine) getPristine(annotations map[string]string, orig *unstr type managedFieldsPristine struct{} func objectIdentityBase(obj *unstructured.Unstructured) *unstructured.Unstructured { - base := &unstructured.Unstructured{Object: map[string]interface{}{ + return &unstructured.Unstructured{Object: map[string]interface{}{ "kind": obj.GetKind(), "apiVersion": obj.GetAPIVersion(), "metadata": map[string]interface{}{ "name": obj.GetName(), }, }} - if ns := obj.GetNamespace(); ns != "" { - _ = unstructured.SetNestedField(base.Object, ns, "metadata", "namespace") - } - return base } func pristineFromManagedFields(obj *unstructured.Unstructured, fieldManager string) (*unstructured.Unstructured, error) { diff --git a/internal/remote/pristine_test.go b/internal/remote/pristine_test.go index 35fe62e4..989d90e3 100644 --- a/internal/remote/pristine_test.go +++ b/internal/remote/pristine_test.go @@ -244,7 +244,7 @@ func TestPristineReaderManagedFields(t *testing.T) { require.NotNil(t, pristine) assert.Equal(t, "managed fields", source) assert.Equal(t, "ssa-config", pristine.GetName()) - assert.Equal(t, "default", pristine.GetNamespace()) + assert.Empty(t, pristine.GetNamespace()) data, found, err := unstructured.NestedStringMap(pristine.Object, "data") require.NoError(t, err) require.True(t, found) @@ -366,6 +366,52 @@ func TestClientSideApplyPristineSkipsInClusterDirectives(t *testing.T) { assert.NotContains(t, string(result.patch), `"directives.qbec.io/update-policy":null`) } +func TestClientSideApplySyntheticPristineOmitsLiveNamespace(t *testing.T) { + tests := []struct { + name string + mod func(*unstructured.Unstructured) + }{ + { + name: "without managed fields", + }, + { + name: "from managed fields", + mod: func(serverObj *unstructured.Unstructured) { + serverObj.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:data":{"f:foo":{}}}`), + }, + }, + }) + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + desired := newConfigMapWithoutNamespace("ssa-config") + serverObj := newConfigMap("default", "ssa-config").ToUnstructured() + if test.mod != nil { + test.mod(serverObj) + } + + pristineBytes, err := pristineBytesForClientSideApply(serverObj) + require.NoError(t, err) + assert.NotContains(t, string(pristineBytes), `"namespace"`) + + p := patcher{cfgProvider: pristineBytesForClientSideApply} + result, err := p.getPatchContents(serverObj, desired) + require.NoError(t, err) + assert.Equal(t, identicalObjects, result.SkipReason) + assert.NotContains(t, string(result.patch), `"namespace":null`) + }) + } +} + func TestManagedFieldsPristinePreservesAssociativeListKeys(t *testing.T) { desired := newDeployment("default", "ssa-deployment") serverObj := desired.ToUnstructured().DeepCopy() From 74cb10d4b8f157436b776050660ffc8c0ea3a71d Mon Sep 17 00:00:00 2001 From: Michael Burt Date: Fri, 24 Apr 2026 14:00:07 -0600 Subject: [PATCH 12/12] Preserve metadata identity in managed-fields pristine --- internal/remote/pristine.go | 9 ++++++++ internal/remote/pristine_test.go | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/internal/remote/pristine.go b/internal/remote/pristine.go index 552a5a4c..ed9cebbd 100644 --- a/internal/remote/pristine.go +++ b/internal/remote/pristine.go @@ -188,6 +188,15 @@ func pristineFromManagedFields(obj *unstructured.Unstructured, fieldManager stri } base := objectIdentityBase(obj) for k, v := range projected { + if k == "metadata" { + if projectedMetadata, ok := v.(map[string]interface{}); ok { + metadata, _ := base.Object["metadata"].(map[string]interface{}) + for metadataKey, metadataValue := range projectedMetadata { + metadata[metadataKey] = metadataValue + } + continue + } + } base.Object[k] = v } return base, nil diff --git a/internal/remote/pristine_test.go b/internal/remote/pristine_test.go index 989d90e3..a88f8af4 100644 --- a/internal/remote/pristine_test.go +++ b/internal/remote/pristine_test.go @@ -252,6 +252,42 @@ func TestPristineReaderManagedFields(t *testing.T) { assert.Equal(t, "remove-me", data["stale"]) } +func TestManagedFieldsPristinePreservesIdentityWhenProjectingMetadata(t *testing.T) { + desired := stripApplyHistoryAnnotations(newConfigMap("default", "ssa-config")) + annotations := desired.ToUnstructured().GetAnnotations() + annotations[model.QbecNames.Directives.ApplyStrategy] = string(model.ApplyStrategyServer) + desired.ToUnstructured().SetAnnotations(annotations) + + serverObj := desired.ToUnstructured().DeepCopy() + serverObj.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: ssaFieldManager, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{ + Raw: []byte(`{"f:data":{"f:foo":{}},"f:metadata":{"f:annotations":{"f:directives.qbec.io~1apply-strategy":{},"f:qbec.io~1component":{}},"f:labels":{"f:qbec.io~1application":{},"f:qbec.io~1environment":{}}}}`), + }, + }, + }) + + pristine, err := pristineFromManagedFields(serverObj, ssaFieldManager) + require.NoError(t, err) + require.NotNil(t, pristine) + assert.Equal(t, "ssa-config", pristine.GetName()) + assert.Equal(t, "ConfigMap", pristine.GetKind()) + assert.Equal(t, "v1", pristine.GetAPIVersion()) + assert.Empty(t, pristine.GetNamespace()) + assert.Equal(t, "comp", pristine.GetAnnotations()[model.QbecNames.ComponentAnnotation]) + assert.Equal(t, string(model.ApplyStrategyServer), pristine.GetAnnotations()[model.QbecNames.Directives.ApplyStrategy]) + assert.Equal(t, "app", pristine.GetLabels()[model.QbecNames.ApplicationLabel]) + assert.Equal(t, "env", pristine.GetLabels()[model.QbecNames.EnvironmentLabel]) + + p := patcher{cfgProvider: pristineBytesForClientSideApply} + result, err := p.getPatchContents(serverObj, desired) + require.NoError(t, err) + assert.Equal(t, identicalObjects, result.SkipReason) +} + func TestClientSideApplyUsesManagedFieldsPristineForDeletion(t *testing.T) { serverObj := newConfigMapWithData("default", "ssa-config", map[string]interface{}{ "foo": "bar",