From 1ae47ae31410e951426bfef90b103d6016e717b2 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 20 Nov 2019 16:31:40 -0800 Subject: [PATCH] states/statefile: tolerate and ignore invalid depends_on in version 3 The state refactoring command "terraform state mv" in Terraform 0.11 does not update existing dependency addresses recorded in the state when it moves objects around, and Terraform only updates the dependency addresses in the state when it performs a full update on a resource instance, and so it's a common problem for folks updating from Terraform 0.11 with resource names that are not valid identifiers to run into state upgrade errors even though they have followed the instructions produced by "terraform 0.12checklist". Dependencies are synced from config during every refresh walk anyway, so in practice we can get away with just discarding invalid dependency addresses and letting the refresh walk update them. In practice these addresses are unlikely to be pointing at a resource that actually exists anyway, because if so Terraform 0.12's configuration parser wouldn't be able to interpret it. Discarding invalid dependency addresses allows the state upgrade to complete successfully in such cases and thus gives the refresh step an opportunity to repair the problem. --- .../roundtrip/v3-invalid-depends.in.tfstate | 42 +++++++++++++++++++ .../roundtrip/v3-invalid-depends.out.tfstate | 33 +++++++++++++++ states/statefile/version3_upgrade.go | 29 +++++++++++-- 3 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 states/statefile/testdata/roundtrip/v3-invalid-depends.in.tfstate create mode 100644 states/statefile/testdata/roundtrip/v3-invalid-depends.out.tfstate diff --git a/states/statefile/testdata/roundtrip/v3-invalid-depends.in.tfstate b/states/statefile/testdata/roundtrip/v3-invalid-depends.in.tfstate new file mode 100644 index 000000000..6943b6139 --- /dev/null +++ b/states/statefile/testdata/roundtrip/v3-invalid-depends.in.tfstate @@ -0,0 +1,42 @@ +{ + "version": 3, + "terraform_version": "0.7.13", + "serial": 0, + "lineage": "f2968801-fa14-41ab-a044-224f3a4adf04", + "modules": [ + { + "path": [ + "root" + ], + "outputs": { + "numbers": { + "sensitive": false, + "type": "string", + "value": "0,1" + } + }, + "resources": { + "null_resource.bar": { + "type": "null_resource", + "depends_on": [ + "null_resource.valid", + "null_resource.1invalid" + ], + "primary": { + "id": "5388490630832483079", + "attributes": { + "id": "5388490630832483079", + "triggers.%": "1", + "triggers.whaaat": "0,1" + }, + "meta": {}, + "tainted": false + }, + "deposed": [], + "provider": "" + } + }, + "depends_on": [] + } + ] +} \ No newline at end of file diff --git a/states/statefile/testdata/roundtrip/v3-invalid-depends.out.tfstate b/states/statefile/testdata/roundtrip/v3-invalid-depends.out.tfstate new file mode 100644 index 000000000..5fdef34f6 --- /dev/null +++ b/states/statefile/testdata/roundtrip/v3-invalid-depends.out.tfstate @@ -0,0 +1,33 @@ +{ + "version": 4, + "serial": 0, + "lineage": "f2968801-fa14-41ab-a044-224f3a4adf04", + "terraform_version": "0.7.13", + "outputs": { + "numbers": { + "type": "string", + "value": "0,1" + } + }, + "resources": [ + { + "mode": "managed", + "type": "null_resource", + "name": "bar", + "provider": "provider.null", + "instances": [ + { + "schema_version": 0, + "attributes_flat": { + "id": "5388490630832483079", + "triggers.%": "1", + "triggers.whaaat": "0,1" + }, + "depends_on": [ + "null_resource.valid" + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/states/statefile/version3_upgrade.go b/states/statefile/version3_upgrade.go index 943e0f45c..64c3c1afb 100644 --- a/states/statefile/version3_upgrade.go +++ b/states/statefile/version3_upgrade.go @@ -3,6 +3,7 @@ package statefile import ( "encoding/json" "fmt" + "log" "strconv" "strings" @@ -299,13 +300,33 @@ func upgradeInstanceObjectV3ToV4(rsOld *resourceStateV2, isOld *instanceStateV2, } } - dependencies := make([]string, len(rsOld.Dependencies)) - for i, v := range rsOld.Dependencies { + dependencies := make([]string, 0, len(rsOld.Dependencies)) + for _, v := range rsOld.Dependencies { depStr, err := parseLegacyDependency(v) if err != nil { - return nil, fmt.Errorf("invalid dependency reference %q: %s", v, err) + // We just drop invalid dependencies on the floor here, because + // they tend to get left behind in Terraform 0.11 when resources + // are renamed or moved between modules and there's no automatic + // way to fix them here. In practice it shouldn't hurt to miss + // a few dependency edges in the state because a subsequent plan + // will run a refresh walk first and re-synchronize the + // dependencies with the configuration. + // + // There is one rough edges where this can cause an incorrect + // result, though: If the first command the user runs after + // upgrading to Terraform 0.12 uses -refresh=false and thus + // prevents the dependency reorganization from occurring _and_ + // that initial plan discovered "orphaned" resources (not present + // in configuration any longer) then when the plan is applied the + // destroy ordering will be incorrect for the instances of those + // resources. We expect that is a rare enough situation that it + // isn't a big deal, and even when it _does_ occur it's common for + // the apply to succeed anyway unless many separate resources with + // complex inter-dependencies are all orphaned at once. + log.Printf("statefile: ignoring invalid dependency address %q while upgrading from state version 3 to version 4: %s", v, err) + continue } - dependencies[i] = depStr + dependencies = append(dependencies, depStr) } return &instanceObjectStateV4{