Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR transitions cluster deployment from Terraform-managed Helm releases to a Flux CD GitOps model. It introduces Flux bootstrap configuration, manifests for infrastructure components (cert-manager, envoy-gateway, external-dns, gitlab, reflector), package sources (Git/Helm/OCI repositories), configuration values via Kubernetes ConfigMaps, and removes equivalent Terraform Helm provider code. Changes
Sequence DiagramsequenceDiagram
participant Git as Git Repository
participant Flux as Flux Bootstrap
participant K8s as Kubernetes Cluster
participant Kustomization as Cluster Kustomization
participant Sources as Package Sources
participant Releases as Helm Releases
participant Apps as Applications
Flux->>Git: Clone flux/ directory
Git-->>Flux: Return manifests
Flux->>K8s: Apply bootstrap configuration
K8s->>Kustomization: Process cluster-kustomization.yaml
Kustomization->>K8s: Load shared-values & runtime-values ConfigMaps
Kustomization->>Sources: Reference cert-manager, envoy-proxy, external-dns, gitlab repositories
Sources->>K8s: Register HelmRepository & OCIRepository resources
Kustomization->>Releases: Apply HelmRelease manifests with dependency ordering
Releases->>Releases: Resolve dependencies (cert-manager → cluster-issuer → dns01-certificate → envoy-gateway → external-dns)
Releases->>Sources: Fetch Helm charts from repositories
Sources-->>Releases: Return chart artifacts
Releases->>K8s: Deploy applications via Helm with templated values
K8s->>Apps: Create namespaces, deployments, services, configmaps
Apps-->>K8s: Services ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
…e for this use case
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c6fd6f0c-898f-4448-aeee-3d984089afb5
📒 Files selected for processing (24)
flux/bootstrap/cluster-kustomization.yamlflux/bootstrap/shared-values.yamlflux/kustomization.yamlflux/releases/cert-manager.yamlflux/releases/cluster-issuer.yamlflux/releases/dns01-certificate.yamlflux/releases/envoy-gateway.yamlflux/releases/external-dns.yamlflux/releases/gateway-config.yamlflux/releases/gitlab.yamlflux/releases/reflector.yamlflux/repositories/emberstack.yamlflux/repositories/envoy-proxy.yamlflux/repositories/external-dns.yamlflux/repositories/github.yamlflux/repositories/gitlab.yamlflux/repositories/jetstack.yamlterraform/dns.tfterraform/fluxcd.tfterraform/helm.tfterraform/kubernetes.tfterraform/providers.tfterraform/variables.tfterraform/versions.tf
💤 Files with no reviewable changes (3)
- terraform/dns.tf
- terraform/helm.tf
- terraform/kubernetes.tf
📜 Review details
🧰 Additional context used
🪛 Checkov (3.2.525)
flux/bootstrap/shared-values.yaml
[low] 1-6: The default namespace should not be used
(CKV_K8S_21)
flux/releases/gitlab.yaml
[low] 31-32: Base64 High Entropy String
(CKV_SECRET_6)
[low] 62-63: Base64 High Entropy String
(CKV_SECRET_6)
[low] 71-72: Base64 High Entropy String
(CKV_SECRET_6)
[low] 82-83: Base64 High Entropy String
(CKV_SECRET_6)
[low] 119-120: Base64 High Entropy String
(CKV_SECRET_6)
🪛 Trivy (0.69.3)
flux/bootstrap/shared-values.yaml
[warning] 1-1: ConfigMap with sensitive content
ConfigMap 'shared-values' in 'default' namespace stores sensitive contents in key(s) or value(s) '{"key"}'
Rule: KSV-01010
(IaC/Kubernetes)
🪛 YAMLlint (1.38.0)
flux/releases/dns01-certificate.yaml
[error] 25-25: too many blank lines (1 > 0)
(empty-lines)
🔇 Additional comments (4)
terraform/variables.tf (1)
52-54: LGTM.New variable is consistent with the existing style in the file.
flux/repositories/external-dns.yaml (1)
1-7: LGTM — correctapiVersion, valid repository URL, namespace scoped toflux-system.flux/repositories/gitlab.yaml (1)
1-7: LGTM — correctapiVersion, validhttps://charts.gitlab.ioURL.flux/releases/reflector.yaml (1)
12-15:⚠️ Potential issue | 🟠 MajorCorrect the Emberstack HelmRepository URL — singular "helm-chart" will cause chart fetch to fail.
The URL in
flux/repositories/emberstack.yamlishttps://emberstack.github.io/helm-chart(singular), but the official Emberstack Helm repository is athttps://emberstack.github.io/helm-charts(plural). The incorrect singular form will return a 404 when Flux's source-controller attempts to fetch the chart index, causing thereflectorHelmRelease to fail.Update the URL in
flux/repositories/emberstack.yaml:diff
spec: - url: https://emberstack.github.io/helm-chart + url: https://emberstack.github.io/helm-charts> Likely an incorrect or invalid review comment.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (8)
flux/repositories/github.yaml (1)
1-9:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd required
spec.intervalfield.The
spec.intervalfield remains missing and is required by the Flux GitRepository v1 API. Without it, this resource will fail CRD validation and prevent reconciliation of dependent resources (cluster-issuer, dns01-certificate, gateway-config HelmReleases).🔧 Proposed fix
spec: + interval: 1m url: https://github.com/clofour/gitlab-kubernetes ref: branch: mainflux/repositories/cert-manager.yaml (2)
1-9:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd required
spec.intervalfield.The
spec.intervalfield is required in Flux OCIRepository resources. Without it, the resource will fail CRD validation and the cert-manager HelmRelease cannot reconcile.🔧 Proposed fix
spec: + interval: 5m url: oci://quay.io/jetstack/charts/cert-manager ref: tag: 1.20.2
6-9: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider adding
layerSelectorfor Helm chart OCI sources.When using an OCIRepository as a Helm chart source, specifying a
layerSelectorwith the Helm chart media type is recommended to explicitly target the correct OCI layer and improve supply-chain clarity.♻️ Proposed addition
spec: interval: 5m url: oci://quay.io/jetstack/charts/cert-manager + layerSelector: + mediaType: "application/vnd.cncf.helm.chart.content.v1.tar+gzip" + operation: copy ref: tag: 1.20.2flux/bootstrap/shared-values.yaml (1)
6-7:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace placeholder content with actual substitution variables.
The
data: key: valueplaceholder provides no real substitution variables. References like${email}incluster-issuer.yamland other${...}tokens in HelmRelease values will remain unresolved at runtime, breaking cluster configuration.🔧 Proposed fix (populate with actual values)
data: - key: value + email: "admin@example.com" + domain_name: "example.com" + # Add other shared variables referenced in HelmReleasesflux/repositories/emberstack.yaml (1)
1-7:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd required
spec.intervalfield.The
spec.intervalfield is required in Flux HelmRepository resources. Without it, the resource will fail CRD validation and the reflector HelmRelease cannot reconcile.🔧 Proposed fix
spec: + interval: 10m url: https://emberstack.github.io/helm-chartsflux/releases/cert-manager.yaml (1)
8-13:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
spec.chart.spec.chartRefis an invalid field placement — the cert-manager HelmRelease will not reconcile.The previous review flagged that fields were misplaced inside
spec.chart.spec. WhiletargetNamespace,install, andvalueshave been moved tospeclevel,chartRefis still nested insidespec.chart.spec.
HelmChartTemplateSpec(the type ofspec.chart.spec) only accepts:chart,version,sourceRef,interval,reconcileStrategy,valuesFiles,ignoreMissingValuesFiles, andverify.spec.chart.specvalues are used by the helm-controller as a template to create a new HelmChart resource.chartRefis not valid there and will be pruned, leaving the controller unable to locate any chart source.
spec.chartRefis used to reference an OCIRepository directly — it belongs at the top-levelspec, not insidespec.chart.spec.chartandspec.chartRefare mutually exclusive; removespec.chartentirely.🔧 Proposed fix
spec: interval: 1m - chart: - spec: - chartRef: - kind: OCIRepository - name: cert-manager - namespace: flux-system + chartRef: + kind: OCIRepository + name: cert-manager + namespace: flux-system targetNamespace: cert-managerterraform/providers.tf (1)
27-29:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing Git authentication —
flux_bootstrap_gitwill fail to clone/push to the private repository.The
gitblock only setsurl; nosshorhttpauthentication sub-block is present. Without credentials, the Flux provider cannot authenticate to the private repository during bootstrap.🔒 Proposed fix (SSH example)
git = { url = var.git_repo + ssh = { + username = "git" + private_key = var.flux_git_ssh_private_key + } }Or for HTTPS:
git = { url = var.git_repo + http = { + username = "oauth2" + password = var.flux_git_token + } }flux/releases/cluster-issuer.yaml (1)
21-21:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify
${email}is defined in theruntime-valuesConfigMap.The Flux kustomize substitution for
${email}relies on this key being present in one of the referenced ConfigMaps. Theshared-valuesConfigMap has noruntime-valuesConfigMap (created by Terraform) also omits it, substitution will silently produce an empty string and theClusterIssuerwill be created with a blank email, breaking Let's Encrypt ACME registration.#!/bin/bash # Verify that terraform/fluxcd.tf creates the runtime-values ConfigMap with an email key fd "fluxcd.tf" --exec cat {} # Also check all ConfigMap definitions for an 'email' key rg -n "email" --type yaml flux/
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b26f23b6-690e-4521-a4fc-e64ddfb066e1
📒 Files selected for processing (24)
flux/bootstrap/cluster-kustomization.yamlflux/bootstrap/shared-values.yamlflux/kustomization.yamlflux/releases/cert-manager.yamlflux/releases/cluster-issuer.yamlflux/releases/dns01-certificate.yamlflux/releases/envoy-gateway.yamlflux/releases/external-dns.yamlflux/releases/gateway-config.yamlflux/releases/gitlab.yamlflux/releases/reflector.yamlflux/repositories/cert-manager.yamlflux/repositories/emberstack.yamlflux/repositories/envoy-proxy.yamlflux/repositories/external-dns.yamlflux/repositories/github.yamlflux/repositories/gitlab.yamlterraform/dns.tfterraform/fluxcd.tfterraform/helm.tfterraform/kubernetes.tfterraform/providers.tfterraform/variables.tfterraform/versions.tf
💤 Files with no reviewable changes (3)
- terraform/kubernetes.tf
- terraform/helm.tf
- terraform/dns.tf
📜 Review details
🧰 Additional context used
🪛 Checkov (3.2.525)
flux/releases/gitlab.yaml
[low] 31-32: Base64 High Entropy String
(CKV_SECRET_6)
[low] 62-63: Base64 High Entropy String
(CKV_SECRET_6)
[low] 71-72: Base64 High Entropy String
(CKV_SECRET_6)
[low] 82-83: Base64 High Entropy String
(CKV_SECRET_6)
[low] 119-120: Base64 High Entropy String
(CKV_SECRET_6)
🪛 Trivy (0.69.3)
flux/bootstrap/shared-values.yaml
[warning] 1-1: ConfigMap with sensitive content
ConfigMap 'shared-values' in 'flux-system' namespace stores sensitive contents in key(s) or value(s) '{"key"}'
Rule: KSV-01010
(IaC/Kubernetes)
🔇 Additional comments (14)
terraform/variables.tf (1)
52-54: LGTM!The
git_repovariable is correctly defined for the Flux provider's Git URL configuration.terraform/versions.tf (1)
17-19: Orphanedprovider "helm"block has been successfully removed.The
helmprovider configuration block interraform/providers.tfhas been removed along with its removal fromrequired_providers. No action needed.flux/repositories/external-dns.yaml (1)
1-7: LGTM!Correct
HelmRepositorydefinition pointing at the officialexternal-dnsHelm index.flux/releases/cluster-issuer.yaml (1)
1-21: LGTM — structural issues from the previous review are resolved.All
spec-level fields (interval,dependsOn,targetNamespace,install,values) are now correctly placed, anddependsOnuses the required- name:format.flux/releases/external-dns.yaml (1)
1-36: LGTM — all critical structural issues from the previous review are resolved.
interval,dependsOn,targetNamespace,install, andvaluesare correctly positioned atspeclevel;dependsOnuses the required- name: reflectorformat;spec.chart.speccontains only the validHelmChartTemplateSpecfields.flux/releases/dns01-certificate.yaml (1)
1-24: LGTM — critical structural issues from the previous review are resolved.
interval,dependsOn(with correct- name:format for both dependencies),targetNamespace,install, andvaluesare all correctly placed atspeclevel.flux/releases/reflector.yaml (1)
1-18: LGTM — critical structural issues from the previous review are resolved.
targetNamespaceandinstallare now atspeclevel, andspec.chart.speccontains only validHelmChartTemplateSpecfields. TheemberstackHelmRepository URL also shows the corrected plural path (/helm-charts).flux/releases/gateway-config.yaml (1)
1-22: LGTM — all critical structural issues from the previous review are resolved.
interval,dependsOn(- name: envoy-gateway),targetNamespace,install, andvaluesare all atspeclevel;spec.chart.speccontains only validHelmChartTemplateSpecfields.flux/bootstrap/cluster-kustomization.yaml (2)
9-11:sourceRef.name: githubnow correctly matches the bootstrap GitRepository.The previous
gitlab-kubernetesreference (non-existent) is resolved. ThegithubGitRepository influx/repositories/github.yamlprovides the correct source.
13-17:runtime-valuesis now created by Terraform — root cause resolved;optional: truestill missing for bootstrap window.The ConfigMap is created in
terraform/fluxcd.tfwithdepends_on = [flux_bootstrap_git.flux_cd], so it will exist for steady-state reconciliations. However, the very first reconciliation cycle afterflux_bootstrap_gitcompletes (before Terraform creates the ConfigMap) will still fail withoutoptional: true. Consider addingoptional: truefor robustness during the bootstrap window.flux/kustomization.yaml (1)
3-5:bootstrap/shared-values.yamlandrepositories/github.yamladditions resolve the previous reconciliation blockers.Both past issues (missing
shared-valuesConfigMap and missing bootstrap GitRepository) are now fixed.terraform/fluxcd.tf (1)
1-34: Both previously flagged issues are resolved:depends_onadded, and bucket values correctly flattened to individual string keys.The
depends_on = [flux_bootstrap_git.flux_cd]ordering guard is in place, and the bucket entries (buckets_artifacts,buckets_uploads, etc.) now match the flat${buckets_*}substitution keys used across the HelmRelease values files.flux/releases/gitlab.yaml (2)
1-14: All three previously flagged critical issues are resolved: namespace corrected, spec structure fixed, and bucket variable syntax flattened.
namespace: flux-system✓dependsOn,targetNamespace,install, andvaluesare correctly placed atspeclevel ✓- Bucket substitution now uses
${buckets_artifacts}/${buckets_uploads}/ etc., matching the flat keys in the Terraform ConfigMap ✓
26-128: CheckovCKV_SECRET_6findings are false positives — no action needed.The flagged lines (31–32, 62–63, 71–72, 82–83, 119–120) all follow the pattern
secret: <k8s-secret-name>/key: password. The string literal"password"is a Kubernetes Secret key name, not a credential value. No secrets are embedded in this file.
1 participant
…migration
Summary by CodeRabbit
New Features
Chores