Repository: tfsec/tfsec
Pull request #509 (2020-11-12 21:16:31)

Don't raise check if `node_config` is empty on `google_container_cluster`
Looking at the [docs for `google_container_cluster` and `node_config`](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#node_config), it says the `node_config` block shouldn't be used together with `google_container_node_pool`.  So, we probably don’t need to flag this check if the `node_config` is not defined in the `google_container_cluster` block since its likely they are not using the default node pool.

owenrumney: This comment comes with the caveat I have zero GCP -

would this check be better to include a verification that `remove_default_node_pool` is `true` and there is no `node_config` rather than assuming it isn't there because they've also got a `google_container_node_pool`?

maaslalani: Ahh that is a great suggestion. That would definitely make this check logic better! Let me add that :)

Patch:
diff --git a/internal/app/tfsec/checks/gcp012.go b/internal/app/tfsec/checks/gcp012.go
index 3e5c586bb..7a9ea1637 100644
--- a/internal/app/tfsec/checks/gcp012.go
+++ b/internal/app/tfsec/checks/gcp012.go
@@ -2,6 +2,7 @@ package checks
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/tfsec/tfsec/internal/app/tfsec/parser"
 	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
@@ -15,15 +16,15 @@ You should create and use a minimally privileged service account to run your GKE
 
 const GCPGKENodeServiceAccountBadExample = `
 resource "google_container_cluster" "my-cluster" {
-  node_config {
-  }
+	node_config {
+	}
 }
 `
 const GCPGKENodeServiceAccountGoodExample = `
 resource "google_container_cluster" "my-cluster" {
-  node_config {
-    service_account = "cool-service-account@example.com"
-  }
+	node_config {
+		service_account = "cool-service-account@example.com"
+	}
 }
 `
 
@@ -44,6 +45,10 @@ func init() {
 		RequiredLabels: []string{"google_container_cluster", "google_container_node_pool"},
 		CheckFunc: func(check *scanner.Check, block *parser.Block, _ *scanner.Context) []scanner.Result {
 
+			if strings.HasPrefix(block.Label(), "google_container_cluster") && block.GetAttribute("remove_default_node_pool").IsTrue() {
+				return nil
+			}
+
 			if !block.HasBlock("node_config") {
 				return []scanner.Result{
 					check.NewResult(


Test patch:
diff --git a/internal/app/tfsec/test/gcp012_test.go b/internal/app/tfsec/test/gcp012_test.go
index fa49618c9..159908572 100644
--- a/internal/app/tfsec/test/gcp012_test.go
+++ b/internal/app/tfsec/test/gcp012_test.go
@@ -16,17 +16,27 @@ func Test_GCPGKENodeServiceAccount(t *testing.T) {
 		mustExcludeResultCode scanner.RuleCode
 	}{
 		{
-			name: "does not define service account in container cluster",
+			name: "does not define service account in container cluster and uses default node pool",
 			source: `
 resource "google_container_cluster" "my-cluster" {
-  node_config {
-  }
+	remove_default_node_pool = false
+	node_config {
+	}
 }
 `,
 			mustIncludeResultCode: checks.GCPGKENodeServiceAccount,
 		},
 		{
-			name: "does not define node_config in container cluster",
+			name: "does not define service account in container cluster but removes default node pool",
+			source: `
+resource "google_container_cluster" "my-cluster" {
+	remove_default_node_pool = true
+}
+`,
+			mustExcludeResultCode: checks.GCPGKENodeServiceAccount,
+		},
+		{
+			name: "does not define node_config in container cluster and uses default node pool",
 			source: `
 resource "google_container_cluster" "my-cluster" {
 }
@@ -37,9 +47,9 @@ resource "google_container_cluster" "my-cluster" {
 			name: "defines service account in container cluster",
 			source: `
 resource "google_container_cluster" "my-cluster" {
-  node_config {
-    service_account = "anything"
-  }
+	node_config {
+		service_account = "anything"
+	}
 }
 `,
 			mustExcludeResultCode: checks.GCPGKENodeServiceAccount,
@@ -48,8 +58,8 @@ resource "google_container_cluster" "my-cluster" {
 			name: "does not define service account in container node pool",
 			source: `
 resource "google_container_node_pool" "my-np-cluster" {
-  node_config {
-  }
+	node_config {
+	}
 }
 `,
 			mustIncludeResultCode: checks.GCPGKENodeServiceAccount,
@@ -66,9 +76,9 @@ resource "google_container_node_pool" "my-np-cluster" {
 			name: "defines service account in container node pool",
 			source: `
 resource "google_container_node_pool" "my-np-cluster" {
-  node_config {
-    service_account = "anything"
-  }
+	node_config {
+		service_account = "anything"
+	}
 }
 `,
 			mustExcludeResultCode: checks.GCPGKENodeServiceAccount,