fix: Refactor name param restrictions

pkg/kn/commands/service/create.go

@@ -78,10 +78,13 @@ 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]
+			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")
 			}
@@ -105,7 +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
 			}
@@ -115,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 {
@@ -281,18 +284,28 @@ func constructServiceFromFile(cmd *cobra.Command, editFlags ConfigurationEditFla
 	if err != nil {
 		return nil, err
 	}
-	if service.Name != name {
-		return nil, fmt.Errorf("provided service name '%s' doesn't match name from file '%s'", name, service.Name)
+	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, err := servinglib.GenerateRevisionName(editFlags.RevisionName, &service)
+	if err != nil {
+		return nil, err
+	}
+	service.Spec.Template.Name = revName
 
 	return &service, nil
 }

pkg/kn/commands/service/create_test.go

@@ -788,6 +788,36 @@ func TestServiceCreateFromJSON(t *testing.T) {
 	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)
@@ -796,10 +826,19 @@ func TestServiceCreateFileNameMismatch(t *testing.T) {
 	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) {
@@ -862,5 +901,5 @@ func TestServiceCreateInvalidDataYAML(t *testing.T) {
 	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", "found", "found", "violates", "violates"))
+	assert.Assert(t, util.ContainsAll(err.Error(), "found", "tab", "violates", "indentation"))
 }

test/e2e/service_file_test.go

@@ -91,18 +91,27 @@ func TestServiceCreateFromFile(t *testing.T) {
 	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"))
+	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"))
+	serviceCreateFromFile(r, "foo-yaml", filepath.Join(tempDir, "foo.yaml"), false)
 
 	t.Log("error message for non-existing file")
-	serviceCreateFromFileNameMismatch(r, "foo", filepath.Join(tempDir, "foo.yaml"))
+	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) {
-	out := r.KnTest().Kn().Run("service", "create", serviceName, "-f", filePath)
+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"))