diff --git a/configs/experiments.go b/configs/experiments.go new file mode 100644 index 0000000000..8af1e951f1 --- /dev/null +++ b/configs/experiments.go @@ -0,0 +1,143 @@ +package configs + +import ( + "fmt" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/experiments" +) + +// sniffActiveExperiments does minimal parsing of the given body for +// "terraform" blocks with "experiments" attributes, returning the +// experiments found. +// +// This is separate from other processing so that we can be sure that all of +// the experiments are known before we process the result of the module config, +// and thus we can take into account which experiments are active when deciding +// how to decode. +func sniffActiveExperiments(body hcl.Body) (experiments.Set, hcl.Diagnostics) { + rootContent, _, diags := body.PartialContent(configFileTerraformBlockSniffRootSchema) + + ret := experiments.NewSet() + + for _, block := range rootContent.Blocks { + content, _, blockDiags := block.Body.PartialContent(configFileExperimentsSniffBlockSchema) + diags = append(diags, blockDiags...) + + attr, exists := content.Attributes["experiments"] + if !exists { + continue + } + + exps, expDiags := decodeExperimentsAttr(attr) + diags = append(diags, expDiags...) + if !expDiags.HasErrors() { + ret = experiments.SetUnion(ret, exps) + } + } + + return ret, diags +} + +func decodeExperimentsAttr(attr *hcl.Attribute) (experiments.Set, hcl.Diagnostics) { + var diags hcl.Diagnostics + + exprs, moreDiags := hcl.ExprList(attr.Expr) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + return nil, diags + } + + var ret = experiments.NewSet() + for _, expr := range exprs { + kw := hcl.ExprAsKeyword(expr) + if kw == "" { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid experiment keyword", + Detail: "Elements of \"experiments\" must all be keywords representing active experiments.", + Subject: expr.Range().Ptr(), + }) + continue + } + + exp, err := experiments.GetCurrent(kw) + switch err := err.(type) { + case experiments.UnavailableError: + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unknown experiment keyword", + Detail: fmt.Sprintf("There is no current experiment with the keyword %q.", kw), + Subject: expr.Range().Ptr(), + }) + case experiments.ConcludedError: + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Experiment has concluded", + Detail: fmt.Sprintf("Experiment %q is no longer available. %s", kw, err.Message), + Subject: expr.Range().Ptr(), + }) + case nil: + // No error at all means it's valid and current. + ret.Add(exp) + + // However, experimental features are subject to breaking changes + // in future releases, so we'll warn about them to help make sure + // folks aren't inadvertently using them in places where that'd be + // inappropriate, particularly if the experiment is active in a + // shared module they depend on. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: fmt.Sprintf("Experimental feature %q is active", exp.Keyword()), + Detail: "Experimental features are subject to breaking changes in future minor or patch releases, based on feedback.\n\nIf you have feedback on the design of this feature, please open a GitHub issue to discuss it.", + Subject: expr.Range().Ptr(), + }) + + default: + // This should never happen, because GetCurrent is not documented + // to return any other error type, but we'll handle it to be robust. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid experiment keyword", + Detail: fmt.Sprintf("Could not parse %q as an experiment keyword: %s.", kw, err.Error()), + Subject: expr.Range().Ptr(), + }) + } + } + return ret, diags +} + +func checkModuleExperiments(m *Module) hcl.Diagnostics { + var diags hcl.Diagnostics + + // When we have current experiments, this is a good place to check that + // the features in question can only be used when the experiments are + // active. Return error diagnostics if a feature is being used without + // opting in to the feature. For example: + /* + if !m.ActiveExperiments.Has(experiments.ResourceForEach) { + for _, rc := range m.ManagedResources { + if rc.ForEach != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Resource for_each is experimental", + Detail: "This feature is currently an opt-in experiment, subject to change in future releases based on feedback.\n\nActivate the feature for this module by adding resource_for_each to the list of active experiments.", + Subject: rc.ForEach.Range().Ptr(), + }) + } + } + for _, rc := range m.DataResources { + if rc.ForEach != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Resource for_each is experimental", + Detail: "This feature is currently an opt-in experiment, subject to change in future releases based on feedback.\n\nActivate the feature for this module by adding resource_for_each to the list of active experiments.", + Subject: rc.ForEach.Range().Ptr(), + }) + } + } + } + */ + + return diags +} diff --git a/configs/experiments_test.go b/configs/experiments_test.go new file mode 100644 index 0000000000..20c8bac683 --- /dev/null +++ b/configs/experiments_test.go @@ -0,0 +1,113 @@ +package configs + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl/v2" + + "github.com/hashicorp/terraform/experiments" +) + +func TestExperimentsConfig(t *testing.T) { + // The experiment registrations are global, so we need to do some special + // patching in order to get a predictable set for our tests. + current := experiments.Experiment("current") + concluded := experiments.Experiment("concluded") + currentExperiments := experiments.NewSet(current) + concludedExperiments := map[experiments.Experiment]string{ + concluded: "Reticulate your splines.", + } + defer experiments.OverrideForTesting(t, currentExperiments, concludedExperiments)() + + t.Run("current", func(t *testing.T) { + parser := NewParser(nil) + mod, diags := parser.LoadConfigDir("testdata/experiments/current") + if got, want := len(diags), 1; got != want { + t.Fatalf("wrong number of diagnostics %d; want %d", got, want) + } + got := diags[0] + want := &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: `Experimental feature "current" is active`, + Detail: "Experimental features are subject to breaking changes in future minor or patch releases, based on feedback.\n\nIf you have feedback on the design of this feature, please open a GitHub issue to discuss it.", + Subject: &hcl.Range{ + Filename: "testdata/experiments/current/current_experiment.tf", + Start: hcl.Pos{Line: 2, Column: 18, Byte: 29}, + End: hcl.Pos{Line: 2, Column: 25, Byte: 36}, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong warning\n%s", diff) + } + if got, want := len(mod.ActiveExperiments), 1; got != want { + t.Errorf("wrong number of experiments %d; want %d", got, want) + } + if !mod.ActiveExperiments.Has(current) { + t.Errorf("module does not indicate current experiment as active") + } + }) + t.Run("concluded", func(t *testing.T) { + parser := NewParser(nil) + _, diags := parser.LoadConfigDir("testdata/experiments/concluded") + if got, want := len(diags), 1; got != want { + t.Fatalf("wrong number of diagnostics %d; want %d", got, want) + } + got := diags[0] + want := &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Experiment has concluded`, + Detail: `Experiment "concluded" is no longer available. Reticulate your splines.`, + Subject: &hcl.Range{ + Filename: "testdata/experiments/concluded/concluded_experiment.tf", + Start: hcl.Pos{Line: 2, Column: 18, Byte: 29}, + End: hcl.Pos{Line: 2, Column: 27, Byte: 38}, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong error\n%s", diff) + } + }) + t.Run("concluded", func(t *testing.T) { + parser := NewParser(nil) + _, diags := parser.LoadConfigDir("testdata/experiments/unknown") + if got, want := len(diags), 1; got != want { + t.Fatalf("wrong number of diagnostics %d; want %d", got, want) + } + got := diags[0] + want := &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Unknown experiment keyword`, + Detail: `There is no current experiment with the keyword "unknown".`, + Subject: &hcl.Range{ + Filename: "testdata/experiments/unknown/unknown_experiment.tf", + Start: hcl.Pos{Line: 2, Column: 18, Byte: 29}, + End: hcl.Pos{Line: 2, Column: 25, Byte: 36}, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong error\n%s", diff) + } + }) + t.Run("invalid", func(t *testing.T) { + parser := NewParser(nil) + _, diags := parser.LoadConfigDir("testdata/experiments/invalid") + if got, want := len(diags), 1; got != want { + t.Fatalf("wrong number of diagnostics %d; want %d", got, want) + } + got := diags[0] + want := &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid expression`, + Detail: `A static list expression is required.`, + Subject: &hcl.Range{ + Filename: "testdata/experiments/invalid/invalid_experiments.tf", + Start: hcl.Pos{Line: 2, Column: 17, Byte: 28}, + End: hcl.Pos{Line: 2, Column: 24, Byte: 35}, + }, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong error\n%s", diff) + } + }) +} diff --git a/configs/module.go b/configs/module.go index bd4182a5cf..a3afcd9fb0 100644 --- a/configs/module.go +++ b/configs/module.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/experiments" ) // Module is a container for a set of configuration constructs that are @@ -25,6 +26,8 @@ type Module struct { CoreVersionConstraints []VersionConstraint + ActiveExperiments experiments.Set + Backend *Backend ProviderConfigs map[string]*Provider ProviderRequirements map[string][]VersionConstraint @@ -53,6 +56,8 @@ type Module struct { type File struct { CoreVersionConstraints []VersionConstraint + ActiveExperiments experiments.Set + Backends []*Backend ProviderConfigs []*Provider ProviderRequirements []*ProviderRequirement @@ -98,6 +103,8 @@ func NewModule(primaryFiles, overrideFiles []*File) (*Module, hcl.Diagnostics) { diags = append(diags, fileDiags...) } + diags = append(diags, checkModuleExperiments(mod)...) + return mod, diags } @@ -124,6 +131,8 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics { m.CoreVersionConstraints = append(m.CoreVersionConstraints, constraint) } + m.ActiveExperiments = experiments.SetUnion(m.ActiveExperiments, file.ActiveExperiments) + for _, b := range file.Backends { if m.Backend != nil { diags = append(diags, &hcl.Diagnostic{ diff --git a/configs/parser_config.go b/configs/parser_config.go index d4cbc945c3..c592dfa456 100644 --- a/configs/parser_config.go +++ b/configs/parser_config.go @@ -42,6 +42,12 @@ func (p *Parser) loadConfigFile(path string, override bool) (*File, hcl.Diagnost file.CoreVersionConstraints, reqDiags = sniffCoreVersionRequirements(body) diags = append(diags, reqDiags...) + // We'll load the experiments first because other decoding logic in the + // loop below might depend on these experiments. + var expDiags hcl.Diagnostics + file.ActiveExperiments, expDiags = sniffActiveExperiments(body) + diags = append(diags, expDiags...) + content, contentDiags := body.Content(configFileSchema) diags = append(diags, contentDiags...) @@ -52,8 +58,9 @@ func (p *Parser) loadConfigFile(path string, override bool) (*File, hcl.Diagnost content, contentDiags := block.Body.Content(terraformBlockSchema) diags = append(diags, contentDiags...) - // We ignore the "terraform_version" attribute here because - // sniffCoreVersionRequirements already dealt with that above. + // We ignore the "terraform_version" and "experiments" attributes + // here because sniffCoreVersionRequirements and + // sniffActiveExperiments already dealt with those above. for _, innerBlock := range content.Blocks { switch innerBlock.Type { @@ -148,7 +155,7 @@ func (p *Parser) loadConfigFile(path string, override bool) (*File, hcl.Diagnost // able to find, but may return no constraints at all if the given body is // so invalid that it cannot be decoded at all. func sniffCoreVersionRequirements(body hcl.Body) ([]VersionConstraint, hcl.Diagnostics) { - rootContent, _, diags := body.PartialContent(configFileVersionSniffRootSchema) + rootContent, _, diags := body.PartialContent(configFileTerraformBlockSniffRootSchema) var constraints []VersionConstraint @@ -213,9 +220,8 @@ var configFileSchema = &hcl.BodySchema{ // a configuration file. var terraformBlockSchema = &hcl.BodySchema{ Attributes: []hcl.AttributeSchema{ - { - Name: "required_version", - }, + {Name: "required_version"}, + {Name: "experiments"}, }, Blocks: []hcl.BlockHeaderSchema{ { @@ -228,8 +234,9 @@ var terraformBlockSchema = &hcl.BodySchema{ }, } -// configFileVersionSniffRootSchema is a schema for sniffCoreVersionRequirements -var configFileVersionSniffRootSchema = &hcl.BodySchema{ +// configFileTerraformBlockSniffRootSchema is a schema for +// sniffCoreVersionRequirements and sniffActiveExperiments. +var configFileTerraformBlockSniffRootSchema = &hcl.BodySchema{ Blocks: []hcl.BlockHeaderSchema{ { Type: "terraform", @@ -245,3 +252,13 @@ var configFileVersionSniffBlockSchema = &hcl.BodySchema{ }, }, } + +// configFileExperimentsSniffBlockSchema is a schema for sniffActiveExperiments, +// to decode a single attribute from inside a "terraform" block. +var configFileExperimentsSniffBlockSchema = &hcl.BodySchema{ + Attributes: []hcl.AttributeSchema{ + { + Name: "experiments", + }, + }, +} diff --git a/configs/testdata/experiments/concluded/concluded_experiment.tf b/configs/testdata/experiments/concluded/concluded_experiment.tf new file mode 100644 index 0000000000..3310638817 --- /dev/null +++ b/configs/testdata/experiments/concluded/concluded_experiment.tf @@ -0,0 +1,3 @@ +terraform { + experiments = [concluded] +} diff --git a/configs/testdata/experiments/current/current_experiment.tf b/configs/testdata/experiments/current/current_experiment.tf new file mode 100644 index 0000000000..d21b660932 --- /dev/null +++ b/configs/testdata/experiments/current/current_experiment.tf @@ -0,0 +1,3 @@ +terraform { + experiments = [current] +} diff --git a/configs/testdata/experiments/invalid/invalid_experiments.tf b/configs/testdata/experiments/invalid/invalid_experiments.tf new file mode 100644 index 0000000000..d3b242b528 --- /dev/null +++ b/configs/testdata/experiments/invalid/invalid_experiments.tf @@ -0,0 +1,3 @@ +terraform { + experiments = invalid +} diff --git a/configs/testdata/experiments/unknown/unknown_experiment.tf b/configs/testdata/experiments/unknown/unknown_experiment.tf new file mode 100644 index 0000000000..bbe36edf48 --- /dev/null +++ b/configs/testdata/experiments/unknown/unknown_experiment.tf @@ -0,0 +1,3 @@ +terraform { + experiments = [unknown] +} diff --git a/experiments/doc.go b/experiments/doc.go new file mode 100644 index 0000000000..5538d739c9 --- /dev/null +++ b/experiments/doc.go @@ -0,0 +1,9 @@ +// Package experiments contains the models and logic for opt-in experiments +// that can be activated for a particular Terraform module. +// +// We use experiments to get feedback on new configuration language features +// in a way that permits breaking changes without waiting for a future minor +// release. Any feature behind an experiment flag is subject to change in any +// way in even a patch release, until we have enough confidence about the +// design of the feature to make compatibility commitments about it. +package experiments diff --git a/experiments/errors.go b/experiments/errors.go new file mode 100644 index 0000000000..a1fdc6f5c4 --- /dev/null +++ b/experiments/errors.go @@ -0,0 +1,26 @@ +package experiments + +import ( + "fmt" +) + +// UnavailableError is the error type returned by GetCurrent when the requested +// experiment is not recognized at all. +type UnavailableError struct { + ExperimentName string +} + +func (e UnavailableError) Error() string { + return fmt.Sprintf("no current experiment is named %q", e.ExperimentName) +} + +// ConcludedError is the error type returned by GetCurrent when the requested +// experiment is recognized as concluded. +type ConcludedError struct { + ExperimentName string + Message string +} + +func (e ConcludedError) Error() string { + return fmt.Sprintf("experiment %q has concluded: %s", e.ExperimentName, e.Message) +} diff --git a/experiments/experiment.go b/experiments/experiment.go new file mode 100644 index 0000000000..fb30a5fc44 --- /dev/null +++ b/experiments/experiment.go @@ -0,0 +1,93 @@ +package experiments + +// Experiment represents a particular experiment, which can be activated +// independently of all other experiments. +type Experiment string + +// All active and defunct experiments must be represented by constants whose +// internal string values are unique. +// +// Each of these declared constants must also be registered as either a +// current or a defunct experiment in the init() function below. +// +// Each experiment is represented by a string that must be a valid HCL +// identifier so that it can be specified in configuration. +const ( +// Example = Experiment("example") +) + +func init() { + // Each experiment constant defined above must be registered here as either + // a current or a concluded experiment. + // registerCurrentExperiment(Example) +} + +// GetCurrent takes an experiment name and returns the experiment value +// representing that expression if and only if it is a current experiment. +// +// If the selected experiment is concluded, GetCurrent will return an +// error of type ConcludedError whose message hopefully includes some guidance +// for users of the experiment on how to migrate to a stable feature that +// succeeded it. +// +// If the selected experiment is not known at all, GetCurrent will return an +// error of type UnavailableError. +func GetCurrent(name string) (Experiment, error) { + exp := Experiment(name) + if currentExperiments.Has(exp) { + return exp, nil + } + + if msg, concluded := concludedExperiments[exp]; concluded { + return Experiment(""), ConcludedError{ExperimentName: name, Message: msg} + } + + return Experiment(""), UnavailableError{ExperimentName: name} +} + +// Keyword returns the keyword that would be used to activate this experiment +// in the configuration. +func (e Experiment) Keyword() string { + return string(e) +} + +// IsCurrent returns true if the receiver is considered a currently-selectable +// experiment. +func (e Experiment) IsCurrent() bool { + return currentExperiments.Has(e) +} + +// IsConcluded returns true if the receiver is a concluded experiment. +func (e Experiment) IsConcluded() bool { + _, exists := concludedExperiments[e] + return exists +} + +// currentExperiments are those which are available to activate in the current +// version of Terraform. +// +// Members of this set are registered in the init function above. +var currentExperiments = make(Set) + +// concludedExperiments are those which were available to activate in an earlier +// version of Terraform but are no longer available, either because the feature +// in question has been implemented or because the experiment failed and the +// feature was abandoned. Each experiment maps to a message describing the +// outcome, so we can give users feedback about what they might do in modules +// using concluded experiments. +// +// After an experiment has been concluded for a whole major release span it can +// be removed, since we expect users to perform upgrades one major release at +// at time without skipping and thus they will see the concludedness error +// message as they upgrade through a prior major version. +// +// Members of this map are registered in the init function above. +var concludedExperiments = make(map[Experiment]string) + +func registerCurrentExperiment(exp Experiment) { + currentExperiments.Add(exp) +} + +func registerConcludedExperiment(exp Experiment, message string) { + concludedExperiments[exp] = message +} diff --git a/experiments/set.go b/experiments/set.go new file mode 100644 index 0000000000..8247e212b5 --- /dev/null +++ b/experiments/set.go @@ -0,0 +1,46 @@ +package experiments + +// Set is a collection of experiments where every experiment is either a member +// or not. +type Set map[Experiment]struct{} + +// NewSet constructs a new Set with the given experiments as its initial members. +func NewSet(exps ...Experiment) Set { + ret := make(Set) + for _, exp := range exps { + ret.Add(exp) + } + return ret +} + +// SetUnion constructs a new Set containing the members of all of the given +// sets. +func SetUnion(sets ...Set) Set { + ret := make(Set) + for _, set := range sets { + for exp := range set { + ret.Add(exp) + } + } + return ret +} + +// Add inserts the given experiment into the set. +// +// If the given experiment is already present then this is a no-op. +func (s Set) Add(exp Experiment) { + s[exp] = struct{}{} +} + +// Remove takes the given experiment out of the set. +// +// If the given experiment not already present then this is a no-op. +func (s Set) Remove(exp Experiment) { + delete(s, exp) +} + +// Has tests whether the given experiment is in the receiving set. +func (s Set) Has(exp Experiment) bool { + _, ok := s[exp] + return ok +} diff --git a/experiments/testing.go b/experiments/testing.go new file mode 100644 index 0000000000..54ff2dfdee --- /dev/null +++ b/experiments/testing.go @@ -0,0 +1,33 @@ +package experiments + +import ( + "testing" +) + +// OverrideForTesting temporarily overrides the global tables +// of experiments in order to allow for a predictable set when unit testing +// the experiments infrastructure code. +// +// The correct way to use this function is to defer a call to its result so +// that the original tables can be restored at the conclusion of the calling +// test: +// +// defer experiments.OverrideForTesting(t, current, concluded)() +// +// This function modifies global variables that are normally fixed throughout +// our execution, so this function must not be called from non-test code and +// any test using it cannot safely run concurrently with other tests. +func OverrideForTesting(t *testing.T, current Set, concluded map[Experiment]string) func() { + // We're not currently using the given *testing.T in here, but we're + // requiring it anyway in case we might need it in future, and because + // it hopefully reinforces that only test code should be calling this. + + realCurrents := currentExperiments + realConcludeds := concludedExperiments + currentExperiments = current + concludedExperiments = concluded + return func() { + currentExperiments = realCurrents + concludedExperiments = realConcludeds + } +}