Conversation
Summary by CodeRabbit
WalkthroughAdds CI workflow to validate Terraform and extends Terraform to provision/consume a DigitalOcean Kubernetes cluster, providers (kubernetes, helm), Spaces buckets, database, Kubernetes namespace/secrets, and a Helm release to deploy GitLab with Helm values. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant TF as Terraform
participant DO as DigitalOcean API
participant K8s as Kubernetes API
participant Spaces as DigitalOcean Spaces
participant Helm as Helm (via provider)
TF->>DO: create/query kubernetes cluster & spaces buckets
DO-->>TF: cluster endpoint, kubeconfig, bucket names
TF->>K8s: create namespace `gitlab`
TF->>K8s: write secrets (postgres, redis, s3-main, s3-registry, s3-backup)
TF->>DO: create database resources (postgres)
TF->>Helm: install `gitlab` chart using values from `helm/gitlab/values.yaml`
Helm->>K8s: create GitLab resources in `gitlab` namespace
K8s->>Spaces: (GitLab) access object storage buckets for artifacts/uploads/registry/backups
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c675e5e5-fc45-4a2e-8667-f639565cf0f1
📒 Files selected for processing (11)
.github/workflows/terraform-validate.yamlMakefilehelm/gitlab/values.yamlterraform/.terraform.lock.hclterraform/cluster.tfterraform/dependencies.tfterraform/helm.tfterraform/kubernetes.tfterraform/postgres.tfterraform/providers.tfterraform/spaces.tf
📜 Review details
🧰 Additional context used
🪛 Checkov (3.2.513)
helm/gitlab/values.yaml
[low] 18-19: Base64 High Entropy String
(CKV_SECRET_6)
[low] 26-27: Base64 High Entropy String
(CKV_SECRET_6)
[low] 34-35: Base64 High Entropy String
(CKV_SECRET_6)
🪛 Trivy (0.69.3)
terraform/cluster.tf
[error] 11-11: Kubernetes clusters should be auto-upgraded to ensure that they always contain the latest security patches.
Kubernetes cluster does not have auto-upgrades enabled.
Rule: DIG-0008
Resource: digitalocean_kubernetes_cluster.main
(IaC/Digital Ocean)
🔇 Additional comments (5)
terraform/spaces.tf (1)
2-2: Rename propagation looks consistentThe local rename to
bucketsis correctly reflected infor_each, and this change is clean.Also applies to: 14-14
terraform/dependencies.tf (1)
9-16: Provider declarations are aligned with new IaC resourcesAdding explicit
kubernetesandhelmproviders here is the right dependency declaration for this PR.terraform/.terraform.lock.hcl (1)
1-86: Lockfile addition is correctCommitting the provider lockfile here is good for reproducible and tamper-evident Terraform runs.
terraform/providers.tf (1)
7-23: Provider wiring is consistentThe Kubernetes and Helm provider configurations are aligned and correctly reuse cluster endpoint/token/CA settings.
terraform/postgres.tf (1)
13-13: Name field changes do not trigger resource replacementThe
namefield ofdigitalocean_database_dbis not marked asForceNew=trueand does not force resource replacement. DigitalOcean's managed database API supports in-place renaming, so this change is safe.> Likely an incorrect or invalid review comment.
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| - dev |
There was a problem hiding this comment.
Validation is not wired for PRs
This runs only on pushes. Add a pull_request trigger so Terraform validation gates changes before merge.
Suggested workflow trigger update
on:
push:
branches:
- main
- dev
+ pull_request:
+ branches:
+ - main
+ - dev📝 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.
| on: | |
| push: | |
| branches: | |
| - main | |
| - dev | |
| on: | |
| push: | |
| branches: | |
| - main | |
| - dev | |
| pull_request: | |
| branches: | |
| - main | |
| - dev |
| appConfig: | ||
| object_store: | ||
| enabled: true | ||
| proxy_download: true | ||
| connection: | ||
| secret: gitlab-s3-main-secret | ||
| key: connection | ||
|
|
||
| arifacts: | ||
| bucket: ${buckets["artifacts"]} | ||
| connection: {} | ||
| uploads: | ||
| bucket: ${buckets["uploads"]} | ||
| connection: {} | ||
| packages: | ||
| bucket: ${buckets["packages"]} | ||
| connection: {} | ||
| lfs: | ||
| bucket: ${buckets["lfs"]} | ||
| connection: {} | ||
| backups: | ||
| bucket: ${buckets["backups"]} | ||
| tmpBucket: ${buckets["backups"]} | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the official GitLab Helm chart documentation for the chart version used here, how should external S3/Spaces credentials be configured for the container registry and for backups? Do those components inherit global.appConfig.object_store.connection, or do they require their own secret references?
💡 Result:
In the official GitLab Helm chart documentation, external S3/Spaces credentials for the container registry and backups do not inherit from global.appConfig.object_store.connection. They require their own dedicated secret references. For the container registry: Credentials are configured via a dedicated secret referenced by registry.storage.secret and registry.storage.key (typically a secret containing a config key with S3 details like s3: bucket: ...). The bucket is also set via global.registry.bucket. Examples include registry.s3.yaml, registry.gcs.yaml. This is separate from Rails object storage. For backups: Credentials are configured via the Toolbox pod's settings: gitlab.toolbox.backups.objectStorage.config.secret and gitlab.toolbox.backups.objectStorage.config.key (typically a secret with a .s3cfg file for S3, or equivalent for GCS/Azure). Buckets are global.appConfig.backups.bucket and global.appConfig.backups.tmpBucket. The backend is set via gitlab.toolbox.backups.objectStorage.backend (s3/gcs/azure). Backups do not use the Rails consolidated object_store connection. global.appConfig.object_store.connection is for Rails consolidated object storage (LFS, artifacts, uploads, etc.), not registry or backups. These components use independent configurations to allow different backends/buckets/credentials if needed.
Citations:
- 1: https://docs.gitlab.com/charts/advanced/external-object-storage/
- 2: https://docs.gitlab.com/charts/backup-restore/
- 3: https://docs.gitlab.com/16.10/charts/advanced/external-object-storage
- 4: https://gitlab.com/gitlab-org/charts/gitlab/blob/master/charts/registry/values.yaml
- 5: https://docs.gitlab.com/17.8/charts/advanced/external-object-storage
- 6: https://docs.gitlab.com/charts/backup-restore
- 7: https://docs.gitlab.com/16.11/charts/charts/registry/index.html
- 8: https://docs.gitlab.com/16.9/charts/charts/gitlab/toolbox/
🏁 Script executed:
# First, let's see the size of the values.yaml file
wc -l helm/gitlab/values.yaml
# Then search for registry and backup credential/secret configurations
echo "=== Searching for registry storage configuration ==="
rg -n "registry\." helm/gitlab/values.yaml | head -30
echo "=== Searching for backup object storage configuration ==="
rg -n "backups?" helm/gitlab/values.yaml -i | head -30
echo "=== Searching for toolbox backup configuration ==="
rg -n "toolbox" helm/gitlab/values.yaml -i | head -20Repository: clofour/gitlab-kubernetes
Length of output: 354
🏁 Script executed:
cat -n helm/gitlab/values.yamlRepository: clofour/gitlab-kubernetes
Length of output: 1651
🏁 Script executed:
# Check the terraform file to confirm registry and backup secrets exist
cat -n terraform/kubernetes.tf | sed -n '55,92p'Repository: clofour/gitlab-kubernetes
Length of output: 1413
Add registry and backup object storage configurations to the Helm values.
This file only wires the Rails-consolidated object storage (via gitlab-s3-main-secret), but the container registry and backups require their own dedicated secret references in the chart. According to the GitLab Helm chart documentation, these components do not inherit from global.appConfig.object_store.connection—they each need independent configurations:
- Registry: Requires
registry.storage.secretandregistry.storage.keypointing to the registry credentials secret - Backups: Requires
gitlab.toolbox.backups.objectStorage.config.secretandgitlab.toolbox.backups.objectStorage.config.keypointing to the backup credentials secret
Currently, neither is configured in values.yaml, leaving the registry and backup services unable to authenticate to their respective Spaces buckets. The secrets are created in Terraform (lines 55–92) but remain unconsumed.
🧰 Tools
🪛 Checkov (3.2.513)
[low] 34-35: Base64 High Entropy String
(CKV_SECRET_6)
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
.github/workflows/terraform-validate.yaml (1)
2-6:⚠️ Potential issue | 🟠 MajorWire Terraform validation to PR events, not only pushes.
This still skips validation during
pull_request, so infra changes can merge without this check running.Suggested trigger update
on: push: branches: - main - dev + pull_request: + branches: + - main + - devterraform/cluster.tf (1)
11-12:⚠️ Potential issue | 🟠 MajorEnable cluster auto-upgrades for patch hygiene.
auto_upgrade = falsekeeps control plane/nodes on potentially stale patch levels.Suggested change
- auto_upgrade = false + auto_upgrade = true surge_upgrade = truehelm/gitlab/values.yaml (1)
29-52:⚠️ Potential issue | 🟠 MajorRegistry and backup object-storage credential wiring still appears incomplete.
You configure consolidated Rails object storage, but registry/toolbox backup credential references are still not explicitly present in values.
For GitLab Helm chart version 9.10.3, which exact values keys configure external object storage credentials for: 1) container registry 2) toolbox backups Do these inherit from global.appConfig.object_store.connection, or require dedicated secret references?
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: de29d0ba-e8af-45a7-974e-bc928f892fd2
📒 Files selected for processing (11)
.github/workflows/terraform-validate.yamlMakefilehelm/gitlab/values.yamlterraform/.terraform.lock.hclterraform/cluster.tfterraform/dependencies.tfterraform/helm.tfterraform/kubernetes.tfterraform/postgres.tfterraform/providers.tfterraform/spaces.tf
📜 Review details
🧰 Additional context used
🪛 Checkov (3.2.513)
helm/gitlab/values.yaml
[low] 18-19: Base64 High Entropy String
(CKV_SECRET_6)
[low] 26-27: Base64 High Entropy String
(CKV_SECRET_6)
[low] 34-35: Base64 High Entropy String
(CKV_SECRET_6)
🪛 Trivy (0.69.3)
terraform/cluster.tf
[error] 11-11: Kubernetes clusters should be auto-upgraded to ensure that they always contain the latest security patches.
Kubernetes cluster does not have auto-upgrades enabled.
Rule: DIG-0008
Resource: digitalocean_kubernetes_cluster.main
(IaC/Digital Ocean)
🔇 Additional comments (7)
terraform/postgres.tf (1)
13-13: Database name alignment looks correct.This update cleanly aligns the provisioned DB name with the values wired into the GitLab Helm deployment.
terraform/spaces.tf (1)
2-2: Local rename is consistent and safe.
locals.bucketsandfor_each = local.bucketsare correctly aligned.Also applies to: 14-14
terraform/.terraform.lock.hcl (1)
1-86: Provider lockfile is correctly committed.Pinned provider versions and checksums improve reproducibility and integrity.
terraform/dependencies.tf (1)
9-16: Required provider declarations are correctly added.The new
kubernetesandhelmrequirements match the introduced provider usage.terraform/helm.tf (1)
1-29: Helm release wiring is solid overall.Pinned chart version plus templated infrastructure outputs is a good, deterministic setup.
terraform/kubernetes.tf (1)
55-91: Verify the chart consumes the renamed S3 Secret names.Renaming these Secrets fixes the Kubernetes object collision, but registry and backup storage will still be miswired if
helm/gitlab/values.yamlcontinues to reference onlygitlab-s3-main-secretor the old names.#!/bin/bash # Verify that Helm values now reference the renamed registry and backup S3 secrets. rg -n -C3 'gitlab-s3-(main|registry|backup)-secret' helm terraform/helm.tf rg -n -C3 'registry|backup|object_store' helm/gitlab/values.yamlterraform/providers.tf (1)
15-22: The Helm provider configuration is correct as written. Terraform Helm provider v3.0.0+ requires thekubernetes = { ... }object syntax, not the block syntax. The current code matches the documented correct syntax and will pass validation. The proposed change would break the configuration.> Likely an incorrect or invalid review comment.
| resource "kubernetes_secret_v1" "gitlab_postgres" { | ||
| metadata { | ||
| name = "gitlab-postgres-secret" | ||
| namespace = kubernetes_namespace_v1.gitlab.metadata[0].name | ||
| } | ||
|
|
||
| data = { | ||
| password = digitalocean_database_user.gitlab.password | ||
| } | ||
|
|
||
| type = "Opaque" | ||
| } | ||
|
|
||
| resource "digitalocean_kubernetes_cluster" "main" { | ||
| name = var.cluster_name | ||
| region = var.region | ||
| version = data.digitalocean_kubernetes_versions.current.latest_version | ||
| vpc_uuid = digitalocean_vpc.main.id | ||
| resource "kubernetes_secret_v1" "gitlab_redis" { | ||
| metadata { | ||
| name = "gitlab-redis-secret" | ||
| namespace = kubernetes_namespace_v1.gitlab.metadata[0].name | ||
| } | ||
|
|
||
| data = { | ||
| password = digitalocean_database_cluster.valkey.password | ||
| } | ||
|
|
||
| auto_upgrade = false | ||
| surge_upgrade = true | ||
| type = "Opaque" | ||
| } | ||
|
|
||
| resource "kubernetes_secret_v1" "gitlab_s3_main" { | ||
| metadata { | ||
| name = "gitlab-s3-main-secret" | ||
| namespace = kubernetes_namespace_v1.gitlab.metadata[0].name | ||
| } | ||
|
|
||
| node_pool { | ||
| name = "default" | ||
| size = var.node_size | ||
| node_count = var.node_count | ||
| labels = { role = "general" } | ||
| data = { | ||
| connection = yamlencode({ | ||
| provider = "AWS" | ||
| region = var.region | ||
| endpoint = "https://${var.region}.digitaloceanspaces.com" | ||
| aws_access_key_id = var.spaces_access_id | ||
| aws_secret_access_key = var.spaces_secret_key | ||
| path_style = true | ||
| }) | ||
| } | ||
|
|
||
| type = "Opaque" | ||
| } | ||
|
|
||
| resource "kubernetes_secret_v1" "gitlab_s3_registry" { | ||
| metadata { | ||
| name = "gitlab-s3-registry-secret" | ||
| namespace = kubernetes_namespace_v1.gitlab.metadata[0].name | ||
| } | ||
|
|
||
| data = { | ||
| connection = yamlencode({ | ||
| accesskey = var.spaces_access_id | ||
| secretkey = var.spaces_secret_key | ||
| region = var.region | ||
| regionendpoint = "https://${var.region}.digitaloceanspaces.com" | ||
| bucket = digitalocean_spaces_bucket.gitlab["registry"].name | ||
| }) | ||
| } | ||
|
|
||
| type = "Opaque" | ||
| } | ||
|
|
||
| resource "kubernetes_secret_v1" "gitlab_s3_backup" { | ||
| metadata { | ||
| name = "gitlab-s3-backup-secret" | ||
| namespace = kubernetes_namespace_v1.gitlab.metadata[0].name | ||
| } | ||
|
|
||
| data = { | ||
| connection = yamlencode({ | ||
| provider = "AWS" | ||
| region = var.region | ||
| endpoint = "https://${var.region}.digitaloceanspaces.com" | ||
| aws_access_key_id = var.spaces_access_id | ||
| aws_secret_access_key = var.spaces_secret_key | ||
| path_style = true | ||
| }) | ||
| } | ||
|
|
||
| type = "Opaque" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Back this stack with an encrypted, access-restricted state backend.
These kubernetes_secret_v1 resources copy database and Spaces credentials into Terraform state as well as into the cluster. Make sure the backend is remote, encrypted, and tightly scoped before rollout.
1 participant
No description provided.