Skip to content

Commit f087c14

Browse files
kevinrizzaankitathomas
authored andcommitted
Validate SA names do not match csv deployment spec (openshift#144)
OLM is unable to properly handle the case where a bundle author explicitly adds a hardcoded reference to a service account with a name that matches the name of the service account defined by the deployment spec in the CSV. This commit adds a validation method to ensure that the name of the service account in a bundle does not match deployment.Spec.Template.Spec.ServiceAccountName in the csv's spec Upstream-repository: api Upstream-commit: 0fe04f80e0102dc6973109a221a7babd6bee005e
1 parent 8da694d commit f087c14

File tree

10 files changed

+423
-0
lines changed

10 files changed

+423
-0
lines changed

staging/api/pkg/validation/internal/bundle.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
99
"github.com/operator-framework/api/pkg/validation/errors"
1010
interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
11+
v1 "k8s.io/api/core/v1"
12+
"k8s.io/apimachinery/pkg/runtime"
1113
"k8s.io/apimachinery/pkg/runtime/schema"
1214
)
1315

@@ -26,9 +28,35 @@ func validateBundles(objs ...interface{}) (results []errors.ManifestResult) {
2628
func validateBundle(bundle *manifests.Bundle) (result errors.ManifestResult) {
2729
result = validateOwnedCRDs(bundle, bundle.CSV)
2830
result.Name = bundle.CSV.Spec.Version.String()
31+
saErrors := validateServiceAccounts(bundle)
32+
if saErrors != nil {
33+
result.Add(saErrors...)
34+
}
2935
return result
3036
}
3137

38+
func validateServiceAccounts(bundle *manifests.Bundle) []errors.Error {
39+
// get service account names defined in the csv
40+
saNamesFromCSV := make(map[string]struct{}, 0)
41+
for _, deployment := range bundle.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
42+
saName := deployment.Spec.Template.Spec.ServiceAccountName
43+
saNamesFromCSV[saName] = struct{}{}
44+
}
45+
46+
// find any hardcoded service account objects are in the bundle, then check if they match any sa definition in the csv
47+
var errs []errors.Error
48+
for _, obj := range bundle.Objects {
49+
sa := v1.ServiceAccount{}
50+
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &sa); err == nil {
51+
if _, ok := saNamesFromCSV[sa.Name]; ok {
52+
errs = append(errs, errors.ErrInvalidBundle("invalid service account found in bundle. sa name cannot match service account defined for deployment spec in CSV", sa.Name))
53+
}
54+
}
55+
}
56+
57+
return errs
58+
}
59+
3260
func validateOwnedCRDs(bundle *manifests.Bundle, csv *operatorsv1alpha1.ClusterServiceVersion) (result errors.ManifestResult) {
3361
ownedKeys := getOwnedCustomResourceDefintionKeys(csv)
3462

staging/api/pkg/validation/internal/bundle_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ func TestValidateBundle(t *testing.T) {
4343
hasError: true,
4444
errString: `duplicate CRD "test.example.com/v1alpha1, Kind=Test" in bundle "test-operator.v0.0.1"`,
4545
},
46+
{
47+
description: "invalid bundle service account can't match sa in csv",
48+
directory: "./testdata/invalid_bundle_sa",
49+
hasError: true,
50+
errString: `Error: Value etcd-operator: invalid service account found in bundle. sa name cannot match service account defined for deployment spec in CSV`,
51+
},
4652
}
4753

4854
for _, tt := range table {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
apiVersion: apiextensions.k8s.io/v1beta1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
name: etcdbackups.etcd.database.coreos.com
5+
spec:
6+
group: etcd.database.coreos.com
7+
names:
8+
kind: EtcdBackup
9+
listKind: EtcdBackupList
10+
plural: etcdbackups
11+
singular: etcdbackup
12+
scope: Namespaced
13+
version: v1beta2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
apiVersion: apiextensions.k8s.io/v1beta1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
name: etcdclusters.etcd.database.coreos.com
5+
spec:
6+
group: etcd.database.coreos.com
7+
names:
8+
kind: EtcdCluster
9+
listKind: EtcdClusterList
10+
plural: etcdclusters
11+
shortNames:
12+
- etcdclus
13+
- etcd
14+
singular: etcdcluster
15+
scope: Namespaced
16+
version: v1beta2

staging/api/pkg/validation/internal/testdata/invalid_bundle_sa/etcdoperator.v0.9.4.clusterserviceversion.yaml

Lines changed: 309 additions & 0 deletions
Large diffs are not rendered by default.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
apiVersion: apiextensions.k8s.io/v1beta1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
name: etcdrestores.etcd.database.coreos.com
5+
spec:
6+
group: etcd.database.coreos.com
7+
names:
8+
kind: EtcdRestore
9+
listKind: EtcdRestoreList
10+
plural: etcdrestores
11+
singular: etcdrestore
12+
scope: Namespaced
13+
version: v1beta2
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
kind: ServiceAccount
2+
apiVersion: v1
3+
metadata:
4+
name: etcd-operator
5+
namespace: etcd
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
kind: ServiceAccount
2+
apiVersion: v1
3+
metadata:
4+
name: etcd-operator2
5+
namespace: etcd

vendor/github.com/operator-framework/api/pkg/validation/internal/bundle.go

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

vendor/github.com/operator-framework/operator-registry/pkg/lib/indexer/index.Dockerfile022377864

Whitespace-only changes.

0 commit comments

Comments
 (0)