[HYPERSHELL-46] Automated PRs to update OpenShell gateway and supervisor images via Renovate - #240
Conversation
…ovate
Add a Renovate customManager (regex) that detects the OpenShell gateway and
supervisor image references pinned as env var values in deploy/base/ manifests.
A regex versioningTemplate filters the 3000+ noisy tags (commit SHAs,
arch-specific, build artifacts) down to the official release convention
(v{major}.{minor}.{patch}-rhaiv.{build}). Both images are grouped into a single
"OpenShell images" PR with automerge on green CI.
Includes the auto-update spec under specs/platform/.
[HYPERSHELL-46]
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The em dash appears literally in the rule documenting that em dashes are forbidden; it must be shown to be unambiguous. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Amber reviewStatus: Complete |
jsell-rh
left a comment
There was a problem hiding this comment.
Verdict
This is a clean, well-documented CI/config change that wires a Renovate custom regex manager to keep the GATEWAY_IMAGE/GATEWAY_SUPERVISOR_IMAGE pins in deploy/base/ up to date, gated behind the e2e suite. The spec is accurate against the code (StaticImageDefaults reads both env vars; the referenced config.go TODO exists), the regex matches all four real references, and the forbidden-terms hook passes with the whitelisted em dash on CLAUDE.md:125. I have no blocking findings, only a few minor items and one policy decision worth a maintainer's explicit sign-off.
Details
Confidence: High on the config/spec correctness, Medium on the Renovate-version-specific notes (no pinned Renovate version to check against).
- Digest-required regex is brittle (Minor).
matchStringsrequires@sha256:<digest>. Any future reference pinned tag-only (no digest) is silently skipped: no update, no error. The spec's "both must be updated together" invariant is only enforced by convention. Consider a CI guard (or a second matchString) so a tag-only pin fails loudly rather than dropping out of auto-update coverage. - Automerge of third-party images is a policy decision (Discussion).
automerge: trueon external OpenShell images is a supply-chain posture. It is reasonably mitigated byminimumReleaseAge: 14 daysand the full e2e gate, but unattended merging of upstream container images should be an explicit maintainer choice, not an implicit one. fileMatchis deprecated in recent Renovate (Minor, low confidence). Newer Renovate renamedfileMatchtomanagerFilePatterns. Depending on the Renovate version this org runs,fileMatchmay only emit a deprecation warning today. Worth confirming.- En dash inconsistent with the new convention (Minor).
specs/platform/openshell-image-auto-update.spec.md:91uses an en dash (U+2013) in00:00–07:00. The hook only rejects em dashes (U+2014), so CI passes, but this contradicts the intent of the "use hyphens" convention this same PR introduces.
Cross-PR coordination
An open pull request that adopts the upstream OpenShell Helm chart for gateway deployments rewrites the exact GATEWAY_IMAGE/GATEWAY_SUPERVISOR_IMAGE values in deploy/base/controller.yaml that this PR's Renovate regex depends on, changing them to a different registry/org, dropping the @sha256: digest, and dropping the -rhaiv.{build} version scheme. That is a material design conflict, not a text merge conflict: it moves the source of truth for gateway images (env-var pins vs. Helm chart values) and would silently disable this PR's auto-update regex (no digest, no matching package name, no matching versioning template), so Renovate would stop detecting bumps without any error. Because both changes are authored by the same person, the author and maintainers need to decide the merge order and reconcile the pinning scheme - specifically whether Renovate should track the pinned env-var images or the Helm chart reference, and update this spec/regex accordingly. Coordination required with PR #194.
Findings Summary (ordered by severity, highest first)
- [Minor] Digest-required
matchStringssilently skips any tag-only pin - no update, no error - Maintainability (renovate.json L14) - [Minor]
automerge: trueon external OpenShell images is an unattended supply-chain merge policy that should be an explicit maintainer decision - Security / Policy (renovate.json L62) - [Minor]
fileMatchis deprecated in recent Renovate in favor ofmanagerFilePatterns- Convention (renovate.json L12) - [Minor] En dash (U+2013) contradicts the "use hyphens" convention this PR adds - Spec Consistency (spec L91)
Convention Checklist
| Convention | Result |
|---|---|
| Forbidden-terms hook passes (em dash whitelisted) | Pass |
| JSON config valid (renovate.json, whitelist) | Pass |
Spec claims match code (StaticImageDefaults, env vars, config.go TODO) |
Pass |
| Image references consistent across manifests | Pass |
| Conventional commit messages | Pass |
| Config separated from code | Pass |
| "description": "OpenShell gateway and supervisor images pinned as env var values in deployment manifests", | ||
| "fileMatch": ["^deploy/base/.*\\.yaml$"], | ||
| "matchStrings": [ | ||
| "-\\s*name:\\s*GATEWAY(?:_SUPERVISOR)?_IMAGE\\n\\s+value:\\s*(?<depName>[^:\\s]+):(?<currentValue>[^@\\s]+)@(?<currentDigest>sha256:[a-f0-9]+)" |
There was a problem hiding this comment.
[Minor] Digest-required regex is brittle. This matchStrings only matches references that carry an @sha256:<digest>. If a future GATEWAY_IMAGE/GATEWAY_SUPERVISOR_IMAGE value is ever pinned tag-only, it is silently skipped - no update proposed and no error raised - so it quietly falls out of auto-update coverage. Consider a CI guard (or a second, digest-optional matchString) so a tag-only pin fails loudly instead of disappearing from Renovate's view.
| "quay.io/opendatahub/odh-openshell-supervisor" | ||
| ], | ||
| "groupName": "OpenShell images", | ||
| "automerge": true, |
There was a problem hiding this comment.
[Minor / Policy] Unattended merge of third-party images. automerge: true on external OpenShell images is a supply-chain posture. It is reasonably mitigated by the inherited minimumReleaseAge: 14 days and the full e2e gate, but auto-merging upstream container images without human review should be an explicit maintainer decision. Please confirm that is intended.
| { | ||
| "customType": "regex", | ||
| "description": "OpenShell gateway and supervisor images pinned as env var values in deployment manifests", | ||
| "fileMatch": ["^deploy/base/.*\\.yaml$"], |
There was a problem hiding this comment.
[Minor] fileMatch deprecation. Recent Renovate renamed fileMatch to managerFilePatterns for custom managers. Depending on the Renovate version this repo runs, fileMatch may only emit a deprecation warning today. Worth confirming against the deployed version to avoid the manager silently no-op'ing on a future upgrade.
|
|
||
| The auto-update follows the repository's existing Renovate conventions: | ||
|
|
||
| - **Schedule:** Monday 00:00–07:00 ET (inherited from top-level `schedule`) |
There was a problem hiding this comment.
[Minor] En dash contradicts the new convention. 00:00–07:00 uses an en dash (U+2013). The forbidden-terms hook only rejects em dashes (U+2014), so CI passes, but this contradicts the intent of the "use hyphens" convention introduced by this same PR. Suggest 00:00-07:00.
- Make digest optional in matchStrings so tag-only pins are still detected instead of silently skipped - Disable automerge on OpenShell images (supply-chain policy: require human review for external image bumps) - Rename fileMatch to managerFilePatterns (fileMatch deprecated in recent Renovate) - Replace en dash with hyphen in spec time range Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Amber reviewStatus: Complete |
jsell-rh
left a comment
There was a problem hiding this comment.
Verdict
This is a clean, well-scoped CI/config change: the Renovate customManagers regex, the version-filtering template, and the matchPackageNames grouping all match the real image pins in deploy/base/controller.yaml and deploy/base/control-plane/deployment.yaml, and both renovate.json and the forbidden-terms check pass locally. I found no blockers or security issues; the notes below are minor robustness/coordination items, plus one cross-PR conflict that needs a maintainer decision.
What I verified
renovate.jsonis valid JSON and the newcustom.regexmanager keys are well-formed.- The
matchStringsregex matches all fourGATEWAY_IMAGE/GATEWAY_SUPERVISOR_IMAGEreferences indeploy/base/with correctdepName/currentValue/currentDigestcapture groups. managerFilePatterns(^deploy/base/.*\.yaml$) correctly includes the base manifests and excludes the IBM overlay (deploy/ibm/kustomization.yaml), which uses an internal-mirror reference and is documented as out of scope.- The current pinned tag
v0.0.109-rhaiv.0satisfies theregex:versioning template, so Renovate can resolve the current version and order future bumps. python3 scripts/check_forbidden_terms.pypasses: the em-dash whitelist entry points atCLAUDE.mdline 125, which is exactly where the new "No em dashes" rule (which must contain a literal em dash) lives.
Cross-PR coordination
Another open pull request adopts the upstream OpenShell Helm chart for gateway deployments and, as part of that work, rewrites the GATEWAY_IMAGE / GATEWAY_SUPERVISOR_IMAGE values in deploy/base/controller.yaml from quay.io/opendatahub/odh-openshell-*:v0.0.109-rhaiv.0@sha256:<digest> to quay.io/bsquizza/openshell-*:16112bc (a different registry/repository, no digest, and a tag that does not follow the v{major}.{minor}.{patch}-rhaiv.{build} convention). This is a direct assumption conflict with this PR: the Renovate manager here is keyed to the opendatahub package names, the tag@digest pin format, and the rhaiv versioning regex. If that PR merges, this manager silently tracks nothing (custom regex managers emit no error when they match no dependency). That PR also shifts image selection toward Helm chart values, which raises the question of whether the deploy/base env-var pins remain the authoritative source Renovate should watch. Maintainers should decide the authoritative image reference/registry/format for deploy/base/controller.yaml and coordinate merge order between that pull request and this one. This is a design decision, not a text merge conflict (the two PRs do not even touch the same files).
Findings
- [Minor] Silent no-match risk (
renovate.jsonmatchStrings) - Renovate customregexmanagers do not error when amatchStringspattern matches nothing; if the image reference format indeploy/baseever changes, the auto-update stops silently with no signal. Consider a lightweight CI guard (a test that asserts the regex still matches the currentdeploy/basemanifests) or completing the still-unchecked "Renovate dry-run" item in the test plan. Observability - [Minor]
depNamecapture assumes no registry port (renovate.jsonmatchStrings) -(?<depName>[^:\s]+)stops at the first colon, so aregistry:port/...reference would be mis-parsed. This is safe fordeploy/basetoday (no ports there; the port-bearing IBM mirror is excluded), so it's only worth a comment for future robustness. Robustness - [Minor] Line-pinned whitelist is fragile (
.forbidden-terms-whitelist.json/CLAUDE.mdL125) - the em-dash exemption is keyed toCLAUDE.mdline 125; any future insertion above that line shifts the rule and tripscheck-forbidden-termsuntil the whitelist is re-pinned. This follows the existing line-based convention, so it's acceptable, but a brief inline note by the rule would reduce future churn. Maintainability
Findings Summary (ordered by severity, highest first):
- [Minor] Custom regex manager fails silently if the
deploy/baseimage format changes; no CI guard - Observability (renovate.json matchStrings) - [Minor]
depNameregex assumes no registry port - Robustness (renovate.json matchStrings) - [Minor] Line-pinned forbidden-terms whitelist entry is fragile - Maintainability (CLAUDE.md L125)
Convention Checklist:
| Convention | Result |
|---|---|
| Valid Renovate JSON config | Pass |
Regex matches actual deploy/base image pins |
Pass |
managerFilePatterns scoping (base in, overlays out) |
Pass |
| Versioning template matches current pinned tag | Pass |
| Forbidden-terms check passes / whitelist correct | Pass |
| Image references consistent across manifests | Pass |
| Conventional commit messages | Pass |
Spec added and indexed (specs/index.spec.md) |
Pass |
| "customType": "regex", | ||
| "description": "OpenShell gateway and supervisor images pinned as env var values in deployment manifests", | ||
| "managerFilePatterns": ["^deploy/base/.*\\.yaml$"], | ||
| "matchStrings": [ |
There was a problem hiding this comment.
Robustness note (Minor): Renovate custom regex managers do not raise an error when a matchStrings pattern matches nothing - they simply produce zero dependencies. If the GATEWAY_IMAGE / GATEWAY_SUPERVISOR_IMAGE reference format in deploy/base ever changes (registry, env-var name, or a move into Helm values), this auto-update will silently stop with no signal. Consider a small CI guard that asserts the regex still matches the current deploy/base manifests, or complete the unchecked "Renovate dry-run" item in the test plan.
Separately: (?<depName>[^:\s]+) stops at the first colon, so a registry:port/... reference would be mis-parsed. Safe for deploy/base today (no ports; the port-bearing IBM mirror is excluded), but worth noting for future robustness.
| - **PatternFly 6 for web UI**: Reuse PatternFly and canonical shared components; do not create duplicate UI components | ||
| - **Narrow hexagonal UI boundary**: Put application workflows and external effects behind application-owned ports; keep React, TanStack Query, Fastify, generated SDKs, and infrastructure outside | ||
| - **Domain probes for UI observability**: Publish typed workflow and dependency facts through a fan-out port; no raw console or direct telemetry calls in production browser/BFF code | ||
| - **No em dashes**: Use hyphens (`-`) instead of em dashes (`—` U+2014) in all text files; the pre-commit hook rejects them |
There was a problem hiding this comment.
Maintainability note (Minor): the em-dash exemption in .forbidden-terms-whitelist.json is pinned to CLAUDE.md line 125. Any future line inserted above this rule shifts it and will trip check-forbidden-terms until the whitelist is re-pinned. This matches the existing line-based whitelist convention so it's acceptable, but the coupling is fragile.

Summary
customManagersregex manager that detects OpenShell gateway and supervisor image references pinned asGATEWAY_IMAGE/GATEWAY_SUPERVISOR_IMAGEenv var values indeploy/base/deployment manifestsregexversioning template to filter 3000+ noisy tags (commit SHAs, arch-specific, build artifacts) down to the 20 official releases matchingv{major}.{minor}.{patch}-rhaiv.{build}specs/platform/openshell-image-auto-update.spec.mddocumenting watched sources, version filtering, validation gate, and merge policyTest plan
deploy/base/controller.yamlanddeploy/base/control-plane/deployment.yaml— matches all 4 image references with correct capture groupsfileMatchcorrectly includesdeploy/base/and excludes IBM/Kind overlaysv0.0.113-rhaiv.2as latest🤖 Generated with Claude Code