Skip to content

feat: add Shipwright in-cluster build support to setup-k8s.sh#148

Open
moonlight16 wants to merge 3 commits into
rossoctl:mainfrom
moonlight16:feat/shipwright-k8s-builds
Open

feat: add Shipwright in-cluster build support to setup-k8s.sh#148
moonlight16 wants to merge 3 commits into
rossoctl:mainfrom
moonlight16:feat/shipwright-k8s-builds

Conversation

@moonlight16

@moonlight16 moonlight16 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Background

The Kind and OpenShift setup paths each have a way to get images built for the
cluster — setup-kind.sh builds locally and kind loads them, and setup-ocp.sh
can lean on OpenShift's built-in oc new-build. The generic Kubernetes path added in
#142 (setup-k8s.sh) consumes prebuilt image refs via --image/--sandbox-image,
since vanilla Kubernetes has no standard in-cluster build system to build them for you.

This PR adds an in-cluster build option for that path, using
Shipwright: a small driver script that builds the harness
(and optionally sandbox) image straight from a git branch to a registry the cluster
can already reach — no local Docker daemon and nothing pushed over the internet — and
prints the resulting image refs ready to hand to setup-k8s.sh. It's opt-in and
changes nothing for users who already build their images elsewhere.

What's included

  • deploy/knative/setup-shipwright-build.sh — driver script that creates a Shipwright
    Build + BuildRun, waits for it to complete, and prints HARNESS_IMAGE=/SANDBOX_IMAGE=
    lines that feed directly into setup-k8s.sh --image/--sandbox-image.
  • Templated Build manifests under deploy/knative/shipwright/ (build-harness.yaml,
    build-sandbox.yaml).
  • Updated deploy/knative/README-k8s.md with usage instructions and where this fits
    alongside the local-build path.
  • Fix (second commit): live-cluster testing surfaced a bug where the script hardcoded
    the Shipwright Build object's name (serverless-harness), so re-running it could
    silently overwrite an existing Build of the same name and clobber its config. Fixed by
    deriving the default Build name from --tag (e.g. serverless-harness-<tag>), adding
    --build-name to opt into targeting a specific existing Build on purpose, and warning
    before overwriting a Build whose spec differs from what the current run would set.
  • Fix (third commit): code review surfaced two more issues in that same collision guard:
    --build-name, when combined with --with-sandbox, produced identical harness/sandbox
    Build names instead of acting as a prefix; and the guard's existence check swallowed real
    kubectl errors as "no existing Build" and used unreliable substring matching. Fixed both.

Test plan

This is a draft PR — code review and live-cluster testing (including a re-verification
pass of the latest fix) are complete. Remaining before marking ready: the user's own final
review.

  • Tested live against a real Kubernetes cluster with Shipwright installed: built the
    harness image in-cluster via setup-shipwright-build.sh.
  • Verified the pushed image was pullable from the in-cluster registry.
  • Independent code review identified and fixed two collision-guard bugs (see third
    commit above).
  • Re-ran the collision-guard test live against the third commit's fix: confirmed
    --build-name now produces distinct harness/sandbox Build names, the guard warns on
    a genuine revision/image mismatch, does not false-positive on an idempotent re-run,
    and does not misreport a not-yet-created Build as an error. Production
    Build/serverless-harness and Build/serverless-harness-sandbox were unchanged
    throughout, and all test objects were cleaned up afterward.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

Adds deploy/knative/setup-shipwright-build.sh plus templated Build manifests
so anyone on a Kubernetes cluster with Shipwright installed can build the
harness/sandbox images in-cluster and feed the resulting refs straight into
setup-k8s.sh --image/--sandbox-image, instead of needing a local Docker
daemon or pushing over the internet.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
… name

Live-cluster testing showed that reusing the hardcoded "serverless-harness"
Build object name silently overwrote an existing Build's revision/output
image. Default the Build name to include --tag (serverless-harness-<tag>)
so a test run with a different tag can't collide with an existing Build,
add --build-name to target a specific Build on purpose, and warn before
overwriting a Build whose revision/output differs from what this run would
set.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
--build-name combined with --with-sandbox produced identical harness/
sandbox Build names instead of acting as a prefix, contradicting its
own help text. Sandbox now gets "-sandbox" appended.

warn_if_build_exists_with_different_spec swallowed all kubectl errors
as "no existing Build" via `2>/dev/null || return 0`, and used grep
substring matching on raw JSON that produces false negatives on
plausible inputs (e.g. v2.1.0 vs 2.1.0). Now distinguishes real
kubectl errors from NotFound, and compares exact jsonpath-extracted
fields instead of substrings.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
@moonlight16

Copy link
Copy Markdown
Member Author

Note on the failing trivy-scan check — this failure is unrelated to the changes in this PR.

  • This PR changes only deploy/knative/README-k8s.md, deploy/knative/setup-shipwright-build.sh, and two YAML manifests under deploy/knative/shipwright/. It touches no dependency manifests — every go.mod / go.sum / pnpm-lock.yaml / package.json is byte-identical to main.
  • trivy-scan runs trivy fs . with severity: CRITICAL,HIGH and exit-code: 1, so it fails on any HIGH/CRITICAL CVE in the existing dependency tree — regardless of what a PR changes.
  • The same job passed on July 21 (parent PR Add setup-k8s.sh: generic Kubernetes install path #142) and fails on July 22 (this PR), one day apart, after Trivy downloaded a freshly-updated vulnerability DB. This is DB drift surfacing a newly-disclosed CVE in an unchanged dependency, not a regression introduced here — the same scan would fail against current main.

Remediation (a dependency bump, .trivyignore, or ignore-unfixed: true) belongs in a separate maintenance PR, not this doc/script change.

@moonlight16
moonlight16 marked this pull request as ready for review July 22, 2026 18:06
@moonlight16
moonlight16 requested review from cwiklik and mrsabath July 22, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant