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

Support dynamic types in collections #973

Open
sebhoss opened this issue Apr 2, 2024 · 6 comments
Open

Support dynamic types in collections #973

sebhoss opened this issue Apr 2, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@sebhoss
Copy link

sebhoss commented Apr 2, 2024

Module version

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

Use-cases

I'm trying out the new dynamic types in version 1.7 and encountered a limitation while trying to use a dynamic type within a collection. My guess is that is the same underlying restriction I previously reported in #147 and therefore (almost?) impossible to fix. However I'm not that deep into the internals of terraform, thus I wanted to ask/clarify whether dynamic types in collections will ever be supported or not.

Attempted Solutions

I simply used my custom dynamic type within a collection and got the following error:

monitoring_coreos_com_pod_monitor_v1_manifest_test.go:29: Schema validation diagnostics: [{detail:When validating the schema, an implementation issue was found. This is always an issue with the provider and should be reported to the provider developers.
        
        "spec.pod_metrics_endpoints" is an attribute that contains a collection type with a nested dynamic type.
        
Dynamic types inside of collections are not currently supported in terraform-plugin-framework. If underlying dynamic values are required, replace the "spec.pod_metrics_endpoints" attribute definition with DynamicAttribute instead. summary:Invalid Schema Implementation}]

The suggestion to change the type of the collection itself to a DynamicAttribute might work, however I fear that this will require lots of attributes to be changed to dynamics and therefore a heavy loss on static type information.

Proposal

Well if it is at all possible, I'd love to have support for dynamic types in collections. If that's not possible, my immediate use case (an IntOrString type) will be solved the same way I do it today: Declare the type as string and tell users that this is something they have to keep in mind while using the provider.

References

#147

metio/terraform-provider-k8s#119

@sebhoss sebhoss added the enhancement New feature or request label Apr 2, 2024
@austinvalle
Copy link
Member

austinvalle commented Apr 3, 2024

Hey there @sebhoss 👋, thanks for reporting the issue.

General note, I'm going to reference list in all my examples below, but the same applies for map and set collections. I'm also going to use module type constraints, but the same type problem applies to resource schemas.

It is possible to support the equivalent of a list(any) type constraint in terraform-plugin-framework, but the reason it wasn't initially included with #147 / #931 was twofold:

  • We wanted to keep the initial scope of dynamic type support to be limited to the scenarios we were sure had consistent behavior across all attribute types and blocks in supported Terraform versions.
  • A "list of dynamic values" could easily be misinterpreted by provider developers in-terms of what's actually possible in Terraform, so we wanted to gauge developer interest (which we can do with this issue 😄) before introducing potentially confusing terminology and explanations.

What is possible

In Terraform, a collection type is multiple elements of a single element type, that's to say when a type constraint like list(any) is used, Terraform will always attempt to find a single exact element type for the collection.

So given the following configuration, assuming example_attribute is a list(any) type constraint, Terraform will convert the tuple literal to a list(string):

resource "examplecloud_thing" "test" {
  # tuple[number, string, bool] -> list(string), success! found single type of string
  example_attribute = [1234, "http", true]
}

So if the feature request here is focused on allowing Terraform to determine a single element type of a collection at runtime, then I believe that's possible to achieve.

What is not possible

The same list(any) would cause Terraform to raise an error if provided a configuration like this:

resource "examplecloud_thing" "test" {
  # tuple[number, bool] -> error! no single type found :(
  example_attribute = [1234, true]
}

It's not possible for us to define a constraint that would allow a collection that contains "multiple" element types, like list(number | bool). You can see where some confusion could arise with the any constraint.

Current Workaround

Suggesting the usage of DynamicAttribute at the root would solve the second configuration because it would be interpreted by Terraform as a tuple[number, bool] and not raise an error. The provider could then validate/handle each element and element type of the tuple appropriately. terraform-plugin-framework doesn't expose a tuple schema attribute since it's real-world usage is mostly limited, but there is a tuple type, which is only possible to receive from a dynamic attribute 😄 .

Adding a list of objects into the mix makes your specific situation a little more complicated in-terms of handling, so I would tend to agree with you that it's not worth it in your situation of number or string. Terraform would always attempt to find a single common type for the two, and the result would always be a string type.

The simplest way to test that kind of "type conversion" is with a little output and conversion:

output "test" {
  value = tolist([123, "456", 789])
}

State output:

{
  "version": 4,
  "terraform_version": "1.8.0",
  "serial": 26,
  "lineage": "87f4a8d4-57e7-2f4e-d081-aef9e7e6b675",
  "outputs": {
    "test": {
      "value": [
        "123",
        "456",
        "789"
      ],
      "type": [
        "list",
        "string"
      ]
    }
  },
  "resources": [],
  "check_results": null
}

@sebhoss
Copy link
Author

sebhoss commented Apr 4, 2024

Hey @austinvalle thanks for the detailed response!

I did not consider automatic type conversion here at all and after playing around with it for a while, I think it does solve my immediate use case quite well. Feel free to close this ticket & thanks again for the pointers ❤️

@austinvalle
Copy link
Member

No problem! I'm going to leave this issue open to collect any potential use-cases from future readers since there is functionality that can be added here, as long as it can fit inside Terraform's type system.

Terraform Type documentation:

@austinvalle
Copy link
Member

While this topic is fresh, it is possible to achieve the list(dynamic), set(dynamic), or map(dynamic) type constraints by creating a custom type.

I built a small "dynamic list" implementation in my sandbox provider@836539:

For this example, if you were to run terraform providers schema -json:

{
  "format_version": "1.0",
  "provider_schemas": {
    "registry.terraform.io/austinvalle/sandbox": {
      "provider": {
        "version": 0,
        "block": {
          "description_kind": "plain"
        }
      },
      "resource_schemas": {
        "examplecloud_thing": {
          "version": 0,
          "block": {
            "attributes": {
              "list_with_dynamics": {
                "type": [
                  "list",
                  "dynamic"
                ],
                "description_kind": "plain",
                "required": true
              }
            },
            "description_kind": "plain"
          }
        }
      }
    }
  }
}

DISCLAIMER: This custom type hasn't been exhaustively tested and I stubbed a few methods (like (DynamicListType).Validate), but just wanted to throw this out there in-case it's useful for future readers 👍🏻

@smartinellimarco
Copy link

@austinvalle This would be super useful for those APIs made with FastAPI or Django where its pretty common to use Union types for certain attributes.

			...
			
			"input": schema.ListNestedAttribute{
				Required: true,
				NestedObject: schema.NestedAttributeObject{
					Attributes: map[string]schema.Attribute{
						"name": schema.StringAttribute{
							Required: true,
						},
						"description": schema.StringAttribute{
							Optional: true,
						},
						"multiple": schema.BoolAttribute{
							Required: true,
						},
						"required": schema.BoolAttribute{
							Required: true,
						},
						"sensitive": schema.BoolAttribute{
							Required: true,
						},
						"value": schema.DynamicAttribute{
							Required: true,
						},
					},
				},
			},
			
			...

In our case we expect value being a float, str, bool, [int], [float], [str] or [bool].
For now we are using a JSONAttribute and sending it as a json.RawMessage.
But it would be nice to have the validation from the provider schema.

@austinvalle
Copy link
Member

Thanks for the example @smartinellimarco! Just a few thoughts to make sure we're on the same page.

In your example where value is a dynamic attribute in a list of objects, i.e. list(object({ value=any })), value will always need to resolve to a single element type in a configuration.

For example, the following two configurations would be valid:

# Resulting type would be list(object({value=list(string)}))
output "input_example_1" {
  value = tolist(
    [
      { value = tolist(["one", "two"]) },
      { value = tolist(["three", "four"]) },
      { value = tolist(["five", "six"]) },
    ]
  )
}

# Resulting type would be `list(object({value=string}))`
output "input_example_2" {
  value = tolist(
    [
      { value = "one" },
      { value = "two" },
      { value = 3 },
      { value = false },
    ]
  )
}

However this would be invalid:

output "input_example" {
  value = tolist(
    [
      { value = "one" },
      { value = "two" },
      { value = tolist(["three", "four"]) },
    ]
  )
}

Depending on the underlying API implementation, that may not be the same expectation, so wanted to be upfront on what is actually possible.

Given that, the types you'd be able to achieve with this constraint would be restricted to one of the following:

  • list(object({ value=number }))
  • list(object({ value=string }))
  • list(object({ value=boolean }))
  • list(object({ value=list(number) }))
  • list(object({ value=list(string) }))
  • list(object({ value=list(boolean) }))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants