Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check upgrade: respect tag and strip-prefix for github #403

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions pkg/checks/testdata/git-checkout-correct-tag.yaml
Original file line number Diff line number Diff line change
@@ -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
19 changes: 19 additions & 0 deletions pkg/checks/testdata/git-checkout-wrong-just-strip.yaml
Original file line number Diff line number Diff line change
@@ -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
18 changes: 18 additions & 0 deletions pkg/checks/testdata/git-checkout-wrong-no-strip.yaml
Original file line number Diff line number Diff line change
@@ -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
18 changes: 18 additions & 0 deletions pkg/checks/testdata/git-checkout-wrong-tag.yaml
Original file line number Diff line number Diff line change
@@ -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
38 changes: 30 additions & 8 deletions pkg/checks/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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, ".") {
Expand All @@ -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))
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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]
Expand All @@ -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)
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand Down
56 changes: 54 additions & 2 deletions pkg/checks/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks

import (
"bytes"
"errors"
"log"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -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)

Expand All @@ -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())
}
})
}
}
Loading