Merge pull request #123 from SKozlovsky/bugfix/firewall-allow-internal

Fix bug for firewall allow-internal rules

Author: aaron-lane

examples/submodule_firewall/main.tf

@@ -48,21 +48,96 @@ module "test-vpc-module" {
   ]
 }
 
+// Custom firewall rules
+locals {
+  custom_rules = {
+    // Example of custom tcp/udp rule
+    deny-ingress-6534-6566 = {
+      description          = "Deny all INGRESS to port 6534-6566"
+      direction            = "INGRESS"
+      action               = "deny"
+      ranges               = ["0.0.0.0/0"] # source or destination ranges (depends on `direction`)
+      use_service_accounts = false         # if `true` targets/sources expect list of instances SA, if false - list of tags
+      targets              = null          # target_service_accounts or target_tags depends on `use_service_accounts` value
+      sources              = null          # source_service_accounts or source_tags depends on `use_service_accounts` value
+      rules = [{
+        protocol = "tcp"
+        ports    = ["6534-6566"]
+        },
+        {
+          protocol = "udp"
+          ports    = ["6534-6566"]
+      }]
+
+      extra_attributes = {
+        disabled = true
+        priority = 95
+      }
+    }
+
+    // Example how to allow connection from instances with `backend` tag, to instances with `databases` tag
+    allow-backend-to-databases = {
+      description          = "Allow backend nodes connection to databases instances"
+      direction            = "INGRESS"
+      action               = "allow"
+      ranges               = null
+      use_service_accounts = false
+      targets              = ["databases"] # target_tags
+      sources              = ["backed"]    # source_tags
+      rules = [{
+        protocol = "tcp"
+        ports    = ["3306", "5432", "1521", "1433"]
+      }]
+
+      extra_attributes = {}
+    }
+
+    // Example how to allow connection from an instance with a given service account
+    allow-all-admin-sa = {
+      description          = "Allow all traffic from admin sa instances"
+      direction            = "INGRESS"
+      action               = "allow"
+      ranges               = null
+      use_service_accounts = true
+      targets              = null
+      sources              = ["[email protected]"]
+      rules = [{
+        protocol = "tcp"
+        ports    = null # all ports
+        },
+        {
+          protocol = "udp"
+          ports    = null # all ports
+        }
+      ]
+      extra_attributes = {
+        priority = 30
+      }
+    }
+  }
+}
+
+
+
 module "test-firewall-submodule" {
   source                  = "../../modules/fabric-net-firewall"
   project_id              = var.project_id
   network                 = module.test-vpc-module.network_name
   internal_ranges_enabled = true
   internal_ranges         = module.test-vpc-module.subnets_ips
 
-  internal_allow = [{
-    protocol = "icmp"
+  internal_allow = [
+    {
+      protocol = "icmp"
     },
     {
-      protocol = "tcp"
+      protocol = "tcp",
+      ports    = ["8080", "1000-2000"]
     },
     {
       protocol = "udp"
+      # all ports will be opened if `ports` key isn't specified
     },
   ]
+  custom_rules = local.custom_rules
 }

modules/fabric-net-firewall/main.tf

@@ -27,14 +27,24 @@ resource "google_compute_firewall" "allow-internal" {
   source_ranges = var.internal_ranges
 
   dynamic "allow" {
-    for_each = [var.internal_allow]
+    for_each = [for rule in var.internal_allow :
+      {
+        protocol = lookup(rule, "protocol", null)
+        ports    = lookup(rule, "ports", null)
+      }
+    ]
     content {
-      protocol = lookup(allow.value[count.index], "protocol", null)
-      ports    = lookup(allow.value[count.index], "ports", null)
+      protocol = allow.value.protocol
+      ports    = allow.value.ports
     }
   }
+
 }
 
+
+
+
+
 resource "google_compute_firewall" "allow-admins" {
   count         = var.admin_ranges_enabled == true ? 1 : 0
   name          = "${var.network}-ingress-admins"

test/integration/submodule_firewall/controls/gcloud.rb

@@ -38,6 +38,148 @@
       end
     end
 
+    describe "allowed internal rules" do
+      it "should contain ICMP rule" do
+        expect(data["allowed"]).to include({"IPProtocol" => "icmp"})
+      end
+
+      it "should contain UDP rule" do
+        expect(data["allowed"]).to include({"IPProtocol" => "udp"})
+      end
+
+      it "should contain TCP rule" do
+        expect(data["allowed"]).to include({"IPProtocol"=>"tcp", "ports"=>["8080", "1000-2000"]})
+      end
+    end
+  end
+
+  # Custom rules
+  describe command("gcloud compute firewall-rules describe allow-backend-to-databases --project=#{project_id} --format=json") do
+    its(:exit_status) { should eq 0 }
+    its(:stderr) { should eq '' }
+
+    let(:data) do
+      if subject.exit_status == 0
+        JSON.parse(subject.stdout)
+      else
+        {}
+      end
+    end
+
+    describe "Custom TAG rule" do
+      it "has backend tag as source" do
+        expect(data).to include(
+          "sourceTags" => ["backed"]
+        )
+      end
+
+      it "has databases tag as target" do
+        expect(data).to include(
+          "targetTags" => ["databases"]
+        )
+      end
+
+      it "has expected TCP rule" do
+        expect(data["allowed"]).to include(
+            {
+              "IPProtocol" => "tcp",
+              "ports" => ["3306", "5432", "1521", "1433"]
+            }
+        )
+      end
+    end
+  end
+
+describe command("gcloud compute firewall-rules describe deny-ingress-6534-6566 --project=#{project_id} --format=json") do
+    its(:exit_status) { should eq 0 }
+    its(:stderr) { should eq '' }
+
+    let(:data) do
+      if subject.exit_status == 0
+        JSON.parse(subject.stdout)
+      else
+        {}
+      end
+    end
+
+    describe "deny-ingress-6534-6566" do
+      it "should be disabled" do
+        expect(data).to include(
+          "disabled" => true
+        )
+      end
+
+      it "has 0.0.0.0/0 source range" do
+        expect(data).to include(
+          "sourceRanges" => ["0.0.0.0/0"]
+        )
+      end
+
+      it "has expected TCP rules" do
+        expect(data["denied"]).to include(
+            {
+              "IPProtocol" => "tcp",
+              "ports" => ["6534-6566"]
+            }
+        )
+      end
+
+      it "has expected UDP rules" do
+        expect(data["denied"]).to include(
+            {
+              "IPProtocol" => "udp",
+              "ports" => ["6534-6566"]
+            }
+        )
+      end
+    end
+  end
+
+
+describe command("gcloud compute firewall-rules describe allow-all-admin-sa --project=#{project_id} --format=json") do
+    its(:exit_status) { should eq 0 }
+    its(:stderr) { should eq '' }
+
+    let(:data) do
+      if subject.exit_status == 0
+        JSON.parse(subject.stdout)
+      else
+        {}
+      end
+    end
+
+    describe "allow-all-admin-sa" do
+      it "should be enabled" do
+        expect(data).to include(
+          "disabled" => false
+        )
+      end
+
+      it "should has correct source SA" do
+        expect(data["sourceServiceAccounts"]).to eq(["[email protected]"])
+      end
+
+      it "should has priority 30" do
+        expect(data["priority"]).to eq(30)
+      end
+
+      it "has expected TCP rules" do
+        expect(data["allowed"]).to include(
+            {
+              "IPProtocol" => "tcp"
+            }
+        )
+      end
+
+      it "has expected UDP rules" do
+        expect(data["allowed"]).to include(
+            {
+              "IPProtocol" => "udp"
+            }
+        )
+      end
+    end
   end
 
 end
+

test/integration/submodule_firewall/controls/gcp.rb

@@ -24,6 +24,9 @@
     its('firewall_names') { should include "#{network_name}-ingress-tag-https" }
     its('firewall_names') { should include "#{network_name}-ingress-tag-ssh" }
     its('firewall_names') { should_not include "default-ingress-admins" }
+    its('firewall_names') { should include "deny-ingress-6534-6566" }
+    its('firewall_names') { should include "allow-backend-to-databases" }
+    its('firewall_names') { should include "allow-all-admin-sa" }
   end
 
 end