-
Notifications
You must be signed in to change notification settings - Fork 74
generate: Add support for import
configuration examples with the id
attribute
#495
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
base: main
Are you sure you want to change the base?
Conversation
import
configuration examples with the id
attribute
README.md
Outdated
| `examples/functions/<function name>/function.tf` | Function example config | | ||
| `examples/resources/<resource name>/resource.tf` | Resource example config | | ||
| `examples/resources/<resource name>/import.sh` | Resource example import command | | ||
| `examples/resources/<resource name>/import-id.tf` | Resource example import by id config | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import-id.tf
Open to other suggestions for naming on this, originally I had import.tf
but I think it's a good idea to add id
in some way because I think we'll want to add identity
as a 2nd type of import configuration example. So with this current naming, we'd introduce another at:
examples/resources/<resource name>/import-identity.tf
Other idea I was thinking of doing was import-by-id.tf
and import-by-identity.tf
, but was split between the two options 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also do import-string-id.tf
and import-resource-id.tf
. It's a bit confusing to treat id
and identity
as separate concepts when they're the same word.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I can see that 🤔, what do you think about:
import-by-string-id.tf
import-by-identity.tf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I like that wording better 👍🏾
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 9caf681 ⚡
| `.ExampleFile` | string | Path to the file with the terraform configuration example | | ||
| `.ProviderName` | string | Canonical provider name (ex. `terraform-provider-random`) | | ||
| `.ProviderShortName` | string | Short version of the rendered provider name (ex. `random`) | | ||
| Field | Type | Description | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diff is much cleaner if you hide whitespace in the GitHub UI PR settings 😄
| `.HasImportIDConfig` | bool | Is there an import terraform config file? (`import` block example with `id`) | | ||
| `.ImportIDConfigFile` | string | Path to the file with the Terraform configuration for importing the resource by `id` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming/duplication is unfortunate, but these variables are an exposed interface that we should avoid breaking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good to me overall, just left a comment about the file naming
9caf681
to
c203513
Compare
Related Issue
Closes #472
Some of the acceptance tests I updated used the scaffolding repo, which doesn't have import shell examples in the repo, so I added those here: hashicorp/terraform-provider-scaffolding-framework#306
We probably should add some import configs as well once we release this 👍🏻
Description
This PR adds support for defining an example import TF configuration file in the same location as the existing
import.sh
file:examples/resources/<resource name>/import-by-string-id.tf
I'm intentionally choosing the
id
suffix following import because identity should probably be it's own config file (import-by-identity.tf
for example). Since it has a different TF version requirement and can live next to the other import methods.Rollback Plan
Changes to Security Controls
No