Skip to content

Commit feaeece

Browse files
authored
✨ Add Service Account requirement to ClusterExtension (#1011)
* Added service account as a required, immutable field for ClusterExtensions Signed-off-by: Manna Kong <[email protected]> * Added unit test and e2e test Signed-off-by: Manna Kong <[email protected]> --------- Signed-off-by: Manna Kong <[email protected]>
1 parent 10ebfdc commit feaeece

9 files changed

+178
-0
lines changed

api/v1alpha1/clusterextension_types.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,19 @@ type ClusterExtensionSpec struct {
8383
//+kubebuilder:Optional
8484
// Preflight defines the configuration of preflight checks.
8585
Preflight *PreflightConfig `json:"preflight,omitempty"`
86+
87+
// ServiceAccount is used to install and manage resources.
88+
// The service account is expected to exist in the InstallNamespace.
89+
ServiceAccount ServiceAccountReference `json:"serviceAccount"`
90+
}
91+
92+
// ServiceAccountReference references a serviceAccount.
93+
type ServiceAccountReference struct {
94+
// name is the metadata.name of the referenced serviceAccount object.
95+
//+kubebuilder:validation:MaxLength:=253
96+
//+kubebuilder:validation:Pattern:=^[a-z0-9]+([.|-][a-z0-9]+)*$
97+
//+kubebuilder:validation:XValidation:rule="self == oldSelf",message="name is immutable"
98+
Name string `json:"name"`
8699
}
87100

88101
// PreflightConfig holds the configuration for the preflight checks.

api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,23 @@ spec:
7272
type: boolean
7373
type: object
7474
type: object
75+
serviceAccount:
76+
description: |-
77+
ServiceAccount is used to install and manage resources.
78+
The service account is expected to exist in the InstallNamespace.
79+
properties:
80+
name:
81+
description: name is the metadata.name of the referenced serviceAccount
82+
object.
83+
maxLength: 253
84+
pattern: ^[a-z0-9]+([.|-][a-z0-9]+)*$
85+
type: string
86+
x-kubernetes-validations:
87+
- message: name is immutable
88+
rule: self == oldSelf
89+
required:
90+
- name
91+
type: object
7592
upgradeConstraintPolicy:
7693
default: Enforce
7794
description: Defines the policy for how to handle upgrade constraints
@@ -93,6 +110,7 @@ spec:
93110
required:
94111
- installNamespace
95112
- packageName
113+
- serviceAccount
96114
type: object
97115
status:
98116
description: ClusterExtensionStatus defines the observed state of ClusterExtension

config/samples/olm_v1alpha1_clusterextension.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,5 @@ spec:
66
installNamespace: default
77
packageName: argocd-operator
88
version: 0.6.0
9+
serviceAccount:
10+
name: argocd-installer

internal/controllers/clusterextension_admission_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
4545
err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
4646
PackageName: tc.pkgName,
4747
InstallNamespace: "default",
48+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
49+
Name: "default",
50+
},
4851
}))
4952
if tc.errMsg == "" {
5053
require.NoError(t, err, "unexpected error for package name %q: %w", tc.pkgName, err)
@@ -134,6 +137,9 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) {
134137
PackageName: "package",
135138
Version: tc.version,
136139
InstallNamespace: "default",
140+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
141+
Name: "default",
142+
},
137143
}))
138144
if tc.errMsg == "" {
139145
require.NoError(t, err, "unexpected error for version %q: %w", tc.version, err)
@@ -179,6 +185,9 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
179185
PackageName: "package",
180186
Channel: tc.channelName,
181187
InstallNamespace: "default",
188+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
189+
Name: "default",
190+
},
182191
}))
183192
if tc.errMsg == "" {
184193
require.NoError(t, err, "unexpected error for channel %q: %w", tc.channelName, err)
@@ -224,6 +233,9 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
224233
err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
225234
PackageName: "package",
226235
InstallNamespace: tc.installNamespace,
236+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
237+
Name: "default",
238+
},
227239
}))
228240
if tc.errMsg == "" {
229241
require.NoError(t, err, "unexpected error for installNamespace %q: %w", tc.installNamespace, err)
@@ -235,6 +247,54 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
235247
}
236248
}
237249

250+
func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
251+
tooLongError := "spec.serviceAccount.name: Too long: may not be longer than 253"
252+
regexMismatchError := "spec.serviceAccount.name in body should match"
253+
254+
testCases := []struct {
255+
name string
256+
serviceAccount string
257+
errMsg string
258+
}{
259+
{"just alphanumeric", "justalphanumeric1", ""},
260+
{"hypen-separated", "hyphenated-name", ""},
261+
{"dot-separated", "dotted.name", ""},
262+
{"longest valid service account name", strings.Repeat("x", 253), ""},
263+
{"too long service account name", strings.Repeat("x", 254), tooLongError},
264+
{"no service account name", "", regexMismatchError},
265+
{"spaces", "spaces spaces", regexMismatchError},
266+
{"capitalized", "Capitalized", regexMismatchError},
267+
{"camel case", "camelCase", regexMismatchError},
268+
{"invalid characters", "many/invalid$characters+in_name", regexMismatchError},
269+
{"starts with hyphen", "-start-with-hyphen", regexMismatchError},
270+
{"ends with hyphen", "end-with-hyphen-", regexMismatchError},
271+
{"starts with period", ".start-with-period", regexMismatchError},
272+
{"ends with period", "end-with-period.", regexMismatchError},
273+
}
274+
275+
t.Parallel()
276+
for _, tc := range testCases {
277+
tc := tc
278+
t.Run(tc.name, func(t *testing.T) {
279+
t.Parallel()
280+
cl := newClient(t)
281+
err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
282+
PackageName: "package",
283+
InstallNamespace: "default",
284+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
285+
Name: tc.serviceAccount,
286+
},
287+
}))
288+
if tc.errMsg == "" {
289+
require.NoError(t, err, "unexpected error for service account name %q: %w", tc.serviceAccount, err)
290+
} else {
291+
require.Error(t, err)
292+
require.Contains(t, err.Error(), tc.errMsg)
293+
}
294+
})
295+
}
296+
}
297+
238298
func buildClusterExtension(spec ocv1alpha1.ClusterExtensionSpec) *ocv1alpha1.ClusterExtension {
239299
return &ocv1alpha1.ClusterExtension{
240300
ObjectMeta: metav1.ObjectMeta{

internal/controllers/clusterextension_controller_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ func TestClusterExtensionNonExistentPackage(t *testing.T) {
5656
Spec: ocv1alpha1.ClusterExtensionSpec{
5757
PackageName: pkgName,
5858
InstallNamespace: "default",
59+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
60+
Name: "default",
61+
},
5962
},
6063
}
6164
require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -98,6 +101,9 @@ func TestClusterExtensionNonExistentVersion(t *testing.T) {
98101
PackageName: pkgName,
99102
Version: "0.50.0", // this version of the package does not exist
100103
InstallNamespace: "default",
104+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
105+
Name: "default",
106+
},
101107
},
102108
}
103109
require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -147,6 +153,7 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) {
147153
pkgVer := "1.0.0"
148154
pkgChan := "beta"
149155
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
156+
serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
150157

151158
clusterExtension := &ocv1alpha1.ClusterExtension{
152159
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -155,6 +162,9 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) {
155162
Version: pkgVer,
156163
Channel: pkgChan,
157164
InstallNamespace: installNamespace,
165+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
166+
Name: serviceAccount,
167+
},
158168
},
159169
}
160170
err := cl.Create(ctx, clusterExtension)
@@ -206,13 +216,18 @@ func TestClusterExtensionChannelExistsNoVersion(t *testing.T) {
206216
pkgVer := ""
207217
pkgChan := "beta"
208218
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
219+
serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
220+
209221
clusterExtension := &ocv1alpha1.ClusterExtension{
210222
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
211223
Spec: ocv1alpha1.ClusterExtensionSpec{
212224
PackageName: pkgName,
213225
Version: pkgVer,
214226
Channel: pkgChan,
215227
InstallNamespace: installNamespace,
228+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
229+
Name: serviceAccount,
230+
},
216231
},
217232
}
218233
err := cl.Create(ctx, clusterExtension)
@@ -265,6 +280,9 @@ func TestClusterExtensionVersionNoChannel(t *testing.T) {
265280
Version: pkgVer,
266281
Channel: pkgChan,
267282
InstallNamespace: "default",
283+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
284+
Name: "default",
285+
},
268286
},
269287
}
270288
require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -313,6 +331,9 @@ func TestClusterExtensionNoChannel(t *testing.T) {
313331
PackageName: pkgName,
314332
Channel: pkgChan,
315333
InstallNamespace: "default",
334+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
335+
Name: "default",
336+
},
316337
},
317338
}
318339
require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -363,6 +384,9 @@ func TestClusterExtensionNoVersion(t *testing.T) {
363384
Version: pkgVer,
364385
Channel: pkgChan,
365386
InstallNamespace: "default",
387+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
388+
Name: "default",
389+
},
366390
},
367391
}
368392
require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -443,6 +467,7 @@ func TestClusterExtensionUpgrade(t *testing.T) {
443467
pkgVer := "1.0.0"
444468
pkgChan := "beta"
445469
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
470+
serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
446471
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
447472
clusterExtension := &ocv1alpha1.ClusterExtension{
448473
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -451,6 +476,9 @@ func TestClusterExtensionUpgrade(t *testing.T) {
451476
Version: pkgVer,
452477
Channel: pkgChan,
453478
InstallNamespace: installNamespace,
479+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
480+
Name: serviceAccount,
481+
},
454482
},
455483
}
456484
// Create a cluster extension
@@ -543,6 +571,7 @@ func TestClusterExtensionUpgrade(t *testing.T) {
543571
pkgVer := "1.0.0"
544572
pkgChan := "beta"
545573
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
574+
serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
546575
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
547576
clusterExtension := &ocv1alpha1.ClusterExtension{
548577
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -551,6 +580,9 @@ func TestClusterExtensionUpgrade(t *testing.T) {
551580
Version: pkgVer,
552581
Channel: pkgChan,
553582
InstallNamespace: installNamespace,
583+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
584+
Name: serviceAccount,
585+
},
554586
},
555587
}
556588
// Create a cluster extension
@@ -654,6 +686,7 @@ func TestClusterExtensionUpgrade(t *testing.T) {
654686
}()
655687

656688
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
689+
serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
657690
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
658691
clusterExtension := &ocv1alpha1.ClusterExtension{
659692
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -663,6 +696,9 @@ func TestClusterExtensionUpgrade(t *testing.T) {
663696
Channel: "beta",
664697
UpgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyIgnore,
665698
InstallNamespace: installNamespace,
699+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
700+
Name: serviceAccount,
701+
},
666702
},
667703
}
668704
// Create a cluster extension
@@ -754,6 +790,7 @@ func TestClusterExtensionDowngrade(t *testing.T) {
754790
}()
755791

756792
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
793+
serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
757794
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
758795
clusterExtension := &ocv1alpha1.ClusterExtension{
759796
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -762,6 +799,9 @@ func TestClusterExtensionDowngrade(t *testing.T) {
762799
Version: "1.0.1",
763800
Channel: "beta",
764801
InstallNamespace: installNamespace,
802+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
803+
Name: serviceAccount,
804+
},
765805
},
766806
}
767807
// Create a cluster extension
@@ -842,6 +882,7 @@ func TestClusterExtensionDowngrade(t *testing.T) {
842882
}()
843883

844884
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
885+
serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
845886
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
846887
clusterExtension := &ocv1alpha1.ClusterExtension{
847888
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -851,6 +892,9 @@ func TestClusterExtensionDowngrade(t *testing.T) {
851892
Channel: "beta",
852893
UpgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyIgnore,
853894
InstallNamespace: installNamespace,
895+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
896+
Name: serviceAccount,
897+
},
854898
},
855899
}
856900
// Create a cluster extension
@@ -1461,6 +1505,9 @@ func TestClusterExtensionErrorGettingBundles(t *testing.T) {
14611505
Spec: ocv1alpha1.ClusterExtensionSpec{
14621506
PackageName: "prometheus",
14631507
InstallNamespace: "default",
1508+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
1509+
Name: "default",
1510+
},
14641511
},
14651512
}
14661513
require.NoError(t, cl.Create(ctx, clusterExtension))

internal/controllers/clusterextension_registryv1_validation_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,16 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) {
121121
}
122122

123123
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
124+
serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
124125
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
125126
clusterExtension := &ocv1alpha1.ClusterExtension{
126127
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
127128
Spec: ocv1alpha1.ClusterExtensionSpec{
128129
PackageName: tt.bundle.Package,
129130
InstallNamespace: installNamespace,
131+
ServiceAccount: ocv1alpha1.ServiceAccountReference{
132+
Name: serviceAccount,
133+
},
130134
},
131135
}
132136
require.NoError(t, cl.Create(ctx, clusterExtension))

0 commit comments

Comments
 (0)