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

Improper partition used when generating SNS Topic ARN #3926

Closed
farvour opened this issue May 10, 2024 · 1 comment · Fixed by #3932
Closed

Improper partition used when generating SNS Topic ARN #3926

farvour opened this issue May 10, 2024 · 1 comment · Fixed by #3932
Assignees
Labels
impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed

Comments

@farvour
Copy link

farvour commented May 10, 2024

What happened?

In US Gov partitions, the provider generates the wrong ARN for the GetTopicAttributes call. This results in the pulumi_aws.sns.Topic() resource from failing to create when the lookup happens.

Example

When using a provider in the US Gov (or China I'm sure too) creation of the topic fails:

test_topic = aws.sns.Topic("test_tjf_1")

Output of pulumi about

pulumi about
CLI          
Version      3.112.0
Go Version   go1.22.1
Go Compiler  gc

Plugins
NAME        VERSION
aws         6.32.0
datadog     4.28.0
kubernetes  4.11.0
postgresql  3.11.0
python      unknown
random      4.16.1

Host     
OS       darwin
Version  13.6.6
Arch     arm64

This project is written in python: executable='/Users/tomfarvour/.local/share/virtualenvs/pulumi-FBQqJsZg/bin/python3' version='3.11.7'

Additional context

This patch, which I presume patches the upstream TF provider, assumes it can use aws as the partition.

+func constructTopicArn(client *sns.Client, account, region, snsTopicName string) string {
+ return fmt.Sprintf("arn:aws:sns:%s:%s:%s", region, account, snsTopicName)
+}
+
+var snsGlobalMutex sync.Map
+

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@farvour farvour added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels May 10, 2024
@t0yv0 t0yv0 added p1 A bug severe enough to be the next item assigned to an engineer impact/regression Something that used to work, but is now broken labels May 10, 2024
@t0yv0
Copy link
Member

t0yv0 commented May 10, 2024

It could be the case that our patch introduced a new failure mode:

	if !tfresource.NotFound(err) {
+		return diag.FromErr(err)
+	}
+

If we're not always predicting ARN correctly, this could surface a failure from by-ARN locator that wasn't a failure in upstream provider.

@corymhall corymhall self-assigned this May 13, 2024
@corymhall corymhall removed the needs-triage Needs attention from the triage team label May 13, 2024
corymhall added a commit that referenced this issue May 13, 2024
Set the AWS partition dynamically instead of hardcoding the default `aws` partition.

I'm not sure there is a good way to test this since we do not have
credentials for other partitions

fixes #3926
corymhall added a commit that referenced this issue May 14, 2024
Set the AWS partition dynamically instead of hardcoding the default
`aws` partition.

I'm not sure there is a good way to test this since we do not have
credentials for other partitions

fixes #3926
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants