Fix #948: Skip YAML comments when parsing config variables

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

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)
+			}
+		})
+	}
+}