From ce8cd82ee9eb40e67fa9b6e3d7515c090ecb180c Mon Sep 17 00:00:00 2001 From: Mike Ng Date: Mon, 11 Jan 2021 15:45:40 -0500 Subject: [PATCH 1/3] fix: (helm) - do not add owner references to resources that contain the Helm keep resource-policy annotation Signed-off-by: Mike Ng --- ...m_operator_fix_helm_keep_annotated_delete.yaml | 5 +++++ internal/helm/client/client.go | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 changelog/fragments/helm_operator_fix_helm_keep_annotated_delete.yaml diff --git a/changelog/fragments/helm_operator_fix_helm_keep_annotated_delete.yaml b/changelog/fragments/helm_operator_fix_helm_keep_annotated_delete.yaml new file mode 100644 index 0000000000..203f734193 --- /dev/null +++ b/changelog/fragments/helm_operator_fix_helm_keep_annotated_delete.yaml @@ -0,0 +1,5 @@ +entries: + - description: > + For Helm-based operators, do not add owner references to resources that contain the Helm annotation: 'helm.sh/resource-policy: keep'. + kind: bugfix + breaking: false \ No newline at end of file diff --git a/internal/helm/client/client.go b/internal/helm/client/client.go index eb7787f696..fb30151be2 100644 --- a/internal/helm/client/client.go +++ b/internal/helm/client/client.go @@ -17,6 +17,7 @@ package client import ( "errors" "io" + "strings" "github.com/operator-framework/operator-lib/handler" "github.com/operator-framework/operator-sdk/internal/util/k8sutil" @@ -144,7 +145,19 @@ func (c *ownerRefInjectingClient) Build(reader io.Reader, validate bool) (kube.R return err } - if useOwnerRef { + // If the resource contains the Helm 'keep' resource-policy annotation, then + // do not add the owner reference. So when the CR is deleted, the Kubernetes won't GCs + // the resource with the annotation. + containsKeepAnno := false + if u.GetAnnotations() != nil { + resourcePolicyType, ok := u.GetAnnotations()[kube.ResourcePolicyAnno] + if ok { + resourcePolicyType = strings.ToLower(strings.TrimSpace(resourcePolicyType)) + containsKeepAnno = resourcePolicyType == kube.KeepPolicy + } + } + + if useOwnerRef && !containsKeepAnno { ownerRef := metav1.NewControllerRef(c.owner, c.owner.GroupVersionKind()) u.SetOwnerReferences([]metav1.OwnerReference{*ownerRef}) } else { From cf84ca2e9c9107de175ced405089396520682275 Mon Sep 17 00:00:00 2001 From: Mike Ng Date: Tue, 12 Jan 2021 09:58:45 -0500 Subject: [PATCH 2/3] refractor helm keep annotation into its function and add unit tests Signed-off-by: Mike Ng --- internal/helm/client/client.go | 28 +++++----- internal/helm/client/client_test.go | 79 +++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 13 deletions(-) create mode 100644 internal/helm/client/client_test.go diff --git a/internal/helm/client/client.go b/internal/helm/client/client.go index fb30151be2..1354576709 100644 --- a/internal/helm/client/client.go +++ b/internal/helm/client/client.go @@ -145,19 +145,9 @@ func (c *ownerRefInjectingClient) Build(reader io.Reader, validate bool) (kube.R return err } - // If the resource contains the Helm 'keep' resource-policy annotation, then - // do not add the owner reference. So when the CR is deleted, the Kubernetes won't GCs - // the resource with the annotation. - containsKeepAnno := false - if u.GetAnnotations() != nil { - resourcePolicyType, ok := u.GetAnnotations()[kube.ResourcePolicyAnno] - if ok { - resourcePolicyType = strings.ToLower(strings.TrimSpace(resourcePolicyType)) - containsKeepAnno = resourcePolicyType == kube.KeepPolicy - } - } - - if useOwnerRef && !containsKeepAnno { + // If the resource contains the Helm resource-policy keep annotation, then do not add + // the owner reference. So when the CR is deleted, Kubernetes won't GCs the resource. + if useOwnerRef && !containsResourcePolicyKeep(u.GetAnnotations()) { ownerRef := metav1.NewControllerRef(c.owner, c.owner.GroupVersionKind()) u.SetOwnerReferences([]metav1.OwnerReference{*ownerRef}) } else { @@ -173,3 +163,15 @@ func (c *ownerRefInjectingClient) Build(reader io.Reader, validate bool) (kube.R } return resourceList, nil } + +func containsResourcePolicyKeep(annotations map[string]string) bool { + if annotations != nil { + resourcePolicyType, ok := annotations[kube.ResourcePolicyAnno] + if ok { + resourcePolicyType = strings.ToLower(strings.TrimSpace(resourcePolicyType)) + return resourcePolicyType == kube.KeepPolicy + } + } + + return false +} diff --git a/internal/helm/client/client_test.go b/internal/helm/client/client_test.go new file mode 100644 index 0000000000..c223da3339 --- /dev/null +++ b/internal/helm/client/client_test.go @@ -0,0 +1,79 @@ +// Copyright 2021 The Operator-SDK Authors +// +// 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 client + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "helm.sh/helm/v3/pkg/kube" +) + +func TestContainsResourcePolicyKeep(t *testing.T) { + tests := []struct { + input map[string]string + expectedVal bool + expectedOut string + name string + }{ + { + input: map[string]string{ + kube.ResourcePolicyAnno: kube.KeepPolicy, + }, + expectedVal: true, + name: "base case true", + }, + { + input: map[string]string{ + "not-" + kube.ResourcePolicyAnno: kube.KeepPolicy, + }, + expectedVal: false, + name: "base case annotation false", + }, + { + input: map[string]string{ + kube.ResourcePolicyAnno: "not-" + kube.KeepPolicy, + }, + expectedVal: false, + name: "base case value false", + }, + { + input: map[string]string{ + kube.ResourcePolicyAnno: strings.ToUpper(kube.KeepPolicy), + }, + expectedVal: true, + name: "true with upper case", + }, + { + input: map[string]string{ + kube.ResourcePolicyAnno: " " + kube.KeepPolicy + " ", + }, + expectedVal: true, + name: "true with spaces", + }, + { + input: map[string]string{ + kube.ResourcePolicyAnno: " " + strings.ToUpper(kube.KeepPolicy) + " ", + }, + expectedVal: true, + name: "true with upper case and spaces", + }, + } + + for _, test := range tests { + assert.Equal(t, test.expectedVal, containsResourcePolicyKeep(test.input), test.name) + } +} From 57371a7de5c00b48297636c8c13ae6a5ec8a4370 Mon Sep 17 00:00:00 2001 From: Mike Ng Date: Tue, 12 Jan 2021 10:35:50 -0500 Subject: [PATCH 3/3] refactor to minimize indentation Signed-off-by: Mike Ng --- internal/helm/client/client.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/helm/client/client.go b/internal/helm/client/client.go index 1354576709..615ad882ad 100644 --- a/internal/helm/client/client.go +++ b/internal/helm/client/client.go @@ -165,13 +165,13 @@ func (c *ownerRefInjectingClient) Build(reader io.Reader, validate bool) (kube.R } func containsResourcePolicyKeep(annotations map[string]string) bool { - if annotations != nil { - resourcePolicyType, ok := annotations[kube.ResourcePolicyAnno] - if ok { - resourcePolicyType = strings.ToLower(strings.TrimSpace(resourcePolicyType)) - return resourcePolicyType == kube.KeepPolicy - } + if annotations == nil { + return false } - - return false + resourcePolicyType, ok := annotations[kube.ResourcePolicyAnno] + if !ok { + return false + } + resourcePolicyType = strings.ToLower(strings.TrimSpace(resourcePolicyType)) + return resourcePolicyType == kube.KeepPolicy }