enable OwnerReferencesPermissionEnforcement in project kind config (#1105)

Signed-off-by: Joe Lanford <[email protected]>

Author: joelanford

Diff for: config/samples/olm_v1alpha1_clusterextension.yaml

@@ -28,7 +28,12 @@ kind: ClusterRole
 metadata:
   name: argocd-installer-clusterrole
 rules:
-# Manage ArgoCD CRDs 
+# Allow ClusterExtension to set blockOwnerDeletion ownerReferences
+- apiGroups: [olm.operatorframework.io]
+  resources: [clusterextensions/finalizers]
+  verbs: [update]
+  resourceNames: [argocd]
+# Manage ArgoCD CRDs
 - apiGroups: [apiextensions.k8s.io]
   resources: [customresourcedefinitions]
   verbs: [create]
@@ -221,32 +226,32 @@ metadata:
 rules:
 - apiGroups: [""]
   resources: [serviceaccounts]
-  verbs: [get, list, watch, create, update, patch, delete]
-  resourceNames: [argocd-operator-controller-manager]
+  verbs: [create]
 - apiGroups: [""]
   resources: [serviceaccounts]
+  verbs: [get, list, watch, update, patch, delete]
+  resourceNames: [argocd-operator-controller-manager]
+- apiGroups: [""]
+  resources: [configmaps]
   verbs: [create]
 - apiGroups: [""]
   resources: [configmaps]
-  verbs: [get, list, watch, create, update, patch, delete]
+  verbs: [get, list, watch, update, patch, delete]
   resourceNames: [argocd-operator-manager-config]
 - apiGroups: [""]
-  resources: [configmaps]
+  resources: [services]
   verbs: [create]
 - apiGroups: [""]
   resources: [services]
-  verbs: [get, list, watch, create, update, patch, delete]
+  verbs: [get, list, watch, update, patch, delete]
   resourceNames: [argocd-operator-controller-manager-metrics-service]
-- apiGroups: [""]
-  resources: [services]
-  verbs: [create]
 - apiGroups: [apps]
   resources: [deployments]
-  verbs: [get, list, watch, create, update, patch, delete]
-  resourceNames: [argocd-operator-controller-manager]
+  verbs: [create]
 - apiGroups: [apps]
   resources: [deployments]
-  verbs: [create]
+  verbs: [get, list, watch, update, patch, delete]
+  resourceNames: [argocd-operator-controller-manager]
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding

Diff for: docs/drafts/permissions-for-owner-references-permission-enforcement-plugin.md

@@ -0,0 +1,13 @@
+# Configuring a service account when the cluster uses the `OwnerReferencesPermissionEnforcement` admission plugin
+
+The [`OwnerReferencesPermissionEnforcement`](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#ownerreferencespermissionenforcement) admission plugin requires a user to have permission to set finalizers on owner objects when creating or updating an object to contain an `ownerReference` with `blockOwnerDeletion: true`.
+
+When operator-controller installs or upgrades a `ClusterExtension`, it sets an `ownerReference` on each object with `blockOwnerDeletion: true`. Therefore serviceaccounts configured in `.spec.serviceAccount.name` must have the following permission in a bound `ClusterRole`:
+
+   ```yaml
+   - apiGroups: ["olm.operatorframework.io"]
+     resources: ["clusterextensions/finalizers"]
+     verbs: ["update"]
+     resourceNames: ["<clusterExtensionName>"]
+   ```
+

Diff for: kind-config.yaml

@@ -8,3 +8,9 @@ nodes:
         hostPort: 30000
         listenAddress: "127.0.0.1"
         protocol: tcp
+    kubeadmConfigPatches:
+      - |
+        kind: ClusterConfiguration
+        apiServer:
+            extraArgs:
+              enable-admission-plugins: OwnerReferencesPermissionEnforcement

Diff for: test/e2e/cluster_extension_install_test.go

@@ -39,7 +39,7 @@ const (
 var pollDuration = time.Minute
 var pollInterval = time.Second
 
-func createServiceAccount(ctx context.Context, name types.NamespacedName) (*corev1.ServiceAccount, error) {
+func createServiceAccount(ctx context.Context, name types.NamespacedName, clusterExtensionName string) (*corev1.ServiceAccount, error) {
 	sa := &corev1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name.Name,
@@ -55,6 +55,18 @@ func createServiceAccount(ctx context.Context, name types.NamespacedName) (*core
 			Name: name.Name,
 		},
 		Rules: []rbacv1.PolicyRule{
+			{
+				APIGroups: []string{
+					"olm.operatorframework.io",
+				},
+				Resources: []string{
+					"clusterextensions/finalizers",
+				},
+				Verbs: []string{
+					"update",
+				},
+				ResourceNames: []string{clusterExtensionName},
+			},
 			{
 				APIGroups: []string{
 					"",
@@ -178,7 +190,7 @@ func testInit(t *testing.T) (*ocv1alpha1.ClusterExtension, *catalogd.ClusterCata
 		Namespace: "default",
 	}
 
-	sa, err := createServiceAccount(context.Background(), defaultNamespace)
+	sa, err := createServiceAccount(context.Background(), defaultNamespace, clusterExtensionName)
 	require.NoError(t, err)
 	return clusterExtension, extensionCatalog, sa
 }
@@ -487,7 +499,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
 			Name: clusterExtensionName,
 		},
 	}
-	sa, err := createServiceAccount(context.Background(), types.NamespacedName{Name: clusterExtensionName, Namespace: "default"})
+	sa, err := createServiceAccount(context.Background(), types.NamespacedName{Name: clusterExtensionName, Namespace: "default"}, clusterExtensionName)
 	require.NoError(t, err)
 	defer testCleanup(t, extensionCatalog, clusterExtension, sa)
 	defer getArtifactsOutput(t)

Diff for: test/extension-developer-e2e/extension_developer_test.go

@@ -14,7 +14,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -39,31 +38,68 @@ func TestExtensionDeveloper(t *testing.T) {
 	require.NoError(t, err)
 
 	ctx := context.Background()
-	saName := fmt.Sprintf("serviceaccounts-%s", rand.String(8))
-	name := types.NamespacedName{
-		Name:      saName,
-		Namespace: "default",
+
+	catalog := &catalogd.ClusterCatalog{
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName: "catalog",
+		},
+		Spec: catalogd.ClusterCatalogSpec{
+			Source: catalogd.CatalogSource{
+				Type: catalogd.SourceTypeImage,
+				Image: &catalogd.ImageSource{
+					Ref:                   os.Getenv("CATALOG_IMG"),
+					InsecureSkipTLSVerify: true,
+				},
+			},
+		},
 	}
+	require.NoError(t, c.Create(context.Background(), catalog))
+
+	installNamespace := "default"
 
 	sa := &corev1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      name.Name,
-			Namespace: name.Namespace,
+			Name:      fmt.Sprintf("serviceaccount-%s", rand.String(8)),
+			Namespace: installNamespace,
 		},
 	}
 	require.NoError(t, c.Create(ctx, sa))
 
+	clusterExtension := &ocv1alpha1.ClusterExtension{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "registryv1",
+		},
+		Spec: ocv1alpha1.ClusterExtensionSpec{
+			PackageName:      os.Getenv("REG_PKG_NAME"),
+			InstallNamespace: installNamespace,
+			ServiceAccount: ocv1alpha1.ServiceAccountReference{
+				Name: sa.Name,
+			},
+		},
+	}
+
 	cr := &rbacv1.ClusterRole{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: name.Name,
+			Name: fmt.Sprintf("clusterrole-%s", rand.String(8)),
 		},
 		Rules: []rbacv1.PolicyRule{
+			{
+				APIGroups: []string{
+					"olm.operatorframework.io",
+				},
+				Resources: []string{
+					"clusterextensions/finalizers",
+				},
+				Verbs: []string{
+					"update",
+				},
+				ResourceNames: []string{clusterExtension.Name},
+			},
 			{
 				APIGroups: []string{
 					"",
 				},
 				Resources: []string{
-					"secrets", // for helm
 					"services",
 					"serviceaccounts",
 				},
@@ -139,72 +175,36 @@ func TestExtensionDeveloper(t *testing.T) {
 
 	crb := &rbacv1.ClusterRoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: name.Name,
+			Name: fmt.Sprintf("clusterrolebinding-%s", rand.String(8)),
 		},
 		Subjects: []rbacv1.Subject{
 			{
 				Kind:      "ServiceAccount",
-				Name:      name.Name,
-				Namespace: name.Namespace,
+				Name:      sa.Name,
+				Namespace: sa.Namespace,
 			},
 		},
 		RoleRef: rbacv1.RoleRef{
 			APIGroup: "rbac.authorization.k8s.io",
 			Kind:     "ClusterRole",
-			Name:     name.Name,
+			Name:     cr.Name,
 		},
 	}
 	require.NoError(t, c.Create(ctx, crb))
 
-	clusterExtensions := []*ocv1alpha1.ClusterExtension{
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "registryv1",
-			},
-			Spec: ocv1alpha1.ClusterExtensionSpec{
-				PackageName:      os.Getenv("REG_PKG_NAME"),
-				InstallNamespace: "default",
-				ServiceAccount: ocv1alpha1.ServiceAccountReference{
-					Name: saName,
-				},
-			},
-		},
-	}
-
-	for _, ce := range clusterExtensions {
-		clusterExtension := ce
-		t.Run(clusterExtension.ObjectMeta.Name, func(t *testing.T) {
-			t.Parallel()
-			catalog := &catalogd.ClusterCatalog{
-				ObjectMeta: metav1.ObjectMeta{
-					GenerateName: "catalog",
-				},
-				Spec: catalogd.ClusterCatalogSpec{
-					Source: catalogd.CatalogSource{
-						Type: catalogd.SourceTypeImage,
-						Image: &catalogd.ImageSource{
-							Ref:                   os.Getenv("CATALOG_IMG"),
-							InsecureSkipTLSVerify: true,
-						},
-					},
-				},
-			}
-			t.Logf("When creating an ClusterExtension that references a package with a %q bundle type", clusterExtension.ObjectMeta.Name)
-			require.NoError(t, c.Create(context.Background(), catalog))
-			require.NoError(t, c.Create(context.Background(), clusterExtension))
-			t.Log("It should have a status condition type of Installed with a status of True and a reason of Success")
-			require.EventuallyWithT(t, func(ct *assert.CollectT) {
-				ext := &ocv1alpha1.ClusterExtension{}
-				assert.NoError(ct, c.Get(context.Background(), client.ObjectKeyFromObject(clusterExtension), ext))
-				cond := meta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeInstalled)
-				if !assert.NotNil(ct, cond) {
-					return
-				}
-				assert.Equal(ct, metav1.ConditionTrue, cond.Status)
-				assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
-			}, 2*time.Minute, time.Second)
-			require.NoError(t, c.Delete(context.Background(), catalog))
-			require.NoError(t, c.Delete(context.Background(), clusterExtension))
-		})
-	}
+	t.Logf("When creating an ClusterExtension that references a package with a %q bundle type", clusterExtension.ObjectMeta.Name)
+	require.NoError(t, c.Create(context.Background(), clusterExtension))
+	t.Log("It should have a status condition type of Installed with a status of True and a reason of Success")
+	require.EventuallyWithT(t, func(ct *assert.CollectT) {
+		ext := &ocv1alpha1.ClusterExtension{}
+		assert.NoError(ct, c.Get(context.Background(), client.ObjectKeyFromObject(clusterExtension), ext))
+		cond := meta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeInstalled)
+		if !assert.NotNil(ct, cond) {
+			return
+		}
+		assert.Equal(ct, metav1.ConditionTrue, cond.Status)
+		assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
+	}, 2*time.Minute, time.Second)
+	require.NoError(t, c.Delete(context.Background(), catalog))
+	require.NoError(t, c.Delete(context.Background(), clusterExtension))
 }