From c4a768122089d93708e7914835cb2d9f13fd0c01 Mon Sep 17 00:00:00 2001 From: Joseph Kavanagh Date: Thu, 2 Jul 2026 06:25:49 +0100 Subject: [PATCH] fix(config): stop save corrupting configs not indented with 2 spaces Service.MarshalYAML pre-marshalled itself to bytes at the marshal-default 2-space indent, so on configs using any other indentation, goccy spliced a mixed-indent subtree into the document. Save's line-based cleanup (reorderYAML) then misread the indent levels and stripped all keys on the next level (e.g. `options`, `latest_version`, `deployed_version`, ...), flattening the service block into a config that failed to parse/validate on the next start. Marshal Service via yaml.InterfaceMarshaler instead, returning a value for the enclosing encoder to marshal, so the document's encode options (indentation) also apply to the Service subtree. --- config/decode/yaml.go | 5 +- config/decode/yaml_test.go | 64 +++++++++++++++++ config/save_test.go | 112 +++++++++++++++++++++++++++++ config/yaml_help_test.go | 35 +++++++++ service/types.go | 14 ++-- service/types_test.go | 2 +- web/api/types/argus_test.go | 4 +- web/api/types/help_test.go | 6 +- web/api/v1/help_test.go | 8 +-- web/api/v1/http-api-config_test.go | 56 +++++++-------- 10 files changed, 257 insertions(+), 49 deletions(-) diff --git a/config/decode/yaml.go b/config/decode/yaml.go index 1915c810..09d1803d 100644 --- a/config/decode/yaml.go +++ b/config/decode/yaml.go @@ -22,11 +22,8 @@ import ( ) var ( - // yamlMarshalIndent is the standard indentation to marshal YAML with. - yamlMarshalIndent = 2 // YAMLMarshalOpts are the options for yaml.Marshaler. YAMLMarshalOpts = []yaml.EncodeOption{ - yaml.Indent(yamlMarshalIndent), yaml.IndentSequence(true), yaml.UseLiteralStyleIfMultiline(true), yaml.UseSingleQuote(true), @@ -39,7 +36,7 @@ func NewYAMLEncoder(w io.Writer, spaces int) *yaml.Encoder { opts := append([]yaml.EncodeOption(nil), YAMLMarshalOpts...) // Override indentation. - if spaces > yamlMarshalIndent { + if spaces > yaml.DefaultIndentSpaces { opts = append(opts, yaml.Indent(spaces)) } diff --git a/config/decode/yaml_test.go b/config/decode/yaml_test.go index bc01df4d..2956b850 100644 --- a/config/decode/yaml_test.go +++ b/config/decode/yaml_test.go @@ -88,6 +88,70 @@ func TestNewYAMLEncoder_Indent(t *testing.T) { } } +// testInterfaceMarshaler substitutes itself with another value to marshal +// (yaml.InterfaceMarshaler), like service.Service does. +type testInterfaceMarshaler struct{} + +func (m testInterfaceMarshaler) MarshalYAML() (any, error) { + return testParent{ + A: "hello", + B: testChild{ + C: "hello", + }, + }, nil +} + +func TestNewYAMLEncoder__NestedMarshaler(t *testing.T) { + // GIVEN: an amount of spaces to indent with. + tests := []struct { + spaces int + }{ + {spaces: 2}, + {spaces: 4}, + {spaces: 8}, + } + + for _, tc := range tests { + name := fmt.Sprintf("%d spaces", tc.spaces) + t.Run(name, func(t *testing.T) { + t.Parallel() + + // AND: a NewYAMLEncoder created with this indent count. + var buf bytes.Buffer + enc := NewYAMLEncoder(&buf, tc.spaces) + + // AND: a struct containing a type with a custom yaml.Marshaler. + a := struct { + Service testInterfaceMarshaler `yaml:"service"` + }{} + + prefix := fmt.Sprintf("%s\nNewYAMLEncoder.Encode()", packageName) + + // WHEN: the struct is encoded. + if err := enc.Encode(a); err != nil { + t.Fatalf( + "%s error = %v", + prefix, err, + ) + } + + // THEN: the custom-marshaled subtree indents uniformly with the document. + indent := strings.Repeat(" ", tc.spaces) + got := buf.String() + want := "service:\n" + + indent + "a: hello\n" + + indent + "b:\n" + + indent + indent + "c: hello\n" + if got != want { + t.Errorf( + "%s mismatch\ngot: %q\nwant: %q", + prefix, got, want, + ) + } + }) + } +} + func TestNewYAMLEncoder_IndentSequence(t *testing.T) { // GIVEN: a YAML encoder. var buf bytes.Buffer diff --git a/config/save_test.go b/config/save_test.go index e9570bfd..c65e0f2b 100644 --- a/config/save_test.go +++ b/config/save_test.go @@ -393,6 +393,118 @@ func TestConfig_Save(t *testing.T) { } } +// reindentLines converts the leading whitespace of lines from indent size +// `from` to `to`, preserving any partial offset (e.g. the 2-space alignment +// of sequence continuation lines). +func reindentLines(lines []string, from, to int) []string { + out := make([]string, len(lines)) + for i, line := range lines { + spaces := len(line) - len(strings.TrimLeft(line, " ")) + levels := spaces / from + offset := spaces % from + out[i] = strings.Repeat(" ", levels*to+offset) + line[spaces:] + } + return out +} + +func TestConfig_Save__indentation(t *testing.T) { + // GIVEN: a valid config file and an indentation count to save with. + tests := []struct { + indentation uint8 + }{ + {indentation: 2}, + {indentation: 4}, + {indentation: 6}, + } + + for _, tc := range tests { + name := fmt.Sprintf("%d spaces", tc.indentation) + t.Run(name, func(t *testing.T) { + // t.Parallel() - Cannot run in parallel since we're using stdout. + releaseStdout := test.CaptureLog(t, logx.Default()) + + file := filepath.Join(t.TempDir(), "config.yml") + testYAML_config_indent4(file) + hadData, _ := os.ReadFile(file) + cfg := testLoadBasic(t, file) + hadIndentation := cfg.Settings.Indentation + + // AND: the indentation replaced with the test count. + cfg.Settings.Indentation = tc.indentation + + // WHEN: the config is saved. + saveDone := make(chan struct{}) + go func() { + loadMu.RLock() + cfg.Save() + loadMu.RUnlock() + close(saveDone) + }() + gotExit := gotExitCodeDuringSave(saveDone, false) + + releaseStdout() + prefix := fmt.Sprintf("%s\nConfig.Save()", packageName) + + // THEN: the save succeeds. + if gotExit { + t.Fatalf("%s unexpected Fatal log to ExitCodeChannel", prefix) + } + + // AND: the file uses this indentation with the service subtree intact. + newData, err := os.ReadFile(cfg.File) + if err != nil { + t.Fatalf( + "%s\nfailed opening the file - %s", + packageName, err, + ) + } + gotLines := strings.Split(string(newData), "\n") + wantLines := reindentLines( + strings.Split(string(hadData), "\n"), + int(hadIndentation), int(tc.indentation), + ) + if testErr := test.AssertSlicesEqualFunc( + t, + gotLines, + wantLines, + func(a, b string) bool { return a == b }, + prefix, + "", + ); testErr != nil { + t.Fatal(testErr) + } + + // AND: the saved config still decodes to the same service structure. + var cfg2 Config + if err := cfg2.Decode(newData); err != nil { + t.Fatalf( + "%s saved config failed to decode: %v", + prefix, err, + ) + } + svc := cfg2.Service["awesome"] + if svc == nil { + t.Fatalf("%s service %q lost on save", prefix, "awesome") + } + if svc.LatestVersion == nil { + t.Errorf("%s latest_version lost on save", prefix) + } + if svc.Options.SemanticVersioning == nil { + t.Errorf("%s options.semantic_versioning lost on save", prefix) + } + if svc.Dashboard.Icon == "" { + t.Errorf("%s dashboard.icon lost on save", prefix) + } + if len(svc.Command) != 1 { + t.Errorf( + "%s command lost on save\ngot: %d commands\nwant: 1", + prefix, len(svc.Command), + ) + } + }) + } +} + func TestConfig_Save__encodeError(t *testing.T) { // GIVEN: a failing YAML encode. original := encodeConfigYAML diff --git a/config/yaml_help_test.go b/config/yaml_help_test.go index ff26c7ab..18b6084c 100644 --- a/config/yaml_help_test.go +++ b/config/yaml_help_test.go @@ -275,6 +275,41 @@ func testYAML_config_small(path string) { writeFile(path, data) } +// testYAML_config_indent4 is for `save.go`. +// +// A config indented with 4 spaces rather than the marshal-default 2 +func testYAML_config_indent4(path string) { + data := test.TrimYAML(` + notify: + gotify: + type: gotify + url_fields: + host: example.com + token: super-secret + service: + awesome: + options: + semantic_versioning: false + latest_version: + type: url + url: https://example.com/releases + url_commands: + - type: regex + regex: '[0-9.]+' + notify: + gotify: {} + command: + - - /bin/echo + - hello + dashboard: + icon: https://example.com/icon.webp + tags: + - NEWS + `) + + writeFile(path, data) +} + func testYAML_Ordering_0(path string) { data := test.TrimYAML(` settings: diff --git a/service/types.go b/service/types.go index b48366ea..5c911957 100644 --- a/service/types.go +++ b/service/types.go @@ -95,16 +95,16 @@ type serviceDecode struct { // MarshalJSON implements the json.Marshaler interface. func (s *Service) MarshalJSON() ([]byte, error) { - return s.marshal("json") + return decode.Marshal("json", s.marshalAux()) //nolint:wrapcheck } -// MarshalYAML implements the yaml.Marshaler interface. -func (s *Service) MarshalYAML() ([]byte, error) { - return s.marshal("yaml") +// MarshalYAML implements the yaml.InterfaceMarshaler interface. +func (s *Service) MarshalYAML() (any, error) { + return s.marshalAux(), nil } -// marshal implements the format.Marshaler interface. -func (s *Service) marshal(format string) ([]byte, error) { +// marshalAux converts the Service to its marshal-only helper representation. +func (s *Service) marshalAux() serviceMarshal { aux := serviceMarshal{ Name: s.Name, Comment: s.Comment, @@ -124,7 +124,7 @@ func (s *Service) marshal(format string) ([]byte, error) { aux.WebHook = s.WebHook } - return decode.Marshal(format, aux) //nolint:wrapcheck + return aux } // UnmarshalJSON implements the json.Marshaler interface. diff --git a/service/types_test.go b/service/types_test.go index 38b8cd27..480b07ad 100644 --- a/service/types_test.go +++ b/service/types_test.go @@ -320,7 +320,7 @@ func TestService_Marshal(t *testing.T) { for _, typ := range util.SortedKeys(results) { want := results[typ] - // WHEN: the Service is marshalled to + // WHEN: the Service is marshaled to bytes, err := decode.Marshal(typ, tc.svc) prefix := fmt.Sprintf( diff --git a/web/api/types/argus_test.go b/web/api/types/argus_test.go index a372af9e..713ec9b2 100644 --- a/web/api/types/argus_test.go +++ b/web/api/types/argus_test.go @@ -1942,7 +1942,7 @@ func TestLatestVersion_String(t *testing.T) { { "type": "github", "url": "owner/repo", - "access_token": ` + secretValueMarshalled + `, + "access_token": ` + secretValueMarshaled + `, "allow_invalid_certs": true, "use_prerelease": false, "url_commands": [ @@ -2108,7 +2108,7 @@ func TestLatestVersionRequire_String(t *testing.T) { "image": "test/app", "tag": "{{ version }}", "username": "user", - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` }, "regex_content": ".*", "regex_version": "([0-9.]+)" diff --git a/web/api/types/help_test.go b/web/api/types/help_test.go index 142bbee6..6cec1c9f 100644 --- a/web/api/types/help_test.go +++ b/web/api/types/help_test.go @@ -28,15 +28,15 @@ import ( ) var packageName = "api.types" -var secretValueMarshalled string +var secretValueMarshaled string func TestMain(m *testing.M) { // Log. logtest.InitLog() // Marshal the secret value '' -> '\u003csecret\u003e'. - secretValueMarshalledBytes, _ := decode.Marshal("json", util.SecretValue) - secretValueMarshalled = string(secretValueMarshalledBytes) + secretValueMarshaledBytes, _ := decode.Marshal("json", util.SecretValue) + secretValueMarshaled = string(secretValueMarshaledBytes) // Run other tests. exitCode := m.Run() diff --git a/web/api/v1/help_test.go b/web/api/v1/help_test.go index 63252e7b..7a34544b 100644 --- a/web/api/v1/help_test.go +++ b/web/api/v1/help_test.go @@ -48,8 +48,8 @@ import ( ) var ( - packageName = "api_v1" - secretValueMarshalled string + packageName = "api_v1" + secretValueMarshaled string ) func TestMain(m *testing.M) { @@ -70,8 +70,8 @@ func TestMain(m *testing.M) { cfg.Load(ctx, g, path, &flags) // Marshal the secret value '' -> '\u003csecret\u003e'. - secretValueMarshalledBytes, _ := decode.Marshal("json", util.SecretValue) - secretValueMarshalled = string(secretValueMarshalledBytes) + secretValueMarshaledBytes, _ := decode.Marshal("json", util.SecretValue) + secretValueMarshaled = string(secretValueMarshaledBytes) // Run other tests. exitCode := m.Run() diff --git a/web/api/v1/http-api-config_test.go b/web/api/v1/http-api-config_test.go index b0ebcf8c..55414fbd 100644 --- a/web/api/v1/http-api-config_test.go +++ b/web/api/v1/http-api-config_test.go @@ -133,7 +133,7 @@ func TestHTTP_Config(t *testing.T) { "interval": "1h" }, "latest_version": { - "access_token": ` + secretValueMarshalled + `, + "access_token": ` + secretValueMarshaled + `, "allow_invalid_certs": true, "use_prerelease": false, "require": { @@ -143,18 +143,18 @@ func TestHTTP_Config(t *testing.T) { "registry": { "ghcr": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "hub": { "auth": { "username": "something", - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "quay": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } } } @@ -235,7 +235,7 @@ func TestHTTP_Config(t *testing.T) { "interval": "1h" }, "latest_version": { - "access_token": ` + secretValueMarshalled + `, + "access_token": ` + secretValueMarshaled + `, "allow_invalid_certs": true, "use_prerelease": false, "require": { @@ -245,18 +245,18 @@ func TestHTTP_Config(t *testing.T) { "registry": { "ghcr": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "hub": { "auth": { "username": "something", - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "quay": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } } } @@ -361,7 +361,7 @@ func TestHTTP_Config(t *testing.T) { "interval": "1h" }, "latest_version": { - "access_token": ` + secretValueMarshalled + `, + "access_token": ` + secretValueMarshaled + `, "allow_invalid_certs": true, "use_prerelease": false, "require": { @@ -371,18 +371,18 @@ func TestHTTP_Config(t *testing.T) { "registry": { "ghcr": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "hub": { "auth": { "username": "something", - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "quay": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } } } @@ -408,7 +408,7 @@ func TestHTTP_Config(t *testing.T) { "message": "hello world" }, "url_fields": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` }, "params": { "title": "UPDATE" @@ -448,7 +448,7 @@ func TestHTTP_Config(t *testing.T) { "interval": "1h" }, "latest_version": { - "access_token": ` + secretValueMarshalled + `, + "access_token": ` + secretValueMarshaled + `, "allow_invalid_certs": true, "use_prerelease": false, "require": { @@ -458,18 +458,18 @@ func TestHTTP_Config(t *testing.T) { "registry": { "ghcr": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "hub": { "auth": { "username": "something", - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "quay": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } } } @@ -495,7 +495,7 @@ func TestHTTP_Config(t *testing.T) { "message": "hello world" }, "url_fields": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` }, "params": { "title": "UPDATE" @@ -507,11 +507,11 @@ func TestHTTP_Config(t *testing.T) { "type": "github", "url": "https://example.com", "allow_invalid_certs": true, - "secret": ` + secretValueMarshalled + `, + "secret": ` + secretValueMarshaled + `, "headers": [ { "key": "X-Header", - "value": ` + secretValueMarshalled + ` + "value": ` + secretValueMarshaled + ` } ], "delay": "4s" @@ -565,7 +565,7 @@ func TestHTTP_Config(t *testing.T) { "interval": "1h" }, "latest_version": { - "access_token": ` + secretValueMarshalled + `, + "access_token": ` + secretValueMarshaled + `, "allow_invalid_certs": true, "use_prerelease": false, "require": { @@ -575,18 +575,18 @@ func TestHTTP_Config(t *testing.T) { "registry": { "ghcr": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "hub": { "auth": { "username": "something", - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } }, "quay": { "auth": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` } } } @@ -612,7 +612,7 @@ func TestHTTP_Config(t *testing.T) { "message": "hello world" }, "url_fields": { - "token": ` + secretValueMarshalled + ` + "token": ` + secretValueMarshaled + ` }, "params": { "title": "UPDATE" @@ -624,11 +624,11 @@ func TestHTTP_Config(t *testing.T) { "type": "github", "url": "https://example.com", "allow_invalid_certs": true, - "secret": ` + secretValueMarshalled + `, + "secret": ` + secretValueMarshaled + `, "headers": [ { "key": "X-Header", - "value": ` + secretValueMarshalled + ` + "value": ` + secretValueMarshaled + ` } ], "delay": "4s" @@ -639,7 +639,7 @@ func TestHTTP_Config(t *testing.T) { "latest_version": { "type": "github", "url": "` + test.ArgusGitHubRepo + `", - "access_token": ` + secretValueMarshalled + ` + "access_token": ` + secretValueMarshaled + ` } }, "bravo": {