-
Notifications
You must be signed in to change notification settings - Fork 159
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
unexpected behavior of defaultTags #1655
Comments
The terraform provider behaves as expected:
|
Thanks for reporting this. I tried to see if this happened with explicit providers (i.e. declaring a provider and using that) and it does as well. Only the provider is updated, but nothing flows to the underlying resources. I suspect this is related to being able to actually DiffConfig/CheckConfig (see pulumi/pulumi-terraform-bridge#244) |
@leezen any updates on this by any chance :)? |
Anyone know of a workaround for this? I already have a stack with hundreds of resources, and I want to add a common tag to all of them. I was hoping I could use |
Any update on this? Or workaround? Almost a year later this still seems to be an issue, defaultTags don't seem to be appearing in any diff at all, and it also doesn't trigger the update of resources. @filip-zyzniewski your alternative sounds promising, but it includes maintaining a long list of resources which are taggable (https://github.com/joeduffy/aws-tags-example/blob/master/autotag-py/taggable.py). Or is there any source of those resources? (eg: somewhere in the pulumi sdk?) |
The way we are approaching this now is: import pulumi
import pulumi.resource as res
import typeguard
def is_taggable(resource):
internal_init = getattr(type(resource), "_internal_init", None)
if not internal_init:
return False
tags_type = internal_init.__annotations__.get("tags", None)
if not tags_type:
return False
try:
typeguard.check_type("tags", {"": ""}, tags_type)
except TypeError:
return False
return True
def register_auto_tags(**auto_tags):
def add_tags(
args: res.ResourceTransformationArgs,
) -> res.ResourceTransformationResult:
props = args.props
if is_taggable(args.resource):
props = dict(props)
props["tags"] = auto_tags | (props.get("tags") or {})
return res.ResourceTransformationResult(props=props, opts=args.opts)
pulumi.runtime.register_stack_transformation(add_tags) |
Also running into this. A fix would be much appreciated. |
This behaviour makes defaultTags effectively useless. Is there a clear root cause of the issue? If the Terraform provider performs as-expected then this is a Pulumi-specific issue, correct? If anyone internal to Pulumi could point the community in the right direction here, I'm sure there might be someone (possibly me) who would be willing to help with pushing a fix. |
This is intended to enable closing: - pulumi/pulumi-aws#1655 We add a new flag on fields, `ComputedInput`, which indicates that the value will only appear as a result of a tf diff computation or planning, and not in `news`. This computed _input_ would ordinarily be computed during `Check()` in our lifecycle model. For reasons unclear to me, we don't include computed properties in detailed diffs, and in stepping through `makeDetailedDiff`, we found that the visitor never saw properties such as `tags_all.foo` when that property was new and computed. With this change, we can mark the `tags_all` field as a `ComputedInput`, and trigger updates on default tags changing: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ```
This is intended to enable closing: - pulumi/pulumi-aws#1655 We add a new flag on fields, `ComputedInput`, which indicates that the value will only appear as a result of a tf diff computation or planning, and not in `news`. This computed _input_ would ordinarily be computed during `Check()` in our lifecycle model. For reasons unclear to me, we don't include computed properties in detailed diffs, and in stepping through `makeDetailedDiff`, we found that the visitor never saw properties such as `tags_all.foo` when that property was new and computed. With this change, we can mark the `tags_all` field as a `ComputedInput`, and trigger updates on default tags changing: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ```
This is intended to enable closing: - pulumi/pulumi-aws#1655 We add a new flag on fields, `ComputedInput`, which indicates that the value will only appear as a result of a tf diff computation or planning, and not in `news`. This computed _input_ would ordinarily be computed during `Check()` in our lifecycle model. For reasons unclear to me, we don't include computed properties in detailed diffs, and in stepping through `makeDetailedDiff`, we found that the visitor never saw properties such as `tags_all.foo` when that property was new and computed. With this change, we can mark the `tags_all` field as a `ComputedInput`, and trigger updates on default tags changing: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ```
The `defaultTags` input property on providers now triggers an update on downstream resources. Because of the nature of the fix in the Pulumi TF Bridge, and to limit the scope of the change, we mark only the `tags_all` field from TF schema as a `ComputedInput`, triggering new diff behavior within the bridge. As the `tags_all` field is otherwise hidden as a "computed" value, we omit detailed diff information. This is a tradeoff to avoid a more invasive change with more risk. Fixes #1655. Example output: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ``` Verified when adding, updating, deleting, and removing entirely the `defaultTags` property.
The `defaultTags` input property on providers now triggers an update on downstream resources. Because of the nature of the fix in the Pulumi TF Bridge, and to limit the scope of the change, we mark only the `tags_all` field from TF schema as a `ComputedInput`, triggering new diff behavior within the bridge. As the `tags_all` field is otherwise hidden as a "computed" value, we omit detailed diff information. This is a tradeoff to avoid a more invasive change with more risk. Fixes #1655. Example output: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ``` Verified when adding, updating, deleting, and removing entirely the `defaultTags` property.
This is intended to enable closing: - pulumi/pulumi-aws#1655 We add a new flag on fields, `ComputedInput`, which indicates that the value will only appear as a result of a tf diff computation or planning, and not in `news`. This computed _input_ would ordinarily be computed during `Check()` in our lifecycle model. For reasons unclear to me, we don't include computed properties in detailed diffs, and in stepping through `makeDetailedDiff`, we found that the visitor never saw properties such as `tags_all.foo` when that property was new and computed. With this change, we can mark the `tags_all` field as a `ComputedInput`, and trigger updates on default tags changing: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ```
The `defaultTags` input property on providers now triggers an update on downstream resources. Because of the nature of the fix in the Pulumi TF Bridge, and to limit the scope of the change, we mark only the `tags_all` field from TF schema as a `ComputedInput`, triggering new diff behavior within the bridge. As the `tags_all` field is otherwise hidden as a "computed" value, we omit detailed diff information. This is a tradeoff to avoid a more invasive change with more risk. Fixes #1655. Example output: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ``` Verified when adding, updating, deleting, and removing entirely the `defaultTags` property.
The `defaultTags` input property on providers now triggers an update on downstream resources. Because of the nature of the fix in the Pulumi TF Bridge, and to limit the scope of the change, we mark only the `tags_all` field from TF schema as a `ComputedInput`, triggering new diff behavior within the bridge. As the `tags_all` field is otherwise hidden as a "computed" value, we omit detailed diff information. This is a tradeoff to avoid a more invasive change with more risk. Fixes #1655. Example output: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ``` Verified when adding, updating, deleting, and removing entirely the `defaultTags` property.
The `defaultTags` input property on providers now triggers an update on downstream resources. Because of the nature of the fix in the Pulumi TF Bridge, and to limit the scope of the change, we mark only the `tags_all` field from TF schema as a `ComputedInput`, triggering new diff behavior within the bridge. As the `tags_all` field is otherwise hidden as a "computed" value, we omit detailed diff information. This is a tradeoff to avoid a more invasive change with more risk. Fixes #1655. Example output: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ``` Verified when adding, updating, deleting, and removing entirely the `defaultTags` property.
The `defaultTags` input property on providers now triggers an update on downstream resources. Because of the nature of the fix in the Pulumi TF Bridge, and to limit the scope of the change, we mark only the `tags_all` field from TF schema as a `ComputedInput`, triggering new diff behavior within the bridge. As the `tags_all` field is otherwise hidden as a "computed" value, we omit detailed diff information. This is a tradeoff to avoid a more invasive change with more risk. Fixes #1655. Example output: ``` pulumi up --skip-preview Updating (test-aws-1655) View in Browser (Ctrl+O): https://app.pulumi.com/pulumi/simple-yaml/test-aws-1655/updates/9 Type Name Status Info pulumi:pulumi:Stack simple-yaml-test-aws-1655 ~ ├─ pulumi:providers:aws aws-provider updated (0.13s) [diff: ~defaultTags] ~ ├─ aws:s3:Bucket my-bucket updated (0.87s) ~ └─ aws:s3:BucketObject index.html updated (0.24s) ``` Verified when adding, updating, deleting, and removing entirely the `defaultTags` property.
I'm pleased to say that the next release of the AWS provider v5 will contain a fix for this issue. Thanks all for your patience and understanding. |
Thank you for jumping on this, I was one of the enterprise customers who called out how bad this issue was impacting my organization. |
@AaronFriel, I updated Is To clarify, I have resources created with both |
@mikhailshilkov can you answer the question above? |
@AaronFriel, @mikhailshilkov - should I open a new issue on this under the |
Fixes #2774 We detected an issue in pulumi-eks test suite against the latest v6.0.3 release. This theoretically could affects users that set resource and or provider tags to unknown Outputs, although I tried and failed to reproduce outside of the pulumi-eks test. The fix for #1655 included reimplementing tag merging (computing effective tags from the set of provider-level tags and resource-level tags) on Pulumi side as a callback to the Check Pulumi gRPC method. This works against resource.PropertyValue model and did not fully account for receiving a Computed when tags are unknown, resulting in a panic in the eks test. The fix returns unknowns if any of the tag inputs are unknown. It also puts asserts in for absence of secrets, as the provider does not expect secret values to be passed to Check and instead handled by Pulumi CLI.
Hello!
Issue details
The AWS provider ignores changes to defaultTags after a resource has been created unless other attributes are being changed
Steps to reproduce
Expected:
Actual:
Inspiration for this issue report: https://stackoverflow.com/a/68829181/13846505
The text was updated successfully, but these errors were encountered: