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.

Sign up for GitHub

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

Jump to bottom

Suppress write-only attributes until properly supported #5215

Open
mjeffryes opened this issue Feb 15, 2025 · 5 comments
Open

Suppress write-only attributes until properly supported #5215

mjeffryes opened this issue Feb 15, 2025 · 5 comments
Assignees
@guineveresaenger
Labels
awaiting/bridge The issue cannot be resolved without action in pulumi-terraform-bridge. kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed

Comments

@mjeffryes
Copy link
Member

AWS provider is adding write-only attributes (hashicorp/terraform-provider-aws#41314)

  • These are not supposed to persist in state anywhere
  • We can't quite match the contract until [Epic] Run program during refresh and destroy· pulumi/16600 is finished:
    • We could just store the value as a secret, but users will be surprised when we use the old value in a refresh or destroy
    • We could not store anything, but then the value will be missing for refresh & destroy
  • In the short term, we should suppress these attributes (there are normal versions of these on the resources), to avoid this confusion.
  • After [Epic] Run program during refresh and destroy· pulumi/16600 is finished, it should be straightforward to support them.
@pulumi-bot pulumi-bot added the needs-triage Needs attention from the triage team label Feb 15, 2025
@guineveresaenger guineveresaenger removed the needs-triage Needs attention from the triage team label Feb 18, 2025
@guineveresaenger guineveresaenger mentioned this issue Feb 21, 2025
Open
@t0yv0 t0yv0 mentioned this issue Feb 27, 2025
Merged
@guineveresaenger guineveresaenger mentioned this issue Mar 5, 2025
Merged
@t0yv0
Copy link
Member

t0yv0 commented Mar 11, 2025 •
edited
Loading

@corymhall check if this obviates any work for the recent P1 #5288 - we can possibly remove the patches for wo_ attributes that caused the problem.

@t0yv0
Copy link
Member

t0yv0 commented Mar 11, 2025

Note from Cory: there are some validators that throw warnings that the user should use write-only attributes that might not be suppressed by the bridge yet.

guineveresaenger added a commit to pulumi/pulumi-terraform-bridge that referenced this issue Mar 11, 2025
@guineveresaenger
Suppress write-only attributes (#2933)
6cd0c72 
Bridge-level fix for pulumi/pulumi-aws#5215.
Related to patch issues in pulumi-gcp and pulumi-azure.

**NOTE:** Please advise on the shape of our TFPF schema support. I've
added WriteOnly to Attribute in my last commit but may be missing some
additional locations here.

~This pull request changes the shape of shim.Schema interface to include
a `WriteOnly` field, and adds this to all implementations.~

This pull request adds a new schema interface, `SchemaWithWriteOnly`, to
the shim package.

It then uses this information to set `Omit()` on any variable that has a
schema with a WriteOnly field.

Of note, the bridge currently does not allow for Omit() to be set on
Required fields, so a test is added that asserts the bridge errors if we
ever see a filed with both WriteOnly and Required set to true.

Since these write-only fields are in practice duplicates of existing
(non-write-only) attributes, we believe omitting them for now will allow
us to avoid future backcompat issues once we can support write-only
fields properly.

Wrinkle:

AWS adds a few extra attributes that are meant to be used in combination
with the new write-only attribute.
See here for an example:
https://github.com/hashicorp/terraform-provider-aws/pull/40952/files#diff-efb779173f4ffe3da1ab88d2773509d6aead150833362a6159bbee214edafcc4R80
There are three new fields, `has_value_wo`, `value_wo_version`, and our
actual write-only field, `value_wo`.

This fix does nothing to remove those related fields. I'm not sure it's
advisable or even possible to detect these programmatially.
One might argue that this is all right, since it's not actually possible
to use them without the omitted write-only attribute, but it may lead to
user confusion.
If we truly do not want these related (currently nonsensical) fields
being written to the schema, we can manually set Omit on them in the
provider's resources.go config.

- **Extend schemaschim to support WriteOnly**
- **Set Omit to true when TF schema is WriteOnly**
@guineveresaenger guineveresaenger added the resolution/fixed This issue was fixed label Mar 11, 2025
@guineveresaenger
Copy link
Contributor

Fixed in pulumi/pulumi-terraform-bridge#2933.

The bridge will now automatically omit attributes with WriteOnly: true, except for cases where Required is also true. In that case, our only option is a patch.

Attributes supporting the write-only-attributes will be needed to manually omitted here in the provider, and we may need to tweak validations once upstream pushes the write-only option harder than a warning/suggestion.

@pulumi-bot pulumi-bot reopened this Mar 11, 2025
@pulumi-bot
Copy link
Contributor

Cannot close issue:

  • does not have required labels: kind/

Please fix these problems and try again.

@guineveresaenger guineveresaenger added the kind/engineering Work that is not visible to an external user label Mar 11, 2025
@t0yv0 t0yv0 reopened this Mar 11, 2025
@t0yv0 t0yv0 added the awaiting/bridge The issue cannot be resolved without action in pulumi-terraform-bridge. label Mar 11, 2025
@t0yv0
Copy link
Member

t0yv0 commented Mar 11, 2025

We've got a fix but it's not in AWS yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@guineveresaenger guineveresaenger

Labels
awaiting/bridge The issue cannot be resolved without action in pulumi-terraform-bridge. kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@t0yv0 @mjeffryes @guineveresaenger @pulumi-bot