sqladmin.googleapis.com/Instance requireSSL Not Triggering

[jdyke]

gcp_sql_ssl_v1.yaml template:

Is looking for:

asset.resource.settings.ipConfiguration.requireSsl == false

but the CAI returns:

[
  {
    "name": "<redacted>",
    "asset_type": "sqladmin.googleapis.com/Instance",
    "ancestry_path": "<redacted>",
    "resource": {
      "version": "v1beta4",
      "discovery_document_uri": "https://www.googleapis.com/discovery/v1/apis/sqladmin/v1beta4/rest",
      "discovery_name": "DatabaseInstance",
      "parent": "<redacted>",
      "data": {
        "databaseVersion": "POSTGRES_9_6",
        "name": "master-instance",
        "project": "<redacted>",
        "region": "us-central1",
        "settings": {
          "ipConfiguration": {
            "ipv4Enabled": true,
            "requireSsl": false
          },
          "pricingPlan": "PER_USE",
          "replicationType": "SYNCHRONOUS",
          "storageAutoResize": true,
          "tier": "db-f1-micro"
        }
      }
    }
  }
]

It looks like it is missing "data" so I added it to the template:

asset.resource.data.settings.ipConfiguration.requireSsl == false

but that still does not catch the entry:

variable project {}

provider "google" {
  project = "${var.project}"
  region  = "us-east1"
}

resource "google_sql_database_instance" "master" {
  name = "master-instance"
  database_version = "POSTGRES_9_6"
  region = "us-central1"

  settings {
    # Second-generation instance tiers are based on the machine
    # type. See argument reference below.
    tier = "db-f1-micro"
    ip_configuration {
        ipv4_enabled = "true"
        require_ssl = "false"
    }
  }
}

[morgante]

Please test in Forseti. Entangling separate problems in one issue makes it harder to test.

[morgante]

@jdyke I opened #125 to address the issue in the constraint template.

I tested the conversion in Terraform Validator and it looks like it properly converts the requireSsl prop:

{
    "name": "//cloudsql.googleapis.com/projects/gcp-foundation-shared-devops/instances/master-instance",
    "asset_type": "sqladmin.googleapis.com/Instance",
    "ancestry_path": "organization/816421441114/project/gcp-foundation-shared-devops",
    "resource": {
        "version": "v1beta4",
        "discovery_document_uri": "https://www.googleapis.com/discovery/v1/apis/sqladmin/v1beta4/rest",
        "discovery_name": "DatabaseInstance",
        "parent": "//cloudresourcemanager.googleapis.com/projects/gcp-foundation-shared-devops",
        "data": {
            "databaseVersion": "POSTGRES_9_6",
            "name": "master-instance",
            "project": "gcp-foundation-shared-devops",
            "region": "us-central1",
            "settings": {
                "ipConfiguration": {
                    "ipv4Enabled": true,
                    "requireSsl": false
                },
                "pricingPlan": "PER_USE",
                "replicationType": "SYNCHRONOUS",
                "storageAutoResize": true,
                "tier": "db-f1-micro"
            }
        }
    }
}

[FanchenBao]

The same problem happens with gcp_sql_allowed_authorized_networks_v1.yaml.

For gcp_sql_allowed_authorized_networks_v1.yaml, In addition to the missing data field, the bug is in the package. The original statement is package templates.gcp.GCPSQLAllowedAuthorizedNetworksV1. When I change it into package templates.gcp.GCPSQLAllowedAuthorizedNetworksConstraintV1 (just add Constraint at the end before V1), it worked like a charm.

So I suspect for gcp_sql_ssl_v1.yaml, the same procedure shall work as well:

1. add data field
2. Change package templates.gcp.GCPSQLSSLV1 into package templates.gcp.GCPSQLSSLConstraintV1

I haven't tested it yet for gcp_sql_ssl_v1.yaml yet.

[morgante]

@FanchenBao Yes, I suspect you are correct. a PR is welcome, of course.

[ocervell]

@morgante @AdrienWalkowiak I've verified this rule works from config-validator, but is still not working from the Forseti scanner, which reports no violation. Even after #125 and #127

[morgante]

@ocervell Does it work with Scorecard?

[AdrienWalkowiak]

Any update on this @ocervell?

[gkowalski-google]

@AdrienWalkowiak @ocervell This was likely fixed for Forseti with this PR. Forseti had some incorrect logic where it was setting falsy values to None. The change can be tested from master branch of Forseti; will be included in v2.26.0.