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_fms_policy validation does not contain full list of acceptable resource types #18551

Closed
willbengtson opened this issue Apr 2, 2021 · 3 comments · Fixed by #18600
Closed
Assignees
Labels
bug Addresses a defect in current functionality. service/fms Issues and PRs that pertain to the fms service.
Milestone

Comments

@willbengtson
Copy link

willbengtson commented Apr 2, 2021

When providing a list of resource types, the validator errors incorrectly on AWS::ElasticLoadBalancing::LoadBalancer and AWS::EC2::EIP

resource "aws_fms_policy" "shiled" {
  name                  = "TestPolicy"
  exclude_resource_tags = false
  remediation_enabled   = false
  resource_type_list = ["AWS::ElasticLoadBalancingV2::LoadBalancer",
    "AWS::ElasticLoadBalancing::LoadBalancer",
  "AWS::EC2::EIP"]

  security_service_policy_data {
    type = "SHIELD_ADVANCED"

    managed_service_data = jsonencode({
      type = "SHIELD_ADVANCED",
    })
  }
}

Panic Output

Error: expected resource_type_list.1 to be one of [AWS::ApiGateway::Stage AWS::CloudFront::Distribution AWS::EC2::NetworkInterface AWS::EC2::Instance AWS::EC2::SecurityGroup AWS::EC2::VPC AWS::ElasticLoadBalancingV2::LoadBalancer], got AWS::ElasticLoadBalancing::LoadBalancer

Error: expected resource_type_list.0 to be one of [AWS::ApiGateway::Stage AWS::CloudFront::Distribution AWS::EC2::NetworkInterface AWS::EC2::Instance AWS::EC2::SecurityGroup AWS::EC2::VPC AWS::ElasticLoadBalancingV2::LoadBalancer], got AWS::EC2::EIP

Expected Behavior

Validation successful

Actual Behavior

Validation error

Steps to Reproduce

  1. terraform plan
@ghost ghost added the service/fms Issues and PRs that pertain to the fms service. label Apr 2, 2021
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Apr 2, 2021
@bflad bflad added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Apr 2, 2021
@bflad
Copy link
Contributor

bflad commented Apr 2, 2021

Thank you for submitting this, @willbengtson 😄 Our list of valid values in that list is currently manually managed, but according to the FMS API Reference the API model specifies the validation is a string regular expression. Especially with the addition of Route 53 Resolver DNS Firewalls, which also is now a supported resource type, we should probably just adjust this validation to match the API modeled regular expression rather than continually updating the resource types manually with bug reports.

bflad added a commit that referenced this issue Apr 6, 2021
…ion for `resource_type` and `resource_type_list` argument plan time validation

Reference: #17399
Reference: #18551

Output from acceptance testing in AWS Commercial (main account):

```
--- PASS: TestAccAWSFms_serial (6.69s)
    --- PASS: TestAccAWSFms_serial/AdminAccount (1.13s)
        --- SKIP: TestAccAWSFms_serial/AdminAccount/basic (1.13s)
    --- PASS: TestAccAWSFms_serial/Policy (5.56s)
        --- SKIP: TestAccAWSFms_serial/Policy/tags (0.10s)
        --- SKIP: TestAccAWSFms_serial/Policy/basic (0.10s)
        --- SKIP: TestAccAWSFms_serial/Policy/cloudfrontDistribution (0.09s)
        --- SKIP: TestAccAWSFms_serial/Policy/includeMap (0.11s)
        --- SKIP: TestAccAWSFms_serial/Policy/update (5.16s)
```

Output from acceptance testing in AWS Commercial (Organizations account):

```
--- PASS: TestAccAWSFms_serial (8148.92s)
    --- PASS: TestAccAWSFms_serial/AdminAccount (1260.56s)
        --- PASS: TestAccAWSFms_serial/AdminAccount/basic (1260.56s)
    --- PASS: TestAccAWSFms_serial/Policy (6888.35s)
        --- PASS: TestAccAWSFms_serial/Policy/cloudfrontDistribution (2277.23s)
        --- PASS: TestAccAWSFms_serial/Policy/includeMap (1253.20s)
        --- PASS: TestAccAWSFms_serial/Policy/update (1147.13s)
        --- PASS: TestAccAWSFms_serial/Policy/tags (1095.08s)
        --- PASS: TestAccAWSFms_serial/Policy/basic (1115.72s)
```
@bflad bflad self-assigned this Apr 7, 2021
bflad added a commit that referenced this issue Apr 15, 2021
…ion for `resource_type` and `resource_type_list` argument plan time validation (#18600)

* service/fms: Fix acceptance testing and use API model regular expression for `resource_type` and `resource_type_list` argument plan time validation

Reference: #17399
Reference: #18551

Output from acceptance testing in AWS Commercial (main account):

```
--- PASS: TestAccAWSFms_serial (6.69s)
    --- PASS: TestAccAWSFms_serial/AdminAccount (1.13s)
        --- SKIP: TestAccAWSFms_serial/AdminAccount/basic (1.13s)
    --- PASS: TestAccAWSFms_serial/Policy (5.56s)
        --- SKIP: TestAccAWSFms_serial/Policy/tags (0.10s)
        --- SKIP: TestAccAWSFms_serial/Policy/basic (0.10s)
        --- SKIP: TestAccAWSFms_serial/Policy/cloudfrontDistribution (0.09s)
        --- SKIP: TestAccAWSFms_serial/Policy/includeMap (0.11s)
        --- SKIP: TestAccAWSFms_serial/Policy/update (5.16s)
```

Output from acceptance testing in AWS Commercial (Organizations account):

```
--- PASS: TestAccAWSFms_serial (8148.92s)
    --- PASS: TestAccAWSFms_serial/AdminAccount (1260.56s)
        --- PASS: TestAccAWSFms_serial/AdminAccount/basic (1260.56s)
    --- PASS: TestAccAWSFms_serial/Policy (6888.35s)
        --- PASS: TestAccAWSFms_serial/Policy/cloudfrontDistribution (2277.23s)
        --- PASS: TestAccAWSFms_serial/Policy/includeMap (1253.20s)
        --- PASS: TestAccAWSFms_serial/Policy/update (1147.13s)
        --- PASS: TestAccAWSFms_serial/Policy/tags (1095.08s)
        --- PASS: TestAccAWSFms_serial/Policy/basic (1115.72s)
```

* Update CHANGELOG for 18600

* tests/resource/aws_fms_admin_account: Remove unused rName parameter

* docs/resource/aws_fms_policy: Switch resource type descriptions to AWS documentation link
@github-actions github-actions bot added this to the v3.37.0 milestone Apr 15, 2021
@ghost
Copy link

ghost commented Apr 16, 2021

This has been released in version 3.37.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented May 16, 2021

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!

@ghost ghost locked as resolved and limited conversation to collaborators May 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/fms Issues and PRs that pertain to the fms service.
Projects
None yet
2 participants