Skip to content

Commit

Permalink
Experiments supported only in alpha/dev builds
Browse files Browse the repository at this point in the history
We originally introduced the idea of language experiments as a way to get
early feedback on not-yet-proven feature ideas, ideally as part of the
initial exploration of the solution space rather than only after a
solution has become relatively clear.

Unfortunately, our tradeoff of making them available in normal releases
behind an explicit opt-in in order to make it easier to participate in the
feedback process had the unintended side-effect of making it feel okay
to use experiments in production and endure the warnings they generate.
This in turn has made us reluctant to make use of the experiments feature
lest experiments become de-facto production features which we then feel
compelled to preserve even though we aren't yet ready to graduate them
to stable features.

In an attempt to tweak that compromise, here we make the availability of
experiments _at all_ a build-time flag which will not be set by default,
and therefore experiments will not be available in most release builds.

The intent (not yet implemented in this PR) is for our release process to
set this flag only when it knows it's building an alpha release or a
development snapshot not destined for release at all, which will therefore
allow us to still use the alpha releases as a vehicle for giving feedback
participants access to a feature (without needing to install a Go
toolchain) but will not encourage pretending that these features are
production-ready before they graduate from experimental.

Only language experiments have an explicit framework for dealing with them
which outlives any particular experiment, so most of the changes here are
to that generalized mechanism. However, the intent is that non-language
experiments, such as experimental CLI commands, would also in future
check Meta.AllowExperimentalFeatures and gate the use of those experiments
too, so that we can be consistent that experimental features will never
be available unless you explicitly choose to use an alpha release or
a custom build from source code.

Since there are already some experiments active at the time of this commit
which were not previously subject to this restriction, we'll pragmatically
leave those as exceptions that will remain generally available for now,
and so this new approach will apply only to new experiments started in the
future. Once those experiments have all concluded, we will be left with
no more exceptions unless we explicitly choose to make an exception for
some reason we've not imagined yet.

It's important that we be able to write tests that rely on experiments
either being available or not being available, so here we're using our
typical approach of making "package main" deal with the global setting
that applies to Terraform CLI executables while making the layers below
all support fine-grain selection of this behavior so that tests with
different needs can run concurrently without trampling on one another.

As a compromise, the integration tests in the terraform package will
run with experiments enabled _by default_ since we commonly need to
exercise experiments in those tests, but they can selectively opt-out
if they need to by overriding the loader setting back to false again.
  • Loading branch information
apparentlymart committed Jun 17, 2022
1 parent 479c71f commit 5f77d9d
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 37 deletions.
2 changes: 2 additions & 0 deletions commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ func initCommands(
ProviderSource: providerSrc,
ProviderDevOverrides: providerDevOverrides,
UnmanagedProviders: unmanagedProviders,

AllowExperimentalFeatures: ExperimentsAllowed(),
}

// The command list is included in the terraform -help
Expand Down
23 changes: 23 additions & 0 deletions experiments.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package main

// experimentsAllowed can be set to any non-empty string using Go linker
// arguments in order to enable the use of experimental features for a
// particular Terraform build:
// go install -ldflags="-X 'main.experimentsAllowed=yes'"
//
// By default this variable is initialized as empty, in which case
// experimental features are not available.
//
// The Terraform release process should arrange for this variable to be
// set for alpha releases and development snapshots, but _not_ for
// betas, release candidates, or final releases.
//
// (NOTE: Some experimental features predate the rule that experiments
// are available only for alpha/dev builds, and so intentionally do not
// make use of this setting to avoid retracting a previously-documented
// open experiment.)
var experimentsAllowed string

func ExperimentsAllowed() bool {
return experimentsAllowed != ""
}
18 changes: 18 additions & 0 deletions internal/command/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,24 @@ type Meta struct {
// just trusting that someone else did it before running Terraform.
UnmanagedProviders map[addrs.Provider]*plugin.ReattachConfig

// AllowExperimentalFeatures controls whether a command that embeds this
// Meta is permitted to make use of experimental Terraform features.
//
// Set this field only during the initial creation of Meta. If you change
// this field after calling methods of type Meta then the resulting
// behavior is undefined.
//
// In normal code this would be set by package main only in builds
// explicitly marked as being alpha releases or development snapshots,
// making experimental features unavailable otherwise. Test code may
// choose to set this if it needs to exercise experimental features.
//
// Some experiments predated the addition of this setting, and may
// therefore still be available even if this flag is false. Our intent
// is that all/most _future_ experiments will be unavailable unless this
// flag is set, to reinforce that experiments are not for production use.
AllowExperimentalFeatures bool

//----------------------------------------------------------
// Protected: commands can set these
//----------------------------------------------------------
Expand Down
1 change: 1 addition & 0 deletions internal/command/meta_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ func (m *Meta) initConfigLoader() (*configload.Loader, error) {
if err != nil {
return nil, err
}
loader.AllowLanguageExperiments(m.AllowExperimentalFeatures)
m.configLoader = loader
if m.View != nil {
m.View.SetConfigSources(loader.Sources)
Expand Down
13 changes: 13 additions & 0 deletions internal/configs/configload/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,16 @@ func (l *Loader) ImportSourcesFromSnapshot(snap *Snapshot) {
}
}
}

// AllowLanguageExperiments specifies whether subsequent LoadConfig (and
// similar) calls will allow opting in to experimental language features.
//
// If this method is never called for a particular loader, the default behavior
// is to disallow language experiments.
//
// Main code should set this only for alpha or development builds. Test code
// is responsible for deciding for itself whether and how to call this
// method.
func (l *Loader) AllowLanguageExperiments(allowed bool) {
l.parser.AllowLanguageExperiments(allowed)
}
19 changes: 14 additions & 5 deletions internal/configs/experiments.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var disableExperimentWarnings = ""
// 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) {
func sniffActiveExperiments(body hcl.Body, allowed bool) (experiments.Set, hcl.Diagnostics) {
rootContent, _, diags := body.PartialContent(configFileTerraformBlockSniffRootSchema)

ret := experiments.NewSet()
Expand Down Expand Up @@ -84,9 +84,18 @@ func sniffActiveExperiments(body hcl.Body) (experiments.Set, hcl.Diagnostics) {
}

exps, expDiags := decodeExperimentsAttr(attr)
diags = append(diags, expDiags...)
if !expDiags.HasErrors() {
ret = experiments.SetUnion(ret, exps)
if allowed {
diags = append(diags, expDiags...)
if !expDiags.HasErrors() {
ret = experiments.SetUnion(ret, exps)
}
} else {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Module uses experimental features",
Detail: "Experimental features are intended only for gathering early feedback on new language designs, and so are available only in alpha releases of Terraform.",
Subject: attr.NameRange.Ptr(),
})
}
}

Expand Down Expand Up @@ -144,7 +153,7 @@ func decodeExperimentsAttr(attr *hcl.Attribute) (experiments.Set, hcl.Diagnostic
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.",
Detail: "Experimental features are available only in alpha releases of Terraform and are subject to breaking changes or total removal in later versions, based on feedback. We recommend against using experimental features in production.\n\nIf you have feedback on the design of this feature, please open a GitHub issue to discuss it.",
Subject: expr.Range().Ptr(),
})
}
Expand Down
28 changes: 27 additions & 1 deletion internal/configs/experiments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func TestExperimentsConfig(t *testing.T) {

t.Run("current", func(t *testing.T) {
parser := NewParser(nil)
parser.AllowLanguageExperiments(true)
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)
Expand All @@ -30,7 +31,7 @@ func TestExperimentsConfig(t *testing.T) {
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.",
Detail: "Experimental features are available only in alpha releases of Terraform and are subject to breaking changes or total removal in later versions, based on feedback. We recommend against using experimental features in production.\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},
Expand All @@ -49,6 +50,7 @@ func TestExperimentsConfig(t *testing.T) {
})
t.Run("concluded", func(t *testing.T) {
parser := NewParser(nil)
parser.AllowLanguageExperiments(true)
_, 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)
Expand All @@ -70,6 +72,7 @@ func TestExperimentsConfig(t *testing.T) {
})
t.Run("concluded", func(t *testing.T) {
parser := NewParser(nil)
parser.AllowLanguageExperiments(true)
_, 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)
Expand All @@ -91,6 +94,7 @@ func TestExperimentsConfig(t *testing.T) {
})
t.Run("invalid", func(t *testing.T) {
parser := NewParser(nil)
parser.AllowLanguageExperiments(true)
_, 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)
Expand All @@ -110,4 +114,26 @@ func TestExperimentsConfig(t *testing.T) {
t.Errorf("wrong error\n%s", diff)
}
})
t.Run("disallowed", func(t *testing.T) {
parser := NewParser(nil)
parser.AllowLanguageExperiments(false) // The default situation for release builds
_, 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.DiagError,
Summary: `Module uses experimental features`,
Detail: `Experimental features are intended only for gathering early feedback on new language designs, and so are available only in alpha releases of Terraform.`,
Subject: &hcl.Range{
Filename: "testdata/experiments/current/current_experiment.tf",
Start: hcl.Pos{Line: 2, Column: 3, Byte: 14},
End: hcl.Pos{Line: 2, Column: 14, Byte: 25},
},
}
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("wrong error\n%s", diff)
}
})
}
20 changes: 20 additions & 0 deletions internal/configs/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ import (
type Parser struct {
fs afero.Afero
p *hclparse.Parser

// allowExperiments controls whether we will allow modules to opt in to
// experimental language features. In main code this will be set only
// for alpha releases and some development builds. Test code must decide
// for itself whether to enable it so that tests can cover both the
// allowed and not-allowed situations.
allowExperiments bool
}

// NewParser creates and returns a new Parser that reads files from the given
Expand Down Expand Up @@ -98,3 +105,16 @@ func (p *Parser) ForceFileSource(filename string, src []byte) {
Bytes: src,
})
}

// AllowLanguageExperiments specifies whether subsequent LoadConfigFile (and
// similar) calls will allow opting in to experimental language features.
//
// If this method is never called for a particular parser, the default behavior
// is to disallow language experiments.
//
// Main code should set this only for alpha or development builds. Test code
// is responsible for deciding for itself whether and how to call this
// method.
func (p *Parser) AllowLanguageExperiments(allowed bool) {
p.allowExperiments = allowed
}
2 changes: 1 addition & 1 deletion internal/configs/parser_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (p *Parser) loadConfigFile(path string, override bool) (*File, hcl.Diagnost
// 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)
file.ActiveExperiments, expDiags = sniffActiveExperiments(body, p.allowExperiments)
diags = append(diags, expDiags...)

content, contentDiags := body.Content(configFileSchema)
Expand Down

This file was deleted.

6 changes: 6 additions & 0 deletions internal/terraform/terraform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config
// change its interface at this late stage.
loader, _ := configload.NewLoaderForTests(t)

// We need to be able to exercise experimental features in our integration tests.
loader.AllowLanguageExperiments(true)

// Test modules usually do not refer to remote sources, and for local
// sources only this ultimately just records all of the module paths
// in a JSON file so that we can load them below.
Expand Down Expand Up @@ -111,6 +114,9 @@ func testModuleInline(t *testing.T, sources map[string]string) *configs.Config {
loader, cleanup := configload.NewLoaderForTests(t)
defer cleanup()

// We need to be able to exercise experimental features in our integration tests.
loader.AllowLanguageExperiments(true)

// Test modules usually do not refer to remote sources, and for local
// sources only this ultimately just records all of the module paths
// in a JSON file so that we can load them below.
Expand Down
3 changes: 3 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ func realMain() int {
}
log.Printf("[INFO] Go runtime version: %s", runtime.Version())
log.Printf("[INFO] CLI args: %#v", os.Args)
if ExperimentsAllowed() {
log.Printf("[INFO] This build of Terraform allows using experimental features")
}

streams, err := terminal.Init()
if err != nil {
Expand Down

0 comments on commit 5f77d9d

Please sign in to comment.