hashicorp / terraform-plugin-framework

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

Computed sensitive attribute alongside computed boolean in SetNestedAttribute causes entire set to be deleted and recreated in subsequent plans #1036

Open henryrecker-pingidentity opened 1 week ago

henryrecker-pingidentity commented 1 week ago

Module version

github.com/hashicorp/terraform-plugin-framework v1.11.0

Relevant provider source code

func (r *sensitiveResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
    resp.Schema = schema.Schema{
        Description: "Sensitive resource.",
        Attributes: map[string]schema.Attribute{
            "tables": schema.SetNestedAttribute{
                Description: "List of configuration tables.",
                Optional:    true,
                NestedObject: schema.NestedAttributeObject{
                    Attributes: map[string]schema.Attribute{
                        "rows": schema.ListNestedAttribute{
                            Description: "List of table rows.",
                            Optional:    true,
                            NestedObject: schema.NestedAttributeObject{
                                Attributes: map[string]schema.Attribute{
                                    "sensitive_fields": schema.SetNestedAttribute{
                                        Description: "The sensitive configuration fields in the row.",
                                        Optional:    true,
                                        Computed:    true,
                                        Default: setdefault.StaticValue(types.SetValueMust(types.ObjectType{AttrTypes: map[string]attr.Type{
                                            "name":  types.StringType,
                                            "value": types.StringType,
                                        }}, nil)),
                                        NestedObject: schema.NestedAttributeObject{
                                            Attributes: map[string]schema.Attribute{
                                                "name": schema.StringAttribute{
                                                    Description: "The name of the configuration field.",
                                                    Required:    true,
                                                },
                                                "value": schema.StringAttribute{
                                                    Description: "The sensitive value for the configuration field.",
                                                    Required:    true,
                                                    Sensitive:   true,
                                                },
                                            },
                                        },
                                    },
                                    "default_row": schema.BoolAttribute{
                                        Description: "Whether this row is the default.",
                                        Computed:    true,
                                        Optional:    true,
                                        Default:     booldefault.StaticBool(false),
                                    },
                                },
                            },
                        },
                    },
                },
            },
        },
    }
}

func (r *sensitiveResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
    resp.State.Raw = req.Plan.Raw
}

func (r *sensitiveResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
    resp.State.Raw = req.State.Raw
}

func (r *sensitiveResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
    resp.State.Raw = req.Plan.Raw
}

func (r *sensitiveResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
}

Terraform Configuration Files

resource "example_sensitive" "sensitive" {
  # If tables is a list instead of a set, the unexpected plans do not occur
  tables = [
    {
      rows = [
        {
          fields = [
            {
              name  = "Key ID"
              value = "jwtSymmetricKey1"
            },
            {
              name  = "Encoding"
              value = "b64u"
            }
          ]
          sensitive_fields = [
            {
              name  = "Key"
              value = "Asdf"
            },
          ]
          # If this attribute is uncommented, the unexpected plans do not occur
          # default_row = false
        }
      ]
    },
  ]
}

Debug Output

https://gist.github.com/henryrecker-pingidentity/9cb39885e2d338fc7189543d501b413c

Expected Behavior

Plans after initial create should indicate no changes needed, infrastructure matches configuration.

Actual Behavior

After a Create runs successfully, a subsequent apply will generate a plan that completely deletes the "tables" attribute. Another subsequent apply after that will recreate the "tables" attribute, as it is written in the HCL. And it will continue to flip-flop from there.

If the "tables" attribute is changed to ListNestedAttribute rather than SetNestedAttribute, the bug stops occurring.

Steps to Reproduce

Apply provided HCL repeatedly. Uncommenting the "default_row" boolean prevents the flip-flopping, as does changing tables to a list in the schema.

This provider can be found on the "SensitivePlanBug" branch here - https://github.com/henryrecker-pingidentity/terraform-provider-example/tree/SensitivePlanBug

References

Seems very similar to #867

austinvalle commented 1 week ago

Hey there @henryrecker-pingidentity 👋🏻 , thanks for reporting the bug and sorry you're running into issues here.

Appreciate the detailed example, I went through the call path and confirmed it's the same framework bug being caused by the default value implementation when applied for nested attributes in sets, i.e. #783 and #867.

The default provided by default_row is modifying the set identity for the element in tables, which causes the eventual lookup of sensitive_fields to return null, thus triggering it's default value of an empty set. Unfortunately, the only workaround until this is fixed in framework would be to remove the default values from attributes inside of set nested attributes and replace with a plan modifier or resource logic (create/read/update/delete).

For your example here:

// Temporary workaround until framework bug is fixed
// https://github.com/hashicorp/terraform-plugin-framework/issues/783
func BoolDefault(defaultVal bool) planmodifier.Bool {
    return boolDefaultPlanModifier{
        defaultVal: defaultVal,
    }
}

type boolDefaultPlanModifier struct {
    defaultVal bool
}

func (m boolDefaultPlanModifier) Description(_ context.Context) string {
    return fmt.Sprintf("value defaults to %t", m.defaultVal)
}

func (m boolDefaultPlanModifier) MarkdownDescription(_ context.Context) string {
    return fmt.Sprintf("value defaults to %t", m.defaultVal)
}

func (m boolDefaultPlanModifier) PlanModifyBool(_ context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) {
    // Only apply the default if config is null.
    if !req.ConfigValue.IsNull() {
        return
    }

    resp.PlanValue = types.BoolValue(m.defaultVal)
}
// Temporary workaround until framework bug is fixed
// https://github.com/hashicorp/terraform-plugin-framework/issues/783
func SetDefault(defaultVal types.Set) planmodifier.Set {
    return setDefaultPlanModifier{
        defaultVal: defaultVal,
    }
}

type setDefaultPlanModifier struct {
    defaultVal types.Set
}

func (m setDefaultPlanModifier) Description(_ context.Context) string {
    return fmt.Sprintf("value defaults to %v", m.defaultVal)
}

func (m setDefaultPlanModifier) MarkdownDescription(_ context.Context) string {
    return fmt.Sprintf("value defaults to %v", m.defaultVal)
}

func (m setDefaultPlanModifier) PlanModifySet(_ context.Context, req planmodifier.SetRequest, resp *planmodifier.SetResponse) {
    // Only apply the default if config is null.
    if !req.ConfigValue.IsNull() {
        return
    }

    resp.PlanValue = m.defaultVal
}

func (r *sensitiveResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
    resp.Schema = schema.Schema{
        Description: "Sensitive resource.",
        Attributes: map[string]schema.Attribute{
            "tables": schema.SetNestedAttribute{
                Description: "List of configuration tables.",
                Optional:    true,
                NestedObject: schema.NestedAttributeObject{
                    Attributes: map[string]schema.Attribute{
                        "rows": schema.ListNestedAttribute{
                            Description: "List of table rows.",
                            Optional:    true,
                            NestedObject: schema.NestedAttributeObject{
                                Attributes: map[string]schema.Attribute{
                                    "sensitive_fields": schema.SetNestedAttribute{
                                        Description: "The sensitive configuration fields in the row.",
                                        Optional:    true,
                                        Computed:    true,
                                        // Temporary workaround until framework bug is fixed
                                        // https://github.com/hashicorp/terraform-plugin-framework/issues/783
                                        PlanModifiers: []planmodifier.Set{
                                            SetDefault(types.SetValueMust(types.ObjectType{AttrTypes: map[string]attr.Type{
                                                "name":  types.StringType,
                                                "value": types.StringType,
                                            }}, nil)),
                                        },
                                        NestedObject: schema.NestedAttributeObject{
                                            Attributes: map[string]schema.Attribute{
                                                "name": schema.StringAttribute{
                                                    Description: "The name of the configuration field.",
                                                    Required:    true,
                                                },
                                                "value": schema.StringAttribute{
                                                    Description: "The sensitive value for the configuration field.",
                                                    Required:    true,
                                                    Sensitive:   true,
                                                },
                                            },
                                        },
                                    },
                                    "default_row": schema.BoolAttribute{
                                        Description: "Whether this row is the default.",
                                        Computed:    true,
                                        Optional:    true,
                                        // Temporary workaround until framework bug is fixed
                                        // https://github.com/hashicorp/terraform-plugin-framework/issues/783
                                        PlanModifiers: []planmodifier.Bool{
                                            BoolDefault(false),
                                        },
                                    },
                                },
                            },
                        },
                    },
                },
            },
        },
    }
}

Sets in general are problematic, for example, this kind of bug would typically raise a more explicit "Provider produced invalid plan" error, but Terraform's data consistency rules can't be consistently applied to nested objects inside of sets because it can't determine when a nested object change inside a set is valid, so it only compares the lengths of both sets: https://github.com/hashicorp/terraform/blob/55d4cbb00a3ba20f8c1db210fed00456bbfe4852/internal/plans/objchange/plan_valid.go#L468-L480

In your case, since your set is inside of another set, those lengths for sensitive_fields don't get compared at all 😞