Add KubernetesIngressAddOn for enhanced Ingress Management #989

Pjv93 · 2024-04-23T19:30:37Z

Issue #, if available:

*Description of changes: This PR introduces the Kubernetes Ingress Add-On class that supports additional configuration options like SSL redirection, cross-zone load balancing, and external DNS integration. The aim is to provide an extensible and configurable Ingress solution within the EKS blueprints framework.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

shapirov103 · 2024-04-23T20:10:33Z

@Pjv93 do you mind adding a blueprint that we can use to validate that the addon works?

Pjv93 · 2024-04-23T20:55:30Z

@shapirov103 OFC! Here is a sample blueprint that:

Sets up a Kubernetes ingress controller with specified configurations for handling ingress traffic, including load balancing settings, SSL/TLS termination, and routing.
Automates DNS record management based on services and ingresses within the cluster, simplifying the process of connecting domain names to dynamically provisioned resources like load balancers.

import * as cdk from 'aws-cdk-lib';
import * as blueprints from '@aws-quickstart/eks-blueprints';
import { KubernetesIngressAddOn } from '../lib/addons/kubernetes-nginx';

const app = new cdk.App();
const account = '1234567890';
const region = 'us-east-2';
const version = 'auto';
const myDomainName = "test.example.com";





// Configure the Kubernetes Ingress AddOn
const kubernetesIngressAddOn = new KubernetesIngressAddOn({
    crossZoneEnabled: true,
    internetFacing: true,
    targetType: 'ip',
    externalDnsHostname: 'example.com',
    certificateResourceName: 'arn:aws:acm:us-east-2:123456789:certificate/xxxxxxxxx',
});

const addOns: Array<blueprints.ClusterAddOn> = [
    new blueprints.addons.CalicoOperatorAddOn(),
    new blueprints.addons.AwsLoadBalancerControllerAddOn(),
    new blueprints.addons.VpcCniAddOn(),
    new blueprints.addons.CoreDnsAddOn(),
    new blueprints.addons.CertManagerAddOn(),
    new blueprints.addons.ExternalsSecretsAddOn(),
    kubernetesIngressAddOn,
    new blueprints.addons.ExternalDnsAddOn({
        hostedZoneResources: ["MyHostedZone1"]
    })
    
];

const stack = blueprints.EksBlueprint.builder()
    .account(account)
    .region(region)
    .version(version)
    .resourceProvider("MyHostedZone1", new blueprints.LookupHostedZoneProvider(myDomainName))
    .addOns(...addOns)
    .build(app, 'eks-blueprint');

Here are the annotations applied to the Ingress Controller

helm get values k8s-ingress -n kube-system
USER-SUPPLIED VALUES:
controller:
  electionID: ingress-controller-leader
  ingressClassResource:
    controllerValue: k8s.io/ingress-nginx
    default: false
    enabled: true
    name: nginx
  service:
    annotations:
      external-dns.alpha.kubernetes.io/hostname: pjv.people.aws.dev
      nginx.ingress.kubernetes.io/force-ssl-redirect: true
      service.beta.kubernetes.io/aws-load-balancer-attributes: load_balancing.cross_zone.enabled=true
      service.beta.kubernetes.io/aws-load-balancer-backend-protocol: http
      service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout: "3600"
      service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip
      service.beta.kubernetes.io/aws-load-balancer-scheme: internet-facing
      service.beta.kubernetes.io/aws-load-balancer-ssl-cert: arn:aws:acm:us-east-2:0123456789:certificate/xxxxxx
      service.beta.kubernetes.io/aws-load-balancer-ssl-ports: https
      service.beta.kubernetes.io/aws-load-balancer-type: external
    targetPorts:
      http: http
      https: http

Simple Ingress using test.pjv.people.aws.dev/

elamaran11

@Pjv93 Awesome work, Thankyou for working on this change. Following are some high level feedback.

Fix all GH action errors, i dont think the addon will compile with these errors. make build and make lint
Add documentation to the addon similar to other Ingress One
Make sure to update mkdocs and doc index for the new addon
Recommend to add a test script for the new addon
Add the addon to example blueprint so this can be tested with e2e.
Also share screenshots of working addon with ALB creation etc.

elamaran11 · 2024-04-23T20:51:50Z

lib/addons/index.ts

@@ -60,6 +60,7 @@ export * from './apache-airflow';
 export * from './neuron';
 export * from './eks-pod-identity-agent';
 export * from './neuron';
+export * from './kubernetes-nginx'


Missing semi colon

elamaran11 · 2024-04-23T20:52:02Z

lib/addons/kubernetes-nginx/index.ts

@@ -0,0 +1,112 @@
+// Import necessary AWS CDK and utility modules
+import { ICertificate, Certificate } from "aws-cdk-lib/aws-certificatemanager";


Remove ICertificate.

elamaran11 · 2024-04-23T20:52:16Z

lib/addons/kubernetes-nginx/index.ts

+import * as dot from 'dot-object';
+import { dependable, supportsALL } from "../../utils";
+import { setPath } from "../../utils/object-utils";
+import { AwsLoadBalancerControllerAddOn, ClusterInfo, Values, HelmAddOn, HelmAddOnProps, HelmAddOnUserProps, GlobalResources } from "@aws-quickstart/eks-blueprints"; 


remove GlobalResources if not used

lib/addons/kubernetes-nginx/index.ts

Pjv93 · 2024-04-23T20:59:36Z

Sounds good! I know it needs a lot of work but wanted to at least have some visibility on it. I will work on your comments. Thanks!

elamaran11 · 2024-04-23T21:02:01Z

Sounds good! I know it needs a lot of work but wanted to at least have some visibility on it. I will work on your comments. Thanks!

Honestly this is great work, addon work is almost there, you just need to complete to cover all grounds.

…es like name, chart..

… with updating mkdocs file

…ess-addon Merging updated files

added - Kubernetes Nginx: 'addons/kubernetes-nginx.md'

Pjv93 · 2024-06-03T04:01:15Z

Hi @elamaran11 & @shapirov103, here is an updated blueprint to test the addon below. I have since fixed the errors from the GH Actions, Add documentation to the addon in the docs folder, and updated mkdocs and doc index for the addon.

import * as cdk from 'aws-cdk-lib';
import * as blueprints from '../lib';

const app = new cdk.App();
const account = 'xxxxxxxxxxx';
const region = 'us-east-1';
const myDomainName = "YourDomainName.com";

// Create the stack
const stack = new cdk.Stack(app, 'EksBlueprintStack', {
    env: {
        account: account,
        region: region,
    }
});

// Lookup the hosted zone by domain name
const hostedZone = cdk.aws_route53.HostedZone.fromLookup(stack, 'HostedZoneLookup', {
    domainName: myDomainName,
});

const addOns: Array<blueprints.ClusterAddOn> = [
    new blueprints.addons.AwsLoadBalancerControllerAddOn(),
    new blueprints.addons.ExternalDnsAddOn({
        hostedZoneResources: [blueprints.GlobalResources.HostedZone]
    }),
    new blueprints.addons.KubernetesIngressAddOn({
        crossZoneEnabled: true,
        internetFacing: true,
        targetType: 'ip',
        externalDnsHostname: myDomainName,
        certificateResourceName: blueprints.GlobalResources.Certificate
    }),
    new blueprints.addons.CalicoOperatorAddOn(),
    new blueprints.addons.VpcCniAddOn(),
    new blueprints.addons.CoreDnsAddOn(),
    new blueprints.addons.KubeProxyAddOn(),
    new blueprints.addons.CertManagerAddOn(),
    new blueprints.addons.ExternalsSecretsAddOn()
];

blueprints.EksBlueprint.builder()
    .resourceProvider(blueprints.GlobalResources.HostedZone, new blueprints.ImportHostedZoneProvider(hostedZone.hostedZoneId))
    .resourceProvider(blueprints.GlobalResources.Certificate, new blueprints.CreateCertificateProvider('DomainWildcardCert', `*.${myDomainName}`, blueprints.GlobalResources.HostedZone)) // referencing hosted zone for automatic DNS validation
    .account(account)
    .region(region)
    .version("auto")
    .addOns(...addOns)
    .build(stack, 'EksBlueprintStack');

Pjv93

GH action errors have been resolved.

elamaran11

@Pjv93 Nice work some minor feedback. Did you test different scenerios like different LBs, using DNS etc? Btw the addon also needs to be added to blueprints e2e stack.

@shapirov103 Overall approach looks great, please check and provide your feedback.

elamaran11 · 2024-06-03T15:26:51Z

docs/addons/kubernetes-nginx.md

@@ -0,0 +1,259 @@
+# Kubernetes NGINX Add-on


Please add this md file entry in to docs/addons/index.md so it gets in to list of addons.

elamaran11 · 2024-06-03T15:27:03Z

bin/main.ts

@@ -8,4 +8,4 @@ const account = process.env.CDK_DEFAULT_ACCOUNT;
 const region = process.env.CDK_DEFAULT_REGION;
 const props = { env: { account, region } };

-new BlueprintConstruct(app, props);
+new BlueprintConstruct(app, props);


Why is this file checked in.

Might have been a mistake as I was testing my own main.ts file and reverted it to the original file. Theres no difference. I think the blank line in line 12 was removed in the process.

elamaran11 · 2024-06-03T15:27:53Z

package.json

@@ -19,7 +19,7 @@
    },
    "devDependencies": {
        "@types/dot-object": "^2.1.6",
-        "@types/jest": "^29.5.11",
+        "@types/jest": "^29.5.12",


Is this a dependency for this addon to work?

elamaran11 · 2024-06-03T15:27:58Z

package.json

@@ -31,7 +31,7 @@
        "jest": "^29.7.0",
        "json-schema-to-typescript": "^13.1.1",
        "lint": "^1.1.2",
-        "ts-jest": "^29.1.1",
+        "ts-jest": "^29.1.2",


Is this a dependency for this addon to work?

elamaran11 · 2024-06-03T15:28:10Z

package.json

-        "aws-cdk-lib": "2.133.0",
-        "aws-cdk": "2.133.0"
+        "aws-cdk": "2.133.0",
+        "aws-cdk-lib": "2.133.0"


What changed?

It looks like I just changed the placement of the aws-cdk above aws-cdk-lib. No real change and actually not necessary.

added kubernetes-nginx to the list of addons

changed name for uniformity.

shapirov103

@Pjv93 great work and much needed functionality. Please see my comments, some are less significant than others. Happy to meet and discuss. If you could demo it, that would be even better.

shapirov103 · 2024-06-19T17:47:08Z

mkdocs.yml

@@ -68,6 +68,7 @@ nav:
    - Kubecost: 'addons/kubecost.md'
    - Kubeflow: 'addons/kubeflow.md'
    - KubeRay Operator: 'addons/kuberay-operator.md'
+    - Kubernetes Ingress: 'addons/kubernetes-ingress.md'


Kubernetes ingress is a very broad definition. We have to be specific that it is nginx as there are many kubernetes ingress controllers. AWS ALB ingress controller is one of the kubernetes ingress controllers.

I see you made changes to abbreviate. I suggest the following naming to make it clear to the customers (using standard nomenclature):

https://github.com/kubernetes/ingress-nginx - is the repo and project name.

Let's rename to ingress-nginx throughout. The addon name will be ingress-nginx and the entries in the doc will be the same. We will specify that the difference between ingress-nginx and nginx addon are that the former is upstream kubernetes nginx and the latter is F5.

shapirov103 · 2024-06-19T17:48:28Z

lib/addons/kubernetes-ingress/index.ts

+        if (props.certificateResourceARN) {
+            presetAnnotations['service.beta.kubernetes.io/aws-load-balancer-ssl-ports'] = 'https';
+            presetAnnotations['service.beta.kubernetes.io/aws-load-balancer-ssl-cert'] = props.certificateResourceARN;
+            presetAnnotations['nginx.ingress.kubernetes.io/force-ssl-redirect'] = true;


Should this be hardcoded?

shapirov103 · 2024-06-19T17:48:58Z

lib/addons/kubernetes-ingress/index.ts

+
+
+        // Setup service annotations based on the properties provided
+        const presetAnnotations: any = {


Have you tested with network load balancer and these annotations set and external DNS?

shapirov103 · 2024-06-19T17:52:06Z

examples/blueprint-construct/index.ts

+                internetFacing: true,
+                targetType: 'ip',
+                externalDnsHostname: 'your-domain.com',
+                certificateResourceName: 'your-certificate-resource-name'


How will our test work if this is set to placeholder? Please note that this blueprint is actually executed when we run do-e2e-tests. Also I am unclear how it will co-exist with the nginx addon that is from F5.

Can you create a separate blueprint with this addon for testing in examples/examples.ts? We will need something executable before releasing.

shapirov103 · 2024-06-19T17:56:20Z

lib/addons/kubernetes-ingress/index.ts

+        };
+
+        // Configure SSL-related annotations if certificate resource name is provided
+        if (props.certificateResourceName) {


Style: the code block on line 188-196 is almost identical to the code block in lines 196-200.

How about you use this logic:

let certificateResourceARN = props.certificateResourceARN; if(!certificateResourceARN && props.certificateResourceName) { certificateResourceARN = <populate from the resource provider>; } if(certificateResourceARN) { // set annotations }

…ess-addon

shapirov103 · 2024-06-28T03:36:19Z

/do-e2e-tests

shapirov103 · 2024-06-28T04:35:17Z

/do-e2e-tests

aws-ia-ci

end to end tests failed. A maintainer can provide more details.

aws-ia-ci

end to end tests failed. A maintainer can provide more details.

shapirov103 · 2024-06-28T14:00:22Z

/do-e2e-tests

aws-ia-ci

end to end tests failed. A maintainer can provide more details.

elamaran11

LGTM

shapirov103 · 2024-06-28T18:48:42Z

e2e failure due to hanging LB provisioned through the ingress-nginx addon (needs more investigation, looks like LB controller was dropped before it had a chance to clean up).

shapirov103 · 2024-06-28T18:49:08Z

/do-e2e-tests

aws-ia-ci

end to end tests failed. A maintainer can provide more details.

Test succeeded, CFN was hanging longer than usual, but removed all the resources

shapirov103

LGTM

Pjv93 · 2024-06-28T20:17:45Z

@elamaran11 & @shapirov103 Thank you both for your patience and allowing me to contribute to the eks blueprint addons!

pjv added 2 commits April 23, 2024 14:54

added kubernetes nginx addon

96a1075

Add KubernetesIngressAddOn for enhanced ingress management

3a0a9fa

Pjv93 requested review from aws-ia-ci, parkand1, shapirov103, youngjeong46, elamaran11 and a team as code owners April 23, 2024 19:30

elamaran11 requested changes Apr 23, 2024

View reviewed changes

elamaran11 linked an issue Apr 23, 2024 that may be closed by this pull request

New Kubernetes Nginx Addon to Blueprints #926

Closed

1 task

pjv and others added 11 commits April 24, 2024 03:04

added kubernetes nginx test

b0ae0e5

added semi-colon

0d26f2d

updated the KubernetesIngressAddOnProps interface to include properti…

a39f0d7

…es like name, chart..

made the corrections Ela suggested and added the docs for addon along…

18d1121

… with updating mkdocs file

Merge remote-tracking branch 'upstream/main' into add-kubernetes-ingr…

29f7ed7

…ess-addon Merging updated files

Resolved merge conflicts

35153df

Resolved merge conflicts

d572c92

Merge branch 'aws-quickstart:main' into add-kubernetes-ingress-addon

08b9b79

Updated Kubernetes-Nginx addon

adf9d3e

uploaded kubernetes-nginx.md file

58505d0

Update mkdocs.yml

02b3604

added - Kubernetes Nginx: 'addons/kubernetes-nginx.md'

Pjv93 commented Jun 3, 2024

View reviewed changes

elamaran11 requested changes Jun 3, 2024

View reviewed changes

Pjv93 added 2 commits June 3, 2024 12:09

Update index.md

e9cad83

added kubernetes-nginx to the list of addons

Update and rename kubernetes-nginx.md to kubernetes-ingress.md

698b52c

changed name for uniformity.

removed test files

fb899b2

shapirov103 reviewed Jun 19, 2024

View reviewed changes

pjv and others added 11 commits June 19, 2024 14:00

resolved conflicts

59be4fb

modified file references from kubernetes-ingress to ingress-nginx.md

ba29239

Merge branch 'aws-quickstart:main' into add-kubernetes-ingress-addon

9f671f2

Merge branch 'aws-quickstart:main' into add-kubernetes-ingress-addon

1f077e8

Update index.ts - corrected ingress addon name and removed params

e554ae2

Update index.ts added ingress-nginx

e58fa7c

added ingress-nginx.md to mkdocs.yml

f89aad8

removed kubernetesNginxAddOn references to IngressNginxAddOn

6f3a6b9

removed kubernetes-nginx conflict

eb528fa

Merge remote-tracking branch 'upstream/main' into add-kubernetes-ingr…

e407116

…ess-addon

updated title to Ingress Nginx Addon

6ad6bb0

modified ingress name from nginx to ingress-nginx

0442bfa

aws-ia-ci requested changes Jun 28, 2024

View reviewed changes

pjv added 2 commits June 28, 2024 11:32

added semi-colon to resolve lint warning

2c1ceb2

added ClusterIP value to blueprint-construct file

14b3b4a

elamaran11 approved these changes Jun 28, 2024

View reviewed changes

aws-ia-ci previously requested changes Jun 28, 2024

View reviewed changes

shapirov103 approved these changes Jun 28, 2024

View reviewed changes

shapirov103 merged commit ba65e69 into aws-quickstart:main Jun 28, 2024

		@@ -0,0 +1,112 @@
		// Import necessary AWS CDK and utility modules
		import { ICertificate, Certificate } from "aws-cdk-lib/aws-certificatemanager";



		// Setup service annotations based on the properties provided
		const presetAnnotations: any = {

Add KubernetesIngressAddOn for enhanced Ingress Management #989

Add KubernetesIngressAddOn for enhanced Ingress Management #989

Conversation

Pjv93 commented Apr 23, 2024

shapirov103 commented Apr 23, 2024

Pjv93 commented Apr 23, 2024 • edited Loading

elamaran11 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pjv93 commented Apr 23, 2024

elamaran11 commented Apr 23, 2024

Pjv93 commented Jun 3, 2024 • edited Loading

Pjv93 left a comment

Choose a reason for hiding this comment

elamaran11 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pjv93 Jun 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shapirov103 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shapirov103 commented Jun 28, 2024

shapirov103 commented Jun 28, 2024

aws-ia-ci left a comment

Choose a reason for hiding this comment

aws-ia-ci left a comment

Choose a reason for hiding this comment

shapirov103 commented Jun 28, 2024

aws-ia-ci left a comment

Choose a reason for hiding this comment

elamaran11 left a comment

Choose a reason for hiding this comment

shapirov103 commented Jun 28, 2024

shapirov103 commented Jun 28, 2024

aws-ia-ci left a comment

Choose a reason for hiding this comment

shapirov103 left a comment

Choose a reason for hiding this comment

Pjv93 commented Jun 28, 2024

Pjv93 commented Apr 23, 2024 •

edited

Loading

elamaran11 left a comment •

edited

Loading

Pjv93 commented Jun 3, 2024 •

edited

Loading

Pjv93 Jun 3, 2024 •

edited

Loading