From 74824ea1eb368942919e66180869c6a4da8fad63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Furkan=20T=C3=BCrkal?= Date: Fri, 22 Sep 2023 09:49:10 +0300 Subject: [PATCH] check upgarade: respect tag and strip-prefix for github MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Furkan Türkal --- .../testdata/git-checkout-correct-tag.yaml | 19 +++++++ .../git-checkout-wrong-just-strip.yaml | 19 +++++++ .../testdata/git-checkout-wrong-no-strip.yaml | 18 ++++++ .../testdata/git-checkout-wrong-tag.yaml | 18 ++++++ pkg/checks/update.go | 38 ++++++++++--- pkg/checks/update_test.go | 56 ++++++++++++++++++- 6 files changed, 158 insertions(+), 10 deletions(-) create mode 100644 pkg/checks/testdata/git-checkout-correct-tag.yaml create mode 100644 pkg/checks/testdata/git-checkout-wrong-just-strip.yaml create mode 100644 pkg/checks/testdata/git-checkout-wrong-no-strip.yaml create mode 100644 pkg/checks/testdata/git-checkout-wrong-tag.yaml diff --git a/pkg/checks/testdata/git-checkout-correct-tag.yaml b/pkg/checks/testdata/git-checkout-correct-tag.yaml new file mode 100644 index 00000000..b34b08a9 --- /dev/null +++ b/pkg/checks/testdata/git-checkout-correct-tag.yaml @@ -0,0 +1,19 @@ +package: + name: git-checkout-correct-tag + version: 1.10.0 + epoch: 0 + description: scandir, a better directory iterator and faster os.walk() + +pipeline: + - uses: git-checkout + with: + expected-commit: 982e6ba60e7165ef965567eacd7138149c9ce292 + repository: https://github.com/benhoyt/scandir + tag: v${{package.version}} + +update: + enabled: true + manual: false + github: + identifier: benhoyt/scandir + strip-prefix: v diff --git a/pkg/checks/testdata/git-checkout-wrong-just-strip.yaml b/pkg/checks/testdata/git-checkout-wrong-just-strip.yaml new file mode 100644 index 00000000..646dfa87 --- /dev/null +++ b/pkg/checks/testdata/git-checkout-wrong-just-strip.yaml @@ -0,0 +1,19 @@ +package: + name: git-checkout-wrong-just-strip + version: 1.10.0 + epoch: 0 + description: scandir, a better directory iterator and faster os.walk() + +pipeline: + - uses: git-checkout + with: + expected-commit: 982e6ba60e7165ef965567eacd7138149c9ce292 + repository: https://github.com/benhoyt/scandir + tag: ${{package.version}} + +update: + enabled: true + manual: false + github: + identifier: benhoyt/scandir + strip-prefix: v diff --git a/pkg/checks/testdata/git-checkout-wrong-no-strip.yaml b/pkg/checks/testdata/git-checkout-wrong-no-strip.yaml new file mode 100644 index 00000000..4fffb1e5 --- /dev/null +++ b/pkg/checks/testdata/git-checkout-wrong-no-strip.yaml @@ -0,0 +1,18 @@ +package: + name: git-checkout-wrong-no-strip + version: 1.10.0 + epoch: 0 + description: scandir, a better directory iterator and faster os.walk() + +pipeline: + - uses: git-checkout + with: + expected-commit: 982e6ba60e7165ef965567eacd7138149c9ce292 + repository: https://github.com/benhoyt/scandir + tag: v${{package.version}} + +update: + enabled: true + manual: false + github: + identifier: benhoyt/scandir diff --git a/pkg/checks/testdata/git-checkout-wrong-tag.yaml b/pkg/checks/testdata/git-checkout-wrong-tag.yaml new file mode 100644 index 00000000..974caae3 --- /dev/null +++ b/pkg/checks/testdata/git-checkout-wrong-tag.yaml @@ -0,0 +1,18 @@ +package: + name: git-checkout-wrong-tag + version: 1.10.0 + epoch: 0 + description: scandir, a better directory iterator and faster os.walk() + +pipeline: + - uses: git-checkout + with: + expected-commit: 982e6ba60e7165ef965567eacd7138149c9ce292 + repository: https://github.com/benhoyt/scandir + tag: ${{package.version}} + +update: + enabled: true + manual: false + github: + identifier: benhoyt/scandir diff --git a/pkg/checks/update.go b/pkg/checks/update.go index b6c6a35a..80227c90 100644 --- a/pkg/checks/update.go +++ b/pkg/checks/update.go @@ -50,7 +50,7 @@ func (o CheckUpdateOptions) CheckUpdates(files []string) error { changedPackages := GetPackagesToUpdate(files) - validateUpdateConfig(changedPackages, &checkErrors) + o.validateUpdateConfig(changedPackages, &checkErrors) latestVersions, err := updateOpts.GetLatestVersions(o.Dir, changedPackages) if err != nil { @@ -74,7 +74,7 @@ func (o CheckUpdateOptions) CheckUpdates(files []string) error { } // validates update configuration -func validateUpdateConfig(files []string, checkErrors *lint.EvalRuleErrors) { +func (o CheckUpdateOptions) validateUpdateConfig(files []string, checkErrors *lint.EvalRuleErrors) { for _, file := range files { // skip hidden files if strings.HasPrefix(file, ".") { @@ -85,6 +85,12 @@ func validateUpdateConfig(files []string, checkErrors *lint.EvalRuleErrors) { if !strings.HasSuffix(file, ".yaml") { file += ".yaml" } + + // if a directory is specified then join the file name instead of relying on the current working directory + if o.Dir != "" { + file = filepath.Join(o.Dir, file) + } + yamlData, err := os.ReadFile(file) if err != nil { addCheckError(checkErrors, errors.Wrapf(err, "failed to read %s", file)) @@ -192,7 +198,7 @@ func (o CheckUpdateOptions) processUpdates(latestVersions map[string]update.NewV defer os.RemoveAll(tempDir) - for packageName, newVersion := range latestVersions { + for packageName, latestVersion := range latestVersions { srcConfigFile := filepath.Join(o.Dir, packageName+".yaml") dryRunConfig, err := config.ParseConfiguration(srcConfigFile) @@ -201,6 +207,9 @@ func (o CheckUpdateOptions) processUpdates(latestVersions map[string]update.NewV } applyOverrides(&o, dryRunConfig) + currentVersion := dryRunConfig.Package.Version + newVersion := latestVersion.Version + data, err := yaml.Marshal(dryRunConfig) if err != nil { return err @@ -213,7 +222,7 @@ func (o CheckUpdateOptions) processUpdates(latestVersions map[string]update.NewV } // melange bump will modify the modified copy of the melange config - err = melange.Bump(tmpConfigFile, newVersion.Version, newVersion.Commit) + err = melange.Bump(tmpConfigFile, latestVersion.Version, latestVersion.Commit) if err != nil { addCheckError(checkErrors, errors.Wrapf(err, "package %s: failed to validate update config, melange bump", packageName)) continue @@ -244,7 +253,7 @@ func (o CheckUpdateOptions) processUpdates(latestVersions map[string]update.NewV } // download or git clone sources into a temp folder to validate the update config - verifyPipelines(o, updated, mutations, checkErrors) + verifyPipelines(o, updated, currentVersion, newVersion, mutations, checkErrors) } return nil } @@ -255,7 +264,7 @@ func applyOverrides(options *CheckUpdateOptions, dryRunConfig *config.Configurat } } -func verifyPipelines(o CheckUpdateOptions, updated *config.Configuration, mutations map[string]string, checkErrors *lint.EvalRuleErrors) { +func verifyPipelines(o CheckUpdateOptions, updated *config.Configuration, currentVersion, newVersion string, mutations map[string]string, checkErrors *lint.EvalRuleErrors) { for i := range updated.Pipeline { var err error pipeline := updated.Pipeline[i] @@ -264,7 +273,7 @@ func verifyPipelines(o CheckUpdateOptions, updated *config.Configuration, mutati err = o.verifyFetch(&pipeline, mutations) } if pipeline.Uses == "git-checkout" { - err = o.verifyGitCheckout(&pipeline, mutations) + err = o.verifyGitCheckout(&pipeline, currentVersion, newVersion, mutations) } if err != nil { addCheckError(checkErrors, err) @@ -296,7 +305,7 @@ func (o CheckUpdateOptions) verifyFetch(p *config.Pipeline, m map[string]string) return os.RemoveAll(filename) } -func (o CheckUpdateOptions) verifyGitCheckout(p *config.Pipeline, m map[string]string) error { +func (o CheckUpdateOptions) verifyGitCheckout(p *config.Pipeline, currentVersion, newVersion string, m map[string]string) error { repoValue := p.With["repository"] if repoValue == "" { return fmt.Errorf("no repository to checkout") @@ -313,6 +322,19 @@ func (o CheckUpdateOptions) verifyGitCheckout(p *config.Pipeline, m map[string]s return err } + evaluatedCurrentTag, err := util.MutateStringFromMap(map[string]string{ + "package.version": currentVersion, + }, tagValue) + if err != nil { + return err + } + + // Check whether the evaluated tag matches the upstream version by + // respecting the backwards compatibility of error messages. + if evaluatedCurrentTag != evaluatedTag && evaluatedCurrentTag != newVersion { + return fmt.Errorf("given tag '%s' does not match upstream version '%s'", evaluatedCurrentTag, evaluatedTag) + } + tempDir, err := os.MkdirTemp("", "wolfictl") if err != nil { return err diff --git a/pkg/checks/update_test.go b/pkg/checks/update_test.go index 1c85792a..fb032f8c 100644 --- a/pkg/checks/update_test.go +++ b/pkg/checks/update_test.go @@ -2,6 +2,7 @@ package checks import ( "bytes" + "errors" "log" "net/http" "net/http/httptest" @@ -147,9 +148,11 @@ func TestUpdateKeyExists(t *testing.T) { t.Fatal(err) } + o := CheckUpdateOptions{} + checkErrors := make(lint.EvalRuleErrors, 0) // check update key exists - validateUpdateConfig([]string{fileContainsUpdate}, &checkErrors) + o.validateUpdateConfig([]string{fileContainsUpdate}, &checkErrors) assert.Empty(t, checkErrors) @@ -165,6 +168,55 @@ func TestUpdateKeyExists(t *testing.T) { } // check the update key does not exist - validateUpdateConfig([]string{fileNoContainsUpdate}, &checkErrors) + o.validateUpdateConfig([]string{fileNoContainsUpdate}, &checkErrors) assert.NotEmpty(t, checkErrors) } + +func TestCheckUpdate(t *testing.T) { + d, err := filepath.Abs("./testdata") + assert.NoError(t, err) + + o := CheckUpdateOptions{ + Dir: d, + Logger: log.New(log.Writer(), "test: ", log.LstdFlags|log.Lmsgprefix), + } + + tests := []struct { + name string + file string + wantErr error + }{ + { + name: "upstream repo has 'v' prefix in tag but manifest does not", + file: "git-checkout-wrong-tag.yaml", + wantErr: errors.New("given tag '1.10.0' does not match upstream version 'v1.10.0'"), + }, + { + name: "upstream repo has 'v' prefix in tag and also manifest does", + file: "git-checkout-correct-tag.yaml", + wantErr: nil, + }, + { + name: "upstream repo has 'v' prefix in tag but only tag prefix is set", + file: "git-checkout-wrong-no-strip.yaml", + wantErr: errors.New("ref vv1.10.0: couldn't find remote ref \"refs/tags/vv1.10.0\""), + }, + { + name: "upstream repo has 'v' prefix in tag but only strip prefix is set", + file: "git-checkout-wrong-just-strip.yaml", + wantErr: errors.New("ref 1.10.0: couldn't find remote ref \"refs/tags/1.10.0\""), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := o.CheckUpdates([]string{tt.file}) + if (err != nil) != (tt.wantErr != nil) { + t.Fatalf("CheckUpdates() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr != nil { + assert.Contains(t, err.Error(), tt.wantErr.Error()) + } + }) + } +}