-
Notifications
You must be signed in to change notification settings - Fork 157
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
Fix import resources with provider default tags #4169
Conversation
We have special logic around applying default provider tags to resources. This logic only applied to the `Check` call which means it was not applied when you were importing resources. This PR extends that logic to also run during the `Read` call. fix #4030, fix 4080
Does the PR have any schema changes?Looking good! No breaking changes found. Maintainer note: consult the runbook for dealing with any breaking changes. |
}) | ||
} | ||
|
||
allTags, err := mergeTags(ctx, configTags, meta) |
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.
I'd appreciate a little bit of elaboration here on what we're trying to do, and how does it work, I see this:
As written, allTags = mergeTags(ctx, config.tags, {defaultTags: config.tagsAll})
.
I think if I recall what TransformOutputs is doing this will be assigning config to the tags and tagsAll coming out of the TF provider. This seems to then ignore the current defaultTags on the provider itself as it were.
Why does this work? I thought perhaps we're not trusting the tags_all output from the provider internals, is it useful here?
delete(ret, "tagsAll") | ||
return ret, nil | ||
} | ||
ret["tags"] = allTags |
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.
Why do we set allTags
to tags
? Shouldn't tags
only include the tags specified on the resource?
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 was also confused here, I was expecting something like this:
-
(1) we receive data from the Read call that has "tags" and "tags_all"
-
(2) I think here we need to understand what this data is and do we trust it; e.g. the TF provider would be grabbing all tags from the AWS hopefully, but then it would also possibly applying the defaultTags to that result to either union or exclude them? We had to disable this behavior for Create/Update lifecycle since we couldn't get the implementation to behave correctly and trust it; perhaps something similar needs to be tweaked here for reads
-
(3) suppose we figured out (2) correctly and we obtained what we believe is the set of in-AWS resource tags. Now what do we decide to surface as "tags". Perhaps here we want to exclude surfacing a k:v pair in "tags" if the defaultTags already has it set at provider level. So perhaps take the set difference here?
Unfortunately it's more complicated than this as TransformOutputs hook is hiding some differences around the context this code is executing in. Let me add below.
ResourceProvider.Read we're trying to modify here is very overloaded in Pulumi unfortunately, so there's a matrix of case here to consider. Case 1: pulumi import on the CLI
Relatively simple version where there are no program inputs to consider and the objective is to generate a minimal program. The program would be minimal if all cloud tags that are already in defaultTags are omitted. Case 2: import with resource optionI think this is somehow meaningfully different from vanilla import because it has some resource inputs to consider from the program. If the program specifies an input tag k:v in the inputs and the cloud doesn't have it or has a contradicting value, Pulumi should probably be failing saying that the inputs do not match so that the user fixes up the program before proceeding. If the program does not specify the tag k:v that cloud has, but defaultTags do, then we should proceed without warning.
Case 3: pulumi refreshThis one is interesting because it starts engaging bridge heuristics ExtractInputsFromOutputs to try to guess which cloud value is an input and which one is an output. I think it's pretty crude right now and might not do what we'd like for tags, worth thinking through. Case 4: Bucket.get()This one I have a bit of a knowledge gap and need to brush up on, but I think it will also call Read. |
The additional matrix dimension is that resources can be PF based or SDKv2 based and the underlying TF behavior can be different . |
TransformOutputs is a blunt tool since it will not only modify Read results, but also modify Create and Update results, and it can't yet understand which kind of Read context it's operating in. There's some additional burden on verifying these changes don't affect Create/Update but looks like the test suite passed so that might be working just fine. I'm wondering if we need something more granular here.. |
@t0yv0 @flostadler here is what my current understanding is. There are two fields that track tags on a resource;
new aws.s3.BucketV2('bucket', {
tags: {
SomeKey: 'SomeValue',
},
});
new aws.Provider('aws', {
defaultTags: {
tags: {
GlobalTag: 'Value',
},
},
}); When it comes time to apply these tags to a resource we merge the tags into a single list of tags. The way that we do this is a hack on the pulumi side as part of the From what I can tell, the intention is for ExampleIf I were to create a resource with this configuration (both const provider = new aws.Provider('aws', {
defaultTags: {
tags: {
GlobalTag: 'Value',
},
},
});
new aws.s3.BucketV2('bucket', {
tags: {
LocalTag: 'Value',
},
}, { provider }); The state of the bucket resource will have the tags like this (merged together): {
"resources": [
{
"type": "aws:s3/bucketV2:BucketV2",
"inputs": {
"tags": {
"GlobalTag": "Value",
"LocalTag": "Value"
},
"tagsAll": {
"GlobalTag": "Value",
"LocalTag": "Value"
}
},
"outputs": {
"tags": {
"GlobalTag": "Value",
"LocalTag": "Value"
},
"tagsAll": {
"GlobalTag": "Value",
"LocalTag": "Value"
}
}
}
]
} If I were to then go an add a new tag in the console {
"method": "/pulumirpc.ResourceProvider/Read",
"request": {...},
"response": {
"tags": {
"GlobalTag": "ValueOther",
"LocalTag": "ValueOther",
"ConsoleTag": "Value"
},
"tagsAll": {
"GlobalTag": "ValueOther",
"LocalTag": "ValueOther",
"ConsoleTag": "Value"
},
"inputs": {
"tags": {
"GlobalTag": "Value",
"LocalTag": "Value"
},
"tagsAll": {
"GlobalTag": "Value",
"LocalTag": "Value"
}
}
}
}
{
"method": "/pulumirpc.ResourceProvider/Diff",
"request": {
"olds": {
"tags": {
"GlobalTag": "ValueOther",
"LocalTag": "ValueOther",
"ConsoleTag": "Value"
},
"tagsAll": {
"GlobalTag": "ValueOther",
"LocalTag": "ValueOther",
"ConsoleTag": "Value"
}
},
"news": {
"tags": {
"GlobalTag": "Value",
"LocalTag": "Value"
},
"tagsAll": {
"GlobalTag": "Value",
"LocalTag": "Value"
}
},
"oldInputs": {
"tags": {
"GlobalTag": "Value",
"LocalTag": "Value"
},
"tagsAll": {
"GlobalTag": "Value",
"LocalTag": "Value"
}
}
},
"response": {
"changes": "DIFF_SOME",
"diffs": ["tags","tags","tags","tagsAll","tagsAll","tagsAll"],
"detailedDiff": {...},
"hasDetailedDiff": true
}
} If instead I try and import a resource with the same configuration of tags there would be no existing
This always leads to a diff because we are comparing an unmerged {
"method": "/pulumirpc.ResourceProvider/Read",
"request": {"properties": {}},
"response": {
"properties": {
"tags": {
"LocalTag": "Value"
},
"tagsAll": {
"GlobalTag": "Value"
},
},
"inputs": {
"tags": {
"LocalTag": "Value"
},
"tagsAll": {
"GlobalTag": "Value"
}
}
}
}
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"olds": {
"tags": {
"LocalTag": "Value"
},
"tagsAll": {
"GlobalTag": "Value"
}
},
"news": {
"tags": {
"LocalTag": "Value"
}
}
},
"response": {
"inputs": {
"tags": {
"LocalTag": "Value" ,
"GlobalTag": "Value"
},
"tagsAll": {
"LocalTag": "Value" ,
"GlobalTag": "Value"
}
}
}
}
{
"method": "/pulumirpc.ResourceProvider/Diff",
"request": {
"olds": {
"tags": {
"LocalTag": "Value"
},
"tagsAll": {
"GlobalTag": "Value"
}
},
"news": {
"tags": {
"LocalTag": "Value" ,
"GlobalTag": "Value"
},
"tagsAll": {
"LocalTag": "Value" ,
"GlobalTag": "Value"
}
},
"oldInputs": {
"tags": {
"LocalTag": "Value"
},
"tagsAll": {
"LocalTag": "Value" ,
"GlobalTag": "Value"
}
}
},
"response": {
"changes": "DIFF_SOME",
"diffs": [ "tags", "tags", "tags" ],
"detailedDiff": {...},
"hasDetailedDiff": true
}
} What this PR attempts to do is to apply the same type of hack that we do in
I think this only matters in a case where we want |
@corymhall Do you have some background on
This is not what TF does: provider "aws" {
region = "us-east-1"
default_tags {
tags = {
GlobalTag = "GlobalVal"
}
}
}
# s3 bucket
resource "aws_s3_bucket" "bucket" {
bucket_prefix = "mybucket"
tags = {
"localTag" = "localValue"
}
} produces
which makes sense to me. Why are we doing something different here? |
Apparently there are a lot of upstream issues around tagging and we decided to do something about it on our side. I think @t0yv0 and @iwahbe are most familiar with this and they are both out this week. Lines 25 to 45 in 0063967
|
What if we just create a test for each of the scenarios Anton mentioned? I think that should ensure nothing unexpected breaks. |
Thanks for the background @corymhall
#3859 has an example of importing in a test. |
return applyTagsOutputs(ctx, config) | ||
} | ||
} else { | ||
prov.Resources[key].TransformOutputs = applyTagsOutputs |
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.
Note that TransformOutputs affects Create and Update, not just read - is this intended here?
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.
It's a side effect, but as long as it is true that tags
and defaultTags
should always be equal then I think we are fine.
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.
Is my understanding correct that instead of following terraform's behavior of merging tags
and default_tags
into tags_all
, we manually do the merge and set the result to both tags/tagsAll. Your change is adding that to the read code path as well.
What I'm still missing is why we need all of the special tag handling? In what areas does terraforms tagging fall short? Are we sure that it's still broken (the tests you've added might be good regression tests to check if we still need it)? Those are rather questions for @t0yv0 / @iwahbe I guess.
The change looks good to me, the tests are great! I think we should add some tests for the static get functions as well though.
provider/provider_yaml_test.go
Outdated
preImportHook: func(t *testing.T, outputs auto.OutputMap) { | ||
t.Helper() | ||
resArn := outputs["resArn"].Value.(string) | ||
addAppconfigEnvironmentTags(context.Background(), resArn, map[string]string{ |
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.
Nit: we could generalize this using tag:TagResources
which can tag all taggable resources.
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!
func generateTagsTest(t *testing.T, step tagsTestStep, testPath string, importId string) { | ||
template := `name: test-aws-%s | ||
runtime: yaml | ||
resources: |
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 should test the static get
functions as well ideally because we're changing the behavior of read
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.
Added!
That's a good idea, i'll work on adding that.
This is one of my big questions - what kind of tests do we need to add in order to be confident of the changes?
This might be more inline with the current |
I'm actually not sure that it is possible to generate the desire program. I think we can choose to have In the current state (before this PR):
{
"method": "/pulumirpc.ResourceProvider/Read",
"request": {},
"response": {
"inputs": {
"tags": {
"LocalTag": "foo"
},
"tagsAll": {
"GlobalTag": "bar",
"LocalTag": "foo"
}
}
}
}
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {...},
"response": {
"inputs": {
"tags": {
"GlobalTag": "bar",
"LocalTag": "foo"
},
"tagsAll": {
"GlobalTag": "bar",
"LocalTag": "foo"
}
}
}
} So we have two options:
|
We need the special tags handling because terraform-provider-aws's native tags handling relies on refresh for correctness, and so doesn't work for most Pulumi users. |
@iwahbe can you expand on what this means? |
I expanded a bit here: #4230 (comment). Basically, if you run |
provider/provider_yaml_test.go
Outdated
}) | ||
bucketName := outputs["id"].Value.(string) | ||
tags := getBucketTagging(context.Background(), bucketName) | ||
assert.Subset(t, tags, []types.Tag{ |
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.
Subset and not Equal? Why? Interesting.
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.
bucketName := stack.Outputs["bucket-name"].(string) | ||
st.assertTagsEqualWithRetry(t, | ||
fetchBucketTags(bucketName), | ||
"bad bucket tags") | ||
} | ||
if k == "legacy-bucket" { | ||
// getTags := stack.Outputs["get-legacy-bucket"].(string) |
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.
Are these comments still useful?
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.
Added another comment with link to the issue. When the issue is fixed the code can be uncommented.
return err | ||
} | ||
|
||
// refresh doesn't work for `forceDelete` & `acl` uncomment when fixed |
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 is interesting, is this something worth linking a ticket to? Thank you.
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.
Added another comment with link to the issue.
This PR has been shipped in release v6.45.2. |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@pulumi/aws](https://pulumi.io) ([source](https://togithub.com/pulumi/pulumi-aws)) | dependencies | minor | [`6.45.0` -> `6.46.0`](https://renovatebot.com/diffs/npm/@pulumi%2faws/6.45.0/6.46.0) | --- ### Release Notes <details> <summary>pulumi/pulumi-aws (@​pulumi/aws)</summary> ### [`v6.46.0`](https://togithub.com/pulumi/pulumi-aws/releases/tag/v6.46.0) [Compare Source](https://togithub.com/pulumi/pulumi-aws/compare/v6.45.2...v6.46.0) ##### Does the PR have any schema changes? Found 8 breaking changes: ##### Resources - `🟢` "aws:elasticache/replicationGroup:ReplicationGroup": required inputs: "description" input has changed to Required ##### Types - `🟢` "aws:bedrock/AgentAgentAliasRoutingConfiguration:AgentAgentAliasRoutingConfiguration": required: "provisionedThroughput" property has changed to Required - `🟡` "aws:ecs/ServiceVolumeConfigurationManagedEbsVolume:ServiceVolumeConfigurationManagedEbsVolume": properties: "throughput" type changed from "string" to "integer" - "aws:kinesis/FirehoseDeliveryStreamRedshiftConfiguration:FirehoseDeliveryStreamRedshiftConfiguration": required: - `🟢` "password" property is no longer Required - `🟢` "username" property is no longer Required - "aws:kinesis/FirehoseDeliveryStreamSnowflakeConfiguration:FirehoseDeliveryStreamSnowflakeConfiguration": required: - `🟢` "privateKey" property is no longer Required - `🟢` "user" property is no longer Required - `🟢` "aws:kinesis/FirehoseDeliveryStreamSplunkConfiguration:FirehoseDeliveryStreamSplunkConfiguration": required: "hecToken" property is no longer Required ##### New resources: - `datazone/project.Project` - `grafana/workspaceServiceAccount.WorkspaceServiceAccount` - `grafana/workspaceServiceAccountToken.WorkspaceServiceAccountToken` - `rds/certificate.Certificate` - `rekognition/streamProcessor.StreamProcessor` ##### New functions: - `cloudfront/getOriginAccessControl.getOriginAccessControl` - `timestreamwrite/getDatabase.getDatabase` - `timestreamwrite/getTable.getTable` ##### What's Changed - Upstream v5.59.0 by [@​t0yv0](https://togithub.com/t0yv0) in [https://github.com/pulumi/pulumi-aws/pull/4295](https://togithub.com/pulumi/pulumi-aws/pull/4295) **Full Changelog**: pulumi/pulumi-aws@v6.45.2...v6.46.0 ### [`v6.45.2`](https://togithub.com/pulumi/pulumi-aws/releases/tag/v6.45.2) [Compare Source](https://togithub.com/pulumi/pulumi-aws/compare/v6.45.0...v6.45.2) ##### Does the PR have any schema changes? Looking good! No breaking changes found. No new resources/functions. ##### What's Changed ##### Fixes - Fix import resources with provider default tags by [@​corymhall](https://togithub.com/corymhall) in [https://github.com/pulumi/pulumi-aws/pull/4169](https://togithub.com/pulumi/pulumi-aws/pull/4169) ##### Dependencies - Upgrade pulumi-terraform-bridge to v3.87.0 by [@​pulumi-bot](https://togithub.com/pulumi-bot) in [https://github.com/pulumi/pulumi-aws/pull/4240](https://togithub.com/pulumi/pulumi-aws/pull/4240) ##### Internal - Rewrite GameLift regression test into TypeScript by [@​t0yv0](https://togithub.com/t0yv0) in [https://github.com/pulumi/pulumi-aws/pull/4285](https://togithub.com/pulumi/pulumi-aws/pull/4285) - Update GitHub Actions workflows. by [@​pulumi-bot](https://togithub.com/pulumi-bot) in [https://github.com/pulumi/pulumi-aws/pull/4290](https://togithub.com/pulumi/pulumi-aws/pull/4290) **Full Changelog**: pulumi/pulumi-aws@v6.45.1...v6.45.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNCIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL21pbm9yIl19--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@pulumi/aws](https://pulumi.io) ([source](https://togithub.com/pulumi/pulumi-aws)) | dependencies | minor | [`6.45.0` -> `6.46.0`](https://renovatebot.com/diffs/npm/@pulumi%2faws/6.45.0/6.46.0) | --- ### Release Notes <details> <summary>pulumi/pulumi-aws (@​pulumi/aws)</summary> ### [`v6.46.0`](https://togithub.com/pulumi/pulumi-aws/releases/tag/v6.46.0) [Compare Source](https://togithub.com/pulumi/pulumi-aws/compare/v6.45.2...v6.46.0) ##### Does the PR have any schema changes? Found 8 breaking changes: ##### Resources - `🟢` "aws:elasticache/replicationGroup:ReplicationGroup": required inputs: "description" input has changed to Required ##### Types - `🟢` "aws:bedrock/AgentAgentAliasRoutingConfiguration:AgentAgentAliasRoutingConfiguration": required: "provisionedThroughput" property has changed to Required - `🟡` "aws:ecs/ServiceVolumeConfigurationManagedEbsVolume:ServiceVolumeConfigurationManagedEbsVolume": properties: "throughput" type changed from "string" to "integer" - "aws:kinesis/FirehoseDeliveryStreamRedshiftConfiguration:FirehoseDeliveryStreamRedshiftConfiguration": required: - `🟢` "password" property is no longer Required - `🟢` "username" property is no longer Required - "aws:kinesis/FirehoseDeliveryStreamSnowflakeConfiguration:FirehoseDeliveryStreamSnowflakeConfiguration": required: - `🟢` "privateKey" property is no longer Required - `🟢` "user" property is no longer Required - `🟢` "aws:kinesis/FirehoseDeliveryStreamSplunkConfiguration:FirehoseDeliveryStreamSplunkConfiguration": required: "hecToken" property is no longer Required ##### New resources: - `datazone/project.Project` - `grafana/workspaceServiceAccount.WorkspaceServiceAccount` - `grafana/workspaceServiceAccountToken.WorkspaceServiceAccountToken` - `rds/certificate.Certificate` - `rekognition/streamProcessor.StreamProcessor` ##### New functions: - `cloudfront/getOriginAccessControl.getOriginAccessControl` - `timestreamwrite/getDatabase.getDatabase` - `timestreamwrite/getTable.getTable` ##### What's Changed - Upstream v5.59.0 by [@​t0yv0](https://togithub.com/t0yv0) in [https://github.com/pulumi/pulumi-aws/pull/4295](https://togithub.com/pulumi/pulumi-aws/pull/4295) **Full Changelog**: pulumi/pulumi-aws@v6.45.2...v6.46.0 ### [`v6.45.2`](https://togithub.com/pulumi/pulumi-aws/releases/tag/v6.45.2) [Compare Source](https://togithub.com/pulumi/pulumi-aws/compare/v6.45.0...v6.45.2) ##### Does the PR have any schema changes? Looking good! No breaking changes found. No new resources/functions. ##### What's Changed ##### Fixes - Fix import resources with provider default tags by [@​corymhall](https://togithub.com/corymhall) in [https://github.com/pulumi/pulumi-aws/pull/4169](https://togithub.com/pulumi/pulumi-aws/pull/4169) ##### Dependencies - Upgrade pulumi-terraform-bridge to v3.87.0 by [@​pulumi-bot](https://togithub.com/pulumi-bot) in [https://github.com/pulumi/pulumi-aws/pull/4240](https://togithub.com/pulumi/pulumi-aws/pull/4240) ##### Internal - Rewrite GameLift regression test into TypeScript by [@​t0yv0](https://togithub.com/t0yv0) in [https://github.com/pulumi/pulumi-aws/pull/4285](https://togithub.com/pulumi/pulumi-aws/pull/4285) - Update GitHub Actions workflows. by [@​pulumi-bot](https://togithub.com/pulumi-bot) in [https://github.com/pulumi/pulumi-aws/pull/4290](https://togithub.com/pulumi/pulumi-aws/pull/4290) **Full Changelog**: pulumi/pulumi-aws@v6.45.1...v6.45.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNCIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL21pbm9yIl19--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
We have special logic around applying default provider tags to
resources. This logic only applied to the
Check
call which means itwas not applied when you were importing resources. This PR extends that
logic to also run during the
Read
call by utilizingTransformOutputs
.While it is true that
TransformOutputs
also runs duringCreate
&Update
this is a side effect that I think is ok. From my understanding
tags
andtagsAll
should always be equal. If we have an additional place where we make sure they are equal
it shouldn't harm anything.
I've added tests (see
testTagsPulumiLifecycle
) which test the complete lifecycle of a pulumi programUp
with both providerdefaultTags
/ignoreTags
and resource leveltags
1a. Run validations on result
Refresh
with no changesImport
using the resource option. Ensures resource can be successfully imported3a. Allows for a hook to be run prior to import being run. e.g. Add tags remotely
Import
using the CLI. Ensures resources can be successfully imported4a. Allows for a hook to be run prior to import being run. e.g. Add tags remotely
Refresh
with no changesfix #4030, fix #4080, fix #3311