-
Notifications
You must be signed in to change notification settings - Fork 136
[fluxcd] Add flux-aio module and migration #1698
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.
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
Conversation
WalkthroughThe PR introduces a new FluxCD AIO Helm chart package replacing the operator-based approach. Changes include new flux deployment automation, TLS certificate provisioning, HTTPS proxy configuration for Helm repositories, removal of legacy flux operator references from bundle definitions, and container image updates for enhanced tooling. Changes
Sequence DiagramsequenceDiagram
participant I as Installer Script
participant FK as Flux-AIO<br/>(Makefile)
participant K8s as Kubernetes API
participant Certs as Certificate<br/>Script
participant Assets as Assets<br/>Service
I->>+FK: invoke: make apply
FK->>+K8s: kubectl apply (Flux AIO)
K8s-->>-FK: ✓ Flux deployed
FK->>+K8s: wait CRDs ready
K8s-->>-FK: ✓ CRDs available
FK-->>-I: ✓ Flux installed
I->>+Certs: issue-flux-certificates.sh
Certs->>+K8s: create CSR<br/>(cozystack-assets-reader)
K8s-->>Certs: CSR submitted
Certs->>K8s: approve CSR
K8s-->>Certs: ✓ signed cert
Certs->>+K8s: create secrets<br/>(cozystack-assets-tls)
K8s-->>-Certs: ✓ secrets in<br/>cozy-system,<br/>cozy-public
Certs-->>-I: ✓ certs issued
I->>+K8s: HelmRepository configured<br/>with HTTPS proxy + TLS
K8s->>Assets: fetch via<br/>kubernetesAPIEndpoint<br/>(/api/v1/namespaces/...)
Assets-->>K8s: ✓ repos available
K8s-->>-I: ✓ repos synced
I->>I: reconcile platform
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Key areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @kvaps, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces the flux-aio module integration and updates the installer script to support it, migrating from an older FluxCD setup. The changes involve adding new CUE and YAML definitions for flux-aio, modifying the Makefile for building and applying the module, and updating the main installer.sh script to use the new install_flux function. A migration script for version 21 to 22 is also included to clean up the old FluxCD HelmRelease. Overall, the changes align with the goal of integrating the new module, but there are a few areas that could be improved for stability, maintainability, and clarity regarding functional changes.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/core/flux-aio/flux-aio.cue (8)
Using version: "latest" for the flux-aio module can lead to unexpected behavior and lack of reproducibility in production environments. It is a best practice to pin to a specific, immutable version tag to ensure consistent deployments and easier debugging.
scripts/installer.sh (52-58)
The install_basic_charts function, which was responsible for installing components like Cilium and KubeOVN, has been removed. The pull request description focuses on adding flux-aio integration but does not explicitly mention the removal of these core components from the installer. This could lead to a functional regression if these components are still expected as part of the CozyStack installation.
Please clarify if the removal of Cilium and KubeOVN installation is intentional and if their functionality is now handled by flux-aio or if they are no longer required. If they are still required, their installation logic needs to be re-integrated or explicitly handled by the new flux-aio module.
packages/core/flux-aio/Makefile (15)
The sed command on this line contains repetitive patterns for removing .cluster.local variations. This can be simplified for better readability and maintainability by combining the multiple s commands into a single, more concise regular expression.
sed -e 's|\.cluster\.local[.,]*||g' -e '/timoni/d' -i manifests/fluxcd.yaml
packages/core/flux-aio/values.yaml (8)
Specifying version: 2.7.x for the Flux distribution uses a floating minor version. While common, pinning to a specific patch version (e.g., 2.7.5) is generally recommended for better reproducibility and to avoid unexpected updates from upstream changes that might introduce breaking changes or regressions.
packages/core/flux-aio/values.yaml (33-34)
The resource limits for CPU (2000m) and memory (2048Mi) are quite high for default controller settings. While Flux controllers can be resource-intensive, these values might be overly generous for typical deployments and could lead to inefficient resource utilization in smaller clusters. Consider if these are optimal defaults or if they should be configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/core/flux-aio/Chart.yaml (1)
1-3: Consider implementing the umbrella chart pattern per coding guidelines.As per coding guidelines, Helm charts should follow the umbrella pattern and vendor upstream charts in a
charts/directory. Currently, this chart descriptor has no dependencies or vendored charts. The flux-aio module is referenced via OCI influx-aio.cue, but the Helm chart itself doesn't follow the documented pattern.Consider either:
- Adding upstream Flux charts as dependencies in
Chart.yaml, or- Vendoring the upstream charts in
packages/core/flux-aio/charts/Based on coding guidelines, this approach would provide better alignment with the established chart management pattern in the repository.
packages/core/flux-aio/Makefile (1)
13-16: Document the sed post-processing rationale.The
updatetarget uses a complexsedcommand with multiple patterns to removecluster.localreferences andtimonilines. Consider adding a comment explaining why this post-processing is necessary, as it could break with upstream changes.Apply this diff to add documentation:
update: + # Build manifests from CUE bundle and post-process: + # - Remove cluster.local DNS suffixes for compatibility + # - Remove timoni metadata lines timoni bundle build -f flux-aio.cue > manifests/fluxcd.yaml sed -e 's|\.cluster\.local\.,||g' -e 's|\.cluster\.local\,||g' -e 's|\.cluster\.local\.||g' -e '/timoni/d' -i manifests/fluxcd.yaml + # Set privileged pod security for flux-aio namespace yq eval '(select(.kind == "Namespace") | .metadata.labels."pod-security.kubernetes.io/enforce") = "privileged"' -i manifests/fluxcd.yaml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/core/flux-aio/Chart.yaml(1 hunks)packages/core/flux-aio/Makefile(1 hunks)packages/core/flux-aio/flux-aio.cue(1 hunks)packages/core/flux-aio/values.yaml(1 hunks)packages/core/installer/templates/cozystack.yaml(1 hunks)scripts/installer.sh(2 hunks)scripts/migrations/21(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/Chart.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Use Helm Charts with the umbrella pattern and vendor upstream charts in
charts/directory
Files:
packages/core/flux-aio/Chart.yaml
🧠 Learnings (1)
📚 Learning: 2025-11-27T11:21:45.808Z
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T11:21:45.808Z
Learning: Applies to **/Chart.yaml : Use Helm Charts with the umbrella pattern and vendor upstream charts in `charts/` directory
Applied to files:
packages/core/flux-aio/Chart.yamlscripts/installer.sh
🪛 checkmake (0.2.2)
packages/core/flux-aio/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (4)
scripts/migrations/21 (1)
1-10: LGTM!The migration script correctly handles the upgrade from version 21 to 22 with appropriate error handling (
set -euo pipefail), safe deletion of the FluxCD HelmRelease with--ignore-not-found, and proper version stamping.packages/core/installer/templates/cozystack.yaml (1)
57-58: LGTM!The addition of the
INSTALL_FLUXenvironment variable correctly enables the Flux installation pathway in the updated installer script.scripts/installer.sh (1)
22-28: LGTM! Simplified Flux installation.The new
install_fluxfunction provides a streamlined approach compared to the previous multi-step installation logic. The function appropriately:
- Short-circuits when
INSTALL_FLUXis not "true"- Delegates to the Makefile for manifest application
- Waits for required CRDs before proceeding
packages/core/flux-aio/values.yaml (1)
1-55: LGTM! Well-structured Flux configuration.The Flux Helm values are comprehensive and well-documented:
- Appropriate resource limits and concurrency settings for production use
- Correctly formatted JSON patches for all component deployments
- Proper service DNS addressing for internal communication
- Documented reasoning for networkPolicy disabling
7e48f94 to
fea9095
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/core/flux-aio/Makefile (1)
1-16: Mark non-file targets as phony and consider dropping unused vars.The targets here don’t produce files and should be marked
.PHONYto avoid surprising no‑ops if files namedshow,apply,diff, orupdateever appear. This was already flagged in a previous review and still applies. Also,NAME/NAMESPACEare currently unused; you can either start using them or remove them to keep the file minimal.You could apply something like:
-NAME=flux-aio -NAMESPACE=cozy-$(NAME) - -show: +.PHONY: show apply diff update + +NAME=flux-aio +NAMESPACE=cozy-$(NAME) + +show: kubectl apply -R -f manifests/ --dry-run=client -o yaml(or drop
NAME/NAMESPACEentirely if they’re not needed elsewhere).
🧹 Nitpick comments (3)
scripts/migrations/21 (1)
1-10: Check/bin/shcompatibility withset -euo pipefail.The migration logic looks fine, but
set -euo pipefailis not supported by all/bin/shimplementations. If your images usedash/BusyBoxsh, this may fail at startup. Consider either switching the shebang to a shell that supportspipefail(e.g./usr/bin/env bash) or droppingpipefailhere if you need strict POSIX/bin/shportability.scripts/installer.sh (1)
22-27: Consider defaultingINSTALL_FLUXto true for backward compatibility.With the new gate, running
scripts/installer.shdirectly withoutINSTALL_FLUXset will silently skip Flux installation, which is a behavior change compared to always ensuring Flux was present. If you want the in-cluster flag to disable Flux when set to something else but keep manual runs installing Flux by default, you could default the variable to"true":-install_flux() { - if [ "$INSTALL_FLUX" != "true" ]; then +install_flux() { + if [ "${INSTALL_FLUX:-true}" != "true" ]; then return fi make -C packages/core/flux-aio apply wait_for_crds helmreleases.helm.toolkit.fluxcd.io helmrepositories.source.toolkit.fluxcd.io }This keeps the new
INSTALL_FLUXknob while avoiding surprises for users invoking the installer script directly.Also applies to: 43-46
scripts/issue-flux-certificates.sh (1)
13-15: Fix trap quoting and optionally clean up CSR objects.The overall flow looks good. Two small robustness tweaks:
- ShellCheck’s SC2064 is right: the trap should use single quotes and quote
$TMPDIRso the value is preserved safely:-TMPDIR=$(mktemp -d) -trap "rm -rf $TMPDIR" EXIT +TMPDIR=$(mktemp -d) +trap 'rm -rf "$TMPDIR"' EXIT
- To avoid accumulating
CertificateSigningRequestobjects over time, consider deleting the CSR after you’ve retrieved the certificate:kubectl get csr "${CSR_NAME}" \ -o jsonpath='{.status.certificate}' | base64 -d > tls.crt + +kubectl delete csr "${CSR_NAME}" || trueBoth changes are low-risk and make the script a bit cleaner and more robust.
Also applies to: 36-45
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/core/flux-aio/Chart.yaml(1 hunks)packages/core/flux-aio/Makefile(1 hunks)packages/core/flux-aio/flux-aio.cue(1 hunks)packages/core/flux-aio/values.yaml(1 hunks)packages/core/installer/templates/cozystack.yaml(1 hunks)packages/core/platform/templates/helmrepos.yaml(3 hunks)scripts/installer.sh(2 hunks)scripts/issue-flux-certificates.sh(1 hunks)scripts/migrations/21(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/flux-aio/Chart.yaml
- packages/core/flux-aio/flux-aio.cue
- packages/core/flux-aio/values.yaml
🧰 Additional context used
🪛 checkmake (0.2.2)
packages/core/flux-aio/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🪛 Shellcheck (0.11.0)
scripts/issue-flux-certificates.sh
[warning] 14-14: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (2)
packages/core/installer/templates/cozystack.yaml (1)
55-58:INSTALL_FLUXflag wiring matches installer logic.Setting
INSTALL_FLUXto"true"here aligns withinstall_flux()inscripts/installer.sh, so the in-cluster cozystack deployment will execute the Flux AIO install path as intended.packages/core/platform/templates/helmrepos.yaml (1)
10-13: HelmRepository proxy URL + TLS wiring looks consistent; confirm secret shape.The move to the Kubernetes API proxy URL and shared
certSecretRef: cozystack-assets-tlsis consistent across all three repositories and matches the new certificate helper script. Please just double‑check that thecozystack-assets-tlssecrets incozy-systemandcozy-publicuse the data keys expected bycertSecretRef(tls.crt/tls.keyand optionalca.crt) so Flux can consume them without additional configuration.Also applies to: 25-27, 38-40
9707dd4 to
2393550
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/core/flux-aio/Makefile (1)
4-11: Add .PHONY declarations for all targets.This concern was already raised in a previous review. The targets
show,apply,diff, andupdateshould be marked as phony.packages/core/flux-aio/flux-aio.cue (1)
6-9: Pin the module version instead of using "latest".This concern was already raised in a previous review. The module version should be pinned to ensure reproducible builds and avoid unexpected breaking changes.
🧹 Nitpick comments (6)
packages/core/platform/images/cozystack-assets/Dockerfile (1)
1-25: New cozystack-assets image build looks correct; consider minor hardeningThe build pipeline (Go build ➝
make repos➝ copy_out/reposanddashboards) is coherent and should produce a self-contained assets image.Two optional improvements you might consider:
- Run as non-root in the final stage (e.g., add a dedicated user and
USERdirective) if there are no root-only needs.- If the binary is fully static/pure Go, explicitly setting
CGO_ENABLED=0for thego buildcan make the-extldflags "-static"intent clearer and slightly reduce toolchain coupling.These are non-blocking polish items.
packages/core/platform/Makefile (1)
4-34: image/image-assets workflow is sound; declare image targets PHONYThe new
image-assetstarget cleanly:
- Builds the cozystack-assets image with buildx and cache hints.
- Extracts the digest from
images/cozystack-assets.json.- Writes the fully qualified reference into
values.yamlas.assets.image.A couple of minor Makefile improvements:
- Mark the image targets as phony (also matches the checkmake hint), since they don’t correspond to real files:
NAME=platform NAMESPACE=cozy-system include ../../../scripts/common-envs.mk +.PHONY: image image-assets + show:
- Since
docker buildx builddoes not load or push by default, ensure$(BUILDX_ARGS)is configured (incommon-envs.mkor CI) with--pushor--loadas appropriate for your workflow.packages/core/platform/templates/cozystack-assets.yaml (3)
31-32: Overly permissive toleration.Using
operator: Existstolerates all taints, meaning this pod can be scheduled on any node regardless of taints (including nodes marked for maintenance or with specific workload restrictions).Consider specifying explicit tolerations:
tolerations: -- operator: Exists +- key: node-role.kubernetes.io/control-plane + operator: Exists + effect: NoSchedule
20-26: Add resource limits and requests.The container has no resource limits or requests defined, which can lead to resource contention and scheduling issues.
Add resource specifications:
containers: - name: assets-server image: "{{ .Values.assets.image }}" command: - /usr/bin/cozystack-assets-server - "-dir=/cozystack/assets" - "-address=:8123" + resources: + requests: + memory: "64Mi" + cpu: "100m" + limits: + memory: "128Mi" + cpu: "200m"
20-30: Add liveness and readiness probes.The container lacks health check probes, which means Kubernetes cannot determine if the container is healthy or ready to serve traffic. This can result in traffic being routed to unhealthy pods.
Add health probes:
- name: assets-server image: "{{ .Values.assets.image }}" command: - /usr/bin/cozystack-assets-server - "-dir=/cozystack/assets" - "-address=:8123" ports: - name: http containerPort: 8123 hostPort: 8123 + livenessProbe: + httpGet: + path: / + port: 8123 + initialDelaySeconds: 10 + periodSeconds: 10 + readinessProbe: + httpGet: + path: / + port: 8123 + initialDelaySeconds: 5 + periodSeconds: 5scripts/installer.sh (1)
55-64: Consider more robust parsing for HelmRelease manipulation.The shell loops parsing kubectl output work but are fragile to output format changes or unusual characters in names.
Consider using
kubectl ... -o json | jqfor more robust parsing:# Example for line 55-61 kubectl get hr -A -o json | jq -r '.items[] | select(.spec.suspend) | "\(.spec.chart.spec.sourceRef.namespace)/\(.spec.chart.spec.sourceRef.name) \(.metadata.namespace) \(.metadata.name)"' | while read repo namespace name; do case "$repo" in cozy-system/cozystack-system|cozy-public/cozystack-extra|cozy-public/cozystack-apps) kubectl patch hr -n "$namespace" "$name" -p '{"spec": {"suspend": null}}' --type=merge --field-manager=flux-client-side-apply ;; esac done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
Makefile(1 hunks)packages/core/flux-aio/Chart.yaml(1 hunks)packages/core/flux-aio/Makefile(1 hunks)packages/core/flux-aio/flux-aio.cue(1 hunks)packages/core/installer/images/cozystack/Dockerfile(1 hunks)packages/core/installer/templates/cozystack.yaml(1 hunks)packages/core/platform/Makefile(2 hunks)packages/core/platform/images/cozystack-assets/Dockerfile(1 hunks)packages/core/platform/templates/_helpers.tpl(1 hunks)packages/core/platform/templates/cozystack-assets.yaml(1 hunks)packages/core/platform/templates/helmrepos.yaml(3 hunks)packages/core/platform/values.yaml(1 hunks)packages/extra/monitoring/templates/dashboards.yaml(1 hunks)scripts/installer.sh(2 hunks)scripts/issue-flux-certificates.sh(1 hunks)scripts/migrations/21(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/flux-aio/Chart.yaml
- scripts/migrations/21
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-07-26T18:12:05.641Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1216
File: packages/apps/virtual-machine/values.yaml:0-0
Timestamp: 2025-07-26T18:12:05.641Z
Learning: In the cozystack repository, for the virtual-machine app's resources.sockets parameter, the value is intentionally kept as a string in values.yaml despite being documented as {int} type, because the string-to-integer parsing happens elsewhere in the system pipeline rather than at the YAML validation level.
Applied to files:
packages/core/installer/templates/cozystack.yaml
📚 Learning: 2025-07-11T06:11:25.438Z
Learnt from: lllamnyp
Repo: cozystack/cozystack PR: 1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is `kuberneteses.apps.cozystack.io`, not `kubernetes.apps.cozystack.io`. This is defined in the API schema even though it's not grammatically perfect.
Applied to files:
packages/core/platform/templates/cozystack-assets.yamlpackages/core/platform/templates/_helpers.tpl
📚 Learning: 2025-07-03T05:54:51.264Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Applied to files:
packages/core/platform/templates/cozystack-assets.yaml
📚 Learning: 2025-07-26T18:26:01.447Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1216
File: packages/extra/monitoring/README.md:30-31
Timestamp: 2025-07-26T18:26:01.447Z
Learning: The cozyvalues-gen tool has a known issue (https://github.com/cozystack/cozyvalues-gen/issues/10) where it incorrectly places Grafana configuration entries under the wrong documentation sections, specifically placing grafana.resources.requests.* entries in the Metrics storage configuration table instead of the Grafana configuration table.
Applied to files:
packages/extra/monitoring/templates/dashboards.yaml
🪛 checkmake (0.2.2)
packages/core/platform/Makefile
[warning] 24-24: Target "image" should be declared PHONY.
(phonydeclared)
packages/core/flux-aio/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🪛 Shellcheck (0.11.0)
scripts/issue-flux-certificates.sh
[warning] 14-14: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (10)
packages/extra/monitoring/templates/dashboards.yaml (1)
14-14: Dashboard URL correctly switched to cozystack-assets serviceThe new hostname
cozystack-assets.cozy-system.svcaligns with the new assets component; the path is unchanged, so this looks compatible as long as cozystack-assets serves/dashboards/{{ . }}.jsonas expected. Please confirm the service and route match this URL.packages/core/installer/images/cozystack/Dockerfile (1)
33-33: Adding openssl to installer image looks appropriateIncluding
opensslalongside kubectl/helm/jq is reasonable for certificate handling and should not affect existing flows beyond image size.Please double-check that any new certificate-related scripts are executed in this image so the dependency is actually available where needed.
packages/core/platform/values.yaml (1)
1-2: assets.image value is consistent with cozystack-assets image wiringDefining
assets.imagehere matches how the platform Makefile later rewrites this field with the built image digest, so the chart has a single source of truth for the cozystack-assets image reference.Makefile (1)
29-29: Top-level build now includes platform (cozystack-assets) imageWiring
make -C packages/core/platform imageintobuildensures the cozystack-assets image is produced as part of the standard pipeline, which keeps platform values in sync. Just be aware this adds another Docker build step tomake build.If CI build times are tight, you may want to confirm that always building this image is acceptable or consider a separate target.
packages/core/installer/templates/cozystack.yaml (1)
57-58: INSTALL_FLUX flag is now always enabled for the installer podPlumbing
INSTALL_FLUX="true"into the cozystack Deployment is straightforward; just confirm this unconditional default matches the desired behavior (e.g., no remaining use cases where Flux installation should be skipped or made configurable via values).packages/core/platform/templates/_helpers.tpl (1)
20-48: kubernetesAPIEndpoint helper is robust with sensible fallbacksThe helper correctly:
- Uses
lookup/digto avoid panics when the Deployment or container/env fields are missing.- Falls back to
kubernetes.default.svc:443when it can't discover values, which keeps first-install rendering safe.This should work well for forming API proxy URLs from templates. Just ensure the expectation about when the cozystack Deployment exists (and whether you want
localhost:7445vskubernetes.default.svc:443) matches how and when these templates are rendered in your install flow.scripts/installer.sh (2)
22-28: LGTM! Simplified Flux installation.The refactored
install_fluxfunction is much cleaner, delegating to a single make target instead of complex pre-check logic. The early return whenINSTALL_FLUX != "true"is appropriate.
67-71: Continuous reconciliation loop is appropriate.The infinite loop with sleep and periodic reconciliation is a valid pattern for ensuring the platform stays in sync. The trap handler ensures clean exit on SIGINT/SIGTERM.
packages/core/platform/templates/cozystack-assets.yaml (1)
27-30: HostPort binding can cause scheduling issues.Binding to
hostPort: 8123restricts pod placement to one instance per node. Combined withreplicas: 1, if the node becomes unavailable, the pod cannot reschedule to another node while the hostPort remains bound elsewhere.Verify whether
hostPortis necessary for this workload's requirements, or if the defined Service alone would provide sufficient connectivity without the scheduling constraints.packages/core/platform/templates/helmrepos.yaml (1)
11-13: The review comment references a file that could not be located in the repository.Unable to verify the hardcoded pod name concern (
cozystack-assets-0) and the suggestion to check StatefulSet configuration, replica scaling, and service alternatives. The filepackages/core/platform/templates/helmrepos.yamldoes not appear to exist in the current codebase.Before proceeding with this review, confirm the correct file path and verify the code still exists in the current state of the repository.
a42fef5 to
cf18f48
Compare
1c7a74e to
a67b257
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/core/flux-aio/flux-aio.cue (1)
6-9: Pin the module version instead of using "latest".As noted in the previous review, using
version: "latest"prevents reproducible builds and can introduce unexpected breaking changes. For production infrastructure components like FluxCD, version stability is critical.packages/core/flux-aio/Makefile (1)
6-22: Add .PHONY declarations for all targets.As noted in the previous review, targets
show,apply, anddiffshould be marked as phony. Additionally, the newupdatetarget should also be included in the .PHONY declaration since it doesn't produce a file named "update".Apply this diff:
+.PHONY: show apply diff update + show: cozypkg show -n $(NAMESPACE) $(NAME) --plain
🧹 Nitpick comments (2)
packages/core/flux-aio/Makefile (2)
18-22: Consider usingyqfor more robust YAML manipulation.The sed patterns on lines 18-22 are fragile and may break if the generated YAML structure changes:
- Line 21:
/value: .svc/acould match unintended lines containing "value: .svc"- Line 22:
/hostNetwork: true/iassumes exact formattingSince you're already using
yqon line 17, consider extending its use for these transformations instead of sed for better reliability and maintainability.
10-10: Note the use of--force-conflicts.The
--force-conflictsflag will forcibly override any conflicting field managers. While appropriate for automated infrastructure deployment scenarios, be aware that this can overwrite manual changes made by operators.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/core/flux-aio/Chart.yaml(1 hunks)packages/core/flux-aio/Makefile(1 hunks)packages/core/flux-aio/flux-aio.cue(1 hunks)packages/core/flux-aio/templates/_helpers.tpl(1 hunks)packages/core/installer/images/cozystack/Dockerfile(1 hunks)packages/core/installer/templates/cozystack.yaml(1 hunks)packages/core/platform/bundles/distro-full.yaml(0 hunks)packages/core/platform/bundles/distro-hosted.yaml(0 hunks)packages/core/platform/bundles/paas-full.yaml(0 hunks)packages/core/platform/bundles/paas-hosted.yaml(0 hunks)packages/core/platform/templates/_helpers.tpl(1 hunks)packages/core/platform/templates/cozystack-assets.yaml(1 hunks)packages/core/platform/templates/helmrepos.yaml(3 hunks)scripts/installer.sh(2 hunks)scripts/issue-flux-certificates.sh(1 hunks)scripts/migrations/21(1 hunks)
💤 Files with no reviewable changes (4)
- packages/core/platform/bundles/distro-full.yaml
- packages/core/platform/bundles/paas-full.yaml
- packages/core/platform/bundles/distro-hosted.yaml
- packages/core/platform/bundles/paas-hosted.yaml
✅ Files skipped from review due to trivial changes (1)
- packages/core/flux-aio/Chart.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/migrations/21
- scripts/issue-flux-certificates.sh
- packages/core/installer/templates/cozystack.yaml
- packages/core/platform/templates/cozystack-assets.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-03T05:54:51.264Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Applied to files:
packages/core/flux-aio/templates/_helpers.tpl
📚 Learning: 2025-07-11T06:11:25.438Z
Learnt from: lllamnyp
Repo: cozystack/cozystack PR: 1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is `kuberneteses.apps.cozystack.io`, not `kubernetes.apps.cozystack.io`. This is defined in the API schema even though it's not grammatically perfect.
Applied to files:
packages/core/platform/templates/_helpers.tpl
🪛 checkmake (0.2.2)
packages/core/flux-aio/Makefile
[warning] 15-15: Target body for "update" exceeds allowed length of 5 (7).
(maxbodylength)
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (5)
packages/core/installer/images/cozystack/Dockerfile (1)
33-33: LGTM!The addition of
opensslto the runtime dependencies aligns with the new TLS certificate management script (issue-flux-certificates.sh) mentioned in the PR objectives.scripts/installer.sh (2)
22-28: LGTM!The
install_fluxfunction is clean and well-structured. The guard clause forINSTALL_FLUXand the singlemaketarget approach simplify the installation flow.
45-46: Verify the certificate script handles edge cases.Ensure the
issue-flux-certificates.shscript exists, is executable, and properly handles scenarios such as:
- Pre-existing certificates (idempotency)
- Certificate renewal
- Missing dependencies (openssl, kubectl)
- Namespace availability
packages/core/platform/templates/_helpers.tpl (1)
20-48: LGTM with a note about initial installation.The helper logic is sound and provides sensible defaults (
kubernetes.default.svc:443). During initial installation, thecozystackDeployment may not exist yet, so the lookup will return empty and fall back to defaults, which should work correctly for accessing the Kubernetes API server.packages/core/platform/templates/helmrepos.yaml (1)
11-13: [rewritten comment]
[classification tag]
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
a67b257 to
373a0d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/core/flux-aio/Makefile (1)
15-22: Consider adding .PHONY declarations.The targets
show,apply,diff, andupdatedon't produce files and should be marked as phony to ensure they always execute even if files with those names exist.Add this near the top of the file:
include ../../../scripts/common-envs.mk +.PHONY: show apply diff update + show: cozypkg show -n $(NAMESPACE) $(NAME) --plainpackages/core/flux-aio/flux-aio.cue (1)
6-9: Pin the module version to v2.7.3-1 instead of using "latest".Using
version: "latest"introduces non-deterministic builds and breaks reproducibility. For production infrastructure components like FluxCD, pin to a specific version (currently v2.7.3-1, the latest stable release as of October 2025).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/core/flux-aio/Chart.yaml(1 hunks)packages/core/flux-aio/Makefile(1 hunks)packages/core/flux-aio/flux-aio.cue(1 hunks)packages/core/flux-aio/templates/_helpers.tpl(1 hunks)packages/core/installer/images/cozystack/Dockerfile(1 hunks)packages/core/installer/templates/cozystack.yaml(1 hunks)packages/core/platform/bundles/distro-full.yaml(0 hunks)packages/core/platform/bundles/distro-hosted.yaml(0 hunks)packages/core/platform/bundles/paas-full.yaml(0 hunks)packages/core/platform/bundles/paas-hosted.yaml(0 hunks)packages/core/platform/templates/_helpers.tpl(1 hunks)packages/core/platform/templates/cozystack-assets.yaml(0 hunks)packages/core/platform/templates/helmrepos.yaml(3 hunks)scripts/installer.sh(2 hunks)scripts/issue-flux-certificates.sh(1 hunks)scripts/migrations/21(1 hunks)
💤 Files with no reviewable changes (5)
- packages/core/platform/bundles/paas-hosted.yaml
- packages/core/platform/templates/cozystack-assets.yaml
- packages/core/platform/bundles/distro-hosted.yaml
- packages/core/platform/bundles/paas-full.yaml
- packages/core/platform/bundles/distro-full.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/core/flux-aio/Chart.yaml
- packages/core/installer/templates/cozystack.yaml
- scripts/migrations/21
- packages/core/installer/images/cozystack/Dockerfile
- packages/core/platform/templates/_helpers.tpl
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-10T11:40:05.186Z
Learnt from: kvaps
Repo: cozystack/cozystack PR: 1698
File: packages/core/flux-aio/templates/_helpers.tpl:3-3
Timestamp: 2025-12-10T11:40:05.186Z
Learning: Do not modify helpers sourced from flux-operator (e.g., kubernetes_envs) in flux-aio templates. Leave them as-is since they are battle-tested upstream. Any changes should be confined to wrapper code that consumes these helpers, not the templates themselves. If modifications are absolutely necessary for compatibility, document rationale and align with upstream behavior.
Applied to files:
packages/core/flux-aio/templates/_helpers.tpl
📚 Learning: 2025-07-03T05:54:51.264Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Applied to files:
packages/core/flux-aio/templates/_helpers.tpl
🪛 checkmake (0.2.2)
packages/core/flux-aio/Makefile
[warning] 15-15: Target body for "update" exceeds allowed length of 5 (7).
(maxbodylength)
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (7)
packages/core/flux-aio/templates/_helpers.tpl (1)
1-13: LGTM - Helper template correctly extracts Kubernetes API environment variables.The template logic properly uses
lookup,dig, and conditional filtering to extractKUBERNETES_SERVICE_HOSTandKUBERNETES_SERVICE_PORTfrom the cozystack deployment. Based on learnings, this helper is sourced from flux-operator and is battle-tested upstream.scripts/issue-flux-certificates.sh (2)
1-63: LGTM - Certificate provisioning script is well-structured and idempotent.The script correctly:
- Performs early exit when secrets already exist in both namespaces
- Uses proper cleanup handler with trap
- Generates CSR, submits to Kubernetes, and waits for approval
- Creates secrets idempotently using
--dry-run=clientwithkubectl applyThe use of dry-run combined with apply ensures the script can be safely re-run.
14-14: Use single quotes in trap for deferred variable expansion.The trap command currently uses double quotes, causing
$TMPDIRto expand when the trap is set rather than when it executes. This is flagged by Shellcheck SC2064.Apply this diff:
-trap 'rm -rf "$TMPDIR"' EXIT +trap 'rm -rf "$TMPDIR"' EXITWait, I see the code already has single quotes. Let me check the annotated code again... Actually, looking at line 14, it shows:
trap 'rm -rf "$TMPDIR"' EXITThis already uses single quotes! The past review comment mentioned double quotes, but the current code has single quotes. This means the issue was already fixed.
packages/core/flux-aio/Makefile (1)
6-13: LGTM - Target implementations are correct.The
show,apply, anddifftargets correctly usecozypkgto render and apply manifests with appropriate kubectl flags.packages/core/platform/templates/helmrepos.yaml (1)
11-13: LGTM - Security improvement with HTTPS and mutual TLS.The changes consistently update all three HelmRepository resources to:
- Use HTTPS instead of HTTP
- Access via Kubernetes API proxy endpoint
- Include TLS certificate reference (
cozystack-assets-tls)This enhances security for Helm repository access. The certificate secret name matches what's provisioned by
scripts/issue-flux-certificates.sh.Also applies to: 25-27, 38-40
scripts/installer.sh (2)
22-28: LGTM - Simplified Flux installation flow.The new
install_fluxfunction is cleaner than the legacy approach, with a clear early-return pattern and direct integration with the flux-aio Makefile. The wait for CRDs ensures FluxCD is ready before proceeding.
43-46: LGTM - Proper installation sequence with certificate provisioning.The flow correctly installs Flux first, then provisions TLS certificates for secure Helm repository access. This ordering ensures the FluxCD components are available before certificate issuance.
| - name: KUBERNETES_SERVICE_HOST | ||
| value: localhost | ||
| - name: INSTALL_FLUX | ||
| value: "true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
#381) This PR adds a new blog article about solving the chicken-and-egg problem of deploying CNI and kube-proxy through Flux while ensuring Flux itself works without CNI and kube-proxy, using Kubernetes API routing and mTLS certificates. The article explains how we: - Use flux-aio to run Flux without CNI and kube-proxy - Route Flux through the Kubernetes API to access internal HTTP server - Obtain client certificates for mTLS authentication - Configure RBAC for pod proxy access Related PR: cozystack/cozystack#1698 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added blog post on deploying CNI and kube-proxy through Flux-aio using Kubernetes API routing with mTLS client certificate authentication, including step-by-step guidance and configuration examples. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This change is extracted from - #1641 and reworked to work standalone requires: - #1705 Adds a new `flux-aio` module and migration script to upgrade FluxCD to version 22. This introduces a new modular approach to FluxCD installation using the flux-aio OCI module. Changes: - Created new `flux-aio` package with Chart.yaml, Makefile, and CUE configuration - Added flux-aio module configuration using OCI module from `ghcr.io/stefanprodan/modules/flux-aio` - Generated large fluxcd.yaml template (11956+ lines) for FluxCD resources - Added migration script (migrations/21) to handle upgrade from version 21 to 22 - Updated installer to include flux-aio module - Added script `issue-flux-certificates.sh` for managing TLS certificates for cozystack-assets - Updated platform templates to support flux-aio module - Updated cozystack-assets service references ```release-note [fluxcd] Add flux-aio module and migration ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **New Features** * Added TLS certificate support for Helm package repositories with automatic certificate provisioning. * **Chores** * Refactored FluxCD integration using Helm chart-based deployment. * Updated system to version 22 with automatic migration support. * Enhanced security dependencies (OpenSSL). <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This change is extracted from - #1641 and reworked to work standalone requires: - #1705 ## What this PR does Adds a new `flux-aio` module and migration script to upgrade FluxCD to version 22. This introduces a new modular approach to FluxCD installation using the flux-aio OCI module. Changes: - Created new `flux-aio` package with Chart.yaml, Makefile, and CUE configuration - Added flux-aio module configuration using OCI module from `ghcr.io/stefanprodan/modules/flux-aio` - Generated large fluxcd.yaml template (11956+ lines) for FluxCD resources - Added migration script (migrations/21) to handle upgrade from version 21 to 22 - Updated installer to include flux-aio module - Added script `issue-flux-certificates.sh` for managing TLS certificates for cozystack-assets - Updated platform templates to support flux-aio module - Updated cozystack-assets service references ### Release note ```release-note [fluxcd] Add flux-aio module and migration ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added TLS certificate support for Helm package repositories with automatic certificate provisioning. * **Chores** * Refactored FluxCD integration using Helm chart-based deployment. * Updated system to version 22 with automatic migration support. * Enhanced security dependencies (OpenSSL). <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This change is extracted from
and reworked to work standalone
requires:
What this PR does
Adds a new
flux-aiomodule and migration script to upgrade FluxCD to version 22. This introduces a new modular approach to FluxCD installation using the flux-aio OCI module.Changes:
flux-aiopackage with Chart.yaml, Makefile, and CUE configurationghcr.io/stefanprodan/modules/flux-aioissue-flux-certificates.shfor managing TLS certificates for cozystack-assetsRelease note
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.