Skip to content

Commit

Permalink
Validate SA names do not match csv deployment spec (#144)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kevinrizza authored Aug 4, 2021
1 parent 9106367 commit 0fe04f8
Show file tree
Hide file tree
Showing 8 changed files with 395 additions and 0 deletions.
28 changes: 28 additions & 0 deletions pkg/validation/internal/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/operator-framework/api/pkg/validation/errors"
interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
)

Expand All @@ -26,9 +28,35 @@ func validateBundles(objs ...interface{}) (results []errors.ManifestResult) {
func validateBundle(bundle *manifests.Bundle) (result errors.ManifestResult) {
result = validateOwnedCRDs(bundle, bundle.CSV)
result.Name = bundle.CSV.Spec.Version.String()
saErrors := validateServiceAccounts(bundle)
if saErrors != nil {
result.Add(saErrors...)
}
return result
}

func validateServiceAccounts(bundle *manifests.Bundle) []errors.Error {
// get service account names defined in the csv
saNamesFromCSV := make(map[string]struct{}, 0)
for _, deployment := range bundle.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
saName := deployment.Spec.Template.Spec.ServiceAccountName
saNamesFromCSV[saName] = struct{}{}
}

// find any hardcoded service account objects are in the bundle, then check if they match any sa definition in the csv
var errs []errors.Error
for _, obj := range bundle.Objects {
sa := v1.ServiceAccount{}
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &sa); err == nil {
if _, ok := saNamesFromCSV[sa.Name]; ok {
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))
}
}
}

return errs
}

func validateOwnedCRDs(bundle *manifests.Bundle, csv *operatorsv1alpha1.ClusterServiceVersion) (result errors.ManifestResult) {
ownedKeys := getOwnedCustomResourceDefintionKeys(csv)

Expand Down
6 changes: 6 additions & 0 deletions pkg/validation/internal/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ func TestValidateBundle(t *testing.T) {
hasError: true,
errString: `duplicate CRD "test.example.com/v1alpha1, Kind=Test" in bundle "test-operator.v0.0.1"`,
},
{
description: "invalid bundle service account can't match sa in csv",
directory: "./testdata/invalid_bundle_sa",
hasError: true,
errString: `Error: Value etcd-operator: invalid service account found in bundle. sa name cannot match service account defined for deployment spec in CSV`,
},
}

for _, tt := range table {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: etcdbackups.etcd.database.coreos.com
spec:
group: etcd.database.coreos.com
names:
kind: EtcdBackup
listKind: EtcdBackupList
plural: etcdbackups
singular: etcdbackup
scope: Namespaced
version: v1beta2
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: etcdclusters.etcd.database.coreos.com
spec:
group: etcd.database.coreos.com
names:
kind: EtcdCluster
listKind: EtcdClusterList
plural: etcdclusters
shortNames:
- etcdclus
- etcd
singular: etcdcluster
scope: Namespaced
version: v1beta2

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: etcdrestores.etcd.database.coreos.com
spec:
group: etcd.database.coreos.com
names:
kind: EtcdRestore
listKind: EtcdRestoreList
plural: etcdrestores
singular: etcdrestore
scope: Namespaced
version: v1beta2
5 changes: 5 additions & 0 deletions pkg/validation/internal/testdata/invalid_bundle_sa/sa.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: ServiceAccount
apiVersion: v1
metadata:
name: etcd-operator
namespace: etcd
5 changes: 5 additions & 0 deletions pkg/validation/internal/testdata/invalid_bundle_sa/sa2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: ServiceAccount
apiVersion: v1
metadata:
name: etcd-operator2
namespace: etcd

0 comments on commit 0fe04f8

Please sign in to comment.