Skip to content

Commit

Permalink
Validate --lib values as image refs
Browse files Browse the repository at this point in the history
This needs a "plugin" to pflag, which I've put in `pkg/cli`.
  • Loading branch information
squaremo committed Mar 31, 2020
1 parent fe3bbde commit 70a1d91
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 3 deletions.
105 changes: 105 additions & 0 deletions pkg/cli/imageref.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package cli

import (
"bytes"
"encoding/csv"
"fmt"
"io"
"strings"

"github.com/google/go-containerregistry/pkg/name"
)

// ImageRefSliceValue is a slice of image references (name.Reference) for
// use with pflag. Adapted from
// https://github.com/spf13/pflag/blob/master/string_slice.go
type ImageRefSliceValue struct {
refs *[]name.Reference
changed bool
}

// NewImageRefSliceValue creates a `pflag.Value` wrapper for a slice
// of name.Reference (image reference) values.
func NewImageRefSliceValue(p *[]name.Reference) *ImageRefSliceValue {
v := new(ImageRefSliceValue)
v.refs = p
return v
}

func writeRefsAsCsv(vals []name.Reference) (string, error) {
strs := make([]string, len(vals), len(vals))
for i := range vals {
strs[i] = vals[i].String()
}
b := &bytes.Buffer{}
w := csv.NewWriter(b)
err := w.Write(strs)
if err != nil {
return "", err
}
w.Flush()
return strings.TrimSuffix(b.String(), "\n"), nil
}

func readAsCSV(val string) ([]string, error) {
if val == "" {
return []string{}, nil
}
stringReader := strings.NewReader(val)
csvReader := csv.NewReader(stringReader)
return csvReader.Read()
}

// String returns a string representation of the value
func (value *ImageRefSliceValue) String() string {
str, _ := writeRefsAsCsv(*value.refs)
return "[" + str + "]"
}

// Set sets the underlying value given a string passed to the flag. As
// with other slice values, if it has already been set, being called
// again will append to the slice.
func (value *ImageRefSliceValue) Set(v string) error {
strs, err := readAsCSV(v)
if err != nil && err != io.EOF {
return err
}

out := make([]name.Reference, len(strs), len(strs))
for i := range strs {
r, err := name.ParseReference(strs[i])
if err != nil {
return err
}
// we want either a tag or a digest
if r.Identifier() == "latest" {
return fmt.Errorf("image ref has no tag or digest, or uses 'latest'")
}

out[i] = r
}

if !value.changed {
*value.refs = out
} else {
*value.refs = append(*value.refs, out...)
}

value.changed = true
return nil
}

// Type returns a name for the type of flag
func (*ImageRefSliceValue) Type() string {
return "imageRefSlice"
}

// getStringSlice is a handy way to get a slice of strings from the
// value, for testing.
func (value *ImageRefSliceValue) getStringSlice() []string {
strs := make([]string, len(*value.refs), len(*value.refs))
for i, ref := range *value.refs {
strs[i] = ref.String()
}
return strs
}
78 changes: 78 additions & 0 deletions pkg/cli/imageref_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package cli

import (
"testing"

"github.com/google/go-containerregistry/pkg/name"
"github.com/spf13/pflag"
"github.com/stretchr/testify/assert"
)

func TestImageRefFlag(t *testing.T) {
positives := []string{
"foo:v1",
"alpine:3.9",
"jkcfg/kubernetes:0.6.2",
"localhost:5000/foo:master",
"jkcfg/kubernetes@sha256:93086646d44cd87edc3cc5b9c48133f36db122683bf5865d3e353e3d132a85bd",
}

for _, arg := range positives {
t.Run(arg, func(t *testing.T) {
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
var refs []name.Reference
val := NewImageRefSliceValue(&refs)
// it's just easier to get a flagset to create flag for us
flags.Var(val, "lib", "a test flag")
assert.NoError(t, flags.Parse([]string{"--lib", arg}))
assert.Equal(t, val.getStringSlice(), []string{arg})
})
}

t.Run("slice parsing", func(t *testing.T) {
// Check that more than one can be passed
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
var refs []name.Reference
val := NewImageRefSliceValue(&refs)
// it's just easier to get a flagset to create flag for us
flags.Var(val, "lib", "a test flag")
args := []string{
"--lib", positives[0],
"--lib", positives[1] + "," + positives[2],
}
assert.NoError(t, flags.Parse(args))
assert.Equal(t, val.getStringSlice(), positives[:3])
})

negatives := []string{
"foo", // no tag
"foo/bar:", // still no tag; malformed
"punctation/'notallowed'", // illegal characters
"foo@sha256:abc123", // malformed digest
}

for _, arg := range negatives {
t.Run("[bad]"+arg, func(t *testing.T) {
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
var refs []name.Reference
// it's just easier to get a flagset to create flag for us
flags.Var(NewImageRefSliceValue(&refs), "lib", "a test flag")
assert.Error(t, flags.Parse([]string{"--lib", arg}))
})
}

// Check that a bad apple results in a parse failure
t.Run("bad apple", func(t *testing.T) {
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
var refs []name.Reference
val := NewImageRefSliceValue(&refs)
// it's just easier to get a flagset to create flag for us
flags.Var(val, "lib", "a test flag")
args := []string{
"--lib", positives[1],
"--lib", positives[2],
"--lib", negatives[0],
}
assert.Error(t, flags.Parse(args))
})
}
8 changes: 5 additions & 3 deletions vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import (
"os"
"path/filepath"

"github.com/google/go-containerregistry/pkg/name"
"github.com/pkg/errors"
"github.com/spf13/cobra"

"github.com/jkcfg/jk/pkg/cli"
"github.com/jkcfg/jk/pkg/deferred"
"github.com/jkcfg/jk/pkg/image"
"github.com/jkcfg/jk/pkg/image/cache"
Expand All @@ -31,7 +33,7 @@ type vmOptions struct {
outputDirectory string
inputDirectory string
cacheDir string
libraryImages []string
libraryImages []name.Reference
parameters std.Params
parameterFiles []string // list of files specified on the command line with -f.
emitDependencies bool
Expand All @@ -48,7 +50,7 @@ func initInputFlags(cmd *cobra.Command, opts *vmOptions) {
func initExecFlags(cmd *cobra.Command, opts *vmOptions) {
opts.parameters = std.NewParams()

cmd.PersistentFlags().StringSliceVar(&opts.libraryImages, "lib", nil, "use image in module search path, downloading it if necessary")
cmd.PersistentFlags().Var(cli.NewImageRefSliceValue(&opts.libraryImages), "lib", "use image in module search path, downloading it if necessary")
cmd.PersistentFlags().StringVar(&opts.cacheDir, "cache", "", "directory to use for caching downloaded images; if empty, the default for the OS will be used")
cmd.PersistentFlags().BoolVarP(&opts.verbose, "verbose", "v", false, "verbose output")
cmd.PersistentFlags().StringVarP(&opts.outputDirectory, "output-directory", "o", "", "where to output generated files")
Expand Down Expand Up @@ -141,7 +143,7 @@ func newVM(opts *vmOptions, workingDirectory string) *vm {
cache := cache.New(vm.vmOptions.cacheDir)

for _, lib := range opts.libraryImages {
imgVfs, err := cache.EnsureImage(lib)
imgVfs, err := cache.EnsureImage(lib.String())
if err != nil {
log.Fatalf("run: unable to fetch image %q: %s", lib, err.Error())
}
Expand Down

0 comments on commit 70a1d91

Please sign in to comment.