Fix clusterresourcequota annotations validation

docs/generated/oc_by_example_content.adoc

@@ -1059,8 +1059,8 @@ Create cluster resource quota resource.
 
 [options="nowrap"]
 ----
-  # Create an cluster resource quota limited to 10 pods
-  oc create clusterresourcequota limit-bob --project-label-selector=openshift.io/requester=user-bob --hard=pods=10
+  # Create a cluster resource quota limited to 10 pods
+  oc create clusterresourcequota limit-bob --project-annotation-selector=openshift.io/requester=user-bob --hard=pods=10
 ----
 ====
 

docs/man/man1/oc-create-clusterresourcequota.1

@@ -108,8 +108,8 @@ Cluster resource quota objects defined quota restrictions that span multiple pro
 .RS
 
 .nf
-  # Create an cluster resource quota limited to 10 pods
-  oc create clusterresourcequota limit\-bob \-\-project\-label\-selector=openshift.io/requester=user\-bob \-\-hard=pods=10
+  # Create a cluster resource quota limited to 10 pods
+  oc create clusterresourcequota limit\-bob \-\-project\-annotation\-selector=openshift.io/requester=user\-bob \-\-hard=pods=10
 
 .fi
 .RE

docs/man/man1/openshift-cli-create-clusterresourcequota.1

@@ -108,8 +108,8 @@ Cluster resource quota objects defined quota restrictions that span multiple pro
 .RS
 
 .nf
-  # Create an cluster resource quota limited to 10 pods
-  openshift cli create clusterresourcequota limit\-bob \-\-project\-label\-selector=openshift.io/requester=user\-bob \-\-hard=pods=10
+  # Create a cluster resource quota limited to 10 pods
+  openshift cli create clusterresourcequota limit\-bob \-\-project\-annotation\-selector=openshift.io/requester=user\-bob \-\-hard=pods=10
 
 .fi
 .RE

pkg/cmd/cli/cmd/create/clusterquota.go

@@ -1,6 +1,7 @@
 package create
 
 import (
+	"encoding/csv"
 	"fmt"
 	"io"
 	"strings"
@@ -27,8 +28,8 @@ Create a cluster resource quota that controls certain resources.
 
 Cluster resource quota objects defined quota restrictions that span multiple projects based on label selectors.`
 
-	clusterQuotaExample = `  # Create an cluster resource quota limited to 10 pods
-  %[1]s limit-bob --project-label-selector=openshift.io/requester=user-bob --hard=pods=10`
+	clusterQuotaExample = `  # Create a cluster resource quota limited to 10 pods
+  %[1]s limit-bob --project-annotation-selector=openshift.io/requester=user-bob --hard=pods=10`
 )
 
 type CreateClusterQuotaOptions struct {
@@ -80,20 +81,17 @@ func (o *CreateClusterQuotaOptions) Complete(cmd *cobra.Command, f *clientcmd.Fa
 		}
 	}
 
-	annotationSelector, err := unversioned.ParseToLabelSelector(cmdutil.GetFlagString(cmd, "project-annotation-selector"))
+	annotationSelector, err := parseAnnotationSelector(cmdutil.GetFlagString(cmd, "project-annotation-selector"))
 	if err != nil {
 		return err
 	}
-	if len(annotationSelector.MatchExpressions) > 0 {
-		return fmt.Errorf("only simple equality selection predicates are allowed for annotation selectors")
-	}
 
 	o.ClusterQuota = &quotaapi.ClusterResourceQuota{
 		ObjectMeta: kapi.ObjectMeta{Name: args[0]},
 		Spec: quotaapi.ClusterResourceQuotaSpec{
 			Selector: quotaapi.ClusterResourceQuotaSelector{
 				LabelSelector:      labelSelector,
-				AnnotationSelector: annotationSelector.MatchLabels,
+				AnnotationSelector: annotationSelector,
 			},
 			Quota: kapi.ResourceQuotaSpec{
 				Hard: kapi.ResourceList{},
@@ -161,3 +159,26 @@ func (o *CreateClusterQuotaOptions) Run() error {
 
 	return o.Printer(actualObj, o.Out)
 }
+
+// parseAnnotationSelector just parses key=value,key=value=...,
+// further validation is left to be done server-side.
+func parseAnnotationSelector(s string) (map[string]string, error) {
+	if len(s) == 0 {
+		return nil, nil
+	}
+	stringReader := strings.NewReader(s)
+	csvReader := csv.NewReader(stringReader)
+	annotations, err := csvReader.Read()
+	if err != nil {
+		return nil, err
+	}
+	parsed := map[string]string{}
+	for _, annotation := range annotations {
+		parts := strings.SplitN(annotation, "=", 2)
+		if len(parts) != 2 {
+			return nil, fmt.Errorf("Malformed annotation selector, expected %q: %s", "key=value", annotation)
+		}
+		parsed[parts[0]] = parts[1]
+	}
+	return parsed, nil
+}

pkg/cmd/cli/cmd/create/clusterquota_test.go

@@ -0,0 +1,64 @@
+package create
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestParseAnnotationSelector(t *testing.T) {
+	tests := []struct {
+		input         string
+		parsed        map[string]string
+		errorExpected bool
+	}{
+		{
+			input:  "",
+			parsed: nil,
+		},
+		{
+			input: "foo=bar",
+			parsed: map[string]string{
+				"foo": "bar",
+			},
+		},
+		{
+			input: "deads=deads,liggitt=liggitt",
+			parsed: map[string]string{
+				"deads":   "deads",
+				"liggitt": "liggitt",
+			},
+		},
+		{
+			input:         "liggitt=liggitt,deadliggitt",
+			errorExpected: true,
+		},
+		{
+			input: `"us=deads,liggitt,ffranz"`,
+			parsed: map[string]string{
+				"us": "deads,liggitt,ffranz",
+			},
+		},
+		{
+			input: `"us=deads,liggitt,ffranz",deadliggitt=deadliggitt`,
+			parsed: map[string]string{
+				"us":          "deads,liggitt,ffranz",
+				"deadliggitt": "deadliggitt",
+			},
+		},
+	}
+	for _, test := range tests {
+		parsed, err := parseAnnotationSelector(test.input)
+		if err != nil {
+			if !test.errorExpected {
+				t.Fatalf("unexpected error: %s", err)
+			}
+			continue
+		}
+		if test.errorExpected {
+			t.Fatalf("expected error, got a parsed output: %q", parsed)
+		}
+		if !reflect.DeepEqual(parsed, test.parsed) {
+			t.Error("expected parsed annotation selector ", test.parsed, ", got ", parsed)
+		}
+	}
+}

pkg/quota/api/validation/validation.go

@@ -23,6 +23,9 @@ func ValidateClusterResourceQuota(clusterquota *quotaapi.ClusterResourceQuota) f
 			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "selector", "labels"), clusterquota.Spec.Selector.LabelSelector, "must restrict the selected projects"))
 		}
 	}
+	if clusterquota.Spec.Selector.AnnotationSelector != nil {
+		allErrs = append(allErrs, validation.ValidateAnnotations(clusterquota.Spec.Selector.AnnotationSelector, field.NewPath("spec", "selector", "annotations"))...)
+	}
 
 	allErrs = append(allErrs, validation.ValidateResourceQuotaSpec(&clusterquota.Spec.Quota, field.NewPath("spec", "quota"))...)
 	allErrs = append(allErrs, validation.ValidateResourceQuotaStatus(&clusterquota.Status.Total, field.NewPath("status", "overall"))...)

test/cmd/quota.sh

@@ -4,6 +4,13 @@ trap os::test::junit::reconcile_output EXIT
 
 os::test::junit::declare_suite_start "cmd/quota"
 
+# Cleanup cluster resources created by this test suite
+(
+  set +e
+  oc delete project foo bar asmail
+  exit 0
+) &>/dev/null
+
 os::test::junit::declare_suite_start "cmd/quota/clusterquota"
 
 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
 os::cmd::try_until_text 'oc get appliedclusterresourcequota -n foo --as deads -o name' "for-deads"
 os::cmd::try_until_text 'oc describe appliedclusterresourcequota/for-deads -n foo --as deads' "secrets.*9"
 
-
+os::cmd::expect_failure_and_text 'oc create clusterquota for-deads-malformed --project-annotation-selector="openshift.#$%/requester=deads"' "prefix part must match the regex"
+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"
 os::cmd::expect_success 'oc create clusterquota for-deads-by-annotation --project-annotation-selector=openshift.io/requester=deads --hard=secrets=50'
+os::cmd::expect_success 'oc create clusterquota for-deads-email-by-annotation --project-annotation-selector=openshift.io/requester=deads@deads.io --hard=secrets=50'
+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'
 os::cmd::expect_success 'oc new-project bar --as=deads'
+os::cmd::expect_success 'oc new-project asmail --as=deads@deads.io'
 os::cmd::try_until_text 'oc get appliedclusterresourcequota -n bar --as deads -o name' "for-deads-by-annotation"
 os::cmd::try_until_text 'oc get appliedclusterresourcequota -n foo --as deads -o name' "for-deads-by-annotation"
+os::cmd::try_until_text 'oc get appliedclusterresourcequota -n asmail --as deads@deads.io -o name' "for-deads-email-by-annotation"
 os::cmd::try_until_text 'oc describe appliedclusterresourcequota/for-deads-by-annotation -n bar --as deads' "secrets.*1[0-9]"
 
 os::cmd::expect_success 'oc delete project foo'
 os::cmd::expect_success 'oc delete project bar'
+os::cmd::expect_success 'oc delete project asmail'
 
 echo "clusterquota: ok"
 os::test::junit::declare_suite_end