From 2cd569139b651508b2cc7a6a92032e7ce6d28f4c Mon Sep 17 00:00:00 2001 From: Ilya Lesikov Date: Tue, 4 Aug 2026 12:03:30 +0300 Subject: [PATCH] fix: ignore helm.sh(werf.io)/resource-policy: keep from cluster Signed-off-by: Ilya Lesikov --- .gitignore | 3 + .../issue-7233/charts/alpine-0.1.0.tgz | Bin 1166 -> 0 bytes pkg/plan/resource_info.go | 40 +---- pkg/plan/resource_policy_live_ai_test.go | 161 ++++++++++++++++++ pkg/resource/resource.go | 16 -- pkg/resource/resource_policy_test.go | 104 ----------- 6 files changed, 172 insertions(+), 152 deletions(-) delete mode 100644 pkg/helm/pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz create mode 100644 pkg/plan/resource_policy_live_ai_test.go diff --git a/.gitignore b/.gitignore index 922d9d9b..2b5e7e09 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,6 @@ node_modules/ /pkg/ts/embed/*/*/deno.gz.*.tmp /.opencode/ /.sisyphus + +# Regenerated by TestDependencyBuildCmdWithHelmV2Hash on every run (fresh mtimes) +/pkg/helm/pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz diff --git a/pkg/helm/pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz b/pkg/helm/pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz deleted file mode 100644 index fa8d1a7ac72209301dd03de5f315c64c3803e608..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1166 zcmV;91abQxiwG0|00000|0w_~VMtOiV@ORlOnEsqVl!4SWK%V1T2nbTPgYhoO;>Dc zVQyr3R8em|NM&qo0PI-bZ`-yL&$IrDgZf&_^sfZQ zBPlDM>;HWqDLJvSCIePG`=NUgM3Eof9q)5@6pE5d8vZXwZIdk);^ONIV~kx+r~b(p z>pt0dHoF+j#^bBY>1;9?U$D`5I-6bqJAp2bq%s!u1^df;b&dOn2$uX4X~UJA!)6p= zSsq^FN%AI+GBT;=rS(H^sT67p2un0Pg=-%?@GY0s9h54Ia#heAa%)R79}aU8MPhRQ zj3l~GA>1OPDxH#A1uH*>u|T z|18p}|F>LbbNHyTs340OTAW7!p?1f+U2;Q$&`{#i#(qE8?UGNSa)g060uZ<)Vcar+ zSMWnl5Mx;;x7CugBuggYYFMDeHD!b4HVzAb8>kFrU=>)6=`GYbPjj$Ji3Te3%?D+G zu;v?*kou+--rHvJsuHkL8ZaxkG*IUXYLwPN8X~B3v<;LFW*9&yQCpr?0=3`EL1{9j zl7=q@IlRO#ddh}5JaEnTq&GYz_zb^R$`b7NPrHIkV^O9QL#pJ4r0cYxz*{oFEfAWm z_X#J!R__MTDnKDXt-$64-yBe#0inSHo1=xAG(oBft#h97CWlpBhkVJ@R>vbl-H~^# ze7y9~!tHjDgloq2p~-yc9Bv+4ja9~NIA^E~t{Vm0#?pTXffT9<5L=Dh?rs0?bxgyz}lk!iCS!+1MZ3(VY1oY1cTp zqhthbTZ>$h_IYB^B$yhxCGAg&7r1sQCsd>P7h)%zYO0C?UE2C79xk~Y8bMUXuj4n2 zvH0h{qj^%A`hWB8`t5h`k|I0B9?$jvv&*Y)|IbGK{r@cTs=36^;kwOn|0hv|oEt?q zmGsF^t_8UpzDpnvZ~aLW`CLeM=-1Y0U`>*=uQioh=(AbFQCQ?2+}EP!XTZW^5G?L{ z&;?{%Q)-powyDD9wGHWQtEAJVEpt$Fn*&Cvs@x~Faup5>vZPH5{@{YUfE5>X5BwA~D~LNYv9{L;sMkcR(sXEpwEqQ3yo z-v1|)4*#bkcGdI$9Ae2>SA)ZU03QZ}5C(sr813bB((vCkgQueZpWuJ@{y(`I&wBo! gLyr0H!l0*rFTM2A%YQ+B4*&rF|CCL6y8s{n03_)<*Z=?k diff --git a/pkg/plan/resource_info.go b/pkg/plan/resource_info.go index b925c4ad..20c4f80f 100644 --- a/pkg/plan/resource_info.go +++ b/pkg/plan/resource_info.go @@ -189,10 +189,9 @@ func buildInstallableResourceInfo(ctx context.Context, localRes *resource.Instal } var ( - getMeta *spec.ResourceMeta - dryApplyObj *unstructured.Unstructured - dryApplyErr error - resourcePolicies = localRes.ResourcePolicies + getMeta *spec.ResourceMeta + dryApplyObj *unstructured.Unstructured + dryApplyErr error ) if getErr == nil { var err error @@ -203,7 +202,6 @@ func buildInstallableResourceInfo(ctx context.Context, localRes *resource.Instal } getMeta = spec.NewResourceMetaFromUnstructured(getObj, releaseNamespace, localRes.FilePath) - resourcePolicies = resource.ResolveResourcePolicies(localRes, getMeta, releaseNamespace) dryApplyObj, dryApplyErr = clientFactory.KubeClient().Apply(ctx, localRes.ResourceSpec, kube.KubeClientApplyOptions{ DefaultNamespace: releaseNamespace, @@ -211,7 +209,7 @@ func buildInstallableResourceInfo(ctx context.Context, localRes *resource.Instal }) } - installType, skippedByPolicy, err := resourceInstallType(ctx, localRes, getObj, dryApplyObj, dryApplyErr, opts.ExtraRuntimeAnnotations, opts.ExtraRuntimeLabels, resourcePolicies, diffPatches) + installType, skippedByPolicy, err := resourceInstallType(ctx, localRes, getObj, dryApplyObj, dryApplyErr, opts.ExtraRuntimeAnnotations, opts.ExtraRuntimeLabels, localRes.ResourcePolicies, diffPatches) if err != nil { return nil, fmt.Errorf("determine install type for resource %q: %w", localRes.IDHuman(), err) } @@ -222,7 +220,7 @@ func buildInstallableResourceInfo(ctx context.Context, localRes *resource.Instal } } - mustDeleteOnSuccess := mustDeleteOnSuccessfulDeploy(localRes, getMeta, installType, releaseNamespace, skippedByPolicy) + mustDeleteOnSuccess := mustDeleteOnSuccessfulDeploy(localRes, getMeta, installType, skippedByPolicy) trackReadiness := mustTrackReadiness(localRes, installType, getObj != nil, prevRelFailed, mustDeleteOnSuccess, skippedByPolicy) return lo.Map(stages, func(stg common.Stage, _ int) *InstallableResourceInfo { @@ -232,7 +230,7 @@ func buildInstallableResourceInfo(ctx context.Context, localRes *resource.Instal DryApplyResult: dryApplyObj, GetResult: getObj, LocalResource: localRes, - MustDeleteOnFailedInstall: mustDeleteOnFailedDeploy(localRes, getMeta, installType, releaseNamespace, trackReadiness, skippedByPolicy), + MustDeleteOnFailedInstall: mustDeleteOnFailedDeploy(localRes, installType, trackReadiness, skippedByPolicy), MustDeleteOnSuccessfulInstall: mustDeleteOnSuccess, MustInstall: installType, MustTrackReadiness: trackReadiness, @@ -383,12 +381,6 @@ func buildDeletableResourceInfo(ctx context.Context, localRes *resource.Deletabl getMeta := spec.NewResourceMetaFromUnstructured(getObj, releaseNamespace, localRes.FilePath) - if err := resource.ValidateResourcePolicy(getMeta); err != nil { - return noDeleteInfo, nil - } else if lo.Contains(resource.ResourcePolicies(getMeta, releaseNamespace), common.ResourcePolicySkipDelete) { - return noDeleteInfo, nil - } - if orphaned(getMeta, releaseName, releaseNamespace) { return noDeleteInfo, nil } @@ -686,7 +678,7 @@ func iterateInstallableResourceInfos(infos []*InstallableResourceInfo) { } } -func mustDeleteOnFailedDeploy(res *resource.InstallableResource, getMeta *spec.ResourceMeta, installType ResourceInstallType, releaseNamespace string, mustTrackReadiness, skippedByPolicy bool) bool { +func mustDeleteOnFailedDeploy(res *resource.InstallableResource, installType ResourceInstallType, mustTrackReadiness, skippedByPolicy bool) bool { if skippedByPolicy || !res.DeleteOnFailed || lo.Contains(res.ResourcePolicies, common.ResourcePolicySkipDelete) || @@ -695,32 +687,16 @@ func mustDeleteOnFailedDeploy(res *resource.InstallableResource, getMeta *spec.R return false } - if getMeta != nil { - if err := resource.ValidateResourcePolicy(getMeta); err != nil { - return false - } else if lo.Contains(resource.ResourcePolicies(getMeta, releaseNamespace), common.ResourcePolicySkipDelete) { - return false - } - } - return true } -func mustDeleteOnSuccessfulDeploy(localRes *resource.InstallableResource, getMeta *spec.ResourceMeta, installType ResourceInstallType, releaseNamespace string, skippedByPolicy bool) bool { +func mustDeleteOnSuccessfulDeploy(localRes *resource.InstallableResource, getMeta *spec.ResourceMeta, installType ResourceInstallType, skippedByPolicy bool) bool { if skippedByPolicy || !localRes.DeleteOnSucceeded || lo.Contains(localRes.ResourcePolicies, common.ResourcePolicySkipDelete) { return false } - if getMeta != nil { - if err := resource.ValidateResourcePolicy(getMeta); err != nil { - return false - } else if lo.Contains(resource.ResourcePolicies(getMeta, releaseNamespace), common.ResourcePolicySkipDelete) { - return false - } - } - if installType == ResourceInstallTypeNone { return getMeta != nil } diff --git a/pkg/plan/resource_policy_live_ai_test.go b/pkg/plan/resource_policy_live_ai_test.go new file mode 100644 index 00000000..8a42a30f --- /dev/null +++ b/pkg/plan/resource_policy_live_ai_test.go @@ -0,0 +1,161 @@ +//go:build ai_tests + +package plan_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/suite" + + "github.com/werf/nelm/pkg/common" + "github.com/werf/nelm/pkg/kube" + "github.com/werf/nelm/pkg/kube/fake" + "github.com/werf/nelm/pkg/plan" +) + +type ResourcePolicyLiveAISuite struct { + suite.Suite + + clientFactory *fake.ClientFactory + releaseName string + releaseNamespace string +} + +func (s *ResourcePolicyLiveAISuite) SetupSubTest() { + var err error + + s.clientFactory, err = fake.NewClientFactory(context.Background()) + s.Require().NoError(err) +} + +func (s *ResourcePolicyLiveAISuite) SetupSuite() { + s.releaseName = "test-release" + s.releaseNamespace = "test-namespace" +} + +func (s *ResourcePolicyLiveAISuite) TestAI_ChartPolicyStillProtectsChartRemovedResource() { + s.Run("chart skip-delete keeps resource", func() { + s.createLiveResource(nil) + + localRes := defaultDeletableResource(s.releaseName, s.releaseNamespace) + localRes.ResourcePolicies = []common.ResourcePolicy{common.ResourcePolicySkipDelete} + + resInfo, err := plan.BuildDeletableResourceInfo(context.Background(), localRes, common.DeployTypeUninstall, s.releaseName, s.releaseNamespace, s.clientFactory) + s.Require().NoError(err) + s.Require().False(resInfo.MustDelete, "chart-side skip-delete must keep protecting the resource") + }) +} + +func (s *ResourcePolicyLiveAISuite) TestAI_ChartPolicyStillSuppressesDeleteOnSucceeded() { + s.Run("chart skip-delete suppresses delete-on-succeeded", func() { + s.createLiveResource(nil) + + localRes := defaultInstallableResource(s.releaseName, s.releaseNamespace) + localRes.DeleteOnSucceeded = true + localRes.ResourcePolicies = []common.ResourcePolicy{common.ResourcePolicySkipDelete} + + resInfos, err := plan.BuildInstallableResourceInfo(context.Background(), localRes, common.DeployTypeInitial, s.releaseNamespace, false, true, s.clientFactory, plan.BuildResourceInfosOptions{}, nil) + s.Require().NoError(err) + s.Require().NotEmpty(resInfos) + s.Require().False(resInfos[0].MustDeleteOnSuccessfulInstall, "chart-side skip-delete must still suppress delete-on-succeeded") + }) +} + +func (s *ResourcePolicyLiveAISuite) TestAI_LiveOnlyPolicyDoesNotProtectChartRemovedResource() { + livePolicies := []map[string]string{ + {"helm.sh/resource-policy": "keep"}, + {"werf.io/resource-policy": "keep"}, + {"werf.io/resource-policy": "skip-delete"}, + {"werf.io/resource-policy": "bogus"}, + } + + for _, policy := range livePolicies { + s.Run(policyName(policy), func() { + s.createLiveResource(policy) + + localRes := defaultDeletableResource(s.releaseName, s.releaseNamespace) + + resInfo, err := plan.BuildDeletableResourceInfo(context.Background(), localRes, common.DeployTypeUninstall, s.releaseName, s.releaseNamespace, s.clientFactory) + s.Require().NoError(err) + s.Require().True(resInfo.MustDelete, "chart-removed resource must be deleted despite live-only policy %v", policy) + }) + } +} + +func (s *ResourcePolicyLiveAISuite) TestAI_LiveOnlyPolicyDoesNotSuppressDeleteOnFailed() { + livePolicies := []map[string]string{ + {"werf.io/resource-policy": "skip-delete"}, + {"werf.io/resource-policy": "bogus"}, + } + + for _, policy := range livePolicies { + s.Run(policyName(policy), func() { + s.createLiveResource(policy) + + localRes := updatedInstallableResource(&s.Suite, s.releaseName, s.releaseNamespace) + localRes.DeleteOnFailed = true + + resInfos, err := plan.BuildInstallableResourceInfo(context.Background(), localRes, common.DeployTypeInitial, s.releaseNamespace, false, true, s.clientFactory, plan.BuildResourceInfosOptions{}, nil) + s.Require().NoError(err) + s.Require().NotEmpty(resInfos) + s.Require().Equal(plan.ResourceInstallTypeUpdate, resInfos[0].MustInstall) + s.Require().True(resInfos[0].MustDeleteOnFailedInstall, "delete-on-failed must not be suppressed by live-only policy %v", policy) + }) + } +} + +func (s *ResourcePolicyLiveAISuite) TestAI_LiveOnlyPolicyDoesNotSuppressDeleteOnSucceeded() { + livePolicies := []map[string]string{ + {"werf.io/resource-policy": "skip-delete"}, + {"werf.io/resource-policy": "bogus"}, + } + + for _, policy := range livePolicies { + s.Run(policyName(policy), func() { + s.createLiveResource(policy) + + localRes := defaultInstallableResource(s.releaseName, s.releaseNamespace) + localRes.DeleteOnSucceeded = true + + resInfos, err := plan.BuildInstallableResourceInfo(context.Background(), localRes, common.DeployTypeInitial, s.releaseNamespace, false, true, s.clientFactory, plan.BuildResourceInfosOptions{}, nil) + s.Require().NoError(err) + s.Require().NotEmpty(resInfos) + s.Require().Equal(plan.ResourceInstallTypeNone, resInfos[0].MustInstall) + s.Require().True(resInfos[0].MustDeleteOnSuccessfulInstall, "delete-on-succeeded must not be suppressed by live-only policy %v", policy) + }) + } +} + +func (s *ResourcePolicyLiveAISuite) createLiveResource(policyAnnotations map[string]string) { + resSpec := defaultResourceSpec(s.releaseName, s.releaseNamespace) + + annotations := resSpec.Unstruct.GetAnnotations() + for k, v := range policyAnnotations { + annotations[k] = v + } + + resSpec.SetAnnotations(annotations) + + _, err := s.clientFactory.KubeClient().Create(context.Background(), resSpec, kube.KubeClientCreateOptions{ + DefaultNamespace: s.releaseNamespace, + }) + s.Require().NoError(err) +} + +func TestAI_ResourcePolicyLiveSuite(t *testing.T) { + suite.Run(t, new(ResourcePolicyLiveAISuite)) +} + +func policyName(policy map[string]string) string { + if len(policy) == 0 { + return "no policy" + } + + var name string + for k, v := range policy { + name += k + "=" + v + } + + return name +} diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go index 95f3eab6..49d446e2 100644 --- a/pkg/resource/resource.go +++ b/pkg/resource/resource.go @@ -372,19 +372,3 @@ func BuildResources(ctx context.Context, deployType common.DeployType, releaseNa return instResources, delResources, nil } - -func ResolveResourcePolicies(localRes *InstallableResource, liveMeta *spec.ResourceMeta, releaseNamespace string) []common.ResourcePolicy { - if len(localRes.ResourcePolicies) > 0 || liveMeta == nil { - return localRes.ResourcePolicies - } - - // TODO(major): in the next major keep/skip-delete should also be read/respected only from the manifest, not the cluster. - livePolicies := lo.Filter(ResourcePolicies(liveMeta, releaseNamespace), func(p common.ResourcePolicy, _ int) bool { - return p == common.ResourcePolicySkipDelete - }) - if len(livePolicies) == 0 { - return nil - } - - return livePolicies -} diff --git a/pkg/resource/resource_policy_test.go b/pkg/resource/resource_policy_test.go index 507bcbf3..3b36d900 100644 --- a/pkg/resource/resource_policy_test.go +++ b/pkg/resource/resource_policy_test.go @@ -1,12 +1,10 @@ package resource_test import ( - "context" "testing" "github.com/samber/lo" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/werf/nelm/pkg/common" @@ -16,84 +14,6 @@ import ( const resourcePolicyTestNamespace = "test-namespace" -func TestResolveResourcePolicies(t *testing.T) { - tests := []struct { - name string - chart, live map[string]string - nilLive bool - want []common.ResourcePolicy - }{ - { - name: "no policies", - nilLive: true, - }, - { - name: "chart skip-create only", - chart: map[string]string{"werf.io/resource-policy": "skip-create"}, - nilLive: true, - want: []common.ResourcePolicy{common.ResourcePolicySkipCreate}, - }, - { - name: "chart all skips", - chart: map[string]string{"werf.io/resource-policy": "skip-create,skip-update,skip-recreate"}, - nilLive: true, - want: []common.ResourcePolicy{common.ResourcePolicySkipCreate, common.ResourcePolicySkipUpdate, common.ResourcePolicySkipRecreate}, - }, - { - name: "live skip-update dropped when chart absent", - live: map[string]string{"werf.io/resource-policy": "skip-update"}, - }, - { - name: "live install skips dropped when chart absent", - live: map[string]string{"werf.io/resource-policy": "skip-create,skip-update,skip-recreate"}, - }, - { - name: "live skip-delete retained when chart absent", - live: map[string]string{"werf.io/resource-policy": "skip-delete"}, - want: []common.ResourcePolicy{common.ResourcePolicySkipDelete}, - }, - { - name: "live werf.io keep retained as skip-delete when chart absent", - live: map[string]string{"werf.io/resource-policy": "keep"}, - want: []common.ResourcePolicy{common.ResourcePolicySkipDelete}, - }, - { - name: "live helm.sh keep retained as skip-delete when chart absent", - live: map[string]string{"helm.sh/resource-policy": "keep"}, - want: []common.ResourcePolicy{common.ResourcePolicySkipDelete}, - }, - { - name: "live mixed policies filtered to skip-delete when chart absent", - live: map[string]string{"werf.io/resource-policy": "skip-update,skip-delete"}, - want: []common.ResourcePolicy{common.ResourcePolicySkipDelete}, - }, - { - name: "chart present takes precedence over live (no merge)", - chart: map[string]string{"werf.io/resource-policy": "skip-update"}, - live: map[string]string{"werf.io/resource-policy": "skip-delete"}, - want: []common.ResourcePolicy{common.ResourcePolicySkipUpdate}, - }, - { - name: "chart helm.sh keep present suppresses live werf.io skips", - chart: map[string]string{"helm.sh/resource-policy": "keep"}, - live: map[string]string{"werf.io/resource-policy": "skip-create"}, - want: []common.ResourcePolicy{common.ResourcePolicySkipDelete}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var liveMeta *spec.ResourceMeta - if !tt.nilLive { - liveMeta = resourcePolicyMeta(tt.live) - } - - localRes := chartInstallableResource(t, tt.chart) - assert.Equal(t, tt.want, resource.ResolveResourcePolicies(localRes, liveMeta, resourcePolicyTestNamespace)) - }) - } -} - func TestResourcePoliciesSkipDelete(t *testing.T) { tests := []struct { name string @@ -151,30 +71,6 @@ func TestValidateResourcePolicy(t *testing.T) { } } -func chartInstallableResource(t *testing.T, annotations map[string]string) *resource.InstallableResource { - t.Helper() - - obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "test-configmap", - }, - }, - } - - resSpec := spec.NewResourceSpec(obj, resourcePolicyTestNamespace, spec.ResourceSpecOptions{}) - if len(annotations) > 0 { - resSpec.SetAnnotations(annotations) - } - - localRes, err := resource.NewInstallableResource(context.Background(), resSpec, nil, resourcePolicyTestNamespace, resource.InstallableResourceOptions{}) - require.NoError(t, err) - - return localRes -} - func resourcePolicyMeta(annotations map[string]string) *spec.ResourceMeta { obj := &unstructured.Unstructured{ Object: map[string]interface{}{