From f41fe4879ee2dd698fe85d2d3fd76aa54a512400 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 28 May 2016 12:39:36 -0700 Subject: [PATCH 1/2] core: produce diff when data resource config becomes computed Previously the plan phase would produce a data diff only if no state was already present. However, this is a faulty approach because a state will already be present in the case where the data resource depends on a managed resource that existed in state during refresh but became computed during plan, due to a "forces new resource" diff. Now we will produce a data diff regardless of the presence of the state when the configuration is computed during the plan phase. This fixes #6824. --- terraform/context_plan_test.go | 87 +++++++++++++++++++ .../main.tf | 6 ++ terraform/transform_resource.go | 31 ++++--- 3 files changed, 110 insertions(+), 14 deletions(-) create mode 100644 terraform/test-fixtures/plan-data-resource-becomes-computed/main.tf diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 956065383..3b6defb94 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -911,6 +911,93 @@ func TestContext2Plan_computedDataResource(t *testing.T) { } } +func TestContext2Plan_dataResourceBecomesComputed(t *testing.T) { + m := testModule(t, "plan-data-resource-becomes-computed") + p := testProvider("aws") + + p.DiffFn = func(info *InstanceInfo, state *InstanceState, config *ResourceConfig) (*InstanceDiff, error) { + if info.Type != "aws_instance" { + t.Fatalf("don't know how to diff %s", info.Id) + return nil, nil + } + + return &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "computed": &ResourceAttrDiff{ + Old: "", + New: "", + NewComputed: true, + }, + }, + }, nil + } + p.ReadDataDiffReturn = &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "foo": &ResourceAttrDiff{ + Old: "", + New: "", + NewComputed: true, + }, + }, + } + + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "data.aws_data_resource.foo": &ResourceState{ + Type: "aws_data_resource", + Primary: &InstanceState{ + ID: "i-abc123", + Attributes: map[string]string{ + "id": "i-abc123", + "value": "baz", + }, + }, + }, + }, + }, + }, + }, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + + if got := len(plan.Diff.Modules); got != 1 { + t.Fatalf("got %d modules; want 1", got) + } + + if !p.ReadDataDiffCalled { + t.Fatal("ReadDataDiff wasn't called, but should've been") + } + if got, want := p.ReadDataDiffInfo.Id, "data.aws_data_resource.foo"; got != want { + t.Fatalf("ReadDataDiff info id is %s; want %s", got, want) + } + + moduleDiff := plan.Diff.Modules[0] + + iDiff, ok := moduleDiff.Resources["data.aws_data_resource.foo"] + if !ok { + t.Fatalf("missing diff for data.aws_data_resource.foo") + } + + if same, _ := p.ReadDataDiffReturn.Same(iDiff); !same { + t.Fatalf( + "incorrect diff for data.data_resource.foo\ngot: %#v\nwant: %#v", + iDiff, p.ReadDataDiffReturn, + ) + } +} + func TestContext2Plan_computedList(t *testing.T) { m := testModule(t, "plan-computed-list") p := testProvider("aws") diff --git a/terraform/test-fixtures/plan-data-resource-becomes-computed/main.tf b/terraform/test-fixtures/plan-data-resource-becomes-computed/main.tf new file mode 100644 index 000000000..91399f9bf --- /dev/null +++ b/terraform/test-fixtures/plan-data-resource-becomes-computed/main.tf @@ -0,0 +1,6 @@ +resource "aws_instance" "foo" { +} + +data "aws_data_resource" "foo" { + foo = "${aws_instance.foo.computed}" +} diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 0b2df921a..1ab848968 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -562,10 +562,6 @@ func (n *graphNodeExpandedResource) dataResourceEvalNodes(resource *Resource, in nodes := make([]EvalNode, 0, 5) // Refresh the resource - // TODO: Interpolate and then check if the config has any computed stuff. - // If it doesn't, then do the diff/apply/writestate steps here so we - // can get this data resource populated early enough for its values to - // be visible during plan. nodes = append(nodes, &EvalOpFilter{ Ops: []walkOperation{walkRefresh}, Node: &EvalSequence{ @@ -654,12 +650,25 @@ func (n *graphNodeExpandedResource) dataResourceEvalNodes(resource *Resource, in Output: &state, }, - // If we already have a state (created either during refresh - // or on a previous apply) then we don't need to do any - // more work on it during apply. + // We need to re-interpolate the config here because some + // of the attributes may have become computed during + // earlier planning, due to other resources having + // "requires new resource" diffs. + &EvalInterpolate{ + Config: n.Resource.RawConfig.Copy(), + Resource: resource, + Output: &config, + }, + &EvalIf{ If: func(ctx EvalContext) (bool, error) { - if state != nil { + computed := config.ComputedKeys != nil && len(config.ComputedKeys) > 0 + + // If the configuration is complete and we + // already have a state then we don't need to + // do any further work during apply, because we + // already populated the state during refresh. + if !computed && state != nil { return true, EvalEarlyExitError{} } @@ -668,12 +677,6 @@ func (n *graphNodeExpandedResource) dataResourceEvalNodes(resource *Resource, in Then: EvalNoop{}, }, - &EvalInterpolate{ - Config: n.Resource.RawConfig.Copy(), - Resource: resource, - Output: &config, - }, - &EvalGetProvider{ Name: n.ProvidedBy()[0], Output: &provider, From 46429968f08b29b97efcd4b9346f2a45dbf7066d Mon Sep 17 00:00:00 2001 From: James Nugent Date: Sun, 29 May 2016 12:00:08 -0700 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0edbc94df..0d1bebc9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,47 +11,48 @@ FEATURES: * **New Command:** `terraform state` to provide access to a variety of state manipulation functions [GH-5811] * **New Provider:** `grafana` [GH-6206] - * **New Resource:** `aws_rds_cluster_parameter_group` [GH-5269] - * **New Resource:** `openstack_blockstorage_volume_v2` [GH-6693] - * **New Resource:** `vsphere_virtual_disk` [GH-6273] + * **New Provider:** `random` - allows generation of random values without constantly generating diffs [GH6672] * **New Resource:** `aws_iam_group_policy_attachment` [GH-6858] * **New Resource:** `aws_iam_role_policy_attachment` [GH-6858] * **New Resource:** `aws_iam_user_policy_attachment` [GH-6858] + * **New Resource:** `aws_rds_cluster_parameter_group` [GH-5269] + * **New Resource:** `openstack_blockstorage_volume_v2` [GH-6693] + * **New Resource:** `vsphere_virtual_disk` [GH-6273] * core: Data Resources are now supported. Values are refreshed, and available during the planning stage [GH-6598] * core: Lists and maps can now be used as first class types for variables, and may be passed between modules [GH-6322] - * core: The `terraform plan` command no longer persists state. [GH-6811] * core: Tainted resources now show up in the plan and respect dependency ordering [GH-6600] + * core: The `terraform plan` command no longer persists state. [GH-6811] IMPROVEMENTS: * core: The `jsonencode` interpolation function now supports encoding lists and maps [GH-6749] * provider/aws: Add `option_settings` to `aws_db_option_group` [GH-6560] + * provider/aws: Add more explicit support for Skipping Final Snapshot in RDS Cluster [GH-6795] * provider/aws: Add support for S3 Bucket Acceleration [GH-6628] * provider/aws: Add support for `kms_key_id` to `aws_db_instance` [GH-6651] - * provider/aws: Support for Redshift Cluster encryption using a KMS key [GH-6712] - * provider/aws: Add more explicit support for Skipping Final Snapshot in RDS Cluster [GH-6795] - * provider/aws: Set default description to "Managed by Terraform" [GH-6104] - * provider/aws: SQS use raw policy string if compact fails [GH-6724] - * provider/aws: Support tags for AWS redshift cluster [GH-5356] * provider/aws: Add support to `aws_redshift_cluster` for `iam_roles` [GH-6647] - * provider/azurerm: Add support for exporting the `azurerm_storage_account` access keys [GH-6742] + * provider/aws: SQS use raw policy string if compact fails [GH-6724] + * provider/aws: Set default description to "Managed by Terraform" [GH-6104] + * provider/aws: Support for Redshift Cluster encryption using a KMS key [GH-6712] + * provider/aws: Support tags for AWS redshift cluster [GH-5356] * provider/azurerm: Add support for EnableIPForwarding to `azurerm_network_interface` [GH-6807] + * provider/azurerm: Add support for exporting the `azurerm_storage_account` access keys [GH-6742] * provider/clc: Add support for hyperscale and bareMetal server types and package installation * provider/clc: Fix optional server password [GH-6414] - * provider/cloudstack: Enable swapping of ACLs without having to rebuild the network tier [GH-6741] * provider/cloudstack: Add support for affinity groups to `cloudstack_instance` [GH-6898] + * provider/cloudstack: Enable swapping of ACLs without having to rebuild the network tier [GH-6741] * provider/datadog: Add support for 'require full window' and 'locked' [GH-6738] + * provider/fastly: Add support for Cache Settings [GH-6781] * provider/fastly: Add support for Service Request Settings on `fastly_service_v1` resources [GH-6622] * provider/fastly: Add support for custom VCL configuration [GH-6662] - * provider/fastly: Add support for Cache Settings [GH-6781] * provider/google: Support optional uuid naming for Instance Template [GH-6604] - * provider/openstack: Increase timeouts for image resize, subnets, and routers [GH-6764] * provider/openstack: Add support for client certificate authentication [GH-6279] - * provider/openstack: Enable DHCP By Default [GH-6838] * provider/openstack: Allow Neutron-based Floating IP to target a specific tenant [GH-6454] + * provider/openstack: Enable DHCP By Default [GH-6838] * provider/openstack: Implement fixed_ip on Neutron floating ip allocations [GH-6837] - * provider/vsphere: Fix bug with `vsphere_virtual_machine` wait for ip [GH-6377] + * provider/openstack: Increase timeouts for image resize, subnets, and routers [GH-6764] * provider/vsphere: Add support for `controller_type` to `vsphere_virtual_machine` [GH-6785] + * provider/vsphere: Fix bug with `vsphere_virtual_machine` wait for ip [GH-6377] * provider/vsphere: Virtual machine update disk [GH-6619] BUG FIXES: