Merge pull request #517 from Facets-cloud/feat/ngf-241-chart-hooks

unni-facets · web-flow · commit fcfdbb5f6bf0 · 2026-02-26T14:20:00.000+05:30
feat: port custom hooks to NGF 2.4.1 chart
diff --git a/modules/ingress/nginx_gateway_fabric_legacy/1.0/charts/nginx-gateway-fabric-2.4.1.tgz b/modules/ingress/nginx_gateway_fabric_legacy/1.0/charts/nginx-gateway-fabric-2.4.1.tgz
diff --git a/modules/ingress/nginx_gateway_fabric_legacy/1.0/facets.yaml b/modules/ingress/nginx_gateway_fabric_legacy/1.0/facets.yaml
@@ -298,22 +298,32 @@ spec:
               type: string
               title: Path
               description: Path of the application (required for HTTP routes)
-              pattern: ^(/[^/]+)*(/)?$
-              x-ui-placeholder: Enter path (e.g., / or /api)
-              x-ui-error-message: "Value doesn't match pattern, eg: / or /api"
+              pattern: ^(/[^\s{};^]*)$
+              x-ui-placeholder: "Enter path (e.g., / or /api or /api/v[0-9]+)"
+              x-ui-error-message: "Path must start with /"
               x-ui-visible-if:
                 field: spec.rules.{{this}}.grpc_config.enabled
                 values:
                   - false
                   - null
+            disable_auth:
+              type: boolean
+              title: Disable Auth for this Route
+              description: Disable basic authentication for this specific route (only relevant when basic_auth is enabled)
+              default: false
+              x-ui-visible-if:
+                field: spec.basic_auth
+                values:
+                  - true
             path_type:
               type: string
               title: Path Type
-              description: "Path matching type. PathPrefix (default) matches paths starting with the specified prefix. Use Exact for exact path matching."
+              description: "Path matching type. RegularExpression (default) matches paths using regex for ingress-nginx parity. Use PathPrefix for prefix matching or Exact for exact path matching."
               enum:
-                - Exact
+                - RegularExpression
                 - PathPrefix
-              default: PathPrefix
+                - Exact
+              default: RegularExpression
               x-ui-visible-if:
                 field: spec.rules.{{this}}.grpc_config.enabled
                 values:
@@ -778,6 +788,7 @@ spec:
             - namespace
           x-ui-order:
             - disable
+            - disable_auth
             - domain_prefix
             - service_name
             - namespace
@@ -800,6 +811,11 @@ spec:
       type: boolean
       title: Force SSL Redirection
       description: Force HTTP to HTTPS redirection
+    basic_auth:
+      type: boolean
+      title: Basic Authentication
+      description: Enable/disable basic auth for all routes (individual routes can opt out with disable_auth)
+      default: false
     body_size:
       type: string
       title: Max Body Size
@@ -841,6 +857,7 @@ spec:
     - disable_base_domain
     - domains
     - force_ssl_redirection
+    - basic_auth
     - body_size
     - disable_endpoint_validation
     - data_plane
@@ -859,6 +876,7 @@ sample:
     private: false
     disable_base_domain: false
     force_ssl_redirection: true
+    basic_auth: false
     body_size: 150m
     data_plane:
       scaling:
diff --git a/modules/ingress/nginx_gateway_fabric_legacy/1.0/main.tf b/modules/ingress/nginx_gateway_fabric_legacy/1.0/main.tf
@@ -238,8 +238,10 @@ locals {
     }
   } : {}
 
-  # HTTPRoute Resources (HTTPS traffic - port 443)
+  # HTTPRoute Resources (HTTPS traffic - port 443, and HTTP - port 80 when force_ssl_redirection is disabled)
   # Note: GatewayClass, Gateway, and NginxProxy are created by the Helm chart
+  force_ssl_redirection = lookup(var.instance.spec, "force_ssl_redirection", false)
+
   httproute_resources = {
     for k, v in local.rulesFiltered : "httproute-${lower(var.instance_name)}-${k}" => {
       apiVersion = "gateway.networking.k8s.io/v1"
@@ -252,21 +254,30 @@ locals {
         # Reference the correct listener(s) for this route's hostnames
         # If route has domain_prefix, reference the additional hostname listeners
         # If route has no domain_prefix, reference the base domain listeners
-        parentRefs = lookup(v, "domain_prefix", null) == null || lookup(v, "domain_prefix", null) == "" ? [
-          # No domain_prefix - use base domain listeners
-          for domain_key, domain in local.domains : {
-            name        = local.name
-            namespace   = var.environment.namespace
-            sectionName = "https-${domain_key}"
-          }
-          ] : [
-          # Has domain_prefix - use additional hostname listeners
-          for domain_key, domain in local.domains : {
+        # When force_ssl_redirection is disabled, also attach to HTTP listener so traffic is served on port 80
+        parentRefs = concat(
+          lookup(v, "domain_prefix", null) == null || lookup(v, "domain_prefix", null) == "" ? [
+            # No domain_prefix - use base domain listeners
+            for domain_key, domain in local.domains : {
+              name        = local.name
+              namespace   = var.environment.namespace
+              sectionName = "https-${domain_key}"
+            }
+            ] : [
+            # Has domain_prefix - use additional hostname listeners
+            for domain_key, domain in local.domains : {
+              name        = local.name
+              namespace   = var.environment.namespace
+              sectionName = "https-${replace(replace("${lookup(v, "domain_prefix", null)}.${domain.domain}", ".", "-"), "*", "wildcard")}"
+            }
+          ],
+          # Also attach to HTTP listener when SSL redirection is disabled
+          !local.force_ssl_redirection ? [{
             name        = local.name
             namespace   = var.environment.namespace
-            sectionName = "https-${replace(replace("${lookup(v, "domain_prefix", null)}.${domain.domain}", ".", "-"), "*", "wildcard")}"
-          }
-        ]
+            sectionName = "http"
+          }] : []
+        )
 
         # Include all domains in hostnames - Gateway API supports multiple hostnames per route
         hostnames = distinct([
@@ -282,7 +293,7 @@ locals {
             [merge(
               {
                 path = {
-                  type  = lookup(v, "path_type", "PathPrefix")
+                  type  = lookup(v, "path_type", "RegularExpression")
                   value = lookup(v, "path", "/")
                 }
               },
@@ -314,6 +325,15 @@ locals {
           )
 
           filters = concat(
+            # Basic auth filter (applied when basic_auth is enabled and route doesn't have disable_auth)
+            lookup(var.instance.spec, "basic_auth", false) && !lookup(v, "disable_auth", false) ? [{
+              type = "ExtensionRef"
+              extensionRef = {
+                group = "gateway.nginx.org"
+                kind  = "AuthenticationFilter"
+                name  = "${local.name}-basic-auth"
+              }
+            }] : [],
             # Static filters
             [
               for filter in [
@@ -432,21 +452,30 @@ locals {
         # Reference the correct listener(s) for this route's hostnames
         # If route has domain_prefix, reference the additional hostname listeners
         # If route has no domain_prefix, reference the base domain listeners
-        parentRefs = lookup(v, "domain_prefix", null) == null || lookup(v, "domain_prefix", null) == "" ? [
-          # No domain_prefix - use base domain listeners
-          for domain_key, domain in local.domains : {
-            name        = local.name
-            namespace   = var.environment.namespace
-            sectionName = "https-${domain_key}"
-          }
-          ] : [
-          # Has domain_prefix - use additional hostname listeners
-          for domain_key, domain in local.domains : {
+        # When force_ssl_redirection is disabled, also attach to HTTP listener
+        parentRefs = concat(
+          lookup(v, "domain_prefix", null) == null || lookup(v, "domain_prefix", null) == "" ? [
+            # No domain_prefix - use base domain listeners
+            for domain_key, domain in local.domains : {
+              name        = local.name
+              namespace   = var.environment.namespace
+              sectionName = "https-${domain_key}"
+            }
+            ] : [
+            # Has domain_prefix - use additional hostname listeners
+            for domain_key, domain in local.domains : {
+              name        = local.name
+              namespace   = var.environment.namespace
+              sectionName = "https-${replace(replace("${lookup(v, "domain_prefix", null)}.${domain.domain}", ".", "-"), "*", "wildcard")}"
+            }
+          ],
+          # Also attach to HTTP listener when SSL redirection is disabled
+          !local.force_ssl_redirection ? [{
             name        = local.name
             namespace   = var.environment.namespace
-            sectionName = "https-${replace(replace("${lookup(v, "domain_prefix", null)}.${domain.domain}", ".", "-"), "*", "wildcard")}"
-          }
-        ]
+            sectionName = "http"
+          }] : []
+        )
 
         # Include all domains in hostnames - Gateway API supports multiple hostnames per route
         hostnames = distinct([
@@ -468,6 +497,16 @@ locals {
             }
           ] : []
 
+          # Basic auth filter (applied when basic_auth is enabled and route doesn't have disable_auth)
+          filters = lookup(var.instance.spec, "basic_auth", false) && !lookup(v, "disable_auth", false) ? [{
+            type = "ExtensionRef"
+            extensionRef = {
+              group = "gateway.nginx.org"
+              kind  = "AuthenticationFilter"
+              name  = "${local.name}-basic-auth"
+            }
+          }] : []
+
           backendRefs = [{
             name      = v.service_name
             port      = tonumber(v.port)
@@ -571,14 +610,36 @@ locals {
     }
   }
 
+  # AuthenticationFilter for basic auth (NGF native CRD)
+  authenticationfilter_resources = lookup(var.instance.spec, "basic_auth", false) ? {
+    "authfilter-${local.name}" = {
+      apiVersion = "gateway.nginx.org/v1alpha1"
+      kind       = "AuthenticationFilter"
+      metadata = {
+        name      = "${local.name}-basic-auth"
+        namespace = var.environment.namespace
+      }
+      spec = {
+        type = "Basic"
+        basic = {
+          realm = "Authentication required"
+          secretRef = {
+            name = "${local.name}-basic-auth"
+          }
+        }
+      }
+    }
+  } : {}
+
   # Merge all Gateway API resources
   gateway_api_resources = merge(
     local.http_redirect_resources,
     local.httproute_resources,
     local.grpcroute_resources,
     local.podmonitor_resources,
     local.referencegrant_resources,
-    local.clientsettingspolicy_resources
+    local.clientsettingspolicy_resources,
+    local.authenticationfilter_resources
   )
 }
 
@@ -808,7 +869,7 @@ module "http01_certificate_additional" {
 resource "helm_release" "nginx_gateway_fabric" {
   name             = "${local.name}-nginx-fabric"
   wait             = lookup(var.instance.spec, "helm_wait", true)
-  chart            = "${path.module}/charts/nginx-gateway-fabric-2.3.0.tgz"
+  chart            = "${path.module}/charts/nginx-gateway-fabric-2.4.1.tgz"
   namespace        = var.environment.namespace
   max_history      = 10
   skip_crds        = false
@@ -835,7 +896,7 @@ resource "helm_release" "nginx_gateway_fabric" {
 
         image = {
           repository = "facetscloud/nginx-gateway-fabric"
-          tag        = "2.3.0"
+          tag        = "v2.4.1"
           pullPolicy = "IfNotPresent"
         }
         imagePullSecrets = lookup(var.inputs, "artifactories", null) != null ? var.inputs.artifactories.attributes.registry_secrets_list : []
@@ -1097,36 +1158,39 @@ module "gateway_api_resources" {
   resources_data  = local.gateway_api_resources
   advanced_config = {}
 
-  depends_on = [helm_release.nginx_gateway_fabric]
+  depends_on = [helm_release.nginx_gateway_fabric, kubernetes_secret.basic_auth]
+}
+
+# Basic Authentication using NGF AuthenticationFilter CRD
+# NGF 2.4.1 supports native basic auth via AuthenticationFilter (gateway.nginx.org/v1alpha1)
+# When basic_auth is enabled: auto-generates credentials, creates htpasswd Secret,
+# and applies AuthenticationFilter to all HTTPRoute rules (per-rule disable_auth to exempt)
+
+resource "random_string" "basic_auth_password" {
+  count   = lookup(var.instance.spec, "basic_auth", false) ? 1 : 0
+  length  = 10
+  special = false
 }
 
-# Basic Authentication
-# NOTE: Basic auth is not natively supported in NGINX Gateway Fabric.
-# Unlike ingress-nginx, NGF doesn't have auth annotations.
-# Implementation would require SnippetsFilter + volume mounts which is complex and fragile.
-# TODO: Implement when NGF adds native policy support or use app-level auth.
-#
-# resource "random_string" "basic_auth_password" {
-#   count   = lookup(var.instance.spec, "basic_auth", false) ? 1 : 0
-#   length  = 16
-#   special = true
-# }
-#
-# resource "kubernetes_secret" "basic_auth" {
-#   count = lookup(var.instance.spec, "basic_auth", false) ? 1 : 0
-#
-#   metadata {
-#     name      = "${local.name}-basic-auth"
-#     namespace = var.environment.namespace
-#   }
-#
-#   data = {
-#     username = "${var.instance_name}-user"
-#     password = random_string.basic_auth_password[0].result
-#   }
-#
-#   type = "Opaque"
-# }
+resource "kubernetes_secret" "basic_auth" {
+  count = lookup(var.instance.spec, "basic_auth", false) ? 1 : 0
+
+  metadata {
+    name      = "${local.name}-basic-auth"
+    namespace = var.environment.namespace
+  }
+
+  data = {
+    auth = "${var.instance_name}user:${bcrypt(random_string.basic_auth_password[0].result)}"
+  }
+
+  type = "nginx.org/htpasswd"
+
+  lifecycle {
+    ignore_changes        = [data]
+    create_before_destroy = true
+  }
+}
 
 # Load Balancer Service Discovery
 # Note: The LoadBalancer service is created by NGINX Gateway Fabric controller
diff --git a/modules/ingress/nginx_gateway_fabric_legacy/1.0/outputs.tf b/modules/ingress/nginx_gateway_fabric_legacy/1.0/outputs.tf
@@ -1,8 +1,7 @@
 locals {
-  # Basic auth is not supported in NGINX Gateway Fabric (see main.tf)
-  # username        = lookup(var.instance.spec, "basic_auth", false) && length(random_string.basic_auth_password) > 0 ? "${var.instance_name}-user" : ""
-  # password        = lookup(var.instance.spec, "basic_auth", false) && length(random_string.basic_auth_password) > 0 ? random_string.basic_auth_password[0].result : ""
-  # is_auth_enabled = length(local.username) > 0 && length(local.password) > 0 ? true : false
+  username        = lookup(var.instance.spec, "basic_auth", false) ? "${var.instance_name}user" : ""
+  password        = lookup(var.instance.spec, "basic_auth", false) && length(random_string.basic_auth_password) > 0 ? random_string.basic_auth_password[0].result : ""
+  is_auth_enabled = length(local.username) > 0 && length(local.password) > 0
 
   output_attributes = merge(
     {
@@ -27,13 +26,12 @@ locals {
 
   output_interfaces = {
     for route_key, route in local.rulesFiltered : route_key => {
-      connection_string = "https://${route.host}"
+      connection_string = local.is_auth_enabled ? "https://${local.username}:${local.password}@${route.host}" : "https://${route.host}"
       host              = route.host
       port              = 443
-      # Basic auth not supported - username/password removed
-      # username          = local.username
-      # password          = local.password
-      secrets = []
+      username          = local.username
+      password          = local.password
+      secrets           = local.is_auth_enabled ? ["connection_string", "password"] : []
     }
   }
 }
