Skip to content
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

all: add support for ephemeral resources #415

Merged
merged 6 commits into from
Nov 1, 2024
Merged

Conversation

SBGoods
Copy link
Contributor

@SBGoods SBGoods commented Oct 31, 2024

Adds support for ephemeral resource documentation:

  • generate: adds documentation generation support for the templates/ephemeral-resources directory and templates/ephemeral-resources.md.tmpl file
  • migrate: adds migration support for website/docs/ephemeral-resources and docs/ephemeral-resources subdirectories
  • validate: adds ephemeral-resources to the list of valid directory names

@SBGoods SBGoods requested a review from a team as a code owner October 31, 2024 14:08
Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

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

LGTM, I just had one consideration about the text archive tests.

Ran your changes on the random provider as well and it looks good 👍🏻
hashicorp/terraform-provider-random@main...av/ephemeral-password

README.md Outdated Show resolved Hide resolved
Comment on lines +55 to +57
creating example file "$WORK/examples/ephemeral-resources/ephemeral_static/example_1.tf"
creating example file "$WORK/examples/ephemeral-resources/ephemeral_static/example_2.tf"
creating import file "$WORK/examples/ephemeral-resources/ephemeral_static/import_1.sh"
Copy link
Member

Choose a reason for hiding this comment

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

Not specific to this test, but just a note in general about tests. I see a lot of the internals of resources are already shared with data sources, which I think is fine.

It's a little difficult to see if the expected output below is meant for an ephemeral resource because all of the templates used seem to be geared towards a managed resource (for example, this test has an import.sh), just running through the ephemeral resource code path. For this example, it's not immediately clear how to recreate this test archive, although there isn't an existing ephemeral resource to test it on yet.

We could wait until there is, or for some of the generate ones, just take an upcoming ephemeral resource like random_password as the example (rather then null_ephemeral_resource, which we don't have planned to exist ATM).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we are talking about tests serving as an example of how to use the tool, then I get what you mean. In terms of verifying the functionality, the tests are easy to write because it verifies that the correct files are being created and that all of the template functions are working. But because terraform-plugin-docs doesn't care about whether the schema is "valid" or not and to a certain extent what "types" are, the tests end up being very arbitrary.

We can consider always having at least one real provider test, like the null provider success test, but we don't have a utility provider that implements all schema types now, so we run into the problem of having either an arbitrary provider or using multiple utility providers to cover all types (time and random for example).

Co-authored-by: Austin Valle <[email protected]>
@SBGoods SBGoods merged commit 97bd6c7 into main Nov 1, 2024
6 checks passed
@SBGoods SBGoods deleted the SBGoods/ephemeral-resources branch November 1, 2024 16:42
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 2, 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.

2 participants