From 2d4e88fba6a7c9b4fa2c063cc8f47aa9dac87718 Mon Sep 17 00:00:00 2001 From: Fabiano Franz Date: Wed, 14 Sep 2016 14:04:29 -0300 Subject: [PATCH] Fix clusterresourcequota annotations validation --- docs/generated/oc_by_example_content.adoc | 4 +- .../man/man1/oc-create-clusterresourcequota.1 | 4 +- ...penshift-cli-create-clusterresourcequota.1 | 4 +- pkg/cmd/cli/cmd/create/clusterquota.go | 35 ++++++++-- pkg/cmd/cli/cmd/create/clusterquota_test.go | 64 +++++++++++++++++++ pkg/quota/api/validation/validation.go | 3 + test/cmd/quota.sh | 15 ++++- 7 files changed, 115 insertions(+), 14 deletions(-) create mode 100644 pkg/cmd/cli/cmd/create/clusterquota_test.go diff --git a/docs/generated/oc_by_example_content.adoc b/docs/generated/oc_by_example_content.adoc index 391014950fa5..5466c3d4cabf 100644 --- a/docs/generated/oc_by_example_content.adoc +++ b/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 ---- ==== diff --git a/docs/man/man1/oc-create-clusterresourcequota.1 b/docs/man/man1/oc-create-clusterresourcequota.1 index 2dd3d2e27668..0fc61234df69 100644 --- a/docs/man/man1/oc-create-clusterresourcequota.1 +++ b/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 diff --git a/docs/man/man1/openshift-cli-create-clusterresourcequota.1 b/docs/man/man1/openshift-cli-create-clusterresourcequota.1 index 53cfc0899c93..7678fd806fd4 100644 --- a/docs/man/man1/openshift-cli-create-clusterresourcequota.1 +++ b/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 diff --git a/pkg/cmd/cli/cmd/create/clusterquota.go b/pkg/cmd/cli/cmd/create/clusterquota.go index 3aceb2561885..809a1c6e01f4 100644 --- a/pkg/cmd/cli/cmd/create/clusterquota.go +++ b/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 = "aapi.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 +} diff --git a/pkg/cmd/cli/cmd/create/clusterquota_test.go b/pkg/cmd/cli/cmd/create/clusterquota_test.go new file mode 100644 index 000000000000..a0f33eaae7ba --- /dev/null +++ b/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) + } + } +} diff --git a/pkg/quota/api/validation/validation.go b/pkg/quota/api/validation/validation.go index f1269db461ac..d92d5cc571df 100644 --- a/pkg/quota/api/validation/validation.go +++ b/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"))...) diff --git a/test/cmd/quota.sh b/test/cmd/quota.sh index 755a3c1b8e86..78fa986a5b99 100755 --- a/test/cmd/quota.sh +++ b/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