mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 05:25:14 +09:00
Aleksei Sviridkin
ff0752d8e9
## What this PR does Fixes #3516. - `packages/apps/kubernetes/templates/oidc-rbac-job.yaml`: the `mode=None` cleanup path of the `oidc-bootstrap` Job guarded its Keycloak deletes with `if kubectl api-resources --api-group=v1.edp.epam.com >/dev/null 2>&1`. That command **exits 0 for an absent API group** (it simply prints an empty list), so the guard always fell through and the delete failed with `error: the server doesn't have a resource type "keycloakclient"` — `--ignore-not-found` covers a missing *object*, not an unknown *type*. Since the Job is a `post-install,post-upgrade` Helm hook with `backoffLimit: 6`, this blocked the Helm upgrade of every `kubernetes-<cluster>` release on any cluster without the EDP Keycloak operator CRDs, i.e. at the chart default `oidc.mode: None`. The probe now captures the discovery output once and gates each delete on its own resource type being present, matching the idiom already used in `packages/core/platform/images/migrations/migrations/{34,38,39}` and `packages/system/ouroboros/.../cleanup-hook.yaml`. `api-resources` is kept (rather than `get crd`) on purpose: it hits discovery, which `system:discovery` grants to every authenticated principal, so the namespaced Job Role needs no cluster-scoped CRD read. - `packages/apps/kubernetes/tests/oidc_test.yaml`: new unit test asserting the rendered script probes by output and that the exit-code form does not come back. All four of its assertions fail against the pre-fix template. `cmd/check-readiness` and the platform migrations were checked too: they already test the output, not the exit code. ## Validation - `helm unittest packages/apps/kubernetes` on the branch rebased onto current `main`: 317/317 pass. Putting `main`'s template back fails only the new case, all 4 assertions, so it is a real regression guard. - `make generate` in `packages/apps/kubernetes`: no diff. - Extracted the rendered `mode=None` script and ran `sh -n` / `shellcheck -s sh` — clean (only pre-existing `SC3040` for `pipefail` and one pre-existing `SC2086`). - Executed the extracted script against a stub `kubectl` reproducing the two real behaviors (`api-resources` exits 0 with empty output for an absent group; `delete <unknown type>` exits 1 despite `--ignore-not-found`): the pre-fix script exits 1 with `the server doesn't have a resource type "keycloakclient"`, the fixed script exits 0 with the CRDs absent and still issues both deletes with the CRDs present. ### 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: The diff touches no `values.schema.json` and no behaviour the docs describe: the template change restores the documented `oidc.mode: None` path rather than altering it. ## Release note ```release-note fix(kubernetes): the oidc-bootstrap hook no longer fails on clusters without the Keycloak operator CRDs, which blocked Helm upgrades of kubernetes-* releases at oidc.mode=None ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * OIDC cleanup now skips deleting Keycloak resources when their resource types are unavailable, preventing cleanup failures. * **Tests** * Added coverage for resource discovery and conditional cleanup behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->