Skip to content

fix(chart): match preflight's image path and observability url refusals - #2362

Merged
freeqaz merged 2 commits into
mainfrom
fix-1271-image-userinfo-parity
Oct 13, 2026
Merged

fix(chart): match preflight's image path and observability url refusals#2362
freeqaz merged 2 commits into
mainfrom
fix-1271-image-userinfo-parity

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Problem

#2358 made profile preflight refuse two spellings that other layers still accepted:

  • An uppercase letter in an image repository path. Kubernetes refuses to pull such an image (InvalidImageName), yet the chart's image rule (images.*, repositoryCredentials.image), prepare-bootstrap-volume, upgrade-production-images and Compute's gateway and Agent image check all accepted it.
  • An @ before the observability.url host (http://@host). URL parsing drops the empty user info, and Installation startup stored the host alone, while preflight refuses it.

Change

  • Chart: images.* and repositoryCredentials.image fail with ... repository path must be lowercase; Kubernetes refuses to pull an uppercase one, using fix(profiles): refuse an @ before url hosts and uppercase image repository paths #2358's rule (uppercase only in the registry host, the first component when a / follows it, and in the tag).
  • prepare-bootstrap-volume --image and upgrade-production-images --controller-image/--broker-image/--runtime-image refuse the same, with the same sentence.
  • Compute refuses an uppercase repository path in its gateway and Agent images, with or without a digest.
  • Installation startup refuses any @ before the observability.url host (refuseWrittenUserinfo, as OCC_AUTH_BASE_URL does since fix(profiles): refuse an @ before url hosts and uppercase image repository paths #2358).
  • API and worker startup report OBSERVABILITY_URL_INVALID for every observability.url refusal (credentials and non-HTTP schemes used to log STARTUP_FAILED) and a new RUNTIME_IMAGE_INVALID for a gateway or Agent image without a required digest or with an uppercase repository path (both were STARTUP_FAILED).
  • Docs: fix(profiles): refuse an @ before url hosts and uppercase image repository paths #2358's breaking-changes entry is extended rather than adding a new one; configuration, controller and observability-link docs name the new refusals and code. production-startup.md and drivers/kubernetes-compute.md are left alone: they sit at 2498 and 2493 words.

Verification

Risks

  • Breaking for configurations that used an uppercase repository path; Kubernetes could never pull those images, so such Installations never worked. Uppercase registry hosts and tags still pass.
  • RUNTIME_IMAGE_INVALID is a new startup log code; operators that matched STARTUP_FAILED for those cases see the more specific code.

@freeqaz
fix(chart): refuse uppercase image repository paths and an @ before o…
8cfd563 
…bservability hosts everywhere

Profile preflight refuses both since #2358. The chart (images.* and
repositoryCredentials.image), prepare-bootstrap-volume,
upgrade-production-images and Compute's gateway and Agent image check now
refuse an uppercase repository path too, which Kubernetes cannot pull.
Installation startup refuses any @ before the observability.url host.
API and worker startup report OBSERVABILITY_URL_INVALID and the new
RUNTIME_IMAGE_INVALID instead of STARTUP_FAILED.
@freeqaz
freeqaz requested a review from a team as a code owner October 13, 2026 20:43
@clawsweeper

clawsweeper Bot commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@freeqaz
fix(chart): report every observability.url refusal as OBSERVABILITY_U…
6ef5e31 
…RL_INVALID

Review follow-up: a URL with credentials or a non-HTTP scheme logged
STARTUP_FAILED, so the configuration docs' claim that any @ before the
host reports OBSERVABILITY_URL_INVALID was only true for an empty user
info. Also reword the breaking-changes clause so it no longer reads as
if the chart lets uppercase repository paths through.
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 13, 2026 20:48 — with GitHub Actions Inactive
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 13, 2026 20:48 — with GitHub Actions Inactive
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 13, 2026
@clawsweeper

clawsweeper Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR aligns Helm, operator tools, and controller startup with existing image-path and observability URL refusals.

Example: Installation YAML sets observability.url to http://@grafana.example.internal/d.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful, focused patch with credible integration coverage and no actionable defect found; behavior evidence is limited to reported test execution.
Proof confidence 🦐 gold shrimp (3/6) The supplied MEMBER classification places this PR outside the ordinary contributor proof gate; inspected tests exercise the real validation owners, while downloaded screenshots concern unrelated role assignment and video inspection was unavailable.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Operators can bypass preflight and reach later image-pull failures or have observability URLs silently rewritten at startup.
Reason: The focused change completes documented validation rules across supported entry points; the supplied maintainer-authored classification establishes direction.

Merge readiness

✅ Ready for maintainer review

Keep this PR open for maintainer triage: it closes verified validation gaps still present on current main, with no concrete introduced defect found.

Priority: P2
Reviewed head: 6ef5e319f2228372ed3d92ec6413d75d89ffa5d2

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: No supplied live terminal evidence demonstrates these validation paths; the body reports base/head regression checks, and the inspected media covers unrelated UI behavior.
Agent review details

How this fits together

Installation profiles, Helm values, and operator flags supply images and URLs; packaging and controller composition validate them before database or Kubernetes operations.

flowchart TD
  A[Installation profile] --> B[Profile preflight]
  B --> C[Helm values and Installation YAML]
  C --> D[Helm validation]
  C --> E[API and worker startup]
  F[Operator image flags] --> G[Image tool validation]
  E --> H[Kubernetes Compute validation]
Loading

Technical review

Best possible solution:

Use the existing validation boundaries consistently, preserving valid image hosts and tags and explicitly refusing rewritten observability URLs.

Do we have a high-confidence way to reproduce the issue?

Current-main source establishes both gaps, and the PR reports failing-before/passing-after cases; no target code was executed during this review.

Is this the best way to solve the issue?

The patch extends the existing owners and shared URL check without adding a competing configuration path or changing supported valid image spellings.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 1a0c30a23f33.

Provenance checked

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • freeqaz: Raw commit 70b7825 adds scripts/upgrade-production-images:68 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 70b782554242; files: scripts/upgrade-production-images)
  • Russell Bryant: Raw commit 3935a45 adds apps/controller/src/composition/installation-config.ts:250 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 3935a4545342; files: apps/controller/src/composition/installation-config.ts)
  • Dallin Romney: Raw commit 75bbb0d adds scripts/upgrade-production-images:48 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 75bbb0d334f2; files: scripts/upgrade-production-images)

Review metrics

Metric Value Why it matters
Change distribution Production +80/-8; tests +198/-34; documentation +27/-16 Most additions cover refusals and preservation of valid inputs across existing entry points; production growth is focused validation and diagnostic code.

Labels

Label changes:

  • add P2: This fixes bounded operator validation gaps that otherwise surface as image-pull failures or silently rewritten configuration.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: This fixes bounded operator validation gaps that otherwise surface as image-pull failures or silently rewritten configuration.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Attach redacted terminal output showing the chart, image tools, and API/worker refusals alongside valid-input acceptance.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 13, 2026, 4:53 PM ET / 20:53 UTC.

@freeqaz
freeqaz merged commit f0845f6 into main Oct 13, 2026
51 checks passed
@freeqaz
freeqaz deleted the fix-1271-image-userinfo-parity branch October 13, 2026 20:56
@freeqaz freeqaz mentioned this pull request Oct 13, 2026
Merged
@clawsweeper clawsweeper Bot mentioned this pull request Oct 13, 2026
Merged
@freeqaz freeqaz mentioned this pull request Oct 14, 2026
Merged
This was referenced Oct 14, 2026
Merged
Merged

This branch was previously deployed

1 inactive deployment
integration-qa-pr — 6ef5e319 Deployed Oct 13, 2026 by freeqaz via QA advisory (kubernetes) #727
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@freeqaz