terraform-compliance / cli

a lightweight, security focused, BDD test framework against terraform.
https://terraform-compliance.com
MIT License
1.36k stars 150 forks source link

Multiple resources dependency #291

Closed KevinBrooke closed 4 years ago

KevinBrooke commented 4 years ago

Question : One of the Azure CIS benchmark requirements is Ensure server parameter 'log_connections' is set to 'ON' for PostgreSQL Database Server.

These settings are configured using azurerm_postgresql_configuration which is not required when creating a azurerm_postgresql_server; so I wanted to do something like this:

Scenario Outline: Ensure server parameter 'log_checkpoints' is set to 'ON' for PostgreSQL Database Server
    Given I have azurerm_postgresql_server defined
    Then I must have azurerm_postgresql_configuration defined
    Then it must contain <configuration>
    And its value must match the "<value>" regex

Which isn't supported. Is there currently a way to check; if creating resource X, resource Y must exist with Z configuration?

Kudbettin commented 4 years ago

Hmm, it looks like that could end up being tested with the current functionality. Could you provide any plan/terraform files?

KevinBrooke commented 4 years ago

Sure:

resource "azurerm_resource_group" "example" {
  name     = "testingtfc-rg"
  location = "northeurope"
}

resource "azurerm_postgresql_server" "example" {
  name                         = "tfcpostgresql1111"
  location                     = azurerm_resource_group.example.location
  resource_group_name          = azurerm_resource_group.example.name
  sku_name                     = "B_Gen5_2"
  storage_mb                   = 5120
  backup_retention_days        = 7
  geo_redundant_backup_enabled = false
  auto_grow_enabled            = true
  administrator_login          = ... removed
  administrator_login_password = ... removed
  version                      = "9.5"
  ssl_enforcement_enabled      = true
}

resource "azurerm_postgresql_configuration" "example" {
  name                = "log_checkpoints"
  resource_group_name = azurerm_resource_group.example.name
  server_name         = azurerm_postgresql_server.example.name
  value               = "on"
}

Test

Feature: Database resources must use approved configurations according to the CIS benchmark
  - Ensure server parameter 'log_checkpoints' is set to 'ON' for PostgreSQL Database Server

  Scenario Outline: Ensure server parameter 'log_checkpoints' is set to 'ON' for PostgreSQL Database Server
    Given I have azurerm_postgresql_server defined
    Then I must have azurerm_postgresql_configuration defined
    Then it must contain <configuration>
    And its value must match the "<value>" regex

    Examples:
      | configuration | value           |
      | name          | log_checkpoints |
      | value         | (on)            |
Kudbettin commented 4 years ago

How about this?

 Scenario: Scenario for issue #291
        Given I have azurerm_postgresql_server defined
        Given I have any resource defined
        Then its type must be azurerm_postgresql_configuration
        When its type is azurerm_postgresql_configuration
        Then it must have value
        Then its value must contain on
Given I have azurerm_postgresql_server defined
Given I have any resource defined
Then its type must be azurerm_postgresql_configuration

This part fails the test if we have a azurerm_postgresql_server defined and azurerm_postgresql_configuration not defined.

However, the "given any" directive is still not filtered down. We can do that by

When its type is azurerm_postgresql_configuration
KevinBrooke commented 4 years ago

Nice, this works for "if creating resource X, resource Y must exist with Z configuration" The next step in this is a problem as the CIS checks a heap of settings which are all set in the same way.

Ensure server parameter 'log_checkpoints' is set to 'ON' for PostgreSQL Database Server
Ensure server parameter 'log_connections' is set to 'ON' for PostgreSQL Database Server
Ensure server parameter 'log_disconnections' is set to 'ON' for PostgreSQL Database Server
Ensure server parameter 'log_duration' is set to 'ON' for PostgreSQL Database Server
Ensure server parameter 'connection_throttling' is set to 'ON' for PostgreSQL Database Server
Ensure server parameter 'log_retention_days' is greater than 3 days for PostgreSQL Database Server

So the Terraform is the same resource type over and over with different settings

resource "azurerm_postgresql_configuration" "log_checkpoints" {
  name                = "log_checkpoints"
  resource_group_name = azurerm_resource_group.example.name
  server_name         = azurerm_postgresql_server.example.name
  value               = "on"
}

resource "azurerm_postgresql_configuration" "log_connections" {
  name                = "log_connections"
  resource_group_name = azurerm_resource_group.example.name
  server_name         = azurerm_postgresql_server.example.name
  value               = "on"
}

resource "azurerm_postgresql_configuration" "log_disconnections" {
  name                = "log_disconnections"
  resource_group_name = azurerm_resource_group.example.name
  server_name         = azurerm_postgresql_server.example.name
  value               = "on"
}

resource "azurerm_postgresql_configuration" "log_duration" {
  name                = "log_duration"
  resource_group_name = azurerm_resource_group.example.name
  server_name         = azurerm_postgresql_server.example.name
  value               = "on"
}

resource "azurerm_postgresql_configuration" "connection_throttling" {
  name                = "connection_throttling"
  resource_group_name = azurerm_resource_group.example.name
  server_name         = azurerm_postgresql_server.example.name
  value               = "on"
}

resource "azurerm_postgresql_configuration" "log_retention_days" {
  name                = "log_retention_days"
  resource_group_name = azurerm_resource_group.example.name
  server_name         = azurerm_postgresql_server.example.name
  value               = "4"
}

Meaning one of them would pass the test for "log_checkpoints" and the rest would fail. Even if we could do Then it must contain <configuration> regex, the tests would pass if one setting was missing. Should I enforce the resource name is aligned with the setting name and target the check in that way? i.e. azurerm_postgresql_configuration.log_disconnections

So if you have azurerm_postgresql_server, you must also have azurerm_postgresql_configuration.log_disconnections, azurerm_postgresql_configuration.log_retention_days, etc. Then if you have azurerm_postgresql_configuration.log_retention_days, its settings must be X.

Kudbettin commented 4 years ago

Hi @KevinBrooke

I'm not sure if I'm understanding you enough. Firstly, is this what you're trying to achieve?

If I have azurerm_postgresql_server defined
    Then I must have azurerm_postgresql_configuration defined

For all azurerm_postgresql_configuration defined
     Their value must be either "on" or a number greater than "3"

You can achieve that by the following scenario:

Scenario: Scenario for issue #291
        Given I have azurerm_postgresql_server defined
        Given I have any resource defined
        Then its type must be azurerm_postgresql_configuration
        When its type is azurerm_postgresql_configuration
        Then it must have value
        Then its value must match the "^(on)$|^([4-9]|\d{2,})$" regex

If you are looking to explicitly test how each configuration is set, you could do something like:

Scenario Outline: Scenario for issue #291
        Given I have azurerm_postgresql_server defined
        Given I have any resource defined
        Then its type must be azurerm_postgresql_configuration
        When its type is azurerm_postgresql_configuration
        Then its name must be <config_name>
        Then it must have value
        Then its value must match the "<value>" regex

Examples:
| config_name        | value            |
| log_checkpoints    | on               |
| log_retention_days | ^([4-9]|\d{2,})$ |
        ...

However, there's a bug with Then its name must be at the moment. If the scenario outline was what you were looking for, I can ping this issue once the bug is fixed.

eerkunt commented 4 years ago

Sure: ...

  sku_name                     = "B_Gen5_2"
  storage_mb                   = 5120
  backup_retention_days        = 7
  geo_redundant_backup_enabled = false
  auto_grow_enabled            = true
  administrator_login          = ... removed
  administrator_login_password = ... removed
  version                      = "9.5"
  ssl_enforcement_enabled      = true
}

...

@KevinBrooke please change your password while creating your resources :)

KevinBrooke commented 4 years ago

haha, I used the Terraform example for this. https://www.terraform.io/docs/providers/azurerm/r/postgresql_server.html Nothing public would ever be exactly what we use.

eerkunt commented 4 years ago

Ok, just panicked :D

Kudbettin commented 4 years ago

Hello @KevinBrooke

With recent release, my previous responses became outdated.

Firstly,

Given I have azurerm_postgresql_server defined
Given I have any resource defined
Then its type must be azurerm_postgresql_configuration

Should not filter to the azurerm_postgresql_configuration. All then directives drill down, so Then its type must be azurerm_postgresql_configuration is actually suppose to drill down to the value of property type.

This bug has been fixed. So how to accomplish the following?

If I have azurerm_postgresql_server defined
    Then I must have azurerm_postgresql_configuration defined

For all azurerm_postgresql_configuration defined
     Their value must be either "on" or a number greater than "3"

We could use the newly added @noskip tag, which will fail on skip.

@noskip_at_line_6 (Line number of when)
Scenario: Ensure server parameter 'log_checkpoints' is set to 'ON' for PostgreSQL Database Server
    Given I have azurerm_postgresql_server defined
    Given I have any resource defined
    When its type is azurerm_postgresql_configuration

So we could change those examples to:

@noskip_at_line_7
Scenario: Ensure server parameter 'log_checkpoints' is set to 'ON' for PostgreSQL Database Server
    Given I have azurerm_postgresql_server defined
    Given I have any resource defined
    When its type is azurerm_postgresql_configuration
    Then it must contain value
    And its value must match the "^(on)$|^([4-9]|\d{2,})$" regex

@noskip_at_lines_22_23 (lines of examples)
Scenario Outline: Scenario for issue #291
    Given I have azurerm_postgresql_server defined
    Given I have any resource defined
    When its type is azurerm_postgresql_configuration
    When its name is <config_name>
    Then it must have value
    Then its value must match the "<value>" regex

Examples:
    | config_name        | value            |
    | log_checkpoints    | on               |
    | log_retention_days | ^([4-9]\|\d{2,})$|

My line numbers could be skewed due to copy and pasting. Please note that I'm tagging examples instead of the when step on the scenario outline.

Could you give 1.2.7 a try?

KevinBrooke commented 4 years ago

Success! The below feature covered all the bases I needed for the Postgres CIS requirements. I tested it using Terraform Compliance 1.2.9.

Feature: Database resources must use approved configurations according to the CIS benchmark
  - Ensure server parameter X is set to X for PostgreSQL Database Server

  @noskip_at_lines_23_24_25_26_27_28
  Scenario Outline: Ensure postgres configurations exist for PostgreSQL Database Server
    Given I have azurerm_postgresql_server defined
    Given I have any resource defined
    When its type is azurerm_postgresql_configuration
    When its name is <config_name>
    Then it must have value
    Then its value must match the "<value>" regex

    Examples:
      | config_name           | value             |
      | log_checkpoints       | on                |
      | log_connections       | on                |
      | log_disconnections    | on                |
      | log_duration          | on                |
      | connection_throttling | on                |
      | log_retention_days    | ^([4-9]\|\d{2,})$ |

Thank-you!!

PS, sorry for the delay!

ghost commented 4 years ago

This issue's conversation is now locked. If you want to continue this discussion please open a new issue.