Add support for common parameters for resources and data sources

[randomswdev]

The current parameters overriding logic has two flaws:

1. The openapi specification allows to define "Common Parameters" for a path, that apply to all the operations (GET, PUT, ...). The openapi generator do not consider these parameters, but only the ones explicitly specified in an operation.
2. It is currently impossible to associate a parameter to a nested attribute, like for example the name parameter of a kubernetes resource to the metadata.name attribute.

This pull request attempts to address both these issues.

[austinvalle]

Hey there @randomswdev 👋🏻, this is awesome, thanks for opening the PR!

After a quick glance, I have a couple of small tweaks for the PR itself, but before I do a review, I'd prefer if we could split this PR into multiple PRs so the two issues noted in your description can be reviewed/discussed independently.

As you'll see below, I think the common parameters issue is pretty cut and dry, but I need some clarification on the "overriding nested attributes with parameters" issue that I think would be best discussed first in a GitHub issue before opening a PR.

Common Parameters

I'll open a GH issue for this one (#115), just for reference, however I don't think there is too much to discuss around the solution.

Your PR changes makes sense to me, the only real tweak I think we could make is supporting operations being able to override parameters with the same name/location from a common parameter, as described in the specification:

parameters - A list of parameters that are applicable for all the operations described under this path. These parameters can be overridden at the operation level, but cannot be removed there. The list MUST NOT include duplicated parameters. A unique parameter is defined by a combination of a name and location.

Might need to decide what best to do if a path parameter name is the same as a query parameter name, as that would result in a duplicate attribute.

Parameters overriding nested attributes

Before accepting a PR, I'd like to understand a little more about this issue, perhaps with an example OpenAPI spec and expected output. This would best be discussed in a GH issue if you wouldn't mind opening one.

Specifically I have some questions about the solution, which could be discussed in that issue:

• Reading through the PR it seems that in order for a parameter name to match a nested attribute, it would need to be adjusted in the OpenAPI spec with the dot-separated syntax.
  • Is this dot-syntax documented in the OpenAPI specification somewhere?
  • You mentioned a kubernetes example (would be great to include this in the GH issue) with name mapping to metadata.name. For a path parameter it might be okay to name the actual parameter metadata.name, since it's not included in the API URL, but I'd imagine a query parameter wouldn't be able to utilize this dot-syntax. Perhaps we could support an extension to the parameter object to indicate the mapping to support both types of parameters?

General contributing notes

Apologies that this repo doesn't have a CONTRIBUTING.MD that describes what we're looking for, but your PR will need:

• A changelog, we use changie, which you can figure out how we use it from one of our other repositories CONTRIBUTING.md.
• Unit tests and/or an acceptance test that cover the changes implemented
  • This repo has a set of golden testdata files that can be generated with make testdata. There are e2e tests that use actual OpenAPI specifications (including kubernetes), if you think there is an applicable example that covers your feature, feel free to add a test this way.
  • There are also more "integration tests" for the resource/datasource/provider mappers if you can't find an example in one of the testdata OpenAPI specs.
  • Then there are unit tests for the more single-purpose Go code like the config_explorer
  • e2e, integration, and unit tests all run via the standard go test command
• Any documentation updates I can help identify and submit those, as most of our documentation lives in another repo 😄

If you're not 100% sure about the changelogs or what tests to add, I can help with that, just let me know!

[randomswdev]

Hi @austinvalle I simplified the pull request, keeping only the changes required for supporting Common Parameters. I added some tests for the new methods.

What do I have to do for the changelog and the docemantation?

[randomswdev]

@austinvalle

As you'll see below, I think the common parameters issue is pretty cut and dry, but I need some clarification on the "overriding nested attributes with parameters" issue that I think would be best discussed first in a GitHub issue before opening a PR.

I just opened an issue describing the problem and plan to submit later today a pull request containing just the related changes.
The issue is: #117

[austinvalle]

Thanks for separating those two PRs and adding the tests @randomswdev!

Overall this PR is looking good, just left some feedback around naming and test separation.

• For documentation, in the DESIGN.MD file, can you add two small notes about common parameters from the path item being merged with the operation parameters?
  • For both resources and data sources
• This PR will need a changelog, here is some documentation on how to create one: https://github.com/hashicorp/terraform-plugin-framework-validators/blob/main/.github/CONTRIBUTING.md#changelog
  • I'd consider this PR an enhancement, maybe with a message of all: Added support for common parameters defined at OAS path items
  • It'll create a file that looks like this: https://github.com/hashicorp/terraform-plugin-codegen-openapi/blob/main/.changes/unreleased/BUG%20FIXES-20231220-133159.yaml
• You'll also need to add the license headers to all new files, which you can do by running make generate in the root of this repo.

[austinvalle]

Another note, the kubernetes spec was incorrectly not apart of the test suite, which I just added back in #121.

Your changes will likely update that golden file, so you'll want to run make testdata and commit the changes to your PR 👍🏻

[randomswdev]

@austinvalle I should have addressed all the review comments. Let me know if additional changes are required.

[austinvalle]

LGTM, thanks for making the changes! 🚀

[github-actions]

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.