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

Add plan modification support to attr.Type #161

Closed
paddycarver opened this issue Sep 15, 2021 · 3 comments
Closed

Add plan modification support to attr.Type #161

paddycarver opened this issue Sep 15, 2021 · 3 comments
Labels
enhancement New feature or request types Issues and pull requests about our types abstraction and implementations.
Milestone

Comments

@paddycarver
Copy link
Contributor

Module version

main@b0e3081ddbdab74c40ed0def9b8c23816a93d8fe

Use-cases

Several types have semantically equivalent representations that are not byte-for-byte equivalent like Terraform requires. A JSON type comes to mind, in which

{
  "key": "value"
}

and

{"key": "value"}

should be considered equal, but Terraform treats as distinct values. By allowing plan modification strategies to be associated with attr.Types, we can make sure this behavior is applied consistently everywhere the type is used.

Attempted Solutions

We can work around this by adding an AttributePlanModifier to each schema.Attribute, but that requires consistency and extra work for provider developers that can be avoided.

Proposal

Similar to attr.TypeWithValidate we create an attr.TypeWithModifyPlan that allows defining plan modification for the type. This plan modification runs before attribute plan modification and before whole resource plan modification.

@paddycarver paddycarver added the enhancement New feature or request label Sep 15, 2021
@paddycarver paddycarver self-assigned this Sep 15, 2021
@paddycarver paddycarver added the types Issues and pull requests about our types abstraction and implementations. label Sep 21, 2021
@bflad bflad added this to the v1.0.0 milestone Mar 16, 2022
bflad added a commit that referenced this issue Jun 22, 2022
Reference: #81
Reference: #161
Reference: #215
Reference: #365

This introduces a native abstraction over terraform-plugin-go's `tftypes.AttributePath`, allowing the framework to own the implementation details and extend the functionality further. Provider developers will be closer to removing a direct dependency on terraform-plugin-go. This is a major breaking change, however it is necessary before 1.0 to prevent compatibility issues in the future.

This functionality is only intended to replace `tftypes.AttributePath` usage and not mess with the `tfsdk.Config`, `tfsdk.Plan`, and `tfsdk.State` type `tftypes.Value` data storage fields or their underlying data manipulation logic. This does leave the framework in an awkward half-state until those are further refactored (likely towards native `attr.Value`), but is done to try and reduce the review complexity of this initial migration. Additional followup changes will introduce the concept of path expressions, which will allow provider developers to match against multiple paths or reference parent paths in schema-based plan modifier and validation functionality.

To prevent import cycles between `attr` and `diag` packages due changing the `attr.TypeWithValidate` type `Validate` method signature from `Validate(context.Context, tftypes.Value, *tftypes.AttributePath) diag.Diagnostics` to `Validate(context.Context, tftypes.Value, path.Path) diag.Diagnostics`, the `TypeWithValidation` interface is moved a new `xattr` package underneath `attr` with Go and website documentation to direct provider developers to the other package. This will also solve a prior issue with trying to implement `TypeWithModifyPlan` support.

Naming and location of anything in this initial implementation can be adjusted as necessary.

Provider developers can migrate to the new path handling by replacing:

```go
tftypes.NewAttributePath().WithAttributeName("example")
```

With the equivalent:

```go
path.RootPath("example")
```

Then using the `(Path).At*` methods to extend the path definition instead of `(*tftypes.AttributePath).With*` methods:

| Current                  | New             |
| ------------------------ | --------------- |
| `WithAttributeName()`    | `AtName()`      |
| `WithElementKeyInt()`    | `AtListIndex()` |
| `WithElementKeyString()` | `AtMapKey()`    |
| `WithElementKeyValue()`  | `AtSetValue()`  |
bflad added a commit that referenced this issue Jun 22, 2022
#390)

* path: Introduce package with initial tftypes.AttributePath abstraction

Reference: #81
Reference: #161
Reference: #172
Reference: #365

This introduces a native abstraction over terraform-plugin-go's `tftypes.AttributePath`, allowing the framework to own the implementation details and extend the functionality further. Provider developers will be closer to removing a direct dependency on terraform-plugin-go. This is a major breaking change, however it is necessary before 1.0 to prevent compatibility issues in the future.

This functionality is only intended to replace `tftypes.AttributePath` usage and not mess with the `tfsdk.Config`, `tfsdk.Plan`, and `tfsdk.State` type `tftypes.Value` data storage fields or their underlying data manipulation logic. This does leave the framework in an awkward half-state until those are further refactored (likely towards native `attr.Value`), but is done to try and reduce the review complexity of this initial migration. Additional followup changes will introduce the concept of path expressions, which will allow provider developers to match against multiple paths or reference parent paths in schema-based plan modifier and validation functionality.

To prevent import cycles between `attr` and `diag` packages due changing the `attr.TypeWithValidate` type `Validate` method signature from `Validate(context.Context, tftypes.Value, *tftypes.AttributePath) diag.Diagnostics` to `Validate(context.Context, tftypes.Value, path.Path) diag.Diagnostics`, the `TypeWithValidation` interface is moved a new `xattr` package underneath `attr` with Go and website documentation to direct provider developers to the other package. This will also solve a prior issue with trying to implement `TypeWithModifyPlan` support.

Naming and location of anything in this initial implementation can be adjusted as necessary.

Provider developers can migrate to the new path handling by replacing:

```go
tftypes.NewAttributePath().WithAttributeName("example")
```

With the equivalent:

```go
path.Root("example")
```

Then using the `(Path).At*` methods to extend the path definition instead of `(*tftypes.AttributePath).With*` methods:

| Current                  | New             |
| ------------------------ | --------------- |
| `WithAttributeName()`    | `AtName()`      |
| `WithElementKeyInt()`    | `AtListIndex()` |
| `WithElementKeyString()` | `AtMapKey()`    |
| `WithElementKeyValue()`  | `AtSetValue()`  |
@bflad bflad modified the milestones: v1.0.0, v1.1.0 Nov 9, 2022
@bflad
Copy link
Member

bflad commented Nov 30, 2022

(Old) example implementation for reference: #162

@bflad bflad modified the milestones: v1.1.0, v1.3.0 Jan 13, 2023
@bflad
Copy link
Member

bflad commented Jun 6, 2023

This is covered by #737 👍

@bflad bflad closed this as completed Jun 6, 2023
@github-actions
Copy link

github-actions bot commented Jul 7, 2023

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.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request types Issues and pull requests about our types abstraction and implementations.
Projects
None yet
Development

No branches or pull requests

2 participants