fix: use kebab case for config file consistently (#2130)

When specifying keys in the configuration file, one must use kebab case.
This commit ensures the instance connection names also use kebab case
and updates the README to accurately describe the feature.

When configuring the Proxy with a config file, one must use matching
flag names and specify any instances with the special key
instance-connection-name, or instance-connection-name-i, where i is the
zero-based index of the instance.

Co-authored-by: Jack Wotherspoon <[email protected]>

README.md

@@ -351,15 +351,16 @@ and YAML. Load the file with the `--config-file` flag:
 ./cloud-sql-proxy --config-file /path/to/config.[toml|json|yaml]
 ```
 
-The configuration file format supports all flags, in lowercase and with an
-underscore instead of a hyphen.
-
-For example:
+The configuration file format supports all flags. The key names should match
+the flag names. For example:
 
 ``` toml
-instance_connection_name = "proj:region:inst"
-auto_iam_authn = true
+# use instance-connection-name-0, instance-connection-name-1, etc.
+# for multiple instances
+instance-connection-name = "proj:region:inst"
+auto-iam-authn = true
 debug = true
+debug-logs = true
 ```
 
 Run `./cloud-sql-proxy --help` for more details.

cmd/config_test.go

@@ -30,41 +30,44 @@ func TestNewCommandWithConfigFile(t *testing.T) {
 		desc   string
 		args   []string
 		setup  func()
-		assert func(t *testing.T, command *Command)
+		assert func(t *testing.T, c *Command)
 	}{
 		{
 			desc:  "toml config file",
-			args:  []string{"--config-file", "testdata/config.toml"},
+			args:  []string{"--config-file", "testdata/config-toml.toml"},
 			setup: func() {},
-			assert: func(t *testing.T, command *Command) {
-				assert(t, 1, len(command.conf.Instances))
-				assert(t, true, command.conf.Debug)
+			assert: func(t *testing.T, c *Command) {
+				assert(t, 1, len(c.conf.Instances))
+				assert(t, true, c.conf.Debug)
+				assert(t, 5555, c.conf.Port)
+				assert(t, true, c.conf.DebugLogs)
+				assert(t, true, c.conf.IAMAuthN)
 			},
 		},
 		{
 			desc:  "yaml config file",
-			args:  []string{"--config-file", "testdata/config.yaml"},
+			args:  []string{"--config-file", "testdata/config-yaml.yaml"},
 			setup: func() {},
-			assert: func(t *testing.T, command *Command) {
-				assert(t, 1, len(command.conf.Instances))
-				assert(t, true, command.conf.Debug)
+			assert: func(t *testing.T, c *Command) {
+				assert(t, 1, len(c.conf.Instances))
+				assert(t, true, c.conf.Debug)
 			},
 		},
 		{
 			desc:  "json config file",
-			args:  []string{"--config-file", "testdata/config.json"},
+			args:  []string{"--config-file", "testdata/config-json.json"},
 			setup: func() {},
-			assert: func(t *testing.T, command *Command) {
-				assert(t, 1, len(command.conf.Instances))
-				assert(t, true, command.conf.Debug)
+			assert: func(t *testing.T, c *Command) {
+				assert(t, 1, len(c.conf.Instances))
+				assert(t, true, c.conf.Debug)
 			},
 		},
 		{
 			desc:  "config file with two instances",
 			args:  []string{"--config-file", "testdata/two-instances.toml"},
 			setup: func() {},
-			assert: func(t *testing.T, command *Command) {
-				assert(t, 2, len(command.conf.Instances))
+			assert: func(t *testing.T, c *Command) {
+				assert(t, 2, len(c.conf.Instances))
 			},
 		},
 		{
@@ -73,8 +76,8 @@ func TestNewCommandWithConfigFile(t *testing.T) {
 			setup: func() {
 				t.Setenv("CSQL_PROXY_INSTANCE_CONNECTION_NAME", "p:r:i")
 			},
-			assert: func(t *testing.T, command *Command) {
-				assert(t, "proj:region:inst", command.conf.Instances[0].Name)
+			assert: func(t *testing.T, c *Command) {
+				assert(t, "proj:region:inst", c.conf.Instances[0].Name)
 			},
 		},
 		{
@@ -83,8 +86,8 @@ func TestNewCommandWithConfigFile(t *testing.T) {
 			setup: func() {
 				t.Setenv("CSQL_PROXY_INSTANCE_CONNECTION_NAME", "p:r:i")
 			},
-			assert: func(t *testing.T, command *Command) {
-				assert(t, "p:r:i", command.conf.Instances[0].Name)
+			assert: func(t *testing.T, c *Command) {
+				assert(t, "p:r:i", c.conf.Instances[0].Name)
 			},
 		},
 		{
@@ -93,8 +96,8 @@ func TestNewCommandWithConfigFile(t *testing.T) {
 			setup: func() {
 				t.Setenv("CSQL_PROXY_DEBUG", "false")
 			},
-			assert: func(t *testing.T, command *Command) {
-				assert(t, true, command.conf.Debug)
+			assert: func(t *testing.T, c *Command) {
+				assert(t, true, c.conf.Debug)
 			},
 		},
 		{
@@ -105,8 +108,8 @@ func TestNewCommandWithConfigFile(t *testing.T) {
 				"--debug",
 			},
 			setup: func() {},
-			assert: func(t *testing.T, command *Command) {
-				assert(t, true, command.conf.Debug)
+			assert: func(t *testing.T, c *Command) {
+				assert(t, true, c.conf.Debug)
 			},
 		},
 		{
@@ -118,8 +121,8 @@ func TestNewCommandWithConfigFile(t *testing.T) {
 			setup: func() {
 				t.Setenv("CSQL_PROXY_DEBUG", "false")
 			},
-			assert: func(t *testing.T, command *Command) {
-				assert(t, false, command.conf.Debug)
+			assert: func(t *testing.T, c *Command) {
+				assert(t, false, c.conf.Debug)
 			},
 		},
 	}

cmd/root.go

@@ -258,20 +258,21 @@ Configuration using a configuration file
 
   The configuration file may look like the following:
 
-      instance_connection_name = "my-project:us-central1:my-server-instance"
+      instance-connection-name = "my-project:us-central1:my-server-instance"
+      auto-iam-authn = true
 
   If multiple instance connection names are used, add the index of the
   instance connection name as a suffix. For example:
 
-      instance_connection_name_0 = "my-project:us-central1:my-db-server"
-      instance_connection_name_1 = "my-other-project:us-central1:my-other-server"
+      instance-connection-name-0 = "my-project:us-central1:my-db-server"
+      instance-connection-name-1 = "my-other-project:us-central1:my-other-server"
 
   The configuration file may also contain the same keys as the environment
   variables and flags. For example:
 
-      auto_iam_authn = true
+      auto-iam-authn = true
       debug = true
-      max_connections = 5
+      max-connections = 5
 
 Localhost Admin Server
 
@@ -532,8 +533,6 @@ func loadConfig(c *Command, args []string, opts []Option) error {
 		return err
 	}
 
-	_ = v.BindPFlags(c.Flags())
-
 	c.Flags().VisitAll(func(f *pflag.Flag) {
 		// Override any unset flags with Viper values to use the pflags
 		// object as a single source of truth.
@@ -661,10 +660,10 @@ func instanceFromEnv(args []string) []string {
 
 func instanceFromConfigFile(v *viper.Viper) []string {
 	var args []string
-	inst := v.GetString("instance_connection_name")
+	inst := v.GetString("instance-connection-name")
 
 	if inst == "" {
-		inst = v.GetString("instance_connection_name_0")
+		inst = v.GetString("instance-connection-name-0")
 		if inst == "" {
 			return nil
 		}
@@ -673,7 +672,7 @@ func instanceFromConfigFile(v *viper.Viper) []string {
 
 	i := 1
 	for {
-		instN := v.GetString(fmt.Sprintf("instance_connection_name_%d", i))
+		instN := v.GetString(fmt.Sprintf("instance-connection-name-%d", i))
 		// if the next instance connection name is not defined, stop checking
 		// environment variables.
 		if instN == "" {

cmd/testdata/config-json.json

@@ -0,0 +1,4 @@
+{
+  "instance-connection-name": "proj:region:inst",
+  "debug": true
+}

cmd/testdata/config-toml.toml

@@ -0,0 +1,5 @@
+instance-connection-name = "proj:region:inst"
+debug = true
+port = "5555"
+debug-logs = true
+auto-iam-authn = true

cmd/testdata/config.yaml → cmd/testdata/config-yaml.yaml

@@ -12,5 +12,5 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-instance_connection_name: "proj:region:inst"
+instance-connection-name: "proj:region:inst"
 debug: true

cmd/testdata/two-instances.toml

@@ -1,2 +1,2 @@
-instance_connection_name_0 = "x:y:z"
-instance_connection_name_1 = "a:b:c"
+instance-connection-name-0 = "x:y:z"
+instance-connection-name-1 = "a:b:c"