Browse code

Fix clusterresourcequota annotations validation

Fabiano Franz authored on 2016/09/15 02:04:29
Showing 7 changed files
... ...
@@ -1059,8 +1059,8 @@ Create cluster resource quota resource.
1059 1059
 
1060 1060
 [options="nowrap"]
1061 1061
 ----
1062
-  # Create an cluster resource quota limited to 10 pods
1063
-  oc create clusterresourcequota limit-bob --project-label-selector=openshift.io/requester=user-bob --hard=pods=10
1062
+  # Create a cluster resource quota limited to 10 pods
1063
+  oc create clusterresourcequota limit-bob --project-annotation-selector=openshift.io/requester=user-bob --hard=pods=10
1064 1064
 ----
1065 1065
 ====
1066 1066
 
... ...
@@ -108,8 +108,8 @@ Cluster resource quota objects defined quota restrictions that span multiple pro
108 108
 .RS
109 109
 
110 110
 .nf
111
-  # Create an cluster resource quota limited to 10 pods
112
-  oc create clusterresourcequota limit\-bob \-\-project\-label\-selector=openshift.io/requester=user\-bob \-\-hard=pods=10
111
+  # Create a cluster resource quota limited to 10 pods
112
+  oc create clusterresourcequota limit\-bob \-\-project\-annotation\-selector=openshift.io/requester=user\-bob \-\-hard=pods=10
113 113
 
114 114
 .fi
115 115
 .RE
... ...
@@ -108,8 +108,8 @@ Cluster resource quota objects defined quota restrictions that span multiple pro
108 108
 .RS
109 109
 
110 110
 .nf
111
-  # Create an cluster resource quota limited to 10 pods
112
-  openshift cli create clusterresourcequota limit\-bob \-\-project\-label\-selector=openshift.io/requester=user\-bob \-\-hard=pods=10
111
+  # Create a cluster resource quota limited to 10 pods
112
+  openshift cli create clusterresourcequota limit\-bob \-\-project\-annotation\-selector=openshift.io/requester=user\-bob \-\-hard=pods=10
113 113
 
114 114
 .fi
115 115
 .RE
... ...
@@ -1,6 +1,7 @@
1 1
 package create
2 2
 
3 3
 import (
4
+	"encoding/csv"
4 5
 	"fmt"
5 6
 	"io"
6 7
 	"strings"
... ...
@@ -27,8 +28,8 @@ Create a cluster resource quota that controls certain resources.
27 27
 
28 28
 Cluster resource quota objects defined quota restrictions that span multiple projects based on label selectors.`
29 29
 
30
-	clusterQuotaExample = `  # Create an cluster resource quota limited to 10 pods
31
-  %[1]s limit-bob --project-label-selector=openshift.io/requester=user-bob --hard=pods=10`
30
+	clusterQuotaExample = `  # Create a cluster resource quota limited to 10 pods
31
+  %[1]s limit-bob --project-annotation-selector=openshift.io/requester=user-bob --hard=pods=10`
32 32
 )
33 33
 
34 34
 type CreateClusterQuotaOptions struct {
... ...
@@ -80,20 +81,17 @@ func (o *CreateClusterQuotaOptions) Complete(cmd *cobra.Command, f *clientcmd.Fa
80 80
 		}
81 81
 	}
82 82
 
83
-	annotationSelector, err := unversioned.ParseToLabelSelector(cmdutil.GetFlagString(cmd, "project-annotation-selector"))
83
+	annotationSelector, err := parseAnnotationSelector(cmdutil.GetFlagString(cmd, "project-annotation-selector"))
84 84
 	if err != nil {
85 85
 		return err
86 86
 	}
87
-	if len(annotationSelector.MatchExpressions) > 0 {
88
-		return fmt.Errorf("only simple equality selection predicates are allowed for annotation selectors")
89
-	}
90 87
 
91 88
 	o.ClusterQuota = &quotaapi.ClusterResourceQuota{
92 89
 		ObjectMeta: kapi.ObjectMeta{Name: args[0]},
93 90
 		Spec: quotaapi.ClusterResourceQuotaSpec{
94 91
 			Selector: quotaapi.ClusterResourceQuotaSelector{
95 92
 				LabelSelector:      labelSelector,
96
-				AnnotationSelector: annotationSelector.MatchLabels,
93
+				AnnotationSelector: annotationSelector,
97 94
 			},
98 95
 			Quota: kapi.ResourceQuotaSpec{
99 96
 				Hard: kapi.ResourceList{},
... ...
@@ -161,3 +159,26 @@ func (o *CreateClusterQuotaOptions) Run() error {
161 161
 
162 162
 	return o.Printer(actualObj, o.Out)
163 163
 }
164
+
165
+// parseAnnotationSelector just parses key=value,key=value=...,
166
+// further validation is left to be done server-side.
167
+func parseAnnotationSelector(s string) (map[string]string, error) {
168
+	if len(s) == 0 {
169
+		return nil, nil
170
+	}
171
+	stringReader := strings.NewReader(s)
172
+	csvReader := csv.NewReader(stringReader)
173
+	annotations, err := csvReader.Read()
174
+	if err != nil {
175
+		return nil, err
176
+	}
177
+	parsed := map[string]string{}
178
+	for _, annotation := range annotations {
179
+		parts := strings.SplitN(annotation, "=", 2)
180
+		if len(parts) != 2 {
181
+			return nil, fmt.Errorf("Malformed annotation selector, expected %q: %s", "key=value", annotation)
182
+		}
183
+		parsed[parts[0]] = parts[1]
184
+	}
185
+	return parsed, nil
186
+}
164 187
new file mode 100644
... ...
@@ -0,0 +1,64 @@
0
+package create
1
+
2
+import (
3
+	"reflect"
4
+	"testing"
5
+)
6
+
7
+func TestParseAnnotationSelector(t *testing.T) {
8
+	tests := []struct {
9
+		input         string
10
+		parsed        map[string]string
11
+		errorExpected bool
12
+	}{
13
+		{
14
+			input:  "",
15
+			parsed: nil,
16
+		},
17
+		{
18
+			input: "foo=bar",
19
+			parsed: map[string]string{
20
+				"foo": "bar",
21
+			},
22
+		},
23
+		{
24
+			input: "deads=deads,liggitt=liggitt",
25
+			parsed: map[string]string{
26
+				"deads":   "deads",
27
+				"liggitt": "liggitt",
28
+			},
29
+		},
30
+		{
31
+			input:         "liggitt=liggitt,deadliggitt",
32
+			errorExpected: true,
33
+		},
34
+		{
35
+			input: `"us=deads,liggitt,ffranz"`,
36
+			parsed: map[string]string{
37
+				"us": "deads,liggitt,ffranz",
38
+			},
39
+		},
40
+		{
41
+			input: `"us=deads,liggitt,ffranz",deadliggitt=deadliggitt`,
42
+			parsed: map[string]string{
43
+				"us":          "deads,liggitt,ffranz",
44
+				"deadliggitt": "deadliggitt",
45
+			},
46
+		},
47
+	}
48
+	for _, test := range tests {
49
+		parsed, err := parseAnnotationSelector(test.input)
50
+		if err != nil {
51
+			if !test.errorExpected {
52
+				t.Fatalf("unexpected error: %s", err)
53
+			}
54
+			continue
55
+		}
56
+		if test.errorExpected {
57
+			t.Fatalf("expected error, got a parsed output: %q", parsed)
58
+		}
59
+		if !reflect.DeepEqual(parsed, test.parsed) {
60
+			t.Error("expected parsed annotation selector ", test.parsed, ", got ", parsed)
61
+		}
62
+	}
63
+}
... ...
@@ -23,6 +23,9 @@ func ValidateClusterResourceQuota(clusterquota *quotaapi.ClusterResourceQuota) f
23 23
 			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "selector", "labels"), clusterquota.Spec.Selector.LabelSelector, "must restrict the selected projects"))
24 24
 		}
25 25
 	}
26
+	if clusterquota.Spec.Selector.AnnotationSelector != nil {
27
+		allErrs = append(allErrs, validation.ValidateAnnotations(clusterquota.Spec.Selector.AnnotationSelector, field.NewPath("spec", "selector", "annotations"))...)
28
+	}
26 29
 
27 30
 	allErrs = append(allErrs, validation.ValidateResourceQuotaSpec(&clusterquota.Spec.Quota, field.NewPath("spec", "quota"))...)
28 31
 	allErrs = append(allErrs, validation.ValidateResourceQuotaStatus(&clusterquota.Status.Total, field.NewPath("status", "overall"))...)
... ...
@@ -4,6 +4,13 @@ trap os::test::junit::reconcile_output EXIT
4 4
 
5 5
 os::test::junit::declare_suite_start "cmd/quota"
6 6
 
7
+# Cleanup cluster resources created by this test suite
8
+(
9
+  set +e
10
+  oc delete project foo bar asmail
11
+  exit 0
12
+) &>/dev/null
13
+
7 14
 os::test::junit::declare_suite_start "cmd/quota/clusterquota"
8 15
 
9 16
 os::cmd::expect_success 'oc new-project foo --as=deads'
... ...
@@ -12,15 +19,21 @@ os::cmd::expect_success 'oc create clusterquota for-deads --project-label-select
12 12
 os::cmd::try_until_text 'oc get appliedclusterresourcequota -n foo --as deads -o name' "for-deads"
13 13
 os::cmd::try_until_text 'oc describe appliedclusterresourcequota/for-deads -n foo --as deads' "secrets.*9"
14 14
 
15
-
15
+os::cmd::expect_failure_and_text 'oc create clusterquota for-deads-malformed --project-annotation-selector="openshift.#$%/requester=deads"' "prefix part must match the regex"
16
+os::cmd::expect_failure_and_text 'oc create clusterquota for-deads-malformed --project-annotation-selector=openshift.io/requester=deads,openshift.io/novalue' "Malformed annotation selector"
16 17
 os::cmd::expect_success 'oc create clusterquota for-deads-by-annotation --project-annotation-selector=openshift.io/requester=deads --hard=secrets=50'
18
+os::cmd::expect_success 'oc create clusterquota for-deads-email-by-annotation --project-annotation-selector=openshift.io/requester=deads@deads.io --hard=secrets=50'
19
+os::cmd::expect_success 'oc create clusterresourcequota annotation-value-with-commas --project-annotation-selector="openshift.io/requester=deads,\"openshift.io/withcomma=yes,true,1\"" --hard=pods=10'
17 20
 os::cmd::expect_success 'oc new-project bar --as=deads'
21
+os::cmd::expect_success 'oc new-project asmail --as=deads@deads.io'
18 22
 os::cmd::try_until_text 'oc get appliedclusterresourcequota -n bar --as deads -o name' "for-deads-by-annotation"
19 23
 os::cmd::try_until_text 'oc get appliedclusterresourcequota -n foo --as deads -o name' "for-deads-by-annotation"
24
+os::cmd::try_until_text 'oc get appliedclusterresourcequota -n asmail --as deads@deads.io -o name' "for-deads-email-by-annotation"
20 25
 os::cmd::try_until_text 'oc describe appliedclusterresourcequota/for-deads-by-annotation -n bar --as deads' "secrets.*1[0-9]"
21 26
 
22 27
 os::cmd::expect_success 'oc delete project foo'
23 28
 os::cmd::expect_success 'oc delete project bar'
29
+os::cmd::expect_success 'oc delete project asmail'
24 30
 
25 31
 echo "clusterquota: ok"
26 32
 os::test::junit::declare_suite_end