Skip to content

Commit

Permalink
lookup(): only treat map as unknown if it is wholly unknown
Browse files Browse the repository at this point in the history
Fix for issue #26320 - this allows us to derive known values
from partially known maps where we can, and may prevent unnecessary
destroy/rebuild cycles during apply in some cases.
  • Loading branch information
OwenTuz committed Sep 30, 2020
1 parent a78d75c commit 04cbaeb
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 20 deletions.
23 changes: 13 additions & 10 deletions command/e2etest/automation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ func TestPlanApplyInAutomation(t *testing.T) {
t.Fatalf("unexpected plan error: %s\nstderr:\n%s", err, stderr)
}

if !strings.Contains(stdout, "1 to add, 0 to change, 0 to destroy") {
t.Errorf("incorrect plan tally; want 1 to add:\n%s", stdout)
if !strings.Contains(stdout, "2 to add, 0 to change, 0 to destroy") {
t.Errorf("incorrect plan tally; want 2 to add:\n%s", stdout)
}

// Because we're running with TF_IN_AUTOMATION set, we should not see
Expand All @@ -73,12 +73,13 @@ func TestPlanApplyInAutomation(t *testing.T) {

// stateResources := plan.Changes.Resources
diffResources := plan.Changes.Resources
if len(diffResources) != 1 {
if len(diffResources) != 2 {
t.Errorf("incorrect number of resources in plan")
}

expected := map[string]plans.Action{
"null_resource.test": plans.Create,
"null_resource.id_known_after_apply": plans.Create,
"null_resource.test": plans.Create,
}

for _, r := range diffResources {
Expand All @@ -97,8 +98,8 @@ func TestPlanApplyInAutomation(t *testing.T) {
t.Fatalf("unexpected apply error: %s\nstderr:\n%s", err, stderr)
}

if !strings.Contains(stdout, "Resources: 1 added, 0 changed, 0 destroyed") {
t.Errorf("incorrect apply tally; want 1 added:\n%s", stdout)
if !strings.Contains(stdout, "Resources: 2 added, 0 changed, 0 destroyed") {
t.Errorf("incorrect apply tally; want 2 added:\n%s", stdout)
}

state, err := tf.LocalState()
Expand All @@ -115,6 +116,7 @@ func TestPlanApplyInAutomation(t *testing.T) {

wantResources := []string{
"data.template_file.test",
"null_resource.id_known_after_apply",
"null_resource.test",
}

Expand Down Expand Up @@ -164,8 +166,8 @@ func TestAutoApplyInAutomation(t *testing.T) {
t.Fatalf("unexpected apply error: %s\nstderr:\n%s", err, stderr)
}

if !strings.Contains(stdout, "Resources: 1 added, 0 changed, 0 destroyed") {
t.Errorf("incorrect apply tally; want 1 added:\n%s", stdout)
if !strings.Contains(stdout, "Resources: 2 added, 0 changed, 0 destroyed") {
t.Errorf("incorrect apply tally; want 2 added:\n%s", stdout)
}

state, err := tf.LocalState()
Expand All @@ -182,6 +184,7 @@ func TestAutoApplyInAutomation(t *testing.T) {

wantResources := []string{
"data.template_file.test",
"null_resource.id_known_after_apply",
"null_resource.test",
}

Expand Down Expand Up @@ -231,8 +234,8 @@ func TestPlanOnlyInAutomation(t *testing.T) {
t.Fatalf("unexpected plan error: %s\nstderr:\n%s", err, stderr)
}

if !strings.Contains(stdout, "1 to add, 0 to change, 0 to destroy") {
t.Errorf("incorrect plan tally; want 1 to add:\n%s", stdout)
if !strings.Contains(stdout, "2 to add, 0 to change, 0 to destroy") {
t.Errorf("incorrect plan tally; want 2 to add:\n%s", stdout)
}

// Because we're running with TF_IN_AUTOMATION set, we should not see
Expand Down
23 changes: 15 additions & 8 deletions command/e2etest/primary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ func TestPrimarySeparatePlan(t *testing.T) {
t.Fatalf("unexpected plan error: %s\nstderr:\n%s", err, stderr)
}

if !strings.Contains(stdout, "1 to add, 0 to change, 0 to destroy") {
t.Errorf("incorrect plan tally; want 1 to add:\n%s", stdout)
if !strings.Contains(stdout, "2 to add, 0 to change, 0 to destroy") {
t.Errorf("incorrect plan tally; want 2 to add:\n%s", stdout)
}

if !strings.Contains(stdout, "This plan was saved to: tfplan") {
Expand All @@ -66,18 +66,24 @@ func TestPrimarySeparatePlan(t *testing.T) {
t.Errorf("missing next-step instruction in plan output\n%s", stdout)
}

// regression test for GitHub issue #26320
if !strings.Contains(stdout, "\"greeting\" = \"Hello, world\"\n") {
t.Errorf("regression: known value for triggers[\"greeting\"] incorrectly marked as unknown in plan output\n%s", stdout)
}

plan, err := tf.Plan("tfplan")
if err != nil {
t.Fatalf("failed to read plan file: %s", err)
}

diffResources := plan.Changes.Resources
if len(diffResources) != 1 {
if len(diffResources) != 2 {
t.Errorf("incorrect number of resources in plan")
}

expected := map[string]plans.Action{
"null_resource.test": plans.Create,
"null_resource.id_known_after_apply": plans.Create,
"null_resource.test": plans.Create,
}

for _, r := range diffResources {
Expand All @@ -96,8 +102,8 @@ func TestPrimarySeparatePlan(t *testing.T) {
t.Fatalf("unexpected apply error: %s\nstderr:\n%s", err, stderr)
}

if !strings.Contains(stdout, "Resources: 1 added, 0 changed, 0 destroyed") {
t.Errorf("incorrect apply tally; want 1 added:\n%s", stdout)
if !strings.Contains(stdout, "Resources: 2 added, 0 changed, 0 destroyed") {
t.Errorf("incorrect apply tally; want 2 added:\n%s", stdout)
}

state, err := tf.LocalState()
Expand All @@ -114,6 +120,7 @@ func TestPrimarySeparatePlan(t *testing.T) {

wantResources := []string{
"data.template_file.test",
"null_resource.id_known_after_apply",
"null_resource.test",
}

Expand All @@ -127,8 +134,8 @@ func TestPrimarySeparatePlan(t *testing.T) {
t.Fatalf("unexpected destroy error: %s\nstderr:\n%s", err, stderr)
}

if !strings.Contains(stdout, "Resources: 1 destroyed") {
t.Errorf("incorrect destroy tally; want 1 destroyed:\n%s", stdout)
if !strings.Contains(stdout, "Resources: 2 destroyed") {
t.Errorf("incorrect destroy tally; want 2 destroyed:\n%s", stdout)
}

state, err = tf.LocalState()
Expand Down
12 changes: 11 additions & 1 deletion command/e2etest/testdata/full-workflow-null/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,19 @@ data "template_file" "test" {
}
}

resource "null_resource" "id_known_after_apply" {
}

locals {
map_with_unknown_value = tomap({
foo = null_resource.id_known_after_apply.id
greeting = "${data.template_file.test.rendered}"
})
}

resource "null_resource" "test" {
triggers = {
greeting = "${data.template_file.test.rendered}"
greeting = lookup(local.map_with_unknown_value, "greeting")
}
}

Expand Down
2 changes: 1 addition & 1 deletion lang/funcs/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ var LookupFunc = function.New(&function.Spec{
mapVar := args[0]
lookupKey := args[1].AsString()

if !mapVar.IsWhollyKnown() {
if !mapVar.IsKnown() {
return cty.UnknownVal(retType), nil
}

Expand Down

0 comments on commit 04cbaeb

Please sign in to comment.