diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aeb2dcc44..34813656b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ BUG FIXES: * Fix Global Schema Cache not working in provider acceptance tests ([#1054](https://github.com/opentofu/opentofu/pull/1054)) * Fix `tofu show` and `tofu state show` not working with state files referencing Terraform registry providers in some instances ([#1141](https://github.com/opentofu/opentofu/pull/1141)) * Improved stability on 32-bit architectures ([#1154](https://github.com/opentofu/opentofu/pull/1154)) +* Don't show false update action when import resource with sensitive datasource([#1220](https://github.com/opentofu/opentofu/pull/1220)) * Fix panic when provisioner source and content are both null ([#1376](https://github.com/opentofu/opentofu/pull/1376)) * Fix large number will be truncated in plan ([#1382](https://github.com/opentofu/opentofu/pull/1382)) diff --git a/internal/tofu/context_import_test.go b/internal/tofu/context_import_test.go index a1d48ef26e..c67683e5ca 100644 --- a/internal/tofu/context_import_test.go +++ b/internal/tofu/context_import_test.go @@ -115,6 +115,83 @@ resource "aws_instance" "foo" { } } +func TestContextImport_importResourceWithSensitiveDataSource(t *testing.T) { + p := testProvider("aws") + m := testModuleInline(t, map[string]string{ + "main.tf": ` +provider "aws" { + foo = "bar" +} + +data "aws_sensitive_data_source" "source" { + id = "source_id" +} + +resource "aws_instance" "foo" { + id = "bar" + var = data.aws_sensitive_data_source.source.value +} +`}) + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), + }, + }) + + p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{ + ImportedResources: []providers.ImportedResource{ + { + TypeName: "aws_instance", + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + }), + }, + }, + } + + p.ReadDataSourceResponse = &providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("source_id"), + "value": cty.StringVal("pass"), + }), + } + + p.ReadResourceResponse = &providers.ReadResourceResponse{ + NewState: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + "var": cty.StringVal("pass"), + }), + } + + state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ + Targets: []*ImportTarget{ + { + CommandLineImportTarget: &CommandLineImportTarget{ + Addr: addrs.RootModuleInstance.ResourceInstance( + addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, + ), + ID: "bar", + }, + }, + }, + }) + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } + + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testImportResourceWithSensitiveDataSource) + if actual != expected { + t.Fatalf("bad: \n%s", actual) + } + + obj := state.ResourceInstance(mustResourceInstanceAddr("aws_instance.foo")) + if len(obj.Current.AttrSensitivePaths) != 1 { + t.Fatalf("Expected 1 sensitive mark for aws_instance.foo, got %#v\n", obj.Current.AttrSensitivePaths) + } +} + func TestContextImport_collision(t *testing.T) { p := testProvider("aws") m := testModule(t, "import-provider") @@ -1090,6 +1167,17 @@ aws_instance.foo.0: provider = provider["registry.opentofu.org/hashicorp/aws"] ` +const testImportResourceWithSensitiveDataSource = ` +data.aws_sensitive_data_source.source: + ID = source_id + provider = provider["registry.opentofu.org/hashicorp/aws"] + value = pass +aws_instance.foo: + ID = bar + provider = provider["registry.opentofu.org/hashicorp/aws"] + var = pass +` + const testImportModuleStr = ` module.child[0]: diff --git a/internal/tofu/context_plan2_test.go b/internal/tofu/context_plan2_test.go index d55480ce38..92f0797f78 100644 --- a/internal/tofu/context_plan2_test.go +++ b/internal/tofu/context_plan2_test.go @@ -6168,3 +6168,111 @@ func TestContext2Plan_removedModuleButModuleBlockStillExists(t *testing.T) { t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want) } } + +func TestContext2Plan_importResourceWithSensitiveDataSource(t *testing.T) { + addr := mustResourceInstanceAddr("test_object.b") + m := testModuleInline(t, map[string]string{ + "main.tf": ` + data "test_data_source" "a" { + } + resource "test_object" "b" { + test_string = data.test_data_source.a.test_string + } + import { + to = test_object.b + id = "123" + } + `, + }) + + p := &MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + Provider: providers.Schema{Block: simpleTestSchema()}, + ResourceTypes: map[string]providers.Schema{ + "test_object": {Block: simpleTestSchema()}, + }, + DataSources: map[string]providers.Schema{ + "test_data_source": {Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "test_string": { + Type: cty.String, + Computed: true, + Sensitive: true, + }, + }, + }}, + }, + }, + } + hook := new(MockHook) + ctx := testContext2(t, &ContextOpts{ + Hooks: []Hook{hook}, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + p.ReadDataSourceResponse = &providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "test_string": cty.StringVal("foo"), + }), + } + + p.ReadResourceResponse = &providers.ReadResourceResponse{ + NewState: cty.ObjectVal(map[string]cty.Value{ + "test_string": cty.StringVal("foo"), + }), + } + p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{ + ImportedResources: []providers.ImportedResource{ + { + TypeName: "test_object", + State: cty.ObjectVal(map[string]cty.Value{ + "test_string": cty.StringVal("foo"), + }), + }, + }, + } + + plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + if diags.HasErrors() { + t.Fatalf("unexpected errors\n%s", diags.Err().Error()) + } + + t.Run(addr.String(), func(t *testing.T) { + instPlan := plan.Changes.ResourceInstance(addr) + if instPlan == nil { + t.Fatalf("no plan for %s at all", addr) + } + + if got, want := instPlan.Addr, addr; !got.Equal(want) { + t.Errorf("wrong current address\ngot: %s\nwant: %s", got, want) + } + if got, want := instPlan.PrevRunAddr, addr; !got.Equal(want) { + t.Errorf("wrong previous run address\ngot: %s\nwant: %s", got, want) + } + if got, want := instPlan.Action, plans.NoOp; got != want { + t.Errorf("wrong planned action\ngot: %s\nwant: %s", got, want) + } + if got, want := instPlan.ActionReason, plans.ResourceInstanceChangeNoReason; got != want { + t.Errorf("wrong action reason\ngot: %s\nwant: %s", got, want) + } + if instPlan.Importing.ID != "123" { + t.Errorf("expected import change from \"123\", got non-import change") + } + + if !hook.PrePlanImportCalled { + t.Fatalf("PostPlanImport hook not called") + } + if addr, wantAddr := hook.PrePlanImportAddr, instPlan.Addr; !addr.Equal(wantAddr) { + t.Errorf("expected addr to be %s, but was %s", wantAddr, addr) + } + + if !hook.PostPlanImportCalled { + t.Fatalf("PostPlanImport hook not called") + } + if addr, wantAddr := hook.PostPlanImportAddr, instPlan.Addr; !addr.Equal(wantAddr) { + t.Errorf("expected addr to be %s, but was %s", wantAddr, addr) + } + }) +} diff --git a/internal/tofu/context_test.go b/internal/tofu/context_test.go index 1e942fb2a5..cb8eee08dc 100644 --- a/internal/tofu/context_test.go +++ b/internal/tofu/context_test.go @@ -672,6 +672,19 @@ func testProviderSchema(name string) *providers.GetProviderSchemaResponse { }, }, }, + name + "_sensitive_data_source": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + "value": { + Type: cty.String, + Optional: true, + Sensitive: true, + }, + }, + }, }, }) } diff --git a/internal/tofu/marks.go b/internal/tofu/marks.go index 2ea320ffe5..725ccf9728 100644 --- a/internal/tofu/marks.go +++ b/internal/tofu/marks.go @@ -58,3 +58,11 @@ func marksEqual(a, b []cty.PathValueMarks) bool { return true } + +func copyMarksFromValue(dst, src cty.Value) cty.Value { + _, pvm := src.UnmarkDeepWithPaths() + if len(pvm) == 0 { + return dst + } + return dst.MarkWithPaths(pvm) +} diff --git a/internal/tofu/node_resource_import.go b/internal/tofu/node_resource_import.go index 3343e07e19..c176224173 100644 --- a/internal/tofu/node_resource_import.go +++ b/internal/tofu/node_resource_import.go @@ -10,6 +10,8 @@ import ( "log" "github.com/opentofu/opentofu/internal/addrs" + "github.com/opentofu/opentofu/internal/configs" + "github.com/opentofu/opentofu/internal/configs/configschema" "github.com/opentofu/opentofu/internal/providers" "github.com/opentofu/opentofu/internal/states" "github.com/opentofu/opentofu/internal/tfdiags" @@ -21,6 +23,10 @@ type graphNodeImportState struct { ProviderAddr addrs.AbsProviderConfig // Provider address given by the user, or implied by the resource type ResolvedProvider addrs.AbsProviderConfig // provider node address after resolution + Schema *configschema.Block // Schema for processing the configuration body + SchemaVersion uint64 // Schema version of "Schema", as decided by the provider + Config *configs.Resource // Config is the resource in the config + states []providers.ImportedResource } @@ -178,6 +184,9 @@ func (n *graphNodeImportState) DynamicExpand(ctx EvalContext) (*Graph, error) { TargetAddr: addrs[i], State: state, ResolvedProvider: n.ResolvedProvider, + Schema: n.Schema, + SchemaVersion: n.SchemaVersion, + Config: n.Config, }) } @@ -194,6 +203,11 @@ type graphNodeImportStateSub struct { TargetAddr addrs.AbsResourceInstance State providers.ImportedResource ResolvedProvider addrs.AbsProviderConfig + + Schema *configschema.Block // Schema for processing the configuration body + SchemaVersion uint64 // Schema version of "Schema", as decided by the provider + Config *configs.Resource // Config is the resource in the config + } var ( @@ -251,6 +265,13 @@ func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) (di return diags } + // Insert marks from configuration + if n.Config != nil { + // Since the import command allow import resource with incomplete configuration, we ignore diagnostics here + valueWithConfigurationSchemaMarks, _, _ := ctx.EvaluateBlock(n.Config.Config, n.Schema, nil, EvalDataForNoInstanceKey) + state.Value = copyMarksFromValue(state.Value, valueWithConfigurationSchemaMarks) + } + diags = diags.Append(riNode.writeResourceInstanceState(ctx, state, workingState)) return diags } diff --git a/internal/tofu/node_resource_plan.go b/internal/tofu/node_resource_plan.go index 10d07aaa90..c2f8ad1750 100644 --- a/internal/tofu/node_resource_plan.go +++ b/internal/tofu/node_resource_plan.go @@ -334,6 +334,9 @@ func (n *nodeExpandPlannableResource) resourceInstanceSubgraph(ctx EvalContext, Addr: c.Addr, ID: c.ID, ResolvedProvider: n.ResolvedProvider, + Schema: n.Schema, + SchemaVersion: n.SchemaVersion, + Config: n.Config, } } } diff --git a/internal/tofu/node_resource_plan_instance.go b/internal/tofu/node_resource_plan_instance.go index 8c60b44085..44059b4b73 100644 --- a/internal/tofu/node_resource_plan_instance.go +++ b/internal/tofu/node_resource_plan_instance.go @@ -554,6 +554,16 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. return instanceRefreshState, diags } + // Insert marks from configuration + if n.Config != nil { + valueWithConfigurationSchemaMarks, _, configDiags := ctx.EvaluateBlock(n.Config.Config, n.Schema, nil, EvalDataForNoInstanceKey) + diags = diags.Append(configDiags) + if configDiags.HasErrors() { + return instanceRefreshState, diags + } + instanceRefreshState.Value = copyMarksFromValue(instanceRefreshState.Value, valueWithConfigurationSchemaMarks) + } + // If we're importing and generating config, generate it now. if len(n.generateConfigPath) > 0 { if n.Config != nil {