Repository: operator-framework/operator-sdk
Pull request #4389 (2021-01-11 20:51:50)

fix: (helm) - do not add owner references to resources that contain the Helm keep resource-policy annotation
Signed-off-by: Mike Ng <ming@redhat.com>

**Description of the change:**
Helm operator: updated the ownerRefInjectingClient to not inject the owner references if it contains the annotation: 
```
  annotations:
    "helm.sh/resource-policy": keep
```
see https://helm.sh/docs/howto/charts_tips_and_tricks/ for more details about the `keep` annotation.

**Motivation for the change:**
For the Helm operator delete CR, namespace scope resources with the Helm keep annotation are being GC'ed by Kubernetes.
This change is to make sure those resources are not owned by the CR so they won't get GC'ed.

Closes: https://github.com/operator-framework/operator-sdk/issues/4378

After this change, I can no longer reproduces the issue described in https://github.com/operator-framework/operator-sdk/issues/4378

**Checklist**

If the pull request includes user-facing changes, extra documentation is required:
- [x] Add a new changelog fragment in `changelog/fragments` (see [`changelog/fragments/00-template.yaml`](https://github.com/operator-framework/operator-sdk/tree/master/changelog/fragments/00-template.yaml))
- [NA] Add or update relevant sections of the docs website in [`website/content/en/docs`](https://github.com/operator-framework/operator-sdk/tree/master/website/content/en/docs)


mikeshng: I followed the `helm` repo on how to check if the `keep` annotation exists. 
https://github.com/helm/helm/blob/v3.4.2/pkg/action/resource_policy.go#L33-L40

Patch:
diff --git a/Makefile b/Makefile
index 7d51a607..c6305d6a 100644
--- a/Makefile
+++ b/Makefile
@@ -144,7 +144,7 @@ export KIND_CLUSTER := operator-sdk-e2e
 export KUBEBUILDER_ASSETS := $(PWD)/$(TOOLS_DIR)
 test-e2e-setup: build
 	$(SCRIPTS_DIR)/fetch kind 0.9.0
-	$(SCRIPTS_DIR)/fetch envtest 0.6.3
+	$(SCRIPTS_DIR)/fetch envtest 0.7.0
 	$(SCRIPTS_DIR)/fetch kubectl $(K8S_VERSION) # Install kubectl AFTER envtest because envtest includes its own kubectl binary
 	[[ "`$(TOOLS_DIR)/kind get clusters`" =~ "$(KIND_CLUSTER)" ]] || $(TOOLS_DIR)/kind create cluster --image="kindest/node:v$(K8S_VERSION)" --name $(KIND_CLUSTER)
 
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 00000000..203f7341
--- /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/changelog/fragments/kuttl-v0.8.0.yaml b/changelog/fragments/kuttl-v0.8.0.yaml
new file mode 100644
index 00000000..07421adb
--- /dev/null
+++ b/changelog/fragments/kuttl-v0.8.0.yaml
@@ -0,0 +1,4 @@
+entries:
+  - description: >
+      Upgraded the `kudobuilder/kuttl` base image version in the `scorecard-test-kuttl` image to v0.8.0
+    kind: change
diff --git a/internal/helm/client/client.go b/internal/helm/client/client.go
index eb7787f6..615ad882 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,9 @@ func (c *ownerRefInjectingClient) Build(reader io.Reader, validate bool) (kube.R
 			return err
 		}
 
-		if useOwnerRef {
+		// 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 {
@@ -160,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 {
+		return false
+	}
+	resourcePolicyType, ok := annotations[kube.ResourcePolicyAnno]
+	if !ok {
+		return false
+	}
+	resourcePolicyType = strings.ToLower(strings.TrimSpace(resourcePolicyType))
+	return resourcePolicyType == kube.KeepPolicy
+}
diff --git a/website/content/en/docs/building-operators/ansible/quickstart.md b/website/content/en/docs/building-operators/ansible/quickstart.md
index e13406a4..8fa68418 100644
--- a/website/content/en/docs/building-operators/ansible/quickstart.md
+++ b/website/content/en/docs/building-operators/ansible/quickstart.md
@@ -32,7 +32,7 @@ This guide walks through an example of building a simple memcached-operator powe
 Make sure to define `IMG` when you call `make`:
 
   ```sh
-  export OPERATOR_IMG="quay.io/example-inc/memcached-operator:v0.0.1"
+  export OPERATOR_IMG="quay.io/example/memcached-operator:v0.0.1"
   make docker-build docker-push IMG=$OPERATOR_IMG
   ```
 
@@ -50,7 +50,7 @@ Make sure to define `IMG` when you call `make`:
   ```sh
   make bundle IMG=$OPERATOR_IMG
   # Note the "-bundle" component in the image name below.
-  export BUNDLE_IMG="quay.io/example-inc/memcached-operator-bundle:v0.0.1"
+  export BUNDLE_IMG="quay.io/example/memcached-operator-bundle:v0.0.1"
   make bundle-build BUNDLE_IMG=$BUNDLE_IMG
   make docker-push IMG=$BUNDLE_IMG
   ```
diff --git a/website/content/en/docs/building-operators/golang/advanced-topics.md b/website/content/en/docs/building-operators/golang/advanced-topics.md
index ef319d96..979023a8 100644
--- a/website/content/en/docs/building-operators/golang/advanced-topics.md
+++ b/website/content/en/docs/building-operators/golang/advanced-topics.md
@@ -43,7 +43,7 @@ The operator's Manager supports the core Kubernetes resource types as found in t
 
 ```Go
 import (
-    cachev1alpha1 "github.com/example-inc/memcached-operator/api/v1alpha1
+    cachev1alpha1 "github.com/example/memcached-operator/api/v1alpha1
     ...
 )
 
diff --git a/website/content/en/docs/building-operators/golang/migration.md b/website/content/en/docs/building-operators/golang/migration.md
index 64dfbf43..10d5c1a8 100644
--- a/website/content/en/docs/building-operators/golang/migration.md
+++ b/website/content/en/docs/building-operators/golang/migration.md
@@ -61,7 +61,7 @@ So let's create a new project with the same domain (`example.com`):
 ```sh
 mkdir memcached-operator
 cd memcached-operator
-operator-sdk init --domain example.com --repo github.com/example-inc/memcached-operator
+operator-sdk init --domain example.com --repo github.com/example/memcached-operator
 ```
 
 **Note**: `operator-sdk` attempts to automatically discover the Go module path of your project by looking for a `go.mod` file, or if in `$GOPATH`, by using the directory path. Use the `--repo` flag to explicitly set the module path.
diff --git a/website/content/en/docs/building-operators/golang/quickstart.md b/website/content/en/docs/building-operators/golang/quickstart.md
index 271b2673..853fd8a5 100644
--- a/website/content/en/docs/building-operators/golang/quickstart.md
+++ b/website/content/en/docs/building-operators/golang/quickstart.md
@@ -19,7 +19,7 @@ This guide walks through an example of building a simple memcached-operator usin
   ```sh
   mkdir memcached-operator
   cd memcached-operator
-  operator-sdk init --domain example.com --repo github.com/example-inc/memcached-operator
+  operator-sdk init --domain example.com --repo github.com/example/memcached-operator
   ```
 
 1. Create a simple Memcached API:
diff --git a/website/content/en/docs/building-operators/golang/references/client.md b/website/content/en/docs/building-operators/golang/references/client.md
index b1a3cadb..f8a76b30 100644
--- a/website/content/en/docs/building-operators/golang/references/client.md
+++ b/website/content/en/docs/building-operators/golang/references/client.md
@@ -316,7 +316,7 @@ Example:
 ```Go
 import (
     "context"
-    cachev1alpha1 "github.com/example-inc/memcached-operator/pkg/apis/cache/v1alpha1"
+    cachev1alpha1 "github.com/example/memcached-operator/pkg/apis/cache/v1alpha1"
     ctrl "sigs.k8s.io/controller-runtime"
 )
 
diff --git a/website/content/en/docs/building-operators/golang/references/event-filtering.md b/website/content/en/docs/building-operators/golang/references/event-filtering.md
index ea193ad2..3e46d122 100644
--- a/website/content/en/docs/building-operators/golang/references/event-filtering.md
+++ b/website/content/en/docs/building-operators/golang/references/event-filtering.md
@@ -66,7 +66,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
-	cachev1alpha1 "github.com/example-inc/app-operator/apis/cache/v1alpha1"
+	cachev1alpha1 "github.com/example/app-operator/apis/cache/v1alpha1"
 )
 
 ...
diff --git a/website/content/en/docs/building-operators/golang/tutorial.md b/website/content/en/docs/building-operators/golang/tutorial.md
index d8799fb6..7cf12a48 100644
--- a/website/content/en/docs/building-operators/golang/tutorial.md
+++ b/website/content/en/docs/building-operators/golang/tutorial.md
@@ -23,7 +23,7 @@ mkdir -p $HOME/projects/memcached-operator
 cd $HOME/projects/memcached-operator
 # we'll use a domain of example.com
 # so all API groups will be <group>.example.com
-operator-sdk init --domain example.com --repo github.com/example-inc/memcached-operator
+operator-sdk init --domain example.com --repo github.com/example/memcached-operator
 ```
 
 To learn about the project directory structure, see [Kubebuilder project layout][kubebuilder_layout_doc] doc.
@@ -217,7 +217,7 @@ Every Controller has a Reconciler object with a `Reconcile()` method that implem
 import (
 	ctrl "sigs.k8s.io/controller-runtime"
 
-	cachev1alpha1 "github.com/example-inc/memcached-operator/api/v1alpha1"
+	cachev1alpha1 "github.com/example/memcached-operator/api/v1alpha1"
 	...
 )
 
diff --git a/website/content/en/docs/building-operators/helm/quickstart.md b/website/content/en/docs/building-operators/helm/quickstart.md
index ebc8b37a..1c6a1fa7 100644
--- a/website/content/en/docs/building-operators/helm/quickstart.md
+++ b/website/content/en/docs/building-operators/helm/quickstart.md
@@ -32,7 +32,7 @@ This guide walks through an example of building a simple nginx-operator powered
 Make sure to define `IMG` when you call `make`:
 
   ```sh
-  export OPERATOR_IMG="quay.io/example-inc/nginx-operator:v0.0.1"
+  export OPERATOR_IMG="quay.io/example/nginx-operator:v0.0.1"
   make docker-build docker-push IMG=$OPERATOR_IMG
   ```
 
@@ -50,7 +50,7 @@ Make sure to define `IMG` when you call `make`:
   ```sh
   make bundle IMG=$OPERATOR_IMG
   # Note the "-bundle" component in the image name below.
-  export BUNDLE_IMG="quay.io/example-inc/nginx-operator-bundle:v0.0.1"
+  export BUNDLE_IMG="quay.io/example/nginx-operator-bundle:v0.0.1"
   make bundle-build BUNDLE_IMG=$BUNDLE_IMG
   make docker-push IMG=$BUNDLE_IMG
   ```
diff --git a/website/content/en/docs/upgrading-sdk-version/v0.1.0-migration-guide.md b/website/content/en/docs/upgrading-sdk-version/v0.1.0-migration-guide.md
index aabdeb62..149d893c 100644
--- a/website/content/en/docs/upgrading-sdk-version/v0.1.0-migration-guide.md
+++ b/website/content/en/docs/upgrading-sdk-version/v0.1.0-migration-guide.md
@@ -19,7 +19,7 @@ $ operator-sdk --version
 operator-sdk version 0.1.0
 
 # Create new project
-$ cd $GOPATH/src/github.com/example-inc/
+$ cd $GOPATH/src/github.com/example/
 $ mv memcached-operator old-memcached-operator
 $ operator-sdk new memcached-operator --skip-git-init
 $ ls
@@ -88,7 +88,7 @@ pkg/controller/
 Inspect the `add()` function in your `pkg/controller/<kind>/<kind>_controller.go` file:
 ```Go
 import (
-    cachev1alpha1 "github.com/example-inc/memcached-operator/pkg/apis/cache/v1alpha1"
+    cachev1alpha1 "github.com/example/memcached-operator/pkg/apis/cache/v1alpha1"
     ...
 )
 
@@ -154,7 +154,7 @@ Be sure to keep the initial section in the `Reconcile()` code that looks up the
 ```Go
 import (
     apierrors "k8s.io/apimachinery/pkg/api/errors"
-    cachev1alpha1 "github.com/example-inc/memcached-operator/pkg/apis/cache/v1alpha1"
+    cachev1alpha1 "github.com/example/memcached-operator/pkg/apis/cache/v1alpha1"
     ...
 )
 func (r *ReconcileMemcached) Reconcile(request reconcile.Request) (reconcile.Result, error) {


Test patch:
diff --git a/images/scorecard-test-kuttl/Dockerfile b/images/scorecard-test-kuttl/Dockerfile
index e8d5fd87..acbf232f 100644
--- a/images/scorecard-test-kuttl/Dockerfile
+++ b/images/scorecard-test-kuttl/Dockerfile
@@ -1,5 +1,5 @@
 #FROM docker.io/kudobuilder/kuttl@sha256:c9c5edc27ba6e5e994ffd8cea03368c0d4e274f3c7a00f51c1e360feb573f24e
-FROM kudobuilder/kuttl:v0.7.1
+FROM kudobuilder/kuttl:v0.8.0
 
 ENV HOME=/opt/scorecard-test-kuttl \
     USER_NAME=scorecard-test-kuttl \
diff --git a/internal/helm/client/client_test.go b/internal/helm/client/client_test.go
new file mode 100644
index 00000000..c223da33
--- /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)
+	}
+}