improve: add backwards compat for connector access key

This commit adds support for the deprecate env: and file: prefix for secrets in the connector. It should allow users to upgrade without changing their config, and print a warning when they are using a deprecated setting.
infrahq · Sep 27, 2022 · a1f771a · a1f771a
1 parent 34fad88
commit a1f771a
Show file tree

Hide file tree

Showing 3 changed files with 158 additions and 30 deletions.
diff --git a/helm/charts/infra/templates/connector/configmap.yaml b/helm/charts/infra/templates/connector/configmap.yaml
@@ -22,7 +22,7 @@ data:
 {{- if and $accessKey (or (hasPrefix "file:" $accessKey) (hasPrefix "env:" $accessKey)) }}
       accessKey: {{ $accessKey }}
 {{- else }}
-      accessKey: file:/var/run/secrets/infrahq.com/access-key/access-key
+      accessKey: /var/run/secrets/infrahq.com/access-key/access-key
 {{- end }}
 
 {{- with .Values.connector.config.serverTrustedCertificate }}

diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go
@@ -295,9 +295,25 @@ func newConnectorCmd() *cobra.Command {
 				return err
 			}
 
+			// backwards compat for old access key values with a prefix
+			switch {
+			case strings.HasPrefix(options.Server.AccessKey.String(), "file:"):
+				filename := strings.TrimPrefix(options.Server.AccessKey.String(), "file:")
+				if err := options.Server.AccessKey.Set(filename); err != nil {
+					return err
+				}
+				logging.L.Warn().Msg("accessKey with 'file:' prefix is deprecated. Use the filename without the file: prefix instead.")
+			case strings.HasPrefix(options.Server.AccessKey.String(), "env:"):
+				key := strings.TrimPrefix(options.Server.AccessKey.String(), "env:")
+				options.Server.AccessKey = types.StringOrFile(os.Getenv(key))
+				logging.L.Warn().Msg("accessKey with 'env:' prefix is deprecated. Use the INFRA_ACCESS_KEY env var instead.")
+			}
+
 			// Also accept the same env var as the CLI for setting the access key
 			if accessKey, ok := os.LookupEnv("INFRA_ACCESS_KEY"); ok {
-				options.Server.AccessKey = types.StringOrFile(accessKey)
+				if err := options.Server.AccessKey.Set(accessKey); err != nil {
+					return err
+				}
 			}
 			return runConnector(cmd.Context(), options)
 		},

diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
@@ -359,14 +359,40 @@ func TestInvalidSessions(t *testing.T) {
 	})
 }
 
-func TestConnectorCmd(t *testing.T) {
-	var actual connector.Options
-	patchRunConnector(t, func(ctx context.Context, options connector.Options) error {
-		actual = options
-		return nil
-	})
+func TestConnectorCmd_LoadConfig(t *testing.T) {
+	type testCase struct {
+		name     string
+		config   string
+		setup    func(t *testing.T)
+		expected func() connector.Options
+	}
+
+	run := func(t *testing.T, tc testCase) {
+		var actual connector.Options
+		patchRunConnector(t, func(ctx context.Context, options connector.Options) error {
+			actual = options
+			return nil
+		})
+
+		if tc.setup != nil {
+			tc.setup(t)
+		}
+
+		dir := fs.NewDir(t, t.Name(), fs.WithFile("config.yaml", tc.config))
+
+		ctx := context.Background()
+		err := Run(ctx, "connector", "-f", dir.Join("config.yaml"))
+		assert.NilError(t, err)
+		assert.DeepEqual(t, actual, tc.expected())
+	}
 
-	content := `
+	keyDir := fs.NewDir(t, t.Name(), fs.WithFile("accesskeyfile", "the-access-key"))
+	filename := keyDir.Join("accesskeyfile")
+
+	testCases := []testCase{
+		{
+			name: "full config",
+			config: `
 server:
   url: the-server
   accessKey: /var/run/secrets/key
@@ -375,30 +401,116 @@ server:
 name: the-name
 caCert: /path/to/cert
 caKey: /path/to/key
-`
-
-	dir := fs.NewDir(t, t.Name(), fs.WithFile("config.yaml", content))
-
-	ctx := context.Background()
-	err := Run(ctx, "connector", "-f", dir.Join("config.yaml"))
-	assert.NilError(t, err)
-
-	expected := connector.Options{
-		Name: "the-name",
-		Addr: connector.ListenerOptions{
-			HTTPS:   ":443",
-			Metrics: ":9090",
+addr:
+  https: localhost:414
+  metrics: 127.0.0.1:8000
+`,
+			expected: func() connector.Options {
+				return connector.Options{
+					Name: "the-name",
+					Addr: connector.ListenerOptions{
+						HTTPS:   "localhost:414",
+						Metrics: "127.0.0.1:8000",
+					},
+					Server: connector.ServerOptions{
+						URL:                "the-server",
+						AccessKey:          "/var/run/secrets/key",
+						SkipTLSVerify:      true,
+						TrustedCertificate: "ca.pem",
+					},
+					CACert: "/path/to/cert",
+					CAKey:  "/path/to/key",
+				}
+			},
+		},
+		{
+			name:   "access key with file: prefix (deprecated)",
+			config: fmt.Sprintf("server:\n  accessKey: file:%v\n", filename),
+			expected: func() connector.Options {
+				expected := defaultConnectorOptions()
+				expected.Server.AccessKey = "the-access-key"
+				return expected
+			},
+		},
+		{
+			name:   "access key from file",
+			config: fmt.Sprintf("server:\n  accessKey: %v\n", filename),
+			expected: func() connector.Options {
+				expected := defaultConnectorOptions()
+				expected.Server.AccessKey = "the-access-key"
+				return expected
+			},
+		},
+		{
+			name: "access key with env: prefix (deprecated)",
+			setup: func(t *testing.T) {
+				t.Setenv("CUSTOM_ENV_VAR", "the-key-from-env")
+			},
+			config: `
+server:
+  accessKey: env:CUSTOM_ENV_VAR
+`,
+			expected: func() connector.Options {
+				expected := defaultConnectorOptions()
+				expected.Server.AccessKey = "the-key-from-env"
+				return expected
+			},
+		},
+		{
+			name: "access key from INFRA_ACCESS_KEY",
+			setup: func(t *testing.T) {
+				t.Setenv("INFRA_ACCESS_KEY", "the-key-from-env")
+			},
+			config: `{}`,
+			expected: func() connector.Options {
+				expected := defaultConnectorOptions()
+				expected.Server.AccessKey = "the-key-from-env"
+				return expected
+			},
 		},
-		Server: connector.ServerOptions{
-			URL:                "the-server",
-			AccessKey:          "/var/run/secrets/key",
-			SkipTLSVerify:      true,
-			TrustedCertificate: "ca.pem",
+		{
+			name: "access key from INFRA_ACCESS_KEY points at a file",
+			setup: func(t *testing.T) {
+				t.Setenv("INFRA_ACCESS_KEY", filename)
+			},
+			config: `{}`,
+			expected: func() connector.Options {
+				expected := defaultConnectorOptions()
+				expected.Server.AccessKey = "the-access-key"
+				return expected
+			},
 		},
-		CACert: "/path/to/cert",
-		CAKey:  "/path/to/key",
+		{
+			name:   "access key from INFRA_CONNECTOR_SERVER_ACCESS_KEY",
+			config: `{}`,
+			setup: func(t *testing.T) {
+				t.Setenv("INFRA_CONNECTOR_SERVER_ACCESS_KEY", "the-key-from-env")
+			},
+			expected: func() connector.Options {
+				expected := defaultConnectorOptions()
+				expected.Server.AccessKey = "the-key-from-env"
+				return expected
+			},
+		},
+		{
+			name: "access key literal from file",
+			config: `
+server:
+  accessKey: the-literal-key
+`,
+			expected: func() connector.Options {
+				expected := defaultConnectorOptions()
+				expected.Server.AccessKey = "the-literal-key"
+				return expected
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			run(t, tc)
+		})
 	}
-	assert.DeepEqual(t, actual, expected)
 }
 
 func patchRunConnector(t *testing.T, fn func(context.Context, connector.Options) error) {