opentofu/internal/tofu/graph_builder_plan_test.go

277 lines
8.4 KiB
Go
Raw Normal View History

// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0
2023-09-20 07:16:53 -05:00
package tofu
import (
"strings"
"testing"
core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary Going back a long time we've had a special magic behavior which tries to recognize a situation where a module author either added or removed the "count" argument from a resource that already has instances, and to silently rename the zeroth or no-key instance so that we don't plan to destroy and recreate the associated object. Now we have a more general idea of "move statements", and specifically the idea of "implied" move statements which replicates the same heuristic we used to use for this behavior, we can treat this magic renaming rule as just another "move statement", special only in that Terraform generates it automatically rather than it being written out explicitly in the configuration. In return for wiring that in, we can now remove altogether the NodeCountBoundary graph node type and its associated graph transformer, CountBoundaryTransformer. We handle moves as a preprocessing step before building the plan graph, so we no longer need to include any special nodes in the graph to deal with that situation. The test updates here are mainly for the graph builders themselves, to acknowledge that indeed we're no longer inserting the NodeCountBoundary vertices. The vertices that NodeCountBoundary previously depended on now become dependencies of the special "root" vertex, although in many cases here we don't see that explicitly because of the transitive reduction algorithm, which notices when there's already an equivalent indirect dependency chain and removes the redundant edge. We already have plenty of test coverage for these "count boundary" cases in the context tests whose names start with TestContext2Plan_count and TestContext2Apply_resourceCount, all of which continued to pass here without any modification and so are not visible in the diff. The test functions particularly relevant to this situation are: - TestContext2Plan_countIncreaseFromNotSet - TestContext2Plan_countDecreaseToOne - TestContext2Plan_countOneIndex - TestContext2Apply_countDecreaseToOneCorrupted The last of those in particular deals with the situation where we have both a no-key instance _and_ a zero-key instance in the prior state, which is interesting here because to exercises an intentional interaction between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves, where we intentionally generate an implied move statement that produces a collision and then expect ApplyMoves to deal with it in the same way as it would deal with all other collisions, and thus ensure we handle both the explicit and implied collisions in the same way. This does affect some UI-level tests, because a nice side-effect of this new treatment of this old feature is that we can now report explicitly in the UI that we're assigning new addresses to these objects, whereas before we just said nothing and hoped the user would just guess what had happened and why they therefore weren't seeing a diff. The backend/local plan tests actually had a pre-existing bug where they were using a state with a different instance key than the config called for but getting away with it because we'd previously silently fix it up. That's still fixed up, but now done with an explicit mention in the UI and so I made the state consistent with the configuration here so that the tests would be able to recognize _real_ differences where present, as opposed to the errant difference caused by that inconsistency.
2021-09-17 17:32:32 -05:00
"github.com/google/go-cmp/cmp"
"github.com/zclconf/go-cty/cty"
"github.com/opentofu/opentofu/internal/addrs"
"github.com/opentofu/opentofu/internal/configs/configschema"
"github.com/opentofu/opentofu/internal/providers"
)
func TestPlanGraphBuilder_impl(t *testing.T) {
var _ GraphBuilder = new(PlanGraphBuilder)
}
func TestPlanGraphBuilder(t *testing.T) {
awsProvider := &MockProvider{
GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{
Provider: providers.Schema{Block: simpleTestSchema()},
ResourceTypes: map[string]providers.Schema{
"aws_security_group": {Block: simpleTestSchema()},
"aws_instance": {Block: simpleTestSchema()},
"aws_load_balancer": {Block: simpleTestSchema()},
},
},
}
openstackProvider := mockProviderWithResourceTypeSchema("openstack_floating_ip", simpleTestSchema())
plugins := newContextPlugins(map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("aws"): providers.FactoryFixed(awsProvider),
addrs.NewDefaultProvider("openstack"): providers.FactoryFixed(openstackProvider),
}, nil)
b := &PlanGraphBuilder{
Config: testModule(t, "graph-builder-plan-basic"),
Plugins: plugins,
Operation: walkPlan,
}
g, err := b.Build(addrs.RootModuleInstance)
if err != nil {
t.Fatalf("err: %s", err)
}
if g.Path.String() != addrs.RootModuleInstance.String() {
t.Fatalf("wrong module path %q", g.Path)
}
core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary Going back a long time we've had a special magic behavior which tries to recognize a situation where a module author either added or removed the "count" argument from a resource that already has instances, and to silently rename the zeroth or no-key instance so that we don't plan to destroy and recreate the associated object. Now we have a more general idea of "move statements", and specifically the idea of "implied" move statements which replicates the same heuristic we used to use for this behavior, we can treat this magic renaming rule as just another "move statement", special only in that Terraform generates it automatically rather than it being written out explicitly in the configuration. In return for wiring that in, we can now remove altogether the NodeCountBoundary graph node type and its associated graph transformer, CountBoundaryTransformer. We handle moves as a preprocessing step before building the plan graph, so we no longer need to include any special nodes in the graph to deal with that situation. The test updates here are mainly for the graph builders themselves, to acknowledge that indeed we're no longer inserting the NodeCountBoundary vertices. The vertices that NodeCountBoundary previously depended on now become dependencies of the special "root" vertex, although in many cases here we don't see that explicitly because of the transitive reduction algorithm, which notices when there's already an equivalent indirect dependency chain and removes the redundant edge. We already have plenty of test coverage for these "count boundary" cases in the context tests whose names start with TestContext2Plan_count and TestContext2Apply_resourceCount, all of which continued to pass here without any modification and so are not visible in the diff. The test functions particularly relevant to this situation are: - TestContext2Plan_countIncreaseFromNotSet - TestContext2Plan_countDecreaseToOne - TestContext2Plan_countOneIndex - TestContext2Apply_countDecreaseToOneCorrupted The last of those in particular deals with the situation where we have both a no-key instance _and_ a zero-key instance in the prior state, which is interesting here because to exercises an intentional interaction between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves, where we intentionally generate an implied move statement that produces a collision and then expect ApplyMoves to deal with it in the same way as it would deal with all other collisions, and thus ensure we handle both the explicit and implied collisions in the same way. This does affect some UI-level tests, because a nice side-effect of this new treatment of this old feature is that we can now report explicitly in the UI that we're assigning new addresses to these objects, whereas before we just said nothing and hoped the user would just guess what had happened and why they therefore weren't seeing a diff. The backend/local plan tests actually had a pre-existing bug where they were using a state with a different instance key than the config called for but getting away with it because we'd previously silently fix it up. That's still fixed up, but now done with an explicit mention in the UI and so I made the state consistent with the configuration here so that the tests would be able to recognize _real_ differences where present, as opposed to the errant difference caused by that inconsistency.
2021-09-17 17:32:32 -05:00
got := strings.TrimSpace(g.String())
want := strings.TrimSpace(testPlanGraphBuilderStr)
if diff := cmp.Diff(want, got); diff != "" {
t.Fatalf("wrong result\n%s", diff)
}
}
func TestPlanGraphBuilder_dynamicBlock(t *testing.T) {
provider := mockProviderWithResourceTypeSchema("test_thing", &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Computed: true},
"list": {Type: cty.List(cty.String), Computed: true},
},
BlockTypes: map[string]*configschema.NestedBlock{
"nested": {
Nesting: configschema.NestingList,
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"foo": {Type: cty.String, Optional: true},
},
},
},
},
})
plugins := newContextPlugins(map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): providers.FactoryFixed(provider),
}, nil)
b := &PlanGraphBuilder{
Config: testModule(t, "graph-builder-plan-dynblock"),
Plugins: plugins,
Operation: walkPlan,
}
g, err := b.Build(addrs.RootModuleInstance)
if err != nil {
t.Fatalf("err: %s", err)
}
if g.Path.String() != addrs.RootModuleInstance.String() {
t.Fatalf("wrong module path %q", g.Path)
}
// This test is here to make sure we properly detect references inside
// the special "dynamic" block construct. The most important thing here
// is that at the end test_thing.c depends on both test_thing.a and
// test_thing.b. Other details might shift over time as other logic in
// the graph builders changes.
core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary Going back a long time we've had a special magic behavior which tries to recognize a situation where a module author either added or removed the "count" argument from a resource that already has instances, and to silently rename the zeroth or no-key instance so that we don't plan to destroy and recreate the associated object. Now we have a more general idea of "move statements", and specifically the idea of "implied" move statements which replicates the same heuristic we used to use for this behavior, we can treat this magic renaming rule as just another "move statement", special only in that Terraform generates it automatically rather than it being written out explicitly in the configuration. In return for wiring that in, we can now remove altogether the NodeCountBoundary graph node type and its associated graph transformer, CountBoundaryTransformer. We handle moves as a preprocessing step before building the plan graph, so we no longer need to include any special nodes in the graph to deal with that situation. The test updates here are mainly for the graph builders themselves, to acknowledge that indeed we're no longer inserting the NodeCountBoundary vertices. The vertices that NodeCountBoundary previously depended on now become dependencies of the special "root" vertex, although in many cases here we don't see that explicitly because of the transitive reduction algorithm, which notices when there's already an equivalent indirect dependency chain and removes the redundant edge. We already have plenty of test coverage for these "count boundary" cases in the context tests whose names start with TestContext2Plan_count and TestContext2Apply_resourceCount, all of which continued to pass here without any modification and so are not visible in the diff. The test functions particularly relevant to this situation are: - TestContext2Plan_countIncreaseFromNotSet - TestContext2Plan_countDecreaseToOne - TestContext2Plan_countOneIndex - TestContext2Apply_countDecreaseToOneCorrupted The last of those in particular deals with the situation where we have both a no-key instance _and_ a zero-key instance in the prior state, which is interesting here because to exercises an intentional interaction between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves, where we intentionally generate an implied move statement that produces a collision and then expect ApplyMoves to deal with it in the same way as it would deal with all other collisions, and thus ensure we handle both the explicit and implied collisions in the same way. This does affect some UI-level tests, because a nice side-effect of this new treatment of this old feature is that we can now report explicitly in the UI that we're assigning new addresses to these objects, whereas before we just said nothing and hoped the user would just guess what had happened and why they therefore weren't seeing a diff. The backend/local plan tests actually had a pre-existing bug where they were using a state with a different instance key than the config called for but getting away with it because we'd previously silently fix it up. That's still fixed up, but now done with an explicit mention in the UI and so I made the state consistent with the configuration here so that the tests would be able to recognize _real_ differences where present, as opposed to the errant difference caused by that inconsistency.
2021-09-17 17:32:32 -05:00
got := strings.TrimSpace(g.String())
want := strings.TrimSpace(`
provider["registry.opentofu.org/hashicorp/test"]
provider["registry.opentofu.org/hashicorp/test"] (close)
test_thing.c (expand)
root
provider["registry.opentofu.org/hashicorp/test"] (close)
test_thing.a (expand)
provider["registry.opentofu.org/hashicorp/test"]
test_thing.b (expand)
provider["registry.opentofu.org/hashicorp/test"]
test_thing.c (expand)
test_thing.a (expand)
test_thing.b (expand)
`)
core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary Going back a long time we've had a special magic behavior which tries to recognize a situation where a module author either added or removed the "count" argument from a resource that already has instances, and to silently rename the zeroth or no-key instance so that we don't plan to destroy and recreate the associated object. Now we have a more general idea of "move statements", and specifically the idea of "implied" move statements which replicates the same heuristic we used to use for this behavior, we can treat this magic renaming rule as just another "move statement", special only in that Terraform generates it automatically rather than it being written out explicitly in the configuration. In return for wiring that in, we can now remove altogether the NodeCountBoundary graph node type and its associated graph transformer, CountBoundaryTransformer. We handle moves as a preprocessing step before building the plan graph, so we no longer need to include any special nodes in the graph to deal with that situation. The test updates here are mainly for the graph builders themselves, to acknowledge that indeed we're no longer inserting the NodeCountBoundary vertices. The vertices that NodeCountBoundary previously depended on now become dependencies of the special "root" vertex, although in many cases here we don't see that explicitly because of the transitive reduction algorithm, which notices when there's already an equivalent indirect dependency chain and removes the redundant edge. We already have plenty of test coverage for these "count boundary" cases in the context tests whose names start with TestContext2Plan_count and TestContext2Apply_resourceCount, all of which continued to pass here without any modification and so are not visible in the diff. The test functions particularly relevant to this situation are: - TestContext2Plan_countIncreaseFromNotSet - TestContext2Plan_countDecreaseToOne - TestContext2Plan_countOneIndex - TestContext2Apply_countDecreaseToOneCorrupted The last of those in particular deals with the situation where we have both a no-key instance _and_ a zero-key instance in the prior state, which is interesting here because to exercises an intentional interaction between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves, where we intentionally generate an implied move statement that produces a collision and then expect ApplyMoves to deal with it in the same way as it would deal with all other collisions, and thus ensure we handle both the explicit and implied collisions in the same way. This does affect some UI-level tests, because a nice side-effect of this new treatment of this old feature is that we can now report explicitly in the UI that we're assigning new addresses to these objects, whereas before we just said nothing and hoped the user would just guess what had happened and why they therefore weren't seeing a diff. The backend/local plan tests actually had a pre-existing bug where they were using a state with a different instance key than the config called for but getting away with it because we'd previously silently fix it up. That's still fixed up, but now done with an explicit mention in the UI and so I made the state consistent with the configuration here so that the tests would be able to recognize _real_ differences where present, as opposed to the errant difference caused by that inconsistency.
2021-09-17 17:32:32 -05:00
if diff := cmp.Diff(want, got); diff != "" {
t.Fatalf("wrong result\n%s", diff)
}
}
func TestPlanGraphBuilder_attrAsBlocks(t *testing.T) {
provider := mockProviderWithResourceTypeSchema("test_thing", &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Computed: true},
"nested": {
Type: cty.List(cty.Object(map[string]cty.Type{
"foo": cty.String,
})),
Optional: true,
},
},
})
plugins := newContextPlugins(map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): providers.FactoryFixed(provider),
}, nil)
b := &PlanGraphBuilder{
Config: testModule(t, "graph-builder-plan-attr-as-blocks"),
Plugins: plugins,
Operation: walkPlan,
}
g, err := b.Build(addrs.RootModuleInstance)
if err != nil {
t.Fatalf("err: %s", err)
}
if g.Path.String() != addrs.RootModuleInstance.String() {
t.Fatalf("wrong module path %q", g.Path)
}
// This test is here to make sure we properly detect references inside
// the "nested" block that is actually defined in the schema as a
// list-of-objects attribute. This requires some special effort
// inside lang.ReferencesInBlock to make sure it searches blocks of
// type "nested" along with an attribute named "nested".
core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary Going back a long time we've had a special magic behavior which tries to recognize a situation where a module author either added or removed the "count" argument from a resource that already has instances, and to silently rename the zeroth or no-key instance so that we don't plan to destroy and recreate the associated object. Now we have a more general idea of "move statements", and specifically the idea of "implied" move statements which replicates the same heuristic we used to use for this behavior, we can treat this magic renaming rule as just another "move statement", special only in that Terraform generates it automatically rather than it being written out explicitly in the configuration. In return for wiring that in, we can now remove altogether the NodeCountBoundary graph node type and its associated graph transformer, CountBoundaryTransformer. We handle moves as a preprocessing step before building the plan graph, so we no longer need to include any special nodes in the graph to deal with that situation. The test updates here are mainly for the graph builders themselves, to acknowledge that indeed we're no longer inserting the NodeCountBoundary vertices. The vertices that NodeCountBoundary previously depended on now become dependencies of the special "root" vertex, although in many cases here we don't see that explicitly because of the transitive reduction algorithm, which notices when there's already an equivalent indirect dependency chain and removes the redundant edge. We already have plenty of test coverage for these "count boundary" cases in the context tests whose names start with TestContext2Plan_count and TestContext2Apply_resourceCount, all of which continued to pass here without any modification and so are not visible in the diff. The test functions particularly relevant to this situation are: - TestContext2Plan_countIncreaseFromNotSet - TestContext2Plan_countDecreaseToOne - TestContext2Plan_countOneIndex - TestContext2Apply_countDecreaseToOneCorrupted The last of those in particular deals with the situation where we have both a no-key instance _and_ a zero-key instance in the prior state, which is interesting here because to exercises an intentional interaction between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves, where we intentionally generate an implied move statement that produces a collision and then expect ApplyMoves to deal with it in the same way as it would deal with all other collisions, and thus ensure we handle both the explicit and implied collisions in the same way. This does affect some UI-level tests, because a nice side-effect of this new treatment of this old feature is that we can now report explicitly in the UI that we're assigning new addresses to these objects, whereas before we just said nothing and hoped the user would just guess what had happened and why they therefore weren't seeing a diff. The backend/local plan tests actually had a pre-existing bug where they were using a state with a different instance key than the config called for but getting away with it because we'd previously silently fix it up. That's still fixed up, but now done with an explicit mention in the UI and so I made the state consistent with the configuration here so that the tests would be able to recognize _real_ differences where present, as opposed to the errant difference caused by that inconsistency.
2021-09-17 17:32:32 -05:00
got := strings.TrimSpace(g.String())
want := strings.TrimSpace(`
provider["registry.opentofu.org/hashicorp/test"]
provider["registry.opentofu.org/hashicorp/test"] (close)
test_thing.b (expand)
root
provider["registry.opentofu.org/hashicorp/test"] (close)
test_thing.a (expand)
provider["registry.opentofu.org/hashicorp/test"]
test_thing.b (expand)
test_thing.a (expand)
`)
core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary Going back a long time we've had a special magic behavior which tries to recognize a situation where a module author either added or removed the "count" argument from a resource that already has instances, and to silently rename the zeroth or no-key instance so that we don't plan to destroy and recreate the associated object. Now we have a more general idea of "move statements", and specifically the idea of "implied" move statements which replicates the same heuristic we used to use for this behavior, we can treat this magic renaming rule as just another "move statement", special only in that Terraform generates it automatically rather than it being written out explicitly in the configuration. In return for wiring that in, we can now remove altogether the NodeCountBoundary graph node type and its associated graph transformer, CountBoundaryTransformer. We handle moves as a preprocessing step before building the plan graph, so we no longer need to include any special nodes in the graph to deal with that situation. The test updates here are mainly for the graph builders themselves, to acknowledge that indeed we're no longer inserting the NodeCountBoundary vertices. The vertices that NodeCountBoundary previously depended on now become dependencies of the special "root" vertex, although in many cases here we don't see that explicitly because of the transitive reduction algorithm, which notices when there's already an equivalent indirect dependency chain and removes the redundant edge. We already have plenty of test coverage for these "count boundary" cases in the context tests whose names start with TestContext2Plan_count and TestContext2Apply_resourceCount, all of which continued to pass here without any modification and so are not visible in the diff. The test functions particularly relevant to this situation are: - TestContext2Plan_countIncreaseFromNotSet - TestContext2Plan_countDecreaseToOne - TestContext2Plan_countOneIndex - TestContext2Apply_countDecreaseToOneCorrupted The last of those in particular deals with the situation where we have both a no-key instance _and_ a zero-key instance in the prior state, which is interesting here because to exercises an intentional interaction between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves, where we intentionally generate an implied move statement that produces a collision and then expect ApplyMoves to deal with it in the same way as it would deal with all other collisions, and thus ensure we handle both the explicit and implied collisions in the same way. This does affect some UI-level tests, because a nice side-effect of this new treatment of this old feature is that we can now report explicitly in the UI that we're assigning new addresses to these objects, whereas before we just said nothing and hoped the user would just guess what had happened and why they therefore weren't seeing a diff. The backend/local plan tests actually had a pre-existing bug where they were using a state with a different instance key than the config called for but getting away with it because we'd previously silently fix it up. That's still fixed up, but now done with an explicit mention in the UI and so I made the state consistent with the configuration here so that the tests would be able to recognize _real_ differences where present, as opposed to the errant difference caused by that inconsistency.
2021-09-17 17:32:32 -05:00
if diff := cmp.Diff(want, got); diff != "" {
t.Fatalf("wrong result\n%s", diff)
}
}
func TestPlanGraphBuilder_targetModule(t *testing.T) {
b := &PlanGraphBuilder{
Config: testModule(t, "graph-builder-plan-target-module-provider"),
Plugins: simpleMockPluginLibrary(),
Targets: []addrs.Targetable{
addrs.RootModuleInstance.Child("child2", addrs.NoKey),
},
Operation: walkPlan,
}
g, err := b.Build(addrs.RootModuleInstance)
if err != nil {
t.Fatalf("err: %s", err)
}
t.Logf("Graph: %s", g.String())
testGraphNotContains(t, g, `module.child1.provider["registry.opentofu.org/hashicorp/test"]`)
testGraphNotContains(t, g, "module.child1.test_object.foo")
}
2019-06-12 10:07:32 -05:00
func TestPlanGraphBuilder_forEach(t *testing.T) {
awsProvider := mockProviderWithResourceTypeSchema("aws_instance", simpleTestSchema())
2019-06-12 10:07:32 -05:00
plugins := newContextPlugins(map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("aws"): providers.FactoryFixed(awsProvider),
}, nil)
2019-06-12 10:07:32 -05:00
b := &PlanGraphBuilder{
Config: testModule(t, "plan-for-each"),
Plugins: plugins,
Operation: walkPlan,
2019-06-12 10:07:32 -05:00
}
g, err := b.Build(addrs.RootModuleInstance)
if err != nil {
t.Fatalf("err: %s", err)
}
if g.Path.String() != addrs.RootModuleInstance.String() {
t.Fatalf("wrong module path %q", g.Path)
}
core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary Going back a long time we've had a special magic behavior which tries to recognize a situation where a module author either added or removed the "count" argument from a resource that already has instances, and to silently rename the zeroth or no-key instance so that we don't plan to destroy and recreate the associated object. Now we have a more general idea of "move statements", and specifically the idea of "implied" move statements which replicates the same heuristic we used to use for this behavior, we can treat this magic renaming rule as just another "move statement", special only in that Terraform generates it automatically rather than it being written out explicitly in the configuration. In return for wiring that in, we can now remove altogether the NodeCountBoundary graph node type and its associated graph transformer, CountBoundaryTransformer. We handle moves as a preprocessing step before building the plan graph, so we no longer need to include any special nodes in the graph to deal with that situation. The test updates here are mainly for the graph builders themselves, to acknowledge that indeed we're no longer inserting the NodeCountBoundary vertices. The vertices that NodeCountBoundary previously depended on now become dependencies of the special "root" vertex, although in many cases here we don't see that explicitly because of the transitive reduction algorithm, which notices when there's already an equivalent indirect dependency chain and removes the redundant edge. We already have plenty of test coverage for these "count boundary" cases in the context tests whose names start with TestContext2Plan_count and TestContext2Apply_resourceCount, all of which continued to pass here without any modification and so are not visible in the diff. The test functions particularly relevant to this situation are: - TestContext2Plan_countIncreaseFromNotSet - TestContext2Plan_countDecreaseToOne - TestContext2Plan_countOneIndex - TestContext2Apply_countDecreaseToOneCorrupted The last of those in particular deals with the situation where we have both a no-key instance _and_ a zero-key instance in the prior state, which is interesting here because to exercises an intentional interaction between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves, where we intentionally generate an implied move statement that produces a collision and then expect ApplyMoves to deal with it in the same way as it would deal with all other collisions, and thus ensure we handle both the explicit and implied collisions in the same way. This does affect some UI-level tests, because a nice side-effect of this new treatment of this old feature is that we can now report explicitly in the UI that we're assigning new addresses to these objects, whereas before we just said nothing and hoped the user would just guess what had happened and why they therefore weren't seeing a diff. The backend/local plan tests actually had a pre-existing bug where they were using a state with a different instance key than the config called for but getting away with it because we'd previously silently fix it up. That's still fixed up, but now done with an explicit mention in the UI and so I made the state consistent with the configuration here so that the tests would be able to recognize _real_ differences where present, as opposed to the errant difference caused by that inconsistency.
2021-09-17 17:32:32 -05:00
got := strings.TrimSpace(g.String())
2019-06-12 10:07:32 -05:00
// We're especially looking for the edge here, where aws_instance.bat
// has a dependency on aws_instance.boo
core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary Going back a long time we've had a special magic behavior which tries to recognize a situation where a module author either added or removed the "count" argument from a resource that already has instances, and to silently rename the zeroth or no-key instance so that we don't plan to destroy and recreate the associated object. Now we have a more general idea of "move statements", and specifically the idea of "implied" move statements which replicates the same heuristic we used to use for this behavior, we can treat this magic renaming rule as just another "move statement", special only in that Terraform generates it automatically rather than it being written out explicitly in the configuration. In return for wiring that in, we can now remove altogether the NodeCountBoundary graph node type and its associated graph transformer, CountBoundaryTransformer. We handle moves as a preprocessing step before building the plan graph, so we no longer need to include any special nodes in the graph to deal with that situation. The test updates here are mainly for the graph builders themselves, to acknowledge that indeed we're no longer inserting the NodeCountBoundary vertices. The vertices that NodeCountBoundary previously depended on now become dependencies of the special "root" vertex, although in many cases here we don't see that explicitly because of the transitive reduction algorithm, which notices when there's already an equivalent indirect dependency chain and removes the redundant edge. We already have plenty of test coverage for these "count boundary" cases in the context tests whose names start with TestContext2Plan_count and TestContext2Apply_resourceCount, all of which continued to pass here without any modification and so are not visible in the diff. The test functions particularly relevant to this situation are: - TestContext2Plan_countIncreaseFromNotSet - TestContext2Plan_countDecreaseToOne - TestContext2Plan_countOneIndex - TestContext2Apply_countDecreaseToOneCorrupted The last of those in particular deals with the situation where we have both a no-key instance _and_ a zero-key instance in the prior state, which is interesting here because to exercises an intentional interaction between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves, where we intentionally generate an implied move statement that produces a collision and then expect ApplyMoves to deal with it in the same way as it would deal with all other collisions, and thus ensure we handle both the explicit and implied collisions in the same way. This does affect some UI-level tests, because a nice side-effect of this new treatment of this old feature is that we can now report explicitly in the UI that we're assigning new addresses to these objects, whereas before we just said nothing and hoped the user would just guess what had happened and why they therefore weren't seeing a diff. The backend/local plan tests actually had a pre-existing bug where they were using a state with a different instance key than the config called for but getting away with it because we'd previously silently fix it up. That's still fixed up, but now done with an explicit mention in the UI and so I made the state consistent with the configuration here so that the tests would be able to recognize _real_ differences where present, as opposed to the errant difference caused by that inconsistency.
2021-09-17 17:32:32 -05:00
want := strings.TrimSpace(testPlanGraphBuilderForEachStr)
if diff := cmp.Diff(want, got); diff != "" {
t.Fatalf("wrong result\n%s", diff)
2019-06-12 10:07:32 -05:00
}
}
const testPlanGraphBuilderStr = `
aws_instance.web (expand)
aws_security_group.firewall (expand)
var.foo
aws_load_balancer.weblb (expand)
aws_instance.web (expand)
aws_security_group.firewall (expand)
provider["registry.opentofu.org/hashicorp/aws"]
local.instance_id (expand)
aws_instance.web (expand)
openstack_floating_ip.random (expand)
provider["registry.opentofu.org/hashicorp/openstack"]
output.instance_id (expand)
local.instance_id (expand)
provider["registry.opentofu.org/hashicorp/aws"]
openstack_floating_ip.random (expand)
provider["registry.opentofu.org/hashicorp/aws"] (close)
aws_load_balancer.weblb (expand)
provider["registry.opentofu.org/hashicorp/openstack"]
provider["registry.opentofu.org/hashicorp/openstack"] (close)
openstack_floating_ip.random (expand)
root
output.instance_id (expand)
provider["registry.opentofu.org/hashicorp/aws"] (close)
provider["registry.opentofu.org/hashicorp/openstack"] (close)
var.foo
`
2019-06-12 10:07:32 -05:00
const testPlanGraphBuilderForEachStr = `
aws_instance.bar (expand)
provider["registry.opentofu.org/hashicorp/aws"]
aws_instance.bar2 (expand)
provider["registry.opentofu.org/hashicorp/aws"]
aws_instance.bat (expand)
aws_instance.boo (expand)
aws_instance.baz (expand)
provider["registry.opentofu.org/hashicorp/aws"]
aws_instance.boo (expand)
provider["registry.opentofu.org/hashicorp/aws"]
aws_instance.foo (expand)
provider["registry.opentofu.org/hashicorp/aws"]
provider["registry.opentofu.org/hashicorp/aws"]
provider["registry.opentofu.org/hashicorp/aws"] (close)
aws_instance.bar (expand)
aws_instance.bar2 (expand)
aws_instance.bat (expand)
aws_instance.baz (expand)
aws_instance.foo (expand)
2019-06-12 10:07:32 -05:00
root
provider["registry.opentofu.org/hashicorp/aws"] (close)
2019-06-12 10:07:32 -05:00
`