Defaults don't work in nested attributes

pksunkara commented 1 year ago

Module version

v1.2.0

Relevant provider source code

type TeamResourceTriageModel struct {
    Enabled types.Bool `tfsdk:"enabled"`
}

type TeamResourceModel struct {
    Id                         types.String  `tfsdk:"id"`
    Name                       types.String  `tfsdk:"name"`
    Triage                     types.Object  `tfsdk:"triage"`
}

func (r *TeamResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
    resp.Schema = schema.Schema{
        MarkdownDescription: "Linear team.",
        Attributes: map[string]schema.Attribute{
            "id": schema.StringAttribute{
                MarkdownDescription: "Identifier of the team.",
                Computed:            true,
                PlanModifiers: []planmodifier.String{
                    stringplanmodifier.UseStateForUnknown(),
                },
            },
            "name": schema.StringAttribute{
                MarkdownDescription: "Name of the team.",
                Required:            true,
                Validators: []validator.String{
                    stringvalidator.UTF8LengthAtLeast(2),
                },
            },
            "triage": schema.SingleNestedAttribute{
                MarkdownDescription: "Triage settings of the team.",
                Optional:            true,
                Computed:            true,
                PlanModifiers: []planmodifier.Object{
                    modifiers.UnknownAttributesOnUnknown(),
                },
                Attributes: map[string]schema.Attribute{
                    "enabled": schema.BoolAttribute{
                        MarkdownDescription: "Enable triage mode for the team. **Default** `false`.",
                        Optional:            true,
                        Computed:            true,
                        Default: booldefault.StaticBool(true),
                    },
                },
            },
        },
    }
}

func (r *TeamResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
    var data *TeamResourceModel
    var triageData *TeamResourceTriageModel

    resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)

    if resp.Diagnostics.HasError() {
        return
    }

    input := TeamCreateInput{
        Name:                          data.Name.ValueString(),
    }

    resp.Diagnostics.Append(data.Triage.As(ctx, &triageData, basetypes.ObjectAsOptions{})...)

    if resp.Diagnostics.HasError() {
        return
    }

    input.TriageEnabled = triageData.Enabled.ValueBool()

    response, err := createTeam(ctx, *r.client, input)

    if err != nil {
        resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create team, got error: %s", err))
        return
    }

    tflog.Trace(ctx, "created a team")

    team := response.TeamCreate.Team

    data.Id = types.StringValue(team.Id)

    data.Triage = types.ObjectValueMust(
        map[string]attr.Type{
            "enabled": types.BoolType,
        },
        map[string]attr.Value{
            "enabled": types.BoolValue(team.TriageEnabled),
        },
    )

    resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
}

Terraform Configuration Files

resource "linear_team" "test" {
  name = "test"
}

Debug Output

Expected Behavior

The Graphql Request sent to the server contains triage_enabled as true (read from the default)

Actual Behavior

The Graphql Request sent to the server contains triage_enabled as false

Steps to Reproduce

References

I suspect this is caused by modifiers.UnknownAttributesOnUnknown, but it is recommended by @bflad at https://github.com/hashicorp/terraform-plugin-framework/issues/489#issuecomment-1255548450

pksunkara commented 1 year ago

@bflad I would really appreciate it if I can get any pointers here.

patrickcping commented 1 year ago

We've been playing with Default: objectdefault.StaticValue(..) on the schema.SingleNestedAttribute to achieve this. Would be interested to hear your experience with this as for us I'm hearing there are cases where this default overrides the declared values in the nested object

bflad commented 1 year ago

Hi folks 👋 Sorry for the delayed response here.

The behavior of schema-based Default value handling occurs when the "object layer" containing the attribute is known (not null or unknown). All root attributes are automatically invoked, however when nested attributes and blocks come into play, it is important to note that behavior versus your desired implementation when a higher "layer" may be null in a configuration. If setting default values in an underlying attribute within a single nested attribute (object) when that object is null is desired, then the single nested attribute itself must declare a default (potentially in addition to having a default on the underlying attribute for when the single nested attribute is not null, but the underlying attribute is still null). The reason for this is because it leaves the choice up to provider developers when they may or may not want objects instantiated, since that behavior is not always desirable.

To summarize this particular case with a single nested attribute with one underlying attribute:

Default on Single Nested Attribute	Default on Underlying Attribute	Configuration	Framework Behavior
No	No	Null single nested attribute	Passthrough (null single nested attribute and implicitly null underlying attribute)
No	No	Known single nested attribute, but null underlying attribute	Passthrough (known single nested attribute, but null underlying attribute)
No	No	Known single nested attribute and known underlying attribute	Passthrough (known single nested attribute and known underlying attribute)
No	Yes	Null single nested attribute	Passthrough (null single nested attribute and implicitly null underlying attribute)
No	Yes	Known single nested attribute, but null underlying attribute	Known single nested attribute with underlying attribute default
No	Yes	Known single nested attribute and known underlying attribute	Passthrough (known single nested attribute and known underlying attribute)
Yes	No	Null single nested attribute	Single nested attribute default
Yes	No	Known single nested attribute, but null underlying attribute	Passthrough (Known single nested attribute with null underlying attribute)
Yes	No	Known single nested attribute and known underlying attribute	Passthrough (known single nested attribute and known underlying attribute)
Yes	Yes	Null single nested attribute	Single nested attribute default
Yes	Yes	Known single nested attribute, but null underlying attribute	Known single nested attribute with underlying attribute default
Yes	Yes	Known single nested attribute and known underlying attribute	Passthrough (known single nested attribute and known underlying attribute)

Hopefully this clarifies some of the behaviors here and please reach out if the functionality is not working as I'm describing. It does not appear that the website documentation walks through this level of detail, so will take this issue to adjust that particular documentation.

pksunkara commented 1 year ago

No Yes Null single nested attribute Passthrough (null single nested attribute and implicitly null underlying attribute)

This is the issue I am running into. Right now, if I want this scenario to not trigger, I need to duplicate the defaults on the object level too.

Default: objectdefault.StaticValue(
    types.ObjectValueMust(
        cyclesAttrTypes,
        map[string]attr.Value{
            "enabled":            types.BoolValue(false),
            "start_day":          types.Float64Value(0),
            "duration":           types.Float64Value(1),
            "cooldown":           types.Float64Value(0),
            "upcoming":           types.Float64Value(2),
            "auto_add_started":   types.BoolValue(true),
            "auto_add_completed": types.BoolValue(true),
            "need_for_active":    types.BoolValue(false),
        },
    ),
),

Is there a way I can prevent this duplication and let the underlying attribute get set to the specified default?

bflad commented 1 year ago

@pksunkara I'm not aware of a way to do that today with existing framework functions/methods, but it may be possible to enhance the framework so that:

The object attribute types and/or nested attributes are passed through to the defaults.ObjectRequest type
A new objectdefault function is created that instantiates a known object using those types will either all null underlying values (and somehow triggering those underlying defaults to be called) or instantiating a known object using the attribute defaults directly

I'll create a feature request issue for tracking that proposal.

bflad commented 1 year ago

Created https://github.com/hashicorp/terraform-plugin-framework/issues/777 with above. The proposal details are a little hazy/unclear since we'd need to avoid an import cycle, but I'm hopeful that it might be possible to work around that issue.

hashicorp / terraform-plugin-framework