Merge pull request #382 from ComputeCanada/bastions

Use all potential bastion tags instead of only the first match

Author: cmd-ntrf

aws/infrastructure.tf

@@ -13,6 +13,7 @@ module "design" {
   pool           = var.pool
   volumes        = var.volumes
   firewall_rules = var.firewall_rules
+  bastion_tags   = var.bastion_tags
 }
 
 module "configuration" {
@@ -24,7 +25,7 @@ module "configuration" {
   sudoer_username = var.sudoer_username
   public_keys     = var.public_keys
   domain_name     = module.design.domain_name
-  bastion_tag     = module.design.bastion_tag
+  bastion_tags    = module.design.bastion_tags
   cluster_name    = var.cluster_name
   guest_passwd    = var.guest_passwd
   nb_users        = var.nb_users

azure/infrastructure.tf

@@ -13,6 +13,7 @@ module "design" {
   pool           = var.pool
   volumes        = var.volumes
   firewall_rules = var.firewall_rules
+  bastion_tags   = var.bastion_tags
 }
 
 module "configuration" {
@@ -24,7 +25,7 @@ module "configuration" {
   sudoer_username = var.sudoer_username
   public_keys     = var.public_keys
   domain_name     = module.design.domain_name
-  bastion_tag     = module.design.bastion_tag
+  bastion_tags    = module.design.bastion_tags
   cluster_name    = var.cluster_name
   guest_passwd    = var.guest_passwd
   nb_users        = var.nb_users

common/configuration/main.tf

@@ -24,7 +24,7 @@ variable "public_keys" {}
 
 variable "skip_upgrade" {}
 variable "puppetfile" {}
-variable "bastion_tag" {}
+variable "bastion_tags" {}
 
 resource "tls_private_key" "ssh" {
   algorithm = "ED25519"
@@ -162,7 +162,7 @@ output "ssh_key" {
 output "bastions" {
   value = {
     for host, values in local.final_inventory : host => values
-    if contains(values.tags, var.bastion_tag) && contains(values.tags, "public") && (!contains(values.tags, "pool"))
+    if length(setintersection(values.tags, var.bastion_tags)) > 0 && (!contains(values.tags, "pool"))
   }
 }
 

common/design/main.tf

@@ -52,32 +52,21 @@ locals {
 
   volume_per_instance = transpose({ for key, value in local.instance_per_volume : key => value["instances"] })
 
-  # We look for a firewall rule that allow SSH connection from the Terraform agent's ip
-  # and we memorize the corresponding tag so we can determine which instances can be used as a
+  # We look for firewall rules that allow SSH connection from the Terraform agent's ip
+  # and we memorize the corresponding tags so we can determine which instances can be used as a
   # first hop when transferring files or executing remote commands with Terraform.
-  # There are room for improvements, but we don't expect users to be very creative with
-  # firewall rules, so we are keeping the algorithm simple for now. One aspect
-  #  that could be improved:
-  # 1. We don't validate if the tag is actually present in any instance, we simply take the
-  # first value, so if there are more than one firewall rules that matches the criteria
-  # but only one that is actually active, we might select the wrong tag. It would be better
-  # to keep all firewall tags that match the criteria, then identify the intersection with
-  # instance tags and select any tag that matches.
   agent_ip = chomp(data.http.agent_ip.response_body)
-  bastion_tag = try(
-    element([
-      for rule, values in var.firewall_rules :
-      values.tag
-      if values.ethertype == "IPv4" &&
-      22 <= values.from_port && values.to_port <= 22 &&
-      alltrue([
-        for i, v in split(".", local.agent_ip) :
-        tonumber(split(".", strcontains(values.cidr, "/") ? cidrhost(values.cidr, 0) : values.cidr)[i]) <= tonumber(v) &&
-        tonumber(split(".", strcontains(values.cidr, "/") ? cidrhost(values.cidr, -1) : values.cidr)[i]) >= tonumber(v)
-      ])
-      ],
-    0),
-  "")
+  bastion_tags = distinct(concat(var.bastion_tags, [
+    for rule, values in var.firewall_rules :
+    values.tag
+    if values.ethertype == "IPv4" &&
+    22 <= values.from_port && values.to_port <= 22 &&
+    alltrue([
+      for i, v in split(".", local.agent_ip) :
+      tonumber(split(".", strcontains(values.cidr, "/") ? cidrhost(values.cidr, 0) : values.cidr)[i]) <= tonumber(v) &&
+      tonumber(split(".", strcontains(values.cidr, "/") ? cidrhost(values.cidr, -1) : values.cidr)[i]) >= tonumber(v)
+    ])
+  ]))
 }
 
 check "disk_space_per_tag" {

common/design/outputs.tf

@@ -18,6 +18,6 @@ output "domain_name" {
   value = local.domain_name
 }
 
-output "bastion_tag" {
-  value = local.bastion_tag
+output "bastion_tags" {
+  value = local.bastion_tags
 }

common/design/variables.tf

@@ -6,3 +6,4 @@ variable "pool" {}
 variable "firewall_rules" {}
 variable "min_disk_size" {}
 variable "image" {}
+variable "bastion_tags" {}

common/provision/main.tf

@@ -1,4 +1,9 @@
-variable "configuration" {}
+variable "configuration" {
+  validation {
+    condition     = length(var.configuration.bastions) > 0
+    error_message = "Firewall rules do not allow Terraform to SSH to at least one instance."
+  }
+}
 variable "hieradata" {}
 variable "hieradata_dir" {}
 variable "eyaml_key" {}
@@ -54,13 +59,17 @@ data "archive_file" "puppetserver_files" {
   }
 }
 
+locals {
+  bastion_host = length(var.configuration.bastions) > 0 ? var.configuration.bastions[keys(var.configuration.bastions)[0]] : null
+}
+
 resource "terraform_data" "deploy_puppetserver_files" {
-  for_each = length(var.configuration.bastions) > 0 ? var.configuration.puppetservers : {}
+  for_each = local.bastion_host != null ? var.configuration.puppetservers : {}
 
   connection {
     type                = "ssh"
     agent               = false
-    bastion_host        = var.configuration.bastions[keys(var.configuration.bastions)[0]].public_ip
+    bastion_host        = contains(local.bastion_host.tags, "public") ? local.bastion_host.public_ip : local.bastion_host.local_ip
     bastion_user        = "tf"
     bastion_private_key = var.configuration.ssh_key.private
     user                = "tf"

common/variables.tf

@@ -169,3 +169,9 @@ variable "eyaml_key" {
     error_message = "Unsupported private key format"
   }
 }
+
+variable "bastion_tags" {
+  type        = list(string)
+  default     = []
+  description = "Tags identifying instances that can be used by Terraform as the first hop to transfer files to the Puppet server."
+}

docs/README.md

@@ -964,6 +964,16 @@ install complementary modules with [r10k](https://github.com/puppetlabs/r10k).
 **Post build modification effect**: trigger scp of Puppetfile at next `terraform apply`.
 Each instance's Puppet agent will be reloaded following the installation of the new modules.
 
+### 4.22 bastion_tags (optional)
+
+**default_value** = `[]`
+
+Defines a list of tags identifying instances that can be used by Terraform as the first hop
+to transfer files to the Puppet server. By default, this list is infered from the list of
+[firewall rules](#416-firewall_rules-optional) and the public ip address of the agent calling
+`terraform apply`. Providing an explicit list of tags allow to bypass the firewall rule inference,
+which can be useful when the agent is in the same network as the cluster.
+
 ## 5. Cloud Specific Configuration
 
 ### 5.1 Amazon Web Services

examples/incus/main.tf

@@ -13,10 +13,16 @@ module "incus" {
 
   instances = {
     mgmt  = { type = "container", cpus = 4, ram = 6000, gpus = 0, tags = ["puppet", "mgmt", "nfs"], count = 1 }
-    login = { type = "container", cpus = 2, ram = 3000, gpus = 0, tags = ["login", "public", "proxy"], count = 1 }
+    login = { type = "container", cpus = 2, ram = 3000, gpus = 0, tags = ["login", "proxy"], count = 1 }
     node  = { type = "container", cpus = 2, ram = 3000, gpus = 0, tags = ["node"], count = 1 }
   }
 
+  firewall_rules = {
+    http  = { "from_port" = 80, "to_port" = 80, tag = "proxy", "cidr" = "0.0.0.0/0" },
+    https = { "from_port" = 443, "to_port" = 443, tag = "proxy", "cidr" = "0.0.0.0/0" },
+  }
+  bastion_tags = ["login"]
+
   volumes = {}
 
   public_keys = []