From 30add93b12269ccde8c41832fa7d1f98e90643ca Mon Sep 17 00:00:00 2001 From: Gaurish Chaddha <20123938+gaurishchaddha@users.noreply.github.com> Date: Thu, 30 Jul 2026 12:17:42 +0530 Subject: [PATCH] feat(action): support field validation across release actions Expose FieldValidationDirective on install and rollback in addition to upgrade. Forward the configured directive through install create and update paths, upgrade updates, and rollback updates. Respect the directive for server-side apply and force-replace operations. Add action-level tests covering configured and unset directives, and kube-level tests verifying the fieldValidation query parameter. Signed-off-by: Gaurish Chaddha <20123938+gaurishchaddha@users.noreply.github.com> --- pkg/action/install.go | 24 +++++-- pkg/action/install_test.go | 56 ++++++++++++++++ pkg/action/rollback.go | 15 +++-- pkg/action/rollback_test.go | 42 ++++++++++++ pkg/action/upgrade.go | 16 ++++- pkg/action/upgrade_test.go | 31 +++++++++ pkg/kube/client_test.go | 96 ++++++++++++++++++++++++++++ pkg/kube/fake/failing_kube_client.go | 12 +++- 8 files changed, 277 insertions(+), 15 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 605c423bc..5637ca35a 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -80,6 +80,9 @@ type Install struct { // // This should be used with caution. ForceReplace bool + // FieldValidationDirective, when non-empty, overrides the Kubernetes client's + // default field validation directive for create and update operations. + FieldValidationDirective kube.FieldValidationDirective // ForceConflicts causes server-side apply to force conflicts ("Overwrite value, become sole manager") // see: https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts ForceConflicts bool @@ -516,18 +519,25 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource // do an update, but it's not clear whether we WANT to do an update if the reuse is set // to true, since that is basically an upgrade operation. if len(toBeAdopted) == 0 && len(resources) > 0 { - _, err = i.cfg.KubeClient.Create( - resources, - kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false)) + createOpts := []kube.ClientCreateOption{ + kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false), + } + if i.FieldValidationDirective != "" { + createOpts = append(createOpts, kube.ClientCreateOptionFieldValidationDirective(i.FieldValidationDirective)) + } + _, err = i.cfg.KubeClient.Create(resources, createOpts...) } else if len(resources) > 0 { updateThreeWayMergeForUnstructured := i.TakeOwnership && !i.ServerSideApply // Use three-way merge when taking ownership (and not using server-side apply) - _, err = i.cfg.KubeClient.Update( - toBeAdopted, - resources, + updateOpts := []kube.ClientUpdateOption{ kube.ClientUpdateOptionForceReplace(i.ForceReplace), kube.ClientUpdateOptionServerSideApply(i.ServerSideApply, i.ForceConflicts), kube.ClientUpdateOptionThreeWayMergeForUnstructured(updateThreeWayMergeForUnstructured), - kube.ClientUpdateOptionUpgradeClientSideFieldManager(true)) + kube.ClientUpdateOptionUpgradeClientSideFieldManager(true), + } + if i.FieldValidationDirective != "" { + updateOpts = append(updateOpts, kube.ClientUpdateOptionFieldValidationDirective(i.FieldValidationDirective)) + } + _, err = i.cfg.KubeClient.Update(toBeAdopted, resources, updateOpts...) } if err != nil { return rel, err diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 2d83abe27..89c046b44 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -177,6 +177,62 @@ func installAction(t *testing.T) *Install { return instAction } +func TestInstallFieldValidationDirectiveOptions(t *testing.T) { + tests := []struct { + name string + directive kube.FieldValidationDirective + toBeAdopted kube.ResourceList + expectedCreateOpts int + expectedUpdateOpts int + }{ + { + name: "create with empty directive", + expectedCreateOpts: 1, + }, + { + name: "create with warn directive", + directive: kube.FieldValidationDirectiveWarn, + expectedCreateOpts: 2, + }, + { + name: "update with empty directive", + toBeAdopted: createDummyResourceList(true), + expectedUpdateOpts: 4, + }, + { + name: "update with warn directive", + directive: kube.FieldValidationDirectiveWarn, + toBeAdopted: createDummyResourceList(true), + expectedUpdateOpts: 5, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := actionConfigFixture(t) + client := NewInstall(config) + client.DisableHooks = true + client.FieldValidationDirective = tt.directive + + resources := createDummyResourceList(true) + rel := releaseStub() + rel.Hooks = nil + + _, err := client.performInstall(rel, tt.toBeAdopted, resources) + require.NoError(t, err) + + failer := config.KubeClient.(*kubefake.FailingKubeClient) + assert.Len(t, failer.RecordedCreateOptions, tt.expectedCreateOpts, + "unexpected create option count for directive %q", tt.directive) + assert.Len(t, failer.RecordedUpdateOptions, tt.expectedUpdateOpts, + "unexpected update option count for directive %q", tt.directive) + + // Only one path should execute for each test case. + assert.NotEqual(t, tt.expectedCreateOpts > 0, tt.expectedUpdateOpts > 0) + }) + } +} + func TestInstallRelease(t *testing.T) { is := assert.New(t) req := require.New(t) diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index f2cdaf177..44bacf130 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -49,6 +49,9 @@ type Rollback struct { // // This should be used with caution. ForceReplace bool + // FieldValidationDirective, when non-empty, overrides the Kubernetes client's + // default field validation directive for update operations. + FieldValidationDirective kube.FieldValidationDirective // ForceConflicts causes server-side apply to force conflicts ("Overwrite value, become sole manager") // see: https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts ForceConflicts bool @@ -225,13 +228,17 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas if err != nil { return targetRelease, fmt.Errorf("unable to set metadata visitor from target release: %w", err) } - results, err := r.cfg.KubeClient.Update( - current, - target, + updateOpts := []kube.ClientUpdateOption{ kube.ClientUpdateOptionForceReplace(r.ForceReplace), kube.ClientUpdateOptionServerSideApply(serverSideApply, r.ForceConflicts), kube.ClientUpdateOptionThreeWayMergeForUnstructured(false), - kube.ClientUpdateOptionUpgradeClientSideFieldManager(true)) + kube.ClientUpdateOptionUpgradeClientSideFieldManager(true), + } + if r.FieldValidationDirective != "" { + updateOpts = append(updateOpts, kube.ClientUpdateOptionFieldValidationDirective(r.FieldValidationDirective)) + } + results, err := r.cfg.KubeClient.Update(current, target, updateOpts...) + if err != nil { msg := fmt.Sprintf("Rollback %q failed: %s", targetRelease.Name, err) r.cfg.Logger().Warn(msg) diff --git a/pkg/action/rollback_test.go b/pkg/action/rollback_test.go index 7ffd90d25..a06f2e7b3 100644 --- a/pkg/action/rollback_test.go +++ b/pkg/action/rollback_test.go @@ -47,6 +47,48 @@ func TestRollbackRun_UnreachableKubeClient(t *testing.T) { assert.Error(t, client.Run("")) } +func TestRollbackFieldValidationDirectiveOptionPassed(t *testing.T) { + tests := []struct { + name string + directive kube.FieldValidationDirective + wantOpts int + }{ + {name: "empty directive", wantOpts: 4}, + {name: "warn directive", directive: kube.FieldValidationDirectiveWarn, wantOpts: 5}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := actionConfigFixture(t) + + rel1 := releaseStub() + rel1.Name = "field-validation-rollback" + rel1.Version = 1 + rel1.Info.Status = "superseded" + rel1.ApplyMethod = "csa" + require.NoError(t, config.Releases.Create(rel1)) + + rel2 := releaseStub() + rel2.Name = rel1.Name + rel2.Version = 2 + rel2.Info.Status = "deployed" + rel2.ApplyMethod = "csa" + require.NoError(t, config.Releases.Create(rel2)) + + client := NewRollback(config) + client.Version = 1 + client.ServerSideApply = "auto" + client.FieldValidationDirective = tt.directive + + require.NoError(t, client.Run(rel1.Name)) + + failer := config.KubeClient.(*kubefake.FailingKubeClient) + assert.Len(t, failer.RecordedUpdateOptions, tt.wantOpts, + "unexpected update option count for directive %q", tt.directive) + }) + } +} + func TestRollback_WaitOptionsPassedDownstream(t *testing.T) { is := assert.New(t) req := require.New(t) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 7f66ceefb..439e49b8f 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -135,6 +135,9 @@ type Upgrade struct { EnableDNS bool // TakeOwnership will skip the check for helm annotations and adopt all existing resources. TakeOwnership bool + // FieldValidationDirective, when non-empty, overrides the Kubernetes client's + // default field validation directive for update operations. + FieldValidationDirective kube.FieldValidationDirective } type resultMessage struct { @@ -466,12 +469,19 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele } upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && serverSideApply // Update client-side field manager if transitioning from client-side to server-side apply + updateOpts := []kube.ClientUpdateOption{ + kube.ClientUpdateOptionForceReplace(u.ForceReplace), + kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager), + } + + if u.FieldValidationDirective != "" { + updateOpts = append(updateOpts, kube.ClientUpdateOptionFieldValidationDirective(u.FieldValidationDirective)) + } results, err := u.cfg.KubeClient.Update( current, target, - kube.ClientUpdateOptionForceReplace(u.ForceReplace), - kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), - kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager)) + updateOpts...) if err != nil { u.cfg.recordRelease(originalRelease) u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 53419b6a8..976aa207d 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -49,6 +49,37 @@ func upgradeAction(t *testing.T) *Upgrade { return upAction } +func TestUpgradeFieldValidationDirectiveOptionPassed(t *testing.T) { + tests := []struct { + name string + directive kube.FieldValidationDirective + wantOpts int + }{ + {name: "empty directive", wantOpts: 3}, + {name: "warn directive", directive: kube.FieldValidationDirectiveWarn, wantOpts: 4}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + upAction := upgradeAction(t) + upAction.ForceReplace = false + upAction.FieldValidationDirective = tt.directive + + rel := releaseStub() + rel.Name = "field-validation-upgrade" + rel.Info.Status = common.StatusDeployed + require.NoError(t, upAction.cfg.Releases.Create(rel)) + + _, err := upAction.Run(rel.Name, buildChart(), map[string]any{}) + require.NoError(t, err) + + failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient) + assert.Len(t, failer.RecordedUpdateOptions, tt.wantOpts, + "unexpected update option count for directive %q", tt.directive) + }) + } +} + func TestUpgradeRelease_Success(t *testing.T) { is := assert.New(t) req := require.New(t) diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 86574a3e5..d5260ff44 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -2272,3 +2272,99 @@ func createManifest(t *testing.T, manifest string, require.NoError(t, err) require.NoError(t, fakeClient.Tracker().Create(mapping.Resource, obj, obj.GetNamespace())) } + +func TestCreateFieldValidationDirective(t *testing.T) { + c := newTestClient(t) + pods := newPodList("whale") + seenRequest := false + + client := NewRequestResponseLogClient(t, func(_ []RequestResponseAction, req *http.Request) (*http.Response, error) { + assert.Equal(t, http.MethodPatch, req.Method) + assert.Equal(t, string(FieldValidationDirectiveWarn), req.URL.Query().Get("fieldValidation")) + seenRequest = true + return newResponse(http.StatusOK, &pods.Items[0]) + }) + + c.Factory.(*cmdtesting.TestFactory).UnstructuredClient = &fake.RESTClient{ + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(client.Do), + } + + resources, err := c.Build(objBody(&pods), false) + require.NoError(t, err) + + result, err := c.Create( + resources, + ClientCreateOptionServerSideApply(true, false), + ClientCreateOptionFieldValidationDirective(FieldValidationDirectiveWarn), + ) + require.NoError(t, err) + assert.Len(t, result.Created, 1) + assert.True(t, seenRequest, "expected a server-side apply request") +} + +func TestUpdateFieldValidationDirective(t *testing.T) { + tests := []struct { + name string + serverSideApply bool + forceReplace bool + expectedMethod string + }{ + { + name: "server-side apply", + serverSideApply: true, + expectedMethod: http.MethodPatch, + }, + { + name: "force replace", + forceReplace: true, + expectedMethod: http.MethodPut, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := newTestClient(t) + originalPods := newPodList("starfish") + targetPods := newPodList("starfish") + targetPods.Items[0].Spec.Containers[0].Ports = []v1.ContainerPort{{Name: "https", ContainerPort: 443}} + seenUpdateRequest := false + + client := NewRequestResponseLogClient(t, func(_ []RequestResponseAction, req *http.Request) (*http.Response, error) { + switch req.Method { + case http.MethodGet: + return newResponse(http.StatusOK, &originalPods.Items[0]) + case tt.expectedMethod: + assert.Equal(t, string(FieldValidationDirectiveWarn), req.URL.Query().Get("fieldValidation")) + seenUpdateRequest = true + return newResponse(http.StatusOK, &targetPods.Items[0]) + default: + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.String()) + return nil, nil + } + }) + + c.Factory.(*cmdtesting.TestFactory).UnstructuredClient = &fake.RESTClient{ + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(client.Do), + } + + originals, err := c.Build(objBody(&originalPods), false) + require.NoError(t, err) + targets, err := c.Build(objBody(&targetPods), false) + require.NoError(t, err) + + result, err := c.Update( + originals, + targets, + ClientUpdateOptionForceReplace(tt.forceReplace), + ClientUpdateOptionServerSideApply(tt.serverSideApply, false), + ClientUpdateOptionUpgradeClientSideFieldManager(false), + ClientUpdateOptionFieldValidationDirective(FieldValidationDirectiveWarn), + ) + require.NoError(t, err) + assert.Len(t, result.Updated, 1) + assert.True(t, seenUpdateRequest, "expected a %s update request", tt.expectedMethod) + }) + } +} diff --git a/pkg/kube/fake/failing_kube_client.go b/pkg/kube/fake/failing_kube_client.go index 75d0c8de1..35be9c29c 100644 --- a/pkg/kube/fake/failing_kube_client.go +++ b/pkg/kube/fake/failing_kube_client.go @@ -48,7 +48,11 @@ type FailingKubeClient struct { WaitForDeleteError error WatchUntilReadyError error WaitDuration time.Duration - // RecordedWaitOptions stores the WaitOptions passed to GetWaiter for testing + // RecordedCreateOptions stores the ClientCreateOptions passed to Create for testing. + RecordedCreateOptions []kube.ClientCreateOption + // RecordedUpdateOptions stores the ClientUpdateOptions passed to Update for testing. + RecordedUpdateOptions []kube.ClientUpdateOption + // RecordedWaitOptions stores the WaitOptions passed to GetWaiter for testing. RecordedWaitOptions []kube.WaitOption mu sync.Mutex } @@ -67,6 +71,9 @@ type FailingKubeWaiter struct { // Create returns the configured error if set or prints func (f *FailingKubeClient) Create(resources kube.ResourceList, options ...kube.ClientCreateOption) (*kube.Result, error) { + f.mu.Lock() + f.RecordedCreateOptions = append(f.RecordedCreateOptions, options...) + f.mu.Unlock() if f.CreateError != nil { return nil, f.CreateError } @@ -125,6 +132,9 @@ func (f *FailingKubeWaiter) WatchUntilReady(resources kube.ResourceList, d time. // Update returns the configured error if set or prints func (f *FailingKubeClient) Update(r, modified kube.ResourceList, options ...kube.ClientUpdateOption) (*kube.Result, error) { + f.mu.Lock() + f.RecordedUpdateOptions = append(f.RecordedUpdateOptions, options...) + f.mu.Unlock() if f.UpdateError != nil { return &kube.Result{}, f.UpdateError }