diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 4bfb25ff1f..8feff68e42 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -61,6 +61,7 @@ kn service create NAME --image IMAGE --concurrency-utilization int Percentage of concurrent requests utilization before scaling up. (default 70) -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-). --env-from stringArray Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). Example: --env-from cm:myconfigmap or --env-from secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --env-from cm:myconfigmap-. + -f, --filename string Create a service from file. --force Create service forcefully, replaces existing service if any. -h, --help help for create --image string Image to run. diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index a188d694db..6e843e2455 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -68,6 +68,8 @@ type ConfigurationEditFlags struct { GenerateRevisionName bool ForceCreate bool + Filename string + // Bookkeeping flags []string } @@ -272,7 +274,8 @@ func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) { p.addSharedFlags(command) command.Flags().BoolVar(&p.ForceCreate, "force", false, "Create service forcefully, replaces existing service if any.") - command.MarkFlagRequired("image") + command.Flags().StringVarP(&p.Filename, "filename", "f", "", "Create a service from file.") + command.MarkFlagFilename("filename") } // Apply mutates the given service according to the flags in the command. diff --git a/pkg/kn/commands/service/create.go b/pkg/kn/commands/service/create.go index 249f3f65bd..8194b77826 100644 --- a/pkg/kn/commands/service/create.go +++ b/pkg/kn/commands/service/create.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "io" + "os" "knative.dev/client/pkg/kn/commands" servinglib "knative.dev/client/pkg/serving" @@ -28,6 +29,8 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/yaml" + servingv1 "knative.dev/serving/pkg/apis/serving/v1" clientservingv1 "knative.dev/client/pkg/serving/v1" @@ -75,11 +78,14 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { Short: "Create a service", Example: create_example, RunE: func(cmd *cobra.Command, args []string) (err error) { - if len(args) != 1 { + if len(args) != 1 && editFlags.Filename == "" { return errors.New("'service create' requires the service name given as single argument") } - name := args[0] - if editFlags.Image == "" { + name := "" + if len(args) == 1 { + name = args[0] + } + if editFlags.Image == "" && editFlags.Filename == "" { return errors.New("'service create' requires the image name to run provided with the --image option") } @@ -88,7 +94,12 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { return err } - service, err := constructService(cmd, editFlags, name, namespace) + var service *servingv1.Service + if editFlags.Filename == "" { + service, err = constructService(cmd, editFlags, name, namespace) + } else { + service, err = constructServiceFromFile(cmd, editFlags, name, namespace) + } if err != nil { return err } @@ -97,8 +108,7 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { if err != nil { return err } - - serviceExists, err := serviceExists(client, name) + serviceExists, err := serviceExists(client, service.Name) if err != nil { return err } @@ -108,7 +118,7 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { if !editFlags.ForceCreate { return fmt.Errorf( "cannot create service '%s' in namespace '%s' "+ - "because the service already exists and no --force option was given", name, namespace) + "because the service already exists and no --force option was given", service.Name, namespace) } err = replaceService(client, service, waitFlags, out) } else { @@ -260,3 +270,42 @@ func constructService(cmd *cobra.Command, editFlags ConfigurationEditFlags, name } return &service, nil } + +// constructServiceFromFile creates struct from provided file +func constructServiceFromFile(cmd *cobra.Command, editFlags ConfigurationEditFlags, name, namespace string) (*servingv1.Service, error) { + var service servingv1.Service + file, err := os.Open(editFlags.Filename) + if err != nil { + return nil, err + } + decoder := yaml.NewYAMLOrJSONDecoder(file, 512) + + err = decoder.Decode(&service) + if err != nil { + return nil, err + } + if name == "" && service.Name != "" { + // keep provided service.Name if name param is empty + } else if name != "" && service.Name == "" { + service.Name = name + } else if name != "" && service.Name != "" { + // throw error if names differ, otherwise use already set value + if name != service.Name { + return nil, fmt.Errorf("provided service name '%s' doesn't match name from file '%s'", name, service.Name) + } + } else { + return nil, fmt.Errorf("no service name provided in command parameter or file") + } + + // Set namespace in case it's specified as --namespace + service.ObjectMeta.Namespace = namespace + + // We need to generate revision to have --force replace working + revName, err := servinglib.GenerateRevisionName(editFlags.RevisionName, &service) + if err != nil { + return nil, err + } + service.Spec.Template.Name = revName + + return &service, nil +} diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go index d31ec821cc..1a1652fa15 100644 --- a/pkg/kn/commands/service/create_test.go +++ b/pkg/kn/commands/service/create_test.go @@ -17,6 +17,9 @@ package service import ( "errors" "fmt" + "io/ioutil" + "os" + "path/filepath" "reflect" "strings" "testing" @@ -707,3 +710,196 @@ func TestServiceCreateWithClusterLocal(t *testing.T) { t.Fatalf("Incorrect VisibilityClusterLocal value '%s'", labelValue) } } + +var serviceYAML = ` +apiVersion: serving.knative.dev/v1 +kind: Service +metadata: + name: foo +spec: + template: + spec: + containers: + - image: gcr.io/foo/bar:baz + env: + - name: TARGET + value: "Go Sample v1" +` + +var serviceJSON = ` +{ + "apiVersion": "serving.knative.dev/v1", + "kind": "Service", + "metadata": { + "name": "foo" + }, + "spec": { + "template": { + "spec": { + "containers": [ + { + "image": "gcr.io/foo/bar:baz", + "env": [ + { + "name": "TARGET", + "value": "Go Sample v1" + } + ] + } + ] + } + } + } +}` + +func TestServiceCreateFromYAML(t *testing.T) { + tempDir, err := ioutil.TempDir("", "kn-file") + defer os.RemoveAll(tempDir) + assert.NilError(t, err) + + tempFile := filepath.Join(tempDir, "service.yaml") + err = ioutil.WriteFile(tempFile, []byte(serviceYAML), os.FileMode(0666)) + assert.NilError(t, err) + + action, created, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--filename", tempFile}, false) + assert.NilError(t, err) + assert.Assert(t, action.Matches("create", "services")) + + assert.Equal(t, created.Name, "foo") + assert.Equal(t, created.Spec.Template.Spec.GetContainer().Image, "gcr.io/foo/bar:baz") +} + +func TestServiceCreateFromJSON(t *testing.T) { + tempDir, err := ioutil.TempDir("", "kn-file") + defer os.RemoveAll(tempDir) + assert.NilError(t, err) + + tempFile := filepath.Join(tempDir, "service.json") + err = ioutil.WriteFile(tempFile, []byte(serviceJSON), os.FileMode(0666)) + assert.NilError(t, err) + + action, created, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--filename", tempFile}, false) + assert.NilError(t, err) + assert.Assert(t, action.Matches("create", "services")) + + assert.Equal(t, created.Name, "foo") + assert.Equal(t, created.Spec.Template.Spec.GetContainer().Image, "gcr.io/foo/bar:baz") +} + +func TestServiceCreateFromFileWithName(t *testing.T) { + tempDir, err := ioutil.TempDir("", "kn-file") + defer os.RemoveAll(tempDir) + assert.NilError(t, err) + + tempFile := filepath.Join(tempDir, "service.yaml") + err = ioutil.WriteFile(tempFile, []byte(serviceYAML), os.FileMode(0666)) + assert.NilError(t, err) + + t.Log("no NAME param provided") + action, created, _, err := fakeServiceCreate([]string{ + "service", "create", "--filename", tempFile}, false) + assert.NilError(t, err) + assert.Assert(t, action.Matches("create", "services")) + + assert.Equal(t, created.Name, "foo") + assert.Equal(t, created.Spec.Template.Spec.GetContainer().Image, "gcr.io/foo/bar:baz") + + t.Log("no service.Name provided in file") + err = ioutil.WriteFile(tempFile, []byte(strings.ReplaceAll(serviceYAML, "name: foo", "")), os.FileMode(0666)) + assert.NilError(t, err) + action, created, _, err = fakeServiceCreate([]string{ + "service", "create", "cli-foo", "--filename", tempFile}, false) + assert.NilError(t, err) + assert.Assert(t, action.Matches("create", "services")) + + assert.Equal(t, created.Name, "cli-foo") + assert.Equal(t, created.Spec.Template.Spec.GetContainer().Image, "gcr.io/foo/bar:baz") +} + +func TestServiceCreateFileNameMismatch(t *testing.T) { + tempDir, err := ioutil.TempDir("", "kn-file") + assert.NilError(t, err) + + tempFile := filepath.Join(tempDir, "service.json") + err = ioutil.WriteFile(tempFile, []byte(serviceJSON), os.FileMode(0666)) + assert.NilError(t, err) + + t.Log("NAME param nad service.Name differ") + _, _, _, err = fakeServiceCreate([]string{ + "service", "create", "anotherFoo", "--filename", tempFile}, false) + assert.Assert(t, err != nil) + assert.Assert(t, util.ContainsAllIgnoreCase(err.Error(), "provided", "'anotherFoo'", "name", "match", "from", "file", "'foo'")) + + t.Log("no NAME param & no service.Name provided in file") + err = ioutil.WriteFile(tempFile, []byte(strings.ReplaceAll(serviceYAML, "name: foo", "")), os.FileMode(0666)) + assert.NilError(t, err) + _, _, _, err = fakeServiceCreate([]string{ + "service", "create", "--filename", tempFile}, false) + assert.Assert(t, err != nil) + assert.Assert(t, util.ContainsAllIgnoreCase(err.Error(), "no", "service", "name", "provided", "parameter", "file")) +} + +func TestServiceCreateFileError(t *testing.T) { + _, _, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--filename", "filepath"}, false) + + assert.Assert(t, util.ContainsAll(err.Error(), "no", "such", "file", "directory", "filepath")) +} + +func TestServiceCreateInvalidDataJSON(t *testing.T) { + tempDir, err := ioutil.TempDir("", "kn-file") + defer os.RemoveAll(tempDir) + assert.NilError(t, err) + tempFile := filepath.Join(tempDir, "invalid.json") + + // Double curly bracket at the beginning of file + invalidData := strings.Replace(serviceJSON, "{\n", "{{\n", 1) + err = ioutil.WriteFile(tempFile, []byte(invalidData), os.FileMode(0666)) + assert.NilError(t, err) + _, _, _, err = fakeServiceCreate([]string{"service", "create", "foo", "--filename", tempFile}, false) + assert.Assert(t, util.ContainsAll(err.Error(), "invalid", "character", "'{'", "beginning")) + + // Remove closing quote on key + invalidData = strings.Replace(serviceJSON, "metadata\"", "metadata", 1) + err = ioutil.WriteFile(tempFile, []byte(invalidData), os.FileMode(0666)) + assert.NilError(t, err) + _, _, _, err = fakeServiceCreate([]string{"service", "create", "foo", "--filename", tempFile}, false) + assert.Assert(t, util.ContainsAll(err.Error(), "invalid", "character", "'\\n'", "string", "literal")) + + // Remove opening square bracket + invalidData = strings.Replace(serviceJSON, " [", "", 1) + err = ioutil.WriteFile(tempFile, []byte(invalidData), os.FileMode(0666)) + assert.NilError(t, err) + _, _, _, err = fakeServiceCreate([]string{"service", "create", "foo", "--filename", tempFile}, false) + assert.Assert(t, util.ContainsAll(err.Error(), "invalid", "character", "']'", "after", "key:value")) +} + +func TestServiceCreateInvalidDataYAML(t *testing.T) { + tempDir, err := ioutil.TempDir("", "kn-file") + defer os.RemoveAll(tempDir) + assert.NilError(t, err) + tempFile := filepath.Join(tempDir, "invalid.yaml") + + // Remove dash + invalidData := strings.Replace(serviceYAML, "- image", "image", 1) + err = ioutil.WriteFile(tempFile, []byte(invalidData), os.FileMode(0666)) + assert.NilError(t, err) + _, _, _, err = fakeServiceCreate([]string{"service", "create", "foo", "--filename", tempFile}, false) + assert.Assert(t, util.ContainsAll(err.Error(), "mapping", "values", "not", "allowed")) + + // Remove name key + invalidData = strings.Replace(serviceYAML, "name:", "", 1) + err = ioutil.WriteFile(tempFile, []byte(invalidData), os.FileMode(0666)) + assert.NilError(t, err) + _, _, _, err = fakeServiceCreate([]string{"service", "create", "foo", "--filename", tempFile}, false) + assert.Assert(t, util.ContainsAll(err.Error(), "cannot", "unmarshal", "Go", "struct", "Service.metadata")) + + // Remove opening square bracket + invalidData = strings.Replace(serviceYAML, "env", "\tenv", 1) + err = ioutil.WriteFile(tempFile, []byte(invalidData), os.FileMode(0666)) + assert.NilError(t, err) + _, _, _, err = fakeServiceCreate([]string{"service", "create", "foo", "--filename", tempFile}, false) + assert.Assert(t, util.ContainsAll(err.Error(), "found", "tab", "violates", "indentation")) +} diff --git a/test/e2e/service_file_test.go b/test/e2e/service_file_test.go new file mode 100644 index 0000000000..e5a0f44125 --- /dev/null +++ b/test/e2e/service_file_test.go @@ -0,0 +1,133 @@ +// Copyright 2020 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. + +// +build e2e +// +build !eventing + +package e2e + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "gotest.tools/assert" + + "knative.dev/client/lib/test" + "knative.dev/client/pkg/util" +) + +const ( + ServiceYAML string = ` +apiVersion: serving.knative.dev/v1 +kind: Service +metadata: + name: foo-yaml +spec: + template: + spec: + containers: + - image: %s + env: + - name: TARGET + value: "Go Sample v1"` + + ServiceJSON string = ` +{ + "apiVersion": "serving.knative.dev/v1", + "kind": "Service", + "metadata": { + "name": "foo-json" + }, + "spec": { + "template": { + "spec": { + "containers": [ + { + "image": "%s", + "env": [ + { + "name": "TARGET", + "value": "Go Sample v1" + } + ] + } + ] + } + } + } +}` +) + +func TestServiceCreateFromFile(t *testing.T) { + t.Parallel() + it, err := test.NewKnTest() + assert.NilError(t, err) + defer func() { + assert.NilError(t, it.Teardown()) + }() + + r := test.NewKnRunResultCollector(t, it) + defer r.DumpIfFailed() + + tempDir, err := ioutil.TempDir("", "kn-file") + defer os.RemoveAll(tempDir) + assert.NilError(t, err) + + test.CreateFile("foo.json", fmt.Sprintf(ServiceJSON, test.KnDefaultTestImage), tempDir, test.FileModeReadWrite) + test.CreateFile("foo.yaml", fmt.Sprintf(ServiceYAML, test.KnDefaultTestImage), tempDir, test.FileModeReadWrite) + + t.Log("create foo-json service from JSON file") + serviceCreateFromFile(r, "foo-json", filepath.Join(tempDir, "foo.json"), true) + + t.Log("create foo-yaml service from YAML file") + serviceCreateFromFile(r, "foo-yaml", filepath.Join(tempDir, "foo.yaml"), false) + + t.Log("error message for non-existing file") + serviceCreateFromFileError(r, "foo", filepath.Join(tempDir, "fake-foo.json")) + serviceCreateFromFileError(r, "foo", filepath.Join(tempDir, "fake-foo.yaml")) + + t.Log("error message for mismatch names") + serviceCreateFromFileNameMismatch(r, "foo", filepath.Join(tempDir, "foo.json")) + serviceCreateFromFileNameMismatch(r, "foo", filepath.Join(tempDir, "foo.yaml")) +} + +func serviceCreateFromFile(r *test.KnRunResultCollector, serviceName, filePath string, useName bool) { + var out test.KnRunResult + if useName { + out = r.KnTest().Kn().Run("service", "create", serviceName, "-f", filePath) + } else { + out = r.KnTest().Kn().Run("service", "create", "-f", filePath) + } + r.AssertNoError(out) + assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, "service", serviceName, "creating", "namespace", r.KnTest().Kn().Namespace(), "ready")) + + out = r.KnTest().Kn().Run("service", "describe", serviceName, "--verbose") + r.AssertNoError(out) + assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, serviceName)) +} + +func serviceCreateFromFileError(r *test.KnRunResultCollector, serviceName, filePath string) { + out := r.KnTest().Kn().Run("service", "create", serviceName, "--filename", filePath) + r.AssertError(out) + assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stderr, "no", "such", "file", "directory", filePath)) +} + +func serviceCreateFromFileNameMismatch(r *test.KnRunResultCollector, serviceName, filePath string) { + out := r.KnTest().Kn().Run("service", "create", serviceName, "--filename", filePath) + r.AssertError(out) + assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stderr, "provided", "'"+serviceName+"'", "name", "match", "from", "file")) +}