From b8fac6e8d021451dda5eb1f22072711917c32a00 Mon Sep 17 00:00:00 2001 From: Junhuan Zheng <3373484735@qq.com> Date: Fri, 7 Aug 2026 16:49:59 +0800 Subject: [PATCH] fix(yaml): pass keyonly condition with searchpattern when at least one file matches When a yaml condition uses searchpattern with keyonly, it should pass as long as at least one matched file contains the specified key, aligning with the file plugin behavior. Previously the condition failed whenever any matched file did not contain the key. Also record a per-file key miss for the yamlpath engine so the searchpattern handling can distinguish 'at least one file contains the key' from 'no file contains it'. Closes #9848 Signed-off-by: Junhuan Zheng <3373484735@qq.com> --- pkg/plugins/resources/yaml/condition.go | 34 +++- pkg/plugins/resources/yaml/condition_test.go | 157 ++++++++++++++++++- 2 files changed, 184 insertions(+), 7 deletions(-) diff --git a/pkg/plugins/resources/yaml/condition.go b/pkg/plugins/resources/yaml/condition.go index cc7ffe5163..328ee2cf06 100644 --- a/pkg/plugins/resources/yaml/condition.go +++ b/pkg/plugins/resources/yaml/condition.go @@ -115,6 +115,13 @@ func (y *Yaml) Condition(_ context.Context, source string, scm scm.ScmHandler) ( docs = append(docs, &doc) } + // Track whether the key was found in at least one document of this + // file. The yamlpath engine does not report an error when the key is + // missing: it returns an empty result set. For keyonly conditions we + // record a miss per file (the same way the go-yaml engine does) so the + // handling below can distinguish "at least one file contains the key" + // (searchpattern) from "no file contains it". + fileKeyFound := false for index, doc := range docs { if y.spec.DocumentIndex != nil { if index != *y.spec.DocumentIndex { @@ -138,6 +145,15 @@ func (y *Yaml) Condition(_ context.Context, source string, scm scm.ScmHandler) ( for i := range founds { results = append(results, founds[i].Value) } + + if len(founds) > 0 { + fileKeyFound = true + } + } + + if !fileKeyFound && y.spec.KeyOnly { + errorMessages = append(errorMessages, + fmt.Errorf("%q - %w", originalFilePath, ErrKeyNotFound)) } default: @@ -145,6 +161,11 @@ func (y *Yaml) Condition(_ context.Context, source string, scm scm.ScmHandler) ( } } + originalFilePaths := make([]string, 0, len(y.files)) + for i := range y.files { + originalFilePaths = append(originalFilePaths, y.files[i].originalFilePath) + } + if len(errorMessages) > 0 { if y.spec.KeyOnly { for i := range errorMessages { @@ -153,17 +174,20 @@ func (y *Yaml) Condition(_ context.Context, source string, scm scm.ScmHandler) ( return false, "", errorsToError(errorMessages) } } + + // When a search pattern is used, the condition passes as soon as at + // least one file contains the key, aligning with the file plugin + // behavior. Otherwise every specified file must contain the key. + if y.spec.SearchPattern && len(results) > 0 { + return true, fmt.Sprintf("key %q found in yaml file(s) [%q]", y.spec.Key, strings.Join(originalFilePaths, ",")), nil + } + return false, "key not found in yaml file(s)", nil } return false, "", errorsToError(errorMessages) } - originalFilePaths := make([]string, len(y.files)) - for i := range y.files { - originalFilePaths = append(originalFilePaths, y.files[i].originalFilePath) - } - // When user want to only check the existence of a YAML key if y.spec.KeyOnly { if len(results) == len(y.files) { diff --git a/pkg/plugins/resources/yaml/condition_test.go b/pkg/plugins/resources/yaml/condition_test.go index a81634a025..987b640306 100644 --- a/pkg/plugins/resources/yaml/condition_test.go +++ b/pkg/plugins/resources/yaml/condition_test.go @@ -3,10 +3,13 @@ package yaml import ( "context" "fmt" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/updatecli/updatecli/pkg/core/pipeline/scm" "github.com/updatecli/updatecli/pkg/core/text" ) @@ -629,11 +632,161 @@ repos: }, isResultWanted: true, }, + { + name: "Passing case with keyonly and searchpattern when at least one file contains the key (yamlpath)", + spec: Spec{ + Files: []string{ + "*.yaml", + }, + Key: "$.github.owner", + KeyOnly: true, + Engine: "yamlpath", + SearchPattern: true, + }, + files: map[string]file{ + "test.yaml": { + filePath: "test.yaml", + originalFilePath: "test.yaml", + }, + "too-much.yaml": { + filePath: "too-much.yaml", + originalFilePath: "too-much.yaml", + }, + }, + mockedContents: map[string]string{ + "test.yaml": `--- +github: + owner: olblak + repository: charts +`, + "too-much.yaml": `--- +name: github +`, + }, + isResultWanted: true, + }, + { + name: "Passing case with keyonly and searchpattern when at least one file contains the key (go-yaml)", + spec: Spec{ + Files: []string{ + "*.yaml", + }, + Key: "$.github.owner", + KeyOnly: true, + SearchPattern: true, + }, + files: map[string]file{ + "test.yaml": { + filePath: "test.yaml", + originalFilePath: "test.yaml", + }, + "too-much.yaml": { + filePath: "too-much.yaml", + originalFilePath: "too-much.yaml", + }, + }, + mockedContents: map[string]string{ + "test.yaml": `--- +github: + owner: olblak + repository: charts +`, + "too-much.yaml": `--- +name: github +`, + }, + isResultWanted: true, + }, + { + name: "Failing case with keyonly and searchpattern when no file contains the key", + spec: Spec{ + Files: []string{ + "*.yaml", + }, + Key: "$.github.owner", + KeyOnly: true, + Engine: "yamlpath", + SearchPattern: true, + }, + files: map[string]file{ + "test.yaml": { + filePath: "test.yaml", + originalFilePath: "test.yaml", + }, + "too-much.yaml": { + filePath: "too-much.yaml", + originalFilePath: "too-much.yaml", + }, + }, + mockedContents: map[string]string{ + "test.yaml": `--- +name: github +`, + "too-much.yaml": `--- +name: updatecli +`, + }, + isResultWanted: false, + }, + { + name: "Failing case with keyonly and multiple files when not all files contain the key (no searchpattern)", + spec: Spec{ + Files: []string{ + "test.yaml", + "too-much.yaml", + }, + Key: "$.github.owner", + KeyOnly: true, + Engine: "yamlpath", + }, + files: map[string]file{ + "test.yaml": { + filePath: "test.yaml", + originalFilePath: "test.yaml", + }, + "too-much.yaml": { + filePath: "too-much.yaml", + originalFilePath: "too-much.yaml", + }, + }, + mockedContents: map[string]string{ + "test.yaml": `--- +github: + owner: olblak + repository: charts +`, + "too-much.yaml": `--- +name: github +`, + }, + isResultWanted: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + mockedContents := tt.mockedContents + var scmHandler scm.ScmHandler + + // When a search pattern is used, initFiles resolves the pattern + // against the working directory, so we need real files on disk. + if tt.spec.SearchPattern { + tempDir := t.TempDir() + for fileName := range tt.files { + if err := os.WriteFile(filepath.Join(tempDir, fileName), []byte{}, 0600); err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + } + scmHandler = &scm.MockScm{WorkingDir: tempDir} + + updatedContents := make(map[string]string) + for fileName, content := range tt.mockedContents { + updatedContents[filepath.Join(tempDir, fileName)] = content + } + mockedContents = updatedContents + } + mockedText := text.MockTextRetriever{ - Contents: tt.mockedContents, + Contents: mockedContents, Err: tt.mockedError, } @@ -643,7 +796,7 @@ repos: assert.NoError(t, err) - gotResult, _, gotErr := y.Condition(context.Background(), tt.inputSourceValue, nil) + gotResult, _, gotErr := y.Condition(context.Background(), tt.inputSourceValue, scmHandler) if tt.isErrorWanted { assert.Error(t, gotErr) return