Skip to content

Commit 6567320

Browse files
dinhxuanvutimflannagan
authored andcommitted
Add validation to CSV name length (openshift#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]> Upstream-repository: api Upstream-commit: db8dead7a7c5091f2ce48fef369af973ee2a4f13
1 parent 4d90610 commit 6567320

File tree

3 files changed

+110
-5
lines changed

3 files changed

+110
-5
lines changed

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ func validateCSVs(objs ...interface{}) (results []errors.ManifestResult) {
3232
func validateCSV(csv *v1alpha1.ClusterServiceVersion) errors.ManifestResult {
3333
result := errors.ManifestResult{Name: csv.GetName()}
3434
// Ensure CSV names are of the correct format.
35-
if err := parseCSVNameFormat(csv.GetName()); err != (errors.Error{}) {
35+
if err := parseCSVNameFormat(csv.GetName()); err != nil {
3636
result.Add(errors.ErrInvalidCSV(fmt.Sprintf("metadata.name %s", err), csv.GetName()))
3737
}
3838
if replaces := csv.Spec.Replaces; replaces != "" {
39-
if err := parseCSVNameFormat(replaces); err != (errors.Error{}) {
39+
if err := parseCSVNameFormat(replaces); err != nil {
4040
result.Add(errors.ErrInvalidCSV(fmt.Sprintf("spec.replaces %s", err), csv.GetName()))
4141
}
4242
}
@@ -52,10 +52,15 @@ func validateCSV(csv *v1alpha1.ClusterServiceVersion) errors.ManifestResult {
5252
}
5353

5454
func parseCSVNameFormat(name string) error {
55-
if violations := k8svalidation.IsDNS1123Subdomain(name); len(violations) != 0 {
56-
return fmt.Errorf("%q is invalid:\n%s", name, violations)
55+
var errStrs []string
56+
errStrs = append(errStrs, k8svalidation.IsDNS1123Subdomain(name)...)
57+
// Give CSV name is used as label value, it should be validated
58+
errStrs = append(errStrs, k8svalidation.IsValidLabelValue(name)...)
59+
60+
if len(errStrs) > 0 {
61+
return fmt.Errorf("%q is invalid: %s", name, strings.Join(errStrs, ","))
5762
}
58-
return errors.Error{}
63+
return nil
5964
}
6065

6166
// checkFields runs checkEmptyFields and returns its errors.

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,16 @@ func TestValidateCSV(t *testing.T) {
6161
},
6262
filepath.Join("testdata", "badAnnotationNames.csv.yaml"),
6363
},
64+
{
65+
validatorFuncTest{
66+
description: "csv with name over 63 characters limit",
67+
wantErr: true,
68+
errors: []errors.Error{
69+
errors.ErrInvalidCSV(`metadata.name "someoperatorwithanextremelylongnamethatmakenosensewhatsoever.v999.999.999" is invalid: must be no more than 63 characters`, "someoperatorwithanextremelylongnamethatmakenosensewhatsoever.v999.999.999"),
70+
},
71+
},
72+
filepath.Join("testdata", "badName.csv.yaml"),
73+
},
6474
}
6575
for _, c := range cases {
6676
b, err := ioutil.ReadFile(c.csvPath)
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
#! validate-crd: deploy/chart/templates/0000_30_02-clusterserviceversion.crd.yaml
2+
#! parse-kind: ClusterServiceVersion
3+
apiVersion: operators.coreos.com/v1alpha1
4+
kind: ClusterServiceVersion
5+
metadata:
6+
name: someoperatorwithanextremelylongnamethatmakenosensewhatsoever.v999.999.999
7+
namespace: placeholder
8+
annotations:
9+
capabilities: Full Lifecycle
10+
tectonic-visibility: ocs
11+
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>"}}}]'
12+
description: etcd is a distributed key value store providing a reliable way to store data across a cluster of machines.
13+
spec:
14+
displayName: etcd
15+
description: something
16+
keywords: ['etcd', 'key value', 'database', 'coreos', 'open source']
17+
version: 0.9.0
18+
icon:
19+
- base64data: N8AmEL9X4ABACNSKMHAgb34AAAAAElFTkSuQmCC
20+
mediatype: image/png
21+
installModes:
22+
- type: OwnNamespace
23+
supported: true
24+
- type: SingleNamespace
25+
supported: true
26+
- type: MultiNamespace
27+
supported: false
28+
- type: AllNamespaces
29+
supported: true
30+
install:
31+
strategy: deployment
32+
spec:
33+
permissions:
34+
- serviceAccountName: etcd-operator
35+
rules:
36+
- apiGroups:
37+
- etcd.database.coreos.com
38+
resources:
39+
- etcdclusters
40+
- etcdbackups
41+
- etcdrestores
42+
verbs:
43+
- "*"
44+
- apiGroups:
45+
- ""
46+
resources:
47+
- pods
48+
- services
49+
- endpoints
50+
- persistentvolumeclaims
51+
- events
52+
verbs:
53+
- "*"
54+
- apiGroups:
55+
- apps
56+
resources:
57+
- deployments
58+
verbs:
59+
- "*"
60+
- apiGroups:
61+
- ""
62+
resources:
63+
- secrets
64+
verbs:
65+
- get
66+
deployments:
67+
- name: etcd-operator
68+
spec:
69+
replicas: 1
70+
selector:
71+
matchLabels:
72+
name: etcd-operator-alm-owned
73+
template:
74+
metadata:
75+
name: etcd-operator-alm-owned
76+
labels:
77+
name: etcd-operator-alm-owned
78+
spec:
79+
serviceAccountName: etcd-operator
80+
customresourcedefinitions:
81+
owned:
82+
- name: etcdclusters.etcd.database.coreos.com
83+
version: v1beta2
84+
kind: EtcdCluster
85+
- name: etcdbackups.etcd.database.coreos.com
86+
version: v1beta2
87+
kind: EtcdBackup
88+
- name: etcdrestores.etcd.database.coreos.com
89+
version: v1beta2
90+
kind: EtcdRestore

0 commit comments

Comments
 (0)