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

New Resource: aws_cloudtrail_event_data_store #22490

Merged
merged 32 commits into from
Feb 15, 2022

Conversation

bschaatsbergen
Copy link
Member

@bschaatsbergen bschaatsbergen commented Jan 9, 2022

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates #22434

Output from acceptance testing:

$ make testacc TESTS=TestAccEventDataStore PKG=cloudtrail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20 -run='TestAccEventDataStore'  -timeout 180m
=== RUN   TestAccEventDataStore_basic
=== PAUSE TestAccEventDataStore_basic
=== RUN   TestAccEventDataStore_disappears
=== PAUSE TestAccEventDataStore_disappears
=== RUN   TestAccEventDataStore_multi_region_enabled
=== PAUSE TestAccEventDataStore_multi_region_enabled
=== RUN   TestAccEventDataStore_advanced_event_selector
=== PAUSE TestAccEventDataStore_advanced_event_selector
=== CONT  TestAccEventDataStore_basic
=== CONT  TestAccEventDataStore_multi_region_enabled
=== CONT  TestAccEventDataStore_advanced_event_selector
=== CONT  TestAccEventDataStore_disappears
--- PASS: TestAccEventDataStore_disappears (17.64s)
--- PASS: TestAccEventDataStore_multi_region_enabled (21.19s)
--- PASS: TestAccEventDataStore_basic (22.35s)
--- PASS: TestAccEventDataStore_advanced_event_selector (24.41s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail 24.473s

@github-actions github-actions bot added provider Pertains to the provider itself, rather than any interaction with AWS. service/cloudtrail Issues and PRs that pertain to the cloudtrail service. needs-triage Waiting for first response or review from a maintainer. size/XL Managed by automation to categorize the size of a PR. labels Jan 9, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome @bschaatsbergen 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

Copy link

@NathanGitgit NathanGitgit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great initiative, I'm looking forward to keep track of this draft PR.

@bschaatsbergen bschaatsbergen changed the title [WIP] New Resource: aws_cloudtrail_lake [WIP] New Resource: aws_cloudtrail_event_data_store Jan 11, 2022
@justinretzolk justinretzolk added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Jan 11, 2022
@bschaatsbergen
Copy link
Member Author

bschaatsbergen commented Jan 25, 2022

Hey @ewbankkit,

The event data store CRUD seems to work, there's just something wrong with the advanced_event_selector.

By default the SDK implicitly sets it to 'capture all management events'. Because this is not reflected in the resource when declaring it, it does end up in the state, but the subsequent plan or apply will again try to delete the advanced_event_selector that's set by the default behaviour.

  # aws_cloudtrail_event_data_store.main will be updated in-place
  ~ resource "aws_cloudtrail_event_data_store" "main" {
        id                             = "arn:aws:cloudtrail:eu-central-1:123456789010:eventdatastore/4481b52a-4892-4c3e-a2b0-b4ec4e40368d"
        name                           = "tf-test-store-7"
      ~ tags                           = {
          + "Name" = "tf-test-store-7"
        }
      ~ tags_all                       = {
          + "Name" = "tf-test-store-7"
        }
        # (5 unchanged attributes hidden)

      - advanced_event_selector {
          - name = "Default management events" -> null

          - field_selector {
              - ends_with       = [] -> null
              - equals          = [
                  - "Management",
                ] -> null
              - field           = "eventCategory" -> null
              - not_ends_with   = [] -> null
              - not_equals      = [] -> null
              - not_starts_with = [] -> null
              - starts_with     = [] -> null
            }
        }
    }


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

Could you advice me on how to tackle this please?

@ewbankkit
Copy link
Contributor

@bschaatsbergen Try marking advanced_event_selector (and its nested attributes) as Computed together with Optional.

@bschaatsbergen
Copy link
Member Author

bschaatsbergen commented Jan 26, 2022

Thanks @ewbankkit. I noticed that if you remove one of the operators (equals, not equals, etc) and only set the field in the field_selector the API returns a 400 (which is apparently retryable by looking at the SDK) and it stays on Creating.. or Modifying.... It should throw an error as a operator is always required in combination with a field (from what I can see).

---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1
Host: cloudtrail.eu-central-1.amazonaws.com
User-Agent: APN/1.0 HashiCorp/1.0 Terraform/1.1.3 (+https://www.terraform.io) terraform-provider-aws/dev (+https://registry.terraform.io/providers/hashicorp/aws) aws-sdk-go/1.42.38 (go1.17.6; linux; amd64)
Content-Length: 254
Authorization: AWS4-HMAC-SHA256 Credential=AKIKEXAMPLEW/20220126/eu-central-1/cloudtrail/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-target, Signature=d47ab6235d3c3b48c058d4ec0fe037d7fa87130ebd7f9dacb0e0d1EXAMPLE
Content-Type: application/x-amz-json-1.1
X-Amz-Date: 20220126T123506Z
X-Amz-Target: com.amazonaws.cloudtrail.v20131101.CloudTrail_20131101.UpdateEventDataStore
Accept-Encoding: gzip

{"AdvancedEventSelectors":[{"FieldSelectors":[{"Field":"eventCategory"}],"Name":"Log Delete* events for S3 buckets"}],"EventDataStore":"arn:aws:cloudtrail:eu-central-1:1234556788:eventdatastore/4521dbaf-be02-462b-ae48-14cf09EXAMPLE","RetentionPeriod":8}
-----------------------------------------------------: timestamp=2022-01-26T13:35:06.718+0100
2022-01-26T13:35:06.849+0100 [INFO]  provider.terraform-provider-aws: 2022/01/26 13:35:06 [DEBUG] [aws-sdk-go] DEBUG: Response cloudtrail/UpdateEventDataStore Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 400 
Connection: close
Content-Length: 58
Content-Type: application/x-amz-json-1.1
Date: Wed, 26 Jan 2022 12:35:06 GMT
X-Amzn-Requestid: 9e8123f4-8ca6-4374-bc53-2d7ffcEXAMPLE


-----------------------------------------------------: timestamp=2022-01-26T13:35:06.849+0100
2022-01-26T13:35:06.849+0100 [INFO]  provider.terraform-provider-aws: 2022/01/26 13:35:06 [DEBUG] [aws-sdk-go] {"__type":"ThrottlingException","message":"Rate exceeded"}: timestamp=2022-01-26T13:35:06.849+0100
2022-01-26T13:35:06.849+0100 [INFO]  provider.terraform-provider-aws: 2022/01/26 13:35:06 [DEBUG] [aws-sdk-go] DEBUG: Validate Response cloudtrail/UpdateEventDataStore failed, attempt 1/25, error ThrottlingException: Rate exceeded
        status code: 400

continues retrying...

@github-actions github-actions bot added the documentation Introduces or discusses updates to documentation. label Jan 27, 2022
@github-actions github-actions bot added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Feb 4, 2022
@bschaatsbergen bschaatsbergen marked this pull request as ready for review February 5, 2022 23:37
@bschaatsbergen bschaatsbergen changed the title [WIP] New Resource: aws_cloudtrail_event_data_store New Resource: aws_cloudtrail_event_data_store Feb 5, 2022
@bschaatsbergen
Copy link
Member Author

Done @ewbankkit, besides the previous comment (which seems related to the AWS API) and that I'm unable to test the organization_enabled, everything is tested and seems to work 👍

Acceptance test output:

% make testacc TESTS=TestAccEventDataStore_basic PKG=cloudtrail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20 -run='TestAccEventDataStore_basic'  -timeout 180m
=== RUN   TestAccEventDataStore_basic
=== PAUSE TestAccEventDataStore_basic
=== CONT  TestAccEventDataStore_basic
--- PASS: TestAccEventDataStore_basic (22.33s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail	27.974s
Acceptance test output:

% make testacc TESTS=TestAccEventDataStore_options PKG=cloudtrail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20 -run='TestAccEventDataStore_options'  -timeout 180m
=== RUN   TestAccEventDataStore_options
=== PAUSE TestAccEventDataStore_options
=== CONT  TestAccEventDataStore_options
--- PASS: TestAccEventDataStore_options (35.91s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail	41.731s
Acceptance test output:

% make testacc TESTS=TestAccEventDataStore_tags PKG=cloudtrail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20 -run='TestAccEventDataStore_tags'  -timeout 180m
=== RUN   TestAccEventDataStore_tags
=== PAUSE TestAccEventDataStore_tags
=== CONT  TestAccEventDataStore_tags
--- PASS: TestAccEventDataStore_tags (40.99s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail	44.986s
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀.

Commercial
% make testacc TESTS=TestAccEventDataStore_ PKG=cloudtrail 
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20 -run='TestAccEventDataStore_'  -timeout 180m
=== RUN   TestAccEventDataStore_basic
=== PAUSE TestAccEventDataStore_basic
=== RUN   TestAccEventDataStore_disappears
=== PAUSE TestAccEventDataStore_disappears
=== RUN   TestAccEventDataStore_tags
=== PAUSE TestAccEventDataStore_tags
=== RUN   TestAccEventDataStore_options
=== PAUSE TestAccEventDataStore_options
=== RUN   TestAccEventDataStore_advancedEventSelector
=== PAUSE TestAccEventDataStore_advancedEventSelector
=== CONT  TestAccEventDataStore_basic
=== CONT  TestAccEventDataStore_options
=== CONT  TestAccEventDataStore_advancedEventSelector
=== CONT  TestAccEventDataStore_disappears
=== CONT  TestAccEventDataStore_tags
=== CONT  TestAccEventDataStore_options
    acctest.go:701: this AWS account must be the management account of an AWS Organization
--- SKIP: TestAccEventDataStore_options (1.25s)
--- PASS: TestAccEventDataStore_disappears (16.09s)
--- PASS: TestAccEventDataStore_basic (20.89s)
--- PASS: TestAccEventDataStore_advancedEventSelector (24.49s)
--- PASS: TestAccEventDataStore_tags (41.64s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail	45.575s
GovCloud
% make testacc TESTS=TestAccEventDataStore_ PKG=cloudtrail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20 -run='TestAccEventDataStore_'  -timeout 180m
=== RUN   TestAccEventDataStore_basic
=== PAUSE TestAccEventDataStore_basic
=== RUN   TestAccEventDataStore_disappears
=== PAUSE TestAccEventDataStore_disappears
=== RUN   TestAccEventDataStore_tags
=== PAUSE TestAccEventDataStore_tags
=== RUN   TestAccEventDataStore_options
=== PAUSE TestAccEventDataStore_options
=== RUN   TestAccEventDataStore_advancedEventSelector
=== PAUSE TestAccEventDataStore_advancedEventSelector
=== CONT  TestAccEventDataStore_basic
=== CONT  TestAccEventDataStore_options
=== CONT  TestAccEventDataStore_disappears
=== CONT  TestAccEventDataStore_tags
=== CONT  TestAccEventDataStore_advancedEventSelector
=== CONT  TestAccEventDataStore_disappears
    acctest.go:1008: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: error creating CloudTrail Event Data Store (tf-acc-test-8727672389834937490): UnsupportedOperationException: The operation requested is not supported in the region.
        
          with aws_cloudtrail_event_data_store.test,
          on terraform_plugin_test.tf line 2, in resource "aws_cloudtrail_event_data_store" "test":
           2: resource "aws_cloudtrail_event_data_store" "test" {
        
=== CONT  TestAccEventDataStore_basic
    acctest.go:1008: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: error creating CloudTrail Event Data Store (tf-acc-test-3181324507280160273): UnsupportedOperationException: The operation requested is not supported in the region.
        
          with aws_cloudtrail_event_data_store.test,
          on terraform_plugin_test.tf line 2, in resource "aws_cloudtrail_event_data_store" "test":
           2: resource "aws_cloudtrail_event_data_store" "test" {
        
=== CONT  TestAccEventDataStore_advancedEventSelector
    acctest.go:1008: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: error creating CloudTrail Event Data Store (tf-acc-test-6934047121505972719): UnsupportedOperationException: The operation requested is not supported in the region.
        
          with aws_cloudtrail_event_data_store.test,
          on terraform_plugin_test.tf line 2, in resource "aws_cloudtrail_event_data_store" "test":
           2: resource "aws_cloudtrail_event_data_store" "test" {
        
=== CONT  TestAccEventDataStore_options
    acctest.go:1008: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: error creating CloudTrail Event Data Store (tf-acc-test-1237862277570900657): UnsupportedOperationException: The operation requested is not supported in the region.
        
          with aws_cloudtrail_event_data_store.test,
          on terraform_plugin_test.tf line 2, in resource "aws_cloudtrail_event_data_store" "test":
           2: resource "aws_cloudtrail_event_data_store" "test" {
        
=== CONT  TestAccEventDataStore_tags
    acctest.go:1008: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: error creating CloudTrail Event Data Store (tf-acc-test-6132664734702259425): UnsupportedOperationException: The operation requested is not supported in the region.
        
          with aws_cloudtrail_event_data_store.test,
          on terraform_plugin_test.tf line 2, in resource "aws_cloudtrail_event_data_store" "test":
           2: resource "aws_cloudtrail_event_data_store" "test" {
        
--- SKIP: TestAccEventDataStore_disappears (9.13s)
--- SKIP: TestAccEventDataStore_advancedEventSelector (9.19s)
--- SKIP: TestAccEventDataStore_options (9.21s)
--- SKIP: TestAccEventDataStore_basic (9.21s)
--- SKIP: TestAccEventDataStore_tags (9.22s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail	12.617s

@ewbankkit
Copy link
Contributor

@bschaatsbergen Thanks for the contribution 🎉 👏.
Overall the contribution was great.
I made a few mainly cosmetic changes and fixed some bugs in the resource Update functionality that additional tests uncovered.

@bschaatsbergen
Copy link
Member Author

@ewbankkit. that's great to hear and for sure more contributions will follow. Thanks for adding the touch-ups!

…NotFound.

Acceptance test output:

% make testacc TESTS=TestAccEventDataStore_ PKG=cloudtrail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20 -run='TestAccEventDataStore_'  -timeout 180m
=== RUN   TestAccEventDataStore_basic
=== PAUSE TestAccEventDataStore_basic
=== RUN   TestAccEventDataStore_disappears
=== PAUSE TestAccEventDataStore_disappears
=== RUN   TestAccEventDataStore_tags
=== PAUSE TestAccEventDataStore_tags
=== RUN   TestAccEventDataStore_options
=== PAUSE TestAccEventDataStore_options
=== RUN   TestAccEventDataStore_advancedEventSelector
=== PAUSE TestAccEventDataStore_advancedEventSelector
=== CONT  TestAccEventDataStore_basic
=== CONT  TestAccEventDataStore_options
=== CONT  TestAccEventDataStore_tags
=== CONT  TestAccEventDataStore_disappears
=== CONT  TestAccEventDataStore_advancedEventSelector
=== CONT  TestAccEventDataStore_options
    acctest.go:701: this AWS account must be the management account of an AWS Organization
--- SKIP: TestAccEventDataStore_options (1.09s)
--- PASS: TestAccEventDataStore_disappears (21.03s)
--- PASS: TestAccEventDataStore_advancedEventSelector (28.89s)
--- PASS: TestAccEventDataStore_tags (49.38s)
--- PASS: TestAccEventDataStore_basic (71.26s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail	77.703s
@ewbankkit ewbankkit merged commit a65002b into hashicorp:main Feb 15, 2022
@bschaatsbergen bschaatsbergen deleted the r/cloudtrail-lake branch February 15, 2022 20:31
@ewbankkit ewbankkit added this to the v4.2.0 milestone Feb 23, 2022
@github-actions
Copy link

I'm going to lock this pull request 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 related to this change, 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 May 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. new-resource Introduces a new resource. provider Pertains to the provider itself, rather than any interaction with AWS. service/cloudtrail Issues and PRs that pertain to the cloudtrail service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants