docs(api): correct the stale helper_image_flavor flavour list - #71
Conversation
The field advertised "(alpine, ubuntu)". Docker Hub publishes seven Linux flavours for the helper image at v19.2.2: alpine as the untagged default, alpine-edge, alpine-latest, alpine3.21, concrete, ubi-fips and ubuntu. The release watcher already compares its Docker Hub query against this string and has been reporting the four omissions on every run. No enum, deliberately. Upstream does not validate the value, it interpolates it straight into the helper image tag (helpers/container/helperimage/linux_info.go at v19.2.2), so an unrecognised flavour surfaces as an ImagePullBackOff on the build pod. That is a visible failure, unlike the security profiles in #66 where the runner drops an unusable value and the pod runs unconfined, which is why those got an enum and this does not. An enum here would also have to be widened every time upstream publishes a flavour, which is the same drift that made the list stale in the first place. The go-flags description tag never reaches the CRD, so kubectl explain has always shown nothing for this field. The doc comment fixes that, and says the part a user acts on: where a wrong value shows up, and that empty means alpine. Refs: #58 Signed-off-by: Alexander Chernov <alexander@chernov.it>
|
Warning Review limit reached
Next review available in: 20 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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 |
The field carried json:"volumes,omitempty" twice, since 8df984f. reflect's StructTag.Get returns the first match, so nothing behaved differently and the generated CRD and deepcopy are byte-identical after the change. staticcheck has always flagged it (SA5008), but the lint job runs with only-new-issues, so it stayed invisible until the gofmt realignment in the previous commit made the line count as new. Refs: #58 Signed-off-by: Alexander Chernov <alexander@chernov.it>
|
Pushed 7ed2aa8. lint failed on a pre-existing duplicate |
) * docs(api): document that terminationGracePeriodSeconds does nothing Upstream removed the key at the v17.0.0 major boundary. The runner decodes with toml.DecodeFile and never inspects MetaData.Undecoded(), so a value set here is accepted by admission, rendered into config.toml, and then dropped with no error, no warning and no log line. The mitigation the issue credits does not exist on any user-facing surface. controller-gen only turns Go doc comments into the CRD description; the go-flags `description:` tag is invisible, so the deprecation text this field has carried since v16.11.0 never reached kubectl explain. Same finding as #71. All three fields here gain their first real description. Both replacements carried the same fossil, "Ignored if KUBERNETES_TERMINATIONGRACEPERIODSECONDS is specified", pointing at a key that no longer exists. Dropped from the tags rather than contradicted in a comment next to them. The v17.0.0 removal also moved a default. At v16.11.0 GetPodTerminationGracePeriodSeconds returned 0 when both keys were unset, which upstream commented as deliberate ("terminate immediately"). That getter went with the field, so the executor now passes the raw pointer to the pod spec and unset means Kubernetes' 30s. A runner upgraded across the boundary waits 30s where it used to exit at once, which is the fact a reader of pod_termination_grace_period_seconds most needs. Not removing the field. kubectl has defaulted to fieldValidation=Strict since 1.25, so dropping it from the CRD fails `kubectl apply` for any manifest that still sets it. That belongs in a release documenting the break, so the key is excluded from the release watcher's stale report instead, which #70 made bidirectional. The exclusion is counted in every release issue body and the watcher flags it as unused once the field goes, so it cannot rot silently. Refs: #62 Refs: #70 Signed-off-by: Alexander Chernov <alexander@chernov.it> * docs(api): add the missing space before DEPRECATED in the tag The description read "kill signal.DEPRECATED:", running two sentences together. Tidied while the neighbouring tags are being edited in this branch anyway. Note this is cosmetic only. CodeRabbit flagged it as breaking generated help text, which does not apply here: nothing in the operator parses go-flags tags and controller-gen ignores them, so the string never reaches a user. Regenerating the CRDs after this change produces no diff, which is the same point the rest of the branch makes. Refs: #62 Signed-off-by: Alexander Chernov <alexander@chernov.it> --------- Signed-off-by: Alexander Chernov <alexander@chernov.it>
1 participant
The field advertised
(alpine, ubuntu). Docker Hub publishes seven Linux flavours of the helper image at v19.2.2: alpine as the untagged default, plusalpine-edge,alpine-latest,alpine3.21,concrete,ubi-fipsandubuntu. The release watcher already compares its Docker Hub query against this exact string, so it has been reporting the four omissions on every run; after this it reports none.No enum, deliberately. Upstream does not validate the value, it interpolates it straight into the helper image tag (
helpers/container/helperimage/linux_info.goat v19.2.2), so an unrecognised flavour surfaces as an ImagePullBackOff on the build pod. That is a visible failure, unlike the security profiles in #66 where the runner silently drops an unusable value and the container runs unconfined, which is why those got an enum and this does not. An enum here would also need widening every time upstream publishes a flavour, which is the drift that made the list stale to begin with.The go-flags
description:tag never reaches the CRD, sokubectl explainhas always shown nothing for this field. The doc comment fixes that and says the part a user acts on: where a wrong value shows up, and that empty means alpine.On the diff size. 176 lines of
api/v1beta2/gitlab_types.gochange, but only 6 are real. gofmt aligns contiguous runs of struct fields, and a comment splits the run, so the 88 fields above it re-align by a few columns.git diff -wreduces the whole file to the five added lines and the one field they sit on. The struct already carries two comment blocks from #66, so this is the file's existing style rather than a new cost.Last of the three parts of #58. The 36 unexposed keys were closed by #66 (23 exposed, 13 autoscaler pairs out by decision, now recorded in
hack/runner-release-watch.suppresson #70), andterminationGracePeriodSecondswas spun out to #62.Closes #58