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_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":     "",

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 {

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

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.

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

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

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) {

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)
 			}
 		}()
 	}

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
 }

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 {

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)
+			}
+		})
+	}
+}

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"'