Skip to content

Conversation

@filippomc
Copy link
Collaborator

@filippomc filippomc commented May 22, 2025

Closes CH-189

@filippomc filippomc requested review from alxbrd and Copilot May 22, 2025 18:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds and refines documentation and default Kubernetes configurations for cluster provisioning across GCP, AWS, and Azure, plus a bootstrap script.

  • Rename and standardize GCP StorageClass and introduce an AWS default StorageClass.
  • Provide AWS-specific ingress Helm values and update the cluster-init script to install ingress-nginx and cert-manager.
  • Add detailed provider setup guides (GCP, AWS, Azure) and link them from the main README.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
infrastructure/cluster-configuration/storageclass.yaml Rename GCP StorageClass to standard with Delete policy
infrastructure/cluster-configuration/storageclass-default-aws.yaml Add default AWS StorageClass configuration
infrastructure/cluster-configuration/ingress/values-aws.yaml Add AWS NLB annotations and NGINX proxy cache settings
infrastructure/cluster-configuration/gcp-setup.md Add quickstart GKE setup instructions
infrastructure/cluster-configuration/cluster-init.sh Enhance bootstrap script with ingress-nginx and cert-manager
infrastructure/cluster-configuration/azure-setup.md Add AKS/AGIC setup guide and conditional ingress template
infrastructure/cluster-configuration/aws-setup.md Add EKS cluster creation and AWS Load Balancer Controller steps
infrastructure/cluster-configuration/README.md Expand README with TLDR and provider-specific links
docs/build-deploy/cluster-configuration.md Introduce high-level cluster configuration overview

Comment on lines +11 to +14
--set crds.enabled=true


helm install --name cert-manager --namespace cert-manager --version v0.14.0 jetstack/cert-manager --set webhook.enabled=false
Copy link

Copilot AI May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two separate helm install commands for cert-manager with differing versions and syntax; this will cause conflicts. Consolidate into a single, consistent installation.

Suggested change
--set crds.enabled=true
helm install --name cert-manager --namespace cert-manager --version v0.14.0 jetstack/cert-manager --set webhook.enabled=false
--set crds.enabled=true \
--set webhook.enabled=false

Copilot uses AI. Check for mistakes.
Note that have to pay attention to the version of the aws-load-balancer-controller to match with the policy. Wrong version will make things fail

```bash
curl -o iam-policy.json https://raw.githubusercontent.com/kubernetes-sigs/aws-load-balancer-controller/main/docs/install/iam_policy.json
Copy link
Contributor

@alxbrd alxbrd May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IAM policy should not use main by default, but rather should be the version of the controller installed, eg: https://raw.githubusercontent.com/kubernetes-sigs/aws-load-balancer-controller/**v2.13.2**/docs/install/iam_policy.json Similarly, a specific controller version, compatible with the policy should be installed.

filippomc and others added 3 commits May 23, 2025 10:28
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Alex <alxbrd@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants