fix: make update-platforms.py safe and self-contained; add catalog checks + CI#146
Open
lfrancke wants to merge 9 commits into
Open
fix: make update-platforms.py safe and self-contained; add catalog checks + CI#146lfrancke wants to merge 9 commits into
lfrancke wants to merge 9 commits into
Conversation
The script had two problems that made it unsafe to run, so the catalog was maintained by hand instead (see the reverted 878c5c1 and the manual 7ce43a2 "do not delete ionos versions"). - IONOS versions were corrupted on every run. `ionosctl k8s version list --output json` does not return a JSON array; it returns a JSON-encoded string of a Go slice, e.g. "[1.34.2 1.33.3 1.32.7]". The script did json.loads() and then iterated the resulting string character by character, turning the digits into fake versions (7, 6, 5, ...) and wiping the real IONOS entry. Parse the slice literal into a real list and validate the result looks like versions. - The whole file was reformatted on every run because output depended on which `yq` happened to be installed (mikefarah vs the python/jq wrapper) and PyYAML's raw layout differs from the committed style. The script now owns the layout itself via _CatalogDumper (indented block sequences, double-quoted numeric versions, no anchors) and no longer shells out to yq. Output is deterministic and re-running on an unchanged catalog produces no diff. Also stop auto-creating catalog entries for unknown Replicated distributions with a hardcoded Replicated instance type (invalid for cloud distributions); warn and let a human add them with the right spec. README documents how to run the script and that platforms.yaml must not be hand-edited.
Regenerated with the fixed script. Changes: - Kubernetes version bumps across kind, k3s, rke2, EKS, AKS, GKE, OKE. - IONOS corrected to real versions (1.34.2, 1.33.3, 1.32.7). - OpenShift version list written literally on both x86 and ARM entries (the &id001/*id001 anchor is gone; the script owns the format).
read_platforms indexed ["platforms"] before the guard that was meant to catch a missing key, so a malformed platforms.yaml raised KeyError instead of the intended logged failure. It also loaded the file twice. Mirror read_providers: load once, guard, then assign.
platforms.yaml and operator-tests.yaml are maintained separately and can drift apart; today that only surfaces mid-test as "platform does not exist". This CI-friendly check verifies every platform referenced by a test exists, every platform has a known provider, and warns about platforms no test uses. Documented in the README.
The "keep at most 5 versions" limit was duplicated (and inconsistently expressed) in two places. Replace with MAX_VERSIONS_PER_PLATFORM. No behaviour change.
The repo had no push/PR-triggered CI (all workflows are workflow_dispatch). Add a lightweight workflow that runs ruff over scripts/ and apps/ and executes check-catalog.py, so catalog drift and lint regressions are caught on every pull request.
Match PR #144: top-level permissions: {} with an explicit per-job contents: read, and persist-credentials: false on checkout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Running
scripts/update-platforms.pyproduced a ~450-line diff and corrupted the IONOS versions. Root causes fixed here, plus the catalog is now safe to regenerate from the script.Fixes
ionosctl k8s version list --output jsonreturns a JSON-encoded string of a Go slice ("[1.34.2 1.33.3 …]"), not an array. The scriptjson.loads()'d it and iterated it character-by-character, turning digits into fake versions and wiping the IONOS entry (this is why878c5c1setversions: []and had to be reverted). Now parsed into a real list and validated.yqhappened to be installed (mikefarah vs the python/jq wrapper). A_CatalogDumpernow emits the canonical style directly (indented sequences, double-quoted numeric versions, no anchors) with no external formatter. Output is deterministic — re-running on an unchanged catalog produces no diff (verified idempotent).r1.xlarge(invalid for cloud distros); the script warns and defers to a human.catalog.pyrobustness.read_platforms()no longer indexes["platforms"]before its own missing-key guard (it raisedKeyError); it now mirrorsread_providers().Additions
scripts/check-catalog.py— validates thatplatforms.yamlandoperator-tests.yamlagree (referenced platforms exist, providers are known; warns on unused platforms). CI-ready..github/workflows/checks.yml— the repo's first PR-triggered CI: ruff overscripts/+apps/, and the catalog check.add-arm-platforms.py.platforms.yamlmust not be hand-edited.Verification
1.34.2 / 1.33.3 / 1.32.7).ruffclean;check-catalog.pypasses.Follow-up PRs (security blockers in the test runner, correctness fixes, Harbor GC safety, hardening, hygiene) are tracked separately.