Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested object attributes are instantiated during plan when config is null #993

Closed
jbardin opened this issue Apr 26, 2024 · 5 comments · Fixed by #995
Closed

Nested object attributes are instantiated during plan when config is null #993

jbardin opened this issue Apr 26, 2024 · 5 comments · Fixed by #995
Assignees
Labels
bug Something isn't working
Milestone

Comments

@jbardin
Copy link
Member

jbardin commented Apr 26, 2024

With a nested map or list attribute, if a null value is passed in from the configuration, the framework always returns a non-null object. Even for computed attributes this is technically not valid, because the "Computed" concept applies to the parent attribute (nested_map below), not the individual items. For non-computed attributes this is also not valid, but Terraform core cannot catch this validation and will let it pass through (upon resolution of hashicorp/terraform#35083)

Example schema containing a MapNestedAttribute (ListNestedAttribute works similarly):

Attributes: map[string]schema.Attribute{
	"nested_map": schema.MapNestedAttribute{
		NestedObject: schema.NestedAttributeObject{
			Attributes: map[string]schema.Attribute{
				"map": schema.MapAttribute{
					ElementType: types.StringType,
					Optional:    true,
				},
				"string": schema.StringAttribute{
					Optional: true,
				},
			},
		},
		Optional: true,
	},
},

With the given configuration:

resource "examplecloud_thing" x {
  nested_map = {
    key1 = {
      map = {}
      string = "test"
    }
    key2 = null
  }
}

The resulting plan looks like:

  # examplecloud_thing.x will be created
  + resource "examplecloud_thing" "x" {
      + nested_map = {
          + "key1" = {
              + map  = {}
              + string = "test"
            },
          + "key2" = {},
        }
    }

With the key2 value changing from null to an object with null attributes.

@jbardin jbardin added the bug Something isn't working label Apr 26, 2024
@bflad
Copy link
Member

bflad commented Apr 26, 2024

Hey @jbardin 👋 I think I see what is happening deep inside the framework server's plan modification handling -- when it is handling a map nested attribute, it is always assuming that the "object" that makes up the map value is always a known object with all its attributes. I am kind of surprised we have not yet seen a bug report about this. If in the unit testing I try to intentionally inject a null object value there, it actually panics. It makes me wonder if Terraform is actually sending over the known object with nulls since a null object panics (I can separately try to verify this with protocol data), but in any event I think we just need to add some additional null/unknown checks to immediately roundtrip the null/unknown back. If that's not the case, will post another update here.

@jbardin
Copy link
Member Author

jbardin commented Apr 26, 2024

Thanks @bflad, I didn't check to see if this was somehow getting encoded as an object with nulls rather than a null object coming from core, but that's possible too. Even better, if we can determine that this could never work in any form I can extend the validation to reject these plans entirely, but that's harder to prove.

@jbardin
Copy link
Member Author

jbardin commented Apr 26, 2024

The value from core looks correct, I see "key2":cty.NullVal(... being encoded for PlanResourceChange

@bflad
Copy link
Member

bflad commented Apr 26, 2024

Okay thanks!

bflad added a commit that referenced this issue Apr 29, 2024
…attributes plan modification if object is null or unknown

Reference: #993

By data definition, a null or unknown object has no underlying data where nested attributes should run plan modification. By Terraform data consistency rules, a null or unknown object should be preserved as-is (unless it is an unknown value being refined into a more known value), however this was not the case where a collection-based nested attribute had a null or unknown object, such as this example configuration:

```
resource "examplecloud_thing" "example" {
  nested_map = {
    examplekey = null # this was previously problematic
  }
}
```

Previously, a null or unknown object would be errantly transformed by the framework plan modification logic into a known object with null/unknown attributes. This could be seen in human-readable Terraform plans, such as (`{}` below):

```
  # examplecloud_thing.example will be created
  + resource "examplecloud_thing" "example" {
      + nested_map = {
          + "examplekey" = {},
        }
    }
```

Following new unit test creation, failures could be seen from the PlanResourceChange RPC:

```
--- FAIL: TestServerPlanResourceChange_AttributeRoundtrip (0.00s)
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-list-nested-optional-and-computed-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attrib`...,
            + 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"list_nested_attribute": schema.ListNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-list-nested-optional-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attrib`...,
            + 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<null>>>`,
              		Schema: schema.Schema{Attributes: {"list_nested_attribute": schema.ListNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-list-nested-optional-and-computed-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attrib`...,
            + 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"list_nested_attribute": schema.ListNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-map-nested-optional-and-computed-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<"string_att`...,
            + 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"map_nested_attribute": schema.MapNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-map-nested-optional-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<"string_att`...,
            + 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"map_nested_attribute": schema.MapNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-set-nested-optional-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attribute"`...,
            + 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"set_nested_attribute": schema.SetNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-set-nested-optional-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attribute"`...,
            + 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<null>>>`,
              		Schema: schema.Schema{Attributes: {"set_nested_attribute": schema.SetNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-set-nested-optional-and-computed-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attribute"`...,
            + 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"set_nested_attribute": schema.SetNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-map-nested-optional-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<"string_att`...,
            + 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<null>>>`,
              		Schema: schema.Schema{Attributes: {"map_nested_attribute": schema.MapNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-set-nested-optional-and-computed-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attribute"`...,
            + 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"set_nested_attribute": schema.SetNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-map-nested-optional-and-computed-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<"string_att`...,
            + 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"map_nested_attribute": schema.MapNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-list-nested-optional-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attrib`...,
            + 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"list_nested_attribute": schema.ListNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
```

```
--- FAIL: TestServerPlanResourceChange_AttributeRoundtrip (0.00s)
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-list-nested-optional-and-computed-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<null>>>`,
            + 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"list_nested_attribute": schema.ListNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-set-nested-optional-and-computed-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<null>>>`,
            + 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"set_nested_attribute": schema.SetNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-map-nested-optional-and-computed-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<null>>>`,
            + 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"map_nested_attribute": schema.MapNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
```

The plan modification logic was updated to return a null/unknown object early, but only after the object plan modifiers have run. This ensures that provider developers still have an escape hatch to run logic before the framework skips the nested attributes.

If provider developers need to override this logic change, object-level plan modifiers can convert a null/unknown object into a known object will all null/unknown attributes, which then the framework will run nested attribute plan modification on those attributes. If this is a common use case, a feature request can be submitted to consider including this type of object plan modifier in the framework.
@bflad bflad self-assigned this Apr 29, 2024
@bflad
Copy link
Member

bflad commented Apr 29, 2024

The framework was errantly converting those null/unknown objects into known objects by always running plan modification on the nested attributes. #995 will now skip that automatic behavior (instead roundtripping the null/unknown object) unless overridden by the provider developer.

bflad added a commit that referenced this issue Apr 30, 2024
…attributes plan modification if object is null or unknown (#995)

Reference: #993

By data definition, a null or unknown object has no underlying data where nested attributes should run plan modification. By Terraform data consistency rules, a null or unknown object should be preserved as-is (unless it is an unknown value being refined into a more known value), however this was not the case where a collection-based nested attribute had a null or unknown object, such as this example configuration:

```
resource "examplecloud_thing" "example" {
  nested_map = {
    examplekey = null # this was previously problematic
  }
}
```

Previously, a null or unknown object would be errantly transformed by the framework plan modification logic into a known object with null/unknown attributes. This could be seen in human-readable Terraform plans, such as (`{}` below):

```
  # examplecloud_thing.example will be created
  + resource "examplecloud_thing" "example" {
      + nested_map = {
          + "examplekey" = {},
        }
    }
```

Following new unit test creation, failures could be seen from the PlanResourceChange RPC:

```
--- FAIL: TestServerPlanResourceChange_AttributeRoundtrip (0.00s)
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-list-nested-optional-and-computed-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attrib`...,
            + 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"list_nested_attribute": schema.ListNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-list-nested-optional-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attrib`...,
            + 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<null>>>`,
              		Schema: schema.Schema{Attributes: {"list_nested_attribute": schema.ListNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-list-nested-optional-and-computed-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attrib`...,
            + 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"list_nested_attribute": schema.ListNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-map-nested-optional-and-computed-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<"string_att`...,
            + 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"map_nested_attribute": schema.MapNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-map-nested-optional-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<"string_att`...,
            + 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"map_nested_attribute": schema.MapNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-set-nested-optional-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attribute"`...,
            + 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"set_nested_attribute": schema.SetNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-set-nested-optional-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attribute"`...,
            + 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<null>>>`,
              		Schema: schema.Schema{Attributes: {"set_nested_attribute": schema.SetNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-set-nested-optional-and-computed-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attribute"`...,
            + 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"set_nested_attribute": schema.SetNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-map-nested-optional-null-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<"string_att`...,
            + 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<null>>>`,
              		Schema: schema.Schema{Attributes: {"map_nested_attribute": schema.MapNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-set-nested-optional-and-computed-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attribute"`...,
            + 		Raw:    s`tftypes.Object["set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]]<"set_nested_attribute":tftypes.Set[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"set_nested_attribute": schema.SetNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-map-nested-optional-and-computed-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<"string_att`...,
            + 		Raw:    s`tftypes.Object["map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]]<"map_nested_attribute":tftypes.Map[tftypes.Object["string_attribute":tftypes.String]]<"null":tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"map_nested_attribute": schema.MapNestedAttribute{NestedObject: {...}, Optional: true, Computed: true}}},
              	},
              	RequiresReplace: s"[]",
              }
    --- FAIL: TestServerPlanResourceChange_AttributeRoundtrip/create-list-nested-optional-unknown-element (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:15072: unexpected difference:   &fwserver.PlanResourceChangeResponse{
              	Diagnostics:    nil,
              	PlannedPrivate: &{Provider: &{data: {}}},
              	PlannedState: &tfsdk.State{
            - 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<"string_attrib`...,
            + 		Raw:    s`tftypes.Object["list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]]<"list_nested_attribute":tftypes.List[tftypes.Object["string_attribute":tftypes.String]]<tftypes.Object["string_attribute":tftypes.String]<unknown>>>`,
              		Schema: schema.Schema{Attributes: {"list_nested_attribute": schema.ListNestedAttribute{NestedObject: {...}, Optional: true}}},
              	},
              	RequiresReplace: s"[]",
              }
```

The plan modification logic was updated to return a null/unknown object early, but only after the object plan modifiers have run. This ensures that provider developers still have an escape hatch to run logic before the framework skips the nested attributes.

If provider developers need to override this logic change, object-level plan modifiers can convert a null/unknown object into a known object will all null/unknown attributes, which then the framework will run nested attribute plan modification on those attributes. If this is a common use case, a feature request can be submitted to consider including this type of object plan modifier in the framework.
@bflad bflad added this to the v1.9.0 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants