From 3fa063b8dcf9c6f94d7bd9a67df3d75a07e1bc95 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 2 Dec 2020 15:42:41 -0500 Subject: [PATCH] command/format: concise diff is now the default (#27079) * command/format: concise diff is no longer an experiment Since state formatting goes through the "diff" printer, I have repurposed the concise flag as a verbose flag, used only when printing state. It's silly but it works! * remove helper/experiment With this experiment concluded, we no longer need helper/experiment. The shadow experiment had not been touched in many years, so I removed all references, and removed the package entirely. Any new experiments are expected to be configuration experiments handled by our (other) experiments package. * check for the verbose flag consistently, in case we end up using it in plans in the future --- command/format/diff.go | 25 +- command/format/diff_test.go | 412 --------------------------- command/format/state.go | 7 +- command/meta.go | 47 --- helper/experiment/experiment.go | 158 ---------- helper/experiment/experiment_test.go | 117 -------- helper/experiment/id.go | 34 --- terraform/terraform_test.go | 6 - 8 files changed, 15 insertions(+), 791 deletions(-) delete mode 100644 helper/experiment/experiment.go delete mode 100644 helper/experiment/experiment_test.go delete mode 100644 helper/experiment/id.go diff --git a/command/format/diff.go b/command/format/diff.go index 7a3df5f094..fd894adc30 100644 --- a/command/format/diff.go +++ b/command/format/diff.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs/configschema" - "github.com/hashicorp/terraform/helper/experiment" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/plans/objchange" "github.com/hashicorp/terraform/states" @@ -99,7 +98,6 @@ func ResourceChange( color: color, action: change.Action, requiredReplace: change.RequiredReplace, - concise: experiment.Enabled(experiment.X_concise_diff), } // Most commonly-used resources have nested blocks that result in us @@ -154,10 +152,9 @@ func OutputChanges( ) string { var buf bytes.Buffer p := blockBodyDiffPrinter{ - buf: &buf, - color: color, - action: plans.Update, // not actually used in this case, because we're not printing a containing block - concise: experiment.Enabled(experiment.X_concise_diff), + buf: &buf, + color: color, + action: plans.Update, // not actually used in this case, because we're not printing a containing block } // We're going to reuse the codepath we used for printing resource block @@ -200,7 +197,8 @@ type blockBodyDiffPrinter struct { color *colorstring.Colorize action plans.Action requiredReplace cty.PathSet - concise bool + // verbose is set to true when using the "diff" printer to format state + verbose bool } type blockBodyDiffResult struct { @@ -326,7 +324,7 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At path = append(path, cty.GetAttrStep{Name: name}) action, showJustNew := getPlanActionAndShow(old, new) - if action == plans.NoOp && p.concise && !identifyingAttribute(name, attrS) { + if action == plans.NoOp && !p.verbose && !identifyingAttribute(name, attrS) { return true } @@ -624,7 +622,7 @@ func (p *blockBodyDiffPrinter) writeSensitiveNestedBlockDiff(name string, old, n } func (p *blockBodyDiffPrinter) writeNestedBlockDiff(name string, label *string, blockS *configschema.Block, action plans.Action, old, new cty.Value, indent int, path cty.Path) bool { - if action == plans.NoOp && p.concise { + if action == plans.NoOp && !p.verbose { return true } @@ -984,7 +982,7 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa action = plans.NoOp } - if action == plans.NoOp && p.concise { + if action == plans.NoOp && !p.verbose { suppressedElements++ continue } @@ -1024,8 +1022,7 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa var changeShown bool for i := 0; i < len(elemDiffs); i++ { - // In concise mode, push any no-op diff elements onto the stack - if p.concise { + if !p.verbose { for i < len(elemDiffs) && elemDiffs[i].Action == plans.NoOp { suppressedElements = append(suppressedElements, elemDiffs[i]) i++ @@ -1161,7 +1158,7 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa } } - if action == plans.NoOp && p.concise { + if action == plans.NoOp && !p.verbose { suppressedElements++ continue } @@ -1262,7 +1259,7 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa action = plans.Update } - if action == plans.NoOp && p.concise { + if action == plans.NoOp && !p.verbose { suppressedElements++ continue } diff --git a/command/format/diff_test.go b/command/format/diff_test.go index 076b1a9a20..db2d0b7dea 100644 --- a/command/format/diff_test.go +++ b/command/format/diff_test.go @@ -7,7 +7,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs/configschema" - "github.com/hashicorp/terraform/helper/experiment" "github.com/hashicorp/terraform/plans" "github.com/mitchellh/colorstring" "github.com/zclconf/go-cty/cty" @@ -362,13 +361,6 @@ new line ~ str = "before" -> "after" # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ~ id = "blah" -> (known after apply) - password = (sensitive value) - ~ str = "before" -> "after" - } `, }, @@ -502,18 +494,6 @@ new line } # (2 unchanged attributes hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ~ ami = "ami-BEFORE" -> "ami-AFTER" - bar = "bar" - foo = "foo" - id = "i-02ae66f368e8518a9" - name = "alice" - tags = { - "name" = "bob" - } - } `, }, } @@ -594,19 +574,6 @@ func TestResourceChange_JSON(t *testing.T) { ) } `, - - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ json_field = jsonencode( - ~ { - aaa = "value" - + bbb = "new_value" - - ccc = 5 -> null - } - ) - } -`, }, "in-place update (from empty tuple)": { Action: plans.Update, @@ -739,17 +706,6 @@ func TestResourceChange_JSON(t *testing.T) { } # forces replacement ) } -`, - VerboseOutput: ` # test_instance.example must be replaced --/+ resource "test_instance" "example" { - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ json_field = jsonencode( - ~ { - aaa = "value" - + bbb = "new_value" - } # forces replacement - ) - } `, }, "in-place update (whitespace change)": { @@ -871,18 +827,6 @@ func TestResourceChange_JSON(t *testing.T) { ] ) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ json_field = jsonencode( - ~ [ - "first", - "second", - - "third", - ] - ) - } `, }, "JSON list item addition": { @@ -916,19 +860,6 @@ func TestResourceChange_JSON(t *testing.T) { ) } `, - - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ json_field = jsonencode( - ~ [ - "first", - "second", - + "third", - ] - ) - } -`, }, "JSON list object addition": { Action: plans.Update, @@ -1223,15 +1154,6 @@ func TestResourceChange_primitiveList(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - + list_field = [ - + "new-element", - ] - } `, }, "in-place update - first addition": { @@ -1266,15 +1188,6 @@ func TestResourceChange_primitiveList(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ list_field = [ - + "new-element", - ] - } `, }, "in-place update - insertion": { @@ -1324,20 +1237,6 @@ func TestResourceChange_primitiveList(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ list_field = [ - "aaaa", - "bbbb", - + "cccc", - "dddd", - "eeee", - "ffff", - ] - } `, }, "force-new update - insertion": { @@ -1381,17 +1280,6 @@ func TestResourceChange_primitiveList(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example must be replaced --/+ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ list_field = [ # forces replacement - "aaaa", - + "bbbb", - "cccc", - ] - } `, }, "in-place update - deletion": { @@ -1438,19 +1326,6 @@ func TestResourceChange_primitiveList(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ list_field = [ - - "aaaa", - "bbbb", - - "cccc", - "dddd", - "eeee", - ] - } `, }, "creation - empty list": { @@ -1515,17 +1390,6 @@ func TestResourceChange_primitiveList(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ list_field = [ - - "aaaa", - - "bbbb", - - "cccc", - ] - } `, }, "in-place update - null to empty": { @@ -1556,13 +1420,6 @@ func TestResourceChange_primitiveList(t *testing.T) { + list_field = [] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - + list_field = [] - } `, }, "update to unknown element": { @@ -1606,18 +1463,6 @@ func TestResourceChange_primitiveList(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ list_field = [ - "aaaa", - - "bbbb", - + (known after apply), - "cccc", - ] - } `, }, "update - two new unknown elements": { @@ -1668,21 +1513,6 @@ func TestResourceChange_primitiveList(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ list_field = [ - "aaaa", - - "bbbb", - + (known after apply), - + (known after apply), - "cccc", - "dddd", - "eeee", - ] - } `, }, } @@ -1734,19 +1564,6 @@ func TestResourceChange_primitiveTuple(t *testing.T) { # (1 unchanged element hidden) ] } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - id = "i-02ae66f368e8518a9" - ~ tuple_field = [ - "aaaa", - "bbbb", - - "dddd", - + "cccc", - "eeee", - "ffff", - ] - } `, }, } @@ -1787,15 +1604,6 @@ func TestResourceChange_primitiveSet(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - + set_field = [ - + "new-element", - ] - } `, }, "in-place update - first insertion": { @@ -1830,15 +1638,6 @@ func TestResourceChange_primitiveSet(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ set_field = [ - + "new-element", - ] - } `, }, "in-place update - insertion": { @@ -1879,17 +1678,6 @@ func TestResourceChange_primitiveSet(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ set_field = [ - "aaaa", - + "bbbb", - "cccc", - ] - } `, }, "force-new update - insertion": { @@ -1932,17 +1720,6 @@ func TestResourceChange_primitiveSet(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example must be replaced --/+ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ set_field = [ # forces replacement - "aaaa", - + "bbbb", - "cccc", - ] - } `, }, "in-place update - deletion": { @@ -1983,17 +1760,6 @@ func TestResourceChange_primitiveSet(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ set_field = [ - - "aaaa", - "bbbb", - - "cccc", - ] - } `, }, "creation - empty set": { @@ -2055,16 +1821,6 @@ func TestResourceChange_primitiveSet(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ set_field = [ - - "aaaa", - - "bbbb", - ] - } `, }, "in-place update - null to empty set": { @@ -2095,13 +1851,6 @@ func TestResourceChange_primitiveSet(t *testing.T) { + set_field = [] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - + set_field = [] - } `, }, "in-place update to unknown": { @@ -2138,16 +1887,6 @@ func TestResourceChange_primitiveSet(t *testing.T) { ] -> (known after apply) # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ set_field = [ - - "aaaa", - - "bbbb", - ] -> (known after apply) - } `, }, "in-place update to unknown element": { @@ -2188,17 +1927,6 @@ func TestResourceChange_primitiveSet(t *testing.T) { ] # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ set_field = [ - "aaaa", - - "bbbb", - ~ (known after apply), - ] - } `, }, } @@ -2239,15 +1967,6 @@ func TestResourceChange_map(t *testing.T) { } # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - + map_field = { - + "new-key" = "new-element" - } - } `, }, "in-place update - first insertion": { @@ -2282,15 +2001,6 @@ func TestResourceChange_map(t *testing.T) { } # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ map_field = { - + "new-key" = "new-element" - } - } `, }, "in-place update - insertion": { @@ -2331,17 +2041,6 @@ func TestResourceChange_map(t *testing.T) { } # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ map_field = { - "a" = "aaaa" - + "b" = "bbbb" - "c" = "cccc" - } - } `, }, "force-new update - insertion": { @@ -2384,17 +2083,6 @@ func TestResourceChange_map(t *testing.T) { } # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example must be replaced --/+ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ map_field = { # forces replacement - "a" = "aaaa" - + "b" = "bbbb" - "c" = "cccc" - } - } `, }, "in-place update - deletion": { @@ -2435,17 +2123,6 @@ func TestResourceChange_map(t *testing.T) { } # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ map_field = { - - "a" = "aaaa" -> null - "b" = "bbbb" - - "c" = "cccc" -> null - } - } `, }, "creation - empty": { @@ -2513,17 +2190,6 @@ func TestResourceChange_map(t *testing.T) { } # (1 unchanged attribute hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ami = "ami-STATIC" - ~ id = "i-02ae66f368e8518a9" -> (known after apply) - ~ map_field = { - "a" = "aaaa" - ~ "b" = "bbbb" -> (known after apply) - "c" = "cccc" - } - } `, }, } @@ -2582,16 +2248,6 @@ func TestResourceChange_nestedList(t *testing.T) { # (1 unchanged block hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ~ ami = "ami-BEFORE" -> "ami-AFTER" - id = "i-02ae66f368e8518a9" - - root_block_device { - volume_type = "gp2" - } - } `, }, "in-place update - creation": { @@ -2756,17 +2412,6 @@ func TestResourceChange_nestedList(t *testing.T) { # (1 unchanged attribute hidden) } } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ~ ami = "ami-BEFORE" -> "ami-AFTER" - id = "i-02ae66f368e8518a9" - - ~ root_block_device { - + new_field = "new_value" - volume_type = "gp2" - } - } `, }, "force-new update (inside block)": { @@ -3344,17 +2989,6 @@ func TestResourceChange_nestedMap(t *testing.T) { # (1 unchanged attribute hidden) } } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ~ ami = "ami-BEFORE" -> "ami-AFTER" - id = "i-02ae66f368e8518a9" - - ~ root_block_device "a" { - + new_field = "new_value" - volume_type = "gp2" - } - } `, }, "in-place update - insertion": { @@ -3422,20 +3056,6 @@ func TestResourceChange_nestedMap(t *testing.T) { } # (1 unchanged block hidden) } -`, - VerboseOutput: ` # test_instance.example will be updated in-place - ~ resource "test_instance" "example" { - ~ ami = "ami-BEFORE" -> "ami-AFTER" - id = "i-02ae66f368e8518a9" - - root_block_device "a" { - volume_type = "gp2" - } - + root_block_device "b" { - + new_field = "new_value" - + volume_type = "gp2" - } - } `, }, "force-new update (whole block)": { @@ -3500,19 +3120,6 @@ func TestResourceChange_nestedMap(t *testing.T) { } # (1 unchanged block hidden) } -`, - VerboseOutput: ` # test_instance.example must be replaced --/+ resource "test_instance" "example" { - ~ ami = "ami-BEFORE" -> "ami-AFTER" - id = "i-02ae66f368e8518a9" - - ~ root_block_device "a" { # forces replacement - ~ volume_type = "gp2" -> "different" - } - root_block_device "b" { - volume_type = "standard" - } - } `, }, "in-place update - deletion": { @@ -4512,10 +4119,6 @@ type testCase struct { RequiredReplace cty.PathSet Tainted bool ExpectedOutput string - - // This field and all associated values can be removed if the concise diff - // experiment succeeds. - VerboseOutput string } func runTestCases(t *testing.T, testCases map[string]testCase) { @@ -4569,25 +4172,11 @@ func runTestCases(t *testing.T, testCases map[string]testCase) { RequiredReplace: tc.RequiredReplace, } - experiment.SetEnabled(experiment.X_concise_diff, true) output := ResourceChange(change, tc.Tainted, tc.Schema, color) if output != tc.ExpectedOutput { t.Errorf("Unexpected diff.\ngot:\n%s\nwant:\n%s\n", output, tc.ExpectedOutput) t.Errorf("%s", cmp.Diff(output, tc.ExpectedOutput)) } - - // Temporary coverage for verbose diff behaviour. All lines below - // in this function can be removed if the concise diff experiment - // succeeds. - if tc.VerboseOutput == "" { - return - } - experiment.SetEnabled(experiment.X_concise_diff, false) - output = ResourceChange(change, tc.Tainted, tc.Schema, color) - if output != tc.VerboseOutput { - t.Errorf("Unexpected diff.\ngot:\n%s\nwant:\n%s\n", output, tc.VerboseOutput) - t.Errorf("%s", cmp.Diff(output, tc.VerboseOutput)) - } }) } } @@ -4694,7 +4283,6 @@ func TestOutputChanges(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - experiment.SetEnabled(experiment.X_concise_diff, true) output := OutputChanges(tc.changes, color) if output != tc.output { t.Errorf("Unexpected diff.\ngot:\n%s\nwant:\n%s\n", output, tc.output) diff --git a/command/format/state.go b/command/format/state.go index e737f3a00d..c43672ce64 100644 --- a/command/format/state.go +++ b/command/format/state.go @@ -45,9 +45,10 @@ func State(opts *StateOpts) string { buf := bytes.NewBufferString("[reset]") p := blockBodyDiffPrinter{ - buf: buf, - color: opts.Color, - action: plans.NoOp, + buf: buf, + color: opts.Color, + action: plans.NoOp, + verbose: true, } // Format all the modules diff --git a/command/meta.go b/command/meta.go index a2f901a7d1..2a44cb3673 100644 --- a/command/meta.go +++ b/command/meta.go @@ -22,7 +22,6 @@ import ( "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/command/webbrowser" "github.com/hashicorp/terraform/configs/configload" - "github.com/hashicorp/terraform/helper/experiment" "github.com/hashicorp/terraform/helper/wrappedstreams" "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/providers" @@ -500,9 +499,6 @@ func (m *Meta) extendedFlagSet(n string) *flag.FlagSet { f.Var(varValues, "var", "variables") f.Var(varFiles, "var-file", "variable file") - // Experimental features - experiment.Flag(f) - // commands that bypass locking will supply their own flag on this var, // but set the initial meta value to true as a failsafe. m.stateLock = true @@ -639,49 +635,6 @@ func (m *Meta) showDiagnostics(vals ...interface{}) { } } -// outputShadowError outputs the error from ctx.ShadowError. If the -// error is nil then nothing happens. If output is false then it isn't -// outputted to the user (you can define logic to guard against outputting). -func (m *Meta) outputShadowError(err error, output bool) bool { - // Do nothing if no error - if err == nil { - return false - } - - // If not outputting, do nothing - if !output { - return false - } - - // Write the shadow error output to a file - path := fmt.Sprintf("terraform-error-%d.log", time.Now().UTC().Unix()) - if err := ioutil.WriteFile(path, []byte(err.Error()), 0644); err != nil { - // If there is an error writing it, just let it go - log.Printf("[ERROR] Error writing shadow error: %s", err) - return false - } - - // Output! - m.Ui.Output(m.Colorize().Color(fmt.Sprintf( - "[reset][bold][yellow]\nExperimental feature failure! Please report a bug.\n\n"+ - "This is not an error. Your Terraform operation completed successfully.\n"+ - "Your real infrastructure is unaffected by this message.\n\n"+ - "[reset][yellow]While running, Terraform sometimes tests experimental features in the\n"+ - "background. These features cannot affect real state and never touch\n"+ - "real infrastructure. If the features work properly, you see nothing.\n"+ - "If the features fail, this message appears.\n\n"+ - "You can report an issue at: https://github.com/hashicorp/terraform/issues\n\n"+ - "The failure was written to %q. Please\n"+ - "double check this file contains no sensitive information and report\n"+ - "it with your issue.\n\n"+ - "This is not an error. Your terraform operation completed successfully\n"+ - "and your real infrastructure is unaffected by this message.", - path, - ))) - - return true -} - // WorkspaceNameEnvVar is the name of the environment variable that can be used // to set the name of the Terraform workspace, overriding the workspace chosen // by `terraform workspace select`. diff --git a/helper/experiment/experiment.go b/helper/experiment/experiment.go deleted file mode 100644 index 72fdeaf9ed..0000000000 --- a/helper/experiment/experiment.go +++ /dev/null @@ -1,158 +0,0 @@ -// experiment package contains helper functions for tracking experimental -// features throughout Terraform. -// -// This package should be used for creating, enabling, querying, and deleting -// experimental features. By unifying all of that onto a single interface, -// we can have the Go compiler help us by enforcing every place we touch -// an experimental feature. -// -// To create a new experiment: -// -// 1. Add the experiment to the global vars list below, prefixed with X_ -// -// 2. Add the experiment variable to the All listin the init() function -// -// 3. Use it! -// -// To remove an experiment: -// -// 1. Delete the experiment global var. -// -// 2. Try to compile and fix all the places where the var was referenced. -// -// To use an experiment: -// -// 1. Use Flag() if you want the experiment to be available from the CLI. -// -// 2. Use Enabled() to check whether it is enabled. -// -// As a general user: -// -// 1. The `-Xexperiment-name` flag -// 2. The `TF_X_` env var. -// 3. The `TF_X_FORCE` env var can be set to force an experimental feature -// without human verifications. -// -package experiment - -import ( - "flag" - "fmt" - "os" - "strconv" - "strings" - "sync" -) - -// The experiments that are available are listed below. Any package in -// Terraform defining an experiment should define the experiments below. -// By keeping them all within the experiment package we force a single point -// of definition and use. This allows the compiler to enforce references -// so it becomes easy to remove the features. -var ( - // Shadow graph. This is already on by default. Disabling it will be - // allowed for awhile in order for it to not block operations. - X_shadow = newBasicID("shadow", "SHADOW", false) - - // Concise plan diff output - X_concise_diff = newBasicID("concise_diff", "CONCISE_DIFF", true) -) - -// Global variables this package uses because we are a package -// with global state. -var ( - // all is the list of all experiements. Do not modify this. - All []ID - - // enabled keeps track of what flags have been enabled - enabled map[string]bool - enabledLock sync.Mutex - - // Hidden "experiment" that forces all others to be on without verification - x_force = newBasicID("force", "FORCE", false) -) - -func init() { - // The list of all experiments, update this when an experiment is added. - All = []ID{ - X_shadow, - X_concise_diff, - x_force, - } - - // Load - reload() -} - -// reload is used by tests to reload the global state. This is called by -// init publicly. -func reload() { - // Initialize - enabledLock.Lock() - enabled = make(map[string]bool) - enabledLock.Unlock() - - // Set defaults and check env vars - for _, id := range All { - // Get the default value - def := id.Default() - - // If we set it in the env var, default it to true - key := fmt.Sprintf("TF_X_%s", strings.ToUpper(id.Env())) - if v := os.Getenv(key); v != "" { - def = v != "0" - } - - // Set the default - SetEnabled(id, def) - } -} - -// Enabled returns whether an experiment has been enabled or not. -func Enabled(id ID) bool { - enabledLock.Lock() - defer enabledLock.Unlock() - return enabled[id.Flag()] -} - -// SetEnabled sets an experiment to enabled/disabled. Please check with -// the experiment docs for when calling this actually affects the experiment. -func SetEnabled(id ID, v bool) { - enabledLock.Lock() - defer enabledLock.Unlock() - enabled[id.Flag()] = v -} - -// Force returns true if the -Xforce of TF_X_FORCE flag is present, which -// advises users of this package to not verify with the user that they want -// experimental behavior and to just continue with it. -func Force() bool { - return Enabled(x_force) -} - -// Flag configures the given FlagSet with the flags to configure -// all active experiments. -func Flag(fs *flag.FlagSet) { - for _, id := range All { - desc := id.Flag() - key := fmt.Sprintf("X%s", id.Flag()) - fs.Var(&idValue{X: id}, key, desc) - } -} - -// idValue implements flag.Value for setting the enabled/disabled state -// of an experiment from the CLI. -type idValue struct { - X ID -} - -func (v *idValue) IsBoolFlag() bool { return true } -func (v *idValue) String() string { return strconv.FormatBool(Enabled(v.X)) } -func (v *idValue) Set(raw string) error { - b, err := strconv.ParseBool(raw) - if err == nil { - SetEnabled(v.X, b) - } - - return err -} diff --git a/helper/experiment/experiment_test.go b/helper/experiment/experiment_test.go deleted file mode 100644 index 32055c2c8b..0000000000 --- a/helper/experiment/experiment_test.go +++ /dev/null @@ -1,117 +0,0 @@ -package experiment - -import ( - "flag" - "fmt" - "os" - "testing" -) - -// Test experiments -var ( - X_test1 = newBasicID("test1", "TEST1", false) - X_test2 = newBasicID("test2", "TEST2", true) -) - -// Reinitializes the package to a clean slate -func testReinit() { - All = []ID{X_test1, X_test2, x_force} - reload() -} - -func init() { - testReinit() - - // Clear all env vars so they don't affect tests - for _, id := range All { - os.Unsetenv(fmt.Sprintf("TF_X_%s", id.Env())) - } -} - -func TestDefault(t *testing.T) { - testReinit() - - if Enabled(X_test1) { - t.Fatal("test1 should not be enabled") - } - - if !Enabled(X_test2) { - t.Fatal("test2 should be enabled") - } -} - -func TestEnv(t *testing.T) { - os.Setenv("TF_X_TEST2", "0") - defer os.Unsetenv("TF_X_TEST2") - - testReinit() - - if Enabled(X_test2) { - t.Fatal("test2 should be enabled") - } -} - -func TestFlag(t *testing.T) { - testReinit() - - // Verify default - if !Enabled(X_test2) { - t.Fatal("test2 should be enabled") - } - - // Setup a flag set - fs := flag.NewFlagSet("test", flag.ContinueOnError) - Flag(fs) - fs.Parse([]string{"-Xtest2=false"}) - - if Enabled(X_test2) { - t.Fatal("test2 should not be enabled") - } -} - -func TestFlag_overEnv(t *testing.T) { - os.Setenv("TF_X_TEST2", "1") - defer os.Unsetenv("TF_X_TEST2") - - testReinit() - - // Verify default - if !Enabled(X_test2) { - t.Fatal("test2 should be enabled") - } - - // Setup a flag set - fs := flag.NewFlagSet("test", flag.ContinueOnError) - Flag(fs) - fs.Parse([]string{"-Xtest2=false"}) - - if Enabled(X_test2) { - t.Fatal("test2 should not be enabled") - } -} - -func TestForce(t *testing.T) { - os.Setenv("TF_X_FORCE", "1") - defer os.Unsetenv("TF_X_FORCE") - - testReinit() - - if !Force() { - t.Fatal("should force") - } -} - -func TestForce_flag(t *testing.T) { - os.Unsetenv("TF_X_FORCE") - - testReinit() - - // Setup a flag set - fs := flag.NewFlagSet("test", flag.ContinueOnError) - Flag(fs) - fs.Parse([]string{"-Xforce"}) - - if !Force() { - t.Fatal("should force") - } -} diff --git a/helper/experiment/id.go b/helper/experiment/id.go deleted file mode 100644 index 8e2f707322..0000000000 --- a/helper/experiment/id.go +++ /dev/null @@ -1,34 +0,0 @@ -package experiment - -// ID represents an experimental feature. -// -// The global vars defined on this package should be used as ID values. -// This interface is purposely not implement-able outside of this package -// so that we can rely on the Go compiler to enforce all experiment references. -type ID interface { - Env() string - Flag() string - Default() bool - - unexported() // So the ID can't be implemented externally. -} - -// basicID implements ID. -type basicID struct { - EnvValue string - FlagValue string - DefaultValue bool -} - -func newBasicID(flag, env string, def bool) ID { - return &basicID{ - EnvValue: env, - FlagValue: flag, - DefaultValue: def, - } -} - -func (id *basicID) Env() string { return id.EnvValue } -func (id *basicID) Flag() string { return id.FlagValue } -func (id *basicID) Default() bool { return id.DefaultValue } -func (id *basicID) unexported() {} diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index b196d44080..115c8ac719 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -17,7 +17,6 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configload" - "github.com/hashicorp/terraform/helper/experiment" "github.com/hashicorp/terraform/internal/initwd" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/providers" @@ -32,11 +31,6 @@ import ( const fixtureDir = "./testdata" func TestMain(m *testing.M) { - // We want to shadow on tests just to make sure the shadow graph works - // in case we need it and to find any race issues. - experiment.SetEnabled(experiment.X_shadow, true) - - experiment.Flag(flag.CommandLine) flag.Parse() // Make sure shadow operations fail our real tests