Repository: apigee/registry (go)

Issue:
Registry tool: setting unknown `registry config` parameters seems to silently fail
`invalid` here could be any unrecognized parameter.

```
% registry config set invalid parameter
----
Need more help?
https://github.com/apigee/registry/wiki

% registry config set invalid 123      
----
Need more help?
https://github.com/apigee/registry/wiki
```

Pull request:
enable usage and error messages, minor refactoring
Fixes #1096

Patch:
diff --git a/cmd/registry/cmd/auth/print_token.go b/cmd/registry/cmd/auth/print_token.go
index 17e2260..d445d99 100644
--- a/cmd/registry/cmd/auth/print_token.go
+++ b/cmd/registry/cmd/auth/print_token.go
@@ -27,9 +27,6 @@ func printTokenCommand() *cobra.Command {
 		Short: "Invoke token-source to print a token",
 		Args:  cobra.ExactArgs(0),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
-
 			_, c, err := config.ActiveRaw()
 			if err != nil {
 				if err == config.ErrNoActiveConfiguration {
diff --git a/cmd/registry/cmd/config/config.go b/cmd/registry/cmd/config/config.go
index 62f1d01..cc1c114 100644
--- a/cmd/registry/cmd/config/config.go
+++ b/cmd/registry/cmd/config/config.go
@@ -15,8 +15,6 @@
 package config
 
 import (
-	"fmt"
-
 	"github.com/apigee/registry/cmd/registry/cmd/config/configurations"
 	"github.com/spf13/cobra"
 )
@@ -38,11 +36,3 @@ func Command() *cobra.Command {
 	cmd.AddCommand(unsetCommand())
 	return cmd
 }
-
-type UnknownPropertyError struct {
-	property string
-}
-
-func (n UnknownPropertyError) Error() string {
-	return fmt.Sprintf("Unknown property: %q.", n.property)
-}
diff --git a/cmd/registry/cmd/config/configurations/activate.go b/cmd/registry/cmd/config/configurations/activate.go
index cf15bee..453829a 100644
--- a/cmd/registry/cmd/config/configurations/activate.go
+++ b/cmd/registry/cmd/config/configurations/activate.go
@@ -27,8 +27,6 @@ func activateCommand() *cobra.Command {
 		Short: "Activates an existing named configuration",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			name := args[0]
 			if err := config.ValidateName(name); err != nil {
 				return err
diff --git a/cmd/registry/cmd/config/configurations/create.go b/cmd/registry/cmd/config/configurations/create.go
index 24d9c34..60dcfa9 100644
--- a/cmd/registry/cmd/config/configurations/create.go
+++ b/cmd/registry/cmd/config/configurations/create.go
@@ -30,8 +30,6 @@ func createCommand() *cobra.Command {
 		Example: "registry config configurations create localhost --registry.address='locahost:8080'",
 		Args:    cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			name := args[0]
 			if err := config.ValidateName(name); err != nil {
 				return err
diff --git a/cmd/registry/cmd/config/configurations/delete.go b/cmd/registry/cmd/config/configurations/delete.go
index 8110121..b4ce254 100644
--- a/cmd/registry/cmd/config/configurations/delete.go
+++ b/cmd/registry/cmd/config/configurations/delete.go
@@ -28,8 +28,6 @@ func deleteCommand() *cobra.Command {
 		Short: "Deletes a named configuration",
 		Args:  cobra.MinimumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			for _, name := range args {
 				if err := config.ValidateName(name); err != nil {
 					return err
diff --git a/cmd/registry/cmd/config/configurations/describe.go b/cmd/registry/cmd/config/configurations/describe.go
index 24733ea..c7f0d8a 100644
--- a/cmd/registry/cmd/config/configurations/describe.go
+++ b/cmd/registry/cmd/config/configurations/describe.go
@@ -28,8 +28,6 @@ func describeCommand() *cobra.Command {
 		Short: "Describes a named configuration by listing its properties",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			name := args[0]
 			s, err := config.Read(name)
 			if err != nil {
diff --git a/cmd/registry/cmd/config/configurations/list.go b/cmd/registry/cmd/config/configurations/list.go
index 5c4eb1e..a664b12 100644
--- a/cmd/registry/cmd/config/configurations/list.go
+++ b/cmd/registry/cmd/config/configurations/list.go
@@ -31,8 +31,6 @@ func listCommand() *cobra.Command {
 		Short: "Lists existing named configs",
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			configs, err := config.Configurations()
 			if errors.Is(err, fs.ErrNotExist) || len(configs) == 0 {
 				cmd.Println("You don't have any configurations. Run 'registry config configurations create' to create a configuration.")
diff --git a/cmd/registry/cmd/config/get.go b/cmd/registry/cmd/config/get.go
index f1fe98a..7738f8c 100644
--- a/cmd/registry/cmd/config/get.go
+++ b/cmd/registry/cmd/config/get.go
@@ -27,8 +27,6 @@ func getCommand() *cobra.Command {
 		Short: "Print a property value in the active configuration",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			_, c, err := config.ActiveRaw()
 			if err != nil {
 				if err == config.ErrNoActiveConfiguration {
@@ -37,13 +35,9 @@ func getCommand() *cobra.Command {
 				return fmt.Errorf("cannot read config: %v", err)
 			}
 
-			if !contains(c.Properties(), args[0]) {
-				return UnknownPropertyError{args[0]}
-			}
-
 			v, err := c.Get(args[0])
 			if err != nil {
-				return fmt.Errorf("cannot decode config: %v", err)
+				return err
 			}
 
 			cmd.Println(v)
diff --git a/cmd/registry/cmd/config/list.go b/cmd/registry/cmd/config/list.go
index f2d4012..ded1a42 100644
--- a/cmd/registry/cmd/config/list.go
+++ b/cmd/registry/cmd/config/list.go
@@ -27,8 +27,6 @@ func listCommand() *cobra.Command {
 		Short: "List properties in the active configuration",
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			target, c, err := config.ActiveRaw()
 			if err != nil {
 				if err == config.ErrNoActiveConfiguration {
diff --git a/cmd/registry/cmd/config/set.go b/cmd/registry/cmd/config/set.go
index f8bda57..5bbb94e 100644
--- a/cmd/registry/cmd/config/set.go
+++ b/cmd/registry/cmd/config/set.go
@@ -35,8 +35,6 @@ func setCommand() *cobra.Command {
 			"registry config set token-source 'gcloud auth print-access-token email@example.com'",
 		Args: cobra.ExactArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			target, c, err := config.ActiveRaw()
 			if err != nil {
 				if err == config.ErrNoActiveConfiguration {
@@ -46,12 +44,8 @@ func setCommand() *cobra.Command {
 			}
 
 			name, value := args[0], args[1]
-			if !contains(c.Properties(), name) {
-				return UnknownPropertyError{name}
-			}
-
 			if err = c.Set(name, value); err != nil {
-				return fmt.Errorf("cannot set value: %v", err)
+				return err
 			}
 
 			if err = c.Write(target); err != nil {
@@ -64,12 +58,3 @@ func setCommand() *cobra.Command {
 	}
 	return cmd
 }
-
-func contains(arr []string, x string) bool {
-	for _, v := range arr {
-		if v == x {
-			return true
-		}
-	}
-	return false
-}
diff --git a/cmd/registry/cmd/config/unset.go b/cmd/registry/cmd/config/unset.go
index 0ccac38..36ff455 100644
--- a/cmd/registry/cmd/config/unset.go
+++ b/cmd/registry/cmd/config/unset.go
@@ -27,8 +27,6 @@ func unsetCommand() *cobra.Command {
 		Short: "Clear the property value from the active configuration",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			target, c, err := config.ActiveRaw()
 			if err != nil {
 				if err == config.ErrNoActiveConfiguration {
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 9bac683..4123027 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -60,6 +60,7 @@ func CreateFlagSet() *pflag.FlagSet {
 	flags := pflag.NewFlagSet("registry", pflag.ExitOnError)
 	flags.StringP("config", "c", "", "name of a configuration profile or path to config file")
 	flags.String("registry.address", "", "the server and port of the Registry API (eg. localhost:8080)")
+	flags.String("address", "", "the server and port of the Registry API (eg. localhost:8080)")
 	flags.Bool("registry.insecure", false, "if specified, client connects via http (not https)")
 	flags.String("registry.location", "", "the API Registry location")
 	flags.String("registry.project", "", "the API Registry project")
@@ -215,11 +216,11 @@ func ReadValid(name string) (c Configuration, err error) {
 // file: does not bind to env vars or flags, resolve, or validate.
 // See also: ReadValid()
 func Read(name string) (c Configuration, err error) {
-	dir, file := filepath.Split(name)
-	if file == "" {
-		return c, fmt.Errorf("configuration name cannot be empty")
+	if err = ValidateName(name); err != nil {
+		return
 	}
 
+	dir, file := filepath.Split(name)
 	if dir == "" {
 		name = filepath.Join(Directory, file)
 	}
diff --git a/pkg/config/configuration.go b/pkg/config/configuration.go
index 4790c63..7646b0f 100644
--- a/pkg/config/configuration.go
+++ b/pkg/config/configuration.go
@@ -63,7 +63,7 @@ func (c Configuration) Write(name string) error {
 	return enc.Encode(c)
 }
 
-// Validate returns an error if Config is invalid.
+// Validate returns a ValidationError if Config is invalid.
 func (c Configuration) Validate() error {
 	if c.Registry.Address == "" {
 		return ValidationError{
@@ -73,6 +73,20 @@ func (c Configuration) Validate() error {
 	return nil
 }
 
+// ValidateProperty returns an UnknownPropertyError if not a valid property.
+func (c Configuration) ValidateProperty(k string) error {
+	kNS := k
+	if !strings.Contains(k, ".") {
+		kNS = default_namespace + "." + kNS
+	}
+	for _, p := range c.Properties() {
+		if p == kNS {
+			return nil
+		}
+	}
+	return UnknownPropertyError{k}
+}
+
 // Properties returns a sorted list of all valid property names.
 func (c Configuration) Properties() []string {
 	props := properties(c, "")
@@ -91,6 +105,9 @@ func (c Configuration) FlatMap() (map[string]interface{}, error) {
 
 // Set sets a property from a qualified or default namespace name.
 func (c *Configuration) Set(k string, v interface{}) error {
+	if err := c.ValidateProperty(k); err != nil {
+		return err
+	}
 	if !strings.Contains(k, ".") {
 		k = default_namespace + "." + k
 	}
@@ -101,6 +118,9 @@ func (c *Configuration) Set(k string, v interface{}) error {
 
 // Unset removed a property by qualified or default namespace name.
 func (c *Configuration) Unset(k string) error {
+	if err := c.ValidateProperty(k); err != nil {
+		return err
+	}
 	if !strings.Contains(k, ".") {
 		k = default_namespace + "." + k
 	}
@@ -111,15 +131,16 @@ func (c *Configuration) Unset(k string) error {
 
 // Get gets a property from a qualified or default namespace name.
 func (c *Configuration) Get(k string) (interface{}, error) {
+	if err := c.ValidateProperty(k); err != nil {
+		return nil, err
+	}
 	m, err := c.FlatMap()
 	if err != nil {
 		return "", fmt.Errorf("cannot decode config: %v", err)
 	}
-
 	if !strings.Contains(k, ".") {
 		k = default_namespace + "." + k
 	}
-
 	return m[k], nil
 }
 
@@ -210,3 +231,11 @@ func unflattenMap(src map[string]interface{}) (dest map[string]interface{}) {
 	}
 	return dest
 }
+
+type UnknownPropertyError struct {
+	Property string
+}
+
+func (n UnknownPropertyError) Error() string {
+	return fmt.Sprintf("unknown property: %q.", n.Property)
+}


Test patch:
diff --git a/cmd/registry/cmd/apply/apply_test.go b/cmd/registry/cmd/apply/apply_test.go
index fee8d00..0ec97d8 100644
--- a/cmd/registry/cmd/apply/apply_test.go
+++ b/cmd/registry/cmd/apply/apply_test.go
@@ -110,8 +110,6 @@ func TestApplyErrors(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
 			cmd := Command()
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			cmd.SetArgs(test.args)
 			if err := cmd.Execute(); err == nil {
 				t.Fatalf("Execute() with args %+v succeeded, expected error", cmd.Args)
diff --git a/cmd/registry/cmd/config/config_test.go b/cmd/registry/cmd/config/config_test.go
index 65e1f4f..5e6ab99 100644
--- a/cmd/registry/cmd/config/config_test.go
+++ b/cmd/registry/cmd/config/config_test.go
@@ -92,7 +92,7 @@ func TestConfig(t *testing.T) {
 
 	cmd := setCommand()
 	cmd.SetArgs([]string{"test", "test"})
-	if err := cmd.Execute(); !errors.Is(err, UnknownPropertyError{"test"}) {
+	if err := cmd.Execute(); !errors.Is(err, config.UnknownPropertyError{Property: "test"}) {
 		t.Errorf("expected UnknownPropertyError")
 	}
 
@@ -122,7 +122,7 @@ func TestConfig(t *testing.T) {
 
 	cmd = getCommand()
 	cmd.SetArgs([]string{"test"})
-	if err := cmd.Execute(); !errors.Is(err, UnknownPropertyError{"test"}) {
+	if err := cmd.Execute(); !errors.Is(err, config.UnknownPropertyError{Property: "test"}) {
 		t.Errorf("expected UnknownPropertyError")
 	}
 
diff --git a/cmd/registry/cmd/delete/delete_test.go b/cmd/registry/cmd/delete/delete_test.go
index d11503e..99a0756 100644
--- a/cmd/registry/cmd/delete/delete_test.go
+++ b/cmd/registry/cmd/delete/delete_test.go
@@ -172,8 +172,6 @@ func TestDeleteInvalidResources(t *testing.T) {
 	for _, r := range resources {
 		t.Run(r, func(t *testing.T) {
 			cmd := Command()
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			args := []string{r, "--force"}
 			cmd.SetArgs(args)
 			if err := cmd.Execute(); err == nil {
@@ -184,8 +182,6 @@ func TestDeleteInvalidResources(t *testing.T) {
 	// Verify that we get an error if --filter is used with a specific resource.
 	t.Run("filter-with-specific-resource", func(t *testing.T) {
 		cmd := Command()
-		cmd.SilenceUsage = true
-		cmd.SilenceErrors = true
 		args := []string{"projects/my-project/locations/global/apis/a", "--filter", "true"}
 		cmd.SetArgs(args)
 		if err := cmd.Execute(); err == nil {
diff --git a/cmd/registry/cmd/get/get_test.go b/cmd/registry/cmd/get/get_test.go
index 0c00f9e..561832a 100644
--- a/cmd/registry/cmd/get/get_test.go
+++ b/cmd/registry/cmd/get/get_test.go
@@ -249,8 +249,6 @@ func TestGetInvalidResources(t *testing.T) {
 			// cycle through output types
 			format := []string{"name", "yaml", "contents"}[i%3]
 			cmd := Command()
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			args := []string{r, "-o", format}
 			cmd.SetArgs(args)
 			if err := cmd.Execute(); err == nil {
@@ -278,8 +276,6 @@ func TestGetInvalidResources(t *testing.T) {
 			// cycle through output types
 			format := []string{"name", "yaml", "contents"}[i%3]
 			cmd := Command()
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			out := bytes.NewBuffer(make([]byte, 0))
 			cmd.SetErr(out)
 			args := []string{r, "-o", format}
@@ -302,8 +298,6 @@ func TestGetInvalidResources(t *testing.T) {
 	for _, r := range resourcesWithoutContents {
 		t.Run(r+"--output-contents", func(t *testing.T) {
 			cmd := Command()
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			args := []string{r, "-o", "contents"}
 			cmd.SetArgs(args)
 			if err := cmd.Execute(); err == nil {
@@ -327,8 +321,6 @@ func TestGetInvalidResources(t *testing.T) {
 	for _, r := range resources {
 		t.Run(r+"--output-invalid", func(t *testing.T) {
 			cmd := Command()
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			args := []string{r, "-o", "invalid"}
 			cmd.SetArgs(args)
 			if err := cmd.Execute(); err == nil {
@@ -341,8 +333,6 @@ func TestGetInvalidResources(t *testing.T) {
 	for _, o := range outputs {
 		t.Run(o+"--nested", func(t *testing.T) {
 			cmd := Command()
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			args := []string{resources[0], "-o", o, "--nested"}
 			cmd.SetArgs(args)
 			if err := cmd.Execute(); err == nil {
@@ -407,8 +397,6 @@ func TestGetValidResourcesWithFilter(t *testing.T) {
 	for _, r := range valid_resources {
 		t.Run(r, func(t *testing.T) {
 			cmd := Command()
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			args := []string{r, "--filter", "name.contains('a')"}
 			cmd.SetArgs(args)
 			if err := cmd.Execute(); err == nil {
@@ -460,8 +448,6 @@ func TestGetMultipleContentRequestsShouldFail(t *testing.T) {
 	for _, r := range multiple_resources {
 		t.Run(r, func(t *testing.T) {
 			cmd := Command()
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
 			args := []string{r, "--output", "contents"}
 			cmd.SetArgs(args)
 			if err := cmd.Execute(); err == nil {