diff --git a/Makefile b/Makefile index f25b5c3..d57c3f1 100644 --- a/Makefile +++ b/Makefile @@ -79,7 +79,7 @@ clean: .PHONY: test test: - $(GO) test -v -cover ./... + $(GO) test -cover ./... .PHONY: coverage coverage: diff --git a/cache/cache_test.go b/cache/cache_test.go index f5c2fb8..f61c9a4 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -64,22 +64,20 @@ func TestCacheKey(t *testing.T) { Path: cPath("component.yaml"), Docker: definitions.Docker{Image: "repo/img:1.2.3"}, Rules: map[string]definitions.Rule{ - "test": definitions.Rule{ + "test": { Description: "test it!", Inputs: []string{"${NAME}_test.go", "go.mod"}, Ignore: []string{"exclude_me.go"}, Outputs: []string{"test_results"}, Command: "go test -v", }, - "build": definitions.Rule{ + "build": { Description: "build it!", Inputs: []string{"${NAME}.go", "go.mod"}, Ignore: []string{"exclude_me.go"}, Outputs: []string{"foo"}, Command: "go build", - Requires: []definitions.Dependency{ - {Rule: "test"}, - }, + Requires: []definitions.Dependency{{Rule: "test"}}, }, }, Environment: map[string]string{"VOLUME": "11"}, diff --git a/cmd/listInputs.go b/cmd/listInputs.go index 7c528de..ab0d762 100644 --- a/cmd/listInputs.go +++ b/cmd/listInputs.go @@ -53,7 +53,11 @@ func NewListInputsCommand() *cobra.Command { var rows []interface{} for _, c := range comps { - for _, r := range c.Rules() { + for _, ruleName := range c.RuleNames() { + r, err := c.Rule(ruleName, nil) + if err != nil { + fatal(err) + } inputs, err := r.Inputs() if err != nil { fatal(err) diff --git a/cmd/listOutputs.go b/cmd/listOutputs.go index d4290b8..8270099 100644 --- a/cmd/listOutputs.go +++ b/cmd/listOutputs.go @@ -58,7 +58,11 @@ func NewListArtifactsCommand() *cobra.Command { var rows []interface{} var rowColors []*color.Color for _, c := range comps { - for _, r := range c.Rules() { + for _, ruleName := range c.RuleNames() { + r, err := c.Rule(ruleName, nil) + if err != nil { + fatal(err) + } missingOutputs, err := r.MissingOutputs().RelativePaths(projDir) if err != nil { fatal(err) diff --git a/cmd/listRules.go b/cmd/listRules.go index 39c8b3a..e3eb269 100644 --- a/cmd/listRules.go +++ b/cmd/listRules.go @@ -38,7 +38,6 @@ func NewListRulesCommand() *cobra.Command { Short: "List rules in the project", Aliases: []string{"r", "rule", "rules"}, Run: func(cmd *cobra.Command, args []string) { - opts := getZimOptions(cmd, args) proj, err := getProject(opts.Directory) if err != nil { @@ -48,13 +47,12 @@ func NewListRulesCommand() *cobra.Command { if err != nil { fatal(err) } - var rows []interface{} for _, c := range comps { - for _, r := range c.Rules() { + for _, ruleName := range c.RuleNames() { rows = append(rows, listRulesViewItem{ Component: c.Name(), - Rule: r.Name(), + Rule: ruleName, }) } } @@ -66,7 +64,6 @@ func NewListRulesCommand() *cobra.Command { if err != nil { fatal(err) } - for _, tableRow := range table { fmt.Println(tableRow) } diff --git a/cmd/run.go b/cmd/run.go index 989e4ee..5fffccb 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -132,8 +132,17 @@ func NewRunCommand() *cobra.Command { // in order of rule dependencies var schedulerErr error scheduler := sched.NewGraphScheduler() - for _, rule := range opts.Rules { - rules := components.Rules([]string{rule}) + for _, ruleName := range opts.Rules { + var rules []*project.Rule + for _, component := range components { + if component.HasRule(ruleName) { + rule, err := component.Rule(ruleName) + if err != nil { + fatal(err) + } + rules = append(rules, rule) + } + } if len(rules) == 0 { return } diff --git a/cmd/showKey.go b/cmd/showKey.go index f9d965d..064d29d 100644 --- a/cmd/showKey.go +++ b/cmd/showKey.go @@ -83,9 +83,9 @@ func NewShowKeyCommand() *cobra.Command { if c == nil { fatal(fmt.Errorf("Unknown component: %s", componentName)) } - r, found := c.Rule(ruleName) - if !found { - fatal(fmt.Errorf("Unknown rule: %s.%s", componentName, ruleName)) + r, err := c.Rule(ruleName, nil) + if err != nil { + fatal(err) } ctx := context.Background() diff --git a/definitions/component.go b/definitions/component.go index bafb034..972110f 100644 --- a/definitions/component.go +++ b/definitions/component.go @@ -63,7 +63,7 @@ type Component struct { Toolchain Toolchain `yaml:"toolchain"` Rules map[string]Rule `yaml:"rules"` Exports map[string]Export `yaml:"exports"` - Environment map[string]string `yaml:"environment"` + Environment interface{} `yaml:"environment"` Path string } diff --git a/definitions/parameter.go b/definitions/parameter.go new file mode 100644 index 0000000..81d0b51 --- /dev/null +++ b/definitions/parameter.go @@ -0,0 +1,21 @@ +package definitions + +// Parameter is used to configure a Rule +type Parameter struct { + Name string `yaml:"name"` + Description string `yaml:"description"` + Type string `yaml:"type"` + Mode string `yaml:"mode"` + Default interface{} `yaml:"default"` +} + +func mergeParameters(a, b map[string]Parameter) map[string]Parameter { + result := map[string]Parameter{} + for k, v := range a { + result[k] = v + } + for k, v := range b { + result[k] = v + } + return result +} diff --git a/definitions/project.go b/definitions/project.go index d0cf01e..8ce7cc5 100644 --- a/definitions/project.go +++ b/definitions/project.go @@ -14,15 +14,27 @@ package definitions import ( + "fmt" "io/ioutil" "github.com/go-yaml/yaml" ) +// Variable definition for creating a shell environment +type Variable struct { + Definition string + Script string +} + +// Environment contains a list of variables used to define a shell environment +type Environment struct { + Variables []*Variable +} + // Project defines project configuration in YAML type Project struct { Name string `yaml:"name"` - Environment map[string]string `yaml:"environment"` + Environment interface{} `yaml:"environment"` Components []string `yaml:"components"` Providers map[string]map[string]interface{} `yaml:"providers"` } @@ -44,3 +56,57 @@ func LoadProjectFromPath(path string) (*Project, error) { } return LoadProject(data) } + +// GetEnvironment returns the Environment as defined at the Project level +func (p Project) GetEnvironment() (*Environment, error) { + return GetEnvironment(p.Environment) +} + +// GetEnvironment returns one Environment definition from its semi-structured +// YAML form +func GetEnvironment(obj interface{}) (*Environment, error) { + + var variables []*Variable + + switch e := obj.(type) { + + case []interface{}: + for _, item := range e { + if itemStr, ok := item.(string); ok { + variables = append(variables, &Variable{Definition: itemStr}) + } else if itemMap, ok := item.(map[interface{}]interface{}); ok { + if len(itemMap) != 1 { + return nil, fmt.Errorf("invalid environment item: %v", itemMap) + } + key, value := getOneKeyValue(itemMap) + keyStr, ok := key.(string) + if !ok { + return nil, fmt.Errorf("environment key must be a string: %v", key) + } + valueMap, ok := value.(map[interface{}]interface{}) + if !ok { + return nil, fmt.Errorf("environment value is invalid: %v", value) + } + script, ok := valueMap["run"].(string) + if !ok { + return nil, fmt.Errorf("environment variable does not define a run statement: %v", keyStr) + } + variables = append(variables, &Variable{ + Definition: keyStr, + Script: script, + }) + } else { + return nil, fmt.Errorf("invalid environment definition: %v", e) + } + } + + case map[interface{}]interface{}: + for k, v := range e { + variables = append(variables, &Variable{Definition: fmt.Sprintf("%v=%v", k, v)}) + } + + default: + return nil, fmt.Errorf("invalid environment definition: %v", e) + } + return &Environment{Variables: variables}, nil +} diff --git a/definitions/project_test.go b/definitions/project_test.go new file mode 100644 index 0000000..0f088bd --- /dev/null +++ b/definitions/project_test.go @@ -0,0 +1,32 @@ +package definitions + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEnvironment(t *testing.T) { + obj := []interface{}{ + "foo", + "bar=fizzle biz", + map[interface{}]interface{}{ + "ACCOUNT": map[interface{}]interface{}{ + "run": "aws sts get-caller-identity", + }, + }, + } + env, err := GetEnvironment(obj) + require.Nil(t, err) + require.Len(t, env.Variables, 3) + + var1 := env.Variables[0] + var2 := env.Variables[1] + var3 := env.Variables[2] + + assert.Equal(t, "foo", var1.Definition) + assert.Equal(t, "bar=fizzle biz", var2.Definition) + assert.Equal(t, "ACCOUNT", var3.Definition) + assert.Equal(t, "aws sts get-caller-identity", var3.Script) +} diff --git a/definitions/rule.go b/definitions/rule.go index 0a0776d..585acbb 100644 --- a/definitions/rule.go +++ b/definitions/rule.go @@ -19,10 +19,11 @@ import ( // Dependency between Rules type Dependency struct { - Component string `yaml:"component"` - Rule string `yaml:"rule"` - Export string `yaml:"export"` - Recurse int `yaml:"recurse"` + Component string `yaml:"component"` + Rule string `yaml:"rule"` + Export string `yaml:"export"` + Parameters map[string]interface{} `yaml:"parameters"` + Recurse int `yaml:"recurse"` } // Providers specifies the name of the Provider type to be used for the @@ -53,19 +54,20 @@ type Condition struct { // Rule defines inputs, commands, and outputs for a build step or action type Rule struct { - Name string `yaml:"name"` - Inputs []string `yaml:"inputs"` - Outputs []string `yaml:"outputs"` - Ignore []string `yaml:"ignore"` - Local bool `yaml:"local"` - Native bool `yaml:"native"` - Requires []Dependency `yaml:"requires"` - Description string `yaml:"description"` - Command string `yaml:"command"` - Commands []interface{} `yaml:"commands"` - Providers Providers `yaml:"providers"` - When Condition `yaml:"when"` - Unless Condition `yaml:"unless"` + Name string `yaml:"name"` + Inputs []string `yaml:"inputs"` + Outputs []string `yaml:"outputs"` + Ignore []string `yaml:"ignore"` + Local bool `yaml:"local"` + Native bool `yaml:"native"` + Requires []Dependency `yaml:"requires"` + Description string `yaml:"description"` + Command string `yaml:"command"` + Commands []interface{} `yaml:"commands"` + Providers Providers `yaml:"providers"` + When Condition `yaml:"when"` + Unless Condition `yaml:"unless"` + Parameters map[string]Parameter `yaml:"parameters"` } // GetCommands returns commands unmarshaled from the rule's semi-structured YAML @@ -158,8 +160,9 @@ func mergeRule(a, b Rule) Rule { Inputs: mergeStr(a.Providers.Inputs, b.Providers.Inputs), Outputs: mergeStr(a.Providers.Outputs, b.Providers.Outputs), }, - When: mergeConditions(a.When, b.When), - Unless: mergeConditions(a.Unless, b.Unless), + When: mergeConditions(a.When, b.When), + Unless: mergeConditions(a.Unless, b.Unless), + Parameters: mergeParameters(a.Parameters, b.Parameters), } // Precedence for commands: diff --git a/envsub/envsub.go b/envsub/envsub.go new file mode 100644 index 0000000..cbe756f --- /dev/null +++ b/envsub/envsub.go @@ -0,0 +1,102 @@ +package envsub + +import ( + "fmt" + "strings" + + "github.com/drone/envsubst" +) + +// Eval performs variable substitution on parameters that may reference each +// other, and stores the final resolved key-values in the provided state map. +// The state map may already contain resolved key-values, and these may also +// be referenced by the parameters. Variable references must follow the envsubt +// format, e.g. ${var}. +func Eval( + state map[string]interface{}, + parameters map[string]interface{}, +) (finalErr error) { + + // Used to prevent infinite recursion + seen := map[string]bool{} + + // Declaration to support recursion + var eval func(s string) string + + // Evaluate a string, returning the final value with all variables resolved. + // Calls itself recursively when there are chains of variable references. + eval = func(s string) string { + result, err := envsubst.Eval(s, func(key string) string { + if stateValue, found := state[key]; found { + return fmt.Sprintf("%v", stateValue) + } + // Return early if recursion is detected + if seen[key] { + finalErr = fmt.Errorf("recursion detected") + return "" + } + seen[key] = true + + paramValue, found := parameters[key] + if !found { + finalErr = fmt.Errorf("unknown variable: %s", key) + return "" + } + resolvedValue := eval(fmt.Sprintf("%v", paramValue)) + state[key] = resolvedValue + return resolvedValue + }) + if err != nil { + finalErr = err + return "" + } + return result + } + + // Resolve each string parameter and store its resolved value in the state + for name, value := range parameters { + if s, ok := value.(string); ok { + state[name] = eval(s) + } else { + state[name] = value + } + } + return +} + +// EvalString performs variable substitution on the given string, using the +// parameters as allowed values for substitution. +func EvalString(input string, parameters map[string]interface{}) (string, error) { + if input == "" || !strings.Contains(input, "${") { + return input, nil + } + state := map[string]interface{}{} + parameters["__input__"] = input + if err := Eval(state, parameters); err != nil { + return "", err + } + return state["__input__"].(string), nil +} + +// EvalStrings performs variable substitution on all the given strings, using +// the parameters as allowed values for substitution. +func EvalStrings(inputs []string, parameters map[string]interface{}) ([]string, error) { + results := make([]string, 0, len(inputs)) + for _, input := range inputs { + result, err := EvalString(input, parameters) + if err != nil { + return nil, err + } + results = append(results, result) + } + return results, nil +} + +// GenericMap transforms a string map to a map of interfaces +func GenericMap(m map[string]string) map[string]interface{} { + result := map[string]interface{}{} + for k, v := range m { + result[k] = v + } + return result +} diff --git a/envsub/envsub_test.go b/envsub/envsub_test.go new file mode 100644 index 0000000..16f8bff --- /dev/null +++ b/envsub/envsub_test.go @@ -0,0 +1,118 @@ +package envsub + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestEvalBasic(t *testing.T) { + + state := map[string]interface{}{} + + params := map[string]interface{}{ + "age": 32, + "name": "fred", + "description": "${name} age ${age}", + } + + err := Eval(state, params) + require.Nil(t, err) + + require.Equal(t, state, map[string]interface{}{ + "age": 32, + "name": "fred", + "description": "fred age 32", + }) +} + +func TestEvalChain(t *testing.T) { + + state := map[string]interface{}{} + + params := map[string]interface{}{ + "a": "A", + "b": "B", + "c": "C", + "alpha": "${a} ${b} ${c}", + "repeat": "|${alpha}|${alpha}|", + } + + err := Eval(state, params) + require.Nil(t, err) + + require.Equal(t, state, map[string]interface{}{ + "a": "A", + "b": "B", + "c": "C", + "alpha": "A B C", + "repeat": "|A B C|A B C|", + }) +} + +func TestEvalTransforms(t *testing.T) { + + state := map[string]interface{}{ + "a": "apple", + "b": "BANANA", + "c": "CARROT", + } + + params := map[string]interface{}{ + "apple": "Eat an ${a^}", + "banana": "${b,,} for scale", + "carrot": "No carrots-${c/CARROT/Broccoli}-instead!", + } + + err := Eval(state, params) + require.Nil(t, err) + + require.Equal(t, state, map[string]interface{}{ + "a": "apple", + "b": "BANANA", + "c": "CARROT", + "apple": "Eat an Apple", + "banana": "banana for scale", + "carrot": "No carrots-Broccoli-instead!", + }) +} + +func TestEvalRecursion(t *testing.T) { + + state := map[string]interface{}{} + + params := map[string]interface{}{ + "a": "${c}", + "b": "bravo", + "c": "${b} ${a}", + } + + err := Eval(state, params) + require.NotNil(t, err) + require.Equal(t, "recursion detected", err.Error()) +} + +func TestEvalUnknownVariable(t *testing.T) { + + state := map[string]interface{}{} + + params := map[string]interface{}{ + "a": "alpha", + "b": "bravo", + "c": "${a} ${b} ${WHAT}", + } + + err := Eval(state, params) + require.NotNil(t, err) + require.Equal(t, "unknown variable: WHAT", err.Error()) +} + +func TestEvalString(t *testing.T) { + input := "Prefix ${NAME} ${FOO} Suffix" + result, err := EvalString(input, map[string]interface{}{ + "FOO": "FOO", + "NAME": "EAGLE", + }) + require.Nil(t, err) + require.Equal(t, "Prefix EAGLE FOO Suffix", result) +} diff --git a/go.mod b/go.mod index 8d95db6..c28c0fb 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/docker/docker v1.13.1 github.com/docker/go-connections v0.4.0 // indirect github.com/docker/go-units v0.4.0 // indirect + github.com/drone/envsubst v1.0.2 github.com/fatih/color v1.7.0 github.com/fatih/structs v1.1.0 github.com/go-yaml/yaml v2.1.0+incompatible diff --git a/go.sum b/go.sum index 24bdb76..7a13692 100644 --- a/go.sum +++ b/go.sum @@ -49,6 +49,8 @@ github.com/docker/go-connections v0.4.0 h1:El9xVISelRB7BuFusrZozjnkIM5YnzCViNKoh github.com/docker/go-connections v0.4.0/go.mod h1:Gbd7IOopHjR8Iph03tsViu4nIes5XhDvyHbTtUxmeec= github.com/docker/go-units v0.4.0 h1:3uh0PgVws3nIA0Q+MwDC8yjEPf9zjRfZZWXZYDct3Tw= github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= +github.com/drone/envsubst v1.0.2 h1:dpYLMAspQHW0a8dZpLRKe9jCNvIGZPhCPrycZzIHdqo= +github.com/drone/envsubst v1.0.2/go.mod h1:bkZbnc/2vh1M12Ecn7EYScpI4YGYU0etwLJICOWi8Z0= github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/structs v1.1.0 h1:Q7juDM0QtcnhCpeyLGQKyg4TOIghuNXrkL32pHAUMxo= diff --git a/paramterized_rules.md b/paramterized_rules.md new file mode 100644 index 0000000..3c47c15 --- /dev/null +++ b/paramterized_rules.md @@ -0,0 +1,178 @@ +# Proposal: Parameterized Rules and Dynamic Variables + +Rules may define parameters that are resolved at runtime. These parameters are +then accessible in the rule definition and rule scripts, similar to how the +environment variable mechanism behaves today. Parameters may define a default +value or, if they do not, the parameter is understood to be required. When one +rule depends on another, arguments may be supplied to configure the dependee. +A mechanism is provided to resolve parameters from environment variables on the +host machine, where the environment variable name may or may not be the same as +the parameter name. + +A new `variables` section in the component definition supports more flexible +variable resolution from the environment and from running scripts. Variables +defined here may subsequently be referenced by rules, as default values for +parameters as an example. + +## Thoughts + + * The resolved parameters factor into the rule identity and their cache key. + * Users should have the option to "connect" a parameter to an environment + variable on the host system. + * Parameter values may also be passed explicitly via the "with" keyword as + part of a requires statement. + * Users should have the option to declare variables as global, so that they + are resolved only once across multiple components. Conflicting definitions + should result in an error or warning perhaps. + * Extracting key-values from JSON environment variables could be helpful. + * Variables referenced by a rule should be resolved immediately before use. + * A derived component should be able to override parameter defaults. + * Consider how conditional behavior should be introduced. Via a switch statement + mechanism perhaps. + * Parameters and their values should be shown to the user during execution by + default. This behavior can be opted out of via `show_parameters: false`. + * Parameters can be marked as sensitive with `sensitive: true` to avoid printing + or otherwise capturing the parameter value in plaintext. + +## Questions and Possible Answers + + * Do we use the existing `environment` map as part of this? + * No, because the environment variables are passed to all rules today, while + in this situation we want to control whether a rule uses a variable. + * Default to "easy to use" mode, or default to "isolated builds" mode? + * We need to maintain the current behavior as defaults. Perhaps opting into + isolated builds on native executions via `isolated: true`. + +## Example + +```yaml +toolchain: + items: + - name: Go + command: go version + +# Pre-existing environment feature. These strings are passed to every rule in +# this component via environment variables. They are not dynamic. +environment: + GO_BUILD_OPTS: -mod=readonly -ldflags="-s -w" + +# New variables feature. These are resolved immediately before being used by a +# specific rule only. A rule references these by having a parameter defined that +# has `env: VAR_NAME` set. When VAR_NAME matches a variable named defined here, +# the variable is understood to be referenced. Consequently, these variables are +# effectively the same as Makefile variables, which act as environment variables +# when used in rules, and which may take on values from the host's shell. +variables: + - name: KMS_KEY_ARN + run: ./key-retrieval-script.sh # Run a script to resolve the variable + global: true # Cache this variable across components + - name: S3_BUCKET + run: ./bucket-retrieval-script.sh + global: true + - name: AWS_PROFILE + default: default # Lowest precedence (if otherwise unspecified) + env: AWS_PROFILE # Higher precedence (if set in the environment) + - name: S3_BUCKET_COMMENT + value: The bucket is ${S3_BUCKET} # Example of referencing another variable + +rules: + build: + docker: + image: myimage/foo:1 + inputs: + - go.mod + - go.sum + - "**/*.go" + ignore: + - "**/*_test.go" + outputs: + - ${NAME}.zip + commands: + - cleandir: build + - run: GO111MODULE=on go build ${GO_BUILD_OPTS} -o build/${NAME} + - zip: + cd: build + output: ../${OUTPUT} + + package: + native: true # Run on the host rather than in a container + isolated: true # Don't pass through env variables from the host shell (NEW) + cached: false # Don't cache the output file (NEW) + local: true # Output file goes into the same directory + inputs: + - cloudformation.yaml + outputs: + - cloudformation_deploy.yaml + requires: + - rule: build + parameters: + region: # Parameter with default values + type: string + default: us-east-1 # Lowest precedence (if otherwise unspecified) + env: AWS_DEFAULT_REGION # Higher precedence (if set in the environment) + profile: + type: string + default: default + env: AWS_PROFILE + kms_key_arn: # Required parameter: no default value + type: string + s3_bucket: # Required parameter: no default value + type: string + commands: + - run: | + aws cloudformation package \ + --region ${region} \ + --profile ${profile} \ + --kms-key-id ${kms_key_arn} \ + --s3-bucket ${s3_bucket} \ + --template-file ${INPUT} \ + --output-template-file ${OUTPUT} + + deploy: + native: true + isolated: true + cached: false + local: true + inputs: + - cloudformation_deploy.yaml + requires: + - rule: package + with: # Example of directly passing arguments to another rule + kms_key_arn: ${kms_key_arn} + s3_bucket: ${s3_bucket} + parameters: + region: + type: string + default: us-east-1 + env: AWS_DEFAULT_REGION + profile: + type: string + default: default + env: AWS_PROFILE + stack_name: + type: string + default: ${NAME} + env_type: + type: string + default: dev + env: ENV_TYPE + kms_key_arn: + type: string + env: KMS_KEY_ARN # Variable reference + s3_bucket: + type: string + env: S3_BUCKET # Variable reference + commands: + - run: | + aws cloudformation deploy \ + --region ${region} \ + --profile ${profile} \ + --s3-bucket ${s3_bucket} \ + --kms-key-id ${kms_key_arn} \ + --template-file cloudformation_deploy.yaml \ + --stack-name ${stack_name} \ + --capabilities CAPABILITY_NAMED_IAM \ + --no-fail-on-empty-changeset \ + --parameter-overrides \ + EnvType=${env_type} +``` diff --git a/project/component.go b/project/component.go index d2e5f70..d3c90a4 100644 --- a/project/component.go +++ b/project/component.go @@ -18,23 +18,24 @@ import ( "fmt" "path" "path/filepath" + "sort" "github.com/fugue/zim/definitions" - "github.com/hashicorp/go-multierror" + "github.com/fugue/zim/envsub" ) // NewComponent initializes a Component from its YAML definition. func NewComponent(p *Project, self *definitions.Component) (*Component, error) { if self == nil { - return nil, errors.New("Component definition is nil") + return nil, errors.New("component definition is nil") } if self.Path == "" { - return nil, errors.New("Component definition path is empty") + return nil, errors.New("component definition path is empty") } absPath, err := filepath.Abs(self.Path) if err != nil { - return nil, fmt.Errorf("Failed to resolve path: %s", err) + return nil, fmt.Errorf("failed to resolve path: %s", err) } componentDir := filepath.Dir(absPath) name := self.Name @@ -43,7 +44,7 @@ func NewComponent(p *Project, self *definitions.Component) (*Component, error) { } relPath, err := filepath.Rel(p.RootAbsPath(), componentDir) if err != nil { - return nil, fmt.Errorf("Failed to determine relative path: %s", err) + return nil, fmt.Errorf("failed to determine relative path: %s", err) } c := Component{ @@ -57,6 +58,7 @@ func NewComponent(p *Project, self *definitions.Component) (*Component, error) { rules: make(map[string]*Rule, len(self.Rules)), exports: make(map[string]*Export, len(self.Exports)), env: self.Environment, + def: self, } for _, item := range self.Toolchain.Items { @@ -83,14 +85,6 @@ func NewComponent(p *Project, self *definitions.Component) (*Component, error) { } } - for name, ruleDef := range self.Rules { - rule, err := NewRule(name, &c, &ruleDef) - if err != nil { - return nil, err - } - c.rules[name] = rule - } - return &c, nil } @@ -111,18 +105,16 @@ type Toolchain struct { type Component struct { project *Project componentDir string - repoDir string relPath string - artifactDir string name string app string kind string dockerImage string - sources []string rules map[string]*Rule exports map[string]*Export env map[string]string toolchain Toolchain + def *definitions.Component } // Project returns the Project that contains this Component @@ -179,41 +171,131 @@ func (c *Component) RelPaths(rs Resources) ([]string, error) { return rs.RelativePaths(c.Directory()) } -// Rule returns the Component rule with the given name, if it exists, -// along with a boolean that indicates whether it was found -func (c *Component) Rule(name string) (r *Rule, found bool) { - r, found = c.rules[name] - return +// RuleName returns the rule name for the given rule configuration +func (c *Component) RuleName(name string, parameters map[string]interface{}) string { + if len(parameters) == 0 { + return name + } + parameterNames := make([]string, 0, len(parameters)) + for k := range parameters { + parameterNames = append(parameterNames, k) + } + sort.Strings(parameterNames) + result := name + for _, parameterName := range parameterNames { + result += fmt.Sprintf("%s=%v", parameterName, parameters[parameterName]) + } + return result } -// MustRule returns the named rule or panics if it is not found -func (c *Component) MustRule(name string) *Rule { - r, found := c.rules[name] +// HasRule returns true if the Component has a Rule defined with the given name +func (c *Component) HasRule(name string) bool { + _, found := c.def.Rules[name] + return found +} + +// Rule returns the Component rule with the given name if it exists +func (c *Component) Rule( + name string, + optParameters ...map[string]interface{}, +) (*Rule, error) { + + ruleDef, found := c.def.Rules[name] if !found { - panic(fmt.Sprintf("Component %s has no rule %s", c.Name(), name)) + return nil, fmt.Errorf("unknown rule: %s.%s", c.Name(), name) + } + var parameters map[string]interface{} + if len(optParameters) > 0 { + parameters = optParameters[0] + } + // Raise an error if unknown parameters were supplied + for paramName := range parameters { + if _, found := ruleDef.Parameters[paramName]; !found { + return nil, fmt.Errorf("unknown parameter for %s.%s: %s", + c.Name(), name, paramName) + } + } + // Build up state available to variable substitution + state := map[string]interface{}{ + "COMPONENT": c.Name(), + "NAME": c.Name(), + "KIND": c.Kind(), + "RULE": name, } - return r + for paramName, value := range parameters { + state[paramName] = value + } + + // Resolve the value for each parameter + values := map[string]interface{}{} + for pName, param := range ruleDef.Parameters { + value, ok := parameters[pName] + if ok { + values[pName] = value + } else if param.Default != nil { + values[pName] = param.Default + } else { + return nil, fmt.Errorf("required parameter was not set for %s.%s: %s", + c.Name(), name, pName) + } + if err := typeCheckParameter(param.Type, values[pName]); err != nil { + return nil, fmt.Errorf("incorrect parameter type for %s.%s %s: %w", + c.Name(), name, pName, err) + } + if strValue, ok := values[pName].(string); ok { + subValue, err := envsub.EvalString(strValue, state) + if err != nil { + return nil, fmt.Errorf("failed to resolve parameter %s in %s.%s: %w", + pName, c.Name(), name, err) + } + values[pName] = subValue + } + } + + // Determine rule name including its parameters + fullName := RuleName(name, values) + rule, found := c.rules[fullName] + if found { + return rule, nil + } + rule, err := NewRule(name, fullName, values, c, &ruleDef) + if err != nil { + return nil, fmt.Errorf("failed to create rule instance: %w", err) + } + c.rules[fullName] = rule + + if err := rule.resolveDeps(); err != nil { + return nil, err + } + return rule, nil } -// Rules returns a slice containing all Rules defined by this Component -func (c *Component) Rules() []*Rule { - rules := make([]*Rule, 0, len(c.rules)) - for _, r := range c.rules { - rules = append(rules, r) +// MustRule returns the named rule or panics if it is not found +func (c *Component) MustRule(name string, parameters ...map[string]interface{}) *Rule { + rule, err := c.Rule(name, parameters...) + if err != nil { + panic(err) } - return rules + return rule } -// HasRule returns true if a Rule with the given name is defined -func (c *Component) HasRule(name string) bool { - _, found := c.rules[name] - return found +// RuleNames returns a list of all rule names defined in this Component +func (c *Component) RuleNames() []string { + var names []string + for name := range c.def.Rules { + names = append(names, name) + } + sort.Strings(names) + return names } // Export returns the Component export with the given name, if it exists -func (c *Component) Export(name string) (e *Export, found bool) { - e, found = c.exports[name] - return +func (c *Component) Export(name string) (*Export, error) { + export, found := c.exports[name] + if !found { + return nil, fmt.Errorf("unknown export %s from component %s", name, c.Name()) + } + return export, nil } // Exports returns a slice containing all Exports defined by this Component @@ -225,32 +307,14 @@ func (c *Component) Exports() []*Export { return exports } -// Select finds Rules belonging to this Component with the provided names. -// Unknown names are just ignored. -func (c *Component) Select(names []string) (result []*Rule) { - for _, name := range names { - if r, exists := c.Rule(name); exists { - result = append(result, r) - } - } - return -} - // Environment returns environment variables applicable to this Component func (c *Component) Environment() map[string]string { // Return a copy so that the original map cannot be modified - return copyEnvironment(c.env) -} - -// resolveDeps processes inter-rule dependencies -func (c *Component) resolveDeps() error { - var result *multierror.Error - for _, rule := range c.rules { - if err := rule.resolveDeps(); err != nil { - result = multierror.Append(result, err) - } - } - return result.ErrorOrNil() + env := copyEnvironment(c.env) + env["COMPONENT"] = c.name + env["NAME"] = c.name + env["KIND"] = c.kind + return env } // Toolchain returns this Components active toolchain information diff --git a/project/component_test.go b/project/component_test.go index dfa758c..ca495da 100644 --- a/project/component_test.go +++ b/project/component_test.go @@ -20,6 +20,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/fugue/zim/definitions" @@ -31,7 +32,7 @@ func TestNewComponentError(t *testing.T) { if err == nil { t.Fatal("Empty definitions should error") } - if err.Error() != "Component definition path is empty" { + if err.Error() != "component definition path is empty" { t.Fatal("Unexpected error:", err) } } @@ -42,27 +43,13 @@ func TestNewComponent(t *testing.T) { Path: "/repo/foo/component.yaml", } c, err := NewComponent(p, self) - if err != nil { - t.Fatal("Unexpected error:", err) - } - if c == nil { - t.Fatal("Expected a Component to be returned; got nil") - } - if c.Directory() != "/repo/foo" { - t.Error("Incorrect directory:", c.Directory()) - } - if c.Name() != "foo" { - t.Error("Incorrect name:", c.Name()) - } - if len(c.Rules()) != 0 { - t.Error("Expected empty rule names") - } - if c.HasRule("HUH?") { - t.Error("Expected rule not to be found") - } - if _, ruleFound := c.Rule("WHUT"); ruleFound { - t.Error("Expected rule not to be found") - } + require.Nil(t, err) + require.NotNil(t, c) + assert.Equal(t, "/repo/foo", c.Directory()) + assert.Equal(t, "foo", c.Name()) + assert.Len(t, c.RuleNames(), 0) + _, err = c.Rule("WHUT") + assert.NotNil(t, err) } func TestNewComponentEmptyRule(t *testing.T) { @@ -72,32 +59,17 @@ func TestNewComponentEmptyRule(t *testing.T) { providers: map[string]Provider{}, } self := &definitions.Component{ - Path: "/repo/foo/component.yaml", - Rules: map[string]definitions.Rule{ - "build": definitions.Rule{}, - }, + Path: "/repo/foo/component.yaml", + Rules: map[string]definitions.Rule{"build": {}}, } c, _ := NewComponent(p, self) - if c == nil { - t.Fatal("Expected a Component to be returned; got nil") - } - if !c.HasRule("build") { - t.Fatal("Expected build rule to exist") - } - rule, found := c.Rule("build") - if !found { - t.Fatal("Expected build rule to exist") - } - if len(rule.Outputs()) != 0 { - t.Error("Expected empty slices") - } + require.NotNil(t, c) + rule, err := c.Rule("build") + require.Nil(t, err) + require.Len(t, rule.Outputs(), 0) inputs, err := rule.Inputs() - if err != nil { - t.Fatal(err) - } - if len(inputs) != 0 { - t.Error("Expected empty inputs") - } + require.Nil(t, err) + require.Len(t, inputs, 0) } func TestNewComponentRule(t *testing.T) { @@ -118,7 +90,7 @@ func TestNewComponentRule(t *testing.T) { self := &definitions.Component{ Path: cDefPath, Rules: map[string]definitions.Rule{ - "build": definitions.Rule{ + "build": { Description: "build it!", Inputs: []string{"${NAME}.go", "*.go", "go.mod"}, Ignore: []string{"exclude_me.go"}, @@ -129,25 +101,11 @@ func TestNewComponentRule(t *testing.T) { Environment: map[string]string{"VOLUME": "11"}, } c, _ := NewComponent(p, self) - if c == nil { - t.Fatal("Expected a Component to be returned; got nil") - } - - rule, found := c.Rule("build") - if !found { - t.Fatal("Expected build rule to exist") - } - if !reflect.DeepEqual(c.Select([]string{"unknown", "build"}), []*Rule{rule}) { - t.Error("Expected build rule to be selected") - } - if !reflect.DeepEqual(c.Rules(), []*Rule{rule}) { - t.Error("Expected build rule to be present") - } - + require.NotNil(t, c) + rule, err := c.Rule("build") + require.Nil(t, err) inputs, err := rule.Inputs() - if err != nil { - t.Fatalf("Unexpected error: %s", err) - } + require.Nil(t, err) if !reflect.DeepEqual(inputs.Paths(), []string{ path.Join(dir, "src/foo/foo.go"), diff --git a/project/components.go b/project/components.go index 107f86f..e33f9ae 100644 --- a/project/components.go +++ b/project/components.go @@ -13,6 +13,8 @@ // limitations under the License. package project +import "sort" + // Components is a list of Components type Components []*Component @@ -71,30 +73,6 @@ func (comps Components) First() *Component { return nil } -// Rules returns a slice of all Rules with the given names across all -// these Components -func (comps Components) Rules(names []string) Rules { - rules := make(Rules, 0, len(names)*len(comps)) - for _, c := range comps { - for _, t := range c.Select(names) { - rules = append(rules, t) - } - } - return rules -} - -// Rule returns a slice of all Rules with the given name across all -// these Components -func (comps Components) Rule(name string) Rules { - rules := make(Rules, 0, len(comps)) - for _, c := range comps { - if rule, found := c.Rule(name); found { - rules = append(rules, rule) - } - } - return rules -} - // First rule in the list, or nil if the list is empty func (rules Rules) First() *Rule { if len(rules) > 0 { @@ -109,7 +87,6 @@ func (comps Components) FilterNames(names []string) []string { for _, name := range names { namesMap[name] = true } - var filteredNames []string for _, comp := range comps { name := comp.Name() @@ -117,7 +94,6 @@ func (comps Components) FilterNames(names []string) []string { filteredNames = append(filteredNames, name) } } - return filteredNames } @@ -127,44 +103,41 @@ func (comps Components) FilterKinds(kinds []string) []string { for _, kind := range kinds { kindsMap[kind] = true } - seenKinds := make(map[string]bool, len(comps)) var filteredKinds []string for _, comp := range comps { kind := comp.Kind() if !seenKinds[kind] { seenKinds[kind] = true - if !kindsMap[kind] { filteredKinds = append(filteredKinds, kind) } } } - return filteredKinds } // FilterRules returns a slice of component rules minus the given rules func (comps Components) FilterRules(rules []string) []string { - rulesMap := make(map[string]bool, len(rules)) - for _, rule := range rules { - rulesMap[rule] = true - } - - seenRules := make(map[string]bool, len(comps)) - var filteredRules []string + remove := stringSet(rules) + seen := make(map[string]bool, len(comps)) + var filtered []string for _, comp := range comps { - for _, rule := range comp.Rules() { - ruleName := rule.Name() - if !seenRules[ruleName] { - seenRules[ruleName] = true - - if !rulesMap[ruleName] { - filteredRules = append(filteredRules, ruleName) - } + for _, ruleName := range comp.RuleNames() { + if !seen[ruleName] && !remove[ruleName] { + seen[ruleName] = true + filtered = append(filtered, ruleName) } } } + sort.Strings(filtered) + return filtered +} - return filteredRules +func stringSet(slice []string) map[string]bool { + set := make(map[string]bool, len(slice)) + for _, s := range slice { + set[s] = true + } + return set } diff --git a/project/condition.go b/project/condition.go index b79d22d..cd58192 100644 --- a/project/condition.go +++ b/project/condition.go @@ -20,6 +20,8 @@ import ( "os" "path" "strings" + + "github.com/fugue/zim/envsub" ) // ConditionScript defines a shell script to run for a Condition check @@ -105,8 +107,13 @@ func CheckCondition( env map[string]string, ) (bool, error) { + variables := envsub.GenericMap(env) + if c.ResourceExists != "" { - pattern := substituteVars(c.ResourceExists, env) + pattern, err := envsub.EvalString(c.ResourceExists, variables) + if err != nil { + return false, err + } // The "resource exists" condition evaluates to true if one or more resources // match the provided filename or glob pattern resources, err := matchResources(r.Component(), r.inProvider, []string{pattern}) @@ -117,7 +124,10 @@ func CheckCondition( } if c.DirectoryExists != "" { - directoryName := substituteVars(c.DirectoryExists, env) + directoryName, err := envsub.EvalString(c.DirectoryExists, variables) + if err != nil { + return false, err + } dirPath := path.Join(r.Component().Directory(), directoryName) if stat, err := os.Stat(dirPath); err == nil && stat.IsDir() { return true, nil @@ -151,9 +161,11 @@ func CheckCondition( // If "with_output" is set, the condition is met if the script output // exactly matches the expected output. if c.ScriptSucceeds.WithOutput != "" { - requiredOutput := substituteVars(c.ScriptSucceeds.WithOutput, env) - outputStr := strings.TrimSpace(outputBuffer.String()) - return outputStr == requiredOutput, nil + requiredOutput, err := envsub.EvalString(c.ScriptSucceeds.WithOutput, variables) + if err != nil { + return false, err + } + return strings.TrimSpace(outputBuffer.String()) == requiredOutput, nil } return true, nil } diff --git a/project/condition_test.go b/project/condition_test.go index c81bd71..cc58640 100644 --- a/project/condition_test.go +++ b/project/condition_test.go @@ -52,29 +52,29 @@ func TestRuleCondition(t *testing.T) { env := map[string]string{} // "when" condition is true - build, found := comp.Rule("build-when-run") - require.True(t, found) + build, err := comp.Rule("build-when-run") + require.Nil(t, err) conditionsMet, err := CheckConditions(ctx, build, execOpts, executor, env) require.Nil(t, err) require.True(t, conditionsMet) // "when" condition is false - build, found = comp.Rule("build-when-skip") - require.True(t, found) + build, err = comp.Rule("build-when-skip") + require.Nil(t, err) conditionsMet, err = CheckConditions(ctx, build, execOpts, executor, env) require.Nil(t, err) require.False(t, conditionsMet) // "unless" condition is false (so the rule should run) - build, found = comp.Rule("build-unless-run") - require.True(t, found) + build, err = comp.Rule("build-unless-run") + require.Nil(t, err) conditionsMet, err = CheckConditions(ctx, build, execOpts, executor, env) require.Nil(t, err) require.True(t, conditionsMet) // "unless" condition is true (so the rule should NOT run) - build, found = comp.Rule("build-unless-skip") - require.True(t, found) + build, err = comp.Rule("build-unless-skip") + require.Nil(t, err) conditionsMet, err = CheckConditions(ctx, build, execOpts, executor, env) require.Nil(t, err) require.False(t, conditionsMet) diff --git a/project/misc.go b/project/misc.go index ab204cd..e25ab0a 100644 --- a/project/misc.go +++ b/project/misc.go @@ -19,7 +19,6 @@ import ( "os" "path" "sort" - "strings" "time" uuid "github.com/satori/go.uuid" @@ -47,10 +46,8 @@ func XDGCache() string { func combineEnvironment(envs ...map[string]string) map[string]string { result := map[string]string{} for _, env := range envs { - if env != nil { - for k, v := range env { - result[k] = v - } + for k, v := range env { + result[k] = v } } return result @@ -77,13 +74,13 @@ func flattenEnvironment(env map[string]string) []string { func latestModification(files []string) (time.Time, error) { if len(files) == 0 { - return time.Time{}, errors.New("No input files") + return time.Time{}, errors.New("no input files") } var latestMod time.Time for _, fpath := range files { info, err := os.Stat(fpath) if err != nil { - return time.Time{}, fmt.Errorf("Failed to stat %s: %s", fpath, err) + return time.Time{}, fmt.Errorf("failed to stat %s: %s", fpath, err) } if info.ModTime().After(latestMod) { latestMod = info.ModTime() @@ -91,34 +88,3 @@ func latestModification(files []string) (time.Time, error) { } return latestMod, nil } - -func substituteVarsSlice(strings []string, variables map[string]string) []string { - if len(strings) == 0 || len(variables) == 0 { - return strings - } - result := make([]string, len(strings)) - for i, s := range strings { - result[i] = substituteVars(s, variables) - } - return result -} - -func substituteVars(s string, variables map[string]string) string { - if s == "" || len(variables) == 0 { - return s - } - // ${NAME}.zip -> foo.zip - for k, v := range variables { - s = strings.ReplaceAll(s, fmt.Sprintf("${%s}", k), v) - } - return s -} - -func copyStrings(input []string) []string { - if input == nil { - return nil - } - result := make([]string, len(input)) - copy(result, input) - return result -} diff --git a/project/parameters.go b/project/parameters.go new file mode 100644 index 0000000..8dcb833 --- /dev/null +++ b/project/parameters.go @@ -0,0 +1,43 @@ +// Copyright 2020 Fugue, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package project + +import "fmt" + +func typeCheckParameter(typeName string, value interface{}) error { + switch typeName { + case "integer": + if _, ok := value.(int); !ok { + return fmt.Errorf("expected an integer; got %v", value) + } + case "string": + if _, ok := value.(string); !ok { + return fmt.Errorf("expected a string; got %v", value) + } + case "float": + if _, ok := value.(float64); !ok { + if _, ok := value.(int); !ok { + return fmt.Errorf("expected a float; got %v", value) + } + } + case "boolean": + if _, ok := value.(bool); !ok { + return fmt.Errorf("expected a boolean; got %v", value) + } + default: + return fmt.Errorf("unknown parameter type: %s", typeName) + } + return nil +} diff --git a/project/project.go b/project/project.go index e564b96..46339bd 100644 --- a/project/project.go +++ b/project/project.go @@ -24,22 +24,34 @@ import ( "sync" "github.com/fugue/zim/definitions" - "github.com/hashicorp/go-multierror" ) +// Variable definition for creating a shell environment +type Variable struct { + Definition string + Script string +} + +// Environment contains a list of variables used to define a shell environment +type Environment struct { + Variables []*Variable +} + // Project is a collection of Components that can be built and deployed type Project struct { sync.Mutex - name string - root string - rootAbs string - artifacts string - cacheDir string - components []*Component - toolchain map[string]string - providers map[string]Provider - providerOptions map[string]map[string]interface{} - executor Executor + name string + root string + rootAbs string + artifacts string + cacheDir string + components []*Component + componentsByName map[string]*Component + toolchain map[string]string + providers map[string]Provider + providerOptions map[string]map[string]interface{} + executor Executor + environment Environment } // Opts defines options used when initializing a Project @@ -55,7 +67,7 @@ type Opts struct { func New(root string) (*Project, error) { projDef, componentDefs, err := Discover(root) if err != nil { - return nil, fmt.Errorf("Failed to discover components: %s", err) + return nil, fmt.Errorf("failed to discover components: %s", err) } return NewWithOptions(Opts{ Root: root, @@ -70,13 +82,13 @@ func NewWithOptions(opts Opts) (*Project, error) { root := opts.Root rootAbs, err := filepath.Abs(root) if err != nil { - return nil, fmt.Errorf("Failed to resolve path %s: %s", root, err) + return nil, fmt.Errorf("failed to resolve path %s: %s", root, err) } // Create artifacts directory at the root level of the repository artifacts := path.Join(rootAbs, "artifacts") if err := os.MkdirAll(artifacts, 0755); err != nil { - return nil, fmt.Errorf("Failed to artifacts dir %s: %s", + return nil, fmt.Errorf("failed to artifacts dir %s: %s", artifacts, err) } @@ -88,18 +100,30 @@ func NewWithOptions(opts Opts) (*Project, error) { } p := &Project{ - root: root, - rootAbs: rootAbs, - artifacts: artifacts, - cacheDir: XDGCache(), - toolchain: map[string]string{}, - providers: map[string]Provider{}, - providerOptions: map[string]map[string]interface{}{}, - executor: executor, + root: root, + rootAbs: rootAbs, + artifacts: artifacts, + cacheDir: XDGCache(), + toolchain: map[string]string{}, + providers: map[string]Provider{}, + providerOptions: map[string]map[string]interface{}{}, + executor: executor, + componentsByName: map[string]*Component{}, + environment: Environment{}, } if opts.ProjectDef != nil { p.name = opts.ProjectDef.Name + projectEnv, err := opts.ProjectDef.GetEnvironment() + if err != nil { + return nil, fmt.Errorf("failed to get project environment: %w", err) + } + for _, variable := range projectEnv.Variables { + p.environment.Variables = append(p.environment.Variables, &Variable{ + Definition: variable.Definition, + Script: variable.Script, + }) + } } for _, provider := range opts.Providers { @@ -118,24 +142,16 @@ func NewWithOptions(opts Opts) (*Project, error) { for _, def := range opts.ComponentDefs { component, err := NewComponent(p, def) if err != nil { - return nil, fmt.Errorf("Failed to load component %s: %s", def.Name, err) + return nil, fmt.Errorf("failed to load component %s: %s", def.Name, err) } - p.components = append(p.components, component) - } - - // Resolve dependencies between rules and return the project - return p, p.resolveDeps() -} - -// resolveDeps processes dependencies between Rules -func (p *Project) resolveDeps() error { - var result *multierror.Error - for _, c := range p.components { - if err := c.resolveDeps(); err != nil { - result = multierror.Append(result, err) + componentName := component.Name() + if _, found := p.componentsByName[componentName]; found { + return nil, fmt.Errorf("duplicate component name: %s", componentName) } + p.componentsByName[componentName] = component + p.components = append(p.components, component) } - return result.ErrorOrNil() + return p, nil } // Name of the project @@ -196,7 +212,7 @@ func (p *Project) Select(names, kinds []string) (Components, error) { // Check that all the selected component names are valid for name := range selectedByName { if found := availableByName[name]; !found { - return nil, fmt.Errorf("Unknown component: %s", name) + return nil, fmt.Errorf("unknown component: %s", name) } } // Filter the set of components to ones that were selected @@ -209,20 +225,20 @@ func (p *Project) Select(names, kinds []string) (Components, error) { return selected, nil } -// Rule returns the specified Rule and a boolean indicating whether it was found -func (p *Project) Rule(component, ruleName string) (*Rule, bool) { - rule := p.Components().WithName(component).Rule(ruleName).First() - if rule != nil { - return rule, true +// Resolve the dependency, returning the Rule it references +func (p *Project) Resolve(dep *Dependency) (*Rule, error) { + c, found := p.componentsByName[dep.Component] + if !found { + return nil, fmt.Errorf("unknown component: %s", dep.Component) } - return nil, false + return c.Rule(dep.Rule, dep.Parameters) } // Export returns the specified Export and a boolean indicating whether it was found -func (p *Project) Export(component, exportName string) (*Export, bool) { - c := p.Components().WithName(component).First() - if c == nil { - return nil, false +func (p *Project) Export(componentName, exportName string) (*Export, error) { + c, found := p.componentsByName[componentName] + if !found { + return nil, fmt.Errorf("unknown component: %s", componentName) } return c.Export(exportName) } @@ -315,7 +331,7 @@ func (p *Project) Provider(name string) (Provider, error) { case "docker": provider, err = NewDocker() default: - return nil, fmt.Errorf("Unknown provider: %s", name) + return nil, fmt.Errorf("unknown provider: %s", name) } if err != nil { return nil, err diff --git a/project/project_test.go b/project/project_test.go index 697d12b..4adc82d 100644 --- a/project/project_test.go +++ b/project/project_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/fugue/zim/definitions" + "github.com/stretchr/testify/require" ) func testDir(parent ...string) string { @@ -195,30 +196,25 @@ func TestResolveDeps(t *testing.T) { defer os.RemoveAll(dir) defs := []*definitions.Component{ - &definitions.Component{ + { Name: "a", Kind: "python", Path: path.Join(dir, "a", "component.yaml"), Rules: map[string]definitions.Rule{ - "build": definitions.Rule{ + "build": { Description: "a-build", Requires: []definitions.Dependency{ - definitions.Dependency{ - Component: "b", - Rule: "build", - }, + {Component: "b", Rule: "build"}, }, }, }, }, - &definitions.Component{ + { Name: "b", Kind: "go", Path: path.Join(dir, "b", "component.yaml"), Rules: map[string]definitions.Rule{ - "build": definitions.Rule{ - Description: "b-build", - }, + "build": {Description: "b-build"}, }, }, } @@ -232,19 +228,11 @@ func TestResolveDeps(t *testing.T) { t.Fatal("Failed to find component 'a'") } a := matches[0] - rule, found := a.Rule("build") - if !found { - t.Fatal("Failed to find build rule") - } - if rule.NodeID() != "a.build" { - t.Errorf("Incorrect rule node ID: %s", rule.NodeID()) - } + rule, err := a.Rule("build") + require.Nil(t, err) + require.Equal(t, "a.build", rule.NodeID()) deps := rule.Dependencies() - if len(deps) != 1 { - t.Fatalf("Incorrect number of dependencies; got %d expected 1", len(deps)) - } + require.Len(t, deps, 1) dep := deps[0] - if dep.NodeID() != "b.build" { - t.Fatal("Expected dependency to be 'b.build'") - } + require.Equal(t, "b.build", dep.NodeID()) } diff --git a/project/rule.go b/project/rule.go index da19a88..8fdebb4 100644 --- a/project/rule.go +++ b/project/rule.go @@ -16,17 +16,21 @@ package project import ( "fmt" "path" + "sort" "strings" "github.com/fugue/zim/definitions" + "github.com/fugue/zim/envsub" ) // Dependency on another Component (a Rule or an Export) type Dependency struct { - Component string - Rule string - Export string - Recurse int + Component string + Rule string + Export string + Parameters map[string]interface{} + Recurse int + ResolvedRule *Rule } // Command to be run by a Rule @@ -44,10 +48,7 @@ func NewCommands(self *definitions.Rule) (result []*Command, err error) { } // This form is used when the rule has a simple string for a command if len(defCommands) == 0 { - result = []*Command{&Command{ - Kind: "run", - Argument: self.Command, - }} + result = []*Command{{Kind: "run", Argument: self.Command}} return } // Otherwise, the rule has a series of commands @@ -62,69 +63,113 @@ func NewCommands(self *definitions.Rule) (result []*Command, err error) { return } +// Parameter is used to configure a Rule +type Parameter struct { + Name string + Description string + Type string + Default interface{} +} + // Rule is an operation on a Component type Rule struct { - component *Component - name string - local bool - native bool - inputs []string - ignore []string - requires []*Dependency - outputs []string - description string - commands []*Command - resolvedDeps []*Rule - resolvedImports []*Export - inProvider Provider - outProvider Provider - when Condition - unless Condition + component *Component + name string + parameterizedName string + local bool + native bool + inputs []string + ignore []string + requires []*Dependency + outputs []string + description string + commands []*Command + resolvedDeps []*Rule + resolvedImports []*Export + inProvider Provider + outProvider Provider + when Condition + unless Condition + parameters map[string]interface{} } // NewRule constructs a Rule from a provided YAML definition -func NewRule(name string, c *Component, self *definitions.Rule) (*Rule, error) { +func NewRule( + name string, + parameterizedName string, + parameters map[string]interface{}, + c *Component, + self *definitions.Rule, +) (*Rule, error) { commands, err := NewCommands(self) if err != nil { - return nil, fmt.Errorf("Failed to create rule commands: %v", err) + return nil, fmt.Errorf("failed to create rule commands: %v", err) } r := &Rule{ - component: c, - name: name, - description: self.Description, - local: self.Local, - native: self.Native, - inputs: self.Inputs, - ignore: self.Ignore, - outputs: self.Outputs, - commands: commands, - requires: make([]*Dependency, 0, len(self.Requires)), - } + component: c, + name: name, + parameterizedName: parameterizedName, + description: self.Description, + local: self.Local, + native: self.Native, + inputs: self.Inputs, + ignore: self.Ignore, + outputs: self.Outputs, + commands: commands, + requires: make([]*Dependency, 0, len(self.Requires)), + parameters: parameters, + } + variables := envsub.GenericMap(r.BaseEnvironment()) + variables["ARTIFACTS_DIR"] = r.ArtifactsDir() + variables["ROOT"] = c.Project().RootAbsPath() for _, dep := range self.Requires { - r.requires = append(r.requires, &Dependency{ - Component: dep.Component, - Rule: dep.Rule, - Export: dep.Export, - Recurse: dep.Recurse, - }) + ruleDep := &Dependency{ + Component: dep.Component, + Rule: dep.Rule, + Export: dep.Export, + Recurse: dep.Recurse, + Parameters: dep.Parameters, + } + if ruleDep.Component == "" { + ruleDep.Component = c.Name() + } + for k, v := range ruleDep.Parameters { + if vStr, ok := v.(string); ok { + value, err := envsub.EvalString(vStr, variables) + if err != nil { + return nil, err + } + ruleDep.Parameters[k] = value + } + } + r.requires = append(r.requires, ruleDep) } r.inProvider, err = c.Provider(self.Providers.Inputs) if err != nil { - return nil, fmt.Errorf("Rule %s provider error: %s", r.NodeID(), err) + return nil, fmt.Errorf("rule %s provider error: %s", r.NodeID(), err) } r.outProvider, err = c.Provider(self.Providers.Outputs) if err != nil { - return nil, fmt.Errorf("Rule %s provider error: %s", r.NodeID(), err) + return nil, fmt.Errorf("rule %s provider error: %s", r.NodeID(), err) + } + + r.inputs, err = envsub.EvalStrings(r.inputs, variables) + if err != nil { + return nil, fmt.Errorf("failed to evaluate inputs: %w", err) + } + r.outputs, err = envsub.EvalStrings(r.outputs, variables) + if err != nil { + return nil, fmt.Errorf("failed to evaluate outputs: %w", err) + } + r.ignore, err = envsub.EvalStrings(r.ignore, variables) + if err != nil { + return nil, fmt.Errorf("failed to evaluate ignore: %w", err) } - variables := r.BaseEnvironment() - r.inputs = substituteVarsSlice(r.inputs, variables) - r.ignore = substituteVarsSlice(r.ignore, variables) - r.outputs = substituteVarsSlice(r.outputs, variables) r.when = Condition{ ResourceExists: self.When.ResourceExists, DirectoryExists: self.When.DirectoryExists, @@ -152,7 +197,7 @@ func (r *Rule) resolveDeps() error { for _, dep := range r.requires { // A Rule cannot depend on itself if r.Component().Name() == dep.Component && r.Name() == dep.Rule { - return fmt.Errorf("Invalid dep - self reference: %s.%s", + return fmt.Errorf("invalid dependency - self reference: %s.%s", dep.Component, dep.Rule) } // If the dependency is an Export, this Rule is using source exported @@ -171,23 +216,6 @@ func (r *Rule) resolveDeps() error { return err } r.resolvedDeps = append(r.resolvedDeps, depRule) - // Currently it is allowed to pull in transitive dependencies that - // are one step removed as dependencies of this Rule, if desired. - // This can be helpful when the immediate dependency doesn't actually - // fully encapsulate its own dependencies outputs. - if dep.Recurse > 1 { - return fmt.Errorf("Invalid dep - recursion: %s.%s", - dep.Component, dep.Rule) - } else if dep.Recurse == 1 { - // Pull in transitive dependencies that are one step removed - for _, rDep := range depRule.requires { - rDepRule, err := r.resolveDep(rDep) - if err != nil { - return err - } - r.resolvedDeps = append(r.resolvedDeps, rDepRule) - } - } } return nil } @@ -195,17 +223,16 @@ func (r *Rule) resolveDeps() error { // Accepts an export Dependency and returns the Export to which it refers. func (r *Rule) resolveExport(dep *Dependency) (*Export, error) { if dep.Component == "" { - return nil, fmt.Errorf("Invalid dep in %s - component name empty", + return nil, fmt.Errorf("invalid dependency in %s: component name empty", r.NodeID()) } if dep.Component == r.Component().Name() { - return nil, fmt.Errorf("Invalid dep in %s - cannot import from self", + return nil, fmt.Errorf("invalid dependency in %s: cannot import from self", r.NodeID()) } - export, found := r.Component().Project().Export(dep.Component, dep.Export) - if !found { - return nil, fmt.Errorf("Invalid dep in %s - export not found: %s.%s", - r.NodeID(), dep.Component, dep.Export) + export, err := r.Component().Project().Export(dep.Component, dep.Export) + if err != nil { + return nil, fmt.Errorf("invalid dependency in %s: %w", r.NodeID(), err) } return export, nil } @@ -214,32 +241,23 @@ func (r *Rule) resolveExport(dep *Dependency) (*Export, error) { // If the Dependency component name is blank, the component is assumed // to be the one containing this Rule. func (r *Rule) resolveDep(dep *Dependency) (*Rule, error) { - - var depCompName string - if dep.Component == "" { - depCompName = r.Component().Name() - } else { - depCompName = dep.Component - } - - depRule, found := r.Component().Project().Rule(depCompName, dep.Rule) - if !found { - return nil, fmt.Errorf("Invalid dep - rule not found: %s.%s", - depCompName, dep.Rule) + depRule, err := r.Component().Project().Resolve(dep) + if err != nil { + return nil, err } + dep.ResolvedRule = depRule return depRule, nil } // BaseEnvironment returns Rule environment variables that are known upfront func (r *Rule) BaseEnvironment() map[string]string { - c := r.Component() - return combineEnvironment(c.Environment(), map[string]string{ - "COMPONENT": c.Name(), - "NAME": c.Name(), - "KIND": c.Kind(), - "RULE": r.Name(), - "NODE_ID": r.NodeID(), - }) + env := r.Component().Environment() // this copy can be freely modified + env["RULE"] = r.Name() + env["NODE_ID"] = r.NodeID() + for k, v := range r.parameters { + env[k] = fmt.Sprintf("%v", v) + } + return env } // Environment returns variables to be used when executing this Rule @@ -286,8 +304,7 @@ func (r *Rule) Environment() (map[string]string, error) { "DEP": firstDep, "DEPS": strings.Join(relDeps, " "), } - combined := combineEnvironment(r.BaseEnvironment(), tEnv) - return combined, nil + return combineEnvironment(r.BaseEnvironment(), tEnv), nil } // Project containing this Rule @@ -307,7 +324,11 @@ func (r *Rule) Name() string { // NodeID makes Rules adhere to the graph.Node interface func (r *Rule) NodeID() string { - return fmt.Sprintf("%s.%s", r.Component().Name(), r.Name()) + ruleName := r.parameterizedName + if ruleName == "" { + ruleName = r.name + } + return fmt.Sprintf("%s.%s", r.Component().Name(), ruleName) } // Image returns the Docker image used to build this Rule, if configured @@ -373,9 +394,7 @@ func (r *Rule) OutputsExist() bool { // DependencyOutputs returns outputs of this Rule's dependencies func (r *Rule) DependencyOutputs() (outputs Resources) { for _, dep := range r.Dependencies() { - for _, out := range dep.Outputs() { - outputs = append(outputs, out) - } + outputs = append(outputs, dep.Outputs()...) } return } @@ -411,14 +430,14 @@ func (r *Rule) Inputs() (Resources, error) { // Find input resources inputs, err := matchResources(r.Component(), r.inProvider, r.inputs) if err != nil { - return nil, fmt.Errorf("Failed to find input: %s", err) + return nil, fmt.Errorf("failed to find input: %s", err) } add(inputs) // Exclude ignored resources ignored, err := matchResources(r.Component(), r.inProvider, r.ignore) if err != nil { - return nil, fmt.Errorf("Failed ignore: %s", err) + return nil, fmt.Errorf("failed ignore: %s", err) } ignore(ignored) @@ -426,7 +445,7 @@ func (r *Rule) Inputs() (Resources, error) { for _, imp := range r.resolvedImports { imports, err := imp.Resolve() if err != nil { - return nil, fmt.Errorf("Failed to find import: %s", err) + return nil, fmt.Errorf("failed to find import: %s", err) } add(imports) } @@ -451,3 +470,20 @@ func matchResources(c *Component, p Provider, patterns []string) (result Resourc } return } + +// RuleName returns the rule name for the given rule configuration +func RuleName(name string, parameters map[string]interface{}) string { + if len(parameters) == 0 { + return name + } + parameterNames := make([]string, 0, len(parameters)) + for k := range parameters { + parameterNames = append(parameterNames, k) + } + sort.Strings(parameterNames) + result := name + for _, parameterName := range parameterNames { + result += fmt.Sprintf(" %s=%v", parameterName, parameters[parameterName]) + } + return result +} diff --git a/project/rule_test.go b/project/rule_test.go index 095ea10..fed1c20 100644 --- a/project/rule_test.go +++ b/project/rule_test.go @@ -120,11 +120,11 @@ func TestRule(t *testing.T) { assert.Equal(t, "foo", foo.Name()) assert.Equal(t, "bar", bar.Name()) - test, found := foo.Rule("test") - require.True(t, found) + test, err := foo.Rule("test") + require.Nil(t, err) - build, found := foo.Rule("build") - require.True(t, found) + build, err := foo.Rule("build") + require.Nil(t, err) assert.Equal(t, "foo.test", test.NodeID()) assert.Equal(t, "foo.build", build.NodeID()) @@ -171,9 +171,14 @@ func TestRuleMissingDepOutputs(t *testing.T) { "main.go": testGoMain, }) - _, err := New(dir) + p, err := New(dir) + require.Nil(t, err) + + c := p.Components().WithName("bar").First() + require.NotNil(t, c) + + _, err = c.Rule("build") require.NotNil(t, err) - assert.Contains(t, err.Error(), "Invalid dep - rule not found: foo.build") } func TestRuleOutputs(t *testing.T) { @@ -194,13 +199,10 @@ func TestRuleOutputs(t *testing.T) { require.NotNil(t, bar) require.Equal(t, "bar", bar.Name()) - build, found := bar.Rule("build") - require.True(t, found) + build, err := bar.Rule("build") + require.Nil(t, err) outs, err := build.Outputs().RelativePaths(p.RootAbsPath()) require.Nil(t, err) assert.Equal(t, []string{path.Join("artifacts", "bar")}, outs) - - // absOuts := build.OutputsAbs() - // assert.Equal(t, []string{path.Join(dir, "artifacts", "bar")}, absOuts) } diff --git a/proposals/paramterized_rules.md b/proposals/paramterized_rules.md new file mode 100644 index 0000000..3cc9c00 --- /dev/null +++ b/proposals/paramterized_rules.md @@ -0,0 +1,178 @@ +# Proposal: Parameterized Rules and Dynamic Variables + +Rules may define parameters that are resolved at runtime. These parameters are +then accessible in the rule definition and rule scripts, similar to how the +environment variable mechanism behaves today. Parameters may define a default +value or, if they do not, the parameter is understood to be required. When one +rule depends on another, arguments may be supplied to configure the dependee. +A mechanism is provided to resolve parameters from environment variables on the +host machine, where the environment variable name may or may not be the same as +the parameter name. + +A new `variables` section in the component definition supports more flexible +variable resolution from the environment and from running scripts. Variables +defined here may subsequently be referenced by rules, as default values for +parameters as an example. + +## Thoughts + + * The resolved parameters factor into the rule identity and their cache key. + * Users should have the option to "connect" a parameter to an environment + variable on the host system. + * Parameter values may also be passed explicitly via the "with" keyword as + part of a requires statement. + * Users should have the option to declare variables as global, so that they + are resolved only once across multiple components. Conflicting definitions + should result in an error or warning perhaps. + * Extracting key-values from JSON environment variables could be helpful. + * Variables referenced by a rule should be resolved immediately before use. + * A derived component should be able to override parameter defaults. + * Consider how conditional behavior should be introduced. Via a switch statement + mechanism perhaps. + * Parameters and their values should be shown to the user during execution by + default. This behavior can be opted out of via `show_parameters: false`. + * Parameters can be marked as sensitive with `sensitive: true` to avoid printing + or otherwise capturing the parameter value in plaintext. + +## Questions and Possible Answers + + * Do we use the existing `environment` map as part of this? + * No, because the environment variables are passed to all rules today, while + in this situation we want to control whether a rule uses a variable. + * Default to "easy to use" mode, or default to "isolated builds" mode? + * We need to maintain the current behavior as defaults. Perhaps opting into + isolated builds on native executions via `isolated: true`. + +## Example + +```yaml +toolchain: + items: + - name: Go + command: go version + +# Pre-existing environment feature. These strings are passed to every rule in +# this component via environment variables. They are not dynamic. +environment: + GO_BUILD_OPTS: -mod=readonly -ldflags="-s -w" + +# New variables feature. These are resolved immediately before being used by a +# specific rule only. A rule references these by having a parameter defined that +# has `env: VAR_NAME` set. When VAR_NAME matches a variable named defined here, +# the variable is understood to be referenced. Consequently, these variables are +# effectively the same as Makefile variables, which act as environment variables +# when used in rules, and which may take on values from the host's shell. +variables: + KMS_KEY_ARN: + run: ./key-retrieval-script.sh # Run a script to resolve the variable + global: true # Cache this variable across components + S3_BUCKET: + run: ./bucket-retrieval-script.sh + global: true + AWS_PROFILE: + default: default # Lowest precedence (if otherwise unspecified) + env: AWS_PROFILE # Higher precedence (if set in the environment) + S3_BUCKET_COMMENT: + default: The bucket is ${S3_BUCKET} # Example of referencing another variable + +rules: + build: + docker: + image: myimage/foo:1 + inputs: + - go.mod + - go.sum + - "**/*.go" + ignore: + - "**/*_test.go" + outputs: + - ${NAME}.zip + commands: + - cleandir: build + - run: GO111MODULE=on go build ${GO_BUILD_OPTS} -o build/${NAME} + - zip: + cd: build + output: ../${OUTPUT} + + package: + native: true # Run on the host rather than in a container + isolated: true # Don't pass through env variables from the host shell (NEW) + cached: false # Don't cache the output file (NEW) + local: true # Output file goes into the same directory + inputs: + - cloudformation.yaml + outputs: + - cloudformation_deploy.yaml + requires: + - rule: build + parameters: + region: # Parameter with default values + type: string + default: us-east-1 # Lowest precedence (if otherwise unspecified) + env: AWS_DEFAULT_REGION # Higher precedence (if set in the environment) + profile: + type: string + default: default + env: AWS_PROFILE + kms_key_arn: # Required parameter: no default value + type: string + s3_bucket: # Required parameter: no default value + type: string + commands: + - run: | + aws cloudformation package \ + --region ${region} \ + --profile ${profile} \ + --kms-key-id ${kms_key_arn} \ + --s3-bucket ${s3_bucket} \ + --template-file ${INPUT} \ + --output-template-file ${OUTPUT} + + deploy: + native: true + isolated: true + cached: false + local: true + inputs: + - cloudformation_deploy.yaml + requires: + - rule: package + with: # Example of directly passing arguments to another rule + kms_key_arn: ${kms_key_arn} + s3_bucket: ${s3_bucket} + parameters: + region: + type: string + default: us-east-1 + env: AWS_DEFAULT_REGION + profile: + type: string + default: default + env: AWS_PROFILE + stack_name: + type: string + default: ${NAME} + env_type: + type: string + default: dev + env: ENV_TYPE + kms_key_arn: + type: string + env: KMS_KEY_ARN # Variable reference + s3_bucket: + type: string + env: S3_BUCKET # Variable reference + commands: + - run: | + aws cloudformation deploy \ + --region ${region} \ + --profile ${profile} \ + --s3-bucket ${s3_bucket} \ + --kms-key-id ${kms_key_arn} \ + --template-file cloudformation_deploy.yaml \ + --stack-name ${stack_name} \ + --capabilities CAPABILITY_NAMED_IAM \ + --no-fail-on-empty-changeset \ + --parameter-overrides \ + EnvType=${env_type} +``` diff --git a/sched/sched_test.go b/sched/sched_test.go index b1623fb..93f7853 100644 --- a/sched/sched_test.go +++ b/sched/sched_test.go @@ -64,7 +64,13 @@ func TestScheduler(t *testing.T) { ComponentDefs: defs, }) require.Nil(t, err) - buildRules := p.Components().Rules([]string{"build"}) + + var buildRules []*project.Rule + for _, component := range p.Components() { + rule, err := component.Rule("build") + require.Nil(t, err) + buildRules = append(buildRules, rule) + } require.Len(t, buildRules, 2) widget := p.Components().WithName("widget").First() diff --git a/sched/worker_test.go b/sched/worker_test.go index ffca3f0..f3e6ec2 100644 --- a/sched/worker_test.go +++ b/sched/worker_test.go @@ -63,8 +63,8 @@ func TestWorker(t *testing.T) { c := p.Components().WithName("widget").First() require.NotNil(t, c) - build, found := c.Rule("build") - require.True(t, found) + build, err := c.Rule("build") + require.Nil(t, err) runner := project.RunnerFunc(func(ctx context.Context, rule *project.Rule, opts project.RunOpts) (project.Code, error) { assert.Equal(t, build, rule)