From 17bb516f3e0279a768d0568c20eb4c3a0d68c8d7 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Wed, 23 Sep 2020 23:54:54 +0900 Subject: [PATCH] Pass globals to imported jobs Imported variant command's globals (top-level parameters and options) are now merged into the command, when it misses parameters and options with the same names. If any imported global's type conflict with the command's corresponding parameter/option type, it fails early so that the imported command doesn't fail due to unexpected value/type provided. Resolves #29 --- .../lib/lib.variant | 16 +++++ .../main.variant | 3 + .../lib/lib.variant | 16 +++++ .../main.variant | 10 ++++ .../defaultdir/project | 1 + .../lib/lib.variant | 16 +++++ .../main.variant | 10 ++++ .../overridedir/project | 1 + main.go | 4 +- main_test.go | 25 +++++++- pkg/app/load.go | 59 +++++++++++++++++-- 11 files changed, 153 insertions(+), 8 deletions(-) create mode 100644 examples/advanced/nested-import-global-propagation-default/lib/lib.variant create mode 100644 examples/advanced/nested-import-global-propagation-default/main.variant create mode 100644 examples/advanced/nested-import-global-propagation-incompatible-type/lib/lib.variant create mode 100644 examples/advanced/nested-import-global-propagation-incompatible-type/main.variant create mode 100644 examples/advanced/nested-import-global-propagation/defaultdir/project create mode 100644 examples/advanced/nested-import-global-propagation/lib/lib.variant create mode 100644 examples/advanced/nested-import-global-propagation/main.variant create mode 100644 examples/advanced/nested-import-global-propagation/overridedir/project diff --git a/examples/advanced/nested-import-global-propagation-default/lib/lib.variant b/examples/advanced/nested-import-global-propagation-default/lib/lib.variant new file mode 100644 index 0000000..3230b94 --- /dev/null +++ b/examples/advanced/nested-import-global-propagation-default/lib/lib.variant @@ -0,0 +1,16 @@ +option "project-dir" { + description = "Terraform projects directory" + default = "./defaultdir" + type = string +} + +job "terraform plan" { + parameter "project" { + type = string + } + + exec { + command = "bash" + args = ["-c", "echo ${opt.project-dir}/${param.project}"] + } +} diff --git a/examples/advanced/nested-import-global-propagation-default/main.variant b/examples/advanced/nested-import-global-propagation-default/main.variant new file mode 100644 index 0000000..64cafed --- /dev/null +++ b/examples/advanced/nested-import-global-propagation-default/main.variant @@ -0,0 +1,3 @@ +job "nested" { + import = "lib" +} diff --git a/examples/advanced/nested-import-global-propagation-incompatible-type/lib/lib.variant b/examples/advanced/nested-import-global-propagation-incompatible-type/lib/lib.variant new file mode 100644 index 0000000..3230b94 --- /dev/null +++ b/examples/advanced/nested-import-global-propagation-incompatible-type/lib/lib.variant @@ -0,0 +1,16 @@ +option "project-dir" { + description = "Terraform projects directory" + default = "./defaultdir" + type = string +} + +job "terraform plan" { + parameter "project" { + type = string + } + + exec { + command = "bash" + args = ["-c", "echo ${opt.project-dir}/${param.project}"] + } +} diff --git a/examples/advanced/nested-import-global-propagation-incompatible-type/main.variant b/examples/advanced/nested-import-global-propagation-incompatible-type/main.variant new file mode 100644 index 0000000..4d51b19 --- /dev/null +++ b/examples/advanced/nested-import-global-propagation-incompatible-type/main.variant @@ -0,0 +1,10 @@ +option "project-dir" { + # The default works from geodesic shell `projects` folder + default = 1 + description = "Terraform projects directory" + type = number +} + +job "nested" { + import = "lib" +} diff --git a/examples/advanced/nested-import-global-propagation/defaultdir/project b/examples/advanced/nested-import-global-propagation/defaultdir/project new file mode 100644 index 0000000..3933966 --- /dev/null +++ b/examples/advanced/nested-import-global-propagation/defaultdir/project @@ -0,0 +1 @@ +DEFAULT \ No newline at end of file diff --git a/examples/advanced/nested-import-global-propagation/lib/lib.variant b/examples/advanced/nested-import-global-propagation/lib/lib.variant new file mode 100644 index 0000000..3230b94 --- /dev/null +++ b/examples/advanced/nested-import-global-propagation/lib/lib.variant @@ -0,0 +1,16 @@ +option "project-dir" { + description = "Terraform projects directory" + default = "./defaultdir" + type = string +} + +job "terraform plan" { + parameter "project" { + type = string + } + + exec { + command = "bash" + args = ["-c", "echo ${opt.project-dir}/${param.project}"] + } +} diff --git a/examples/advanced/nested-import-global-propagation/main.variant b/examples/advanced/nested-import-global-propagation/main.variant new file mode 100644 index 0000000..e1d97e1 --- /dev/null +++ b/examples/advanced/nested-import-global-propagation/main.variant @@ -0,0 +1,10 @@ +option "project-dir" { + # The default works from geodesic shell `projects` folder + default = "./overridedir" + description = "Terraform projects directory" + type = string +} + +job "nested" { + import = "lib" +} diff --git a/examples/advanced/nested-import-global-propagation/overridedir/project b/examples/advanced/nested-import-global-propagation/overridedir/project new file mode 100644 index 0000000..0ac28cc --- /dev/null +++ b/examples/advanced/nested-import-global-propagation/overridedir/project @@ -0,0 +1 @@ +OVERRIDE \ No newline at end of file diff --git a/main.go b/main.go index 08c23d5..e7122c7 100644 --- a/main.go +++ b/main.go @@ -1,5 +1,7 @@ package variant +import "fmt" + func RunMain(env Env, opts ...Option) error { cmd, path, args := GetPathAndArgsFromEnv(env) @@ -11,7 +13,7 @@ func RunMain(env Env, opts ...Option) error { } })) if err != nil { - panic(err) + return fmt.Errorf("loading command: %w", err) } return m.Run(args, RunOptions{DisableLocking: false}) diff --git a/main_test.go b/main_test.go index 8191367..7fdc60f 100644 --- a/main_test.go +++ b/main_test.go @@ -154,6 +154,27 @@ func TestExamples(t *testing.T) { args: []string{"variant", "test"}, wd: "./examples/advanced/import-multi", }, + { + subject: "nested-import-global-propagation", + args: []string{"variant", "run", "nested", "terraform", "plan", "project"}, + variantName: "", + wd: "./examples/advanced/nested-import-global-propagation", + expectOut: "./overridedir/project\n", + }, + { + subject: "nested-import-global-propagation-default", + args: []string{"variant", "run", "nested", "terraform", "plan", "project"}, + variantName: "", + wd: "./examples/advanced/nested-import-global-propagation-default", + expectOut: "./defaultdir/project\n", + }, + { + subject: "nested-import-global-propagation-incompatible-type", + args: []string{"variant", "run", "nested", "terraform", "plan", "project"}, + variantName: "", + wd: "./examples/advanced/nested-import-global-propagation-incompatible-type", + expectErr: "loading command: merging globals: imported job \"\" has incompatible option \"project-dir\": needs type of cty.Number, encountered cty.String", + }, { subject: "options", variantName: "", @@ -292,8 +313,8 @@ func TestExamples(t *testing.T) { if tc.expectErr != "" { if err == nil { t.Fatalf("Expected error didn't occur") - } else if err.Error() != tc.expectErr { - t.Fatalf("Unexpected error: want %q, got %q\n%s", tc.expectErr, err.Error(), errOut) + } else if d := cmp.Diff(tc.expectErr, err.Error()); d != "" { + t.Fatalf("Unexpected error:\nDIFF:\n%s\nSTDERR:\n%s", d, errOut) } } else if err != nil { t.Fatalf("%+v\n%s", err, errOut) diff --git a/pkg/app/load.go b/pkg/app/load.go index 6b50ffb..e85f928 100644 --- a/pkg/app/load.go +++ b/pkg/app/load.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" + "github.com/hashicorp/hcl/v2/ext/typeexpr" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclparse" @@ -257,6 +259,8 @@ func newApp(app *App, cc *HCL2Config, importDir func(string) (*App, error)) (*Ap var conf *HCL2Config + var globals []JobSpec + jobByName := map[string]JobSpec{} for _, j := range jobs { jobByName[j.Name] = j @@ -288,7 +292,10 @@ func newApp(app *App, cc *HCL2Config, importDir func(string) (*App, error)) (*Ap // // If the user-side has a global parameter/option that has the same name as the library-side, // their types MUST match. - merged := mergeJobs(importedJob, j) + merged, err := mergeParamsAndOpts(importedJob, j) + if err != nil { + return nil, fmt.Errorf("merging globals: %w", err) + } merged.Name = "" @@ -302,6 +309,9 @@ func newApp(app *App, cc *HCL2Config, importDir func(string) (*App, error)) (*Ap } else { // Import the top-level job in the library as the non-top-level job on the user side newJobName = j.Name + + // And merge global parameters and options + globals = append(globals, importedJob) } importedJob.Name = newJobName @@ -316,12 +326,27 @@ func newApp(app *App, cc *HCL2Config, importDir func(string) (*App, error)) (*Ap } } + root := jobByName[""] + + for _, g := range globals { + merged, err := mergeParamsAndOpts(g, root) + if err != nil { + return nil, fmt.Errorf("merging globals: %w", err) + } + + root = *merged + } + + jobByName[""] = root + if conf == nil { conf = cc } app.Config = conf + app.Config.JobSpec = root + app.JobByName = jobByName var newJobs []JobSpec @@ -335,7 +360,7 @@ func newApp(app *App, cc *HCL2Config, importDir func(string) (*App, error)) (*Ap return app, nil } -func mergeJobs(src JobSpec, dst JobSpec) *JobSpec { +func mergeParamsAndOpts(src JobSpec, dst JobSpec) (*JobSpec, error) { paramMap := map[string]Parameter{} optMap := map[string]OptionSpec{} @@ -348,14 +373,38 @@ func mergeJobs(src JobSpec, dst JobSpec) *JobSpec { } for _, p := range src.Parameters { - if _, exists := paramMap[p.Name]; !exists { + if existing, exists := paramMap[p.Name]; !exists { paramMap[p.Name] = p + } else { + exTy, err := typeexpr.TypeConstraint(existing.Type) + if err != nil { + return nil, fmt.Errorf("parsing parameter type: %w", err) + } + toTy, err := typeexpr.TypeConstraint(p.Type) + if err != nil { + return nil, fmt.Errorf("parsing parameter type: %w", err) + } + if exTy != toTy { + return nil, fmt.Errorf("imported job %q has incompatible parameter %q: needs type of %v, encountered %v", src.Name, p.Name, exTy.GoString(), toTy.GoString()) + } } } for _, o := range src.Options { - if _, exists := optMap[o.Name]; !exists { + if existing, exists := optMap[o.Name]; !exists { optMap[o.Name] = o + } else { + exTy, err := typeexpr.TypeConstraint(existing.Type) + if err != nil { + return nil, fmt.Errorf("parsing option type: %w", err) + } + toTy, err := typeexpr.TypeConstraint(o.Type) + if err != nil { + return nil, fmt.Errorf("parsing option type: %w", err) + } + if exTy != toTy { + return nil, fmt.Errorf("imported job %q has incompatible option %q: needs type of %v, encountered %v", src.Name, o.Name, exTy.GoString(), toTy.GoString()) + } } } @@ -375,5 +424,5 @@ func mergeJobs(src JobSpec, dst JobSpec) *JobSpec { dst.Parameters = params dst.Options = opts - return &dst + return &dst, nil }