diff options
author | Dan Duvall <dduvall@wikimedia.org> | 2017-11-02 17:44:44 -0700 |
---|---|---|
committer | Dan Duvall <dduvall@wikimedia.org> | 2017-11-07 12:57:21 -0800 |
commit | 1d8f07a35c5ac6091a4d7932551d46ebe6b097fa (patch) | |
tree | 3f13c3aaf6ce0476d3c7642fe5de8e1aa9b16a16 | |
parent | 5b5ad0496ff75e787405ab9eec17f13214d0b834 (diff) | |
download | blubber-1d8f07a35c5ac6091a4d7932551d46ebe6b097fa.tar.gz |
Validate configuration after unmarshalling
Summary:
Implemented a validation system using the
`github.com/go-playground/validator` package, extending it with custom
validation tags, and implemented translation of validation errors into
somewhat human-friendly messages.
Fixes T175186
Depends on D845
Test Plan: Run the unit tests and try running blubber against some bad config.
Reviewers: thcipriani, hashar, Jrbranaa, Joe, #release-engineering-team, mobrovac
Reviewed By: thcipriani, #release-engineering-team
Tags: #release-engineering-team
Maniphest Tasks: T175186
Differential Revision: https://phabricator.wikimedia.org/D868
-rw-r--r-- | config/apt.go | 2 | ||||
-rw-r--r-- | config/apt_test.go | 43 | ||||
-rw-r--r-- | config/artifacts.go | 6 | ||||
-rw-r--r-- | config/artifacts_test.go | 52 | ||||
-rw-r--r-- | config/common.go | 12 | ||||
-rw-r--r-- | config/common_test.go | 32 | ||||
-rw-r--r-- | config/config.go | 2 | ||||
-rw-r--r-- | config/config_test.go | 35 | ||||
-rw-r--r-- | config/flag_test.go | 1 | ||||
-rw-r--r-- | config/node.go | 4 | ||||
-rw-r--r-- | config/node_test.go | 32 | ||||
-rw-r--r-- | config/reader.go | 6 | ||||
-rw-r--r-- | config/runs.go | 16 | ||||
-rw-r--r-- | config/runs_test.go | 190 | ||||
-rw-r--r-- | config/validation.go | 187 | ||||
-rw-r--r-- | config/validation_test.go | 17 | ||||
-rw-r--r-- | config/variant.go | 6 | ||||
-rw-r--r-- | config/variant_test.go | 74 | ||||
-rw-r--r-- | main.go | 9 |
19 files changed, 698 insertions, 28 deletions
diff --git a/config/apt.go b/config/apt.go index 3a7a688..01f914e 100644 --- a/config/apt.go +++ b/config/apt.go @@ -8,7 +8,7 @@ import ( // existing APT sources. // type AptConfig struct { - Packages []string `yaml:"packages"` + Packages []string `yaml:"packages" validate:"dive,debianpackage"` } // Merge takes another AptConfig and combines the packages declared within diff --git a/config/apt_test.go b/config/apt_test.go index 222e4c0..2f0df11 100644 --- a/config/apt_test.go +++ b/config/apt_test.go @@ -1,6 +1,7 @@ package config_test import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -60,3 +61,45 @@ func TestAptConfigInstructions(t *testing.T) { assert.Empty(t, cfg.InstructionsForPhase(build.PhasePostInstall)) }) } + +func TestAptConfigValidation(t *testing.T) { + t.Run("packages", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + apt: + packages: + - f1 + - foo-fighter + - bar+b.az + - bar+b.az=0:0.1~foo1-1 + - bar+b.az/stable + - bar+b.az/jessie-wikimedia + variants: {}`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("bad", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + apt: + packages: + - foo + - foo fighter + - bar_baz + - 'bar=0.1*bad version' + - bar/0bad_release + variants: {}`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, strings.Join([]string{ + `packages[1]: "foo fighter" is not a valid Debian package name`, + `packages[2]: "bar_baz" is not a valid Debian package name`, + `packages[3]: "bar=0.1*bad version" is not a valid Debian package name`, + `packages[4]: "bar/0bad_release" is not a valid Debian package name`, + }, "\n"), msg) + } + }) + }) +} diff --git a/config/artifacts.go b/config/artifacts.go index bd1bdeb..74efcab 100644 --- a/config/artifacts.go +++ b/config/artifacts.go @@ -15,9 +15,9 @@ import ( // VariantConfig.Copies. // type ArtifactsConfig struct { - From string `yaml:"from"` // source variant from which to copy - Source string `yaml:"source"` // source path within variant from which to copy - Destination string `yaml:"destination"` // destination path within current variant + From string `yaml:"from" validate:"required,variantref"` // source variant from which to copy + Source string `yaml:"source" validate:"required"` // source variant path from which to copy + Destination string `yaml:"destination" validate:"required"` // destination path within current variant } // InstructionsForPhase injects instructions into the given build phase that diff --git a/config/artifacts_test.go b/config/artifacts_test.go index b2134f2..b7ba07f 100644 --- a/config/artifacts_test.go +++ b/config/artifacts_test.go @@ -11,6 +11,7 @@ import ( func TestArtifactsConfig(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- + base: foo variants: build: {} production: @@ -69,3 +70,54 @@ func TestArtifactsConfigInstructions(t *testing.T) { ) }) } + +func TestArtifactsConfigValidation(t *testing.T) { + t.Run("from", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: + artifacts: + - from: build + source: /foo + destination: /bar`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("missing", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: + artifacts: + - from: ~ + source: /foo + destination: /bar`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `from: is required`, msg) + } + }) + + t.Run("bad", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: + artifacts: + - from: foo bar + source: /foo + destination: /bar`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `from: references an unknown variant "foo bar"`, msg) + } + }) + }) +} diff --git a/config/common.go b/config/common.go index 62fb483..668b543 100644 --- a/config/common.go +++ b/config/common.go @@ -8,12 +8,12 @@ import ( // and each configured variant. // type CommonConfig struct { - Base string `yaml:"base"` // name/path to base image - Apt AptConfig `yaml:"apt"` // APT related configuration - Node NodeConfig `yaml:"node"` // Node related configuration - Runs RunsConfig `yaml:"runs"` // runtime environment configuration - SharedVolume Flag `yaml:"sharedvolume"` // define a volume instead of copying in source files - EntryPoint []string `yaml:"entrypoint"` // entry-point executable + Base string `yaml:"base" validate:"omitempty,baseimage"` // name/path to base image + Apt AptConfig `yaml:"apt"` // APT related configuration + Node NodeConfig `yaml:"node"` // Node related configuration + Runs RunsConfig `yaml:"runs"` // runtime environment configuration + SharedVolume Flag `yaml:"sharedvolume"` // define a volume for application + EntryPoint []string `yaml:"entrypoint"` // entry-point executable } // Merge takes another CommonConfig and merges its fields this one's. diff --git a/config/common_test.go b/config/common_test.go index 54c4dab..20a07f5 100644 --- a/config/common_test.go +++ b/config/common_test.go @@ -28,3 +28,35 @@ func TestCommonConfig(t *testing.T) { assert.Equal(t, true, variant.SharedVolume.True) assert.Equal(t, []string{"/bin/foo"}, variant.EntryPoint) } + +func TestCommonConfigValidation(t *testing.T) { + t.Run("base", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + base: foo + variants: {}`)) + + assert.Nil(t, err) + }) + + t.Run("optional", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + base: + variants: {}`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("bad", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + base: foo fighter + variants: {}`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `base: "foo fighter" is not a valid base image reference`, msg) + } + }) + }) +} diff --git a/config/config.go b/config/config.go index 3360012..46be3d1 100644 --- a/config/config.go +++ b/config/config.go @@ -8,5 +8,5 @@ package config // type Config struct { CommonConfig `yaml:",inline"` - Variants map[string]VariantConfig `yaml:"variants"` + Variants map[string]VariantConfig `yaml:"variants" validate:"variants,dive"` } diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 0000000..051e110 --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,35 @@ +package config_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "phabricator.wikimedia.org/source/blubber/config" +) + +func TestConfigValidation(t *testing.T) { + t.Run("variants", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: {}`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("bad", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build foo: {} + foo bar: {}`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `variants: contains a bad variant name`, msg) + } + }) + }) +} diff --git a/config/flag_test.go b/config/flag_test.go index 2bd26b2..d2651f6 100644 --- a/config/flag_test.go +++ b/config/flag_test.go @@ -10,6 +10,7 @@ import ( func TestFlagOverwrite(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- + base: foo node: { dependencies: true } sharedvolume: false variants: diff --git a/config/node.go b/config/node.go index 561c915..bd5d4db 100644 --- a/config/node.go +++ b/config/node.go @@ -9,8 +9,8 @@ import ( // whether/how to install NPM packages. // type NodeConfig struct { - Dependencies Flag `yaml:"dependencies"` // install dependencies declared in package.json - Env string `yaml:"env"` // environment name ("production" install) + Dependencies Flag `yaml:"dependencies"` // install dependencies declared in package.json + Env string `yaml:"env" validate:"omitempty,nodeenv"` // environment name ("production" install) } // Merge takes another NodeConfig and merges its fields into this one's, diff --git a/config/node_test.go b/config/node_test.go index 7b70a8e..5e0c4f3 100644 --- a/config/node_test.go +++ b/config/node_test.go @@ -11,6 +11,7 @@ import ( func TestNodeConfig(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- + base: foo node: dependencies: true env: foo @@ -156,3 +157,34 @@ func TestNodeConfigInstructionsEnvironmentOnly(t *testing.T) { ) }) } + +func TestNodeConfigValidation(t *testing.T) { + t.Run("env", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + node: + env: production`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("optional", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + node: {}`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("bad", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + node: + env: foo bar`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `env: "foo bar" is not a valid Node environment name`, msg) + } + }) + }) +} diff --git a/config/reader.go b/config/reader.go index 7aaa22e..1496575 100644 --- a/config/reader.go +++ b/config/reader.go @@ -61,6 +61,12 @@ func ReadConfig(data []byte) (*Config, error) { err := yaml.Unmarshal(data, &config) + if err != nil { + return nil, err + } + + err = Validate(config) + return &config, err } diff --git a/config/runs.go b/config/runs.go index 5986112..6769de2 100644 --- a/config/runs.go +++ b/config/runs.go @@ -1,7 +1,7 @@ package config import ( - "strconv" + "fmt" "phabricator.wikimedia.org/source/blubber/build" ) @@ -15,11 +15,11 @@ const LocalLibPrefix = "/opt/lib" // runtime environment. // type RunsConfig struct { - In string `yaml:"in"` // working directory - As string `yaml:"as"` // unprivileged user - UID int `yaml:"uid"` // unprivileged user ID - GID int `yaml:"gid"` // unprivileged group ID - Environment map[string]string `yaml:"environment"` // environment variables + In string `yaml:"in" validate:"omitempty,abspath"` // working directory + As string `yaml:"as" validate:"omitempty,username"` // unprivileged user + UID uint `yaml:"uid"` // unprivileged user ID + GID uint `yaml:"gid"` // unprivileged group ID + Environment map[string]string `yaml:"environment" validate:"envvars"` // environment variables } // Merge takes another RunsConfig and overwrites this struct's fields. All @@ -92,9 +92,9 @@ func (run RunsConfig) InstructionsForPhase(phase build.Phase) []build.Instructio if run.As != "" { runAll.Runs = append(runAll.Runs, build.Run{"groupadd -o -g %s -r", - []string{strconv.Itoa(run.GID), run.As}}, + []string{fmt.Sprint(run.GID), run.As}}, build.Run{"useradd -o -m -d %s -r -g %s -u %s", - []string{run.Home(), run.As, strconv.Itoa(run.UID), run.As}}, + []string{run.Home(), run.As, fmt.Sprint(run.UID), run.As}}, build.Run{"chown %s:%s", []string{run.As, run.As, LocalLibPrefix}}, ) diff --git a/config/runs_test.go b/config/runs_test.go index 3aa8825..0b7d26a 100644 --- a/config/runs_test.go +++ b/config/runs_test.go @@ -11,6 +11,7 @@ import ( func TestRunsConfig(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- + base: foo runs: as: someuser in: /some/directory @@ -28,8 +29,8 @@ func TestRunsConfig(t *testing.T) { assert.Equal(t, "someuser", variant.Runs.As) assert.Equal(t, "/some/directory", variant.Runs.In) - assert.Equal(t, 666, variant.Runs.UID) - assert.Equal(t, 777, variant.Runs.GID) + assert.Equal(t, uint(666), variant.Runs.UID) + assert.Equal(t, uint(777), variant.Runs.GID) assert.Equal(t, map[string]string{"FOO": "bar"}, variant.Runs.Environment) } @@ -100,3 +101,188 @@ func TestRunsConfigInstructions(t *testing.T) { assert.Empty(t, cfg.InstructionsForPhase(build.PhasePostInstall)) }) } + +func TestRunsConfigValidation(t *testing.T) { + t.Run("in", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + in: /foo`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("optional", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: {}`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("non-root", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + in: /`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `in: "/" is not a valid absolute non-root path`, msg) + } + }) + + t.Run("non-root tricky", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + in: /foo/..`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `in: "/foo/.." is not a valid absolute non-root path`, msg) + } + }) + + t.Run("absolute", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + in: foo/bar`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `in: "foo/bar" is not a valid absolute non-root path`, msg) + } + }) + }) + + t.Run("as", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + as: foo-bar.baz`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("optional", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: {}`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("no spaces", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + as: foo bar`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `as: "foo bar" is not a valid user name`, msg) + } + }) + + t.Run("long enough", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + as: fo`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `as: "fo" is not a valid user name`, msg) + } + }) + + t.Run("not root", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + as: root`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `as: "root" is not a valid user name`, msg) + } + }) + }) + + t.Run("environment", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + environment: + foo: bar + f1oo: bar + FOO: bar + foo_fighter: bar + FOO_FIGHTER: bar + _FOO_FIGHTER: bar`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("optional", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: {}`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("bad", func(t *testing.T) { + t.Run("spaces", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + environment: + foo fighter: bar`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `environment: contains invalid environment variable names`, msg) + } + }) + + t.Run("dashes", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + environment: + foo-fighter: bar`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `environment: contains invalid environment variable names`, msg) + } + }) + + t.Run("dots", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + environment: + foo.fighter: bar`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `environment: contains invalid environment variable names`, msg) + } + }) + + t.Run("starts with number", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + runs: + environment: + 1foo: bar`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `environment: contains invalid environment variable names`, msg) + } + }) + }) + }) +} diff --git a/config/validation.go b/config/validation.go new file mode 100644 index 0000000..652a3c8 --- /dev/null +++ b/config/validation.go @@ -0,0 +1,187 @@ +package config + +import ( + "bytes" + "context" + "fmt" + "path" + "reflect" + "regexp" + "strings" + "text/template" + + "github.com/docker/distribution/reference" + "gopkg.in/go-playground/validator.v9" +) + +var ( + // See Debian Policy + // https://www.debian.org/doc/debian-policy/#s-f-source + // https://www.debian.org/doc/debian-policy/#s-f-version + debianPackageName = `[a-z0-9][a-z0-9+.-]+` + debianVersionSpec = `(?:[0-9]+:)?[0-9]+[a-zA-Z0-9\.\+\-~]*` + debianReleaseName = `[a-zA-Z](?:[a-zA-Z0-9\-]*[a-zA-Z0-9]+)?` + debianPackageRegexp = regexp.MustCompile(fmt.Sprintf( + `^%s(?:=%s|/%s)?$`, debianPackageName, debianVersionSpec, debianReleaseName)) + + // See IEEE Std 1003.1-2008 (http://pubs.opengroup.org/onlinepubs/9699919799/) + environmentVariableRegexp = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]+$`) + + // Pattern for valid variant names + variantNameRegexp = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9\-\.]+[a-zA-Z0-9]$`) + + humanizedErrors = map[string]string{ + "abspath": `{{.Field}}: "{{.Value}}" is not a valid absolute non-root path`, + "baseimage": `{{.Field}}: "{{.Value}}" is not a valid base image reference`, + "debianpackage": `{{.Field}}: "{{.Value}}" is not a valid Debian package name`, + "envvars": `{{.Field}}: contains invalid environment variable names`, + "nodeenv": `{{.Field}}: "{{.Value}}" is not a valid Node environment name`, + "required": `{{.Field}}: is required`, + "username": `{{.Field}}: "{{.Value}}" is not a valid user name`, + "variantref": `{{.Field}}: references an unknown variant "{{.Value}}"`, + "variants": `{{.Field}}: contains a bad variant name`, + } + + validatorAliases = map[string]string{ + "nodeenv": "alphanum", + "username": "hostname,ne=root", + } + + validatorFuncs = map[string]validator.FuncCtx{ + "abspath": isAbsNonRootPath, + "baseimage": isBaseImage, + "debianpackage": isDebianPackage, + "envvars": isEnvironmentVariables, + "variantref": isVariantReference, + "variants": hasVariantNames, + } +) + +type ctxKey uint8 + +const rootCfgCtx ctxKey = iota + +// Validate runs all validations defined for config fields against the given +// Config value. If the returned error is not nil, it will contain a +// user-friendly message describing all invalid field values. +// +func Validate(config Config) error { + validate := validator.New() + + validate.RegisterTagNameFunc(resolveYAMLTagName) + + for name, tags := range validatorAliases { + validate.RegisterAlias(name, tags) + } + + for name, f := range validatorFuncs { + validate.RegisterValidationCtx(name, f) + } + + ctx := context.WithValue(context.Background(), rootCfgCtx, config) + + return validate.StructCtx(ctx, config) +} + +// HumanizeValidationError transforms the given validator.ValidationErrors +// into messages more likely to be understood by human beings. +// +func HumanizeValidationError(err error) string { + var message bytes.Buffer + + if err == nil { + return "" + } else if !IsValidationError(err) { + return err.Error() + } + + templates := map[string]*template.Template{} + + for name, tmplString := range humanizedErrors { + if tmpl, err := template.New(name).Parse(tmplString); err == nil { + templates[name] = tmpl + } + } + + for _, ferr := range err.(validator.ValidationErrors) { + if tmpl, ok := templates[ferr.Tag()]; ok { + tmpl.Execute(&message, ferr) + } else if trueErr, ok := err.(error); ok { + message.WriteString(trueErr.Error()) + } + + message.WriteString("\n") + } + + return strings.TrimSpace(message.String()) +} + +// IsValidationError tests whether the given error is a +// validator.ValidationErrors and can be safely iterated over as such. +// +func IsValidationError(err error) bool { + if err == nil { + return false + } else if _, ok := err.(*validator.InvalidValidationError); ok { + return false + } else if _, ok := err.(validator.ValidationErrors); ok { + return true + } + + return false +} + +func hasVariantNames(_ context.Context, fl validator.FieldLevel) bool { + for _, name := range fl.Field().MapKeys() { + if !variantNameRegexp.MatchString(name.String()) { + return false + } + } + + return true +} + +func isAbsNonRootPath(_ context.Context, fl validator.FieldLevel) bool { + value := fl.Field().String() + + return path.IsAbs(value) && path.Base(path.Clean(value)) != "/" +} + +func isBaseImage(_ context.Context, fl validator.FieldLevel) bool { + value := fl.Field().String() + + return reference.ReferenceRegexp.MatchString(value) +} + +func isDebianPackage(_ context.Context, fl validator.FieldLevel) bool { + value := fl.Field().String() + + return debianPackageRegexp.MatchString(value) +} + +func isEnvironmentVariables(_ context.Context, fl validator.FieldLevel) bool { + for _, key := range fl.Field().MapKeys() { + if !environmentVariableRegexp.MatchString(key.String()) { + return false + } + } + + return true +} + +func isVariantReference(ctx context.Context, fl validator.FieldLevel) bool { + cfg := ctx.Value(rootCfgCtx).(Config) + ref := fl.Field().String() + + for name := range cfg.Variants { + if name == ref { + return true + } + } + + return false +} + +func resolveYAMLTagName(field reflect.StructField) string { + return strings.SplitN(field.Tag.Get("yaml"), ",", 2)[0] +} diff --git a/config/validation_test.go b/config/validation_test.go new file mode 100644 index 0000000..0af68cf --- /dev/null +++ b/config/validation_test.go @@ -0,0 +1,17 @@ +package config_test + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "gopkg.in/go-playground/validator.v9" + + "phabricator.wikimedia.org/source/blubber/config" +) + +func TestIsValidationError(t *testing.T) { + assert.False(t, config.IsValidationError(nil)) + assert.False(t, config.IsValidationError(errors.New("foo"))) + assert.True(t, config.IsValidationError(validator.ValidationErrors{})) +} diff --git a/config/variant.go b/config/variant.go index 60fa325..2f7be21 100644 --- a/config/variant.go +++ b/config/variant.go @@ -7,9 +7,9 @@ import ( // VariantConfig holds configuration fields for each defined build variant. // type VariantConfig struct { - Includes []string `yaml:"includes"` // references to one or more - Copies string `yaml:"copies"` // copy standard artifacts from another variant - Artifacts []ArtifactsConfig `yaml:"artifacts"` // non-standard artifact configuration + Includes []string `yaml:"includes" validate:"dive,variantref"` // other variants + Copies string `yaml:"copies" validate:"omitempty,variantref"` // copy artifacts from variant + Artifacts []ArtifactsConfig `yaml:"artifacts" validate:"dive"` // artifact configuration CommonConfig `yaml:",inline"` } diff --git a/config/variant_test.go b/config/variant_test.go index a67739a..89a1e28 100644 --- a/config/variant_test.go +++ b/config/variant_test.go @@ -1,6 +1,7 @@ package config_test import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -11,6 +12,7 @@ import ( func TestVariantConfig(t *testing.T) { cfg, err := config.ReadConfig([]byte(`--- + base: foo variants: build: {} production: @@ -162,3 +164,75 @@ func TestVariantConfigInstructions(t *testing.T) { }) }) } + +func TestVariantConfigValidation(t *testing.T) { + t.Run("includes", func(t *testing.T) { + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: { includes: [build] }`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("optional", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: {}`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("bad", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: { includes: [build, foobuild, foo_build] }`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, strings.Join([]string{ + `includes[1]: references an unknown variant "foobuild"`, + `includes[2]: references an unknown variant "foo_build"`, + }, "\n"), msg) + } + }) + }) + + t.Run("copies", func(t *testing.T) { + + t.Run("ok", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: { copies: build }`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("optional", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: {}`)) + + assert.False(t, config.IsValidationError(err)) + }) + + t.Run("bad", func(t *testing.T) { + _, err := config.ReadConfig([]byte(`--- + variants: + build: {} + foo: { copies: foobuild }`)) + + if assert.True(t, config.IsValidationError(err)) { + msg := config.HumanizeValidationError(err) + + assert.Equal(t, `copies: references an unknown variant "foobuild"`, msg) + } + }) + }) +} @@ -26,8 +26,13 @@ func main() { cfg, err := config.ReadConfigFile(os.Args[1]) if err != nil { - log.Printf("Error reading config: %v\n", err) - os.Exit(2) + if config.IsValidationError(err) { + log.Printf("Your config is invalid:\n%v", config.HumanizeValidationError(err)) + os.Exit(4) + } else { + log.Printf("Error reading config: %v\n", err) + os.Exit(2) + } } dockerFile, err := docker.Compile(cfg, os.Args[2]) |