hashicorp / terraform-plugin-framework

A next-generation framework for building Terraform providers.
https://developer.hashicorp.com/terraform/plugin/framework
Mozilla Public License 2.0
298 stars 92 forks source link

Allow Upgrade State with SetNull for SetNestedAttribute with NestedAttributeObject #962

Closed akinross closed 6 months ago

akinross commented 6 months ago

Module version

542e128d5f7b:/terraform-provider-aci# go list -m github.com/hashicorp/terraform-plugin-framework
github.com/hashicorp/terraform-plugin-framework v1.6.1

Relevant provider source code

...
            StateUpgrader: func(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) {
                var priorStateData MonEPGPolResourceModelV1

                resp.Diagnostics.Append(req.State.Get(ctx, &priorStateData)...)

                if resp.Diagnostics.HasError() {
                    return
                }

                upgradedStateData := MonEPGPolResourceModel{
                    Id:         priorStateData.Id,
                    ParentDn:   priorStateData.TenantDn,
                    Name:       priorStateData.Name,
                    Annotation: priorStateData.Annotation,
                    Descr:      priorStateData.Descr,
                    NameAlias:  priorStateData.NameAlias,
                    OwnerKey:   basetypes.NewStringNull(),
                    OwnerTag:   basetypes.NewStringNull(),
                }
                upgradedStateData.TagAnnotation = types.SetNull(upgradedStateData.TagAnnotation.ElementType(ctx))
                upgradedStateData.TagTag = types.SetNull(upgradedStateData.TagTag.ElementType(ctx))

                resp.Diagnostics.Append(resp.State.Set(ctx, upgradedStateData)...)
            },
...

Terraform Configuration Files

Debug Output

Expected Behavior

The state upgrader allowing me to set the value to SetNull for (SetNestedAttribute with NestedAttributeObject).

Actual Behavior

I'm facing the following error, where it expects type to be specified.

│ Expected framework type from provider logic: types.SetType[types.ObjectType["key":basetypes.StringType,
│ "value":basetypes.StringType]] / underlying type: tftypes.Set[tftypes.Object["key":tftypes.String, "value":tftypes.String]]
│ Received framework type from provider logic: types.SetType[!!! MISSING TYPE !!!] / underlying type:
│ tftypes.Set[tftypes.DynamicPseudoType]
│ Path: annotations

Steps to Reproduce

References

bflad commented 6 months ago

Hi @akinross 👋 Thank you for raising this and sorry you are running into trouble here.

Could you also provide the associated schema, MonEPGPolResourceModel, and MonEPGPolResourceModelV1 code? It is unfortunately a little difficult to help troubleshoot this sort of upgrade state issue without that additional context.

Upfront though from the error messaging, it is mentioning an attribute path of annotations however it only appears that Annotation (presumably with field tag tfsdk:"annotation") and TagAnnotation (presumably with field tag tfsdk:"tag_annotation") in MonEPGPolResourceModel are being set. Given the plural naming, this seems to suggest that upgradedStateData.Annotations should also be set in some fashion. If that data is not available from the prior state then it might require manually coding upgradedStateData.Annotations = types.SetNull(/* hardcode element type here */). That hardcoded element type information could be retrieved automatically by fetching the schema and using schema methods to get the type of the attribute, or by splitting out the attribute definition into its own variable and calling methods on that to get the type information.

The framework does not make any assumptions about what type information should be associated with a value and a wholly unset types.Set value by itself has no element type information, which is the reason for the error. The framework does not do any automatic typing or null value setting in these cases because it could enable developers to introduce subtle data loss bugs that could be very painful for practitioners if there is no way to refresh the data from the real world resource. Put differently, this is intentionally designed this way to prioritize data safety by raising this sort of implementation error.

Please let us know how it goes or the additional information if you are still having trouble.

akinross commented 6 months ago

Hi @bflad,

Thank you for your response. That makes sense as to why it error is occurring. Would you have a example of the hardcoding element types in the set?

The resource models are as follow:

type MonEPGPolResourceModel struct {
    Id            types.String `tfsdk:"id"`
    ParentDn      types.String `tfsdk:"parent_dn"`
    Annotation    types.String `tfsdk:"annotation"`
    Descr         types.String `tfsdk:"description"`
    Name          types.String `tfsdk:"name"`
    NameAlias     types.String `tfsdk:"name_alias"`
    OwnerKey      types.String `tfsdk:"owner_key"`
    OwnerTag      types.String `tfsdk:"owner_tag"`
    TagAnnotation types.Set    `tfsdk:"annotations"`
    TagTag        types.Set    `tfsdk:"tags"`
}

type TagAnnotationMonEPGPolResourceModel struct {
    Key   types.String `tfsdk:"key"`
    Value types.String `tfsdk:"value"`
}

type TagTagMonEPGPolResourceModel struct {
    Key   types.String `tfsdk:"key"`
    Value types.String `tfsdk:"value"`
}

type MonEPGPolResourceModelV1 struct {
    Id         types.String `tfsdk:"id"`
    TenantDn   types.String `tfsdk:"tenant_dn"`
    Name       types.String `tfsdk:"name"`
    NameAlias  types.String `tfsdk:"name_alias"`
    Descr      types.String `tfsdk:"description"`
    Annotation types.String `tfsdk:"annotation"`
}

The old schema is:

PriorSchema: &schema.Schema{
                Attributes: map[string]schema.Attribute{
                    "id": schema.StringAttribute{
                        Computed: true,
                    },
                    "tenant_dn": schema.StringAttribute{
                        Required: true,
                    },
                    "name": schema.StringAttribute{
                        Required: true,
                    },
                    "name_alias": schema.StringAttribute{
                        Optional: true,
                        Computed: true,
                    },
                    "annotation": schema.StringAttribute{
                        Optional: true,
                        Computed: true,
                    },
                    "description": schema.StringAttribute{
                        Optional: true,
                        Computed: true,
                    },
                },
            },

The new schema is:

resp.Schema = schema.Schema{
        // This description is used by the documentation generator and the language server.
        MarkdownDescription: "The monitoring_policy resource for the 'monEPGPol' class",
        Version:             2,
        Attributes: map[string]schema.Attribute{
            "id": schema.StringAttribute{
                Computed:            true,
                MarkdownDescription: "The distinguished name (DN) of the Monitoring Policy object.",
                PlanModifiers: []planmodifier.String{
                    stringplanmodifier.UseStateForUnknown(),
                },
            },
            "parent_dn": schema.StringAttribute{
                Required:            true,
                MarkdownDescription: "The distinguished name (DN) of the parent object.",
                PlanModifiers: []planmodifier.String{
                    stringplanmodifier.UseStateForUnknown(),
                    stringplanmodifier.RequiresReplace(),
                },
            },
            "annotation": schema.StringAttribute{
                Optional: true,
                Computed: true,
                PlanModifiers: []planmodifier.String{
                    stringplanmodifier.UseStateForUnknown(),
                },
                Default:             stringdefault.StaticString(globalAnnotation),
                MarkdownDescription: `The annotation of the Monitoring Policy object.`,
            },
            "description": schema.StringAttribute{
                Optional: true,
                Computed: true,
                PlanModifiers: []planmodifier.String{
                    stringplanmodifier.UseStateForUnknown(),
                },
                MarkdownDescription: `The description of the Monitoring Policy object.`,
            },
            "name": schema.StringAttribute{
                Required: true,
                PlanModifiers: []planmodifier.String{
                    stringplanmodifier.UseStateForUnknown(),
                    stringplanmodifier.RequiresReplace(),
                },
                MarkdownDescription: `The name of the Monitoring Policy object.`,
            },
            "name_alias": schema.StringAttribute{
                Optional: true,
                Computed: true,
                PlanModifiers: []planmodifier.String{
                    stringplanmodifier.UseStateForUnknown(),
                },
                MarkdownDescription: `The name alias of the Monitoring Policy object.`,
            },
            "owner_key": schema.StringAttribute{
                Optional: true,
                Computed: true,
                PlanModifiers: []planmodifier.String{
                    stringplanmodifier.UseStateForUnknown(),
                },
                MarkdownDescription: `The key for enabling clients to own their data for entity correlation.`,
            },
            "owner_tag": schema.StringAttribute{
                Optional: true,
                Computed: true,
                PlanModifiers: []planmodifier.String{
                    stringplanmodifier.UseStateForUnknown(),
                },
                MarkdownDescription: `A tag for enabling clients to add their own data. For example, to indicate who created this object.`,
            },
            "annotations": schema.SetNestedAttribute{
                MarkdownDescription: ``,
                Optional:            true,
                Computed:            true,
                PlanModifiers: []planmodifier.Set{
                    setplanmodifier.UseStateForUnknown(),
                },
                NestedObject: schema.NestedAttributeObject{
                    Attributes: map[string]schema.Attribute{
                        "key": schema.StringAttribute{
                            Required: true,
                            PlanModifiers: []planmodifier.String{
                                stringplanmodifier.UseStateForUnknown(),
                            },
                            MarkdownDescription: `The key used to uniquely identify this configuration object.`,
                        },
                        "value": schema.StringAttribute{
                            Required: true,
                            PlanModifiers: []planmodifier.String{
                                stringplanmodifier.UseStateForUnknown(),
                            },
                            MarkdownDescription: `The value of the property.`,
                        },
                    },
                },
            },
            "tags": schema.SetNestedAttribute{
                MarkdownDescription: ``,
                Optional:            true,
                Computed:            true,
                PlanModifiers: []planmodifier.Set{
                    setplanmodifier.UseStateForUnknown(),
                },
                NestedObject: schema.NestedAttributeObject{
                    Attributes: map[string]schema.Attribute{
                        "key": schema.StringAttribute{
                            Required: true,
                            PlanModifiers: []planmodifier.String{
                                stringplanmodifier.UseStateForUnknown(),
                            },
                            MarkdownDescription: `The key used to uniquely identify this configuration object.`,
                        },
                        "value": schema.StringAttribute{
                            Required: true,
                            PlanModifiers: []planmodifier.String{
                                stringplanmodifier.UseStateForUnknown(),
                            },
                            MarkdownDescription: `The value of the property.`,
                        },
                    },
                },
            },
        },
    }
akinross commented 6 months ago

Hi @bflad, do you have an example for the hardcoding: types.SetNull(/* hardcode element type here */)?

akinross commented 6 months ago

HI @bflad, fixed by types.SetNull(types.ObjectType{AttrTypes: map[string]attr.Type{"key": basetypes.StringType{}, "value": basetypes.StringType{}}}). Thanks for your help.

github-actions[bot] commented 5 months ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.