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

aws provider: normalize json of cloudwatch event_pattern before updating terraform #6025

Conversation

keymon
Copy link
Contributor

@keymon keymon commented Apr 5, 2016

Summary

Normalise the event_pattern of the aws_cloudwatch_event_rule resource
before uploading it to AWS.

AWS seems to accept a event_pattern with a JSON with new lines, but then
the rule does not seem to work. Creating the rule in the AWS console works,
but will setup the pattern as a json without newlines or spaces, and
display a formatted JSON.

Edit: This PR was reporting some misbehaviour in the configured resource, but that was not true, apologies for that. Instead adding targets in the console automatically updates the target to add the required policies. See this comment for more details.

Detailed problem description

Given a config like this:

resource "aws_cloudwatch_event_rule" "CloudTrail_inline" {
  name = "cloudtrail-manipulation-inline"
  description = "Capture tempering with CloudTrail settings - Inline"
  event_pattern = <<PATTERN
{
  "detail-type": [
    "AWS API Call via CloudTrail"
  ],
  "detail": {
    "eventSource": [
      "cloudtrail.amazonaws.com"
    ],
    "eventName": [
      "StopLogging",
      "UpdateTrail",
      "DeleteTrail"
    ]
  }
}
PATTERN
}

Terraform runs cleanly, and even in the interface you can see the rule created and everything fine. But it does not work. But then, if you create it manually, it does work!

But I found the issue running aws cli:

$ aws events list-rules
{
    "Rules": [
        {
            "EventPattern": "{\"detail-type\":[\"AWS API Call via CloudTrail\"],\"detail\":{\"eventSource\":[\"cloudtrail.amazonaws.com\"],\"eventName\":[\"StopLogging\",\"UpdateTrail\",\"DeleteTrail\"]}}",
            "State": "DISABLED",
            "Name": "manually",
            "Arn": "arn:aws:events:us-east-1:....:rule/manually",
            "Description": "manually"
        },
        {
            "EventPattern": "{\n  \"detail-type\": [\n    \"AWS API Call via CloudTrail\"\n  ],\n  \"detail\": {\n    \"eventSource\": [\n      \"cloudtrail.amazonaws.com\"\n    ],\n    \"eventName\": [\n      \"StopLogging\",\n      \"UpdateTrail\",\n      \"DeleteTrail\"\n    ]\n  }\n}\n",
            "State": "ENABLED",
            "Name": "cloudtrail-manipulation",
            "Arn": "arn:aws:events:us-east-1:....:rule/cloudtrail-manipulation",
            "Description": "Capture tempering with CloudTrail settings"
        }
}

You can see that the rule created manually has not newlines.

But in the terraform.tfstate, the rule has no new lines:

$ grep event_pattern terraform.tfstate
                            "event_pattern": "{\"detail\":{\"eventName\":[\"StopLogging\",\"UpdateTrail\",\"DeleteTrail\"],\"eventSource\":[\"cloudtrail.amazonaws.com\"]},\"detail-type\":[\"AWS API Call via CloudTrail\"]}",

Expected result

It should work even with new lines, but I think that is a bug in AWS itself. We should file a bug if somebody can confirm that behaviour.

Meanwhile, I think terraform should send a json without new lines. as done in this PR.

Terraform should push to AWS the same json it stores in the tfstate and that is uploaded when using the console, that is, without newlines and/or spaces.

Normalise the event_pattern of the aws_cloudwatch_event_rule resource
before uploading it to AWS.

AWS seems to accept a event_pattern with a JSON with new lines, but then
the rule does not seem to work. Creating the rule in the AWS console works,
but will setup the pattern  as a json without newlines or spaces, and
display a formatted JSON.
@stack72
Copy link
Contributor

stack72 commented Apr 5, 2016

Hi @keymon

Thanks for the PR - this looks good. Just a FYI, there is some of the cloudformation that handles the JSON in exactly the same way before pushing it to AWS so nice job here.

TF_LOG=1 make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchEventRule' 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=TestAccAWSCloudWatchEventRule -timeout 120m
=== RUN   TestAccAWSCloudWatchEventRule_basic
--- PASS: TestAccAWSCloudWatchEventRule_basic (214.73s)
=== RUN   TestAccAWSCloudWatchEventRule_full
--- PASS: TestAccAWSCloudWatchEventRule_full (68.77s)
=== RUN   TestAccAWSCloudWatchEventRule_enable
--- PASS: TestAccAWSCloudWatchEventRule_enable (141.39s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    424.906s

Thanks

Paul

@stack72 stack72 merged commit eb4fef1 into hashicorp:master Apr 5, 2016
@keymon
Copy link
Contributor Author

keymon commented Apr 6, 2016

FYI, it happens the same with AWS client, which confirms that the main issue is in AWS API:

FYI, the AWS client uploads using new lines, but I found that it trims the json removing newlines after the JSON. From my tests, I found that with new lines at the end does NOT work, but without works.

$ cat > cloudtrail_event_rule.txt <<EOF 
{
  "detail-type": [
    "AWS API Call via CloudTrail"
  ],
  "detail": {
    "eventSource": [
      "cloudtrail.amazonaws.com"
    ],
    "eventName": [
      "StopLogging",
      "UpdateTrail",
      "DeleteTrail"
    ]
  }
}


EOF
$ aws events put-rule  --name test --event-pattern "$(< cloudtrail_event_rule.txt)"
{
    "RuleArn": "arn:aws:events:us-east-1:...:rule/test"
}
$ aws events list-rules
{
    "Rules": [
        {
            "EventPattern": "{\n  \"detail-type\": [\n    \"AWS API Call via CloudTrail\"\n  ],\n  \"detail\": {\n    \"eventSource\": [\n      \"cloudtrail.amazonaws.com\"\n    ],\n    \"eventName\": [\n      \"StopLogging\",\n      \"UpdateTrail\",\n      \"DeleteTrail\"\n    ]\n  }\n}", 
            "State": "ENABLED", 
            "Name": "test", 
            "Arn": "arn:aws:events:us-east-1:...:rule/test"
        }
    ]
}

Meanwhile, the object created by terraform (see the PR summary) contains a new line at the end:

            "EventPattern": "{\n  \"detail-type\": [\n    \"AWS API Call via CloudTrail\"\n  ],\n  \"detail\": {\n    \"eventSource\": [\n      \"cloudtrail.amazonaws.com\"\n    ],\n    \"eventName\": [\n      \"StopLogging\",\n      \"UpdateTrail\",\n      \"DeleteTrail\"\n    ]\n  }\n}\n",

So, I would say that "trimming" the json string would have been sufficient.

@keymon
Copy link
Contributor Author

keymon commented Apr 6, 2016

Updated

The original issue reported in this PR was a misinterpretation from my side: I was testing this by using a SNS target added it manually using the web console, but I was unaware that the web interface automatically updates the target to add the required policies

This miss-leaded me and I assumed that it was an issue in the cloudwatch definition, not a permission issue, as reported originally.

Anyway, this PR is still a good addition as:

  • Puts the same json in AWS as the one stored in tfstate.
  • We do a verification of the JSON before pushing.
  • Editing the rule in AWS web console does format the json in the same way.

I will update the PR and comments to reflect my findings, and cross what is not true.

As follow ups, we might:

@ghost
Copy link

ghost commented Apr 26, 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 26, 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.

2 participants