search

hashicorp / terraform-plugin-sdk

Terraform Plugin SDK enables building plugins (providers) to manage any service providers or custom in-house solutions
https://developer.hashicorp.com/terraform/plugin
Mozilla Public License 2.0
442 stars 232 forks source link

Upgrading OpenAPI Terraform provider to Terraform SDK 2.0: TypeMap with Elem*Resource not supported #616

Open dikhan opened 4 years ago

dikhan commented 4 years ago

Context

I am in the process of upgrading the OpenAPI Terraform provider to use the new Terraform SDK 2.0, and some int tests are now throwing the following error: TypeMap with Elem*Resource not supported

Previous versions of the SDK allowed TypeMap to have an associated Elem schema.Resource; however the behaviour was unpredictable causing diff issues etc. The main goal was to have an object type definition that contained different properties types (eg: string, int, etc) and configurations (eg: mix of props where some are expected as input from the user and others are are computed aka readOnly). As per other issues raised in the past 21217 and 22511, this seemed not to be supported and the recommendation from Terraform maintainers back then was to configure the property instead as TypeList with Elem Resource and limit size of 1. However, as per 21217 comment it was suggested that future versions of the SDK would support complex objects natively without having to use the workaround of TypeList with 1 Elem and I'd really appreciate if you could shed some light on it.

After updating to v2.0.3, looks like Terraform no longer supports TypeMap with Elem schema.Resource (as per debug output section) so I was wondering if the expectation/recommendation is still to configure complex objects using TypeList with Elem Resource and limit size of 1?

SDK version

github.com/hashicorp/terraform-plugin-sdk/v2 v2.0.3

Relevant provider source code

The OpenAPI Terraform provider automatically converts the following ContentDeliveryNetworkV1 definition into its corresponding Terraform schema.Schema as shown below (pasting the yaml too for the ref). Option 2 works as expected; Option 1 now fails due to SDK 2.0 no longer supporting TypeMap with Elem *Resource.

  ContentDeliveryNetworkV1:
    type: "object"
    properties:
      ...
      object_property_argument: # Option 1 of handling complex objects (eg: object containing props with different types (eg: string, int, etc) or configurations (eg: mix of props where some are expected as input from the user and others are are computed aka readOnly))
        type: "object"
        properties:
          account:
            type: string
          object_read_only_property:
            type: string
            readOnly: true
      object_property_block: # Option 2 of handling complex objects, the flag `x-terraform-complex-object-legacy-config` tells the provider to set up the property using the block format, as suggested by Hashi maintainers in the past using TypeList with Elem *Resource and limit size of 1
        type: "object"
        x-terraform-complex-object-legacy-config: true
        properties:
          account:
            type: string
          object_read_only_property:
            type: string
            readOnly: true
(map[string]*schema.Schema) (len=4) {
 (string) (len=24) "object_property_argument": (*schema.Schema)(0xc0002152c0)({
  Type: (schema.ValueType) TypeMap,
  ConfigMode: (schema.SchemaConfigMode) 0,
  Optional: (bool) true,
  Required: (bool) false,
  DiffSuppressFunc: (schema.SchemaDiffSuppressFunc) <nil>,
  Default: (interface {}) <nil>,
  DefaultFunc: (schema.SchemaDefaultFunc) <nil>,
  Description: (string) "",
  InputDefault: (string) "",
  Computed: (bool) false,
  ForceNew: (bool) false,
  StateFunc: (schema.SchemaStateFunc) <nil>,
  Elem: (*schema.Resource)(0xc00065c160)({
   Schema: (map[string]*schema.Schema) (len=2) {
    (string) (len=7) "account": (*schema.Schema)(0xc000215400)({
     Type: (schema.ValueType) TypeString,
     ConfigMode: (schema.SchemaConfigMode) 0,
     Optional: (bool) true,
     Required: (bool) false,
     DiffSuppressFunc: (schema.SchemaDiffSuppressFunc) <nil>,
     Default: (interface {}) <nil>,
     DefaultFunc: (schema.SchemaDefaultFunc) <nil>,
     Description: (string) "",
     InputDefault: (string) "",
     Computed: (bool) false,
     ForceNew: (bool) false,
     StateFunc: (schema.SchemaStateFunc) <nil>,
     Elem: (interface {}) <nil>,
     MaxItems: (int) 0,
     MinItems: (int) 0,
     Set: (schema.SchemaSetFunc) <nil>,
     ComputedWhen: ([]string) <nil>,
     ConflictsWith: ([]string) <nil>,
     ExactlyOneOf: ([]string) <nil>,
     AtLeastOneOf: ([]string) <nil>,
     RequiredWith: ([]string) <nil>,
     Deprecated: (string) "",
     ValidateFunc: (schema.SchemaValidateFunc) 0x1cf87a0,
     ValidateDiagFunc: (schema.SchemaValidateDiagFunc) <nil>,
     Sensitive: (bool) false
    }),
    (string) (len=25) "object_read_only_property": (*schema.Schema)(0xc000215540)({
     Type: (schema.ValueType) TypeString,
     ConfigMode: (schema.SchemaConfigMode) 0,
     Optional: (bool) true,
     Required: (bool) false,
     DiffSuppressFunc: (schema.SchemaDiffSuppressFunc) <nil>,
     Default: (interface {}) <nil>,
     DefaultFunc: (schema.SchemaDefaultFunc) <nil>,
     Description: (string) "",
     InputDefault: (string) "",
     Computed: (bool) true,
     ForceNew: (bool) false,
     StateFunc: (schema.SchemaStateFunc) <nil>,
     Elem: (interface {}) <nil>,
     MaxItems: (int) 0,
     MinItems: (int) 0,
     Set: (schema.SchemaSetFunc) <nil>,
     ComputedWhen: ([]string) <nil>,
     ConflictsWith: ([]string) <nil>,
     ExactlyOneOf: ([]string) <nil>,
     AtLeastOneOf: ([]string) <nil>,
     RequiredWith: ([]string) <nil>,
     Deprecated: (string) "",
     ValidateFunc: (schema.SchemaValidateFunc) 0x1cf87a0,
     ValidateDiagFunc: (schema.SchemaValidateDiagFunc) <nil>,
     Sensitive: (bool) false
    })
   },
   SchemaVersion: (int) 0,
   MigrateState: (schema.StateMigrateFunc) <nil>,
   StateUpgraders: ([]schema.StateUpgrader) <nil>,
   Create: (schema.CreateFunc) <nil>,
   Read: (schema.ReadFunc) <nil>,
   Update: (schema.UpdateFunc) <nil>,
   Delete: (schema.DeleteFunc) <nil>,
   Exists: (schema.ExistsFunc) <nil>,
   CreateContext: (schema.CreateContextFunc) <nil>,
   ReadContext: (schema.ReadContextFunc) <nil>,
   UpdateContext: (schema.UpdateContextFunc) <nil>,
   DeleteContext: (schema.DeleteContextFunc) <nil>,
   CustomizeDiff: (schema.CustomizeDiffFunc) <nil>,
   Importer: (*schema.ResourceImporter)(<nil>),
   DeprecationMessage: (string) "",
   Timeouts: (*schema.ResourceTimeout)(<nil>),
   Description: (string) ""
  }),
  MaxItems: (int) 0,
  MinItems: (int) 0,
  Set: (schema.SchemaSetFunc) <nil>,
  ComputedWhen: ([]string) <nil>,
  ConflictsWith: ([]string) <nil>,
  ExactlyOneOf: ([]string) <nil>,
  AtLeastOneOf: ([]string) <nil>,
  RequiredWith: ([]string) <nil>,
  Deprecated: (string) "",
  ValidateFunc: (schema.SchemaValidateFunc) <nil>,
  ValidateDiagFunc: (schema.SchemaValidateDiagFunc) <nil>,
  Sensitive: (bool) false
 }),
 (string) (len=21) "object_property_block": (*schema.Schema)(0xc0002157c0)({
  Type: (schema.ValueType) TypeList,
  ConfigMode: (schema.SchemaConfigMode) 0,
  Optional: (bool) true,
  Required: (bool) false,
  DiffSuppressFunc: (schema.SchemaDiffSuppressFunc) <nil>,
  Default: (interface {}) <nil>,
  DefaultFunc: (schema.SchemaDefaultFunc) <nil>,
  Description: (string) "",
  InputDefault: (string) "",
  Computed: (bool) false,
  ForceNew: (bool) false,
  StateFunc: (schema.SchemaStateFunc) <nil>,
  Elem: (*schema.Resource)(0xc00065c210)({
   Schema: (map[string]*schema.Schema) (len=2) {
    (string) (len=25) "object_read_only_property": (*schema.Schema)(0xc0000fc000)({
     Type: (schema.ValueType) TypeString,
     ConfigMode: (schema.SchemaConfigMode) 0,
     Optional: (bool) true,
     Required: (bool) false,
     DiffSuppressFunc: (schema.SchemaDiffSuppressFunc) <nil>,
     Default: (interface {}) <nil>,
     DefaultFunc: (schema.SchemaDefaultFunc) <nil>,
     Description: (string) "",
     InputDefault: (string) "",
     Computed: (bool) true,
     ForceNew: (bool) false,
     StateFunc: (schema.SchemaStateFunc) <nil>,
     Elem: (interface {}) <nil>,
     MaxItems: (int) 0,
     MinItems: (int) 0,
     Set: (schema.SchemaSetFunc) <nil>,
     ComputedWhen: ([]string) <nil>,
     ConflictsWith: ([]string) <nil>,
     ExactlyOneOf: ([]string) <nil>,
     AtLeastOneOf: ([]string) <nil>,
     RequiredWith: ([]string) <nil>,
     Deprecated: (string) "",
     ValidateFunc: (schema.SchemaValidateFunc) 0x1cf87a0,
     ValidateDiagFunc: (schema.SchemaValidateDiagFunc) <nil>,
     Sensitive: (bool) false
    }),
    (string) (len=7) "account": (*schema.Schema)(0xc0000fc3c0)({
     Type: (schema.ValueType) TypeString,
     ConfigMode: (schema.SchemaConfigMode) 0,
     Optional: (bool) true,
     Required: (bool) false,
     DiffSuppressFunc: (schema.SchemaDiffSuppressFunc) <nil>,
     Default: (interface {}) <nil>,
     DefaultFunc: (schema.SchemaDefaultFunc) <nil>,
     Description: (string) "",
     InputDefault: (string) "",
     Computed: (bool) false,
     ForceNew: (bool) false,
     StateFunc: (schema.SchemaStateFunc) <nil>,
     Elem: (interface {}) <nil>,
     MaxItems: (int) 0,
     MinItems: (int) 0,
     Set: (schema.SchemaSetFunc) <nil>,
     ComputedWhen: ([]string) <nil>,
     ConflictsWith: ([]string) <nil>,
     ExactlyOneOf: ([]string) <nil>,
     AtLeastOneOf: ([]string) <nil>,
     RequiredWith: ([]string) <nil>,
     Deprecated: (string) "",
     ValidateFunc: (schema.SchemaValidateFunc) 0x1cf87a0,
     ValidateDiagFunc: (schema.SchemaValidateDiagFunc) <nil>,
     Sensitive: (bool) false
    })
   },
   SchemaVersion: (int) 0,
   MigrateState: (schema.StateMigrateFunc) <nil>,
   StateUpgraders: ([]schema.StateUpgrader) <nil>,
   Create: (schema.CreateFunc) <nil>,
   Read: (schema.ReadFunc) <nil>,
   Update: (schema.UpdateFunc) <nil>,
   Delete: (schema.DeleteFunc) <nil>,
   Exists: (schema.ExistsFunc) <nil>,
   CreateContext: (schema.CreateContextFunc) <nil>,
   ReadContext: (schema.ReadContextFunc) <nil>,
   UpdateContext: (schema.UpdateContextFunc) <nil>,
   DeleteContext: (schema.DeleteContextFunc) <nil>,
   CustomizeDiff: (schema.CustomizeDiffFunc) <nil>,
   Importer: (*schema.ResourceImporter)(<nil>),
   DeprecationMessage: (string) "",
   Timeouts: (*schema.ResourceTimeout)(<nil>),
   Description: (string) ""
  }),
  MaxItems: (int) 1,
  MinItems: (int) 0,
  Set: (schema.SchemaSetFunc) <nil>,
  ComputedWhen: ([]string) <nil>,
  ConflictsWith: ([]string) <nil>,
  ExactlyOneOf: ([]string) <nil>,
  AtLeastOneOf: ([]string) <nil>,
  RequiredWith: ([]string) <nil>,
  Deprecated: (string) "",
  ValidateFunc: (schema.SchemaValidateFunc) <nil>,
  ValidateDiagFunc: (schema.SchemaValidateDiagFunc) <nil>,
  Sensitive: (bool) false
 })
}

Terraform Configuration Files

resource "openapi_cdn_v1" "my_cdn" {
  ...
  object_property_block {
    account = "my_account"
  }
  object_property_argument = {
    account = "my_account"
    object_read_only_property = "some computed value for object read only" // This is a known issue due to TypeMap with Elem*Resource unpredictability and a workaround users will have to do in order to fix the diff issues with objects that contain readOnly properties
  }
  ...
}

Debug Output

=== RUN   TestAccCDN_Create_and_UpdateSubResource

2020/10/17 17:22:01 [INFO] data source 'openapi_cdn_v1' successfully registered in the provider (time:72.546µs)
    gray_box_cdns_test.go:577: Step 1/2 error: Error running pre-apply refresh: 2020/10/17 17:22:01 [DEBUG] Using modified User-Agent: Terraform/0.12.29 HashiCorp-terraform-exec/0.10.0

        Error: InternalValidate

        Internal validation of the provider failed! This is always a bug
        with the provider itself, and not a user issue. Please report
        this bug:

        3 errors occurred:
            * resource openapi_cdn_v1: object_property_argument: TypeMap with Elem
        *Resource not supported,use TypeList/TypeSet with Elem *Resource or TypeMap
        with Elem *Schema
            * data source openapi_cdn_v1_instance: object_property_argument: TypeMap with
        Elem *Resource not supported,use TypeList/TypeSet with Elem *Resource or
        TypeMap with Elem *Schema
            * data source openapi_cdn_v1: object_property_argument: TypeMap with Elem
        *Resource not supported,use TypeList/TypeSet with Elem *Resource or TypeMap
        with Elem *Schema

--- FAIL: TestAccCDN_Create_and_UpdateSubResource (0.33s)
FAIL

Expected Behavior

TypeMap with Elem Resource is supported so objects that objects containing props with different types (eg: string, int, etc) or configurations (eg: mix of props where some are expected as input from the user and others are are computed aka readOnly) can be defined in the terraform schema without having to use the workaround of configuring the property as TypeList with Elem Resource and limit size of 1

P.S: I was hoping this could be supported as part of the major upgrade to SDK 2.0.0 considering it would potentially involve introducing breaking changes and seemed like a good opportunity to do it.

Actual Behavior

TypeMap with Elem *Resource not supported

Steps to Reproduce

Please list the full steps required to reproduce the issue, for example:

  1. The provider exposes a resource with a schema containing one property set up as TypeMap with Elem *Resource (example provided above)
  2. terraform init
  3. terraform apply
  4. Terraform will complain with the following output:
        Error: InternalValidate

        Internal validation of the provider failed! This is always a bug
        with the provider itself, and not a user issue. Please report
        this bug:

        3 errors occurred:
            * resource openapi_cdn_v1: object_property_argument: TypeMap with Elem
        *Resource not supported,use TypeList/TypeSet with Elem *Resource or TypeMap
        with Elem *Schema

References

dikhan commented 4 years ago

Hi Terraform team,

Any updates on this? I am not really sure how to handle this use case and would appreciate some guidance.

Thanks! Dani

paddycarver commented 3 years ago

Sorry for the delay in responding here. :(

However, as per 21217 comment it was suggested that future versions of the SDK would support complex objects natively without having to use the workaround of TypeList with 1 Elem and I'd really appreciate if you could shed some light on it.

This is accurate. terraform-plugin-go, for example, can use objects natively. This functionality hasn't made its way to the SDK just yet, but we're working on it.

I was wondering if the expectation/recommendation is still to configure complex objects using TypeList with Elem *Resource and limit size of 1?

It depends, but generally, yes.

P.S: I was hoping this could be supported as part of the major upgrade to SDK 2.0.0 considering it would potentially involve introducing breaking changes and seemed like a good opportunity to do it.

Me, too. :( Unfortunately, even though we were going to be making some breaking changes, we needed to temper the number of breaking changes we made to make the upgrade palatable and achievable for busy provider developers. While we technically could break all the existing provider code, doing so is really expensive and places a huge barrier to adoption. So we had to pick and choose carefully what we broke and what we left, even if it meant leaving some things in states we weren't totally happy with.

Sadly, supporting maps of resources is not as straightforward a request as it seems. It involves untangling a lot of code that has been around for six years, that hundreds of providers have written thousands of resources against. As we learned with Terraform 0.12, providers sometimes rely on implementation details of that code, and so it's really risky for us to change it. Adding support for Resources as map values is a really large undertaking, because it means picking through and modifying all that code, and then making sure it's not going to have some really unfortunate unintended consequences.

We have a way out of this mapped out, but we're a small team and a lot of people have direct dependencies on the code we're writing, so we're trying to be careful and it just takes time. I know that's frustrating--I'm frustrated by it, too :( -- but we're moving in the right direction. Hopefully terraform-plugin-go is an indicator of that.