Implement ingress-nginx and cert-manager by clofour · Pull Request #3 · clofour/gitlab-kubernetes

clofour · 2026-04-12T16:38:50Z

No description provided.

coderabbitai · 2026-04-12T16:39:03Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 331916b0-313e-43db-b9f4-407506093de1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Walkthrough

Adds Helm values and Terraform/kubernetes resources to deploy cert-manager and ingress-nginx, creates a Let's Encrypt ClusterIssuer using HTTP-01 via the nginx ingress class, and updates GitLab Helm values to disable bundled components and configure external ingress/cert-manager integration.

Changes

Cohort / File(s)	Summary
Helm: cert-manager `helm/cert-manager/values.yaml`	New values file: enable CRD install/keep, set `replicaCount: 1`, enable Prometheus metrics.
Helm: ingress-nginx `helm/ingress-nginx/values.yaml`	New values file: controller replicas=2, ingressClassResource `nginx` default, Service `LoadBalancer` with DigitalOcean annotations, proxy/protocol/HSTS/timeouts, metrics and defaultBackend enabled.
Helm: gitlab `helm/gitlab/values.yaml`	Updated global ingress settings: `configureCertmanager: false`, `class: nginx`, TLS enabled, `cert-manager.io/cluster-issuer: letsencrypt`; changed backup tmp bucket reference; disabled bundled minio/prometheus/nginx-ingress/certmanager.
Kubernetes `kubernetes/cluster-issuer.yaml`	New `ClusterIssuer` resource `letsencrypt` using ACME v2 with HTTP-01 solver via `ingressClassName: nginx`.
Terraform: Helm releases `terraform/helm.tf`	Added `helm_release.ingress_nginx` and `helm_release.cert_manager`, each referencing `../helm/<chart>/values.yaml`.
Terraform: Kubernetes resources `terraform/kubernetes.tf`	Added `kubernetes_namespace_v1.ingress_nginx`, `kubernetes_namespace_v1.cert_manager`, and `kubernetes_manifest.cluster_issuer` that depends on `helm_release.cert_manager`.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Terraform as Terraform
participant K8sAPI as Kubernetes API
participant Helm as Helm Provider
participant CertManager as cert-manager
participant Ingress as ingress-nginx
participant LetsEncrypt as Let's Encrypt ACME
Terraform->>K8sAPI: create namespaces (ingress-nginx, cert-manager)
Terraform->>Helm: install cert-manager (values.yaml)
Helm->>K8sAPI: deploy cert-manager CRDs & controllers
K8sAPI->>CertManager: start cert-manager controller
Terraform->>Helm: install ingress-nginx (values.yaml)
Helm->>K8sAPI: deploy ingress-nginx controller
Terraform->>K8sAPI: apply ClusterIssuer manifest (depends on cert-manager)
CertManager->>LetsEncrypt: request account & certificate (ACME HTTP-01)
LetsEncrypt->>Ingress: perform HTTP-01 challenge -> HTTP request to ingress-nginx
Ingress->>CertManager: route challenge to solver pod
CertManager->>LetsEncrypt: respond to challenge -> certificate issued
CertManager->>K8sAPI: store certificate/secret

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Configure Terraform #1: Introduced the DigitalOcean Kubernetes cluster resource referenced by new namespace resources and helm releases in this PR.
Implement GitLab and add terraform-validate action #2: Previously added/modified helm/gitlab/values.yaml; closely related to the GitLab values updates included here.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess relevance to the changeset.	Add a description explaining what ingress-nginx and cert-manager are being implemented for, and how they integrate with the GitLab deployment.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Implement ingress-nginx and cert-manager' clearly and directly describes the main changes in the pull request, which adds configuration and deployment for both ingress-nginx and cert-manager components.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 5

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 734c8e7a-7ee0-4274-8e11-41015a53333e

📥 Commits

Reviewing files that changed from the base of the PR and between fb4f0ae and 40ba5dd.

📒 Files selected for processing (6)

helm/cert-manager/values.yaml
helm/gitlab/values.yaml
helm/ingress-nginx/values.yaml
kubernetes/cluster-issuer.yaml
terraform/helm.tf
terraform/kubernetes.tf

📜 Review details

🔇 Additional comments (3)

terraform/helm.tf (1)

1-11: Ingress Helm release wiring looks good.

Line 3 correctly binds to kubernetes_namespace_v1.ingress_nginx, and Line 9 cleanly sources the dedicated values file.

terraform/kubernetes.tf (1)

1-15: Namespace provisioning and cluster dependency are correctly wired.

Lines 6 and 14 correctly enforce cluster creation before namespace creation.

helm/gitlab/values.yaml (1)

12-18: External ingress/cert-manager integration is consistent and correctly disabled in-chart.

The new ingress block and component toggles are aligned with the external ingress-nginx + cert-manager installation model in this PR.

Also applies to: 59-59, 69-75

clofour · 2026-04-12T16:55:25Z

@coderabbitai full review

coderabbitai · 2026-04-12T16:55:31Z

✅ Actions performed

Full review triggered.

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7447096d-28db-4312-9789-461e8e87435e

📥 Commits

Reviewing files that changed from the base of the PR and between fb4f0ae and e3cb72b.

📒 Files selected for processing (6)

helm/cert-manager/values.yaml
helm/gitlab/values.yaml
helm/ingress-nginx/values.yaml
kubernetes/cluster-issuer.yaml
terraform/helm.tf
terraform/kubernetes.tf

📜 Review details

🔇 Additional comments (9)

helm/cert-manager/values.yaml (1)

1-8: Looks good: cert-manager CRD lifecycle and metrics are configured correctly.

crds.enabled/crds.keep and the values structure align with the Terraform Helm release wiring in terraform/helm.tf.

terraform/helm.tf (2)

1-11: Ingress-NGINX Helm release wiring is solid.

Namespace binding, chart pinning, and external values file usage are coherent.

13-23: Cert-manager Helm release wiring is correct.

The OCI repository path, version pin, and values file integration are consistent with the cert-manager setup in this PR.

terraform/kubernetes.tf (2)

1-15: Namespace provisioning changes are correct.

Both namespaces are explicit and properly ordered after cluster creation.

25-29: Good dependency guard for ClusterIssuer apply order.

The explicit dependency on helm_release.cert_manager addresses CRD ordering risk before applying ClusterIssuer.

helm/ingress-nginx/values.yaml (1)

1-32: Ingress controller values are coherent for this integration.

The class name, proxy-protocol pairing, TLS-related config, and metrics settings align with the rest of the PR.

helm/gitlab/values.yaml (3)

12-19: Ingress/cert-manager integration values look correct.

Disabling bundled cert-manager wiring while setting the external letsencrypt ClusterIssuer annotation is consistent with this PR’s architecture.

59-59: tmpBucket mapping update is correct.

Using ${buckets["tmp"]} aligns with the provisioned Spaces bucket keys.

69-74: Bundled component toggles are aligned with external Helm releases.

Disabling in-chart minio, prometheus, and nginx-ingress matches the Terraform-managed components introduced in this PR.

coderabbitai · 2026-04-12T16:59:04Z

+    solvers:
+      - http01:
+          ingressClassName: nginx


⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

For cert-manager v1.20.x, what is the exact YAML path for setting nginx ingress class in a ClusterIssuer HTTP-01 solver?

💡 Result:

The exact YAML path for setting the nginx ingress class in a ClusterIssuer HTTP-01 solver for cert-manager v1.20.x is spec.acme.solvers[0].http01.ingress.ingressClassName: nginx. This uses the modern ingressClassName field (recommended since cert-manager 1.12), which sets spec.ingressClassName on the challenge Ingress resource. The legacy spec.acme.solvers[0].http01.ingress.class: nginx sets the kubernetes.io/ingress.class annotation instead. Example: apiVersion: cert-manager.io/v1 kind: ClusterIssuer metadata: name: example spec: acme: # ... other fields solvers: - http01: ingress: ingressClassName: nginx

Citations:

1: https://cert-manager.io/docs/configuration/acme/http01/

2: Conflicting ingressClassName http01 issuer spec and acme.cert-manager.io/http01-ingress-class annotation cert-manager/cert-manager#6184

3: https://cert-manager.io/docs/releases/release-notes/release-notes-1.20/

Fix HTTP-01 solver structure: ingressClassName must be nested under http01.ingress.

The current structure will fail cert-manager schema validation. Move ingressClassName: nginx to http01.ingress.ingressClassName so the ClusterIssuer can be applied successfully.

Proposed fix

solvers: - http01: - ingressClassName: nginx + ingress: + ingressClassName: nginx

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

solvers:

- http01:

ingressClassName: nginx

solvers:

- http01:

ingress:

ingressClassName: nginx

clofour added 2 commits April 12, 2026 16:52

Bug fixes

b520162

Implement ingress-nginx and cert-manager

40ba5dd

coderabbitai Bot reviewed Apr 12, 2026

View reviewed changes

Comment thread helm/cert-manager/values.yaml Outdated

Comment thread helm/ingress-nginx/values.yaml

Comment thread kubernetes/cluster-issuer.yaml Outdated

Comment thread terraform/helm.tf Outdated

Comment thread terraform/kubernetes.tf

Bug fixes

e3cb72b

coderabbitai Bot reviewed Apr 12, 2026

View reviewed changes

Bugfix

c043f37

clofour merged commit 7f0fb27 into main Apr 12, 2026
2 checks passed

This was referenced Apr 18, 2026

Bug fixes and improvements #5

Merged

Implement wildcard certificate #7

Merged

coderabbitai Bot mentioned this pull request May 1, 2026

Migrate to GitOps #24

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement ingress-nginx and cert-manager#3

Implement ingress-nginx and cert-manager#3
clofour merged 4 commits intomainfrom
dev

clofour commented Apr 12, 2026

Uh oh!

coderabbitai Bot commented Apr 12, 2026 •

edited

Loading

Review skipped

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clofour commented Apr 12, 2026

Uh oh!

coderabbitai Bot commented Apr 12, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Apr 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

clofour commented Apr 12, 2026

Uh oh!

coderabbitai Bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clofour commented Apr 12, 2026

Uh oh!

coderabbitai Bot commented Apr 12, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Apr 12, 2026 •

edited

Loading