mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
Aleksei Sviridkin
d6b38f0a79
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does In `spec.oidc.mode: System`, the post-install/post-upgrade bootstrap Job patches the chart-owned `<release>-oidc-kubeconfig` Secret with a real kubeconfig built from the tenant admin kubeconfig's CA data. When that CA data was empty, the script logged a WARNING and continued, then exited 0. The hook was satisfied, the HelmRelease reported `Ready`, and the Secret was left holding the install-time placeholder kubeconfig, which cannot authenticate. This PR makes the Job fail (`exit 1`) instead, so the failure surfaces as a red install and can be retried (the Job already has `backoffLimit: 6` and `restartPolicy: OnFailure`) rather than reported as a falsely healthy release. This is the smallest of the three directions raised on the linked issue, and matches how the neighbouring failure mode in the same script already behaves (a failed `kubectl apply` of the ClusterRoleBindings already fails the Job under `set -euo pipefail`). Behaviour for `mode: None` and `mode: CustomConfig` is unchanged, and the common case — CA data present — runs the identical patch commands as before. A regression test was added to `packages/apps/kubernetes/tests/oidc_test.yaml` asserting the rendered Job script contains the `exit 1` on empty `CA_DATA` and no longer contains the old warn-and-continue text. Confirmed it fails against the pre-fix template and passes against the fix. Validation: - `helm unittest .` in `packages/apps/kubernetes` — 150/150 tests pass (up from 149 before the new test). - `helm template ... --show-only templates/oidc-rbac-job.yaml` with representative `System`-mode values renders valid YAML; the extracted bootstrap script passes `bash -n` and `shellcheck -s sh` introduces no new warnings versus the pre-change script. - `make generate` was not run/needed — this change touches neither `values.yaml`, `values.schema.json`, `Chart.yaml`, nor `README.md`. - Not run: a live cluster reproduction (Kamaji-issued admin kubeconfig actually missing CA data at Job run time). The defect is provable from the script logic itself and the added unit test demonstrates the fail-then-pass behaviour; there is no live cluster available in this environment. ### Screenshots Not applicable — no UI change. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: This change is confined to the shell logic inside `packages/apps/kubernetes/templates/oidc-rbac-job.yaml`'s post-install hook Job (a `Values.oidc.mode: System`-only failure path) and its accompanying helm-unittest test. It does not add, rename or remove a package, a `values.yaml` key, a CRD, a namespace, a release-owned object name, or any other surface the trigger map lists, so no downstream repository is affected. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(kubernetes): a System-mode OIDC install no longer reports Ready while the tenant OIDC kubeconfig Secret still holds the install-time placeholder — the bootstrap Job now fails (and retries) instead of only logging a warning when the tenant cluster's CA data isn't available yet. ``` Fixes #3765 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * OIDC setup now fails with an error if the tenant admin kubeconfig is missing certificate authority data, rather than leaving the OIDC kubeconfig Secret at its install-time placeholder. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
How to test packages local
cd packages/core/installer
make image-cozystack REGISTRY=YOUR_CUSTOM_REGISTRY
make apply
kubectl delete po -l app=source-controller -n cozy-fluxcd