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

provider/aws: cloudwatch_logs_subscription_filter resource #5996

Merged
merged 1 commit into from
Apr 20, 2016

Conversation

joshmyers
Copy link
Contributor

Instead of #4813

@apparentlymart apparentlymart changed the title Add cloudwatch_logs_subscription_filter provider provider/aws: cloudwatch_logs_subscription_filter resource Apr 4, 2016
@stack72
Copy link
Contributor

stack72 commented Apr 5, 2016

Hi @joshmyers

thanks so much for the PR, on trying to start reviewing this, I ran the tests and got the following:

TF_LOG=1 make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudwatchLogSubscriptionFilter' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudwatchLogSubscriptionFilter -timeout 120m
=== RUN   TestAccAWSCloudwatchLogSubscriptionFilter_basic
--- FAIL: TestAccAWSCloudwatchLogSubscriptionFilter_basic (0.01s)
    testing.go:154: Step 0 error: Configuration is invalid.

        Warnings: []string(nil)

        Errors: []string{"aws_lambda_function.test_lambdafunction: \"role\": required field is not set"}
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    0.030s

Please can you have a look at these?

Thanks

Paul

@stack72 stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Apr 5, 2016
@stack72 stack72 self-assigned this Apr 5, 2016
@joshmyers
Copy link
Contributor Author

@stack72 I've fixed up the tests and rebased

~/go/src/github.com/hashicorp/terraform(branch:cwl_subscription_filter) » TF_LOG=DEBUG make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudwatchLogSubscriptionFilter' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
/Users/joshmyers/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudwatchLogSubscriptionFilter -timeout 120m
=== RUN   TestAccAWSCloudwatchLogSubscriptionFilter_basic
--- PASS: TestAccAWSCloudwatchLogSubscriptionFilter_basic (19.26s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    19.290s
------------------------------------------------------------

@stack72
Copy link
Contributor

stack72 commented Apr 6, 2016

Hi @joshmyers

this all looks good - there is 1 issue that we see in a few places right now

F_LOG=DEBUG make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudwatchLogSubscriptionFilter' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
/Users/pstack/code/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudwatchLogSubscriptionFilter -timeout 120m
=== RUN   TestAccAWSCloudwatchLogSubscriptionFilter_basic
--- FAIL: TestAccAWSCloudwatchLogSubscriptionFilter_basic (18.78s)
    testing.go:154: Step 0 error: Error applying: 1 error(s) occurred:

        * aws_cloudwatch_log_subscription_filter.test_lambdafunction_logfilter: InvalidParameterException: Could not execute the lambda function. Make sure you have given CloudWatch Logs permission to execute your function.
            status code: 400, request id: 4e68910f-fc35-11e5-943c-b58a46d37bfe
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    18.798s

This is due to IAM eventual consistency issues. I will speak to some of HC team about that before I merge

I will keep you informed

Paul

@joshmyers
Copy link
Contributor Author

@stack72 I can't reproduce this locally but I guess it is intermittent. Would be great to get this merged as we are using our fork of TF and have fallen behind a few releases.

@joshmyers
Copy link
Contributor Author

@stack72 Good talk last night, was hoping to catch up with you after but it went on longer than expected. Is there any update on this?

@stack72
Copy link
Contributor

stack72 commented Apr 20, 2016

Hi @joshmyers

Glad you enjoyed the talk. I haven't been able to reproduce this error again so this is good to merge

Thanks for all the work here :)

Paul

@stack72 stack72 removed the waiting-response An issue/pull request is waiting for a response from the community label Apr 20, 2016
@stack72 stack72 merged commit 3be66aa into hashicorp:master Apr 20, 2016
@catsby
Copy link
Contributor

catsby commented May 11, 2016

Hello friends –

TestAccAWSCloudwatchLogSubscriptionFilter_basic has been failing consistently in our nightly acceptances tests since 2016-04-21 when it was introduced:

--- FAIL: TestAccAWSCloudwatchLogSubscriptionFilter_basic (16.47s)
    testing.go:174: Step 0 error: Error applying: 1 error(s) occurred:

        * aws_cloudwatch_log_subscription_filter.test_lambdafunction_logfilter: InvalidParameterException: Could not execute the lambda function. Make sure you have given CloudWatch Logs permission to execute your function.

Unfortunately, I am not a IAM / Policy Guru, can anyone shed some light on what we're missing here in the config?

resource "aws_cloudwatch_log_subscription_filter" "test_lambdafunction_logfilter" {
  name            = "test_lambdafunction_logfilter"
  log_group_name  = "example_lambda_name"
  filter_pattern  = "logtype test"
  destination_arn = "${aws_lambda_function.test_lambdafunction.arn}"
}

resource "aws_lambda_function" "test_lambdafunction" {
  filename      = "test-fixtures/lambdatest.zip"
  function_name = "example_lambda_name"
  role          = "${aws_iam_role.iam_for_lambda.arn}"
  handler       = "exports.handler"
}

resource "aws_cloudwatch_log_group" "logs" {
  name              = "example_lambda_name"
  retention_in_days = 1
}

resource "aws_lambda_permission" "allow_cloudwatch_logs" {
  statement_id  = "AllowExecutionFromCloudWatchLogs"
  action        = "lambda:*"
  function_name = "${aws_lambda_function.test_lambdafunction.arn}"
  principal     = "logs.eu-west-1.amazonaws.com"
}

resource "aws_iam_role" "iam_for_lambda" {
  name = "test_lambdafuntion_iam_role"

  assume_role_policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": "sts:AssumeRole",
      "Principal": {
        "Service": "lambda.amazonaws.com"
      },
      "Effect": "Allow",
      "Sid": ""
    }
  ]
}
EOF
}

resource "aws_iam_role_policy" "test_lambdafunction_iam_policy" {
  name = "test_lambdafunction_iam_policy"
  role = "${aws_iam_role.iam_for_lambda.id}"

  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "Stmt1441111030000",
      "Effect": "Allow",
      "Action": [
        "dynamodb:*"
      ],
      "Resource": [
        "*"
      ]
    }
  ]
}
EOF
}

@radeksimko
Copy link
Member

@catsby I originally thought this would be YAIAMECI™ (yet another IAM eventual consistency issue), but probably not this time.

I took the exact config you pasted above, just changed eu-west-1 to us-east-1, pointed Lambda function to a valid ZIP file and got to this:

Apply complete! Resources: 6 added, 0 changed, 0 destroyed.

All of our acceptance tests run in us-west-2 AFAIK, so that's the issue.

@stack72
Copy link
Contributor

stack72 commented May 11, 2016

Oh no! Hard coded string :( my AWS creds have a default of region of eu-west so I didn't even notice that. Doh!

@catsby
Copy link
Contributor

catsby commented May 11, 2016

@radeksimko BRILLIANT thank you :)

@joshmyers
Copy link
Contributor Author

Hi folks, just seen this.While writing tests I saw others which used a hardcoded string so I did the same. I thought it doesn't feel right but didn't get picked up. Apologies!

@catsby
Copy link
Contributor

catsby commented Jun 21, 2016

@joshmyers no worries! (sorry for the very late reply)

@ghost
Copy link

ghost commented Apr 25, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants