objects with resource policy "keep" should use the annotation-based w…

…atch (#83)

pkg/client/actionclient.go

@@ -21,7 +21,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"strings"
 
 	sdkhandler "github.com/operator-framework/operator-lib/handler"
 	"gomodules.xyz/jsonpatch/v2"
@@ -46,6 +45,7 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"github.com/joelanford/helm-operator/pkg/internal/sdk/controllerutil"
+	"github.com/joelanford/helm-operator/pkg/manifestutil"
 )
 
 type ActionClientGetter interface {
@@ -326,7 +326,7 @@ func (pr *ownerPostRenderer) Run(in *bytes.Buffer) (*bytes.Buffer, error) {
 		if err != nil {
 			return err
 		}
-		if useOwnerRef && !containsResourcePolicyKeep(u.GetAnnotations()) {
+		if useOwnerRef && !manifestutil.HasResourcePolicyKeep(u.GetAnnotations()) {
 			ownerRef := metav1.NewControllerRef(pr.owner, pr.owner.GetObjectKind().GroupVersionKind())
 			ownerRefs := append(u.GetOwnerReferences(), *ownerRef)
 			u.SetOwnerReferences(ownerRefs)
@@ -349,15 +349,3 @@ func (pr *ownerPostRenderer) Run(in *bytes.Buffer) (*bytes.Buffer, error) {
 	}
 	return &out, 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
-}

pkg/client/actionclient_test.go

@@ -21,7 +21,6 @@ import (
 	"context"
 	"errors"
 	"strconv"
-	"strings"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -544,33 +543,6 @@ var _ = Describe("ActionClient", func() {
 			Expect(err).NotTo(BeNil())
 		})
 	})
-
-	var _ = Describe("containsResourcePolicyKeep", func() {
-		It("returns true on base case", func() {
-			annotations := map[string]string{kube.ResourcePolicyAnno: kube.KeepPolicy}
-			Expect(containsResourcePolicyKeep(annotations)).To(BeTrue())
-		})
-		It("returns false when annotation key is not found", func() {
-			annotations := map[string]string{"not-" + kube.ResourcePolicyAnno: kube.KeepPolicy}
-			Expect(containsResourcePolicyKeep(annotations)).To(BeFalse())
-		})
-		It("returns false when annotation value is not 'keep'", func() {
-			annotations := map[string]string{"not-" + kube.ResourcePolicyAnno: "not-" + kube.KeepPolicy}
-			Expect(containsResourcePolicyKeep(annotations)).To(BeFalse())
-		})
-		It("returns true when annotation is uppercase", func() {
-			annotations := map[string]string{kube.ResourcePolicyAnno: strings.ToUpper(kube.KeepPolicy)}
-			Expect(containsResourcePolicyKeep(annotations)).To(BeTrue())
-		})
-		It("returns true when annotation is has whitespace prefix and/or suffix", func() {
-			annotations := map[string]string{kube.ResourcePolicyAnno: " " + kube.KeepPolicy + "  "}
-			Expect(containsResourcePolicyKeep(annotations)).To(BeTrue())
-		})
-		It("returns true when annotation is uppercase and has whitespace prefix and/or suffix", func() {
-			annotations := map[string]string{kube.ResourcePolicyAnno: " " + strings.ToUpper(kube.KeepPolicy) + "  "}
-			Expect(containsResourcePolicyKeep(annotations)).To(BeTrue())
-		})
-	})
 })
 
 func manifestToObjects(manifest string) []client.Object {

pkg/manifestutil/manifest_suite_test.go

@@ -0,0 +1,29 @@
+/*
+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 manifestutil_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestManifest(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Manifest Suite")
+}

pkg/manifestutil/resourcepolicykeep.go

@@ -0,0 +1,35 @@
+/*
+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 manifestutil
+
+import (
+	"strings"
+
+	"helm.sh/helm/v3/pkg/kube"
+)
+
+func HasResourcePolicyKeep(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
+}

pkg/manifestutil/resourcepolicykeep_test.go

@@ -0,0 +1,57 @@
+/*
+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 manifestutil_test
+
+import (
+	"strings"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"helm.sh/helm/v3/pkg/kube"
+
+	"github.com/joelanford/helm-operator/pkg/manifestutil"
+)
+
+var _ = Describe("HasResourcePolicyKeep", func() {
+	It("returns false for nil annotations", func() {
+		Expect(manifestutil.HasResourcePolicyKeep(nil)).To(BeFalse())
+	})
+	It("returns true on base case", func() {
+		annotations := map[string]string{kube.ResourcePolicyAnno: kube.KeepPolicy}
+		Expect(manifestutil.HasResourcePolicyKeep(annotations)).To(BeTrue())
+	})
+	It("returns false when annotation key is not found", func() {
+		annotations := map[string]string{"not-" + kube.ResourcePolicyAnno: kube.KeepPolicy}
+		Expect(manifestutil.HasResourcePolicyKeep(annotations)).To(BeFalse())
+	})
+	It("returns false when annotation value is not 'keep'", func() {
+		annotations := map[string]string{"not-" + kube.ResourcePolicyAnno: "not-" + kube.KeepPolicy}
+		Expect(manifestutil.HasResourcePolicyKeep(annotations)).To(BeFalse())
+	})
+	It("returns true when annotation is uppercase", func() {
+		annotations := map[string]string{kube.ResourcePolicyAnno: strings.ToUpper(kube.KeepPolicy)}
+		Expect(manifestutil.HasResourcePolicyKeep(annotations)).To(BeTrue())
+	})
+	It("returns true when annotation is has whitespace prefix and/or suffix", func() {
+		annotations := map[string]string{kube.ResourcePolicyAnno: " " + kube.KeepPolicy + "  "}
+		Expect(manifestutil.HasResourcePolicyKeep(annotations)).To(BeTrue())
+	})
+	It("returns true when annotation is uppercase and has whitespace prefix and/or suffix", func() {
+		annotations := map[string]string{kube.ResourcePolicyAnno: " " + strings.ToUpper(kube.KeepPolicy) + "  "}
+		Expect(manifestutil.HasResourcePolicyKeep(annotations)).To(BeTrue())
+	})
+})

pkg/reconciler/internal/hook/hook.go

@@ -34,6 +34,7 @@ import (
 	"github.com/joelanford/helm-operator/pkg/hook"
 	"github.com/joelanford/helm-operator/pkg/internal/sdk/controllerutil"
 	"github.com/joelanford/helm-operator/pkg/internal/sdk/predicate"
+	"github.com/joelanford/helm-operator/pkg/manifestutil"
 )
 
 func NewDependentResourceWatcher(c controller.Controller, rm meta.RESTMapper) hook.PostHook {
@@ -77,7 +78,7 @@ func (d *dependentResourceWatcher) Exec(owner *unstructured.Unstructured, rel re
 			return err
 		}
 
-		if useOwnerRef {
+		if useOwnerRef && !manifestutil.HasResourcePolicyKeep(obj.GetAnnotations()) {
 			if err := d.controller.Watch(&source.Kind{Type: &obj}, &handler.EnqueueRequestForOwner{
 				OwnerType:    owner,
 				IsController: true,

pkg/reconciler/internal/hook/hook_test.go

@@ -150,6 +150,18 @@ var _ = Describe("Hook", func() {
 					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
 					Expect(c.WatchCalls[1].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
 				})
+				It("should watch resource policy keep resources with annotation handler", func() {
+					rel = &release.Release{
+						Manifest: strings.Join([]string{rsOwnerNamespaceWithKeep, ssOtherNamespaceWithKeep, clusterRoleWithKeep, clusterRoleBindingWithKeep}, "---\n"),
+					}
+					drw = internalhook.NewDependentResourceWatcher(c, rm)
+					Expect(drw.Exec(owner, *rel, log)).To(Succeed())
+					Expect(c.WatchCalls).To(HaveLen(4))
+					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
+					Expect(c.WatchCalls[1].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
+					Expect(c.WatchCalls[2].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
+					Expect(c.WatchCalls[3].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
+				})
 			})
 
 			Context("when the owner is namespace-scoped", func() {
@@ -165,7 +177,6 @@ var _ = Describe("Hook", func() {
 						},
 					}
 				})
-
 				It("should watch namespace-scoped dependent resources in the same namespace with ownerRef handler", func() {
 					rel = &release.Release{
 						Manifest: strings.Join([]string{rsOwnerNamespace}, "---\n"),
@@ -175,7 +186,6 @@ var _ = Describe("Hook", func() {
 					Expect(c.WatchCalls).To(HaveLen(1))
 					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
 				})
-
 				It("should watch cluster-scoped resources with annotation handler", func() {
 					rel = &release.Release{
 						Manifest: strings.Join([]string{clusterRole}, "---\n"),
@@ -185,7 +195,6 @@ var _ = Describe("Hook", func() {
 					Expect(c.WatchCalls).To(HaveLen(1))
 					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
 				})
-
 				It("should watch namespace-scoped resources in a different namespace with annotation handler", func() {
 					rel = &release.Release{
 						Manifest: strings.Join([]string{ssOtherNamespace}, "---\n"),
@@ -195,6 +204,17 @@ var _ = Describe("Hook", func() {
 					Expect(c.WatchCalls).To(HaveLen(1))
 					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
 				})
+				It("should watch resource policy keep resources with annotation handler", func() {
+					rel = &release.Release{
+						Manifest: strings.Join([]string{rsOwnerNamespaceWithKeep, ssOtherNamespaceWithKeep, clusterRoleWithKeep}, "---\n"),
+					}
+					drw = internalhook.NewDependentResourceWatcher(c, rm)
+					Expect(drw.Exec(owner, *rel, log)).To(Succeed())
+					Expect(c.WatchCalls).To(HaveLen(3))
+					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
+					Expect(c.WatchCalls[1].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
+					Expect(c.WatchCalls[2].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
+				})
 			})
 		})
 	})
@@ -207,24 +227,58 @@ kind: ReplicaSet
 metadata:
   name: testReplicaSet
   namespace: ownerNamespace
+`
+	rsOwnerNamespaceWithKeep = `
+apiVersion: apps/v1
+kind: ReplicaSet
+metadata:
+  name: testReplicaSet
+  namespace: ownerNamespace
+  annotations:
+    helm.sh/resource-policy: keep
 `
 	ssOtherNamespace = `
 apiVersion: apps/v1
 kind: StatefulSet
 metadata:
   name: otherTestStatefulSet
   namespace: otherNamespace
+`
+	ssOtherNamespaceWithKeep = `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: otherTestStatefulSet
+  namespace: otherNamespace
+  annotations:
+    helm.sh/resource-policy: keep
 `
 	clusterRole = `
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:
   name: testClusterRole
+`
+	clusterRoleWithKeep = `
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: testClusterRole
+  annotations:
+    helm.sh/resource-policy: keep
 `
 	clusterRoleBinding = `
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
 metadata:
   name: testClusterRoleBinding
+`
+	clusterRoleBindingWithKeep = `
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: testClusterRoleBinding
+  annotations:
+    helm.sh/resource-policy: keep
 `
 )