[eks/echo-server] Deprecate ALB controller specific echo-server (cloudposse/terraform-aws-components#893)

Author: Nuru

src/CHANGELOG.md

@@ -0,0 +1,22 @@
+## Changes in PR #893, components version ~v1.337.0
+
+- Moved `eks/echo-server` v1.147.0 to `/deprecated/eks/echo-server` for those
+who still need it and do not want to switch. It may later become the basis
+for an example app or something similar.
+- Removed dependency on and connection to the `eks/alb-controller-ingress-group` component
+- Added liveness probe, and disabled logging of probe requests. Probe request
+logging can be restored by setting `livenessProbeLogging: true` in `chart_values`
+- This component no longer configures automatic redirects from HTTP to HTTPS. This
+is because for ALB controller, setting that on one ingress sets it for all
+ingresses in the same IngressGroup, and it is a design goal that deploying
+this component does not affect other Ingresses (with the obvious exception
+of possibly being the first to create the Application Load Balancer).
+- Removed from `chart_values`:`ingress.nginx.class` (was set to "nginx") and
+`ingress.alb.class` (was set to "alb"). IngressClass should usually not be set,
+as this component is intended to be used to test the defaults, including the
+default IngressClass. However, if you do want to set it, you can do so by
+setting `ingress.class` in `chart_values`.
+- Removed the deprecated `kubernetes.io/ingress.class` annotation by default.
+It can be restored by setting `ingress.use_ingress_class_annotation: true` in `chart_values`.
+IngressClass is now set using the preferred `ingressClassName` field of the
+Ingress resource.

src/README.md

@@ -5,17 +5,26 @@ This is copied from [cloudposse/terraform-aws-components](https://github.com/clo
 This component installs the [Ealenn/Echo-Server](https://github.com/Ealenn/Echo-Server) to EKS clusters.
 The echo server is a server that sends it back to the client a JSON representation of all the data
 the server received, which is a combination of information sent by the client and information sent
-by the web server infrastructure. For further details, please see [Echo-Server documentation](https://ealenn.github.io/Echo-Server/).
+by the web server infrastructure. For further details, please consult the [Echo-Server documentation](https://ealenn.github.io/Echo-Server/).
 
 ## Prerequisites
 
-Echo server is intended to provide end-to-end testing of everything needed to deploy an application or service with a public HTTPS endpoint.
-Therefore, it requires several other components.
+Echo server is intended to provide end-to-end testing of everything needed
+to deploy an application or service with a public HTTPS endpoint. It uses
+defaults where possible, such as using the default IngressClass, in order
+to verify that the defaults are sufficient for a typical application.
 
-At the moment, it supports 2 configurations:
+In order to minimize the impact of the echo server on the rest of the cluster,
+it does not set any configuration that would affect other ingresses, such
+as WAF rules, logging, or redirecting HTTP to HTTPS. Those settings should
+be configured in the IngressClass where possible.
+
+Therefore, it requires several other components. At the moment, it supports 2 configurations:
 
 1. ALB with ACM Certificate
   - AWS Load Balancer Controller (ALB) version 2.2.0 or later, with ACM certificate auto-discovery enabled
+  - A default IngressClass, which can be provisioned by the `alb-controller` component as part of deploying
+    the controller, or can be provisioned separately, for example by the `alb-controller-ingress-class` component.
   - Pre-provisioned ACM TLS certificate covering the provisioned host name (typically a wildcard certificate covering all hosts in the domain)
 2. Nginx with Cert Manager Certificate
   - Nginx (via `kubernetes/ingress-nginx` controller). We recommend `ingress-nginx` v1.1.0 or later, but `echo-server`
@@ -24,6 +33,7 @@ At the moment, it supports 2 configurations:
     (by default, named `letsEncrypt-prod`).
 
 In both configurations, it has these common requirements:
+- EKS component deployed, with component name specified in `eks_component_name` (defaults to "eks/cluster")
 - Kubernetes version 1.19 or later
 - Ingress API version `networking.k8s.io/v1`
 - [kubernetes-sigs/external-dns](https://github.com/kubernetes-sigs/external-dns)
@@ -43,12 +53,31 @@ a "plan" file.
 
 Use this in the catalog or use these variables to overwrite the catalog values.
 
+Set `ingress_type` to "alb" if using `alb-controller` or "nginx" if using `ingress-nginx`.
+
+Normally, you should not set the IngressClass or IngressGroup, as this component is intended to test the defaults.
+However, if you need to, set them in `chart_values`:
+```yaml
+chart_values:
+  ingress:
+    class: "other-ingress-class"
+    alb:
+      # IngressGroup is specific to alb-controller
+      group_name: "other-ingress-group"
+```
+
+Note that if you follow recommendations and do not set the ingress class name,
+the deployed Ingress will have the ingressClassName setting injected by the
+Ingress controller, set to the then-current default. This means that if later
+you change the default IngressClass, the Ingress will be NOT be updated to use
+the new default. Furthermore, because of limitations in the Helm provider, this
+will not be detected as drift. You will need to destroy and re-deploy the
+echo server to update the Ingress to the new default.
+
 ```yaml
 components:
   terraform:
-    eks/echo-server:
-      metadata:
-        component: eks/echo-server
+    echo-server:
       settings:
         spacelift:
           workspace_enabled: true
@@ -63,11 +92,15 @@ components:
         atomic: true
         cleanup_on_fail: true
 
-        ingress_type: "alb"
+        ingress_type: "alb" # or "nginx"
         # %[1]v is the tenant name, %[2]v is the stage name, %[3]v is the region name
         hostname_template: "echo.%[3]v.%[2]v.%[1]v.sample-domain.net"
 ```
 
+In rare cases where some ingress controllers do not support the `ingressClassName` field,
+you can restore the old `kubernetes.io/ingress.class` annotation by setting
+`ingress.use_ingress_class_annotation: true` in `chart_values`.
+
 <!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
 ## Requirements
 
@@ -88,8 +121,7 @@ components:
 
 | Name | Source | Version |
 |------|--------|---------|
-| <a name="module_alb"></a> [alb](#module\_alb) | cloudposse/stack-config/yaml//modules/remote-state | 1.5.0 |
-| <a name="module_echo_server"></a> [echo\_server](#module\_echo\_server) | cloudposse/helm-release/aws | 0.10.0 |
+| <a name="module_echo_server"></a> [echo\_server](#module\_echo\_server) | cloudposse/helm-release/aws | 0.10.1 |
 | <a name="module_eks"></a> [eks](#module\_eks) | cloudposse/stack-config/yaml//modules/remote-state | 1.5.0 |
 | <a name="module_iam_roles"></a> [iam\_roles](#module\_iam\_roles) | ../../account-map/modules/iam-roles | n/a |
 | <a name="module_this"></a> [this](#module\_this) | cloudposse/label/null | 0.25.0 |
@@ -105,7 +137,6 @@ components:
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
 | <a name="input_additional_tag_map"></a> [additional\_tag\_map](#input\_additional\_tag\_map) | Additional key-value pairs to add to each map in `tags_as_list_of_maps`. Not added to `tags` or `id`.<br>This is for some rare cases where resources want additional configuration of tags<br>and therefore take a list of maps with tag key, value, and additional configuration. | `map(string)` | `{}` | no |
-| <a name="input_alb_controller_ingress_group_component_name"></a> [alb\_controller\_ingress\_group\_component\_name](#input\_alb\_controller\_ingress\_group\_component\_name) | The name of the alb\_controller\_ingress\_group component | `string` | `"eks/alb-controller-ingress-group"` | no |
 | <a name="input_atomic"></a> [atomic](#input\_atomic) | If set, installation process purges chart on fail. The wait flag will be set automatically if atomic is used. | `bool` | `true` | no |
 | <a name="input_attributes"></a> [attributes](#input\_attributes) | ID element. Additional attributes (e.g. `workers` or `cluster`) to add to `id`,<br>in the order they appear in the list. New attributes are appended to the<br>end of the list. The elements of the list are joined by the `delimiter`<br>and treated as a single ID element. | `list(string)` | `[]` | no |
 | <a name="input_chart_values"></a> [chart\_values](#input\_chart\_values) | Addition map values to yamlencode as `helm_release` values. | `any` | `{}` | no |
@@ -154,6 +185,7 @@ components:
 
 | Name | Description |
 |------|-------------|
+| <a name="output_hostname"></a> [hostname](#output\_hostname) | Hostname of the deployed echo server |
 | <a name="output_metadata"></a> [metadata](#output\_metadata) | Block status of the deployed release |
 <!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
 

src/charts/echo-server/Chart.yaml

@@ -15,10 +15,10 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 0.3.0
+version: 0.4.0
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
 # It is recommended to use it with quotes.
-appVersion: "0.3.0"
+appVersion: "0.8.0"

src/charts/echo-server/templates/deployment.yaml

@@ -24,7 +24,31 @@ spec:
           args:
           # Disable the feature that turns the echo server into a file browser on the server (security risk)
           - "--enable:file=false"
+          {{- if eq (printf "%v" .Values.livenessProbeLogging) "false" }}
+          - "--logs:ignore:ping=true"
+          {{- end }}
           ports:
             - name: http
               containerPort: 80
               protocol: TCP
+          livenessProbe:
+            httpGet:
+              port: http
+              path: /ping
+              httpHeaders:
+                - name: x-echo-code
+                  value: "200"
+            initialDelaySeconds: 5
+            periodSeconds: 10
+            timeoutSeconds: 2
+            failureThreshold: 3
+            successThreshold: 1
+          {{- with index .Values "resources" }}
+          resources:
+            limits:
+              cpu: {{ index . "limits.cpu" | default "50m" }}
+              memory: {{ index . "limits.memory" | default "128Mi" }}
+            requests:
+              cpu: {{ index . "requests.cpu" | default "50m" }}
+              memory: {{ index . "requests.memory" | default "128Mi" }}
+          {{- end }}

src/charts/echo-server/templates/ingress.yaml

@@ -2,41 +2,42 @@
   {{- $fullName := include "echo-server.fullname" . -}}
   {{- $svcName := include "echo-server.name" . -}}
   {{- $svcPort := .Values.service.port -}}
-  {{- $nginxTlsEnabled := and (eq (printf "%v" .Values.ingress.nginx.enabled) "true") (eq (printf "%v" .Values.tlsEnabled) "true")}}
+  {{- $nginxTlsEnabled := and (eq (printf "%v" .Values.ingress.nginx.enabled) "true") (eq (printf "%v" .Values.tlsEnabled) "true") }}
 apiVersion: networking.k8s.io/v1
 kind: Ingress
 metadata:
   name: {{ $fullName }}
   annotations:
-    {{- if eq (printf "%v" .Values.ingress.nginx.enabled) "true" }}
-    kubernetes.io/ingress.class: {{ .Values.ingress.nginx.class }}
-      {{- if (index .Values.ingress.nginx "tls_certificate_cluster_issuer") }}
-    cert-manager.io/cluster-issuer: {{ .Values.ingress.nginx.tls_certificate_cluster_issuer }}
-      {{- end }}
-    {{- else if eq (printf "%v" .Values.ingress.alb.enabled) "true" }}
-    kubernetes.io/ingress.class: {{ .Values.ingress.alb.class }}
-      {{- if not .Values.ingress.alb.group_name }}
-    alb.ingress.kubernetes.io/load-balancer-name: {{ index .Values.ingress.alb "load_balancer_name" | default "k8s-common" }}
-      {{- end }}
-    alb.ingress.kubernetes.io/group.name: {{ index .Values.ingress.alb "group_name" | default "common" }}
-    alb.ingress.kubernetes.io/scheme: {{ index .Values.ingress.alb "scheme" | default "internet-facing" }}
-      {{- if .Values.ingress.alb.access_logs.enabled }}
-    alb.ingress.kubernetes.io/load-balancer-attributes: access_logs.s3.enabled=true,access_logs.s3.bucket={{.Values.ingress.alb.access_logs.s3_bucket_name}},access_logs.s3.prefix={{.Values.ingress.alb.access_logs.s3_bucket_prefix}}
-      {{- end }}
-    alb.ingress.kubernetes.io/target-type: 'ip'
-      {{- if eq (printf "%v" .Values.ingress.alb.ssl_redirect.enabled) "true" }}
-    alb.ingress.kubernetes.io/ssl-redirect: '{{ .Values.ingress.alb.ssl_redirect.port }}'
+    {{- with and (eq (printf "%v" .Values.ingress.use_ingress_class_annotation) "true") (index .Values.ingress "class") }}
+    kubernetes.io/ingress.class: {{ . }}
+    {{- end }}
+    {{- with and $nginxTlsEnabled (index .Values.ingress.nginx "tls_certificate_cluster_issuer") }}
+    cert-manager.io/cluster-issuer: {{ . }}
+    {{- end }}
+    {{- if eq (printf "%v" .Values.ingress.alb.enabled) "true" }}
+    alb.ingress.kubernetes.io/healthcheck-path: /ping
+      {{- with index .Values.ingress.alb "group_name" }}
+    alb.ingress.kubernetes.io/group.name: {{ . }}
       {{- end }}
       {{- if eq (printf "%v" .Values.tlsEnabled) "true" }}
     alb.ingress.kubernetes.io/backend-protocol: HTTP
-    alb.ingress.kubernetes.io/listen-ports: '[{"HTTP": 80},{"HTTPS":443}]'
+    alb.ingress.kubernetes.io/listen-ports: '[{"HTTP":80},{"HTTPS":443}]'
       {{- else }}
-    alb.ingress.kubernetes.io/listen-ports: '[{"HTTP": 80}]'
+    alb.ingress.kubernetes.io/listen-ports: '[{"HTTP":80}]'
       {{- end }}
+    # See https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.5/guide/ingress/annotations/#target-type
+    alb.ingress.kubernetes.io/target-type: {{ if eq (printf "%v" .Values.service.type) "NodePort" -}} "instance" {{- else -}} "ip" {{- end }}
     {{- end }}
   labels:
     {{- include "echo-server.labels" . | nindent 4 }}
 spec:
+  # If not specified, the Ingress controller will insert the ingressClassName field
+  # when creating the Ingress resource, setting ingressClassName to the name of the then-default IngressClass.
+  {{- with and (ne (printf "%v" .Values.ingress.use_ingress_class_annotation) "true") (index .Values.ingress "class") }}
+  ingressClassName: {{ . }}
+  {{- end }}
+  # ALB controller will auto-discover the ACM certificate based on rules[].host
+  # Nginx needs explicit configuration of location of cert-manager TLS certificate
   {{- if $nginxTlsEnabled }}
   tls: # < placing a host in the TLS config will indicate a certificate should be created
     - hosts:

src/charts/echo-server/values.yaml

@@ -8,89 +8,52 @@ image:
   # image.repository -- https://hub.docker.com/r/ealen/echo-server
   repository: ealen/echo-server
   # image.tag -- https://github.com/Ealenn/Echo-Server/releases
-  tag: 0.4.2
-  pullPolicy: Always
+  tag: 0.8.12
+  pullPolicy: IfNotPresent
 
 #imagePullSecrets: []
 nameOverride: ""
 #fullnameOverride: ""
 
-#serviceAccount:
-#  # Specifies whether a service account should be created
-#  create: true
-#  # Annotations to add to the service account
-#  annotations: {}
-#  # The name of the service account to use.
-#  # If not set and create is true, a name is generated using the fullname template
-#  name: ""
-
-#podAnnotations: {}
-
-#podSecurityContext: {}
-#  # fsGroup: 2000
-
-#securityContext: {}
-#  # capabilities:
-#  #   drop:
-#  #   - ALL
-#  # readOnlyRootFilesystem: true
-#  # runAsNonRoot: true
-#  # runAsUser: 1000
 
 service:
   type: ClusterIP
   port: 80
 
 tlsEnabled: true
+# If livenessProbeLogging is false, requests to /ping will not be logged
+livenessProbeLogging: false
 
 ingress:
+  ## Allow class to be specified, but use default class (not class named "default") by default
+  # class: default
+
+  # Use deprecated `kubernetes.io/ingress.class` annotation
+  use_ingress_class_annotation: false
   nginx:
     # ingress.nginx.enabled -- Enable NGiNX ingress
     enabled: false
-    # annotation values
-    ## kubernetes.io/ingress.class:
-    class: "nginx"
-    ## cert-manager.io/cluster-issuer:
     tls_certificate_cluster_issuer: "letsencrypt-prod"
   alb:
-    enabled: true
-    # annotation values
-    ## kubernetes.io/ingress.class:
-    class: "alb"
-    ## alb.ingress.kubernetes.io/load-balancer-name:
-    ### load_balancer_name: "k8s-common"
-    ## alb.ingress.kubernetes.io/group.name:
-    ### group_name: "common"
-    ssl_redirect:
-      enabled: true
-    ## alb.ingress.kubernetes.io/ssl-redirect:
-      port: 443
-    access_logs:
-      enabled: false
-      ## s3_bucket_name: "acme-ue2-prod-eks-cluster-alb-access-logs"
-      s3_bucket_prefix: "echo-server"
+    enabled: false
+    ## Allow group to be specified, but use default by default
+    # group_name: common
 
-#resources: {}
-#  # We usually recommend not to specify default resources and to leave this as a conscious
-#  # choice for the user. This also increases chances charts run on environments with little
-#  # resources, such as Minikube. If you do want to specify resources, uncomment the following
-#  # lines, adjust them as necessary, and remove the curly braces after 'resources:'.
-#  # limits:
-#  #   cpu: 100m
-#  #   memory: 128Mi
-#  # requests:
-#  #   cpu: 100m
-#  #   memory: 128Mi
+    # Do NOT allow SSL redirect to be specified, because that affects other ingresses.
+    # "Once defined on a single Ingress, it impacts every Ingress within IngressGroup."
+    # See https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.6/guide/ingress/annotations/#ssl-redirect
+
+resources:
+  limits:
+    cpu: 50m
+    memory: 128Mi
+#  requests:
+#    cpu: 50m
+#    memory: 128Mi
 
 autoscaling:
   enabled: false
   #minReplicas: 1
   #maxReplicas: 100
   #targetCPUUtilizationPercentage: 80
   #targetMemoryUtilizationPercentage: 80
-
-#nodeSelector: {}
-
-#tolerations: []
-
-#affinity: {}