From ce9879ac532e7d1066c0a416b156cdf954cb1e49 Mon Sep 17 00:00:00 2001 From: Jon Donovan Date: Thu, 12 Mar 2020 13:13:59 -0700 Subject: [PATCH 1/4] Delete roles and rolebindings as the final step of cleanup. This allows us to take advantage of the permissions granted to cluster admins when performing cleanup. If the approach in https://github.com/knative/serving-operator/pull/291 is approved, this will become necessary. Additionally, it makes sense to remove roles as the final step to allow human Operators to modify any resources they may have permissions on as a result of the Knative installation (that is, we should not remove any access until we are 'almost done' cleaning up). --- pkg/reconciler/knativeserving/knativeserving_controller.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/knativeserving/knativeserving_controller.go b/pkg/reconciler/knativeserving/knativeserving_controller.go index 969fd2b5..32af82eb 100644 --- a/pkg/reconciler/knativeserving/knativeserving_controller.go +++ b/pkg/reconciler/knativeserving/knativeserving_controller.go @@ -246,11 +246,16 @@ func (r *Reconciler) delete(instance *servingv1alpha1.KnativeServing) error { if len(instance.GetFinalizers()) == 0 || instance.GetFinalizers()[0] != finalizerName { return nil } + var RbacResources = mf.Any(mf.ByKind("Role"), mf.ByKind("ClusterRole"), mf.ByKind("RoleBinding"), mf.ByKind("ClusterRoleBinding")) if len(r.servings) == 0 { if err := r.config.Filter(mf.ByKind("Deployment")).Delete(); err != nil { return err } - if err := r.config.Filter(mf.NoCRDs).Delete(); err != nil { + if err := r.config.Filter(mf.All(mf.NoCRDs, mf.None(RbacResources))).Delete(); err != nil { + return err + } + // Delete Roles last, as they may be granting us permissions that are necessary. + if err := r.config.Filter(RbacResources).Delete(); err != nil { return err } } From 6479312ea9454222eb40d615e34d7ececf457ef5 Mon Sep 17 00:00:00 2001 From: Jon Donovan Date: Thu, 12 Mar 2020 13:16:44 -0700 Subject: [PATCH 2/4] Gofmt --- pkg/reconciler/knativeserving/knativeserving_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/knativeserving/knativeserving_controller.go b/pkg/reconciler/knativeserving/knativeserving_controller.go index 32af82eb..bd8c4129 100644 --- a/pkg/reconciler/knativeserving/knativeserving_controller.go +++ b/pkg/reconciler/knativeserving/knativeserving_controller.go @@ -255,7 +255,7 @@ func (r *Reconciler) delete(instance *servingv1alpha1.KnativeServing) error { return err } // Delete Roles last, as they may be granting us permissions that are necessary. - if err := r.config.Filter(RbacResources).Delete(); err != nil { + if err := r.config.Filter(RbacResources).Delete(); err != nil { return err } } From c3e3f2ddafc4524c2db71d1f1c31ac19a8dedd93 Mon Sep 17 00:00:00 2001 From: Jon Donovan Date: Thu, 12 Mar 2020 15:05:18 -0700 Subject: [PATCH 3/4] More golang idiomatic var names. --- .../knativeserving/knativeserving_controller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/reconciler/knativeserving/knativeserving_controller.go b/pkg/reconciler/knativeserving/knativeserving_controller.go index bd8c4129..26713b48 100644 --- a/pkg/reconciler/knativeserving/knativeserving_controller.go +++ b/pkg/reconciler/knativeserving/knativeserving_controller.go @@ -246,16 +246,16 @@ func (r *Reconciler) delete(instance *servingv1alpha1.KnativeServing) error { if len(instance.GetFinalizers()) == 0 || instance.GetFinalizers()[0] != finalizerName { return nil } - var RbacResources = mf.Any(mf.ByKind("Role"), mf.ByKind("ClusterRole"), mf.ByKind("RoleBinding"), mf.ByKind("ClusterRoleBinding")) + var RBAC = mf.Any(mf.ByKind("Role"), mf.ByKind("ClusterRole"), mf.ByKind("RoleBinding"), mf.ByKind("ClusterRoleBinding")) if len(r.servings) == 0 { if err := r.config.Filter(mf.ByKind("Deployment")).Delete(); err != nil { return err } - if err := r.config.Filter(mf.All(mf.NoCRDs, mf.None(RbacResources))).Delete(); err != nil { + if err := r.config.Filter(mf.All(mf.NoCRDs, mf.None(RBAC))).Delete(); err != nil { return err } - // Delete Roles last, as they may be granting us permissions that are necessary. - if err := r.config.Filter(RbacResources).Delete(); err != nil { + // Delete Roles last, as they may be useful for human operators to clean up. + if err := r.config.Filter(RBAC).Delete(); err != nil { return err } } From dac534a7a109aa80f6bf00cf6b458322e921487d Mon Sep 17 00:00:00 2001 From: Jon Donovan Date: Fri, 13 Mar 2020 11:32:00 -0700 Subject: [PATCH 4/4] Remove redundant All() --- pkg/reconciler/knativeserving/knativeserving_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/knativeserving/knativeserving_controller.go b/pkg/reconciler/knativeserving/knativeserving_controller.go index 26713b48..ad1aae4c 100644 --- a/pkg/reconciler/knativeserving/knativeserving_controller.go +++ b/pkg/reconciler/knativeserving/knativeserving_controller.go @@ -251,7 +251,7 @@ func (r *Reconciler) delete(instance *servingv1alpha1.KnativeServing) error { if err := r.config.Filter(mf.ByKind("Deployment")).Delete(); err != nil { return err } - if err := r.config.Filter(mf.All(mf.NoCRDs, mf.None(RBAC))).Delete(); err != nil { + if err := r.config.Filter(mf.NoCRDs, mf.None(RBAC)).Delete(); err != nil { return err } // Delete Roles last, as they may be useful for human operators to clean up.