diff --git a/.github/scripts/check-pr-linked-issue.mjs b/.github/scripts/check-pr-linked-issue.mjs index 0dda2fdd4e74..e2c30c22c5ca 100644 --- a/.github/scripts/check-pr-linked-issue.mjs +++ b/.github/scripts/check-pr-linked-issue.mjs @@ -12,7 +12,11 @@ import { fileURLToPath } from 'node:url'; const ISSUE_PATTERNS = [ - /(?:fixes|closes|resolves|refs)\s+#\d+/i, + // The leading `(? { + assert.equal(checkLinkedIssue('derefs BLO-20901', 'fix: bug').passed, false); + assert.equal(checkLinkedIssue('unresolves BLO-20901', 'fix: bug').passed, false); + assert.equal(checkLinkedIssue('prefixes BLO-20901', 'fix: bug').passed, false); +}); + +// The #NNN keyword pattern has the same class of left-boundary gap, but a +// bare `#NNN` (with no keyword at all) is separately accepted as evidence — +// see "passes with bare #NNN reference" above — so an embedded keyword next +// to a bare #NNN still passes overall. Use a body with no other qualifying +// pattern to isolate the keyword-boundary behavior itself. + +test('embedded keyword next to #NNN does not smuggle in extra credit, but bare #NNN still passes on its own', () => { + assert.equal(checkLinkedIssue('derefs #123', 'fix: bug').passed, true); +}); + // Prefix-aware skip behavior test('skips check for docs: prefix', () => { diff --git a/deploy/helm/paperclip/Chart.yaml b/deploy/helm/paperclip/Chart.yaml index 940afc4fb6a7..71aae84a4551 100644 --- a/deploy/helm/paperclip/Chart.yaml +++ b/deploy/helm/paperclip/Chart.yaml @@ -4,10 +4,14 @@ description: Self-hosted Paperclip AI — company orchestration platform with a type: application version: 0.1.0 appVersion: "v2026.416.0" -# The api Deployment's topologySpreadConstraints (BLO-20901) uses minDomains -# (GA in 1.30) and matchLabelKeys (beta, default-enabled since 1.27). 1.30 is -# the binding floor between the two. -kubeVersion: ">=1.30.0-0" +# No chart-wide kubeVersion floor: the api Deployment's topologySpreadConstraints +# (BLO-20901) needs minDomains (GA in 1.30) and matchLabelKeys (beta since 1.27), +# but that requirement only applies on the api.enabled && api.spreadAcrossNodes +# path. A chart-level `kubeVersion` field is evaluated unconditionally at +# install/template time regardless of .Values, so it would reject installs on +# older clusters even when that path never renders. The version is instead +# enforced in templates/deployment-api.yaml, guarded by the same condition +# that renders the constraint. keywords: - paperclip - ai diff --git a/deploy/helm/paperclip/templates/deployment-api.yaml b/deploy/helm/paperclip/templates/deployment-api.yaml index 72515ef1499b..b58dfd72ae7d 100644 --- a/deploy/helm/paperclip/templates/deployment-api.yaml +++ b/deploy/helm/paperclip/templates/deployment-api.yaml @@ -92,6 +92,16 @@ spec: priorityClassName: {{ . }} {{- end }} {{- if .Values.api.spreadAcrossNodes }} + {{- /* + BLO-20901 follow-up: minDomains (GA 1.30) and matchLabelKeys (beta 1.27) + below need 1.30. This used to be a chart-wide `kubeVersion` floor in + Chart.yaml, which rejected installs on older clusters even with + api.enabled=false, where none of this renders. Scope the check to the + path that actually needs it instead. + */}} + {{- if not (semverCompare ">=1.30.0-0" .Capabilities.KubeVersion.Version) }} + {{- fail (printf "paperclip-api: api.spreadAcrossNodes requires Kubernetes >=1.30.0-0 (cluster is %s) for topologySpreadConstraints minDomains/matchLabelKeys support. Set api.spreadAcrossNodes=false to install on this cluster." .Capabilities.KubeVersion.Version) }} + {{- end }} affinity: podAntiAffinity: preferredDuringSchedulingIgnoredDuringExecution: diff --git a/deploy/helm/paperclip/tests/topology-spread.test.mjs b/deploy/helm/paperclip/tests/topology-spread.test.mjs index 0c4d905c779a..8191ab784b3b 100644 --- a/deploy/helm/paperclip/tests/topology-spread.test.mjs +++ b/deploy/helm/paperclip/tests/topology-spread.test.mjs @@ -1,4 +1,5 @@ import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; import assert from "node:assert/strict"; import { test } from "node:test"; import path from "node:path"; @@ -30,6 +31,32 @@ function renderTemplate(extraArgs = []) { ); } +function renderTemplateExpectingFailure(extraArgs = []) { + try { + execFileSync( + "helm", + [ + "template", + "paperclip", + "deploy/helm/paperclip", + "--namespace", + "paperclip", + "-f", + "deploy/helm/paperclip/values.blockcast.yaml", + "--show-only", + "templates/deployment-api.yaml", + "--set", + "api.enabled=true", + ...extraArgs, + ], + { cwd: repoRoot, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }, + ); + assert.fail("expected helm template to fail"); + } catch (err) { + return err.stderr?.toString() ?? ""; + } +} + test("API deployment hard-enforces node spread by default (BLO-20901)", () => { const rendered = renderTemplate(); @@ -78,3 +105,62 @@ test("API deployment topologySpreadConstraints merges with custom .Values.affini assert.match(rendered, /nodeAffinity:/); assert.match(rendered, /requiredDuringSchedulingIgnoredDuringExecution:/); }); + +// BLO-20901 follow-up (Ally review on #965): a chart-wide `kubeVersion` floor +// in Chart.yaml rejected every install on an older cluster, even with +// api.enabled=false where none of this ever renders. The version check moved +// into the template, gated by the same condition that renders the +// minDomains/matchLabelKeys constraint. + +test("chart has no chart-wide kubeVersion floor", () => { + const chartYaml = readFileSync( + path.join(repoRoot, "deploy/helm/paperclip/Chart.yaml"), + "utf8", + ); + assert.doesNotMatch(chartYaml, /^kubeVersion:/m); +}); + +test("installs on an older cluster when api.enabled=false", () => { + const rendered = execFileSync( + "helm", + [ + "template", + "paperclip", + "deploy/helm/paperclip", + "--namespace", + "paperclip", + "-f", + "deploy/helm/paperclip/values.blockcast.yaml", + "--set", + "api.enabled=false", + "--kube-version", + "1.28.0", + ], + { cwd: repoRoot, encoding: "utf8" }, + ); + + assert.doesNotMatch(rendered, /deployment-api/); +}); + +test("installs on an older cluster when api.enabled=true and spreadAcrossNodes=false", () => { + const rendered = renderTemplate([ + "--set", + "api.spreadAcrossNodes=false", + "--kube-version", + "1.28.0", + ]); + + assert.doesNotMatch(rendered, /topologySpreadConstraints:/); +}); + +test("fails clearly on an older cluster when api.enabled=true and spreadAcrossNodes=true", () => { + const stderr = renderTemplateExpectingFailure([ + "--set", + "api.spreadAcrossNodes=true", + "--kube-version", + "1.28.0", + ]); + + assert.match(stderr, /spreadAcrossNodes requires Kubernetes >=1\.30\.0-0/); + assert.match(stderr, /cluster is v1\.28\.0/); +});