hashicorp / terraform-plugin-framework

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

Provider Attribute Default to properly support muxing with SDKv2 provider #539

Open timofurrer opened 1 year ago

timofurrer commented 1 year ago

This "request" is related to https://github.com/hashicorp/terraform-plugin-mux as well, but I figured it makes more sense here, because the framework doesn't yet provide a proper solution for this - but feel free to move it to the appropriate repository.

I'm currently prototyping a migration from a SDKv2 provider to the framework and implemented a muxing provider server with:

  1. upgraded SDKv2 provider (to protocol v6)
  2. framework provider

The SDKv2 provider has a provider attribute schema with defaults, e.g.:

"token": {
    Type:        schema.TypeString,
    Optional:    true,
    DefaultFunc: schema.EnvDefaultFunc("GITLAB_TOKEN", nil),
},
"cacert_file": {
    Type:        schema.TypeString,
    Optional:    true,
    Default:     "",
},
"insecure": {
    Type:        schema.TypeBool,
    Optional:    true,
    Default:     false,
},

Accordingly the framework provider must have the same attribute schema, e.g.:

"token": {
        Type:                types.StringType,
        Optional:            true,
},
"cacert_file": {
        Type:                types.StringType,
        Optional:            true,
},
"insecure": {
        Type:                types.BoolType,
        Optional:            true,
},

AFAIK The framework doesn't yet support a mechanism to declaratively set a default value - which "functionally" isn't a problem, because I can set them in the Configure() method.

However, it's a problem with muxing, because the tf6muxserver actually checks if the PreparedConfig RPC responses match from all servers:

https://github.com/hashicorp/terraform-plugin-mux/blob/411419dcd18e833d58a543344d009dadb7594bb9/tf6muxserver/mux_server_ValidateProviderConfig.go#L58

The thing is that the PreparedConfig from the SDKv2 provider contains the evaluated defaults ...

A related problem to the inability to declare defaults in framework providers when using muxing, is that previously required attributes with a default now must be optional - which also requires a change in the SDKv2 provider to make muxing work.

One solution could be to remove the defaults from the SDKv2 provider schema and also evaluate them in the configure hook function - but I'd rather not do that.

Is this the only solution? Am I missing something? If that's really the only possible solution atm - is there a change to be expected to make this more ergonomic ?

bflad commented 1 year ago

Hi @timofurrer 👋 Thank you for raising this feature request.

The current migration path in this case is, as you gathered, removing the Default/DefaultFunc definitions in the terraform-plugin-sdk provider schema and instead handling them in the Configure function logic similar to the terraform-plugin-framework implementation.

Handling schema-defined defaults in the framework is not an area that has had much design effort done yet. There are some potential implementation considerations similar to problems that arose with terraform-plugin-sdk's design, which prevents them from being straightforward.

Provider developers may wish to distinguish whether the value was sourced from the configuration or the default. If the default value handling overrides the configuration value rather than being separate data, then that cannot happen. A solution may be to introduce "merged" configuration + defaults data as a separate field. Having that separate data may raise its own problems though since it could be an easy implementation detail to miss.

Defaults values may be static or potentially come from another source. Static values and environment variables are the easiest sources as they are always "available", but theoretically a provider may also wish to get a default from its API, based on the given credentials. Whether the framework should enable this level of extensibility with its implementation is questionable, however it diminishes the value of providing the option if provider developers cannot implement functionality consistently.

Another quirk of environment variable values in the framework will be whether to distinguish between empty strings and missing environment variables. Likely the answer is that the distinction should happen, but it may influence how the implementation should work.

Another particular example, many providers opt to group similar settings together in a block or single nested attribute rather than polluting the top-level attribute space. Code example:

schema.SingleNestedAttribute{
  Attributes: map[string]schema.Attribute{
    "example_attribute": schema.StringAttribute{
      Optional: true,
      // Example "default" handling field/implementation; not a proposal
      Default: stringdefault.ValueFromEnvironmentVariable("XXX"),
    },
    // ... other attributes ...
  },
}

In this situation, defaults are only handled if the object itself is configured. Otherwise, the provider receives a null object and must manually re-check the environment variable in the Configure logic if it is an important implementation detail, partially defeating the purpose of defining the default.

Any "default" implementation should also consider the implications of whether it can be provided across all the schema types as well. Providers and data sources tend to be the "easier" cases since they are only potentially in play with relation to null configuration values, however resources also have to potentially contend with differing plan values.

Considerations like these do not necessarily mean that the framework cannot or will not support functionality like this, however it does raise questions whether a potentially confusing implementation outweighs the potential simplifications. Given that provider implementations can be easier to reason about when static defaults can be added to the schema though, proposals are certainly welcome.

bflad commented 7 months ago

Since terraform-plugin-mux v0.12.0, it no longer performs this same provider configuration data equality check across all underlying providers. It is up to provider developers to ensure their provider configuration implementations are consistent. Therefore that artificial constraint where the differing schema implementations between terraform-plugin-sdk/v2 and terraform-plugin-framework would cause issues is no longer in place.

In terms of the general ability to set provider attribute defaults, this likely falls under the same implementation design tradeoffs as being discussed in the data source defaults feature request over here: https://github.com/hashicorp/terraform-plugin-framework/issues/751