Add validation to CSV name length (#134)

* Add validation to CSV name length

Given CSV's name is used as label for OLM, its length should be
validated to ensure it's not over the label character limit
which is currently set at 63 character.

This commit will enable CSV validation returning an error if CSV's
name is more than 63 characters.

Signed-off-by: Vu Dinh <[email protected]>

* Add unit test for CSV with bad name

The unit test is targeted with CSV with name that is over 63
characters limit.

Signed-off-by: Vu Dinh <[email protected]>

Author: dinhxuanvu

pkg/validation/internal/csv.go

@@ -32,11 +32,11 @@ func validateCSVs(objs ...interface{}) (results []errors.ManifestResult) {
 func validateCSV(csv *v1alpha1.ClusterServiceVersion) errors.ManifestResult {
 	result := errors.ManifestResult{Name: csv.GetName()}
 	// Ensure CSV names are of the correct format.
-	if err := parseCSVNameFormat(csv.GetName()); err != (errors.Error{}) {
+	if err := parseCSVNameFormat(csv.GetName()); err != nil {
 		result.Add(errors.ErrInvalidCSV(fmt.Sprintf("metadata.name %s", err), csv.GetName()))
 	}
 	if replaces := csv.Spec.Replaces; replaces != "" {
-		if err := parseCSVNameFormat(replaces); err != (errors.Error{}) {
+		if err := parseCSVNameFormat(replaces); err != nil {
 			result.Add(errors.ErrInvalidCSV(fmt.Sprintf("spec.replaces %s", err), csv.GetName()))
 		}
 	}
@@ -52,10 +52,15 @@ func validateCSV(csv *v1alpha1.ClusterServiceVersion) errors.ManifestResult {
 }
 
 func parseCSVNameFormat(name string) error {
-	if violations := k8svalidation.IsDNS1123Subdomain(name); len(violations) != 0 {
-		return fmt.Errorf("%q is invalid:\n%s", name, violations)
+	var errStrs []string
+	errStrs = append(errStrs, k8svalidation.IsDNS1123Subdomain(name)...)
+	// Give CSV name is used as label value, it should be validated
+	errStrs = append(errStrs, k8svalidation.IsValidLabelValue(name)...)
+
+	if len(errStrs) > 0 {
+		return fmt.Errorf("%q is invalid: %s", name, strings.Join(errStrs, ","))
 	}
-	return errors.Error{}
+	return nil
 }
 
 // checkFields runs checkEmptyFields and returns its errors.

pkg/validation/internal/csv_test.go

@@ -61,6 +61,16 @@ func TestValidateCSV(t *testing.T) {
 			},
 			filepath.Join("testdata", "badAnnotationNames.csv.yaml"),
 		},
+		{
+			validatorFuncTest{
+				description: "csv with name over 63 characters limit",
+				wantErr:     true,
+				errors: []errors.Error{
+					errors.ErrInvalidCSV(`metadata.name "someoperatorwithanextremelylongnamethatmakenosensewhatsoever.v999.999.999" is invalid: must be no more than 63 characters`, "someoperatorwithanextremelylongnamethatmakenosensewhatsoever.v999.999.999"),
+				},
+			},
+			filepath.Join("testdata", "badName.csv.yaml"),
+		},
 	}
 	for _, c := range cases {
 		b, err := ioutil.ReadFile(c.csvPath)

pkg/validation/internal/testdata/badName.csv.yaml

@@ -0,0 +1,90 @@
+#! validate-crd: deploy/chart/templates/0000_30_02-clusterserviceversion.crd.yaml
+#! parse-kind: ClusterServiceVersion
+apiVersion: operators.coreos.com/v1alpha1
+kind: ClusterServiceVersion
+metadata:
+  name: someoperatorwithanextremelylongnamethatmakenosensewhatsoever.v999.999.999
+  namespace: placeholder
+  annotations:
+    capabilities: Full Lifecycle
+    tectonic-visibility: ocs
+    alm-examples: '[{"apiVersion":"etcd.database.coreos.com/v1beta2","kind":"EtcdCluster","metadata":{"name":"example","namespace":"default"},"spec":{"size":3,"version":"3.2.13"}},{"apiVersion":"etcd.database.coreos.com/v1beta2","kind":"EtcdRestore","metadata":{"name":"example-etcd-cluster"},"spec":{"etcdCluster":{"name":"example-etcd-cluster"},"backupStorageType":"S3","s3":{"path":"<full-s3-path>","awsSecret":"<aws-secret>"}}},{"apiVersion":"etcd.database.coreos.com/v1beta2","kind":"EtcdBackup","metadata":{"name":"example-etcd-cluster-backup"},"spec":{"etcdEndpoints":["<etcd-cluster-endpoints>"],"storageType":"S3","s3":{"path":"<full-s3-path>","awsSecret":"<aws-secret>"}}}]'
+    description: etcd is a distributed key value store providing a reliable way to store data across a cluster of machines.
+spec:
+  displayName: etcd
+  description: something
+  keywords: ['etcd', 'key value', 'database', 'coreos', 'open source']
+  version: 0.9.0
+  icon:
+  - base64data: N8AmEL9X4ABACNSKMHAgb34AAAAAElFTkSuQmCC
+    mediatype: image/png
+  installModes:
+  - type: OwnNamespace
+    supported: true
+  - type: SingleNamespace
+    supported: true
+  - type: MultiNamespace
+    supported: false
+  - type: AllNamespaces
+    supported: true
+  install:
+    strategy: deployment
+    spec:
+      permissions:
+      - serviceAccountName: etcd-operator
+        rules:
+        - apiGroups:
+          - etcd.database.coreos.com
+          resources:
+          - etcdclusters
+          - etcdbackups
+          - etcdrestores
+          verbs:
+          - "*"
+        - apiGroups:
+          - ""
+          resources:
+          - pods
+          - services
+          - endpoints
+          - persistentvolumeclaims
+          - events
+          verbs:
+          - "*"
+        - apiGroups:
+          - apps
+          resources:
+          - deployments
+          verbs:
+          - "*"
+        - apiGroups:
+          - ""
+          resources:
+          - secrets
+          verbs:
+          - get
+      deployments:
+      - name: etcd-operator
+        spec:
+          replicas: 1
+          selector:
+            matchLabels:
+              name: etcd-operator-alm-owned
+          template:
+            metadata:
+              name: etcd-operator-alm-owned
+              labels:
+                name: etcd-operator-alm-owned
+            spec:
+              serviceAccountName: etcd-operator
+  customresourcedefinitions:
+    owned:
+    - name: etcdclusters.etcd.database.coreos.com
+      version: v1beta2
+      kind: EtcdCluster
+    - name: etcdbackups.etcd.database.coreos.com
+      version: v1beta2
+      kind: EtcdBackup
+    - name: etcdrestores.etcd.database.coreos.com
+      version: v1beta2
+      kind: EtcdRestore