Skip to content

Various bug fixes and improvements #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
clofour merged 3 commits into from
Apr 12, 2026
Merged

Various bug fixes and improvements #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b301f97
Various bug fixes and improvements
clofour Apr 12, 2026
1c31b90
Bug fixes
clofour Apr 12, 2026
f414c1e
Bug fixes
clofour Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
.PHONY: init plan deploy destroy

TERRAFORM_DIR := terraform

init:
cd $(TERRAFORM_DIR) && terraform init

plan:
cd $(TERRAFORM_DIR) && terraform plan

deploy:
cd $(TERRAFORM_DIR) && terraform apply

destroy:
cd $(TERRAFORM_DIR) && terraform destroy
8 changes: 8 additions & 0 deletions helm/gitlab/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
global:
edition: ce

initialRootPassword:
secret: gitlab-initial-root-password
key: password

hosts:
domain: ${domain}
https: true
Expand All @@ -27,6 +31,7 @@ global:
key: password

redis:
scheme: rediss
host: ${redis_host}
port: ${redis_port}
auth:
Expand Down Expand Up @@ -60,6 +65,9 @@ global:

time_zone: UTC

extraEnv:
PGSSLMODE: require


postgresql:
install: false
Expand Down
2 changes: 1 addition & 1 deletion helm/ingress-nginx/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ controller:
service:
type: LoadBalancer
annotations:
service.beta.kubernetes.io/do-name: "gitlab-lb"
service.beta.kubernetes.io/do-loadbalancer-name: "gitlab-lb"
service.beta.kubernetes.io/do-loadbalancer-protocol: "tcp"
service.beta.kubernetes.io/do-loadbalancer-size-unit: "1"
service.beta.kubernetes.io/do-loadbalancer-enable-proxy-protocol: "true"
Expand Down
1 change: 1 addition & 0 deletions kubernetes/cluster-issuer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ metadata:
spec:
acme:
server: https://acme-v02.api.letsencrypt.org/directory
email: ${email}
privateKeySecretRef:
name: letsencrypt-account-key
solvers:
Expand Down
17 changes: 17 additions & 0 deletions terraform/dns.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
locals {
lb_ip = try(data.kubernetes_service_v1.ingress_nginx.status[0].load_balancer[0].ingress[0].ip, null)
}

resource "digitalocean_domain" "main" {
name = var.domain_name
}

resource "digitalocean_record" "gitlab" {
count = local.lb_ip != null ? 1 : 0

domain = digitalocean_domain.main.name
type = "A"
name = var.gitlab_host
value = locals.lb_ip
ttl = 300
Comment on lines +9 to +16
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use var.gitlab_host for the A record name.

The chart already allows overriding the GitLab hostname, but this record is pinned to "gitlab". Any non-default gitlab_host will publish the wrong DNS name and the ingress host won't resolve.

🛠 Suggested change 
 resource "digitalocean_record" "gitlab" {
     domain = digitalocean_domain.main.name
     type = "A"
-    name = "gitlab"
+    name = var.gitlab_host
     value = data.kubernetes_service_v1.ingress_nginx.status[0].load_balancer[0].ingress[0].ip
     ttl = 300
 }
📝 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
resource "digitalocean_record" "gitlab" {
domain = digitalocean_domain.main.name
type = "A"
name = "gitlab"
value = data.kubernetes_service_v1.ingress_nginx.status[0].load_balancer[0].ingress[0].ip
ttl = 300
resource "digitalocean_record" "gitlab" {
domain = digitalocean_domain.main.name
type = "A"
name = var.gitlab_host
value = data.kubernetes_service_v1.ingress_nginx.status[0].load_balancer[0].ingress[0].ip
ttl = 300
}

}
3 changes: 0 additions & 3 deletions terraform/domain.tf
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions terraform/helm.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ resource "helm_release" "gitlab" {
]

depends_on = [
kubernetes_secret_v1.gitlab_initial_root_password,
kubernetes_secret_v1.gitlab_postgres,
kubernetes_secret_v1.gitlab_redis,
kubernetes_secret_v1.gitlab_s3_main,
Expand Down
30 changes: 29 additions & 1 deletion terraform/kubernetes.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,30 @@ resource "kubernetes_namespace_v1" "gitlab" {
}

resource "kubernetes_manifest" "cluster_issuer" {
manifest = yamldecode(file("${path.module}/../kubernetes/cluster-issuer.yaml"))
manifest = yamldecode(templatefile("${path.module}/../kubernetes/cluster-issuer.yaml", {
email = var.email
}))

depends_on = [ helm_release.cert_manager ]
}

resource "random_password" "gitlab_root" {
length = 64
}

resource "kubernetes_secret_v1" "gitlab_initial_root_password" {
metadata {
name = "gitlab-initial-root-password"
namespace = kubernetes_namespace_v1.gitlab.metadata[0].name
}

data = {
password = random_password.gitlab_root.result
}

type = "Opaque"
}
Comment on lines +37 to +48
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add this secret to helm_release.gitlab.depends_on.

helm/gitlab/values.yaml now makes the release depend on gitlab-initial-root-password, but the provided terraform/helm.tf snippet still only waits on the older secrets. On a fresh apply, Helm can start before this secret exists and fail the install.

🛠 Follow-up change outside this hunk 
 resource "helm_release" "gitlab" {
   ...
   depends_on = [
+    kubernetes_secret_v1.gitlab_initial_root_password,
     kubernetes_secret_v1.gitlab_postgres,
     kubernetes_secret_v1.gitlab_redis,
     kubernetes_secret_v1.gitlab_s3_main,
     digitalocean_database_db.gitlab
   ]
 }


resource "kubernetes_secret_v1" "gitlab_postgres" {
metadata {
name = "gitlab-postgres-secret"
Expand Down Expand Up @@ -111,4 +130,13 @@ resource "kubernetes_secret_v1" "gitlab_s3_backup" {
}

type = "Opaque"
}

data "kubernetes_service_v1" "ingress_nginx" {
metadata {
name = "ingress-nginx-controller"
namespace = kubernetes_namespace_v1.ingress_nginx.metadata[0].name
}

depends_on = [helm_release.ingress_nginx]
Comment on lines +135 to +141
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

git ls-files | grep -E '\.tf$' | head -20

Repository: clofour/gitlab-kubernetes

Length of output: 316

🏁 Script executed:

wc -l terraform/kubernetes.tf

Repository: clofour/gitlab-kubernetes

Length of output: 96

🏁 Script executed:

cat -n terraform/kubernetes.tf

Repository: clofour/gitlab-kubernetes

Length of output: 4560

🏁 Script executed:

cat -n terraform/dns.tf

Repository: clofour/gitlab-kubernetes

Length of output: 445

🏁 Script executed:

cat -n terraform/helm.tf

Repository: clofour/gitlab-kubernetes

Length of output: 2275

🏁 Script executed:

rg "ingress_nginx" terraform/ -A 3 -B 3

Repository: clofour/gitlab-kubernetes

Length of output: 1448

🏁 Script executed:

rg "digitalocean_record\|digitalocean_domain" terraform/ -A 2 -B 2

Repository: clofour/gitlab-kubernetes

Length of output: 51

🏁 Script executed:

cat -n terraform/dns.tf

Repository: clofour/gitlab-kubernetes

Length of output: 445

🏁 Script executed:

rg "timeouts\|wait" terraform/ -i

Repository: clofour/gitlab-kubernetes

Length of output: 51

🏁 Script executed:

rg "lifecycle\|create_before_destroy" terraform/

Repository: clofour/gitlab-kubernetes

Length of output: 51

🏁 Script executed:

cat -n terraform/outputs.tf

Repository: clofour/gitlab-kubernetes

Length of output: 1478

Add timeout to data source to wait for load balancer provisioning.

The depends_on = [helm_release.ingress_nginx] only ensures the Service is created. DigitalOcean's cloud controller provisioning the load balancer IP in status.load_balancer.ingress is asynchronous and can complete after the helm release, causing the DNS record creation in terraform/dns.tf line 9 to fail when it tries to access an empty ingress list on first apply.

Add a timeouts block to the data source or use a resource-based waiter to poll until the load balancer is ready before the DNS record is created.

}
5 changes: 5 additions & 0 deletions terraform/outputs.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ output "kubeconfig" {
sensitive = true
}

output "gitlab_initial_root_password" {
value = random_password.gitlab_root.result
sensitive = true
}
Comment thread
clofour marked this conversation as resolved.


output "postgres_host" {
value = digitalocean_database_cluster.postgres.private_host
Expand Down
8 changes: 8 additions & 0 deletions terraform/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ variable "node_count" {
default = 2
}

variable "email" {
type = string
validation {
condition = trimspace(var.email) != ""
error_message = "email must not be empty."
}
}
Comment on lines +42 to +48
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

git ls-files | grep -E "terraform|kubernetes" | head -20

Repository: clofour/gitlab-kubernetes

Length of output: 419

🏁 Script executed:

cat -n terraform/variables.tf | head -60

Repository: clofour/gitlab-kubernetes

Length of output: 1245

🏁 Script executed:

cat -n terraform/kubernetes.tf | grep -A 5 -B 5 "email"

Repository: clofour/gitlab-kubernetes

Length of output: 434

🏁 Script executed:

cat -n kubernetes/cluster-issuer.yaml

Repository: clofour/gitlab-kubernetes

Length of output: 485

🏁 Script executed:

cat -n terraform/variables.tf | grep -A 10 "validation"

Repository: clofour/gitlab-kubernetes

Length of output: 51

🏁 Script executed:

web_search
What is the Terraform variable validation block syntax for string variables?

Repository: clofour/gitlab-kubernetes

Length of output: 162

🌐 Web query:

Terraform variable validation block syntax for string trimspace

💡 Result:

The syntax for a Terraform variable validation block using trimspace checks if a string variable has no leading or trailing whitespace by comparing the original value to its trimmed version. Example: variable "hostname" { type = string validation { condition = var.hostname == trimspace(var.hostname) error_message = "Hostname must not contain leading or trailing whitespace." } } This uses the built-in trimspace function, which removes Unicode whitespace (spaces, tabs, newlines, etc.) from both ends of the string. The validation block requires a boolean condition expression that must evaluate to true and a descriptive error_message. Terraform evaluates this during plan creation; if false, it errors with the message and halts. You can add multiple validation blocks for different checks or use trimspace(var.name) in locals for normalization without validation.

Citations:

Reject empty ACME emails at plan time.

This variable is required, but "" and whitespace-only values still pass and render an invalid ClusterIssuer manifest. Add validation so the failure happens during terraform plan, not after cert-manager tries to apply the manifest.

🔍 Suggested change 
 variable "email" {
     type = string
+    validation {
+        condition     = trimspace(var.email) != ""
+        error_message = "email must be non-empty."
+    }
 }
📝 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
variable "email" {
type = string
}
variable "email" {
type = string
validation {
condition = trimspace(var.email) != ""
error_message = "email must be non-empty."
}
}


variable "domain_name" {
type = string
}
Expand Down
0 terraform/dependencies.tf → terraform/versions.tf
View file
Open in desktop
File renamed without changes.
Loading