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

Resource Import Documentation Handling #160

Open
bflad opened this issue Sep 14, 2021 · 1 comment
Open

Resource Import Documentation Handling #160

bflad opened this issue Sep 14, 2021 · 1 comment
Labels
non-protocol-info Issues and PRs about surfacing information that Terraform doesn't need, but other clients do.

Comments

@bflad
Copy link
Contributor

bflad commented Sep 14, 2021

Module version

v0.4.0 (not yet released)

Use-cases

Provider developers will want to document whether a resource supports import and what type of import identifier(s) are supported. Tooling such as terraform-plugin-docs (or similar) should be able to determine the implementation and automatically generate import documentation.

For example, most simple resources follow this simple type of documentation pattern for "passthrough" import:

Some resources may need to document more complicated cases, such as "complex" identifiers that get split into multiple attributes:

The most advanced cases require documenting multiple example import identifiers (sometimes to make the process easier for practitioners, other times to workaround incomplete Read APIs):

Attempted Solutions

Version 0.4.0 of the framework supports resource import via a required ImportState method on the Resource type. This makes it difficult for tooling to determine if import is supported or not as it is reliant on specific implementation details, which may or may not be consistently implemented (such as the recommended ResourceImportStateNotImplemented() helper).

There is no current methodology for tooling to know import examples, except by describing it manually in resource descriptions.

Proposal

Consider migrating the ImportState method onto its own interface type, e.g.

type ResourceWithImportState interface {
  Resource

  ImportState(context.Context, ImportResourceStateRequest, *ImportStateResponse)
}

This will effectively make import support optional, against the design goals described in https://github.com/hashicorp/terraform-plugin-framework/blob/main/docs/design/import.md, however this means the framework can fully own the non-existent support implementation and downstream tooling will be able to easily determine if the implementation exists.

To handle documentation for all cases, an additional declarative documentation method could be added to that interface as well, e.g.

type ResourceWithImportState interface {
  Resource
  ImportState(context.Context, ImportResourceStateRequest, *ImportStateResponse)

  ImportExamples() []ImportExample
}

type ImportExample struct {
  // optional
  Description string

  Identifier string
}

Potentially with helpers to make this easier for provider developers. Theoretically then, tooling could loop through those examples to fully generate import documentation for each import example such as:

{DESCRIPTION}

```console
$ terraform import {RESOURCE_TYPE}.example {IDENTIFIER}
```

References

@bflad bflad added the enhancement New feature or request label Sep 14, 2021
@bflad
Copy link
Contributor Author

bflad commented Sep 14, 2021

@paddycarver paddycarver added non-protocol-info Issues and PRs about surfacing information that Terraform doesn't need, but other clients do. and removed enhancement New feature or request labels Sep 21, 2021
@bflad bflad added this to the v1.0.0 milestone Apr 21, 2022
bflad added a commit that referenced this issue Apr 21, 2022
…tState interface

Reference: #160

Due to provider developer feedback, it has been suggested to make the current `Resource` interface `ImportState` method optional. This change accomplishes that by moving the existing method signature to a new `ResourceWithImportState` interface.

This also deprecates the `ResourceImportStateNotImplemented()` function. Providers can now signal that a resource does not support import by omitting the `ImportState` method and the framework will automatically return an error diagnostic. From a quick GitHub search, it appeared there was only one usage of a custom error message outside the framework testing. However, it was only being used to include the resource type in the message and was of no real utility over the generic messaging. A code comment is left with a suggested implementation, should there be a feature request for customized messaging.
bflad added a commit that referenced this issue Apr 25, 2022
…tState interface (#297)

Reference: #160

Due to provider developer feedback, it has been suggested to make the current `Resource` interface `ImportState` method optional. This change accomplishes that by moving the existing method signature to a new `ResourceWithImportState` interface.

This also deprecates the `ResourceImportStateNotImplemented()` function. Providers can now signal that a resource does not support import by omitting the `ImportState` method and the framework will automatically return an error diagnostic. From a quick GitHub search, it appeared there was only one usage of a custom error message outside the framework testing. However, it was only being used to include the resource type in the message and was of no real utility over the generic messaging. A code comment is left with a suggested implementation, should there be a feature request for customized messaging.
@vravind1 vravind1 removed this from the v1.0.0 milestone Aug 29, 2022
jamonation pushed a commit to jamonation/terraform-plugin-images-readme that referenced this issue Sep 18, 2023
hashicorp#160)

Bumps [github.com/hashicorp/terraform-plugin-testing](https://github.com/hashicorp/terraform-plugin-testing) from 1.3.0 to 1.4.0.
- [Release notes](https://github.com/hashicorp/terraform-plugin-testing/releases)
- [Changelog](https://github.com/hashicorp/terraform-plugin-testing/blob/main/CHANGELOG.md)
- [Commits](hashicorp/terraform-plugin-testing@v1.3.0...v1.4.0)

---
updated-dependencies:
- dependency-name: github.com/hashicorp/terraform-plugin-testing
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
non-protocol-info Issues and PRs about surfacing information that Terraform doesn't need, but other clients do.
Projects
None yet
Development

No branches or pull requests

3 participants