From ae1367f97d7195cb1184363e99ef47b16445e9e9 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Fri, 21 Jun 2024 12:54:36 -0400 Subject: [PATCH] Fix upgrade bug for GitHub Actions container refs (#85) There was a bug with "upgrade" wherein GitHub Actions container refs were losing their "docker://" prefix. This also introduces a new "-pin" flag to the "upgrade" command, which can be used to skip pinning upgraded refs. --- command/command_gen.go | 11 ++++---- command/command_test.go | 1 + command/upgrade.go | 8 ++++-- parser/actions.go | 10 +++++++ parser/circleci.go | 5 ++++ parser/cloudbuild.go | 5 ++++ parser/drone.go | 5 ++++ parser/gitlabci.go | 5 ++++ parser/parser.go | 6 ++-- parser/tekton.go | 16 +++++++---- resolver/actions_test.go | 6 ++-- resolver/container_test.go | 56 ++++++++++++++++++++++++++++++++++++++ testdata/docker.yml | 8 ++++++ 13 files changed, 124 insertions(+), 18 deletions(-) create mode 100644 resolver/container_test.go create mode 100644 testdata/docker.yml diff --git a/command/command_gen.go b/command/command_gen.go index d7dd9ff0fa..d2a90e682d 100644 --- a/command/command_gen.go +++ b/command/command_gen.go @@ -4,11 +4,11 @@ package command const topLevelHelp = `Usage: ratchet COMMAND - check Check if all versions are pinned - pin Resolve and pin all versions - unpin Revert pinned versions to their unpinned values - update Update all pinned versions to the latest value - upgrade Upgrade all pinned versions to the latest version + check Check if all versions are pinned + pin Resolve and pin all versions + unpin Revert pinned versions to their unpinned values + update Update all pinned versions to the latest value + upgrade Upgrade all pinned versions to the latest version Available parsers: @@ -17,4 +17,5 @@ Available parsers: cloudbuild drone gitlabci + tekton ` diff --git a/command/command_test.go b/command/command_test.go index c0aa4f9d02..9e828ac904 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -20,6 +20,7 @@ func Test_loadYAMLFiles(t *testing.T) { "c.yml": "", "circleci.yml": "", "cloudbuild.yml": "", + "docker.yml": "", "drone.yml": "", "github-crazy-indent.yml": "github.yml", "github-issue-80.yml": "", diff --git a/command/upgrade.go b/command/upgrade.go index 17b47dbd27..60f6b8cb07 100644 --- a/command/upgrade.go +++ b/command/upgrade.go @@ -38,6 +38,7 @@ type UpgradeCommand struct { flagConcurrency int64 flagParser string flagOut string + flagPin bool } func (c *UpgradeCommand) Desc() string { @@ -55,6 +56,7 @@ func (c *UpgradeCommand) Flags() *flag.FlagSet { "maximum number of concurrent resolutions") f.StringVar(&c.flagParser, "parser", "actions", "parser to use") f.StringVar(&c.flagOut, "out", "", "output path (defaults to input file)") + f.BoolVar(&c.flagPin, "pin", true, "pin resolved upgraded versions (defaults to true)") return f } @@ -94,8 +96,10 @@ func (c *UpgradeCommand) Run(ctx context.Context, originalArgs []string) error { return fmt.Errorf("failed to upgrade refs: %w", err) } - if err := parser.Pin(ctx, res, par, files.nodes(), c.flagConcurrency); err != nil { - return fmt.Errorf("failed to pin upgraded refs: %w", err) + if c.flagPin { + if err := parser.Pin(ctx, res, par, files.nodes(), c.flagConcurrency); err != nil { + return fmt.Errorf("failed to pin upgraded refs: %w", err) + } } for _, f := range files { diff --git a/parser/actions.go b/parser/actions.go index 17c1f4a833..27ae9b0680 100644 --- a/parser/actions.go +++ b/parser/actions.go @@ -12,6 +12,16 @@ import ( type Actions struct{} +// DenormalizeRef changes the resolved ref into a ref that the parser expects. +func (a *Actions) DenormalizeRef(ref string) string { + isContainer := strings.HasPrefix(ref, resolver.ContainerProtocol) + ref = resolver.DenormalizeRef(ref) + if isContainer { + return "docker://" + ref + } + return ref +} + // Parse pulls the GitHub Actions refs from the documents. func (a *Actions) Parse(nodes []*yaml.Node) (*RefsList, error) { var refs RefsList diff --git a/parser/circleci.go b/parser/circleci.go index b0e0b91abc..c5794b3de0 100644 --- a/parser/circleci.go +++ b/parser/circleci.go @@ -11,6 +11,11 @@ import ( type CircleCI struct{} +// DenormalizeRef changes the resolved ref into a ref that the parser expects. +func (c *CircleCI) DenormalizeRef(ref string) string { + return resolver.DenormalizeRef(ref) +} + // Parse pulls the CircleCI refs from the documents. Unfortunately it does not // process "orbs" because there is no documented API for resolving orbs to an // absolute version. diff --git a/parser/cloudbuild.go b/parser/cloudbuild.go index 4acc73367d..7e15682b16 100644 --- a/parser/cloudbuild.go +++ b/parser/cloudbuild.go @@ -11,6 +11,11 @@ import ( type CloudBuild struct{} +// DenormalizeRef changes the resolved ref into a ref that the parser expects. +func (c *CloudBuild) DenormalizeRef(ref string) string { + return resolver.DenormalizeRef(ref) +} + // Parse pulls the Google Cloud Build refs from the documents. func (c *CloudBuild) Parse(nodes []*yaml.Node) (*RefsList, error) { var refs RefsList diff --git a/parser/drone.go b/parser/drone.go index a02c4bb15c..16b91e5a72 100644 --- a/parser/drone.go +++ b/parser/drone.go @@ -11,6 +11,11 @@ import ( type Drone struct{} +// DenormalizeRef changes the resolved ref into a ref that the parser expects. +func (d *Drone) DenormalizeRef(ref string) string { + return resolver.DenormalizeRef(ref) +} + // Parse pulls the Drone Ci refs from the documents. func (d *Drone) Parse(nodes []*yaml.Node) (*RefsList, error) { var refs RefsList diff --git a/parser/gitlabci.go b/parser/gitlabci.go index c1c59c523e..a799b5f10a 100644 --- a/parser/gitlabci.go +++ b/parser/gitlabci.go @@ -11,6 +11,11 @@ import ( type GitLabCI struct{} +// DenormalizeRef changes the resolved ref into a ref that the parser expects. +func (c *GitLabCI) DenormalizeRef(ref string) string { + return resolver.DenormalizeRef(ref) +} + // Parse pulls the image references from GitLab CI configuration files. It does // not support references with variables. func (c *GitLabCI) Parse(nodes []*yaml.Node) (*RefsList, error) { diff --git a/parser/parser.go b/parser/parser.go index d6fa299770..b2047c4fb6 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -23,6 +23,7 @@ const ( // Parser defines an interface which parses references out of the given yaml // node. type Parser interface { + DenormalizeRef(ref string) string Parse(nodes []*yaml.Node) (*RefsList, error) } @@ -220,11 +221,12 @@ func Upgrade(ctx context.Context, res resolver.Resolver, parser Parser, nodes [] merrLock.Unlock() } - denormLatest := resolver.DenormalizeRef(latest) + denormRef := parser.DenormalizeRef(ref) + denormLatest := parser.DenormalizeRef(latest) for _, node := range nodes { node.LineComment = appendOriginalToComment(node.LineComment, denormLatest) - node.Value = denormLatest + node.Value = strings.Replace(node.Value, denormRef, denormLatest, 1) } }() } diff --git a/parser/tekton.go b/parser/tekton.go index ca752f3493..886b113b16 100644 --- a/parser/tekton.go +++ b/parser/tekton.go @@ -11,11 +11,16 @@ import ( type Tekton struct{} +// DenormalizeRef changes the resolved ref into a ref that the parser expects. +func (t *Tekton) DenormalizeRef(ref string) string { + return resolver.DenormalizeRef(ref) +} + // Parse pulls the Tekton Ci refs from the documents. -func (d *Tekton) Parse(nodes []*yaml.Node) (*RefsList, error) { +func (t *Tekton) Parse(nodes []*yaml.Node) (*RefsList, error) { var refs RefsList for i, node := range nodes { - if err := d.parseOne(&refs, node); err != nil { + if err := t.parseOne(&refs, node); err != nil { return nil, fmt.Errorf("failed to parse node %d: %w", i, err) } } @@ -48,17 +53,17 @@ func (d *Tekton) parseOne(refs *RefsList, node *yaml.Node) error { return nil } -func (d *Tekton) findSpecs(refs *RefsList, node *yaml.Node) error { + +func (d *Tekton) findSpecs(refs *RefsList, node *yaml.Node) { for i, specsMap := range node.Content { if specsMap.Value == "spec" { specs := node.Content[i+1] d.findImages(refs, specs) } } - return nil } -func (d *Tekton) findImages(refs *RefsList, node *yaml.Node) error { +func (d *Tekton) findImages(refs *RefsList, node *yaml.Node) { for i, property := range node.Content { if property.Value == "image" { image := node.Content[i+1] @@ -69,5 +74,4 @@ func (d *Tekton) findImages(refs *RefsList, node *yaml.Node) error { d.findImages(refs, property) } } - return nil } diff --git a/resolver/actions_test.go b/resolver/actions_test.go index b4d748a033..b9680b8a2a 100644 --- a/resolver/actions_test.go +++ b/resolver/actions_test.go @@ -8,7 +8,7 @@ import ( "testing" ) -func TestResolve(t *testing.T) { +func TestActions_Resolve(t *testing.T) { t.Parallel() ctx := context.Background() @@ -57,7 +57,7 @@ func TestResolve(t *testing.T) { } } -func TestLatestVersion(t *testing.T) { +func TestActions_LatestVersion(t *testing.T) { t.Parallel() ctx := context.Background() @@ -125,7 +125,7 @@ func TestLatestVersion(t *testing.T) { } } -func TestParseRef(t *testing.T) { +func TestParseActionRef(t *testing.T) { t.Parallel() cases := []struct { diff --git a/resolver/container_test.go b/resolver/container_test.go new file mode 100644 index 0000000000..822613a469 --- /dev/null +++ b/resolver/container_test.go @@ -0,0 +1,56 @@ +package resolver + +import ( + "context" + "regexp" + "testing" +) + +func TestContainer_Resolve(t *testing.T) { + t.Parallel() + + ctx := context.Background() + resolver, err := NewContainer(ctx) + if err != nil { + t.Fatal(err) + } + + cases := []struct { + name string + in string + exp string + }{ + { + name: "default", + in: "alpine:3", + exp: "index.docker.io/library/alpine@sha256:[0-9a-f]{64}", + }, + { + name: "sha", + in: "alpine@sha256:dabf91b69c191a1a0a1628fd6bdd029c0c4018041c7f052870bb13c5a222ae76", + exp: "alpine@sha256:dabf91b69c191a1a0a1628fd6bdd029c0c4018041c7f052870bb13c5a222ae76", + }, + } + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + result, err := resolver.Resolve(ctx, tc.in) + if err != nil { + t.Fatal(err) + } + + match, err := regexp.MatchString(tc.exp, result) + if err != nil { + t.Fatal(err) + } + + if !match { + t.Errorf("expected %q to match %q", result, tc.exp) + } + }) + } +} diff --git a/testdata/docker.yml b/testdata/docker.yml new file mode 100644 index 0000000000..0df39af729 --- /dev/null +++ b/testdata/docker.yml @@ -0,0 +1,8 @@ +jobs: + docker: + steps: + - name: 'docker' + uses: 'docker://alpine:3' + with: + entrypoint: '/bin/sh' + args: '-euc "test -n "/bin/sh"'