From 4da088f587eada7ace7c9baac030d9f7f0d36218 Mon Sep 17 00:00:00 2001 From: Ewuji John Date: Sat, 19 Sep 2026 17:08:01 +0100 Subject: [PATCH 1/5] Fix 7394 Signed-off-by: Ewuji John --- .gitignore | 2 + .../configuration-reference.md | 4 +- pkg/app/piped/eventwatcher/eventwatcher.go | 8 +-- .../piped/eventwatcher/eventwatcher_test.go | 65 ++++++++++++++++++ pkg/app/pipedv1/eventwatcher/eventwatcher.go | 7 +- .../pipedv1/eventwatcher/eventwatcher_test.go | 65 ++++++++++++++++++ pkg/configv1/application.go | 6 ++ pkg/configv1/application_test.go | 23 +++++++ pkg/configv1/event_watcher.go | 67 +++++++++++++------ pkg/configv1/event_watcher_test.go | 52 ++++++++++++++ 10 files changed, 263 insertions(+), 36 deletions(-) diff --git a/.gitignore b/.gitignore index f4e4acaa43..14945ce1e4 100644 --- a/.gitignore +++ b/.gitignore @@ -67,3 +67,5 @@ gomock_reflect_*/ # hack hack/oidc/realm.local.json + +WORKABLE.md \ No newline at end of file diff --git a/docs/content/en/docs-v1.0.x/user-guide/managing-application/configuration-reference.md b/docs/content/en/docs-v1.0.x/user-guide/managing-application/configuration-reference.md index e315c77757..b633ec013f 100644 --- a/docs/content/en/docs-v1.0.x/user-guide/managing-application/configuration-reference.md +++ b/docs/content/en/docs-v1.0.x/user-guide/managing-application/configuration-reference.md @@ -214,14 +214,12 @@ At least one of `name`, `kind`, or `labels` must be set. ## EventWatcherReplacement -Only one of `yamlField`, `jsonField`, `HCLField`, or `regex` may be set alongside `file`. +Only one of `yamlField` or `regex` may be set alongside `file`. | Field | Type | Description | Required | | --- | --- | --- | --- | | `file` | string | Path to the file to update. | Yes | | `yamlField` | string | YAML path to the field to update. Must start with `$`. e.g. `$.foo.bar[0].baz`. | No | -| `jsonField` | string | JSON path to the field to update. | No | -| `HCLField` | string | HCL path to the field to update. | No | | `regex` | string | Regular expression specifying what to replace. Only the first capturing group `()` is replaced. e.g. `host.xz/foo/bar:(v[0-9].[0-9].[0-9])`. | No | ## DriftDetection diff --git a/pkg/app/piped/eventwatcher/eventwatcher.go b/pkg/app/piped/eventwatcher/eventwatcher.go index f29b244e20..be931b5cc4 100644 --- a/pkg/app/piped/eventwatcher/eventwatcher.go +++ b/pkg/app/piped/eventwatcher/eventwatcher.go @@ -689,9 +689,9 @@ func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eve case r.YAMLField != "": newContent, upToDate, err = modifyYAML(path, r.YAMLField, latestEvent.Data) case r.JSONField != "": - // TODO: Empower Event watcher to parse JSON format + return "", fmt.Errorf("jsonField replacements are not supported") case r.HCLField != "": - // TODO: Empower Event watcher to parse HCL format + return "", fmt.Errorf("HCLField replacements are not supported") case r.Regex != "": newContent, upToDate, err = modifyText(path, r.Regex, latestEvent.Data) } @@ -703,10 +703,6 @@ func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eve continue } - if err := os.WriteFile(path, newContent, os.ModePerm); err != nil { - w.logger.Error("failed to write file", zap.Error(err)) - return "", err - } changes[filePath] = newContent } if len(changes) == 0 { diff --git a/pkg/app/piped/eventwatcher/eventwatcher_test.go b/pkg/app/piped/eventwatcher/eventwatcher_test.go index a8cb3be013..b97a186a72 100644 --- a/pkg/app/piped/eventwatcher/eventwatcher_test.go +++ b/pkg/app/piped/eventwatcher/eventwatcher_test.go @@ -15,9 +15,17 @@ package eventwatcher import ( + "context" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" + + config "github.com/pipe-cd/pipecd/pkg/config" + "github.com/pipe-cd/pipecd/pkg/git/gittest" + "github.com/pipe-cd/pipecd/pkg/model" ) func TestConvertStr(t *testing.T) { @@ -81,6 +89,63 @@ func TestConvertStr(t *testing.T) { } } +func TestCommitFilesDoesNotTruncateUnsupportedReplacementFiles(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + replacement config.EventWatcherReplacement + wantError string + }{ + { + name: "JSON field", + replacement: config.EventWatcherReplacement{ + File: "version.json", + JSONField: "$.image", + }, + wantError: "jsonField replacements are not supported", + }, + { + name: "HCL field", + replacement: config.EventWatcherReplacement{ + File: "version.hcl", + HCLField: "image", + }, + wantError: "HCLField replacements are not supported", + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, tc.replacement.File) + original := []byte("must not be truncated\n") + assert.NoError(t, os.WriteFile(path, original, 0o600)) + + ctrl := gomock.NewController(t) + repo := gittest.NewMockRepo(ctrl) + repo.EXPECT().GetPath().Return(dir) + + w := &watcher{} + _, err := w.commitFiles( + context.Background(), + &model.Event{Data: "new-value"}, + "image-update", + "", + "", + []config.EventWatcherReplacement{tc.replacement}, + repo, + false, + ) + assert.EqualError(t, err, tc.wantError) + + actual, readErr := os.ReadFile(path) + assert.NoError(t, readErr) + assert.Equal(t, original, actual) + }) + } +} + func TestModifyYAML(t *testing.T) { t.Parallel() diff --git a/pkg/app/pipedv1/eventwatcher/eventwatcher.go b/pkg/app/pipedv1/eventwatcher/eventwatcher.go index 96adf31450..65433280aa 100644 --- a/pkg/app/pipedv1/eventwatcher/eventwatcher.go +++ b/pkg/app/pipedv1/eventwatcher/eventwatcher.go @@ -625,9 +625,9 @@ func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eve case r.YAMLField != "": newContent, upToDate, err = modifyYAML(path, r.YAMLField, latestEvent.Data) case r.JSONField != "": - // TODO: Empower Event watcher to parse JSON format + return "", fmt.Errorf("jsonField replacements are not supported") case r.HCLField != "": - // TODO: Empower Event watcher to parse HCL format + return "", fmt.Errorf("HCLField replacements are not supported") case r.Regex != "": newContent, upToDate, err = modifyText(path, r.Regex, latestEvent.Data) } @@ -638,9 +638,6 @@ func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eve continue } - if err := os.WriteFile(path, newContent, os.ModePerm); err != nil { - return "", fmt.Errorf("failed to write file: %w", err) - } changes[filePath] = newContent } if len(changes) == 0 { diff --git a/pkg/app/pipedv1/eventwatcher/eventwatcher_test.go b/pkg/app/pipedv1/eventwatcher/eventwatcher_test.go index a8cb3be013..3d9dd021f5 100644 --- a/pkg/app/pipedv1/eventwatcher/eventwatcher_test.go +++ b/pkg/app/pipedv1/eventwatcher/eventwatcher_test.go @@ -15,9 +15,17 @@ package eventwatcher import ( + "context" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" + + config "github.com/pipe-cd/pipecd/pkg/configv1" + "github.com/pipe-cd/pipecd/pkg/git/gittest" + "github.com/pipe-cd/pipecd/pkg/model" ) func TestConvertStr(t *testing.T) { @@ -81,6 +89,63 @@ func TestConvertStr(t *testing.T) { } } +func TestCommitFilesDoesNotTruncateUnsupportedReplacementFiles(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + replacement config.EventWatcherReplacement + wantError string + }{ + { + name: "JSON field", + replacement: config.EventWatcherReplacement{ + File: "version.json", + JSONField: "$.image", + }, + wantError: "jsonField replacements are not supported", + }, + { + name: "HCL field", + replacement: config.EventWatcherReplacement{ + File: "version.hcl", + HCLField: "image", + }, + wantError: "HCLField replacements are not supported", + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, tc.replacement.File) + original := []byte("must not be truncated\n") + assert.NoError(t, os.WriteFile(path, original, 0o600)) + + ctrl := gomock.NewController(t) + repo := gittest.NewMockRepo(ctrl) + repo.EXPECT().GetPath().Return(dir) + + w := &watcher{} + _, err := w.commitFiles( + context.Background(), + &model.Event{Data: "new-value"}, + "image-update", + "", + "", + []config.EventWatcherReplacement{tc.replacement}, + repo, + false, + ) + assert.EqualError(t, err, tc.wantError) + + actual, readErr := os.ReadFile(path) + assert.NoError(t, readErr) + assert.Equal(t, original, actual) + }) + } +} + func TestModifyYAML(t *testing.T) { t.Parallel() diff --git a/pkg/configv1/application.go b/pkg/configv1/application.go index 0eae80e8d6..3825c94183 100644 --- a/pkg/configv1/application.go +++ b/pkg/configv1/application.go @@ -157,6 +157,12 @@ func (s *GenericApplicationSpec) Validate() error { } } + for _, ew := range s.EventWatcher { + if err := ew.Validate(); err != nil { + return err + } + } + return nil } diff --git a/pkg/configv1/application_test.go b/pkg/configv1/application_test.go index f8a0781725..097cd89633 100644 --- a/pkg/configv1/application_test.go +++ b/pkg/configv1/application_test.go @@ -545,6 +545,29 @@ func TestGenericPostSyncConfiguration(t *testing.T) { } } +func TestGenericApplicationSpecValidatesEventWatcher(t *testing.T) { + t.Parallel() + + spec := GenericApplicationSpec{ + EventWatcher: []EventWatcherConfig{ + { + Handler: EventWatcherHandler{ + Config: EventWatcherHandlerConfig{ + Replacements: []EventWatcherReplacement{ + { + File: "version.json", + JSONField: "$.image", + }, + }, + }, + }, + }, + }, + } + + assert.Error(t, spec.Validate()) +} + func TestGetStageConfigByte(t *testing.T) { testcases := []struct { name string diff --git a/pkg/configv1/event_watcher.go b/pkg/configv1/event_watcher.go index 3bbcad5b04..366267e5dc 100644 --- a/pkg/configv1/event_watcher.go +++ b/pkg/configv1/event_watcher.go @@ -70,6 +70,22 @@ type EventWatcherHandlerConfig struct { Replacements []EventWatcherReplacement `json:"replacements"` } +func (c EventWatcherConfig) Validate() error { + if err := c.Handler.Config.Validate(); err != nil { + return fmt.Errorf("invalid event watcher handler config: %w", err) + } + return nil +} + +func (c EventWatcherHandlerConfig) Validate() error { + for _, r := range c.Replacements { + if err := r.Validate(); err != nil { + return err + } + } + return nil +} + type EventWatcherReplacement struct { // The path to the file to be updated. File string `json:"file"` @@ -88,6 +104,33 @@ type EventWatcherReplacement struct { Regex string `json:"regex"` } +func (r EventWatcherReplacement) Validate() error { + if r.File == "" { + return fmt.Errorf("replacement has no file name") + } + if r.JSONField != "" { + return fmt.Errorf("replacement has an unsupported jsonField") + } + if r.HCLField != "" { + return fmt.Errorf("replacement has an unsupported HCLField") + } + + count := 0 + if r.YAMLField != "" { + count++ + } + if r.Regex != "" { + count++ + } + if count == 0 { + return fmt.Errorf("replacement has no field") + } + if count > 1 { + return fmt.Errorf("replacement has multiple fields") + } + return nil +} + // EventWatcherHandlerType represents the type of an event watcher handler. type EventWatcherHandlerType string @@ -202,28 +245,8 @@ func (e *EventWatcherEvent) Validate() error { return fmt.Errorf("there must be at least one replacement to an event") } for _, r := range e.Replacements { - if r.File == "" { - return fmt.Errorf("event %q has a replacement with no file name", e.Name) - } - - var count int - if r.YAMLField != "" { - count++ - } - if r.JSONField != "" { - count++ - } - if r.HCLField != "" { - count++ - } - if r.Regex != "" { - count++ - } - if count == 0 { - return fmt.Errorf("event %q has a replacement with no field", e.Name) - } - if count > 2 { - return fmt.Errorf("event %q has multiple fields", e.Name) + if err := r.Validate(); err != nil { + return fmt.Errorf("event %q: %w", e.Name, err) } } return nil diff --git a/pkg/configv1/event_watcher_test.go b/pkg/configv1/event_watcher_test.go index 20629cfc00..0c82adf752 100644 --- a/pkg/configv1/event_watcher_test.go +++ b/pkg/configv1/event_watcher_test.go @@ -166,6 +166,58 @@ func TestEventWatcherValidate(t *testing.T) { }, wantErr: true, }, + { + name: "json field given", + eventWatcherSpec: EventWatcherSpec{ + Events: []EventWatcherEvent{ + { + Name: "event-a", + Replacements: []EventWatcherReplacement{ + { + File: "file.json", + JSONField: "$.value", + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "HCL field given", + eventWatcherSpec: EventWatcherSpec{ + Events: []EventWatcherEvent{ + { + Name: "event-a", + Replacements: []EventWatcherReplacement{ + { + File: "file.hcl", + HCLField: "value", + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "both supported fields given", + eventWatcherSpec: EventWatcherSpec{ + Events: []EventWatcherEvent{ + { + Name: "event-a", + Replacements: []EventWatcherReplacement{ + { + File: "file", + YAMLField: "$.value", + Regex: "(value)", + }, + }, + }, + }, + }, + wantErr: true, + }, { name: "valid config given", eventWatcherSpec: EventWatcherSpec{ From 50d421bbdad9da109c91c0433be2f9a5a1c21252 Mon Sep 17 00:00:00 2001 From: Ewuji John Date: Tue, 22 Sep 2026 18:13:48 +0100 Subject: [PATCH 2/5] Fix event watcher replacement handling Signed-off-by: Ewuji John --- pkg/app/piped/eventwatcher/eventwatcher.go | 12 +++++++++++- pkg/app/pipedv1/eventwatcher/eventwatcher.go | 11 ++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pkg/app/piped/eventwatcher/eventwatcher.go b/pkg/app/piped/eventwatcher/eventwatcher.go index be931b5cc4..a12f8f8c42 100644 --- a/pkg/app/piped/eventwatcher/eventwatcher.go +++ b/pkg/app/piped/eventwatcher/eventwatcher.go @@ -350,6 +350,7 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve gitUpdateEvent = false branchHandledEvents = make(map[string][]*pipedservice.ReportEventStatusesRequest_Event, len(eventCfgs)) gitNoChangeEvents = make([]*pipedservice.ReportEventStatusesRequest_Event, 0) + failedEvents = make([]*pipedservice.ReportEventStatusesRequest_Event, 0) ) for _, e := range eventCfgs { for _, cfg := range e.Configs { @@ -411,7 +412,7 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve Status: model.EventStatus_EVENT_FAILURE, StatusDescription: fmt.Sprintf("Failed to change files: %v", err), } - branchHandledEvents[branchName] = append(branchHandledEvents[branchName], handledEvent) + failedEvents = append(failedEvents, handledEvent) continue } @@ -446,6 +447,11 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve } w.logger.Info(fmt.Sprintf("successfully made %d events OUTDATED", len(outDatedEvents))) } + if len(failedEvents) > 0 { + if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: failedEvents}); err != nil { + w.logger.Error("failed to report event statuses", zap.Error(err)) + } + } if !gitUpdateEvent { return nil @@ -702,6 +708,10 @@ func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eve if upToDate { continue } + if err := os.WriteFile(path, newContent, os.ModePerm); err != nil { + w.logger.Error("failed to write file", zap.Error(err)) + return "", err + } changes[filePath] = newContent } diff --git a/pkg/app/pipedv1/eventwatcher/eventwatcher.go b/pkg/app/pipedv1/eventwatcher/eventwatcher.go index 65433280aa..78669f2f6a 100644 --- a/pkg/app/pipedv1/eventwatcher/eventwatcher.go +++ b/pkg/app/pipedv1/eventwatcher/eventwatcher.go @@ -339,6 +339,7 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve outDatedDuration = time.Hour gitUpdateEvent = false branchHandledEvents = make(map[string][]*pipedservice.ReportEventStatusesRequest_Event, len(eventCfgs)) + failedEvents = make([]*pipedservice.ReportEventStatusesRequest_Event, 0) ) for _, e := range eventCfgs { for _, cfg := range e.Configs { @@ -398,7 +399,7 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve Status: model.EventStatus_EVENT_FAILURE, StatusDescription: fmt.Sprintf("Failed to change files: %v", err), } - branchHandledEvents[branchName] = append(branchHandledEvents[branchName], handledEvent) + failedEvents = append(failedEvents, handledEvent) continue } handledEvent := &pipedservice.ReportEventStatusesRequest_Event{ @@ -426,6 +427,11 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve } w.logger.Info(fmt.Sprintf("successfully made %d events OUTDATED", len(outDatedEvents))) } + if len(failedEvents) > 0 { + if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: failedEvents}); err != nil { + w.logger.Error("failed to report event statuses", zap.Error(err)) + } + } if !gitUpdateEvent { return nil @@ -637,6 +643,9 @@ func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eve if upToDate { continue } + if err := os.WriteFile(path, newContent, os.ModePerm); err != nil { + return "", fmt.Errorf("failed to write file: %w", err) + } changes[filePath] = newContent } From 9474ff97bf8dc7f81bcdae287834a416a545912b Mon Sep 17 00:00:00 2001 From: Ewuji John Date: Tue, 22 Sep 2026 18:28:59 +0100 Subject: [PATCH 3/5] Harden event watcher replacement handling Signed-off-by: Ewuji John --- pkg/app/piped/eventwatcher/eventwatcher.go | 8 ++- .../piped/eventwatcher/eventwatcher_test.go | 6 +- pkg/app/pipedv1/eventwatcher/eventwatcher.go | 8 ++- .../pipedv1/eventwatcher/eventwatcher_test.go | 6 +- pkg/config/application.go | 6 ++ pkg/config/event_watcher.go | 60 ++++++++++++------- 6 files changed, 62 insertions(+), 32 deletions(-) diff --git a/pkg/app/piped/eventwatcher/eventwatcher.go b/pkg/app/piped/eventwatcher/eventwatcher.go index a12f8f8c42..f24c674d4f 100644 --- a/pkg/app/piped/eventwatcher/eventwatcher.go +++ b/pkg/app/piped/eventwatcher/eventwatcher.go @@ -449,7 +449,7 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve } if len(failedEvents) > 0 { if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: failedEvents}); err != nil { - w.logger.Error("failed to report event statuses", zap.Error(err)) + return fmt.Errorf("failed to report event statuses: %w", err) } } @@ -677,6 +677,12 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string // commitFiles commits changes if the data in Git is different from the latest event. // If there are no changes to commit, it returns errNoChanges. func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eventName, commitMsg, gitPath string, replacements []config.EventWatcherReplacement, repo git.Repo, newBranch bool) (string, error) { + for _, r := range replacements { + if err := r.Validate(); err != nil { + return "", err + } + } + // Determine files to be changed by comparing with the latest event. changes := make(map[string][]byte, len(replacements)) for _, r := range replacements { diff --git a/pkg/app/piped/eventwatcher/eventwatcher_test.go b/pkg/app/piped/eventwatcher/eventwatcher_test.go index b97a186a72..3c8ab52117 100644 --- a/pkg/app/piped/eventwatcher/eventwatcher_test.go +++ b/pkg/app/piped/eventwatcher/eventwatcher_test.go @@ -103,7 +103,7 @@ func TestCommitFilesDoesNotTruncateUnsupportedReplacementFiles(t *testing.T) { File: "version.json", JSONField: "$.image", }, - wantError: "jsonField replacements are not supported", + wantError: "replacement has an unsupported jsonField", }, { name: "HCL field", @@ -111,7 +111,7 @@ func TestCommitFilesDoesNotTruncateUnsupportedReplacementFiles(t *testing.T) { File: "version.hcl", HCLField: "image", }, - wantError: "HCLField replacements are not supported", + wantError: "replacement has an unsupported HCLField", }, } { t.Run(tc.name, func(t *testing.T) { @@ -124,8 +124,6 @@ func TestCommitFilesDoesNotTruncateUnsupportedReplacementFiles(t *testing.T) { ctrl := gomock.NewController(t) repo := gittest.NewMockRepo(ctrl) - repo.EXPECT().GetPath().Return(dir) - w := &watcher{} _, err := w.commitFiles( context.Background(), diff --git a/pkg/app/pipedv1/eventwatcher/eventwatcher.go b/pkg/app/pipedv1/eventwatcher/eventwatcher.go index 78669f2f6a..f3314a7bfd 100644 --- a/pkg/app/pipedv1/eventwatcher/eventwatcher.go +++ b/pkg/app/pipedv1/eventwatcher/eventwatcher.go @@ -429,7 +429,7 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve } if len(failedEvents) > 0 { if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: failedEvents}); err != nil { - w.logger.Error("failed to report event statuses", zap.Error(err)) + return fmt.Errorf("failed to report event statuses: %w", err) } } @@ -613,6 +613,12 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string // commitFiles commits changes if the data in Git is different from the latest event. func (w *watcher) commitFiles(ctx context.Context, latestEvent *model.Event, eventName, commitMsg, gitPath string, replacements []config.EventWatcherReplacement, repo git.Repo, newBranch bool) (string, error) { + for _, r := range replacements { + if err := r.Validate(); err != nil { + return "", err + } + } + // Determine files to be changed by comparing with the latest event. changes := make(map[string][]byte, len(replacements)) for _, r := range replacements { diff --git a/pkg/app/pipedv1/eventwatcher/eventwatcher_test.go b/pkg/app/pipedv1/eventwatcher/eventwatcher_test.go index 3d9dd021f5..0e39ac6fe8 100644 --- a/pkg/app/pipedv1/eventwatcher/eventwatcher_test.go +++ b/pkg/app/pipedv1/eventwatcher/eventwatcher_test.go @@ -103,7 +103,7 @@ func TestCommitFilesDoesNotTruncateUnsupportedReplacementFiles(t *testing.T) { File: "version.json", JSONField: "$.image", }, - wantError: "jsonField replacements are not supported", + wantError: "replacement has an unsupported jsonField", }, { name: "HCL field", @@ -111,7 +111,7 @@ func TestCommitFilesDoesNotTruncateUnsupportedReplacementFiles(t *testing.T) { File: "version.hcl", HCLField: "image", }, - wantError: "HCLField replacements are not supported", + wantError: "replacement has an unsupported HCLField", }, } { t.Run(tc.name, func(t *testing.T) { @@ -124,8 +124,6 @@ func TestCommitFilesDoesNotTruncateUnsupportedReplacementFiles(t *testing.T) { ctrl := gomock.NewController(t) repo := gittest.NewMockRepo(ctrl) - repo.EXPECT().GetPath().Return(dir) - w := &watcher{} _, err := w.commitFiles( context.Background(), diff --git a/pkg/config/application.go b/pkg/config/application.go index 7c8d2b222d..36c29b5159 100644 --- a/pkg/config/application.go +++ b/pkg/config/application.go @@ -177,6 +177,12 @@ func (s *GenericApplicationSpec) Validate() error { } } + for _, ew := range s.EventWatcher { + if err := ew.Handler.Config.Validate(); err != nil { + return fmt.Errorf("invalid event watcher handler config: %w", err) + } + } + return nil } diff --git a/pkg/config/event_watcher.go b/pkg/config/event_watcher.go index 282df6d3ed..6d8a2589bc 100644 --- a/pkg/config/event_watcher.go +++ b/pkg/config/event_watcher.go @@ -70,6 +70,15 @@ type EventWatcherHandlerConfig struct { Replacements []EventWatcherReplacement `json:"replacements"` } +func (c EventWatcherHandlerConfig) Validate() error { + for _, r := range c.Replacements { + if err := r.Validate(); err != nil { + return err + } + } + return nil +} + type EventWatcherReplacement struct { // The path to the file to be updated. File string `json:"file"` @@ -88,6 +97,33 @@ type EventWatcherReplacement struct { Regex string `json:"regex"` } +func (r EventWatcherReplacement) Validate() error { + if r.File == "" { + return fmt.Errorf("replacement has no file name") + } + if r.JSONField != "" { + return fmt.Errorf("replacement has an unsupported jsonField") + } + if r.HCLField != "" { + return fmt.Errorf("replacement has an unsupported HCLField") + } + + count := 0 + if r.YAMLField != "" { + count++ + } + if r.Regex != "" { + count++ + } + if count == 0 { + return fmt.Errorf("replacement has no field") + } + if count > 1 { + return fmt.Errorf("replacement has multiple fields") + } + return nil +} + // EventWatcherHandlerType represents the type of an event watcher handler. type EventWatcherHandlerType string @@ -202,28 +238,8 @@ func (e *EventWatcherEvent) Validate() error { return fmt.Errorf("there must be at least one replacement to an event") } for _, r := range e.Replacements { - if r.File == "" { - return fmt.Errorf("event %q has a replacement with no file name", e.Name) - } - - var count int - if r.YAMLField != "" { - count++ - } - if r.JSONField != "" { - count++ - } - if r.HCLField != "" { - count++ - } - if r.Regex != "" { - count++ - } - if count == 0 { - return fmt.Errorf("event %q has a replacement with no field", e.Name) - } - if count > 2 { - return fmt.Errorf("event %q has multiple fields", e.Name) + if err := r.Validate(); err != nil { + return fmt.Errorf("event %q: %w", e.Name, err) } } return nil From 0a3dd44d0c15362a5f41ced3d503fdcfd75d8c6a Mon Sep 17 00:00:00 2001 From: Ewuji John Date: Tue, 29 Sep 2026 00:51:13 +0100 Subject: [PATCH 4/5] Preserve legacy event watcher configuration Signed-off-by: Ewuji John --- .gitignore | 2 -- pkg/config/application.go | 6 ------ pkg/configv1/application.go | 6 ------ pkg/configv1/application_test.go | 23 ----------------------- 4 files changed, 37 deletions(-) diff --git a/.gitignore b/.gitignore index 14945ce1e4..f4e4acaa43 100644 --- a/.gitignore +++ b/.gitignore @@ -67,5 +67,3 @@ gomock_reflect_*/ # hack hack/oidc/realm.local.json - -WORKABLE.md \ No newline at end of file diff --git a/pkg/config/application.go b/pkg/config/application.go index 36c29b5159..7c8d2b222d 100644 --- a/pkg/config/application.go +++ b/pkg/config/application.go @@ -177,12 +177,6 @@ func (s *GenericApplicationSpec) Validate() error { } } - for _, ew := range s.EventWatcher { - if err := ew.Handler.Config.Validate(); err != nil { - return fmt.Errorf("invalid event watcher handler config: %w", err) - } - } - return nil } diff --git a/pkg/configv1/application.go b/pkg/configv1/application.go index 3825c94183..0eae80e8d6 100644 --- a/pkg/configv1/application.go +++ b/pkg/configv1/application.go @@ -157,12 +157,6 @@ func (s *GenericApplicationSpec) Validate() error { } } - for _, ew := range s.EventWatcher { - if err := ew.Validate(); err != nil { - return err - } - } - return nil } diff --git a/pkg/configv1/application_test.go b/pkg/configv1/application_test.go index 097cd89633..f8a0781725 100644 --- a/pkg/configv1/application_test.go +++ b/pkg/configv1/application_test.go @@ -545,29 +545,6 @@ func TestGenericPostSyncConfiguration(t *testing.T) { } } -func TestGenericApplicationSpecValidatesEventWatcher(t *testing.T) { - t.Parallel() - - spec := GenericApplicationSpec{ - EventWatcher: []EventWatcherConfig{ - { - Handler: EventWatcherHandler{ - Config: EventWatcherHandlerConfig{ - Replacements: []EventWatcherReplacement{ - { - File: "version.json", - JSONField: "$.image", - }, - }, - }, - }, - }, - }, - } - - assert.Error(t, spec.Validate()) -} - func TestGetStageConfigByte(t *testing.T) { testcases := []struct { name string From 188869d5a877b7b4249686f7694438834313dc4e Mon Sep 17 00:00:00 2001 From: Ewuji John Date: Wed, 30 Sep 2026 10:42:43 +0100 Subject: [PATCH 5/5] Preserve legacy event watcher loading Signed-off-by: Ewuji John --- pkg/config/event_watcher.go | 24 ++++++++++++-- pkg/configv1/event_watcher.go | 24 ++++++++++++-- pkg/configv1/event_watcher_test.go | 52 ------------------------------ 3 files changed, 44 insertions(+), 56 deletions(-) diff --git a/pkg/config/event_watcher.go b/pkg/config/event_watcher.go index 6d8a2589bc..5292ea0455 100644 --- a/pkg/config/event_watcher.go +++ b/pkg/config/event_watcher.go @@ -238,8 +238,28 @@ func (e *EventWatcherEvent) Validate() error { return fmt.Errorf("there must be at least one replacement to an event") } for _, r := range e.Replacements { - if err := r.Validate(); err != nil { - return fmt.Errorf("event %q: %w", e.Name, err) + if r.File == "" { + return fmt.Errorf("event %q has a replacement with no file name", e.Name) + } + + var count int + if r.YAMLField != "" { + count++ + } + if r.JSONField != "" { + count++ + } + if r.HCLField != "" { + count++ + } + if r.Regex != "" { + count++ + } + if count == 0 { + return fmt.Errorf("event %q has a replacement with no field", e.Name) + } + if count > 2 { + return fmt.Errorf("event %q has multiple fields", e.Name) } } return nil diff --git a/pkg/configv1/event_watcher.go b/pkg/configv1/event_watcher.go index 366267e5dc..bcfdcf6643 100644 --- a/pkg/configv1/event_watcher.go +++ b/pkg/configv1/event_watcher.go @@ -245,8 +245,28 @@ func (e *EventWatcherEvent) Validate() error { return fmt.Errorf("there must be at least one replacement to an event") } for _, r := range e.Replacements { - if err := r.Validate(); err != nil { - return fmt.Errorf("event %q: %w", e.Name, err) + if r.File == "" { + return fmt.Errorf("event %q has a replacement with no file name", e.Name) + } + + var count int + if r.YAMLField != "" { + count++ + } + if r.JSONField != "" { + count++ + } + if r.HCLField != "" { + count++ + } + if r.Regex != "" { + count++ + } + if count == 0 { + return fmt.Errorf("event %q has a replacement with no field", e.Name) + } + if count > 2 { + return fmt.Errorf("event %q has multiple fields", e.Name) } } return nil diff --git a/pkg/configv1/event_watcher_test.go b/pkg/configv1/event_watcher_test.go index 0c82adf752..20629cfc00 100644 --- a/pkg/configv1/event_watcher_test.go +++ b/pkg/configv1/event_watcher_test.go @@ -166,58 +166,6 @@ func TestEventWatcherValidate(t *testing.T) { }, wantErr: true, }, - { - name: "json field given", - eventWatcherSpec: EventWatcherSpec{ - Events: []EventWatcherEvent{ - { - Name: "event-a", - Replacements: []EventWatcherReplacement{ - { - File: "file.json", - JSONField: "$.value", - }, - }, - }, - }, - }, - wantErr: true, - }, - { - name: "HCL field given", - eventWatcherSpec: EventWatcherSpec{ - Events: []EventWatcherEvent{ - { - Name: "event-a", - Replacements: []EventWatcherReplacement{ - { - File: "file.hcl", - HCLField: "value", - }, - }, - }, - }, - }, - wantErr: true, - }, - { - name: "both supported fields given", - eventWatcherSpec: EventWatcherSpec{ - Events: []EventWatcherEvent{ - { - Name: "event-a", - Replacements: []EventWatcherReplacement{ - { - File: "file", - YAMLField: "$.value", - Regex: "(value)", - }, - }, - }, - }, - }, - wantErr: true, - }, { name: "valid config given", eventWatcherSpec: EventWatcherSpec{