-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
New Resource: aws_cloudwatch_event_permission #2888
New Resource: aws_cloudwatch_event_permission #2888
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one nitpick.
policyStatement = &statement | ||
return nil | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: I wonder if we could decouple this code (for loop) into a separate function, similar to https://github.com/terraform-providers/terraform-provider-aws/blob/29327322186bd8005832b338060a418c8e29bfc2/aws/resource_aws_lambda_permission.go#L329-L344
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly simplifies things! Hadn't run into resource.NotFoundError
yet, looks super handy. I'll update this PR with a similar implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated and verified locally:
make testacc TEST=./aws TESTARGS='-run=TestAccAWSCloudWatchEventPermission'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCloudWatchEventPermission -timeout 120m
=== RUN TestAccAWSCloudWatchEventPermission_Basic
--- PASS: TestAccAWSCloudWatchEventPermission_Basic (19.04s)
=== RUN TestAccAWSCloudWatchEventPermission_Action
--- PASS: TestAccAWSCloudWatchEventPermission_Action (8.98s)
=== RUN TestAccAWSCloudWatchEventPermission_Import
--- PASS: TestAccAWSCloudWatchEventPermission_Import (12.85s)
=== RUN TestAccAWSCloudWatchEventPermission_Multiple
--- PASS: TestAccAWSCloudWatchEventPermission_Multiple (17.88s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 58.793s
* Add and utilize findCloudWatchEventPermissionPolicyStatementByID function in logic and testing
This has been released in terraform-provider-aws version 1.7.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
* commit 'b9284490eff637460fef663794a496d363e19f10': (190 commits) v1.7.0 Update CHANGELOG.md d/aws_ssm_parameter: Support returning raw encrypted SecureString value. (hashicorp#2777) Bump aws-sdk-go to v1.12.60 Update CHANGELOG.md Add acceptance test for import + randomization Removed reference to Core fixes Add instructions for vendor updates Use AWS example instead of Azure Update CHANGELOG for hashicorp#2833 Update CHANGELOG.md r/lb_target_group: Fix validation rules for LB's healthcheck Update CHANGELOG for hashicorp#2911 r/aws_guardduty_member: Provide given ID in error message when incorrect format Update CHANGELOG.md Update CHANGELOG with hashicorp#2888 r/aws_guardduty_member: hashicorp#2911 PR review r/aws_cloudwatch_event_permission: hashicorp#2888 PR review Makefile: Fixed test outputs resource/aws_lb+aws_elb: Fix regression with undefined 'name' ... # Conflicts: # .gitignore # aws/data_source_aws_s3_bucket_object.go # aws/resource_aws_elasticsearch_domain.go
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Closes #1042
Please note the implementation is a little awkward because there are
PutPermission
/RemovePermission
API actions but reading is done via the backend created IAM policy on the event bus, available via thePolicy
attribute onDescribeEventBus
. There's also a peculiarity with thePutPermission
API concurrency where two "parallel" operations will only write the last received entry, but it always returns 200 OKs for both requests. If there's a good solution for a resource-wide mutex, this would be a good place for one.References: