Added unit test and e2e test

Signed-off-by: Manna Kong <mkong@redhat.com>
operator-framework · Jul 8, 2024 · c6a3bd5 · c6a3bd5
1 parent 748fad8
commit c6a3bd5
Show file tree

Hide file tree

Showing 8 changed files with 173 additions and 8 deletions.
diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go
@@ -84,10 +84,18 @@ type ClusterExtensionSpec struct {
 	// Preflight defines the configuration of preflight checks.
 	Preflight *PreflightConfig `json:"preflight,omitempty"`
 
-	//+kubebuilder:validation:MaxLength:=48
-	//+kubebuilder:validation:Pattern:=^[a-z0-9]+(-[a-z0-9]+)*$
-	// ServiceAccount is used to install and manage content
-	ServiceAccount string `json:"serviceAccount"`
+	// ServiceAccount is used to install and manage resources.
+	// The service account is expected to exist in the InstallNamespace.
+	ServiceAccount ServiceAccountReference `json:"serviceAccount"`
+}
+
+// ServiceAccountReference references a serviceAccount.
+type ServiceAccountReference struct {
+	// name is the metadata.name of the referenced serviceAccount object.
+	//+kubebuilder:validation:MaxLength:=253
+	//+kubebuilder:validation:Pattern:=^[a-z0-9]+([.|-][a-z0-9]+)*$
+	//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="serviceAccount is immutable"
+	Name string `json:"name"`
 }
 
 // PreflightConfig holds the configuration for the preflight checks.

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml b/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml
@@ -73,10 +73,22 @@ spec:
                     type: object
                 type: object
               serviceAccount:
-                description: ServiceAccount is used to install and manage content
-                maxLength: 48
-                pattern: ^[a-z0-9]+(-[a-z0-9]+)*$
-                type: string
+                description: |-
+                  ServiceAccount is used to install and manage resources.
+                  The service account is expected to exist in the InstallNamespace.
+                properties:
+                  name:
+                    description: name is the metadata.name of the referenced serviceAccount
+                      object.
+                    maxLength: 253
+                    pattern: ^[a-z0-9]+([.|-][a-z0-9]+)*$
+                    type: string
+                    x-kubernetes-validations:
+                    - message: serviceAccount is immutable
+                      rule: self == oldSelf
+                required:
+                - name
+                type: object
               upgradeConstraintPolicy:
                 default: Enforce
                 description: Defines the policy for how to handle upgrade constraints

diff --git a/internal/controllers/clusterextension_admission_test.go b/internal/controllers/clusterextension_admission_test.go
@@ -45,6 +45,9 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
 			err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
 				PackageName:      tc.pkgName,
 				InstallNamespace: "default",
+				ServiceAccount: ocv1alpha1.ServiceAccountReference{
+					Name: "default",
+				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for package name %q: %w", tc.pkgName, err)
@@ -134,6 +137,9 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) {
 				PackageName:      "package",
 				Version:          tc.version,
 				InstallNamespace: "default",
+				ServiceAccount: ocv1alpha1.ServiceAccountReference{
+					Name: "default",
+				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for version %q: %w", tc.version, err)
@@ -179,6 +185,9 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
 				PackageName:      "package",
 				Channel:          tc.channelName,
 				InstallNamespace: "default",
+				ServiceAccount: ocv1alpha1.ServiceAccountReference{
+					Name: "default",
+				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for channel %q: %w", tc.channelName, err)
@@ -224,6 +233,9 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
 			err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
 				PackageName:      "package",
 				InstallNamespace: tc.installNamespace,
+				ServiceAccount: ocv1alpha1.ServiceAccountReference{
+					Name: "default",
+				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for installNamespace %q: %w", tc.installNamespace, err)
@@ -235,6 +247,54 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
 	}
 }
 
+func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
+	tooLongError := "spec.serviceAccount.name: Too long: may not be longer than 253"
+	regexMismatchError := "spec.serviceAccount.name in body should match"
+
+	testCases := []struct {
+		name           string
+		serviceAccount string
+		errMsg         string
+	}{
+		{"just alphanumeric", "justalphanumeric1", ""},
+		{"hypen-separated", "hyphenated-name", ""},
+		{"dot-separated", "dotted.name", ""},
+		{"longest valid service account name", strings.Repeat("x", 253), ""},
+		{"too long service account name", strings.Repeat("x", 254), tooLongError},
+		{"no service account name", "", regexMismatchError},
+		{"spaces", "spaces spaces", regexMismatchError},
+		{"capitalized", "Capitalized", regexMismatchError},
+		{"camel case", "camelCase", regexMismatchError},
+		{"invalid characters", "many/invalid$characters+in_name", regexMismatchError},
+		{"starts with hyphen", "-start-with-hyphen", regexMismatchError},
+		{"ends with hyphen", "end-with-hyphen-", regexMismatchError},
+		{"starts with period", ".start-with-period", regexMismatchError},
+		{"ends with period", "end-with-period.", regexMismatchError},
+	}
+
+	t.Parallel()
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			cl := newClient(t)
+			err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
+				PackageName:      "package",
+				InstallNamespace: "default",
+				ServiceAccount: ocv1alpha1.ServiceAccountReference{
+					Name: tc.serviceAccount,
+				},
+			}))
+			if tc.errMsg == "" {
+				require.NoError(t, err, "unexpected error for service account name %q: %w", tc.serviceAccount, err)
+			} else {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), tc.errMsg)
+			}
+		})
+	}
+}
+
 func buildClusterExtension(spec ocv1alpha1.ClusterExtensionSpec) *ocv1alpha1.ClusterExtension {
 	return &ocv1alpha1.ClusterExtension{
 		ObjectMeta: metav1.ObjectMeta{

diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go
@@ -56,6 +56,9 @@ func TestClusterExtensionNonExistentPackage(t *testing.T) {
 		Spec: ocv1alpha1.ClusterExtensionSpec{
 			PackageName:      pkgName,
 			InstallNamespace: "default",
+			ServiceAccount: ocv1alpha1.ServiceAccountReference{
+				Name: "default",
+			},
 		},
 	}
 	require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -98,6 +101,9 @@ func TestClusterExtensionNonExistentVersion(t *testing.T) {
 			PackageName:      pkgName,
 			Version:          "0.50.0", // this version of the package does not exist
 			InstallNamespace: "default",
+			ServiceAccount: ocv1alpha1.ServiceAccountReference{
+				Name: "default",
+			},
 		},
 	}
 	require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -147,6 +153,7 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) {
 	pkgVer := "1.0.0"
 	pkgChan := "beta"
 	installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+	serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
 
 	clusterExtension := &ocv1alpha1.ClusterExtension{
 		ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -155,6 +162,9 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) {
 			Version:          pkgVer,
 			Channel:          pkgChan,
 			InstallNamespace: installNamespace,
+			ServiceAccount: ocv1alpha1.ServiceAccountReference{
+				Name: serviceAccount,
+			},
 		},
 	}
 	err := cl.Create(ctx, clusterExtension)
@@ -206,13 +216,18 @@ func TestClusterExtensionChannelExistsNoVersion(t *testing.T) {
 	pkgVer := ""
 	pkgChan := "beta"
 	installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+	serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
+
 	clusterExtension := &ocv1alpha1.ClusterExtension{
 		ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
 		Spec: ocv1alpha1.ClusterExtensionSpec{
 			PackageName:      pkgName,
 			Version:          pkgVer,
 			Channel:          pkgChan,
 			InstallNamespace: installNamespace,
+			ServiceAccount: ocv1alpha1.ServiceAccountReference{
+				Name: serviceAccount,
+			},
 		},
 	}
 	err := cl.Create(ctx, clusterExtension)
@@ -265,6 +280,9 @@ func TestClusterExtensionVersionNoChannel(t *testing.T) {
 			Version:          pkgVer,
 			Channel:          pkgChan,
 			InstallNamespace: "default",
+			ServiceAccount: ocv1alpha1.ServiceAccountReference{
+				Name: "default",
+			},
 		},
 	}
 	require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -313,6 +331,9 @@ func TestClusterExtensionNoChannel(t *testing.T) {
 			PackageName:      pkgName,
 			Channel:          pkgChan,
 			InstallNamespace: "default",
+			ServiceAccount: ocv1alpha1.ServiceAccountReference{
+				Name: "default",
+			},
 		},
 	}
 	require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -363,6 +384,9 @@ func TestClusterExtensionNoVersion(t *testing.T) {
 			Version:          pkgVer,
 			Channel:          pkgChan,
 			InstallNamespace: "default",
+			ServiceAccount: ocv1alpha1.ServiceAccountReference{
+				Name: "default",
+			},
 		},
 	}
 	require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -443,6 +467,7 @@ func TestClusterExtensionUpgrade(t *testing.T) {
 		pkgVer := "1.0.0"
 		pkgChan := "beta"
 		installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+		serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
 		extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
 		clusterExtension := &ocv1alpha1.ClusterExtension{
 			ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -451,6 +476,9 @@ func TestClusterExtensionUpgrade(t *testing.T) {
 				Version:          pkgVer,
 				Channel:          pkgChan,
 				InstallNamespace: installNamespace,
+				ServiceAccount: ocv1alpha1.ServiceAccountReference{
+					Name: serviceAccount,
+				},
 			},
 		}
 		// Create a cluster extension
@@ -543,6 +571,7 @@ func TestClusterExtensionUpgrade(t *testing.T) {
 		pkgVer := "1.0.0"
 		pkgChan := "beta"
 		installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+		serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
 		extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
 		clusterExtension := &ocv1alpha1.ClusterExtension{
 			ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -551,6 +580,9 @@ func TestClusterExtensionUpgrade(t *testing.T) {
 				Version:          pkgVer,
 				Channel:          pkgChan,
 				InstallNamespace: installNamespace,
+				ServiceAccount: ocv1alpha1.ServiceAccountReference{
+					Name: serviceAccount,
+				},
 			},
 		}
 		// Create a cluster extension
@@ -654,6 +686,7 @@ func TestClusterExtensionUpgrade(t *testing.T) {
 				}()
 
 				installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+				serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
 				extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
 				clusterExtension := &ocv1alpha1.ClusterExtension{
 					ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -663,6 +696,9 @@ func TestClusterExtensionUpgrade(t *testing.T) {
 						Channel:                 "beta",
 						UpgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyIgnore,
 						InstallNamespace:        installNamespace,
+						ServiceAccount: ocv1alpha1.ServiceAccountReference{
+							Name: serviceAccount,
+						},
 					},
 				}
 				// Create a cluster extension
@@ -754,6 +790,7 @@ func TestClusterExtensionDowngrade(t *testing.T) {
 				}()
 
 				installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+				serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
 				extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
 				clusterExtension := &ocv1alpha1.ClusterExtension{
 					ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -762,6 +799,9 @@ func TestClusterExtensionDowngrade(t *testing.T) {
 						Version:          "1.0.1",
 						Channel:          "beta",
 						InstallNamespace: installNamespace,
+						ServiceAccount: ocv1alpha1.ServiceAccountReference{
+							Name: serviceAccount,
+						},
 					},
 				}
 				// Create a cluster extension
@@ -842,6 +882,7 @@ func TestClusterExtensionDowngrade(t *testing.T) {
 				}()
 
 				installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+				serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
 				extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
 				clusterExtension := &ocv1alpha1.ClusterExtension{
 					ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -851,6 +892,9 @@ func TestClusterExtensionDowngrade(t *testing.T) {
 						Channel:                 "beta",
 						UpgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyIgnore,
 						InstallNamespace:        installNamespace,
+						ServiceAccount: ocv1alpha1.ServiceAccountReference{
+							Name: serviceAccount,
+						},
 					},
 				}
 				// Create a cluster extension
@@ -1461,6 +1505,9 @@ func TestClusterExtensionErrorGettingBundles(t *testing.T) {
 		Spec: ocv1alpha1.ClusterExtensionSpec{
 			PackageName:      "prometheus",
 			InstallNamespace: "default",
+			ServiceAccount: ocv1alpha1.ServiceAccountReference{
+				Name: "default",
+			},
 		},
 	}
 	require.NoError(t, cl.Create(ctx, clusterExtension))

diff --git a/internal/controllers/clusterextension_registryv1_validation_test.go b/internal/controllers/clusterextension_registryv1_validation_test.go
@@ -121,12 +121,16 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) {
 			}
 
 			installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+			serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
 			extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
 			clusterExtension := &ocv1alpha1.ClusterExtension{
 				ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
 				Spec: ocv1alpha1.ClusterExtensionSpec{
 					PackageName:      tt.bundle.Package,
 					InstallNamespace: installNamespace,
+					ServiceAccount: ocv1alpha1.ServiceAccountReference{
+						Name: serviceAccount,
+					},
 				},
 			}
 			require.NoError(t, cl.Create(ctx, clusterExtension))