hashicorp / terraform-plugin-codegen-openapi

OpenAPI to Terraform Provider Code Generation Specification
Mozilla Public License 2.0
49 stars 9 forks source link

Ensure that `description` is populated if at least one description exists between merged operations #48

Closed austinvalle closed 1 year ago

austinvalle commented 1 year ago

This could be a can of worms, as the merging functionality hasn't been refactored yet (maybe should be, as a part of this)

Background

Given the following OAS + config, in the context of a data source:

    "paths": {
        "/pet/{petId}": {
            "get": {
                "tags": [
                    "pet"
                ],
                "summary": "Find pet by ID",
                "description": "Returns a single pet",
                "operationId": "getPetById",
                "parameters": [
                    {
                        "name": "petId",
                        "in": "path",
                        "description": "ID of pet to return",
                        "required": true,
                        "schema": {
                            "type": "integer",
                            "format": "int64"
                        }
                    }
                ],
...
    "components": {
        "schemas": {
            "Pet": {
                "properties": {
                    "id": {
                        "type": "integer",
                        "format": "int64",
                        "example": 10
                    },
data_sources:
  pet:
    read:
      path: /pet/{petId}
      method: GET
    merge_options:
      param_matches:
        petId: id

You'll generate the following IR, with a description:

{
    "name": "id",
    "int64": {
        "computed_optional_required": "required",
        "description": "ID of pet to return"
    }
},

However, in the context of a resource, with the same OAS, you don't get a description in the resulting IR:

resources:
  pet:
    create:
      path: /pet
      method: POST
    read:
      path: /pet/{petId}
      method: GET
    update:
      path: /pet
      method: PUT
    delete:
      path: /pet/{petId}
      method: DELETE
    merge_options:
      param_matches:
        petId: id
{
    "name": "id",
    "int64": {
        "computed_optional_required": "computed_optional"
    }
},

Proposal

This is occurring because during the merging process (currently a very simple merge of n schemas across request body/response body, etc.), there is no functionality to preserve descriptions across multiple merges.

We should refactor and update the merging process to have the following behavior after merge:

bflad commented 1 year ago

Proposal sounds good to me, nice write up. 👍 If it makes any potential difference at all, one additional thing to consider is that a developer may want to bring their own attribute description in the configuration at some point, which should always be preserved/highest precedence.

austinvalle commented 1 year ago

Good point, I think most of this suggests we should refactor the merge functions so we can eventually consider configuration or other sources 😵

github-actions[bot] commented 3 months ago

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