From 27d8f4330aab68a76e3e6f2d0b1a7a7a4956f1d9 Mon Sep 17 00:00:00 2001 From: Aaron Lerner Date: Tue, 13 Aug 2019 17:07:07 -0700 Subject: [PATCH] Service and Revision labels (#342) * Add support for service labels with --label or -l * Add tests * Fix typo * Move MapToArray helper method to separate file * Allow unsetting labels by passing empty value * Fix test * Add test case for label removal * Wrap error message with flag * Update docs to include --label * Update labels on both services and revisions * Add some test cases around weirder user input * Change unset behavior and allow setting empty env and label * Update docs for new unset behavior * Make single keys to map to empty values * Move helper to util * Use assert.DeepEqual * Use new mock test + check both service and revision * Use new method and correct year --- docs/cmd/kn_service_create.md | 3 +- docs/cmd/kn_service_update.md | 3 +- .../service/configuration_edit_flags.go | 55 +++++-- .../service/service_create_mock_test.go | 55 +++++++ .../commands/service/service_create_test.go | 8 +- .../commands/service/service_update_test.go | 60 ++++++- pkg/serving/config_changes.go | 37 ++++- pkg/serving/config_changes_test.go | 154 ++++++++++++++++-- pkg/util/parsing_helper.go | 48 ++++++ pkg/util/parsing_helper_test.go | 62 +++++++ 10 files changed, 448 insertions(+), 37 deletions(-) create mode 100644 pkg/util/parsing_helper.go create mode 100644 pkg/util/parsing_helper_test.go diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 44e71491ad..6aa44876eb 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -42,10 +42,11 @@ kn service create NAME --image IMAGE [flags] --async Create service and don't wait for it to become ready. --concurrency-limit int Hard Limit of concurrent requests to be processed by a single replica. --concurrency-target int Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given. - -e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. + -e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-). --force Create service forcefully, replaces existing service if any. -h, --help help for create --image string Image to run. + -l, --label stringArray Service label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). --limits-cpu string The limits on the requested CPU (e.g., 1000m). --limits-memory string The limits on the requested memory (e.g., 1024Mi). --max-scale int Maximal number of replicas. diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index 33c5c47c78..b908587a90 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -30,9 +30,10 @@ kn service update NAME [flags] --async Update service and don't wait for it to become ready. --concurrency-limit int Hard Limit of concurrent requests to be processed by a single replica. --concurrency-target int Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given. - -e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. + -e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-). -h, --help help for update --image string Image to run. + -l, --label stringArray Service label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). --limits-cpu string The limits on the requested CPU (e.g., 1000m). --limits-memory string The limits on the requested memory (e.g., 1024Mi). --max-scale int Maximal number of replicas. diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index 01b5a7a37e..aefab59823 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -15,11 +15,12 @@ package service import ( - "fmt" "strings" servinglib "github.com/knative/client/pkg/serving" + util "github.com/knative/client/pkg/util" servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" + errors "github.com/pkg/errors" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -35,6 +36,7 @@ type ConfigurationEditFlags struct { ConcurrencyTarget int ConcurrencyLimit int Port int32 + Labels []string } type ResourceFlags struct { @@ -46,7 +48,8 @@ func (p *ConfigurationEditFlags) AddUpdateFlags(command *cobra.Command) { command.Flags().StringVar(&p.Image, "image", "", "Image to run.") command.Flags().StringArrayVarP(&p.Env, "env", "e", []string{}, "Environment variable to set. NAME=value; you may provide this flag "+ - "any number of times to set multiple environment variables.") + "any number of times to set multiple environment variables. "+ + "To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).") command.Flags().StringVar(&p.RequestsFlags.CPU, "requests-cpu", "", "The requested CPU (e.g., 250m).") command.Flags().StringVar(&p.RequestsFlags.Memory, "requests-memory", "", "The requested memory (e.g., 64Mi).") command.Flags().StringVar(&p.LimitsFlags.CPU, "limits-cpu", "", "The limits on the requested CPU (e.g., 1000m).") @@ -56,6 +59,10 @@ func (p *ConfigurationEditFlags) AddUpdateFlags(command *cobra.Command) { command.Flags().IntVar(&p.ConcurrencyTarget, "concurrency-target", 0, "Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.") command.Flags().IntVar(&p.ConcurrencyLimit, "concurrency-limit", 0, "Hard Limit of concurrent requests to be processed by a single replica.") command.Flags().Int32VarP(&p.Port, "port", "p", 0, "The port where application listens on.") + command.Flags().StringArrayVarP(&p.Labels, "label", "l", []string{}, + "Service label to set. name=value; you may provide this flag "+ + "any number of times to set multiple labels. "+ + "To unset, specify the label name followed by a \"-\" (e.g., name-).") } func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) { @@ -71,18 +78,22 @@ func (p *ConfigurationEditFlags) Apply(service *servingv1alpha1.Service, cmd *co return err } - envMap := map[string]string{} - for _, pairStr := range p.Env { - pairSlice := strings.SplitN(pairStr, "=", 2) - if len(pairSlice) <= 1 { - return fmt.Errorf( - "--env argument requires a value that contains the '=' character; got %s", - pairStr) + if cmd.Flags().Changed("env") { + envMap, err := util.MapFromArrayAllowingSingles(p.Env, "=") + if err != nil { + return errors.Wrap(err, "Invalid --env") + } + envToRemove := []string{} + for name := range envMap { + if strings.HasSuffix(name, "-") { + envToRemove = append(envToRemove, name[:len(name)-1]) + delete(envMap, name) + } + } + err = servinglib.UpdateEnvVars(template, envMap, envToRemove) + if err != nil { + return err } - envMap[pairSlice[0]] = pairSlice[1] - } - if err := servinglib.UpdateEnvVars(template, envMap); err != nil { - return err } if cmd.Flags().Changed("image") { @@ -139,6 +150,24 @@ func (p *ConfigurationEditFlags) Apply(service *servingv1alpha1.Service, cmd *co } } + if cmd.Flags().Changed("label") { + labelsMap, err := util.MapFromArrayAllowingSingles(p.Labels, "=") + if err != nil { + return errors.Wrap(err, "Invalid --label") + } + labelsToRemove := []string{} + for key := range labelsMap { + if strings.HasSuffix(key, "-") { + labelsToRemove = append(labelsToRemove, key[:len(key)-1]) + delete(labelsMap, key) + } + } + err = servinglib.UpdateLabels(service, template, labelsMap, labelsToRemove) + if err != nil { + return err + } + } + return nil } diff --git a/pkg/kn/commands/service/service_create_mock_test.go b/pkg/kn/commands/service/service_create_mock_test.go index aa04714674..f453656781 100644 --- a/pkg/kn/commands/service/service_create_mock_test.go +++ b/pkg/kn/commands/service/service_create_mock_test.go @@ -19,10 +19,13 @@ import ( "github.com/knative/pkg/apis" "gotest.tools/assert" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/knative/serving/pkg/apis/serving/v1alpha1" + servinglib "github.com/knative/client/pkg/serving" knclient "github.com/knative/client/pkg/serving/v1alpha1" "github.com/knative/client/pkg/util" @@ -53,6 +56,58 @@ func TestServiceCreateImageMock(t *testing.T) { r.Validate() } +func TestServiceCreateLabel(t *testing.T) { + client := knclient.NewMockKnClient(t) + + r := client.Recorder() + r.GetService("foo", nil, errors.NewNotFound(v1alpha1.Resource("service"), "foo")) + + service := getService("foo") + expected := map[string]string{ + "a": "mouse", + "b": "cookie", + "empty": "", + } + service.ObjectMeta.Labels = expected + template, err := servinglib.RevisionTemplateOfService(service) + assert.NilError(t, err) + template.ObjectMeta.Labels = expected + template.Spec.DeprecatedContainer.Image = "gcr.io/foo/bar:baz" + r.CreateService(service, nil) + + output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "-l", "a=mouse", "--label", "b=cookie", "--label=empty", "--async") + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "created", "foo", "default")) + + r.Validate() +} + +func getService(name string) *v1alpha1.Service { + service := &v1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + Spec: v1alpha1.ServiceSpec{ + DeprecatedRunLatest: &v1alpha1.RunLatestType{ + Configuration: v1alpha1.ConfigurationSpec{ + DeprecatedRevisionTemplate: &v1alpha1.RevisionTemplateSpec{ + Spec: v1alpha1.RevisionSpec{ + DeprecatedContainer: &corev1.Container{ + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{}, + Requests: corev1.ResourceList{}, + }, + }, + }, + }, + }, + }, + }, + } + return service +} + func getServiceWithUrl(name string, urlName string) *v1alpha1.Service { service := v1alpha1.Service{} url, _ := apis.ParseURL(urlName) diff --git a/pkg/kn/commands/service/service_create_test.go b/pkg/kn/commands/service/service_create_test.go index 8bafc37906..efab0580a5 100644 --- a/pkg/kn/commands/service/service_create_test.go +++ b/pkg/kn/commands/service/service_create_test.go @@ -158,7 +158,7 @@ func TestServiceCreateImageSync(t *testing.T) { func TestServiceCreateEnv(t *testing.T) { action, created, _, err := fakeServiceCreate([]string{ - "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "-e", "A=DOGS", "--env", "B=WOLVES", "--async"}, false, false) + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "-e", "A=DOGS", "--env", "B=WOLVES", "--env=EMPTY", "--async"}, false, false) if err != nil { t.Fatal(err) @@ -167,8 +167,10 @@ func TestServiceCreateEnv(t *testing.T) { } expectedEnvVars := map[string]string{ - "A": "DOGS", - "B": "WOLVES"} + "A": "DOGS", + "B": "WOLVES", + "EMPTY": "", + } template, err := servinglib.RevisionTemplateOfService(created) actualEnvVars, err := servinglib.EnvToMap(template.Spec.DeprecatedContainer.Env) diff --git a/pkg/kn/commands/service/service_update_test.go b/pkg/kn/commands/service/service_update_test.go index 3a832447d6..aa3cb234e4 100644 --- a/pkg/kn/commands/service/service_update_test.go +++ b/pkg/kn/commands/service/service_update_test.go @@ -232,11 +232,15 @@ func TestServiceUpdateEnv(t *testing.T) { if err != nil { t.Fatal(err) } + template.Spec.DeprecatedContainer.Env = []corev1.EnvVar{ + {Name: "EXISTING", Value: "thing"}, + {Name: "OTHEREXISTING"}, + } servinglib.UpdateImage(template, "gcr.io/foo/bar:baz") action, updated, _, err := fakeServiceUpdate(orig, []string{ - "service", "update", "foo", "-e", "TARGET=Awesome", "--async"}, false) + "service", "update", "foo", "-e", "TARGET=Awesome", "--env", "EXISTING-", "--env=OTHEREXISTING-=whatever", "--async"}, false) if err != nil { t.Fatal(err) @@ -374,6 +378,60 @@ func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) { } } +func TestServiceUpdateLabelWhenEmpty(t *testing.T) { + original := newEmptyService() + + action, updated, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", "-l", "a=mouse", "--label", "b=cookie", "-l=single", "--async"}, false) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("update", "services") { + t.Fatalf("Bad action %v", action) + } + + expected := map[string]string{ + "a": "mouse", + "b": "cookie", + "single": "", + } + actual := updated.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) + + template, err := servinglib.RevisionTemplateOfService(updated) + assert.NilError(t, err) + actual = template.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) +} + +func TestServiceUpdateLabelExisting(t *testing.T) { + original := newEmptyService() + original.ObjectMeta.Labels = map[string]string{"already": "here", "tobe": "removed"} + originalTemplate, _ := servinglib.RevisionTemplateOfService(original) + originalTemplate.ObjectMeta.Labels = map[string]string{"already": "here", "tobe": "removed"} + + action, updated, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", "-l", "already=gone", "--label=tobe-", "--label", "b=", "--async"}, false) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("update", "services") { + t.Fatalf("Bad action %v", action) + } + + expected := map[string]string{ + "already": "gone", + "b": "", + } + actual := updated.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) + + template, err := servinglib.RevisionTemplateOfService(updated) + assert.NilError(t, err) + actual = template.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) +} + func newEmptyService() *v1alpha1.Service { return &v1alpha1.Service{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index 7cc8bfbf1c..6bfdd66475 100644 --- a/pkg/serving/config_changes.go +++ b/pkg/serving/config_changes.go @@ -28,12 +28,13 @@ import ( // UpdateEnvVars gives the configuration all the env var values listed in the given map of // vars. Does not touch any environment variables not mentioned, but it can add // new env vars and change the values of existing ones. -func UpdateEnvVars(template *servingv1alpha1.RevisionTemplateSpec, vars map[string]string) error { +func UpdateEnvVars(template *servingv1alpha1.RevisionTemplateSpec, toUpdate map[string]string, toRemove []string) error { container, err := ContainerOfRevisionTemplate(template) if err != nil { return err } - container.Env = updateEnvVarsFromMap(container.Env, vars) + envVars := updateEnvVarsFromMap(container.Env, toUpdate) + container.Env = removeEnvVars(envVars, toRemove) return nil } @@ -152,6 +153,26 @@ func UpdateResources(template *servingv1alpha1.RevisionTemplateSpec, requestsRes return nil } +// UpdateLabels updates the labels identically on a service and template. +// Does not overwrite the entire Labels field, only makes the requested updates +func UpdateLabels(service *servingv1alpha1.Service, template *servingv1alpha1.RevisionTemplateSpec, toUpdate map[string]string, toRemove []string) error { + if service.ObjectMeta.Labels == nil { + service.ObjectMeta.Labels = make(map[string]string) + } + if template.ObjectMeta.Labels == nil { + template.ObjectMeta.Labels = make(map[string]string) + } + for key, value := range toUpdate { + service.ObjectMeta.Labels[key] = value + template.ObjectMeta.Labels[key] = value + } + for _, key := range toRemove { + delete(service.ObjectMeta.Labels, key) + delete(template.ObjectMeta.Labels, key) + } + return nil +} + // ======================================================================================= func updateEnvVarsFromMap(env []corev1.EnvVar, vars map[string]string) []corev1.EnvVar { @@ -176,3 +197,15 @@ func updateEnvVarsFromMap(env []corev1.EnvVar, vars map[string]string) []corev1. } return env } + +func removeEnvVars(env []corev1.EnvVar, toRemove []string) []corev1.EnvVar { + for _, name := range toRemove { + for i, envVar := range env { + if envVar.Name == name { + env = append(env[:i], env[i+1:]...) + break + } + } + } + return env +} diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go index e07dc5e089..a3a3f9c393 100644 --- a/pkg/serving/config_changes_test.go +++ b/pkg/serving/config_changes_test.go @@ -26,6 +26,7 @@ import ( servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestUpdateAutoscalingAnnotations(t *testing.T) { @@ -81,13 +82,11 @@ func testUpdateEnvVarsNew(t *testing.T, template *servingv1alpha1.RevisionTempla "a": "foo", "b": "bar", } - err := UpdateEnvVars(template, env) + err := UpdateEnvVars(template, env, []string{}) assert.NilError(t, err) found, err := EnvToMap(container.Env) assert.NilError(t, err) - if !reflect.DeepEqual(env, found) { - t.Fatalf("Env did not match expected %v found %v", env, found) - } + assert.DeepEqual(t, env, found) } func TestUpdateEnvVarsAppendOld(t *testing.T) { @@ -107,7 +106,7 @@ func testUpdateEnvVarsAppendOld(t *testing.T, template *servingv1alpha1.Revision env := map[string]string{ "b": "bar", } - err := UpdateEnvVars(template, env) + err := UpdateEnvVars(template, env, []string{}) assert.NilError(t, err) expected := map[string]string{ @@ -117,9 +116,7 @@ func testUpdateEnvVarsAppendOld(t *testing.T, template *servingv1alpha1.Revision found, err := EnvToMap(container.Env) assert.NilError(t, err) - if !reflect.DeepEqual(expected, found) { - t.Fatalf("Env did not match expected %v, found %v", env, found) - } + assert.DeepEqual(t, expected, found) } func TestUpdateEnvVarsModify(t *testing.T) { @@ -138,7 +135,7 @@ func testUpdateEnvVarsModify(t *testing.T, revision *servingv1alpha1.RevisionTem env := map[string]string{ "a": "fancy", } - err := UpdateEnvVars(revision, env) + err := UpdateEnvVars(revision, env, []string{}) assert.NilError(t, err) expected := map[string]string{ @@ -147,9 +144,35 @@ func testUpdateEnvVarsModify(t *testing.T, revision *servingv1alpha1.RevisionTem found, err := EnvToMap(container.Env) assert.NilError(t, err) - if !reflect.DeepEqual(expected, found) { - t.Fatalf("Env did not match expected %v, found %v", env, found) + assert.DeepEqual(t, expected, found) +} + +func TestUpdateEnvVarsRemove(t *testing.T) { + template, container := getV1alpha1RevisionTemplateWithOldFields() + testUpdateEnvVarsRemove(t, template, container) + assertNoV1alpha1(t, template) + + template, container = getV1alpha1Config() + testUpdateEnvVarsRemove(t, template, container) + assertNoV1alpha1Old(t, template) +} + +func testUpdateEnvVarsRemove(t *testing.T, revision *servingv1alpha1.RevisionTemplateSpec, container *corev1.Container) { + container.Env = []corev1.EnvVar{ + {Name: "a", Value: "foo"}, + {Name: "b", Value: "bar"}, } + remove := []string{"b"} + err := UpdateEnvVars(revision, map[string]string{}, remove) + assert.NilError(t, err) + + expected := map[string]string{ + "a": "foo", + } + + found, err := EnvToMap(container.Env) + assert.NilError(t, err) + assert.DeepEqual(t, expected, found) } func TestUpdateMinScale(t *testing.T) { @@ -238,23 +261,26 @@ func checkPortUpdate(t *testing.T, template *servingv1alpha1.RevisionTemplateSpe func TestUpdateEnvVarsBoth(t *testing.T) { template, container := getV1alpha1RevisionTemplateWithOldFields() - testUpdateEnvVarsBoth(t, template, container) + testUpdateEnvVarsAll(t, template, container) assertNoV1alpha1(t, template) template, container = getV1alpha1Config() - testUpdateEnvVarsBoth(t, template, container) + testUpdateEnvVarsAll(t, template, container) assertNoV1alpha1Old(t, template) } -func testUpdateEnvVarsBoth(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec, container *corev1.Container) { +func testUpdateEnvVarsAll(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec, container *corev1.Container) { container.Env = []corev1.EnvVar{ {Name: "a", Value: "foo"}, - {Name: "c", Value: "caroline"}} + {Name: "c", Value: "caroline"}, + {Name: "d", Value: "byebye"}, + } env := map[string]string{ "a": "fancy", "b": "boo", } - err := UpdateEnvVars(template, env) + remove := []string{"d"} + err := UpdateEnvVars(template, env, remove) assert.NilError(t, err) expected := map[string]string{ @@ -270,11 +296,81 @@ func testUpdateEnvVarsBoth(t *testing.T, template *servingv1alpha1.RevisionTempl } } +func TestUpdateLabelsNew(t *testing.T) { + service, template, _ := getV1alpha1Service() + + labels := map[string]string{ + "a": "foo", + "b": "bar", + } + err := UpdateLabels(service, template, labels, []string{}) + assert.NilError(t, err) + + actual := service.ObjectMeta.Labels + if !reflect.DeepEqual(labels, actual) { + t.Fatalf("Service labels did not match expected %v found %v", labels, actual) + } + + actual = template.ObjectMeta.Labels + if !reflect.DeepEqual(labels, actual) { + t.Fatalf("Template labels did not match expected %v found %v", labels, actual) + } +} + +func TestUpdateLabelsExisting(t *testing.T) { + service, template, _ := getV1alpha1Service() + service.ObjectMeta.Labels = map[string]string{"a": "foo", "b": "bar"} + template.ObjectMeta.Labels = map[string]string{"a": "foo", "b": "bar"} + + labels := map[string]string{ + "a": "notfoo", + "c": "bat", + "d": "", + } + err := UpdateLabels(service, template, labels, []string{}) + assert.NilError(t, err) + expected := map[string]string{ + "a": "notfoo", + "b": "bar", + "c": "bat", + "d": "", + } + + actual := service.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) + + actual = template.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) +} + +func TestUpdateLabelsRemoveExisting(t *testing.T) { + service, template, _ := getV1alpha1Service() + service.ObjectMeta.Labels = map[string]string{"a": "foo", "b": "bar"} + template.ObjectMeta.Labels = map[string]string{"a": "foo", "b": "bar"} + + remove := []string{"b"} + err := UpdateLabels(service, template, map[string]string{}, remove) + assert.NilError(t, err) + expected := map[string]string{ + "a": "foo", + } + + actual := service.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) + + actual = template.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) +} + // ========================================================================================================= func getV1alpha1RevisionTemplateWithOldFields() (*servingv1alpha1.RevisionTemplateSpec, *corev1.Container) { container := &corev1.Container{} template := &servingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: "template-foo", + Namespace: "default", + }, Spec: servingv1alpha1.RevisionSpec{ DeprecatedContainer: container, }, @@ -285,6 +381,10 @@ func getV1alpha1RevisionTemplateWithOldFields() (*servingv1alpha1.RevisionTempla func getV1alpha1Config() (*servingv1alpha1.RevisionTemplateSpec, *corev1.Container) { containers := []corev1.Container{{}} template := &servingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: "template-foo", + Namespace: "default", + }, Spec: servingv1alpha1.RevisionSpec{ RevisionSpec: v1beta1.RevisionSpec{ PodSpec: v1beta1.PodSpec{ @@ -296,6 +396,28 @@ func getV1alpha1Config() (*servingv1alpha1.RevisionTemplateSpec, *corev1.Contain return template, &containers[0] } +func getV1alpha1Service() (*servingv1alpha1.Service, *servingv1alpha1.RevisionTemplateSpec, *corev1.Container) { + template, container := getV1alpha1RevisionTemplateWithOldFields() + service := &servingv1alpha1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: "knative.dev/v1alph1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "default", + }, + Spec: servingv1alpha1.ServiceSpec{ + DeprecatedRunLatest: &servingv1alpha1.RunLatestType{ + Configuration: servingv1alpha1.ConfigurationSpec{ + DeprecatedRevisionTemplate: template, + }, + }, + }, + } + return service, template, container +} + func assertNoV1alpha1Old(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec) { if template.Spec.DeprecatedContainer != nil { t.Error("Assuming only new v1alpha1 fields but found spec.container") diff --git a/pkg/util/parsing_helper.go b/pkg/util/parsing_helper.go new file mode 100644 index 0000000000..0deff0994b --- /dev/null +++ b/pkg/util/parsing_helper.go @@ -0,0 +1,48 @@ +// Copyright © 2019 The Knative Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "fmt" + "strings" +) + +func MapFromArrayAllowingSingles(arr []string, delimiter string) (map[string]string, error) { + return mapFromArray(arr, delimiter, true) +} + +func MapFromArray(arr []string, delimiter string) (map[string]string, error) { + return mapFromArray(arr, delimiter, false) +} + +// mapFromArray takes an array of strings where each item is a (key, value) pair +// separated by a delimiter and returns a map where keys are mapped to their respsective values. +// If allowSingles is true, values without a delimiter will be added as keys pointing to empty strings +func mapFromArray(arr []string, delimiter string, allowSingles bool) (map[string]string, error) { + returnMap := map[string]string{} + for _, pairStr := range arr { + pairSlice := strings.SplitN(pairStr, delimiter, 2) + if len(pairSlice) <= 1 { + if len(pairSlice) == 0 || !allowSingles { + return nil, fmt.Errorf("Argument requires a value that contains the %q character; got %q", delimiter, pairStr) + } else { + returnMap[pairSlice[0]] = "" + } + } else { + returnMap[pairSlice[0]] = pairSlice[1] + } + } + return returnMap, nil +} diff --git a/pkg/util/parsing_helper_test.go b/pkg/util/parsing_helper_test.go new file mode 100644 index 0000000000..686c18015e --- /dev/null +++ b/pkg/util/parsing_helper_test.go @@ -0,0 +1,62 @@ +// Copyright © 2019 The Knative Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "testing" + + "gotest.tools/assert" +) + +func TestMapFromArray(t *testing.T) { + testMapFromArray(t, []string{"good=value"}, "=", map[string]string{"good": "value"}) + testMapFromArray(t, []string{"multi=value", "other=value"}, "=", map[string]string{"multi": "value", "other": "value"}) + testMapFromArray(t, []string{"over|write", "over|written"}, "|", map[string]string{"over": "written"}) + testMapFromArray(t, []string{"only,split,once", "just,once,"}, ",", map[string]string{"only": "split,once", "just": "once,"}) + testMapFromArray(t, []string{"empty=", "="}, "=", map[string]string{"empty": "", "": ""}) +} + +func testMapFromArray(t *testing.T, input []string, delimiter string, expected map[string]string) { + actual, err := MapFromArray(input, delimiter) + assert.NilError(t, err) + assert.DeepEqual(t, expected, actual) +} + +func TestMapFromArrayNoDelimiter(t *testing.T) { + input := []string{"badvalue"} + _, err := MapFromArray(input, "+") + assert.ErrorContains(t, err, "Argument requires") + assert.ErrorContains(t, err, "+") +} + +func TestMapFromArrayNoDelimiterAllowingSingles(t *testing.T) { + input := []string{"okvalue"} + actual, err := MapFromArrayAllowingSingles(input, "+") + expected := map[string]string{"okvalue": ""} + assert.NilError(t, err) + assert.DeepEqual(t, expected, actual) +} + +func TestMapFromArrayEmptyValueEmptyDelimiter(t *testing.T) { + input := []string{""} + _, err := MapFromArray(input, "") + assert.ErrorContains(t, err, "Argument requires") +} + +func TestMapFromArrayEmptyValueEmptyDelimiterAllowingSingles(t *testing.T) { + input := []string{""} + _, err := MapFromArrayAllowingSingles(input, "") + assert.ErrorContains(t, err, "Argument requires") +}