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 73ab98d
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 0 deletions.
19 changes: 19 additions & 0 deletions builtin/providers/test/provider.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package test

import (
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/terraform"
)

func Provider() terraform.ResourceProvider {
return &schema.Provider{
ResourcesMap: map[string]*schema.Resource{
"test_resource": testResource(),
},
ConfigureFunc: providerConfigure,
}
}

func providerConfigure(d *schema.ResourceData) (interface{}, error) {
return nil, nil
}
16 changes: 16 additions & 0 deletions builtin/providers/test/provider_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package test

import (
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/terraform"
)

var testAccProviders map[string]terraform.ResourceProvider
var testAccProvider *schema.Provider

func init() {
testAccProvider = Provider().(*schema.Provider)
testAccProviders = map[string]terraform.ResourceProvider{
"test": testAccProvider,
}
}
53 changes: 53 additions & 0 deletions builtin/providers/test/resource.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package test

import (
"fmt"

"github.com/hashicorp/terraform/helper/schema"
)

func testResource() *schema.Resource {
return &schema.Resource{
Create: testResourceCreate,
Read: testResourceRead,
Update: testResourceUpdate,
Delete: testResourceDelete,
Schema: map[string]*schema.Schema{
"required": &schema.Schema{
Type: schema.TypeString,
Required: true,
},
"optional": &schema.Schema{
Type: schema.TypeString,
Optional: true,
},
"optional_force_new": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
},
},
}
}

func testResourceCreate(d *schema.ResourceData, meta interface{}) error {
d.SetId("testId")

// Required must make it through to Create
if _, ok := d.GetOk("required"); !ok {
return fmt.Errorf("Missing attribute 'required', but it's required!")
}
return nil
}

func testResourceRead(d *schema.ResourceData, meta interface{}) error {
return nil
}

func testResourceUpdate(d *schema.ResourceData, meta interface{}) error {
return nil
}

func testResourceDelete(d *schema.ResourceData, meta interface{}) error {
return nil
}
54 changes: 54 additions & 0 deletions builtin/providers/test/resource_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package test

import (
"strings"
"testing"

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)

func TestResource_basic(t *testing.T) {
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource" "foo" {
required = "yep"
}
`),
Check: func(s *terraform.State) error {
return nil
},
},
},
})
}

func TestResource_ignoreChangesRequired(t *testing.T) {
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource" "foo" {
required = "yep"
lifecycle {
ignore_changes = ["required"]
}
}
`),
Check: func(s *terraform.State) error {
return nil
},
},
},
})
}

func testAccCheckResourceDestroy(s *terraform.State) error {
return nil
}
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 73ab98d

Please sign in to comment.