Apologies for the large PR, but I think in this case it's important t…

…o present all the pieces at the same time.

This PR creates a system for supporting YAML files using different `api_version`s from a single binary. This is implemented by having a separate set of structs for each `(kind,api_version)` pair. Each api_version can change the semantics or existence of YAML fields. The structs are stored in `templates/model/$KIND/$APIVERSION`.

Instructions for adding a new `api_version` are in `templates/model/README.md`, that might be a good starting point for review.

Old versions are automatically converted to new versions before being used: old structs define an Upgrade() method that define how to upgrade them.

There's a registry of `api_version`s in decode.go that defines which versions exist and which structs are associated with each.

To demonstrate that this all actually works, this PR creates a new API version `v1beta1`. The only change in this new version is adding the optional `if` field for each step in spec.yaml (#220).

Alternatives considered to try to avoid this huge copy-pasting of structs:
- Struct embedding to reduce copy-pasta: this would expose a more complicated API to the rest of the code that has to use the structs. It makes creating a new version more painful. It also triggers golang/go#15924 since we use `reflect.StructOf`.
- Use github.com/mitchellh/mapstructure` to avoid so much double-parsing: we'd lose YAML position information, and our users really appreciate getting line numbers on their error messages.
- Use `switch(apiVersion)` within a single struct instead of copy-pasting structs: high risk of accidentally changing the behavior of old API versions and generally creating fragile code.

I've marked files that are mostly copy-pasted so you can mostly skip reviewing them

README.md

@@ -258,15 +258,24 @@ steps:
           with: '{{.whomever}}'
 ```
 
-The `api_version` field currently has only one valid value,
-`cli.abcxyz.dev/v1alpha1`. The field name `api_version` may also be named
-`apiVersion` in old templates, but the newer form `api_version` is preferred.
+#### List of api_versions
+
+The `api_version` field controls the interpretation of the YAML file. Some
+features are only available in more recent versions.
+
+The currently valid versions are:
+
+| api_version                               | Binary versions | Notes                                         |
+| ----------------------------------------- | --------------- | --------------------------------------------- |
+| cli.abcxyz.dev/v1alpha1                   | From 0.0.0      | version                                       |
+| cli.abcxyz.dev/v1beta1 (not released yet) | From 0.2.0      | Adds support for `if` predicates on each step |
 
 #### Template inputs
 
 Typically the CLI user will supply certain values as `--input=inputname=value`
 which will be used by the spec file (such as `whomever` in the preceding
-example).
+example). Alternatively, the user can use `--prompt` rather than `--input` to
+enter values interactively.
 
 A template may not need any inputs, in which case the `inputs` top-level field
 can be omitted.

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/google/cel-go v0.17.1
 	github.com/google/go-cmp v0.5.9
 	github.com/hashicorp/go-getter/v2 v2.2.1
+	github.com/jinzhu/copier v0.4.0
 	github.com/mattn/go-isatty v0.0.19
 	github.com/posener/complete/v2 v2.1.0
 	golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63

go.sum

@@ -36,6 +36,8 @@ github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhE
 github.com/hashicorp/go-safetemp v1.0.0/go.mod h1:oaerMy3BhqiTbVye6QuFhFtIceqFoDHxNAB65b+Rj1I=
 github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek=
 github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
+github.com/jinzhu/copier v0.4.0 h1:w3ciUoD19shMCRargcpm0cm91ytaBhDvuRpz1ODO/U8=
+github.com/jinzhu/copier v0.4.0/go.mod h1:DfbEm0FYsaqBcKcFuvmOZb218JkPGtvSHsKg8S8hyyg=
 github.com/klauspost/compress v1.16.7 h1:2mk3MPGNzKyxErAw8YaohYh69+pa4sIQSC0fPGCFR9I=
 github.com/klauspost/compress v1.16.7/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=

templates/commands/goldentest/record_test.go

@@ -40,7 +40,8 @@ steps:
     params:
       paths: ['.']
 `
-	testYaml := "api_version: 'cli.abcxyz.dev/v1alpha1'"
+	testYaml := `api_version: 'cli.abcxyz.dev/v1alpha1'
+kind: 'GoldenTest'`
 
 	cases := []struct {
 		name                  string

templates/commands/goldentest/test_funcs.go

@@ -24,7 +24,8 @@ import (
 	"syscall"
 
 	"github.com/abcxyz/abc/templates/commands/render"
-	"github.com/abcxyz/abc/templates/model/goldentest"
+	"github.com/abcxyz/abc/templates/model/decode"
+	goldentest "github.com/abcxyz/abc/templates/model/goldentest/v1alpha1"
 )
 
 // TestCase describes a template golden test case.
@@ -103,11 +104,16 @@ func parseTestConfig(path string) (*goldentest.Test, error) {
 	}
 	defer f.Close()
 
-	test, err := goldentest.DecodeTest(f)
+	testI, err := decode.DecodeValidateUpgrade(f, path, decode.KindGoldenTest)
 	if err != nil {
 		return nil, fmt.Errorf("error reading golden test config file: %w", err)
 	}
-	return test, nil
+	out, ok := testI.(*goldentest.Test)
+	if !ok {
+		return nil, fmt.Errorf("internal error: expected golden test config to be of type *goldentest.Test but got %T", testI)
+	}
+
+	return out, nil
 }
 
 // clearTestDir clears a test directory and only keeps the test config file.

templates/commands/goldentest/test_funcs_test.go

@@ -21,7 +21,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/goldentest"
+	goldentest "github.com/abcxyz/abc/templates/model/goldentest/v1alpha1"
 	"github.com/abcxyz/pkg/testutil"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -30,11 +30,10 @@ import (
 func TestParseTestCases(t *testing.T) {
 	t.Parallel()
 
-	validYaml := `api_version: 'cli.abcxyz.dev/v1alpha1'`
+	validYaml := `api_version: 'cli.abcxyz.dev/v1alpha1'
+kind: 'GoldenTest'`
 	invalidYaml := "bad yaml"
-	validTestCase := &goldentest.Test{
-		APIVersion: model.String{Val: "cli.abcxyz.dev/v1alpha1"},
-	}
+	validTestCase := &goldentest.Test{}
 
 	cases := []struct {
 		name         string
@@ -238,10 +237,8 @@ steps:
 		{
 			name: "simple_test_succeeds",
 			testCase: &TestCase{
-				TestName: "test",
-				TestConfig: &goldentest.Test{
-					APIVersion: model.String{Val: "cli.abcxyz.dev/v1alpha1"},
-				},
+				TestName:   "test",
+				TestConfig: &goldentest.Test{},
 			},
 			filesContent: map[string]string{
 				"spec.yaml":                      specYaml,
@@ -260,7 +257,6 @@ steps:
 			testCase: &TestCase{
 				TestName: "test",
 				TestConfig: &goldentest.Test{
-					APIVersion: model.String{Val: "cli.abcxyz.dev/v1alpha1"},
 					Inputs: []*goldentest.InputValue{
 						{
 							Name:  model.String{Val: "input_a"},

templates/commands/render/action_append.go

@@ -18,7 +18,7 @@ import (
 	"context"
 	"strings"
 
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 )
 
 func actionAppend(ctx context.Context, ap *spec.Append, sp *stepParams) error {

templates/commands/render/action_append_test.go

@@ -21,7 +21,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/testutil"
 	"github.com/google/go-cmp/cmp"
 )

templates/commands/render/action_foreach.go

@@ -18,7 +18,7 @@ import (
 	"context"
 
 	"github.com/abcxyz/abc/templates/common"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 )
 
 func actionForEach(ctx context.Context, fe *spec.ForEach, sp *stepParams) error {

templates/commands/render/action_foreach_test.go

@@ -21,7 +21,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/testutil"
 	"github.com/google/go-cmp/cmp"
 )

templates/commands/render/action_gotemplate.go

@@ -18,7 +18,7 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 )
 
 func actionGoTemplate(ctx context.Context, p *spec.GoTemplate, sp *stepParams) error {

templates/commands/render/action_gotemplate_test.go

@@ -19,7 +19,7 @@ import (
 	"testing"
 
 	"github.com/abcxyz/abc/templates/common"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/testutil"
 	"github.com/google/go-cmp/cmp"
 )

templates/commands/render/action_include.go

@@ -23,7 +23,7 @@ import (
 	"path/filepath"
 
 	"github.com/abcxyz/abc/templates/common"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"golang.org/x/exp/maps"
 )
 

templates/commands/render/action_include_test.go

@@ -22,7 +22,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/logging"
 	"github.com/abcxyz/pkg/testutil"
 	"github.com/google/go-cmp/cmp"

templates/commands/render/action_print.go

@@ -19,7 +19,7 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 )
 
 func actionPrint(ctx context.Context, p *spec.Print, sp *stepParams) error {

templates/commands/render/action_print_test.go

@@ -21,7 +21,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/logging"
 	"github.com/abcxyz/pkg/testutil"
 	"github.com/google/go-cmp/cmp"

templates/commands/render/action_regexnamelookup.go

@@ -20,7 +20,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"golang.org/x/exp/maps"
 )
 

templates/commands/render/action_regexnamelookup_test.go

@@ -20,7 +20,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/testutil"
 	"github.com/google/go-cmp/cmp"
 )

templates/commands/render/action_regexreplace.go

@@ -20,7 +20,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 )
 
 // The regex_replace action replaces a regex match (or a subgroup thereof) with

templates/commands/render/action_regexreplace_test.go

@@ -20,7 +20,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/testutil"
 	"github.com/google/go-cmp/cmp"
 )

templates/commands/render/action_stringreplace.go

@@ -18,7 +18,7 @@ import (
 	"context"
 	"strings"
 
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 )
 
 func actionStringReplace(ctx context.Context, sr *spec.StringReplace, sp *stepParams) error {

templates/commands/render/action_stringreplace_test.go

@@ -21,7 +21,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/testutil"
 	"github.com/google/go-cmp/cmp"
 )

templates/commands/render/render.go

@@ -33,7 +33,8 @@ import (
 	"time"
 
 	"github.com/abcxyz/abc/templates/common"
-	"github.com/abcxyz/abc/templates/model/spec"
+	"github.com/abcxyz/abc/templates/model/decode"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/cli"
 	"github.com/abcxyz/pkg/logging"
 	"github.com/hashicorp/go-getter/v2"
@@ -181,7 +182,7 @@ func (c *Command) realRun(ctx context.Context, rp *runParams) (outErr error) {
 		return err
 	}
 
-	spec, err := loadSpecFile(rp.fs, templateDir, specName)
+	spec, err := loadSpecFile(rp.fs, templateDir)
 	if err != nil {
 		return err
 	}
@@ -568,19 +569,24 @@ func executeOneStep(ctx context.Context, step *spec.Step, sp *stepParams) error
 	}
 }
 
-func loadSpecFile(fs common.FS, templateDir, flagSpec string) (*spec.Spec, error) {
-	specPath := filepath.Join(templateDir, flagSpec)
+func loadSpecFile(fs common.FS, templateDir string) (*spec.Spec, error) {
+	specPath := filepath.Join(templateDir, specName)
 	f, err := fs.Open(specPath)
 	if err != nil {
 		return nil, fmt.Errorf("error opening template spec: ReadFile(): %w", err)
 	}
 	defer f.Close()
 
-	spec, err := spec.Decode(f)
+	specI, err := decode.DecodeValidateUpgrade(f, specName, decode.KindTemplate)
 	if err != nil {
 		return nil, fmt.Errorf("error reading template spec file: %w", err)
 	}
 
+	spec, ok := specI.(*spec.Spec)
+	if !ok {
+		return nil, fmt.Errorf("internal error: spec file did not decode to spec.Spec")
+	}
+
 	return spec, nil
 }
 

templates/commands/render/render_test.go

@@ -29,7 +29,7 @@ import (
 
 	"github.com/abcxyz/abc/templates/common"
 	"github.com/abcxyz/abc/templates/model"
-	"github.com/abcxyz/abc/templates/model/spec"
+	spec "github.com/abcxyz/abc/templates/model/spec/v1beta1"
 	"github.com/abcxyz/pkg/cli"
 	"github.com/abcxyz/pkg/logging"
 	"github.com/abcxyz/pkg/testutil"
@@ -284,7 +284,7 @@ steps:
 			wantTemplateContents: map[string]string{
 				"spec.yaml": "this is an unparseable YAML file *&^#%$",
 			},
-			wantErr: "error parsing spec",
+			wantErr: "error parsing file spec.yaml",
 		},
 		{
 			name: "existing_dest_file_with_overwrite_flag_should_succeed",