Conversation
|
Note to self: update this when we merge v1alpha2 because it adds conversion-gen which also needs a version pin synced with k8s.io. |
|
|
|
I also want to rethink the github workflows. Converting back to draft. |
|
|
| - name: Checkout PR | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
| ref: ${{ github.event.pull_request.head.ref }} | ||
| # Use the default GITHUB_TOKEN — dependabot PRs from the same repo | ||
| # have write access when triggered via pull_request_target. | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| path: pr | ||
|
|
||
| - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 |
Check failure
Code scanning / CodeQL
Checkout of untrusted code in trusted context High
There was a problem hiding this comment.
Pull request overview
Adds a set of maintenance scripts and CI workflows to keep “multi-location” version references (Go/tooling/CAPI/K8s/envtest, etc.) consistent across the repo, and to auto-complete partial Dependabot bumps.
Changes:
- Introduces
hack/helpers.shplus bump/verify scripts (bump-*,auto-bump.sh,verify-versions.sh,envtest-ver.sh) and ShellSpec tests for them. - Updates CI to run shell script lint/tests and adds a “Version Consistency” workflow (with Dependabot auto-bump support).
- Updates version references/derivation in
Makefile,go.mod, and docs; extends Sonar configuration for shell coverage.
Reviewed changes
Copilot reviewed 45 out of 46 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
sonar-project.properties |
Adds Sonar generic coverage report path for shell coverage. |
hack/verify-versions.sh |
New consistency checker for key version references across the repo. |
hack/spec/verify_versions_spec.sh |
ShellSpec coverage for verify-versions.sh. |
hack/spec/spec_helper.sh |
Shared ShellSpec fixture/mocking utilities. |
hack/spec/release_spec.sh |
ShellSpec coverage for release.sh. |
hack/spec/helpers_spec.sh |
ShellSpec coverage for pure helper functions. |
hack/spec/helpers_metadata_spec.sh |
ShellSpec coverage for metadata helpers. |
hack/spec/helpers_gomod_spec.sh |
ShellSpec coverage for go.mod helpers. |
hack/spec/helpers_file_versions_spec.sh |
ShellSpec coverage for file-based version helpers. |
hack/spec/fixtures/test/e2e/data/shared/v1beta1/metadata.yaml |
Test fixture for CAPI metadata. |
hack/spec/fixtures/test/e2e/config/proxmox-dev.yaml |
Test fixture for e2e config (dev). |
hack/spec/fixtures/test/e2e/config/proxmox-ci.yaml |
Test fixture for e2e config (ci). |
hack/spec/fixtures/sonar-project.properties |
Fixture for sonar version checks. |
hack/spec/fixtures/metadata.yaml |
Fixture for CAPMOX metadata release series. |
hack/spec/fixtures/go.mod |
Fixture go.mod for helper/bump tests. |
hack/spec/fixtures/docs/Usage.md |
Fixture for docs --kubernetes-version updates. |
hack/spec/fixtures/docs/Development.md |
Fixture for docs Go version updates. |
hack/spec/fixtures/clusterctl-settings.json |
Fixture for release tooling. |
hack/spec/fixtures/Makefile |
Fixture for envtest derivation via envtest-ver.sh. |
hack/spec/fixtures/Dockerfile |
Fixture for Dockerfile Go version updates. |
hack/spec/fixtures/.golangci-kal.yml |
Fixture for golangci-kal Go version updates. |
hack/spec/fixtures/.custom-gcl.yaml |
Fixture for custom golangci-lint version file. |
hack/spec/cobertura-to-sonar.xslt |
Adds converter intended to map Cobertura XML to Sonar generic coverage XML. |
hack/spec/bump_k8s_spec.sh |
ShellSpec coverage for bump-k8s.sh. |
hack/spec/bump_golangci_lint_spec.sh |
ShellSpec coverage for bump-golangci-lint.sh. |
hack/spec/bump_go_spec.sh |
ShellSpec coverage for bump-go.sh. |
hack/spec/bump_capi_spec.sh |
ShellSpec coverage for bump-capi.sh. |
hack/spec/.shellspec |
ShellSpec configuration. |
hack/release.sh |
New release helper to bump CAPMOX version references consistently. |
hack/helpers.sh |
New shared helper library for version detection/updates across files. |
hack/envtest-ver.sh |
New helper to derive ENVTEST_K8S_VERSION from go.mod. |
hack/bump-k8s.sh |
New k8s.io version bump helper (go.mod + e2e config + docs). |
hack/bump-golangci-lint.sh |
New golangci-lint version bump helper. |
hack/bump-go.sh |
New Go version bump helper across go.mod/Dockerfile/docs/golangci-kal config. |
hack/bump-capi.sh |
New cluster-api version bump helper (go.mod + e2e config + e2e metadata). |
hack/auto-bump.sh |
New automation to detect mismatches and run the right bump scripts (Dependabot-focused). |
go.mod |
Adjusts k8s code-generator pinning to use module-level replace. |
docs/Development.md |
Updates documented Go prerequisite version. |
Makefile |
Derives ENVTEST_K8S_VERSION dynamically and adds lint-sh/test-sh targets. |
.shellcheckrc |
Configures ShellCheck to follow sourced files. |
.gitignore |
Ignores ShellSpec coverage output directory. |
.github/workflows/versions.yml |
Adds version consistency workflow + Dependabot auto-bump job. |
.github/workflows/test.yml |
Runs ShellSpec tests and installs kcov; tweaks Sonar args. |
.github/workflows/lint.yml |
Adds a ShellCheck job. |
.github/dependabot.yml |
Removes Docker ecosystem updates. |
.github/copilot-instructions.md |
Documents the new version scripts and workflows. |
| .PHONY: test-sh | ||
| test-sh: ## Run ShellSpec tests for hack/ scripts with kcov coverage. | ||
| cd hack/spec && shellspec --kcov --kcov-options='--include-path=$(CURDIR)/hack/' $(SHELLSPEC_ARGS) | ||
|
|
There was a problem hiding this comment.
ENVTEST_K8S_VERSION is now derived dynamically and you’ve added verify-versions.sh, but there’s still no make verify-versions target (it’s referenced in docs/CI guidance). Consider adding a .PHONY: verify-versions target that runs hack/verify-versions.sh (and optionally wiring it into verify).
| .PHONY: verify-versions | |
| verify-versions: ## Verify repository version definitions are up to date. | |
| hack/verify-versions.sh |
| make verify # Verify modules and generated files are up to date | ||
| make verify-modules # Check if go.mod and go.sum are up to date | ||
| make verify-gen # Check if generated files are up to date | ||
| make verify-versions # Check that version references are consistent across the repo |
There was a problem hiding this comment.
This doc references make verify-versions, but the Makefile currently doesn’t define a verify-versions target. Either add the Makefile target or update the docs to invoke hack/verify-versions.sh directly to avoid broken instructions.
| make verify-versions # Check that version references are consistent across the repo | |
| hack/verify-versions.sh # Check that version references are consistent across the repo |
| # Bump golangci-lint in go.mod and .custom-gcl.yaml | ||
| hack/bump-golangci-lint.sh v2.10.0 | ||
|
|
||
| # Bump k8s.io/{api,apimachinery,client-go} and ENVTEST_K8S_VERSION in Makefile |
There was a problem hiding this comment.
The comment says bump-k8s.sh bumps “ENVTEST_K8S_VERSION in Makefile”, but hack/bump-k8s.sh explicitly does not update envtest and the Makefile derives it via hack/envtest-ver.sh. Update the documentation wording to match the actual behavior (envtest is derived, not bumped).
| # Bump k8s.io/{api,apimachinery,client-go} and ENVTEST_K8S_VERSION in Makefile | |
| # Bump k8s.io/{api,apimachinery,client-go}; envtest is derived via hack/envtest-ver.sh |
| - name: Install ShellSpec | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| repository: shellspec/shellspec | ||
| ref: 90e48c950239f3b8a9fdfa3e869592872c77b981 # v0.28.1 | ||
| path: .shellspec | ||
| - name: Add ShellSpec to PATH | ||
| run: echo "${{ github.workspace }}/.shellspec" >> "$GITHUB_PATH" | ||
|
|
||
| - name: Install kcov | ||
| run: sudo apt-get update && sudo apt-get install -y kcov | ||
|
|
||
| - run: make verify | ||
| - run: make test | ||
| - run: make test-sh | ||
|
|
There was a problem hiding this comment.
make test-sh runs ShellSpec tests which call yq extensively (via hack/helpers.sh), but this workflow doesn’t install mikefarah/yq. On ubuntu-latest this typically means make test-sh will fail (or pick up an incompatible yq). Install the same yq you use in versions.yml before running make test-sh.
There was a problem hiding this comment.
This is handled in another PR. I will rebase when it's merged to resolve this comment.
| args: > | ||
| -Dsonar.pullrequest.key=${{ github.event.pull_request.number }} | ||
| -Dsonar.scm.revision=${{ github.event.pull_request.head.sha }} | ||
| -X |
There was a problem hiding this comment.
The Sonar scan is run with -X (debug) enabled. This can produce very noisy logs and may risk leaking sensitive details depending on scanner output. Consider removing -X or gating it behind an explicit workflow input/env var used only for troubleshooting.
| -X |
| sonar.go.coverage.reportPaths=cover.out | ||
| sonar.coverageReportPaths=hack/spec/coverage/sonarqube.xml |
There was a problem hiding this comment.
sonar.coverageReportPaths points to hack/spec/coverage/sonarqube.xml, but the PR doesn’t add any step/target that generates this file (e.g., converting kcov’s output to Sonar’s generic coverage XML). As-is, Sonar analysis may fail or silently miss shell coverage. Ensure make test-sh (or the workflow) produces hack/spec/coverage/sonarqube.xml, or change this property to the actual generated report path/format.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 46 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
.github/dependabot.yml:22
- This PR removes the Dependabot
dockerecosystem update configuration, but the PR description focuses on adding version-bump helpers/verification and doesn’t mention disabling Dockerfile base-image updates. If the intent is still to keep thegolang:base image current automatically, consider restoring the Docker ecosystem entry (or explicitly documenting why it’s being removed).
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "daily"
| exit 1 | ||
| fi | ||
|
|
||
| NEW=$(ensure_v_prefix "$1") |
There was a problem hiding this comment.
bump-golangci-lint.sh accepts any string as the version and will write it into go.mod/.custom-gcl.yaml. The other bump scripts validate their inputs (validate_semver / validate_go_version), so it’s easy to accidentally produce an invalid replace directive here. Consider validating the input (after ensure_v_prefix) with the existing semver validator before applying changes.
| NEW=$(ensure_v_prefix "$1") | |
| NEW=$(ensure_v_prefix "$1") | |
| validate_semver "${NEW}" |
| # fallback below only runs when kcov is older (e.g. Debian 12, Ubuntu <= 24.04). | ||
| # TODO: drop this rule and hack/spec/cobertura-to-sonar.xslt once CI's kcov is >= 43. | ||
| hack/spec/coverage/sonarqube.xml: hack/spec/coverage/cobertura.xml hack/spec/cobertura-to-sonar.xslt | ||
| @if [ ! -s $@ ]; then xsltproc --nonet --novalid hack/spec/cobertura-to-sonar.xslt $< > $@; fi |
There was a problem hiding this comment.
The hack/spec/coverage/sonarqube.xml rule falls back to running xsltproc when sonarqube.xml is missing/empty, but neither make test-sh nor the CI workflow installs xsltproc. On systems where kcov is < 43 (i.e., where the fallback is actually needed), make test-sh will fail with xsltproc: command not found. Consider either adding an explicit dependency/install step for xsltproc (and documenting it), or making the rule fail fast with a clearer message when xsltproc is unavailable.
| @if [ ! -s $@ ]; then xsltproc --nonet --novalid hack/spec/cobertura-to-sonar.xslt $< > $@; fi | |
| @if [ ! -s $@ ]; then \ | |
| if ! command -v xsltproc >/dev/null 2>&1; then \ | |
| echo "error: xsltproc is required to generate $@ when kcov does not write sonarqube.xml natively (kcov < 43). Please install xsltproc/libxslt." >&2; \ | |
| exit 1; \ | |
| fi; \ | |
| xsltproc --nonet --novalid hack/spec/cobertura-to-sonar.xslt $< > $@; \ | |
| fi |
There was a problem hiding this comment.
I considered it and am rejecting it.
| GOLANGCI_VERSION_GOMOD=$(gomod_get_replace 'github.com/golangci/golangci-lint/v2') | ||
| GOLANGCI_VERSION_CUSTOM=$(customgcl_get_version) | ||
| if versions_differ "${GOLANGCI_VERSION_GOMOD}" "${GOLANGCI_VERSION_CUSTOM}"; then | ||
| fail "golangci-lint version mismatch: go.mod replace has '${GOLANGCI_VERSION_GOMOD}', .custom-gcl.yaml has '${GOLANGCI_VERSION_CUSTOM}'" | ||
| fi |
There was a problem hiding this comment.
verify-versions.sh only compares the golangci-lint replace version against .custom-gcl.yaml, but it never checks the (indirect) require version from go.mod. This means a typical Dependabot bump that updates require github.com/golangci/golangci-lint/v2 while leaving the replace (and .custom-gcl.yaml) stale will not be detected by this verifier. Consider also reading gomod_get_require 'github.com/golangci/golangci-lint/v2' and failing when require, replace, and .custom-gcl.yaml disagree.
| # docs_get_k8s returns the first --kubernetes-version value found in docs | ||
| # (e.g. "v1.31.6"). Returns empty string if not found. | ||
| docs_get_k8s() { | ||
| grep -roh -- '--kubernetes-version v[0-9]\+\.[0-9]\+\.[0-9]\+' "${REPO_ROOT}/docs/" 2>/dev/null \ |
There was a problem hiding this comment.
docs_get_k8s uses a basic-regex grep pattern with \+ quantifiers, which is GNU-specific and can fail on BSD/macOS grep. Switch this to an ERE invocation (e.g., grep -E with +) or another portable extractor so the script matches the same docs lines across developer environments.
| grep -roh -- '--kubernetes-version v[0-9]\+\.[0-9]\+\.[0-9]\+' "${REPO_ROOT}/docs/" 2>/dev/null \ | |
| grep -Eroh -- '--kubernetes-version v[0-9]+\.[0-9]+\.[0-9]+' "${REPO_ROOT}/docs/" 2>/dev/null \ |
| # makefile_get_envtest returns the ENVTEST_K8S_VERSION value from the | ||
| # Makefile (e.g. "1.32"). | ||
| makefile_get_envtest() { | ||
| make -C "${REPO_ROOT}" --no-print-directory print-envtest-ver 2>/dev/null |
There was a problem hiding this comment.
makefile_get_envtest suppresses all make stderr output (2>/dev/null). When make fails (missing target, syntax error, missing make, etc.), callers will exit due to set -e but with no diagnostic, making failures hard to debug. Prefer letting stderr through, or catch the failure and emit a clear error message before returning non-zero.
| make -C "${REPO_ROOT}" --no-print-directory print-envtest-ver 2>/dev/null | |
| if ! make -C "${REPO_ROOT}" --no-print-directory print-envtest-ver; then | |
| echo "ERROR: failed to read ENVTEST_K8S_VERSION from Makefile using 'make print-envtest-ver' in ${REPO_ROOT}" >&2 | |
| return 1 | |
| fi |
Replace CLAUDE.md and .github/copilot-instructions.md with a single AGENTS.md as the canonical source of truth for AI coding agent instructions. CLAUDE.md becomes a symlink to AGENTS.md; copilot- instructions.md is removed since copilot reads AGENTS.md natively. Co-Authored-By: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-Authored-By: Vic Kerr <wiktor.kerr@ionos.com>
Add AI-Assisted Contributions section to CONTRIBUTING.md covering authorship, transparency, human oversight, and quality expectations. Add general PR scope guidelines (complete, self-contained, reasonable size, dependent PRs). Reference CONTRIBUTING.md from AGENTS.md. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Shell linting via shellcheck in CI and ShellSpec test runner with kcov coverage reporting and SonarQube integration. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Updates Go version across go.mod, Dockerfile, and docs/Development.md. Also removes the Docker dependabot ecosystem since Go images are now managed through bump-go.sh. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Updates golangci-lint version across go.mod replace directive and .custom-gcl.yaml. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Bumps k8s.io/{api,apimachinery,client-go}, manages replace pins, and
updates KUBERNETES_VERSION in e2e configs and docs. Adds gomod_make_envtest
helper and hack/envtest-ver.sh to derive ENVTEST_K8S_VERSION from go.mod.
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Philipp Born <98814235+pborn-ionos@users.noreply.github.com>
Bumps cluster-api and cluster-api/test in go.mod, adds releaseSeries entries to e2e metadata, and updates CAPI provider references in e2e configs. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Checks that Go, golangci-lint, cluster-api, and k8s.io versions are consistent across go.mod, Dockerfile, docs, Makefile, .custom-gcl.yaml, and e2e configs. Includes integration tests for each consistency check. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Detects version mismatches in go.mod and runs the appropriate bump-* scripts to fix them. Adds the versions.yml workflow that auto-bumps related packages on dependabot PRs and verifies version consistency on all PRs. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
The conversion-gen tool lives under k8s.io/code-generator and its replace pin must stay in sync with the k8s.io trio when the Kubernetes version is bumped. - bump-k8s.sh: update the replace pin after the k8s.io trio bump - verify-versions.sh: fail on drift between code-generator and k8s.io/api - auto-bump.sh: treat drift as a trigger for running bump-k8s.sh - go.mod: switch from a sub-path replace of .../cmd/conversion-gen to a parent-module replace of k8s.io/code-generator so it resolves cleanly via go list -m Co-authored-by: Claude <noreply@anthropic.com>
The KAL linter config has its own run.go field that must stay in sync with go.mod's Go version. - helpers.sh: add golangcikal_get_go / golangcikal_set_go (yq-based, preserves the double-quoted string style so the value isn't parsed as a YAML float) - bump-go.sh: update .golangci-kal.yml alongside go.mod, Dockerfile, and docs - verify-versions.sh: fail on drift between .golangci-kal.yml run.go and go.mod - auto-bump.sh: include .golangci-kal.yml in the Go drift trigger Co-authored-by: Claude <noreply@anthropic.com>
release.sh always updates clusterctl-settings.json (nextVersion) and sonar-project.properties (sonar.projectVersion); on a new major.minor it also appends a releaseSeries entry to metadata.yaml and bumps the capmox sentinel in test/e2e/config/proxmox-*.yaml. Pre-release suffixes are preserved in the clusterctl/sonar files and stripped for the metadata entry and the e2e sentinel (both track major.minor only). The CAPI contract for a new entry defaults to the contract of the latest existing entry and can be overridden via an optional second argument. While here, clean up metadata helper naming per the METADATA_FILE / E2E_METADATA_FILE convention: metadata_has_release and metadata_add_release now operate on the top-level metadata.yaml (the capmox release catalog). The previous e2e-facing variants are renamed to e2emetadata_*. Callers in bump-capi.sh and verify-versions.sh are updated. New helpers: clusterctl_get/set_version (JSON via yq), sonar_get/set_version (line-based), e2econfig_get/set_capmox (yq selects on InfrastructureProvider). Fixtures: add clusterctl-settings.json + sonar-project.properties, reorder metadata.yaml to chronological (newest last) to match the real file, and add an InfrastructureProvider section to the e2e config fixtures so the sentinel helpers and release.sh are testable. Co-authored-by: Claude <noreply@anthropic.com>
Catch drift between the four files release.sh updates together:
- clusterctl-settings.json (nextVersion) and sonar-project.properties
(projectVersion) must match after stripping the v-prefix
- clusterctl nextVersion's major.minor must be listed in metadata.yaml
releaseSeries
- the capmox sentinel in test/e2e/config/proxmox-*.yaml must equal
v{major}.{minor}.99 for that same major.minor
Pre-release suffixes (-rc.0, -beta.1, ...) are stripped from clusterctl
nextVersion before checking major.minor.
Co-authored-by: Claude <noreply@anthropic.com>
Wraps hack/verify-versions.sh so docs and CI can refer to a named make target instead of the script path directly. Co-authored-by: Claude <noreply@anthropic.com>
The debug flag made the scanner output very noisy with no benefit in normal runs. Drop it. Co-authored-by: Claude <noreply@anthropic.com>
Turn test-sh into a target that depends on the Sonar generic coverage XML file. kcov 43+ produces this file natively alongside cobertura.xml; the xsltproc rule is a fallback for older kcov installs (Ubuntu <= 24.04 ships kcov 42). The fallback skips itself when kcov already produced the file. TODO comment flags the rule + XSLT for removal once CI's kcov is >= 43. Co-authored-by: Claude <noreply@anthropic.com>
…gcl) golangci-lint appears in three places: go.mod's require (indirect) directive, go.mod's replace directive, and .custom-gcl.yaml. Dependabot can bump any one of these and leave the others stale, so all three must be checked and bumped together. - bump-golangci-lint.sh now validates the input version and updates the require directive alongside the replace pin and .custom-gcl.yaml - verify-versions.sh compares all three references instead of just replace vs .custom-gcl.yaml - auto-bump.sh picks the newest of the three references as the bump target (previously assumed require) Co-authored-by: Claude <noreply@anthropic.com>
make test-sh falls back to xsltproc to convert kcov's cobertura.xml into Sonar's generic coverage XML when kcov < 43 doesn't emit sonarqube.xml natively. ubuntu-latest still ships kcov 42, so we need xsltproc on the runner. Co-authored-by: Claude <noreply@anthropic.com>
Documents the version bumping scripts and the verify-versions target added in this PR. Co-Authored-By: Vic Kerr <wiktor.kerr@ionos.com> Co-Authored-By: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Issue #, if available:
fixes #635
fixes #580
Description of changes:
Adds a number of scripts for bumping deps and verifying version consistency.
``
Added shellcheck and shellspec-based tests, run make lint-sh test-sh.
Added workflows/jobs to test these automatically.
Added a workflow that runs on dependabot PRs and completes the version bump.
Dockerfile removed from dependabot config as it is now managed by bump-go.
Testing performed:
make lint-sh test-sh. Actions.
Haven't tested the dependabot action because it needs a relevant dependabot PR.
The test workflow fails because sonar-project.properties pulls in the shell test coverage report which isn't produced because we made it so that the workflow version in main runs.