hashicorp / terraform-plugin-codegen-openapi

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

Better reflect computability relationship between related operations #11

Closed austinvalle closed 1 year ago

austinvalle commented 1 year ago

Working through the demo and just logging any problems I run into. This issue is a discrepancy between the RFC and the current implementation of the OpenAPI codegen.

Current Behavior

Today, the OpenAPI code generator combines multiple operations (POST request, GET response, GET parameters, etc.) JSON schemas to create a single TF schema. Currently, the code generator respects every operations JSON schema equally when it comes to determining the computability of an attribute (Computed Optional vs. Required).

For example, if a response body for a GET operation marks a property as Required in the OAS, and it does not exist in the POST request body, then the resulting TF schema will mark it as Required. This signals that the user needs to provide the value from configuration and is not a correct interpretation of the OAS.

OAS Example

This particularly gets out of hand with OAS like GitHub, where almost the entire response body is marked as required, here is the Repository GET response body for GitHub:

{
// GET Repository -  /repos/{owner}/{repo}
"required": [
          "archive_url",
          "assignees_url",
          "blobs_url",
          "branches_url",
          "collaborators_url",
          "comments_url",
          "commits_url",
          "compare_url",
          "contents_url",
          "contributors_url",
          "deployments_url",
          "description",
          "downloads_url",
          "events_url",
          "fork",
          "forks_url",
          "full_name",
          "git_commits_url",
          "git_refs_url",
          "git_tags_url",
          "hooks_url",
          "html_url",
          "id",
          "node_id",
          "issue_comment_url",
          "issue_events_url",
          "issues_url",
          "keys_url",
          "labels_url",
          "languages_url",
          "merges_url",
          "milestones_url",
          "name",
          "notifications_url",
          "owner",
          "private",
          "pulls_url",
          "releases_url",
          "stargazers_url",
          "statuses_url",
          "subscribers_url",
          "subscription_url",
          "tags_url",
          "teams_url",
          "trees_url",
          "url",
          "clone_url",
          "default_branch",
          "forks",
          "forks_count",
          "git_url",
          "has_downloads",
          "has_issues",
          "has_projects",
          "has_wiki",
          "has_pages",
          "homepage",
          "language",
          "archived",
          "disabled",
          "mirror_url",
          "open_issues",
          "open_issues_count",
          "license",
          "pushed_at",
          "size",
          "ssh_url",
          "stargazers_count",
          "svn_url",
          "watchers",
          "watchers_count",
          "created_at",
          "updated_at"
        ]
}

Expected behavior

The RFC has a more accurate assumption made in it about computability that we should follow:

Resources

... any properties that do not exist from the initial requestBody mapping will be marked as Computed Optional on the resource schema.

Data Sources

... any properties that do not exist from the initial parameters mapping will default to Computed Optional on the data source schema..

Since OpenAPI spec's commonly mark response body's with the required property (😞), we should only check the required property on:

All other attributes should ignore the required property and default to Computed Optional

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.