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

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

Open
henryrecker-pingidentity opened this issue Sep 13, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@henryrecker-pingidentity

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

@henryrecker-pingidentity henryrecker-pingidentity added the bug Something isn't working label Sep 13, 2024
@austinvalle
Copy link
Member

austinvalle commented Sep 16, 2024

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 😞

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
Development

No branches or pull requests

2 participants