-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider how best to handle array response mapping (like data source query endpoints) #16
Comments
I think your proposed solution here is on the right track -- returning the array of objects as a list/set of objects -- best represented in the schema as a list/set nested attribute. Naming the collection I think we could bikeshed between some "standard" attribute name, e.g. |
What do we think about resources/future provider schemas? Should they follow the same logic of:
Request body's can technically be arrays 😆 |
Sorry I feel like I'm being very dense, what does an array of request bodies actually mean in practice? Do you have an example handy? |
No worries, the actual use-case is obtuse and hopefully not common. This is a technically valid, but weird API example: {
"/pet": {
"post": {
"summary": "Add new pets to the store",
"description": "Add new pets to the store",
"operationId": "addPets",
"requestBody": {
"description": "Create new pets in the store",
"content": {
"application/json": {
"schema": {
"type": "array",
"items": {
"$ref": "#/components/schemas/Pet"
}
}
}
},
"required": true
},
"responses": {
"200": {
"description": "Successful operation",
"content": {
"application/json": {
"schema": {
"type": "array",
"items": {
"$ref": "#/components/schemas/Pet"
}
}
}
}
}
}
}
}
} That would be an array of Pet objects, but you could "technically" have an array of primitives too: {
"/pet": {
"post": {
"summary": "Add new strings to something?",
"description": "Add new strings to something?",
"operationId": "addStrings",
"requestBody": {
"description": "Create new strings",
"content": {
"application/json": {
"schema": {
"type": "array",
"items": {
"type": "string"
}
}
}
},
"required": true
},
"responses": {
"200": {
"description": "Successful operation",
"content": {
"application/json": {
"schema": {
"type": "array",
"items": {
"type": "string"
}
}
}
}
}
}
}
}
} |
Best use-case I can think of is some type of "bulk" create API? Doesn't seem like a valid use-case really |
Terraform provider design suggests that resources should be a 1:1 mapping from Terraform resource to external system "object", so bulk creation feels outside that model. 👍 https://developer.hashicorp.com/terraform/plugin/best-practices/hashicorp-provider-design-principles#resources-should-represent-a-single-api-object |
If necessary because an OAS only specifies that bulk creation method, we could consider a "collapse" configuration to drop the array handling to a single object. |
Sounds good, data source only sounds good to me 👍🏻 |
For future reference, another great "production" example of this issue can also be seen on the "/gists": {
"get": {
"summary": "List gists for the authenticated user",
"description": "Lists the authenticated user's gists or if called anonymously, this endpoint returns all public gists:",
"tags": [
"gists"
],
"operationId": "gists/list",
"externalDocs": {
"description": "API method documentation",
"url": "https://docs.github.com/rest/reference/gists#list-gists-for-the-authenticated-user"
},
"parameters": [
{
"$ref": "#/components/parameters/since"
},
{
"$ref": "#/components/parameters/per-page"
},
{
"$ref": "#/components/parameters/page"
}
],
"responses": {
"200": {
"content": {
"application/json": {
"schema": {
"type": "array",
"items": {
"$ref": "#/components/schemas/base-gist"
}
}
}
}
}
}
}
} |
This issue will be released with https://github.com/hashicorp/terraform-plugin-codegen-openapi/milestones |
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. |
Problem
A great example of this problem can be found in the petstore OAS,
GET /pet/findByStatus
Generator config:
The response schema
#/components/schemas/Pet
is not picked up because currently, the mapping of resources and datasources always assumes the top level JSON schema istype: object
. So you end up with a framework IR that doesn't have the response body mapping at all 😞IR output:
Solution
I'm not sure the best way to represent this in a TF Schema, but I would guess that we might want to end up with a list nested attribute in some way? One problem will be that the array here doesn't have a "name" as it's not a property, so maybe we could default to something like
responses
?Potential IR output
The text was updated successfully, but these errors were encountered: