Consider adding "mapping" to associated_external_type

[bendbennett]

Background

Currently, associated_external_type is structured as follows:

"associated_external_type": {
  "import": {
    "path": "github.com/aws/aws-sdk-go/service/imagebuilder"
  },
  "type": "*imagebuilder.ImageTestsConfiguration"
}

The generation of "expand" and "flatten" methods by _terraform_codegenframework will need to make assumptions about how to map from/to the model struct fields to the associated_external_type.type, and in the first instance would use a direct one-to-one mapping. For example, if a resource schema looked as follows:

{
  "resources": [
    {
      "name": "aws_imagebuilder_image",
      "schema": {
        "attributes": [
          /* ... */
        ],
        "blocks": [
          {
            "name": "image_tests_configuration",
            "single_nested": {
              "associated_external_type": {
                "import": {
                  "path": "github.com/aws/aws-sdk-go/service/imagebuilder"
                },
                "type": "*imagebuilder.ImageTestsConfiguration"
              },
              "attributes": [
                {
                  "name": "image_tests_enabled",
                  "bool": {
                    "computed_optional_required": "optional"
                  }
                },
                {
                  "name": "timeout_minutes",
                  "int64": {
                    "computed_optional_required": "computed_optional"
                  }
                }
              ]
            }
          }
          /* ... */
        ]
      }
    }
    /* ... */
  ],
  /* ... */
}

Using an assumed one-to-one mapping for associated_external_type.type would yield the following "expand" and "flatten" functions:

"expand"

func (m ImageTestsConfigurationModel) ToImageTestsConfiguration(ctx context.Context, tfObject types.Object) (*imagebuilder.ImageTestsConfiguration, diag.Diagnostics) {
  var diags diag.Diagnostics

  if tfObject.IsNull() || tfObject.IsUnknown() {
    return nil, diags
  }

  var tfModel ImageTestsConfigurationModel

  diags.Append(tfObject.As(ctx, &tfModel, basetypes.ObjectAsOptions{})...)

  if diags.HasError() {
    return nil, diags
  }

  apiObject := &imagebuilder.ImageTestsConfiguration{
    ImageTestsEnabled:  tfModel.ImageTestsEnabled.ValueBoolPointer(),
    TimeoutMinutes:     tfModel.TimeoutMinutes.ValueInt64Pointer(),
  }

  return apiObject, diags
}

"flatten"

func (m ImageTestsConfigurationModel) FromImageTestsConfiguration(ctx context.Context, apiObject *imagebuilder.ImageTestsConfiguration) (types.Object, diag.Diagnostics) {
  var diags diag.Diagnostics
  var tfModel ImageTestsConfigurationModel

  if apiObject == nil {
    return m.objectNull(ctx), diags
  }

  tfModel.ImageTestsEnabled = types.BoolPointerValue(apiObject.ImageTestsEnabled)
  tfModel.TimeoutMinutes = types.Int64PointerValue(apiObject.TimeoutMinutes)

  return m.objectValueFrom(ctx)
}

There are assumptions made in this mapping, including but not limited to:

• The name of the field in the "apiObject" (*imagebuilder.ImageTestsConfiguration) is identical to the name of the field in the model.
• The type of the field in the "apiObject" (imagebuilder.ImageTestsConfiguration) is a pointer to the Go type that correlates with the plugin Framework type used by the model (e.g., types.Bool <=> `bool`).

Proposal

To provide additional flexibility in the mapping to/from models to API objects defined through associated_external_type we should consider the introduction of a "mapping" field, or some other representation, that can be used to shape how the "expand" and "flatten" functions are constructed within _terraform_codegenframework. For instance, we could allow the defining of request and response fields for each associated_external_type:

"associated_external_type": {
  "imports": [
    {
      "path": "github.com/aws/aws-sdk-go/service/imagebuilder"
    }
  ],
  "type": "*imagebuilder.ImageTestsConfiguration",
  "request": {
    "timeout_minutes": {
      "name": "MinutesTimeout",
    },
  "response": {
    "MinutesTimeoutReturnedVal": {
      "name": "timeout_minutes",
    }
  }
},

This contrived example would yield the following "expand" and "flatten" functions:

"expand"

func (m ImageTestsConfigurationModel) ToImageTestsConfiguration(ctx context.Context, tfObject types.Object) (*imagebuilder.ImageTestsConfiguration, diag.Diagnostics) {
  var diags diag.Diagnostics

  if tfObject.IsNull() || tfObject.IsUnknown() {
    return nil, diags
  }

  var tfModel ImageTestsConfigurationModel

  diags.Append(tfObject.As(ctx, &tfModel, basetypes.ObjectAsOptions{})...)

  if diags.HasError() {
    return nil, diags
  }

  apiObject := &imagebuilder.ImageTestsConfiguration{
    ImageTestsEnabled:  tfModel.ImageTestsEnabled.ValueBoolPointer(),
    MinutesTimeout:     tfModel.TimeoutMinutes.ValueInt64Pointer(),
  }

  return apiObject, diags
}

"flatten"

func (m ImageTestsConfigurationModel) FromImageTestsConfiguration(ctx context.Context, apiObject *imagebuilder.ImageTestsConfiguration) (types.Object, diag.Diagnostics) {
  var diags diag.Diagnostics
  var tfModel ImageTestsConfigurationModel

  if apiObject == nil {
    return m.objectNull(ctx), diags
  }

  tfModel.ImageTestsEnabled = types.BoolPointerValue(apiObject.ImageTestsEnabled)
  tfModel.TimeoutMinutes = types.Int64PointerValue(apiObject.MinutesTimeoutReturnedVal)

  return m.objectValueFrom(ctx)
}

[bflad]

I'm a little confused about what is being proposed here 😅

  "request": {
    "timeout_minutes": {
      "name": "MinutesTimeout",
    },
  "response": {
    "MinutesTimeoutReturnedVal": {
      "name": "timeout_minutes",
    }
  }

Seems to suggest that there might be two Go type definitions for the same type with different fields. I would have expected it to be multiple types involved, so it would require specifying the import and second type, e.g.

package imagebuilder

type ImageTestsConfiguration struct {
  MinutesTimeout // ...
}

type SomethingElse struct {
  MinutesTimeoutReturnedVal // ...
}

I was under the presumption that at some point, if we needed support for multiple associated external types, we would support it as an array of associated external types each with its own attribute name mapping. In the above:

"associated_external_types": [
{
  "imports": [
    {
      "path": "github.com/aws/aws-sdk-go/service/imagebuilder"
    }
  ],
  "type": "*imagebuilder.ImageTestsConfiguration",
  "mapping": {
    "timeout_minutes": {
      "name": "MinutesTimeout",
    }
  }
},
{
  "imports": [
    {
      "path": "github.com/aws/aws-sdk-go/service/imagebuilder"
    }
  ],
  "type": "*imagebuilder.SomethingElse",
  "mapping": {
    "timeout_minutes": {
      "name": "MinutesTimeoutReturnedVal",
    }
  }
}
]

Then, a separate challenge would be somehow describing "intermediate" objects that an API and its SDK may introduce, such as an API response that "wraps" an object for the whole resource, for example: https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/imagebuilder#GetInfrastructureConfigurationOutput

type GetInfrastructureConfigurationOutput struct { // <- API response type

    // The infrastructure configuration object.
    InfrastructureConfiguration *types.InfrastructureConfiguration // <- Object for resource model

    // The request ID that uniquely identifies this request.
    RequestId *string // <- Not needed for Terraform state

    // Metadata pertaining to the operation's result.
    ResultMetadata middleware.Metadata // <- Not needed for Terraform state
    // contains filtered or unexported fields
}

However, this would be a problem for solving later. Am I misunderstanding the proposal?

[bendbennett]

I'm a little confused about what is being proposed here

Think I was the one who was a little confused 😆

I think the approach you proposed of having an array of associated_external_types sounds good. I think we'll need to be able to identify input/output, so perhaps we'll need a map that identifies request and response:

  "associated_external_types": {
    "request": {
      "import": {
        "alias": "...",
        "path": "github.com/aws/aws-sdk-go/service/imagebuilder"
      },
      "type": "*imagebuilder.ImageTestsConfiguration",
      "mapping": {
        "timeout_minutes": {
          "name": "MinutesTimeout"
        }
      }
    },
    "response": {
      "import": {
        "alias": "...",
        "path": "github.com/aws/aws-sdk-go/service/imagebuilder"
      },
      "type": "*imagebuilder.SomethingElse",
      "mapping": {
        "timeout_minutes": {
          "name": "MinutesTimeoutReturnedVal"
        }
      }
    }
  }

Alternatively we could have an array with an additional field, perhaps request_response as an optional enum field, maybe something like:

  "associated_external_types": [
    {
      "request_response": "request",
      "import": {
        "path": "github.com/aws/aws-sdk-go/service/imagebuilder"
      },
      "type": "*imagebuilder.ImageTestsConfiguration",
      "mapping": {
        "timeout_minutes": {
          "name": "MinutesTimeout"
        }
      }
    },
    {
      "request_response": "response",
      "import": {
        "path": "github.com/aws/aws-sdk-go/service/imagebuilder"
      },
      "type": "*imagebuilder.SomethingElse",
      "mapping": {
        "timeout_minutes": {
          "name": "MinutesTimeoutReturnedVal"
        }
      }
    }
  ]

[bflad]

My 🌶️ take is that the associated_external_types specification should not be concerned with API operations at all, only the the linkage of API SDK type to Terraform Schema type. As we get closer to supporting API operations, I think that configuration/specification would handle what an API operation request/response needs in terms of data handling and linking to the schema. There are plenty of cases where an API create operation request might not use "wrapping", but the create/update operation response might. I've also seen APIs where the update operation request can also have "wrapping" of sorts (sorry don't have an example off top of head handy). I would encourage shying away from a shared request/response delineation, because ultimately it might be different per API operation.