Skip to content

Add support for maps based on additionalProperties field#27

Merged
austinvalle merged 16 commits into
mainfrom
av/add-map-support
Jun 27, 2023
Merged

Add support for maps based on additionalProperties field#27
austinvalle merged 16 commits into
mainfrom
av/add-map-support

Conversation

@austinvalle

@austinvalle austinvalle commented Jun 26, 2023

Copy link
Copy Markdown
Member

Context

RFC: https://docs.google.com/document/d/15PJokoO7mkMYsc2Z7DN1copykQgH5Kt7nnzb-mcNcTo/edit#heading=h.4syvri1n60bk

References:

This PR adds support for:

  • MapNestedAttribute
  • MapAttribute
  • MapType (only possible under a collection type, ListAttribute, SetAttribute, or MapAttribute)

I also added documentation to the design doc and added unit/integration tests to cover since this there aren't any use-cases in existing OAS tests.

Details

additionalProperties indicates that key names in a schema can be any string, with a value defined by the schema defined in additionalProperties.type.

Examples

This works for simple maps that have primitive element types (MapAttribute)

// Keys can be any string, value can be any string (map of strings)
{
  "type": "object",
  "additionalProperties": { 
    "type": "string" 
  }
}

This also works for more complex maps that have objects (MapNestedAttribute)

// Keys can be any string, value can be an object that matches the schema
{
  "type": "object",
  "additionalProperties": { 
    "type": "object",
    "properties": {
        // ... define object for value of map
    }
  }
}

Finally, it can also exist as a MapType in an array:

// Array with items, keys can be any string, value can be any string (map of strings)
{
  "type": "array",
  "items": {
    "type": "object"
    "additionalProperties": {
      "type": "string"
    }
  }
}

I wrote integration tests to cover this specific functionality as the existing OAS do not cover this use-case 👍🏻

Notes:

  • It's worth noting that the usage of additionalProperties will override any existing properties that are set. In JSON schema, additionalProperties is "in addition" to the defined properties, however we cannot represent that in a TF Plugin Framework schema.
  • This override will only happen if a schema is provided in additionalProperties, if additionalProperties: false is found (indicating that NO additional properties are allowed), it will treat the schema like an object.

commit dd5627e
Author: Austin Valle <austinvalle@gmail.com>
Date:   Fri Jun 23 16:01:17 2023 -0400

    match openapi spec to name

commit 4696790
Author: Austin Valle <austinvalle@gmail.com>
Date:   Fri Jun 23 15:46:11 2023 -0400

    update to latest commit

commit b41fedc
Author: Austin Valle <austinvalle@gmail.com>
Date:   Fri Jun 23 15:41:12 2023 -0400

    update design doc for sets

commit bc38b31
Author: Austin Valle <austinvalle@gmail.com>
Date:   Fri Jun 23 15:30:54 2023 -0400

    implement set and run `make testdata`

commit 6315b97
Author: Austin Valle <austinvalle@gmail.com>
Date:   Fri Jun 23 15:28:37 2023 -0400

    add resource example

commit 2aa0bce
Author: Austin Valle <austinvalle@gmail.com>
Date:   Fri Jun 23 15:27:48 2023 -0400

    update test to include resources

commit 60b134e
Author: Austin Valle <austinvalle@gmail.com>
Date:   Fri Jun 23 12:26:01 2023 -0400

    Add test before changes

commit 9a8bdd3
Author: Austin Valle <austinvalle@gmail.com>
Date:   Fri Jun 23 12:04:07 2023 -0400

    update go mod with latest `codegen-spec`
@austinvalle austinvalle requested a review from a team as a code owner June 26, 2023 16:45

@bflad bflad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable to me 🚀 We can always have our own configuration/annotations if the additionalProperties data model does not work in practice.

@bendbennett bendbennett left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎸

@github-actions

Copy link
Copy Markdown

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants