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

When passing in SNS Topic, there's a race condition if it's not already created #46

Closed
bushong1 opened this issue Nov 5, 2019 · 20 comments

Comments

@bushong1
Copy link
Contributor

bushong1 commented Nov 5, 2019

If the SNS topic passed in is NOT already created, the notify-slack module will fail. This is due to notify-slack's data objects being resolved before my resource objects.

resource "aws_sns_topic" "my_sns" {
  name  = "my-sns"
}

module "notify_slack" {
  source  = "terraform-aws-modules/notify-slack/aws"
  version = "2.0.0"
  slack_channel        = "my_channel"
  slack_username       = "My SNS"
  slack_webhook_url    = "https://hooks.slack.com/services/<SNIP>"
  sns_topic_name       = aws_sns_topic.my_sns.name  #<<<<<<<
  create_sns_topic     = false                #<<<<<<<<<
  slack_emoji          = ":yuk:"
  lambda_function_name = "${var.name_prefix}-my-slack-lambda"
}

From what I can tell, the only usage of the data resource is to ensure that SNS exists before running, and then to craft the ARN. I'd suggest hand-crafting the ARN with the Topic Name instead. I can submit a PR if it would be helpful.

@bushong1
Copy link
Contributor Author

bushong1 commented Nov 5, 2019

The workaround for this is to target your sns topic and apply it before you plan the rest of your stuff. It works, but when your system is set up for applying terraform with a CI Pipeline, this falls apart.

terraform apply -target aws_sns_topic.my_sns
...
terraform plan
terraform apply

@antonbabenko
Copy link
Member

Hi @bushong1 !

I agree it should work automatically within a CI/CD pipeline and, I believe, it does so.

I tried to reproduce this issue on version 2.0.0 with the exact code as you provide. See this updated example.

The sequence of execution is correct:

aws_sns_topic.my_sns: Creating...
module.notify_slack.aws_iam_role.lambda[0]: Creating...
aws_sns_topic.my_sns: Creation complete after 1s [id=arn:aws:sns:eu-west-1:835367859852:my-sns]
module.notify_slack.data.aws_sns_topic.this[0]: Refreshing state...
...

Can you please show the sequence of resources it tries to create during terraform apply?

@bushong1
Copy link
Contributor Author

Thanks for looking into it. Sorry about the 'sample code' that worked. I was trying to condense the issue from our code found here: https://github.com/USSBA/terraform-aws-sns-notification-framework/blob/master/main.tf

I'm curious if it has something to do with nested modules? I'll try testing it out.

@bushong1
Copy link
Contributor Author

Okay @antonbabenko, here's an update:

TL;DR: Try your sample code again, but with a workspace that has already had 1 deployment to it.

Here's my brain-train of how I got to replicate the issue:
With a file sns.tf

resource "aws_sns_topic" "my_sns" {
  name = "my-sns"
}
data "aws_sns_topic" "this" {
  name = aws_sns_topic.my_sns.name
}

Running this with a workspace that has some of my application or account code, I get the error:

Error: No topic with name "my-sns" found in this region.

  on sns.tf line 4, in data "aws_sns_topic" "this":
   4: data "aws_sns_topic" "this" {

But... running this same code in a fresh workspace with no other code applied, i get a totally clean plan, just like your example code:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create
 <= read (data resources)

Terraform will perform the following actions:

  # data.aws_sns_topic.this will be read during apply
  # (config refers to values not yet known)
 <= data "aws_sns_topic" "this"  {
      + arn  = (known after apply)
      + id   = (known after apply)
      + name = "my-sns"
    }

  # aws_sns_topic.my_sns will be created
  + resource "aws_sns_topic" "my_sns" {
      + arn    = (known after apply)
      + id     = (known after apply)
      + name   = "my-sns"
      + policy = (known after apply)
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Super strange. HOWEVER.... If in my new workspace I comment out these sns creations first, create a temporary resource:

resource "aws_ssm_parameter" "my_test_parameter" {
  name = "test_parameter"
  type = "String"
  value = "foo"
}
#resource "aws_sns_topic" "my_sns" {
#  name = "my-sns"
#}
#data "aws_sns_topic" "this" {
 # name = aws_sns_topic.my_sns.name
#}

Apply this, to "initialize" the workspace, then un-comment this sample code...

resource "aws_ssm_parameter" "my_test_parameter" {
  name = "test_parameter"
  type = "String"
  value = "foo"
}
resource "aws_sns_topic" "my_sns" {
  name = "my-sns"
}
data "aws_sns_topic" "this" {
  name = aws_sns_topic.my_sns.name
}

I get the error:

Error: No topic with name "my-sns" found in this region.

  on sns.tf line 9, in data "aws_sns_topic" "this":
   9: data "aws_sns_topic" "this" {

And, in fact, if I re-enable the notify-slack module, i get the same failure to plan.

Error: No topic with name "my-sns" found in this region.

  on .terraform/modules/notify_slack/terraform-aws-modules-terraform-aws-notify-slack-e3ffbba/main.tf line 1, in data "aws_sns_topic" "this":
   1: data "aws_sns_topic" "this" {

@bushong1
Copy link
Contributor Author

Any update on this, @antonbabenko ?

@antonbabenko
Copy link
Member

Unfortunately, I don't understand what is going on here in your code. You are using Terraform workspaces and I am almost certain that the problem is related to that.

PS: I am against using Terraform workspaces in 99% of cases (including yours) :)

@bushong1
Copy link
Contributor Author

@antonbabenko okay, as I said at the top with some rewording:
Try your sample code again, but with a workspace terraform state that has already had 1 deployment to it.

For example:
tf apply this:

provider "aws" {
  region = "eu-west-1"
}

resource "aws_sns_topic" "bogus_sns" {
  name = "some-bogus-sns-topic"
}

and then edit the file to be this and apply again:

provider "aws" {
  region = "eu-west-1"
}

resource "aws_sns_topic" "my_sns" {
  name = "my-sns"
}

module "notify_slack" {
  source = "../../"

  sns_topic_name   = aws_sns_topic.my_sns.name
  create_sns_topic = false

  slack_webhook_url = "https://hooks.slack.com/services/AAA/BBB/CCC"
  slack_channel     = "aws-notification"
  slack_username    = "reporter"

  tags = {
    Name = "notify-slack-simple"
  }
}

@dynamike
Copy link
Member

I just ran into this issue as well and have hit it in other modules in the past. I believe this is due to a change of behavior in terraform 0.12. The suggested change is we should no longer be looking up data sources from inside modules, but instead we should be passing in the entire resource objects as variables. See hashicorp/terraform#22730 for details.

@bushong1
Copy link
Contributor Author

@antonbabenko any chance to take a look at this PR?

@bushong1
Copy link
Contributor Author

Bump

@kromol
Copy link

kromol commented Apr 2, 2020

I also face this issue with terraform 0.12 and it makes the pipeline to fail as well. @antonbabenko would you be able to find some time to review MR and hopefully merge it?

Thanks in advance

@bermannoah
Copy link

Just want to +1 here, I've run into a lot of issues with using this module and an already existing topic. 😓 Would be great if this fix could be merged.

@antonbabenko
Copy link
Member

I still don't understand why it is happening, and the solution of passing the whole resource as an object does work but it is not how we handle arguments in terraform-aws-modules. Module arguments should always have plan types - string, list, map, bool, but not objects.

Also, I could not reproduce this problem myself which makes the fix rather hard for me. If anyone can give me 100% working instructions (as similar to examples/notify-slack-simple as possible) I can take another look.

@bushong1
Copy link
Contributor Author

bushong1 commented Apr 24, 2020

@antonbabenko I gave an example of the bug that you never replied to. I'll re-paste it here:

The bug only crops up in an already existing terraform state. So... terraform apply this:

provider "aws" {
  region = "eu-west-1"
}

resource "aws_sns_topic" "bogus_sns" {
  name = "some-bogus-sns-topic"
}

and then edit the file to be this and apply again:

provider "aws" {
  region = "eu-west-1"
}

resource "aws_sns_topic" "my_sns" {
  name = "my-sns"
}

module "notify_slack" {
  source = "../../"

  sns_topic_name   = aws_sns_topic.my_sns.name
  create_sns_topic = false

  slack_webhook_url = "https://hooks.slack.com/services/AAA/BBB/CCC"
  slack_channel     = "aws-notification"
  slack_username    = "reporter"

  tags = {
    Name = "notify-slack-simple"
  }
}

bushong1 added a commit to bushong1/terraform-aws-notify-slack that referenced this issue Apr 29, 2020
…race condition

Data Resources are evaluated before any code is executed, so if a single terraform apply intends to create an SNS topic _and_ create this module, the apply will fail because the data resource cannot resolve.  Avoiding the use of data resources by deriving the SNS manually corrects the issue in this case.
bushong1 added a commit to bushong1/terraform-aws-notify-slack that referenced this issue Apr 29, 2020
…race condition

Data Resources are evaluated before any code is executed, so if a single terraform apply intends to create an SNS topic _and_ create this module, the apply will fail because the data resource cannot resolve.  Avoiding the use of data resources by deriving the SNS manually corrects the issue in this case.
bushong1 added a commit to bushong1/terraform-aws-notify-slack that referenced this issue Apr 29, 2020
…race condition

Data Resources are evaluated before any code is executed, so if a single terraform apply intends to create an SNS topic _and_ create this module, the apply will fail because the data resource cannot resolve.  Avoiding the use of data resources by deriving the SNS manually corrects the issue in this case.
bushong1 added a commit to bushong1/terraform-aws-notify-slack that referenced this issue Apr 29, 2020
…race condition

Data Resources are evaluated before any code is executed, so if a single terraform apply intends to create an SNS topic _and_ create this module, the apply will fail because the data resource cannot resolve.  Avoiding the use of data resources by deriving the SNS manually corrects the issue in this case.
bushong1 added a commit to bushong1/terraform-aws-notify-slack that referenced this issue Apr 29, 2020
…form-aws-modulesGH-46

Data Resources are evaluated before any code is executed, so if a single terraform apply intends to create an SNS topic _and_ create this module, the apply will fail because the data resource cannot resolve.  Avoiding the use of data resources by deriving the SNS manually corrects the issue in this case.
@bushong1
Copy link
Contributor Author

@antonbabenko I've created a fairly straightforward PR to address the problem. Hopefully this helps clear things up.

bushong1 added a commit to bushong1/terraform-aws-notify-slack that referenced this issue Apr 29, 2020
…form-aws-modulesGH-46

Data Resources are evaluated before any code is executed, so if a single terraform apply intends to create an SNS topic _and_ create this module, the apply will fail because the data resource cannot resolve.  Avoiding the use of data resources by deriving the SNS manually corrects the issue in this case.
@sohailnajar
Copy link

Not sure if you are still facing this but I managed to get around by adding depends_on in data section of the module instead of creating topic resource outside of the module.

data "aws_sns_topic" "this" {
  name = var.sns_topic_name
  depends_on = [
    aws_sns_topic.this,
  ]
}

@latacora-tomekr
Copy link

Commenting to mention that I also ran into this issue

@bushong1
Copy link
Contributor Author

Bump

@antonbabenko
Copy link
Member

Finally! :)

v4.12.0 has been just released.

@github-actions
Copy link

github-actions bot commented Nov 8, 2022

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants