diff --git a/internal/glance/config.go b/internal/glance/config.go index d4d6af0e8..2557b2d34 100644 --- a/internal/glance/config.go +++ b/internal/glance/config.go @@ -139,10 +139,35 @@ var configVariablePattern = regexp.MustCompile(`(^|.)\$\{(?:([a-zA-Z]+):)?([a-zA // // TODO: don't match against commented out sections, not sure exactly how since // variables can be placed anywhere and used to modify the YAML structure itself +// findYAMLCommentStart returns the index of the YAML comment start (# preceded +// by whitespace or at the start of a line) while respecting quoted strings. +// Returns -1 if there is no comment on the line. +func findYAMLCommentStart(line []byte) int { + inSingle := false + inDouble := false + for i, b := range line { + switch b { + case '\'': + if !inDouble { + inSingle = !inSingle + } + case '"': + if !inSingle { + inDouble = !inDouble + } + case '#': + if !inSingle && !inDouble && (i == 0 || line[i-1] == ' ' || line[i-1] == '\t') { + return i + } + } + } + return -1 +} + func parseConfigVariables(contents []byte) ([]byte, error) { var err error - replaced := configVariablePattern.ReplaceAllFunc(contents, func(match []byte) []byte { + replaceFunc := func(match []byte) []byte { if err != nil { return nil } @@ -177,13 +202,29 @@ func parseConfigVariables(contents []byte) ([]byte, error) { } return []byte(prefix + parsedValue) - }) + } + + // Process line by line so we can skip YAML comments, which should not + // have their variables expanded (fixes #948). + lines := bytes.Split(contents, []byte("\n")) + for i, line := range lines { + commentIdx := findYAMLCommentStart(line) + if commentIdx >= 0 { + // Only apply variable substitution to the part before the comment + lines[i] = append( + configVariablePattern.ReplaceAllFunc(line[:commentIdx], replaceFunc), + line[commentIdx:]..., + ) + } else { + lines[i] = configVariablePattern.ReplaceAllFunc(line, replaceFunc) + } + } if err != nil { return nil, err } - return replaced, nil + return bytes.Join(lines, []byte("\n")), nil } // When the bool return value is true, it indicates that the caller should use the original value diff --git a/internal/glance/config_test.go b/internal/glance/config_test.go new file mode 100644 index 000000000..a91693a71 --- /dev/null +++ b/internal/glance/config_test.go @@ -0,0 +1,91 @@ +package glance + +import ( + "os" + "testing" +) + +func TestParseConfigVariablesIgnoresComments(t *testing.T) { + // Set up an environment variable that should be resolved + os.Setenv("TEST_API_KEY", "my-secret-value") + defer os.Unsetenv("TEST_API_KEY") + + tests := []struct { + name string + input string + expected string + wantErr bool + }{ + { + name: "variable in comment is not expanded", + input: "api-key: ${TEST_API_KEY} # Use secrets with ${secret:my-token}", + expected: "api-key: my-secret-value # Use secrets with ${secret:my-token}", + }, + { + name: "variable before comment is expanded", + input: "key: ${TEST_API_KEY} # this is a comment", + expected: "key: my-secret-value # this is a comment", + }, + { + name: "no comment, variable is expanded", + input: "key: ${TEST_API_KEY}", + expected: "key: my-secret-value", + }, + { + name: "hash inside double quotes is not a comment", + input: `key: "${TEST_API_KEY} # not a comment ${TEST_API_KEY}"`, + expected: `key: "my-secret-value # not a comment my-secret-value"`, + }, + { + name: "hash inside single quotes is not a comment", + input: `key: '${TEST_API_KEY} # not a comment ${TEST_API_KEY}'`, + expected: `key: 'my-secret-value # not a comment my-secret-value'`, + }, + { + name: "comment-only line is not expanded", + input: "# ${secret:some-secret}", + expected: "# ${secret:some-secret}", + }, + { + name: "multiple lines with mixed comments", + input: "key1: ${TEST_API_KEY}\n# comment with ${secret:x}\nkey2: ${TEST_API_KEY} # ${secret:y}", + expected: "key1: my-secret-value\n# comment with ${secret:x}\nkey2: my-secret-value # ${secret:y}", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := parseConfigVariables([]byte(tt.input)) + if (err != nil) != tt.wantErr { + t.Fatalf("parseConfigVariables() error = %v, wantErr %v", err, tt.wantErr) + } + if !tt.wantErr && string(result) != tt.expected { + t.Errorf("parseConfigVariables()\ngot: %q\nwant: %q", string(result), tt.expected) + } + }) + } +} + +func TestFindYAMLCommentStart(t *testing.T) { + tests := []struct { + input string + expected int + }{ + {"no comment here", -1}, + {"# full line comment", 0}, + {"key: value # comment", 11}, + {`key: "value # not comment"`, -1}, + {`key: 'value # not comment'`, -1}, + {`key: "quoted" # comment`, 14}, + {"key: value#no-space-not-comment", -1}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got := findYAMLCommentStart([]byte(tt.input)) + if got != tt.expected { + t.Errorf("findYAMLCommentStart(%q) = %d, want %d", tt.input, got, tt.expected) + } + }) + } +}