Skip to content

Commit

Permalink
core: Don't filter ignore_changes on create
Browse files Browse the repository at this point in the history
The ignore_changes diff filter was stripping out attributes on Create
but the diff was still making it down to the provider, so Create would
end up missing attributes, causing a full failure if any required
attributes were being ignored.

This should fix `ignore_changes` problems with Required attributes.

Here we also introduce a `test` provider meant as an aid to exposing
via automated tests issues involving interactions between
`helper/schema` and Terraform core.

This has been helpful so far in diagnosing `ignore_changes` problems,
and I imagine it will be helpful in other contexts as well.

We'll have to be careful to prevent the `test` provider from becoming a
dumping ground for poorly specified tests that have a clear home
elsewhere. But for bug exposure I think it's useful to have.

Refs #5627
  • Loading branch information
phinze committed Mar 15, 2016
1 parent a1f7789 commit 52aca02
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 0 deletions.
16 changes: 16 additions & 0 deletions helper/resource/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,22 @@ func Test(t TestT, c TestCase) {
}
}

// UnitTest is a helper to force the acceptance testing harness to run in the
// normal unit test suite. This should only be used for resource that don't
// have any external dependencies.
func UnitTest(t TestT, c TestCase) {
oldEnv := os.Getenv(TestEnvVar)
if err := os.Setenv(TestEnvVar, "UnitTestOverride"); err != nil {
t.Fatal(err)
}
defer func() {
if err := os.Setenv(TestEnvVar, oldEnv); err != nil {
t.Fatal(err)
}
}()
Test(t, c)
}

func testStep(
opts terraform.ContextOpts,
state *terraform.State,
Expand Down
41 changes: 41 additions & 0 deletions terraform/context_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4084,3 +4084,44 @@ aws_instance.ifailedprovisioners: (1 tainted)
t.Fatalf("expected state: \n%s\ngot: \n%s", expected, actual)
}
}

func TestContext2Apply_ignoreChangesCreate(t *testing.T) {
m := testModule(t, "apply-ignore-changes-create")
p := testProvider("aws")
p.ApplyFn = testApplyFn
p.DiffFn = testDiffFn
ctx := testContext2(t, &ContextOpts{
Module: m,
Providers: map[string]ResourceProviderFactory{
"aws": testProviderFuncFixed(p),
},
})

if p, err := ctx.Plan(); err != nil {
t.Fatalf("err: %s", err)
} else {
t.Logf(p.String())
}

state, err := ctx.Apply()
if err != nil {
t.Fatalf("err: %s", err)
}

mod := state.RootModule()
if len(mod.Resources) != 1 {
t.Fatalf("bad: %s", state)
}

actual := strings.TrimSpace(state.String())
// Expect no changes from original state
expected := strings.TrimSpace(`
aws_instance.foo:
ID = foo
required_field = set
type = aws_instance
`)
if actual != expected {
t.Fatalf("bad: \n%s", actual)
}
}
6 changes: 6 additions & 0 deletions terraform/eval_ignore_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ func (n *EvalIgnoreChanges) Eval(ctx EvalContext) (interface{}, error) {
diff := *n.Diff
ignoreChanges := n.Resource.Lifecycle.IgnoreChanges

// If we're just creating the resource, we shouldn't alter the
// Diff at all
if diff.ChangeType() == DiffCreate {
return nil, nil
}

for _, ignoredName := range ignoreChanges {
for name := range diff.Attributes {
if strings.HasPrefix(name, ignoredName) {
Expand Down
7 changes: 7 additions & 0 deletions terraform/test-fixtures/apply-ignore-changes-create/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
resource "aws_instance" "foo" {
required_field = "set"

lifecycle {
ignore_changes = ["required_field"]
}
}

0 comments on commit 52aca02

Please sign in to comment.