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

BUG: SNS FIFO topic gives an error #670

Closed
mgshirali opened this issue Apr 13, 2023 · 9 comments · Fixed by #1031
Closed

BUG: SNS FIFO topic gives an error #670

mgshirali opened this issue Apr 13, 2023 · 9 comments · Fixed by #1031
Labels
bug Something isn't working is:triaged Indicates that an issue has been reviewed.

Comments

@mgshirali
Copy link

What environment did it happen in?

  • Crossplane Version: 1.11.2
  • Provider Version: xpkg.upbound.io/upbound/provider-aws:v0.29.0
  • Kubernetes Version: 1.25.3
  • Kubernetes Distribution: kind

I'm trying to create a FIFO SNS topic using the following yaml file:

apiVersion: sns.aws.upbound.io/v1beta1
kind: Topic
metadata:
  name: some.fifo
spec:
  forProvider:
    policy: |
      {
          "Version":"2012-10-17",
          "Statement":[{
              "Effect": "Allow",
              "Principal": {
                  "AWS": "*"
              },
              "Action": "SNS:Publish",
              "Resource": "arn:aws:sns:*:*:some.fifo"
          }]
      }
    fifoTopic: true
    contentBasedDeduplication: true
    region: us-east-1
    kmsMasterKeyId: alias/aws/sns

This results in the following error:

Events:
  Type     Reason                           Age                From                                            Message
  ----     ------                           ----               ----                                            -------
  Warning  CannotInitializeManagedResource  24s                managed/sns.aws.upbound.io/v1beta1, kind=topic  Operation cannot be fulfilled on topics.sns.aws.upbound.io "some.fifo": the object has been modified; please apply your changes to the latest version and try again
  Warning  CannotConnectToProvider          15s (x4 over 24s)  managed/sns.aws.upbound.io/v1beta1, kind=topic  cannot get a terraform workspace for resource: cannot init workspace: There are some problems with the configuration, described below.

The Terraform configuration must be valid before initialization so that
Terraform can determine which modules and providers need to be installed.
╷
│ Error: Invalid resource name
│
│   on main.tf.json line 1, in resource.aws_sns_topic:
│    1: {"provider":{"aws":{"access_key":"REDACTED","region":"REDACTED","secret_key":"REDACTED","token":""}},"resource":{"aws_sns_topic":{"some.fifo":{"content_based_deduplication":true,"fifo_topic":true,"kms_master_key_id":"alias/aws/sns","lifecycle":{"prevent_destroy":true},"name":"some.fifo","policy":"{\n    \"Version\":\"2012-10-17\",\n    \"Statement\":[{\n        \"Effect\": \"Allow\",\n        \"Principal\": {\n            \"AWS\": \"*\"\n        },\n        \"Action\": \"SNS:Publish\",\n        \"Resource\": \"arn:aws:sns:*:*:some.fifo\"\n    }]\n}\n","tags":{"crossplane-kind":"topic.sns.aws.upbound.io","crossplane-name":"some.fifo","crossplane-providerconfig":"default"}}}},"terraform":{"required_providers":{"aws":{"source":"hashicorp/aws","version":"4.52.0"}}}}
│
│ A name must start with a letter or underscore and may contain only letters,
│ digits, underscores, and dashes.
╵

: exit status 1

If I skip the .fifo in the name, and keep the other params as is, then the topic is never created. There is no error either, however, the system just continues to show "Creating"

Am I missing something or is there an issue with creating fifo topics using upbound provider?

@mgshirali mgshirali added bug Something isn't working needs:triage labels Apr 13, 2023
@svscheg
Copy link
Contributor

svscheg commented Apr 14, 2023

Hi @mgshirali ! Could you please attach output yaml (kubectl get topic.sns.aws.upbound.io/ -o yaml)?
For now I can reproduce this issue but in case If I skip the .fifo in the name - I can see an error in output:

  • lastTransitionTime: "2023-04-14T17:32:28Z"
    message: 'apply failed: invalid topic name: sometestingfifo: '
    reason: ApplyFailure
    status: "False"
    type: LastAsyncOperation

@mgshirali
Copy link
Author

Hi @svscheg , thanks for taking a look.
Incase .fifo is appended to the name, there is the below error

apiVersion: sns.aws.upbound.io/v1beta1
kind: Topic
metadata:
  annotations:
    crossplane.io/external-name: some.fifo
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"sns.aws.upbound.io/v1beta1","kind":"Topic","metadata":{"annotations":{},"name":"some.fifo"},"spec":{"forProvider":{"contentBasedDeduplication":true,"fifoTopic":true,"kmsMasterKeyId":"alias/aws/sns","policy":"{\n    \"Version\":\"2012-10-17\",\n    \"Statement\":[{\n        \"Effect\": \"Allow\",\n        \"Principal\": {\n            \"AWS\": \"*\"\n        },\n        \"Action\": \"SNS:Publish\",\n        \"Resource\": \"arn:aws:sns:*:*:some.fifo\"\n    }]\n}\n","region":"us-east-1"}}}
  creationTimestamp: "2023-04-17T03:30:01Z"
  generation: 2
  name: some.fifo
  resourceVersion: "1173559"
  uid: b76b2e1f-eab3-4121-9267-a432597f127f
spec:
  deletionPolicy: Delete
  forProvider:
    contentBasedDeduplication: true
    fifoTopic: true
    kmsMasterKeyId: alias/aws/sns
    policy: |
      {
          "Version":"2012-10-17",
          "Statement":[{
              "Effect": "Allow",
              "Principal": {
                  "AWS": "*"
              },
              "Action": "SNS:Publish",
              "Resource": "arn:aws:sns:*:*:some.fifo"
          }]
      }
    region: us-east-1
    tags:
      crossplane-kind: topic.sns.aws.upbound.io
      crossplane-name: some.fifo
      crossplane-providerconfig: default
  providerConfigRef:
    name: default
status:
  atProvider: {}
  conditions:
  - lastTransitionTime: "2023-04-17T03:30:01Z"
    message: "connect failed: cannot get a terraform workspace for resource: cannot
      init workspace: \e[31m\e[0mThere are some problems with the configuration, described
      below.\n\nThe Terraform configuration must be valid before initialization so
      that\nTerraform can determine which modules and providers need to be installed.\e[0m\e[0m\n\e[31m\e[31m╷\e[0m\e[0m\n\e[31m│\e[0m
      \e[0m\e[1m\e[31mError: \e[0m\e[0m\e[1mInvalid resource name\e[0m\n\e[31m│\e[0m
      \e[0m\n\e[31m│\e[0m \e[0m\e[0m  on main.tf.json line 1, in resource.aws_sns_topic:\n\e[31m│\e[0m
      \e[0m   1: {\"provider\":{\"aws\":{\"access_key\":\"REDACTED\",\"region\":\"REDACTED\",\"secret_key\":\"REDACTED\",\"token\":\"\"}},\"resource\":{\"aws_sns_topic\":{\e[4m\"some.fifo\"\e[0m:{\"content_based_deduplication\":true,\"fifo_topic\":true,\"kms_master_key_id\":\"alias/aws/sns\",\"lifecycle\":{\"prevent_destroy\":true},\"name\":\"some.fifo\",\"policy\":\"{\\n
      \   \\\"Version\\\":\\\"2012-10-17\\\",\\n    \\\"Statement\\\":[{\\n        \\\"Effect\\\":
      \\\"Allow\\\",\\n        \\\"Principal\\\": {\\n            \\\"AWS\\\": \\\"*\\\"\\n
      \       },\\n        \\\"Action\\\": \\\"SNS:Publish\\\",\\n        \\\"Resource\\\":
      \\\"arn:aws:sns:*:*:some.fifo\\\"\\n    }]\\n}\\n\",\"tags\":{\"crossplane-kind\":\"topic.sns.aws.upbound.io\",\"crossplane-name\":\"some.fifo\",\"crossplane-providerconfig\":\"default\"}}}},\"terraform\":{\"required_providers\":{\"aws\":{\"source\":\"hashicorp/aws\",\"version\":\"4.52.0\"}}}}\e[0m\n\e[31m│\e[0m
      \e[0m\n\e[31m│\e[0m \e[0mA name must start with a letter or underscore and may
      contain only letters,\n\e[31m│\e[0m \e[0mdigits, underscores, and dashes.\n\e[31m╵\e[0m\e[0m\n\e[0m\e[0m\n:
      exit status 1"
    reason: ReconcileError
    status: "False"
    type: Synced

I checked this again, and yes incase .fifo is skipped from the name, then I do see the error below (as you've confirmed). I guess I missed it.

  - lastTransitionTime: "2023-04-17T03:35:41Z"
    message: 'apply failed: invalid topic name: some: '
    reason: ApplyFailure
    status: "False"
    type: LastAsyncOperation

So, does that mean there is a bug with creation of fifo SNS topic? Is there a config piece/work around for this?

@svscheg
Copy link
Contributor

svscheg commented Apr 18, 2023

Thank @mgshirali for confirm error.
Issue will be triaged.

The issue is reproduced on the latest master.

@svscheg svscheg added is:triaged Indicates that an issue has been reviewed. and removed needs:information needs:triage labels Apr 18, 2023
@jayz28
Copy link

jayz28 commented Jun 2, 2023

I am blocked by this issue as well.
If the .fifo is removed from the name, the following error occurs:

- lastTransitionTime: '2023-05-26T18:40:40Z'
      message: >-
        apply failed: invalid topic name:
        resbio-prod-euw2-a-pipeline-work-status: 
      reason: ApplyFailure
      status: 'False'
      type: LastAsyncOperation

This is expected, as .fifo is required for SNS fifo topics.

A reasonable solution may be for the provider to automatically append .fifo to the name if fifoTopic: true is set in the resource.

@jayz28
Copy link

jayz28 commented Jun 2, 2023

@mgshirali Wanted to mention a workaround is to set the annotation crossplane.io/external-name to the correct name with .fifo appended. This will override metadata.name as the name of the resource, and allow the provider to successfully create the SNS topic.

@haooliveira84
Copy link

We are blocked by this issue as well.

Look in this documentation we can see:

The name of a FIFO queue must end with the .fifo suffix. The suffix counts towards the 80-character queue name quota. To determine whether a queue is FIFO, you can check whether the queue name ends with the suffix.

@mbbush
Copy link
Collaborator

mbbush commented Dec 18, 2023

I don't think this is a bug, it's just an unfortunate consequence of the naming restrictions. To the extent that the previous behavior was broken, it looks like that was fixed by switching to the no-fork architecture in provider-aws version 0.44.

I just posted a PR which adds two valid examples of creating a FIFO SNS topic, and then ran a test which creates them, imports them, and deletes them successfully. You can see the test logs here until they expire.

You can either use the metadata.name of your kubernetes resource as the name of the topic, and make it end with .fifo (which is allowed, but AFAIK not generally recommended), or you can use the crossplane.io/external-name annotation to set the name of the topic in AWS (which must end in .fifo) according to the aws-specific naming restrictions, and the kubernetes metadata.name can be whatever you want. I gave examples of both in #1031.

It does seem like there are some undocumented (or inconsistently-documented) restrictions on the fifo SNS topic name on the aws side. I didn't exhaustively explore possible combinations, but some of my examples worked and others didn't. If you're running into an invalid name error, please try changing the format of the name to match what aws expects (which is possibly different than what AWS says it expects. I got several names rejected that should have been valid according to what the cloudformation documentation says, but I suspect the docs are wrong.)

@haooliveira84
Copy link

I see the same issue occurs in SQS, IMHO it seems to be a bug!

@mbbush
Copy link
Collaborator

mbbush commented Dec 24, 2023

@haooliveira84 can you clarify what precisely you think is a bug? Do the naming conventions I used in the examples I added to the linked PR not work for you?

Something along the lines of "when I X I expect Y but instead observe Z" would be helpful in understanding the issue here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working is:triaged Indicates that an issue has been reviewed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants