From 08720b6612479c85c03e11c16a88d986b24fb121 Mon Sep 17 00:00:00 2001 From: Paddy Carver Date: Wed, 22 Sep 2021 06:03:06 -0700 Subject: [PATCH 1/6] Fix planning. This fixes #175. Essentially, our behavior did exactly what we thought it did, but didn't combine with Terraform in the ways we thought it would. We need to set all computed attributes that are null in the _config_ to unknown, not all computed attributes that are null in the _plan_ to unknown, because once a computed attribute has a value in state, it will not have a null plan again unless the provider modifies the plan to null explicitly. This means once a computed attribute gets a value, under the existing code, it can't be updated by the provider. As part of this change, I moved when the null->unknown transform happens. It used to happen at the very end, which gave provider developers no opportunity to override the auto-unknown values with values the provider may know at plan time, such as a provider-level default or the combination of other fields, etc. By doing this first, the provider has the ability to understand that the value is unknown and replace it with a known value. I also fixed a bug that would return an error when trying to use Schema.AttributeAtPath with a path that pointed to an element inside a list, map, or set nested attribute. I also fixed plan modification to still run resource-level plan modification even when the plan is null (i.e., the resource is being deleted) so that diagnostics can be returned. This is useful, for example, when a resource _can't_ be deleted on the API, but the provider is going to just remove it from Terraform's state. This allows provider developers to surface a warning at plan time to practitioners about this limitation, letting them know _before_ they apply the change that that is what's going to happen. Finally, some tests had nonsensical inputs to them that Terraform could never produce--a null config that still had planned values. I updated those tests to have more realistic values by removing the plan and removing the RequiresReplace set in the response, as a deleted resource needing to be replaced doesn't super make sense. --- tfsdk/schema.go | 12 +++ tfsdk/serve.go | 157 ++++++++++++++++++------------- tfsdk/serve_resource_two_test.go | 4 +- tfsdk/serve_test.go | 135 +++++++++++--------------- 4 files changed, 160 insertions(+), 148 deletions(-) diff --git a/tfsdk/schema.go b/tfsdk/schema.go index d51204763..035ec10f7 100644 --- a/tfsdk/schema.go +++ b/tfsdk/schema.go @@ -126,6 +126,18 @@ func (s Schema) AttributeAtPath(path *tftypes.AttributePath) (Attribute, error) return Attribute{}, ErrPathInsideAtomicAttribute } + // list, set, and map nested attributes all return a + // map[string]Attribute when the path points to an element in the list, + // set, or path. We need it to point to an attribute specifically, so + // we're choosing to use the parent attribute in this case, which is + // usually what we want. + if len(path.Steps()) > 1 { + lastStep := path.Steps()[len(path.Steps())-1] + if _, ok := lastStep.(tftypes.AttributeName); !ok { + return s.AttributeAtPath(path.WithoutLastStep()) + } + } + a, ok := res.(Attribute) if !ok { return Attribute{}, fmt.Errorf("got unexpected type %T", res) diff --git a/tfsdk/serve.go b/tfsdk/serve.go index fee036a4a..a1c4ae3d6 100644 --- a/tfsdk/serve.go +++ b/tfsdk/serve.go @@ -584,9 +584,16 @@ func (s *server) readResource(ctx context.Context, req *tfprotov6.ReadResourceRe resp.NewState = &newState } -func markComputedNilsAsUnknown(ctx context.Context, resourceSchema Schema) func(*tftypes.AttributePath, tftypes.Value) (tftypes.Value, error) { +func markComputedNilsAsUnknown(ctx context.Context, config tftypes.Value, resourceSchema Schema) func(*tftypes.AttributePath, tftypes.Value) (tftypes.Value, error) { return func(path *tftypes.AttributePath, val tftypes.Value) (tftypes.Value, error) { - if !val.IsNull() { + // we are only modifying attributes, not the entire resource + if len(path.Steps()) < 1 { + return val, nil + } + configVal, _, err := tftypes.WalkAttributePath(config, path) + if err != tftypes.ErrInvalidStep && err != nil { + return val, err + } else if err != tftypes.ErrInvalidStep && !configVal.(tftypes.Value).IsNull() { return val, nil } attribute, err := resourceSchema.AttributeAtPath(path) @@ -676,11 +683,6 @@ func (s *server) planResourceChange(ctx context.Context, req *tfprotov6.PlanReso resp.PlannedState = req.ProposedNewState - if plan.IsNull() || !plan.IsKnown() { - // on null or unknown plans, just bail, we can't do anything - return - } - // create the resource instance, so we can call its methods and handle // the request resource, diags := resourceType.NewResource(ctx, s.p) @@ -689,64 +691,96 @@ func (s *server) planResourceChange(ctx context.Context, req *tfprotov6.PlanReso return } - // first, execute any AttributePlanModifiers - modifySchemaPlanReq := ModifySchemaPlanRequest{ - Config: Config{ - Schema: resourceSchema, - Raw: config, - }, - State: State{ - Schema: resourceSchema, - Raw: state, - }, - Plan: Plan{ - Schema: resourceSchema, - Raw: plan, - }, - } - if pm, ok := s.p.(ProviderWithProviderMeta); ok { - pmSchema, diags := pm.GetMetaSchema(ctx) - if diags != nil { - resp.Diagnostics.Append(diags...) - if resp.Diagnostics.HasError() { - return - } + // first, mark any computed attributes that are null in the config as + // unknown in the plan, so providers have the choice to update them + // + // do this first so that providers can override the unknown with a + // known value using any plan modifiers + // + // we only do this if there's a plan to modify; otherwise, it + // represents a resource being deleted and there's no point + if !plan.IsNull() { + modifiedPlan, err := tftypes.Transform(plan, markComputedNilsAsUnknown(ctx, config, resourceSchema)) + if err != nil { + resp.Diagnostics.AddError( + "Error modifying plan", + "There was an unexpected error updating the plan. This is always a problem with the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return } - modifySchemaPlanReq.ProviderMeta = Config{ - Schema: pmSchema, - Raw: tftypes.NewValue(pmSchema.TerraformType(ctx), nil), + plan = modifiedPlan + } + + // next, execute any AttributePlanModifiers as long as there's a plan + // to modify. + // + // We only do this if there's a plan to modify; otherwise, it + // represents a resource being deleted and there's no point. + if !plan.IsNull() { + modifySchemaPlanReq := ModifySchemaPlanRequest{ + Config: Config{ + Schema: resourceSchema, + Raw: config, + }, + State: State{ + Schema: resourceSchema, + Raw: state, + }, + Plan: Plan{ + Schema: resourceSchema, + Raw: plan, + }, } + if pm, ok := s.p.(ProviderWithProviderMeta); ok { + pmSchema, diags := pm.GetMetaSchema(ctx) + if diags != nil { + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + } + modifySchemaPlanReq.ProviderMeta = Config{ + Schema: pmSchema, + Raw: tftypes.NewValue(pmSchema.TerraformType(ctx), nil), + } - if req.ProviderMeta != nil { - pmValue, err := req.ProviderMeta.Unmarshal(pmSchema.TerraformType(ctx)) - if err != nil { - resp.Diagnostics.AddError( - "Error parsing provider_meta", - "There was an error parsing the provider_meta block. Please report this to the provider developer:\n\n"+err.Error(), - ) - return + if req.ProviderMeta != nil { + pmValue, err := req.ProviderMeta.Unmarshal(pmSchema.TerraformType(ctx)) + if err != nil { + resp.Diagnostics.AddError( + "Error parsing provider_meta", + "There was an error parsing the provider_meta block. Please report this to the provider developer:\n\n"+err.Error(), + ) + return + } + modifySchemaPlanReq.ProviderMeta.Raw = pmValue } - modifySchemaPlanReq.ProviderMeta.Raw = pmValue } - } - modifySchemaPlanResp := ModifySchemaPlanResponse{ - Plan: Plan{ - Schema: resourceSchema, - Raw: plan, - }, - Diagnostics: resp.Diagnostics, - } + modifySchemaPlanResp := ModifySchemaPlanResponse{ + Plan: Plan{ + Schema: resourceSchema, + Raw: plan, + }, + Diagnostics: resp.Diagnostics, + } - resourceSchema.modifyAttributePlans(ctx, modifySchemaPlanReq, &modifySchemaPlanResp) - resp.RequiresReplace = modifySchemaPlanResp.RequiresReplace - plan = modifySchemaPlanResp.Plan.Raw - resp.Diagnostics = modifySchemaPlanResp.Diagnostics - if resp.Diagnostics.HasError() { - return + resourceSchema.modifyAttributePlans(ctx, modifySchemaPlanReq, &modifySchemaPlanResp) + resp.RequiresReplace = modifySchemaPlanResp.RequiresReplace + plan = modifySchemaPlanResp.Plan.Raw + resp.Diagnostics = modifySchemaPlanResp.Diagnostics + if resp.Diagnostics.HasError() { + return + } } - // second, execute any ModifyPlan func + // third, execute any resource-level ModifyPlan method + // + // we do this regardless of whether the plan is null or not, because we + // want resources to be able to return diagnostics when planning to + // delete resources, e.g. to inform practitioners that the resource + // _can't_ be deleted in the API and will just be removed from + // Terraform's state var modifyPlanResp ModifyResourcePlanResponse if resource, ok := resource.(ResourceWithModifyPlan); ok { modifyPlanReq := ModifyResourcePlanRequest{ @@ -800,16 +834,7 @@ func (s *server) planResourceChange(ctx context.Context, req *tfprotov6.PlanReso plan = modifyPlanResp.Plan.Raw } - modifiedPlan, err := tftypes.Transform(plan, markComputedNilsAsUnknown(ctx, resourceSchema)) - if err != nil { - resp.Diagnostics.AddError( - "Error modifying plan", - "There was an unexpected error updating the plan. This is always a problem with the provider. Please report the following to the provider developer:\n\n"+err.Error(), - ) - return - } - - plannedState, err := tfprotov6.NewDynamicValue(modifiedPlan.Type(), modifiedPlan) + plannedState, err := tfprotov6.NewDynamicValue(plan.Type(), plan) if err != nil { resp.Diagnostics.AddError( "Error converting response", diff --git a/tfsdk/serve_resource_two_test.go b/tfsdk/serve_resource_two_test.go index da0bb6530..a7152c2e1 100644 --- a/tfsdk/serve_resource_two_test.go +++ b/tfsdk/serve_resource_two_test.go @@ -171,5 +171,7 @@ func (r testServeResourceTwo) ModifyPlan(ctx context.Context, req ModifyResource r.provider.planResourceChangeProviderMetaSchema = req.ProviderMeta.Schema r.provider.planResourceChangeCalledResourceType = "test_two" r.provider.planResourceChangeCalledAction = "modify_plan" - r.provider.modifyPlanFunc(ctx, req, resp) + if r.provider.modifyPlanFunc != nil { + r.provider.modifyPlanFunc(ctx, req, resp) + } } diff --git a/tfsdk/serve_test.go b/tfsdk/serve_test.go index 38a8c5027..333858cd9 100644 --- a/tfsdk/serve_test.go +++ b/tfsdk/serve_test.go @@ -185,7 +185,7 @@ func TestMarkComputedNilsAsUnknown(t *testing.T) { }), }) - got, err := tftypes.Transform(input, markComputedNilsAsUnknown(context.Background(), s)) + got, err := tftypes.Transform(input, markComputedNilsAsUnknown(context.Background(), input, s)) if err != nil { t.Errorf("Unexpected error: %s", err) return @@ -753,6 +753,9 @@ func TestServerValidateProviderConfig(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { + for _, d := range got.Diagnostics { + t.Log(d.Detail) + } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } }) @@ -1072,6 +1075,9 @@ func TestServerConfigureProvider(t *testing.T) { t.Errorf("Expected Terraform version to be %q, got %q", tc.tfVersion, s.configuredTFVersion) } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { + for _, d := range got.Diagnostics { + t.Log(d.Detail) + } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } if diff := cmp.Diff(s.configuredVal, tc.config); diff != "" { @@ -1310,6 +1316,9 @@ func TestServerValidateResourceConfig(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { + for _, d := range got.Diagnostics { + t.Log(d.Detail) + } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } }) @@ -1643,6 +1652,9 @@ func TestServerReadResource(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { + for _, d := range got.Diagnostics { + t.Log(d.Detail) + } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } if diff := cmp.Diff(s.readResourceCurrentStateValue, tc.currentState); diff != "" { @@ -1737,7 +1749,7 @@ func TestServerPlanResourceChange(t *testing.T) { tftypes.NewValue(tftypes.String, "red"), tftypes.NewValue(tftypes.String, "orange"), }), - "created_timestamp": tftypes.NewValue(tftypes.String, "when the earth was young"), + "created_timestamp": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), }), }, "one_nil_state_and_config": { @@ -1918,7 +1930,7 @@ func TestServerPlanResourceChange(t *testing.T) { }), }), proposedNewState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "123456"), + "id": tftypes.NewValue(tftypes.String, "1234567"), "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "name": tftypes.String, "size_gb": tftypes.Number, @@ -1935,35 +1947,8 @@ func TestServerPlanResourceChange(t *testing.T) { }), }), }), - config: tftypes.NewValue(testServeResourceTypeTwoType, nil), - resource: "test_two", - resourceType: testServeResourceTypeTwoType, - expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "123456"), - "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ - "name": tftypes.String, - "size_gb": tftypes.Number, - "boot": tftypes.Bool, - }}}, []tftypes.Value{ - tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ - "name": tftypes.String, - "size_gb": tftypes.Number, - "boot": tftypes.Bool, - }}, map[string]tftypes.Value{ - "name": tftypes.NewValue(tftypes.String, "my-disk"), - "size_gb": tftypes.NewValue(tftypes.Number, 10), - "boot": tftypes.NewValue(tftypes.Bool, false), - }), - }), - }), - modifyPlanFunc: func(ctx context.Context, req ModifyResourcePlanRequest, resp *ModifyResourcePlanResponse) { - resp.RequiresReplace = []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")} - }, - expectedRequiresReplace: []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")}, - }, - "two_modifyplan_diags_warning": { - priorState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "123456"), + config: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "1234567"), "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "name": tftypes.String, "size_gb": tftypes.Number, @@ -1980,8 +1965,10 @@ func TestServerPlanResourceChange(t *testing.T) { }), }), }), - proposedNewState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "123456"), + resource: "test_two", + resourceType: testServeResourceTypeTwoType, + expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "1234567"), "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "name": tftypes.String, "size_gb": tftypes.Number, @@ -1998,10 +1985,13 @@ func TestServerPlanResourceChange(t *testing.T) { }), }), }), - config: tftypes.NewValue(testServeResourceTypeTwoType, nil), - resource: "test_two", - resourceType: testServeResourceTypeTwoType, - expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + modifyPlanFunc: func(ctx context.Context, req ModifyResourcePlanRequest, resp *ModifyResourcePlanResponse) { + resp.RequiresReplace = []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")} + }, + expectedRequiresReplace: []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")}, + }, + "two_modifyplan_diags_warning": { + priorState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ "id": tftypes.NewValue(tftypes.String, "123456"), "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "name": tftypes.String, @@ -2019,11 +2009,16 @@ func TestServerPlanResourceChange(t *testing.T) { }), }), }), + proposedNewState: tftypes.NewValue(testServeResourceTypeTwoType, nil), + config: tftypes.NewValue(testServeResourceTypeTwoType, nil), + resource: "test_two", + resourceType: testServeResourceTypeTwoType, + // when the config is null, the resource has been + // deleted, and the plan should reflect that + expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, nil), modifyPlanFunc: func(ctx context.Context, req ModifyResourcePlanRequest, resp *ModifyResourcePlanResponse) { - resp.RequiresReplace = []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")} resp.AddWarning("I'm warning you", "You have been warned") }, - expectedRequiresReplace: []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")}, expectedDiags: []*tfprotov6.Diagnostic{ { Severity: tfprotov6.DiagnosticSeverityWarning, @@ -2051,50 +2046,16 @@ func TestServerPlanResourceChange(t *testing.T) { }), }), }), - proposedNewState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "123456"), - "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ - "name": tftypes.String, - "size_gb": tftypes.Number, - "boot": tftypes.Bool, - }}}, []tftypes.Value{ - tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ - "name": tftypes.String, - "size_gb": tftypes.Number, - "boot": tftypes.Bool, - }}, map[string]tftypes.Value{ - "name": tftypes.NewValue(tftypes.String, "my-disk"), - "size_gb": tftypes.NewValue(tftypes.Number, 10), - "boot": tftypes.NewValue(tftypes.Bool, false), - }), - }), - }), - config: tftypes.NewValue(testServeResourceTypeTwoType, nil), - resource: "test_two", - resourceType: testServeResourceTypeTwoType, - expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ - "id": tftypes.NewValue(tftypes.String, "123456"), - "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ - "name": tftypes.String, - "size_gb": tftypes.Number, - "boot": tftypes.Bool, - }}}, []tftypes.Value{ - tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ - "name": tftypes.String, - "size_gb": tftypes.Number, - "boot": tftypes.Bool, - }}, map[string]tftypes.Value{ - "name": tftypes.NewValue(tftypes.String, "my-disk"), - "size_gb": tftypes.NewValue(tftypes.Number, 10), - "boot": tftypes.NewValue(tftypes.Bool, false), - }), - }), - }), + proposedNewState: tftypes.NewValue(testServeResourceTypeTwoType, nil), + config: tftypes.NewValue(testServeResourceTypeTwoType, nil), + resource: "test_two", + resourceType: testServeResourceTypeTwoType, + // when the config is null, that means the resource has + // been deleted, so the plan should reflect that + expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, nil), modifyPlanFunc: func(ctx context.Context, req ModifyResourcePlanRequest, resp *ModifyResourcePlanResponse) { - resp.RequiresReplace = []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")} resp.AddError("This is an error", "More details about the error") }, - expectedRequiresReplace: []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")}, expectedDiags: []*tfprotov6.Diagnostic{ { Severity: tfprotov6.DiagnosticSeverityError, @@ -2784,6 +2745,9 @@ func TestServerPlanResourceChange(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { + for _, d := range got.Diagnostics { + t.Log(d.Detail) + } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } gotPlannedState, err := got.PlannedState.Unmarshal(tc.resourceType) @@ -3910,6 +3874,9 @@ func TestServerApplyResourceChange(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { + for _, d := range got.Diagnostics { + t.Log(d.Detail) + } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } if s.applyResourceChangeCalledResourceType != tc.resource { @@ -4201,6 +4168,9 @@ func TestServerValidateDataResourceConfig(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { + for _, d := range got.Diagnostics { + t.Log(d.Detail) + } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } }) @@ -4423,6 +4393,9 @@ func TestServerReadDataSource(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { + for _, d := range got.Diagnostics { + t.Log(d.Detail) + } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } if diff := cmp.Diff(s.readDataSourceConfigValue, tc.config); diff != "" { From e32f00ec191859edf7bef49673392e9adf062a56 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 27 Sep 2021 17:24:42 -0400 Subject: [PATCH 2/6] Revert t.Log() for diagnostics debugging for now It will likely get documented and consistently applied later either as-is or using a custom go-cmp formatter. See also https://github.com/hashicorp/terraform-plugin-framework/issues/135. --- tfsdk/serve_test.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/tfsdk/serve_test.go b/tfsdk/serve_test.go index 333858cd9..4892ae648 100644 --- a/tfsdk/serve_test.go +++ b/tfsdk/serve_test.go @@ -753,9 +753,6 @@ func TestServerValidateProviderConfig(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { - for _, d := range got.Diagnostics { - t.Log(d.Detail) - } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } }) @@ -1075,9 +1072,6 @@ func TestServerConfigureProvider(t *testing.T) { t.Errorf("Expected Terraform version to be %q, got %q", tc.tfVersion, s.configuredTFVersion) } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { - for _, d := range got.Diagnostics { - t.Log(d.Detail) - } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } if diff := cmp.Diff(s.configuredVal, tc.config); diff != "" { @@ -1316,9 +1310,6 @@ func TestServerValidateResourceConfig(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { - for _, d := range got.Diagnostics { - t.Log(d.Detail) - } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } }) @@ -1652,9 +1643,6 @@ func TestServerReadResource(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { - for _, d := range got.Diagnostics { - t.Log(d.Detail) - } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } if diff := cmp.Diff(s.readResourceCurrentStateValue, tc.currentState); diff != "" { @@ -2745,9 +2733,6 @@ func TestServerPlanResourceChange(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { - for _, d := range got.Diagnostics { - t.Log(d.Detail) - } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } gotPlannedState, err := got.PlannedState.Unmarshal(tc.resourceType) @@ -3874,9 +3859,6 @@ func TestServerApplyResourceChange(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { - for _, d := range got.Diagnostics { - t.Log(d.Detail) - } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } if s.applyResourceChangeCalledResourceType != tc.resource { @@ -4168,9 +4150,6 @@ func TestServerValidateDataResourceConfig(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { - for _, d := range got.Diagnostics { - t.Log(d.Detail) - } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } }) @@ -4393,9 +4372,6 @@ func TestServerReadDataSource(t *testing.T) { return } if diff := cmp.Diff(got.Diagnostics, tc.expectedDiags); diff != "" { - for _, d := range got.Diagnostics { - t.Log(d.Detail) - } t.Errorf("Unexpected diff in diagnostics (+wanted, -got): %s", diff) } if diff := cmp.Diff(s.readDataSourceConfigValue, tc.config); diff != "" { From b2fd6379eb019f49f0501cba39d1f3aadd726484 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 28 Sep 2021 10:15:47 -0400 Subject: [PATCH 3/6] Revert Schema.AttributeAtPath implicit parent Attribute handling and add unit testing --- tfsdk/schema.go | 12 +- tfsdk/schema_test.go | 771 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 773 insertions(+), 10 deletions(-) diff --git a/tfsdk/schema.go b/tfsdk/schema.go index 035ec10f7..7894fc630 100644 --- a/tfsdk/schema.go +++ b/tfsdk/schema.go @@ -126,16 +126,8 @@ func (s Schema) AttributeAtPath(path *tftypes.AttributePath) (Attribute, error) return Attribute{}, ErrPathInsideAtomicAttribute } - // list, set, and map nested attributes all return a - // map[string]Attribute when the path points to an element in the list, - // set, or path. We need it to point to an attribute specifically, so - // we're choosing to use the parent attribute in this case, which is - // usually what we want. - if len(path.Steps()) > 1 { - lastStep := path.Steps()[len(path.Steps())-1] - if _, ok := lastStep.(tftypes.AttributeName); !ok { - return s.AttributeAtPath(path.WithoutLastStep()) - } + if _, ok := res.(nestedAttributes); ok { + return Attribute{}, ErrPathInsideAtomicAttribute } a, ok := res.(Attribute) diff --git a/tfsdk/schema_test.go b/tfsdk/schema_test.go index dc664016b..ea0154a7a 100644 --- a/tfsdk/schema_test.go +++ b/tfsdk/schema_test.go @@ -12,6 +12,777 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) +func TestSchemaAttributeAtPath(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + schema Schema + path *tftypes.AttributePath + expected Attribute + expectedErr string + }{ + "empty-root": { + schema: Schema{}, + path: tftypes.NewAttributePath(), + expected: Attribute{}, + expectedErr: "got unexpected type tfsdk.Schema", + }, + "empty-nil": { + schema: Schema{}, + path: nil, + expected: Attribute{}, + expectedErr: "got unexpected type tfsdk.Schema", + }, + "root": { + schema: Schema{ + Attributes: map[string]Attribute{ + "test": { + Type: types.StringType, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath(), + expected: Attribute{}, + expectedErr: "got unexpected type tfsdk.Schema", + }, + "nil": { + schema: Schema{ + Attributes: map[string]Attribute{ + "test": { + Type: types.StringType, + Required: true, + }, + }, + }, + path: nil, + expected: Attribute{}, + expectedErr: "got unexpected type tfsdk.Schema", + }, + "WithAttributeName": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Type: types.StringType, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test"), + expected: Attribute{ + Type: types.StringType, + Required: true, + }, + }, + "WithAttributeName-ListNestedAttributes-WithAttributeName": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: ListNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, ListNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithAttributeName("sub_test"), + expected: Attribute{}, + expectedErr: "AttributeName(\"sub_test\") still remains in the path: can't apply tftypes.AttributeName to ListNestedAttributes", + }, + "WithAttributeName-ListNestedAttributes-WithElementKeyInt": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: ListNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, ListNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyInt(0), + expected: Attribute{}, + expectedErr: ErrPathInsideAtomicAttribute.Error(), + }, + "WithAttributeName-ListNestedAttributes-WithElementKeyInt-WithAttributeName": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: ListNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, ListNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyInt(0).WithAttributeName("sub_test"), + expected: Attribute{ + Type: types.StringType, + Required: true, + }, + }, + "WithAttributeName-ListNestedAttributes-WithElementKeyString": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: ListNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, ListNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyString("sub_test"), + expected: Attribute{}, + expectedErr: "ElementKeyString(\"sub_test\") still remains in the path: can't apply tftypes.ElementKeyString to ListNestedAttributes", + }, + "WithAttributeName-ListNestedAttributes-WithElementKeyValue": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: ListNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, ListNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "sub_test")), + expected: Attribute{}, + expectedErr: "ElementKeyValue(tftypes.String<\"sub_test\">) still remains in the path: can't apply tftypes.ElementKeyValue to ListNestedAttributes", + }, + "WithAttributeName-MapNestedAttributes-WithAttributeName": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: MapNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, MapNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithAttributeName("sub_test"), + expected: Attribute{}, + expectedErr: "AttributeName(\"sub_test\") still remains in the path: can't use tftypes.AttributeName on maps", + }, + "WithAttributeName-MapNestedAttributes-WithElementKeyInt": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: MapNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, MapNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyInt(0), + expected: Attribute{}, + expectedErr: "ElementKeyInt(0) still remains in the path: can't use tftypes.ElementKeyInt on maps", + }, + "WithAttributeName-MapNestedAttributes-WithElementKeyString": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: MapNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, MapNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyString("sub_test"), + expected: Attribute{}, + expectedErr: ErrPathInsideAtomicAttribute.Error(), + }, + "WithAttributeName-MapNestedAttributes-WithElementKeyString-WithAttributeName": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: MapNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, MapNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyString("element").WithAttributeName("sub_test"), + expected: Attribute{ + Type: types.StringType, + Required: true, + }, + }, + "WithAttributeName-MapNestedAttributes-WithElementKeyValue": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: MapNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, MapNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "sub_test")), + expected: Attribute{}, + expectedErr: "ElementKeyValue(tftypes.String<\"sub_test\">) still remains in the path: can't use tftypes.ElementKeyValue on maps", + }, + "WithAttributeName-SetNestedAttributes-WithAttributeName": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: SetNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, SetNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithAttributeName("sub_test"), + expected: Attribute{}, + expectedErr: "AttributeName(\"sub_test\") still remains in the path: can't use tftypes.AttributeName on sets", + }, + "WithAttributeName-SetNestedAttributes-WithElementKeyInt": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: SetNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, SetNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyInt(0), + expected: Attribute{}, + expectedErr: "ElementKeyInt(0) still remains in the path: can't use tftypes.ElementKeyInt on sets", + }, + "WithAttributeName-SetNestedAttributes-WithElementKeyString": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: SetNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, SetNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyString("sub_test"), + expected: Attribute{}, + expectedErr: "ElementKeyString(\"sub_test\") still remains in the path: can't use tftypes.ElementKeyString on sets", + }, + "WithAttributeName-SetNestedAttributes-WithElementKeyValue": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: SetNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, SetNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "sub_test")), + expected: Attribute{}, + expectedErr: ErrPathInsideAtomicAttribute.Error(), + }, + "WithAttributeName-SetNestedAttributes-WithElementKeyValue-WithAttributeName": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: SetNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }, SetNestedAttributesOptions{}), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "element")).WithAttributeName("sub_test"), + expected: Attribute{ + Type: types.StringType, + Required: true, + }, + }, + "WithAttributeName-SingleNestedAttributes-WithAttributeName": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: SingleNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithAttributeName("sub_test"), + expected: Attribute{ + Type: types.StringType, + Required: true, + }, + }, + "WithAttributeName-SingleNestedAttributes-WithElementKeyInt": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: SingleNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyInt(0), + expected: Attribute{}, + expectedErr: "ElementKeyInt(0) still remains in the path: can't apply tftypes.ElementKeyInt to Attributes", + }, + "WithAttributeName-SingleNestedAttributes-WithElementKeyString": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: SingleNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyString("sub_test"), + expected: Attribute{}, + expectedErr: "ElementKeyString(\"sub_test\") still remains in the path: can't apply tftypes.ElementKeyString to Attributes", + }, + "WithAttributeName-SingleNestedAttributes-WithElementKeyValue": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Attributes: SingleNestedAttributes(map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "sub_test": { + Type: types.StringType, + Required: true, + }, + }), + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "sub_test")), + expected: Attribute{}, + expectedErr: "ElementKeyValue(tftypes.String<\"sub_test\">) still remains in the path: can't apply tftypes.ElementKeyValue to Attributes", + }, + "WithAttributeName-Object-WithAttributeName": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Type: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "sub_test": types.StringType, + }, + }, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithAttributeName("sub_test"), + expected: Attribute{}, + expectedErr: ErrPathInsideAtomicAttribute.Error(), + }, + "WithAttributeName-WithElementKeyInt-invalid-parent": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Type: types.StringType, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyInt(0), + expected: Attribute{}, + expectedErr: "ElementKeyInt(0) still remains in the path: cannot apply AttributePathStep tftypes.ElementKeyInt to types.StringType", + }, + "WithAttributeName-WithElementKeyInt-valid-parent": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Type: types.ListType{ + ElemType: types.StringType, + }, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyInt(0), + expected: Attribute{}, + expectedErr: ErrPathInsideAtomicAttribute.Error(), + }, + "WithAttributeName-WithElementKeyString-invalid-parent": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Type: types.StringType, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyString("element"), + expected: Attribute{}, + expectedErr: "ElementKeyString(\"element\") still remains in the path: cannot apply AttributePathStep tftypes.ElementKeyString to types.StringType", + }, + "WithAttributeName-WithElementKeyString-valid-parent": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Type: types.MapType{ + ElemType: types.StringType, + }, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyString("element"), + expected: Attribute{}, + expectedErr: ErrPathInsideAtomicAttribute.Error(), + }, + "WithAttributeName-WithElementKeyValue-invalid-parent": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Type: types.StringType, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "element")), + expected: Attribute{}, + expectedErr: "ElementKeyValue(tftypes.String<\"element\">) still remains in the path: cannot apply AttributePathStep tftypes.ElementKeyValue to types.StringType", + }, + "WithAttributeName-WithElementKeyValue-valid-parent": { + schema: Schema{ + Attributes: map[string]Attribute{ + "other": { + Type: types.BoolType, + Optional: true, + }, + "test": { + Type: types.SetType{ + ElemType: types.StringType, + }, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithAttributeName("test").WithElementKeyValue(tftypes.NewValue(tftypes.String, "element")), + expected: Attribute{}, + expectedErr: ErrPathInsideAtomicAttribute.Error(), + }, + "WithElementKeyInt": { + schema: Schema{ + Attributes: map[string]Attribute{ + "test": { + Type: types.StringType, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithElementKeyInt(0), + expected: Attribute{}, + expectedErr: "ElementKeyInt(0) still remains in the path: cannot apply AttributePathStep tftypes.ElementKeyInt to schema", + }, + "WithElementKeyString": { + schema: Schema{ + Attributes: map[string]Attribute{ + "test": { + Type: types.StringType, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithElementKeyString("test"), + expected: Attribute{}, + expectedErr: "ElementKeyString(\"test\") still remains in the path: cannot apply AttributePathStep tftypes.ElementKeyString to schema", + }, + "WithElementKeyValue": { + schema: Schema{ + Attributes: map[string]Attribute{ + "test": { + Type: types.StringType, + Required: true, + }, + }, + }, + path: tftypes.NewAttributePath().WithElementKeyValue(tftypes.NewValue(tftypes.String, "test")), + expected: Attribute{}, + expectedErr: "ElementKeyValue(tftypes.String<\"test\">) still remains in the path: cannot apply AttributePathStep tftypes.ElementKeyValue to schema", + }, + } + + for name, tc := range testCases { + name, tc := name, tc + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got, err := tc.schema.AttributeAtPath(tc.path) + + if err != nil { + if tc.expectedErr == "" { + t.Errorf("Unexpected error: %s", err) + return + } + if err.Error() != tc.expectedErr { + t.Errorf("Expected error to be %q, got %q", tc.expectedErr, err.Error()) + return + } + // got expected error + return + } + + if err == nil && tc.expectedErr != "" { + t.Errorf("Expected error to be %q, got nil", tc.expectedErr) + return + } + + if diff := cmp.Diff(got, tc.expected); diff != "" { + t.Errorf("Unexpected result (+wanted, -got): %s", diff) + } + }) + } +} + func TestSchemaAttributeType(t *testing.T) { testSchema := Schema{ Attributes: map[string]Attribute{ From d82b2be81993474ebbfdf24edb70f70a0e7b1818 Mon Sep 17 00:00:00 2001 From: Katy Moe Date: Tue, 28 Sep 2021 15:34:40 +0100 Subject: [PATCH 4/6] populate missing config values in tests --- tfsdk/serve_test.go | 149 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 134 insertions(+), 15 deletions(-) diff --git a/tfsdk/serve_test.go b/tfsdk/serve_test.go index 4892ae648..cc85d9741 100644 --- a/tfsdk/serve_test.go +++ b/tfsdk/serve_test.go @@ -1838,7 +1838,24 @@ func TestServerPlanResourceChange(t *testing.T) { }), }), }), - config: tftypes.NewValue(testServeResourceTypeTwoType, nil), + config: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "123456"), + "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}}, []tftypes.Value{ + tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "my-disk"), + "size_gb": tftypes.NewValue(tftypes.Number, 10), + "boot": tftypes.NewValue(tftypes.Bool, false), + }), + }), + }), resource: "test_two", resourceType: testServeResourceTypeTwoType, expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ @@ -1997,16 +2014,67 @@ func TestServerPlanResourceChange(t *testing.T) { }), }), }), - proposedNewState: tftypes.NewValue(testServeResourceTypeTwoType, nil), - config: tftypes.NewValue(testServeResourceTypeTwoType, nil), - resource: "test_two", - resourceType: testServeResourceTypeTwoType, - // when the config is null, the resource has been - // deleted, and the plan should reflect that - expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, nil), + proposedNewState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "123456"), + "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}}, []tftypes.Value{ + tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "my-disk"), + "size_gb": tftypes.NewValue(tftypes.Number, 10), + "boot": tftypes.NewValue(tftypes.Bool, false), + }), + }), + }), + config: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "123456"), + "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}}, []tftypes.Value{ + tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "my-disk"), + "size_gb": tftypes.NewValue(tftypes.Number, 10), + "boot": tftypes.NewValue(tftypes.Bool, false), + }), + }), + }), + resource: "test_two", + resourceType: testServeResourceTypeTwoType, + expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "123456"), + "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}}, []tftypes.Value{ + tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "my-disk"), + "size_gb": tftypes.NewValue(tftypes.Number, 10), + "boot": tftypes.NewValue(tftypes.Bool, false), + }), + }), + }), modifyPlanFunc: func(ctx context.Context, req ModifyResourcePlanRequest, resp *ModifyResourcePlanResponse) { + resp.RequiresReplace = []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")} resp.AddWarning("I'm warning you", "You have been warned") }, + expectedRequiresReplace: []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")}, expectedDiags: []*tfprotov6.Diagnostic{ { Severity: tfprotov6.DiagnosticSeverityWarning, @@ -2034,16 +2102,67 @@ func TestServerPlanResourceChange(t *testing.T) { }), }), }), - proposedNewState: tftypes.NewValue(testServeResourceTypeTwoType, nil), - config: tftypes.NewValue(testServeResourceTypeTwoType, nil), - resource: "test_two", - resourceType: testServeResourceTypeTwoType, - // when the config is null, that means the resource has - // been deleted, so the plan should reflect that - expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, nil), + proposedNewState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "123456"), + "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}}, []tftypes.Value{ + tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "my-disk"), + "size_gb": tftypes.NewValue(tftypes.Number, 10), + "boot": tftypes.NewValue(tftypes.Bool, false), + }), + }), + }), + config: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "123456"), + "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}}, []tftypes.Value{ + tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "my-disk"), + "size_gb": tftypes.NewValue(tftypes.Number, 10), + "boot": tftypes.NewValue(tftypes.Bool, false), + }), + }), + }), + resource: "test_two", + resourceType: testServeResourceTypeTwoType, + expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "123456"), + "disks": tftypes.NewValue(tftypes.List{ElementType: tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}}, []tftypes.Value{ + tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "size_gb": tftypes.Number, + "boot": tftypes.Bool, + }}, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "my-disk"), + "size_gb": tftypes.NewValue(tftypes.Number, 10), + "boot": tftypes.NewValue(tftypes.Bool, false), + }), + }), + }), modifyPlanFunc: func(ctx context.Context, req ModifyResourcePlanRequest, resp *ModifyResourcePlanResponse) { + resp.RequiresReplace = []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")} resp.AddError("This is an error", "More details about the error") }, + expectedRequiresReplace: []*tftypes.AttributePath{tftypes.NewAttributePath().WithAttributeName("id")}, expectedDiags: []*tfprotov6.Diagnostic{ { Severity: tfprotov6.DiagnosticSeverityError, From 78deca308fe1f392e775b0505e10ba7ddca5b0dc Mon Sep 17 00:00:00 2001 From: Katy Moe Date: Tue, 28 Sep 2021 16:58:38 +0100 Subject: [PATCH 5/6] test nested unknown value setting --- tfsdk/serve_provider_test.go | 1 + tfsdk/serve_resource_three_test.go | 142 +++++++++++++++++++++++++++++ tfsdk/serve_test.go | 98 ++++++++++++++++++++ 3 files changed, 241 insertions(+) create mode 100644 tfsdk/serve_resource_three_test.go diff --git a/tfsdk/serve_provider_test.go b/tfsdk/serve_provider_test.go index c86020b91..df75a0fd0 100644 --- a/tfsdk/serve_provider_test.go +++ b/tfsdk/serve_provider_test.go @@ -554,6 +554,7 @@ func (t *testServeProvider) GetResources(_ context.Context) (map[string]Resource return map[string]ResourceType{ "test_one": testServeResourceTypeOne{}, "test_two": testServeResourceTypeTwo{}, + "test_three": testServeResourceTypeThree{}, "test_attribute_plan_modifiers": testServeResourceTypeAttributePlanModifiers{}, "test_config_validators": testServeResourceTypeConfigValidators{}, "test_import_state": testServeResourceTypeImportState{}, diff --git a/tfsdk/serve_resource_three_test.go b/tfsdk/serve_resource_three_test.go new file mode 100644 index 000000000..8b11c408b --- /dev/null +++ b/tfsdk/serve_resource_three_test.go @@ -0,0 +1,142 @@ +package tfsdk + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +func (rt testServeResourceTypeThree) GetSchema(_ context.Context) (Schema, diag.Diagnostics) { + return Schema{ + Version: 1, + Attributes: map[string]Attribute{ + "name": { + Required: true, + Type: types.StringType, + }, + "last_updated": { + Computed: true, + Type: types.StringType, + }, + "first_updated": { + Computed: true, + Type: types.StringType, + }, + "map_nested": { + Required: true, + Attributes: MapNestedAttributes(map[string]Attribute{ + "computed_string": { + Computed: true, + Type: types.StringType, + }, + "string": { + Optional: true, + Type: types.StringType, + }, + }, MapNestedAttributesOptions{}), + }, + }, + }, nil +} + +func (rt testServeResourceTypeThree) NewResource(_ context.Context, p Provider) (Resource, diag.Diagnostics) { + provider, ok := p.(*testServeProvider) + if !ok { + prov, ok := p.(*testServeProviderWithMetaSchema) + if !ok { + panic(fmt.Sprintf("unexpected provider type %T", p)) + } + provider = prov.testServeProvider + } + return testServeResourceThree{ + provider: provider, + }, nil +} + +var testServeResourceTypeThreeSchema = &tfprotov6.Schema{ + Version: 1, + Block: &tfprotov6.SchemaBlock{ + Attributes: []*tfprotov6.SchemaAttribute{ + { + Name: "first_updated", + Computed: true, + Type: tftypes.String, + }, + { + Name: "last_updated", + Computed: true, + Type: tftypes.String, + }, + { + Name: "map_nested", + Required: true, + NestedType: &tfprotov6.SchemaObject{ + Nesting: tfprotov6.SchemaObjectNestingModeMap, + Attributes: []*tfprotov6.SchemaAttribute{ + { + Name: "computed_string", + Computed: true, + Type: tftypes.String, + }, + { + Name: "string", + Optional: true, + Type: tftypes.String, + }, + }, + }, + }, + { + Name: "name", + Required: true, + Type: tftypes.String, + }, + }, + }, +} + +var testServeResourceTypeThreeType = tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + "last_updated": tftypes.String, + "first_updated": tftypes.String, + "map_nested": tftypes.Map{ + AttributeType: tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "computed_string": tftypes.String, + "string": tftypes.String, + }, + }, + }, + }, +} + +type testServeResourceThree struct { + provider *testServeProvider +} + +type testServeResourceTypeThree struct{} + +func (r testServeResourceThree) Create(ctx context.Context, req CreateResourceRequest, resp *CreateResourceResponse) { + // Intentionally blank. Not expected to be called during testing. +} + +func (r testServeResourceThree) Read(ctx context.Context, req ReadResourceRequest, resp *ReadResourceResponse) { + // Intentionally blank. Not expected to be called during testing. +} + +func (r testServeResourceThree) Update(ctx context.Context, req UpdateResourceRequest, resp *UpdateResourceResponse) { + // Intentionally blank. Not expected to be called during testing. +} + +func (r testServeResourceThree) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) { + // Intentionally blank. Not expected to be called during testing. +} + +func (r testServeResourceThree) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) { + ResourceImportStateNotImplemented(ctx, "Not expected to be called during testing.", resp) +} diff --git a/tfsdk/serve_test.go b/tfsdk/serve_test.go index cc85d9741..e32212ae1 100644 --- a/tfsdk/serve_test.go +++ b/tfsdk/serve_test.go @@ -275,6 +275,7 @@ func TestServerGetProviderSchema(t *testing.T) { ResourceSchemas: map[string]*tfprotov6.Schema{ "test_one": testServeResourceTypeOneSchema, "test_two": testServeResourceTypeTwoSchema, + "test_three": testServeResourceTypeThreeSchema, "test_attribute_plan_modifiers": testServeResourceTypeAttributePlanModifiersSchema, "test_config_validators": testServeResourceTypeConfigValidatorsSchema, "test_import_state": testServeResourceTypeImportStateSchema, @@ -309,6 +310,7 @@ func TestServerGetProviderSchemaWithProviderMeta(t *testing.T) { ResourceSchemas: map[string]*tfprotov6.Schema{ "test_one": testServeResourceTypeOneSchema, "test_two": testServeResourceTypeTwoSchema, + "test_three": testServeResourceTypeThreeSchema, "test_attribute_plan_modifiers": testServeResourceTypeAttributePlanModifiersSchema, "test_config_validators": testServeResourceTypeConfigValidatorsSchema, "test_import_state": testServeResourceTypeImportStateSchema, @@ -1781,6 +1783,102 @@ func TestServerPlanResourceChange(t *testing.T) { resourceType: testServeResourceTypeTwoType, expectedPlannedState: tftypes.NewValue(testServeResourceTypeTwoType, nil), }, + "three_nested_computed_unknown": { + resource: "test_three", + resourceType: testServeResourceTypeThreeType, + priorState: tftypes.NewValue(testServeResourceTypeThreeType, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "myname"), + "last_updated": tftypes.NewValue(tftypes.String, "yesterday"), + "first_updated": tftypes.NewValue(tftypes.String, "last year"), + "map_nested": tftypes.NewValue(tftypes.Map{ + AttributeType: tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "computed_string": tftypes.String, + "string": tftypes.String, + }, + }, + }, map[string]tftypes.Value{ + "one": tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "computed_string": tftypes.String, + "string": tftypes.String, + }, + }, map[string]tftypes.Value{ + "computed_string": tftypes.NewValue(tftypes.String, "mycompstring"), + "string": tftypes.NewValue(tftypes.String, "mystring"), + }), + }), + }), + config: tftypes.NewValue(testServeResourceTypeThreeType, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "myname"), + "last_updated": tftypes.NewValue(tftypes.String, nil), + "first_updated": tftypes.NewValue(tftypes.String, nil), + "map_nested": tftypes.NewValue(tftypes.Map{ + AttributeType: tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "computed_string": tftypes.String, + "string": tftypes.String, + }, + }, + }, map[string]tftypes.Value{ + "one": tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "computed_string": tftypes.String, + "string": tftypes.String, + }, + }, map[string]tftypes.Value{ + "computed_string": tftypes.NewValue(tftypes.String, nil), + "string": tftypes.NewValue(tftypes.String, nil), + }), + }), + }), + proposedNewState: tftypes.NewValue(testServeResourceTypeThreeType, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "myname"), + "last_updated": tftypes.NewValue(tftypes.String, nil), + "first_updated": tftypes.NewValue(tftypes.String, nil), + "map_nested": tftypes.NewValue(tftypes.Map{ + AttributeType: tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "computed_string": tftypes.String, + "string": tftypes.String, + }, + }, + }, map[string]tftypes.Value{ + "one": tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "computed_string": tftypes.String, + "string": tftypes.String, + }, + }, map[string]tftypes.Value{ + "computed_string": tftypes.NewValue(tftypes.String, nil), + "string": tftypes.NewValue(tftypes.String, nil), + }), + }), + }), + expectedPlannedState: tftypes.NewValue(testServeResourceTypeThreeType, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "myname"), + "last_updated": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + "first_updated": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + "map_nested": tftypes.NewValue(tftypes.Map{ + AttributeType: tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "computed_string": tftypes.String, + "string": tftypes.String, + }, + }, + }, map[string]tftypes.Value{ + "one": tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "computed_string": tftypes.String, + "string": tftypes.String, + }, + }, map[string]tftypes.Value{ + "computed_string": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + "string": tftypes.NewValue(tftypes.String, nil), + }), + }), + }), + }, "one_add": { priorState: tftypes.NewValue(testServeResourceTypeOneType, nil), proposedNewState: tftypes.NewValue(testServeResourceTypeOneType, map[string]tftypes.Value{ From 7a23a4f4c90111086442928f954fe2f96419c619 Mon Sep 17 00:00:00 2001 From: Katy Moe Date: Tue, 28 Sep 2021 17:08:33 +0100 Subject: [PATCH 6/6] map AttributeType -> ElementType --- tfsdk/serve_resource_three_test.go | 2 +- tfsdk/serve_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tfsdk/serve_resource_three_test.go b/tfsdk/serve_resource_three_test.go index 8b11c408b..1c63cb7a5 100644 --- a/tfsdk/serve_resource_three_test.go +++ b/tfsdk/serve_resource_three_test.go @@ -105,7 +105,7 @@ var testServeResourceTypeThreeType = tftypes.Object{ "last_updated": tftypes.String, "first_updated": tftypes.String, "map_nested": tftypes.Map{ - AttributeType: tftypes.Object{ + ElementType: tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ "computed_string": tftypes.String, "string": tftypes.String, diff --git a/tfsdk/serve_test.go b/tfsdk/serve_test.go index e32212ae1..beb61e786 100644 --- a/tfsdk/serve_test.go +++ b/tfsdk/serve_test.go @@ -1791,7 +1791,7 @@ func TestServerPlanResourceChange(t *testing.T) { "last_updated": tftypes.NewValue(tftypes.String, "yesterday"), "first_updated": tftypes.NewValue(tftypes.String, "last year"), "map_nested": tftypes.NewValue(tftypes.Map{ - AttributeType: tftypes.Object{ + ElementType: tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ "computed_string": tftypes.String, "string": tftypes.String, @@ -1814,7 +1814,7 @@ func TestServerPlanResourceChange(t *testing.T) { "last_updated": tftypes.NewValue(tftypes.String, nil), "first_updated": tftypes.NewValue(tftypes.String, nil), "map_nested": tftypes.NewValue(tftypes.Map{ - AttributeType: tftypes.Object{ + ElementType: tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ "computed_string": tftypes.String, "string": tftypes.String, @@ -1837,7 +1837,7 @@ func TestServerPlanResourceChange(t *testing.T) { "last_updated": tftypes.NewValue(tftypes.String, nil), "first_updated": tftypes.NewValue(tftypes.String, nil), "map_nested": tftypes.NewValue(tftypes.Map{ - AttributeType: tftypes.Object{ + ElementType: tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ "computed_string": tftypes.String, "string": tftypes.String, @@ -1860,7 +1860,7 @@ func TestServerPlanResourceChange(t *testing.T) { "last_updated": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), "first_updated": tftypes.NewValue(tftypes.String, tftypes.UnknownValue), "map_nested": tftypes.NewValue(tftypes.Map{ - AttributeType: tftypes.Object{ + ElementType: tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ "computed_string": tftypes.String, "string": tftypes.String,