crossplane-contrib / provider-upjet-aws

Official AWS Provider for Crossplane by Upbound.
https://marketplace.upbound.io/providers/upbound/provider-aws
Apache License 2.0
143 stars 120 forks source link

[Bug]: SNS Topics Not Creating #1220

Closed bgould-careplanner closed 6 months ago

bgould-careplanner commented 6 months ago

Is there an existing issue for this?

Affected Resource(s)

sns.aws.upbound.io/v1beta1

Resource MRs required to reproduce the bug

apiVersion: sns.aws.upbound.io/v1beta1
kind: Topic
metadata:
  labels:
    testing.upbound.io/example-name: sns
  name: example
spec:
  forProvider:
    region: us-west-1

Steps to Reproduce

Apply the above manifest with the below versions

What happened?

Resource fails to create due to an invalid parameter

Relevant Error Output Snippet

Warning  CannotObserveExternalResource  13m                   managed/sns.aws.upbound.io/v1beta1, kind=topic  failed to observe the resource: [{0 reading SNS Topic (arn:aws:sns:<no value>:xxxxx:example): operation error SNS: GetTopicAttributes, https response error StatusCode: 400, RequestID: 6af5cf45-fe35-5c0e-8e20-912ba24ff906, InvalidParameter: Invalid parameter: TopicArn Reason: A <no value> ARN must begin with arn:null, not arn:aws:sns:<no value>:xxxxx:example  []}]


### Crossplane Version

1.15.0

### Provider Version

v1.2.0

### Kubernetes Version

1.27

### Kubernetes Distribution

EKS

### Additional Info

_No response_
mbbush commented 6 months ago

I can reproduce the issue on main. The SNS Topic resource works as expected in v1.1.1, so this is a regression in v1.2.0.

It looks like something changed that affected the way we parse the region out of the config in TemplatedStringAsIdentifier external name configs that are building ARNs through string interpolation. This likely affects roughly 30 resource kinds that use this type of external name config.

@turkenf We're going to need a v1.2.1 to fix this.

mbbush commented 6 months ago

@erhancagirici I'm not sure, but I think #1204 might have been the cause of this regression.

erhancagirici commented 6 months ago

@bgould-careplanner thanks for reporting and @mbbush thanks for checking. The problem is indeed at #1204 side. I'll send a fix very soon.

The reason is we switched to native TF SDK config and setup.Configuration became obsolete, however as you mentioned, some external name configs with TemplatedStringAsIdentifier use setup.Configuration to get the region parameter. Since the value is not available anymore, the constructed external name is wrong.

I've checked other usages of setup.configuration and there are 25 of them. All of them retrieve region parameter only. Adding back region will suffice to fix the issue.