Content-Length: 11166 | pFad | http://github.com/coder/coder/pull/19009.patch

thub.com From 8cc543c43e54e4e0b87417de41b552cde4bfe238 Mon Sep 17 00:00:00 2001 From: "blink-so[bot]" <211532188+blink-so[bot]@users.noreply.github.com> Date: Wed, 23 Jul 2025 03:46:09 +0000 Subject: [PATCH 1/3] fix: merge VS Code settings instead of overwriting them This change modifies OverrideVSCodeConfigs to merge Coder's essential settings with existing VS Code settings instead of completely overwriting them. This preserves developer-defined settings from Terraform modules while ensuring Coder's git authentication still works properly. Fixes #19007 Co-authored-by: matifali <10648092+matifali@users.noreply.github.com> --- cli/gitauth/vscode.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/cli/gitauth/vscode.go b/cli/gitauth/vscode.go index fbd22651929b1..2a0d14b51cd73 100644 --- a/cli/gitauth/vscode.go +++ b/cli/gitauth/vscode.go @@ -11,20 +11,27 @@ import ( "golang.org/x/xerrors" ) -// OverrideVSCodeConfigs overwrites a few properties to consume -// GIT_ASKPASS from the host instead of VS Code-specific authentication. +// OverrideVSCodeConfigs merges essential Coder settings with existing VS Code settings +// to ensure GIT_ASKPASS and Git authentication work properly with Coder. func OverrideVSCodeConfigs(fs afero.Fs) error { home, err := os.UserHomeDir() if err != nil { return err } - mutate := func(m map[string]interface{}) { + // Define the essential settings that Coder needs to override + coderSettings := map[string]interface{}{ // This prevents VS Code from overriding GIT_ASKPASS, which // we use to automatically authenticate Git providers. - m["git.useIntegratedAskPass"] = false + "git.useIntegratedAskPass": false, // This prevents VS Code from using it's own GitHub authentication // which would circumvent cloning with Coder-configured providers. - m["github.gitAuthentication"] = false + "github.gitAuthentication": false, + } + mutate := func(m map[string]interface{}) { + // Merge Coder's essential settings with existing settings + for key, value := range coderSettings { + m[key] = value + } } for _, configPath := range []string{ @@ -47,7 +54,8 @@ func OverrideVSCodeConfigs(fs afero.Fs) error { return xerrors.Errorf("stat %q: %w", configPath, err) } - m := map[string]interface{}{} + // Create new settings file with only Coder's essential settings + m := make(map[string]interface{}) mutate(m) data, err := json.MarshalIndent(m, "", "\t") if err != nil { From c89841bf6400d0d03b202a4f067c2c5f882ad050 Mon Sep 17 00:00:00 2001 From: "blink-so[bot]" <211532188+blink-so[bot]@users.noreply.github.com> Date: Wed, 23 Jul 2025 03:54:06 +0000 Subject: [PATCH 2/3] test: add comprehensive tests for VS Code settings merge scenarios Added tests to cover: 1. No existing settings (Create test - already existed) 2. Existing user settings without Coder settings 3. Existing settings including Coder-specific settings 4. Complex JSON structures to ensure valid output without duplication Each test verifies: - Coder settings are properly applied - User settings are preserved - No duplication occurs - Valid JSON output is maintained Co-authored-by: matifali <10648092+matifali@users.noreply.github.com> --- cli/gitauth/vscode_test.go | 100 +++++++++++++++++++++++++++++++++++-- 1 file changed, 95 insertions(+), 5 deletions(-) diff --git a/cli/gitauth/vscode_test.go b/cli/gitauth/vscode_test.go index 7bff62fafdb06..255d6b73b4233 100644 --- a/cli/gitauth/vscode_test.go +++ b/cli/gitauth/vscode_test.go @@ -36,13 +36,17 @@ func TestOverrideVSCodeConfigs(t *testing.T) { require.Equal(t, false, mapping["github.gitAuthentication"]) } }) - t.Run("Append", func(t *testing.T) { + t.Run("MergeWithExistingSettings", func(t *testing.T) { t.Parallel() fs := afero.NewMemMapFs() - mapping := map[string]interface{}{ - "hotdogs": "something", + // Create existing settings with user preferences + existingSettings := map[string]interface{}{ + "workbench.colorTheme": "Dracula", + "editor.fontSize": 14, + "editor.tabSize": 2, + "files.autoSave": "onWindowChange", } - data, err := json.Marshal(mapping) + data, err := json.MarshalIndent(existingSettings, "", "\t") require.NoError(t, err) for _, configPath := range configPaths { err = afero.WriteFile(fs, configPath, data, 0o600) @@ -56,9 +60,95 @@ func TestOverrideVSCodeConfigs(t *testing.T) { mapping := map[string]interface{}{} err = json.Unmarshal(data, &mapping) require.NoError(t, err) + // Verify Coder settings are applied + require.Equal(t, false, mapping["git.useIntegratedAskPass"]) + require.Equal(t, false, mapping["github.gitAuthentication"]) + // Verify user settings are preserved + require.Equal(t, "Dracula", mapping["workbench.colorTheme"]) + require.Equal(t, float64(14), mapping["editor.fontSize"]) + require.Equal(t, float64(2), mapping["editor.tabSize"]) + require.Equal(t, "onWindowChange", mapping["files.autoSave"]) + // Verify no duplication - should have exactly 6 settings + require.Len(t, mapping, 6) + } + }) + + t.Run("MergeWithExistingCoderSettings", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + // Create existing settings that include Coder-specific settings with different values + existingSettings := map[string]interface{}{ + "workbench.colorTheme": "Dark+", + "git.useIntegratedAskPass": true, // This should be overridden to false + "github.gitAuthentication": true, // This should be overridden to false + "editor.wordWrap": "on", + "terminal.integrated.shell.linux": "/bin/bash", + } + data, err := json.MarshalIndent(existingSettings, "", "\t") + require.NoError(t, err) + for _, configPath := range configPaths { + err = afero.WriteFile(fs, configPath, data, 0o600) + require.NoError(t, err) + } + err = gitauth.OverrideVSCodeConfigs(fs) + require.NoError(t, err) + for _, configPath := range configPaths { + data, err := afero.ReadFile(fs, configPath) + require.NoError(t, err) + mapping := map[string]interface{}{} + err = json.Unmarshal(data, &mapping) + require.NoError(t, err) + // Verify Coder settings override existing values + require.Equal(t, false, mapping["git.useIntegratedAskPass"]) + require.Equal(t, false, mapping["github.gitAuthentication"]) + // Verify user settings are preserved + require.Equal(t, "Dark+", mapping["workbench.colorTheme"]) + require.Equal(t, "on", mapping["editor.wordWrap"]) + require.Equal(t, "/bin/bash", mapping["terminal.integrated.shell.linux"]) + // Verify no duplication - should have exactly 5 settings + require.Len(t, mapping, 5) + } + }) + + t.Run("ValidJSONOutput", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + // Test with complex existing settings to ensure valid JSON output + existingSettings := map[string]interface{}{ + "workbench.colorCustomizations": map[string]interface{}{ + "editor.background": "#1e1e1e", + "sideBar.background": "#252526", + }, + "extensions.recommendations": []string{"ms-python.python", "golang.go"}, + "git.useIntegratedAskPass": true, + "editor.rulers": []int{80, 120}, + } + data, err := json.MarshalIndent(existingSettings, "", "\t") + require.NoError(t, err) + for _, configPath := range configPaths { + err = afero.WriteFile(fs, configPath, data, 0o600) + require.NoError(t, err) + } + err = gitauth.OverrideVSCodeConfigs(fs) + require.NoError(t, err) + for _, configPath := range configPaths { + data, err := afero.ReadFile(fs, configPath) + require.NoError(t, err) + // Verify the output is valid JSON + mapping := map[string]interface{}{} + err = json.Unmarshal(data, &mapping) + require.NoError(t, err, "Output should be valid JSON") + // Verify complex structures are preserved + colorCustomizations, ok := mapping["workbench.colorCustomizations"].(map[string]interface{}) + require.True(t, ok, "Complex objects should be preserved") + require.Equal(t, "#1e1e1e", colorCustomizations["editor.background"]) + // Verify arrays are preserved + recommendations, ok := mapping["extensions.recommendations"].([]interface{}) + require.True(t, ok, "Arrays should be preserved") + require.Len(t, recommendations, 2) + // Verify Coder settings are applied require.Equal(t, false, mapping["git.useIntegratedAskPass"]) require.Equal(t, false, mapping["github.gitAuthentication"]) - require.Equal(t, "something", mapping["hotdogs"]) } }) } From 16a5c54661f9a8e2a32ad75e7cbf9432a5160004 Mon Sep 17 00:00:00 2001 From: "blink-so[bot]" <211532188+blink-so[bot]@users.noreply.github.com> Date: Wed, 23 Jul 2025 03:57:19 +0000 Subject: [PATCH 3/3] fix: format test file to align map keys Co-authored-by: matifali <10648092+matifali@users.noreply.github.com> --- cli/gitauth/vscode_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cli/gitauth/vscode_test.go b/cli/gitauth/vscode_test.go index 255d6b73b4233..e448f1f5f63a6 100644 --- a/cli/gitauth/vscode_test.go +++ b/cli/gitauth/vscode_test.go @@ -42,9 +42,9 @@ func TestOverrideVSCodeConfigs(t *testing.T) { // Create existing settings with user preferences existingSettings := map[string]interface{}{ "workbench.colorTheme": "Dracula", - "editor.fontSize": 14, - "editor.tabSize": 2, - "files.autoSave": "onWindowChange", + "editor.fontSize": 14, + "editor.tabSize": 2, + "files.autoSave": "onWindowChange", } data, err := json.MarshalIndent(existingSettings, "", "\t") require.NoError(t, err) @@ -78,10 +78,10 @@ func TestOverrideVSCodeConfigs(t *testing.T) { fs := afero.NewMemMapFs() // Create existing settings that include Coder-specific settings with different values existingSettings := map[string]interface{}{ - "workbench.colorTheme": "Dark+", - "git.useIntegratedAskPass": true, // This should be overridden to false - "github.gitAuthentication": true, // This should be overridden to false - "editor.wordWrap": "on", + "workbench.colorTheme": "Dark+", + "git.useIntegratedAskPass": true, // This should be overridden to false + "github.gitAuthentication": true, // This should be overridden to false + "editor.wordWrap": "on", "terminal.integrated.shell.linux": "/bin/bash", } data, err := json.MarshalIndent(existingSettings, "", "\t") @@ -116,12 +116,12 @@ func TestOverrideVSCodeConfigs(t *testing.T) { // Test with complex existing settings to ensure valid JSON output existingSettings := map[string]interface{}{ "workbench.colorCustomizations": map[string]interface{}{ - "editor.background": "#1e1e1e", + "editor.background": "#1e1e1e", "sideBar.background": "#252526", }, "extensions.recommendations": []string{"ms-python.python", "golang.go"}, - "git.useIntegratedAskPass": true, - "editor.rulers": []int{80, 120}, + "git.useIntegratedAskPass": true, + "editor.rulers": []int{80, 120}, } data, err := json.MarshalIndent(existingSettings, "", "\t") require.NoError(t, err)

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!