Skip to content

chore(github-app): sync chart to v1.18.0 - #76

Merged
chrisleekr merged 3 commits into
mainfrom
chore/github-app-1.18.0
Sep 9, 2026
Merged

chrisleekr merged 3 commits into
mainfrom
chore/github-app-1.18.0

Conversation

@chrisleekr

@chrisleekr chrisleekr commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Chart sync: github-app v1.17.2 to v1.18.0

Chart version: 0.21.1 to 0.22.0
Image tag (appVersion): 1.18.0

Env surface changed (values.yaml / configmap.yaml / secret.yaml reconciled by claude-code-action, review that diff):

  • added: WORKFLOW_RUNNER_CPU_LIMIT,WORKFLOW_RUNNER_CPU_REQUEST,WORKFLOW_RUNNER_MEMORY_LIMIT,WORKFLOW_RUNNER_MEMORY_REQUEST,WORKFLOW_RUNNER_STORAGE_LIMIT,WORKFLOW_RUNNER_STORAGE_REQUEST
  • removed: ``

Upstream release notes (github-app v1.18.0)

1.18.0 (2026-09-09)

Features

  • k8s: make workflow-runner Pod resources configurable (#305) (3c8b39d)
  • orchestrator: capture runner Pod post-mortem before cleanup (#306) (4a68f45)
  • workflows: route every mention through one classifier (#307) (75bb54c)

Validated by lint.yml: ct lint (version-increment), plus the github-app gate in lint-github-app.yml -- the helm template matrix and the env-parity gate against the v1.18.0 contract.

Summary by CodeRabbit

  • New Features

    • Added configuration options for workflow-runner Pod CPU, memory, and ephemeral storage requests and limits.
    • Resource values are validated and use application defaults when left empty.
  • Documentation

    • Updated release metadata for application version 1.18.0 and Helm chart version 0.22.0.
    • Added an Artifact Hub entry documenting the v1.18.0 synchronization.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Warning

Review limit reached

Next included review available in 47 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: fd89be91-beab-48d1-9953-e1a550d7e858

📥 Commits

Reviewing files that changed from the base of the PR and between 9cfda22 and bb99cc3.

📒 Files selected for processing (6)
  • charts/github-app/Chart.yaml
  • charts/github-app/templates/configmap.yaml
  • charts/github-app/templates/workflow-runner-admission.yaml
  • charts/github-app/templates/workflow-runner-boundary.yaml
  • charts/github-app/values.yaml
  • scripts/check-github-app-render.sh

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 789564a6-1d71-4e64-8d05-cdc435f00be7

📥 Commits

Reviewing files that changed from the base of the PR and between c6a3f5f and 9cfda22.

📒 Files selected for processing (3)
  • charts/github-app/Chart.yaml
  • charts/github-app/templates/configmap.yaml
  • charts/github-app/values.yaml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Helm chart adds configurable workflow-runner Pod resource requests and limits, renders non-empty settings into the ConfigMap, and updates chart and Artifact Hub metadata for application version 1.18.0.

Changes

Workflow runner resources

Layer / File(s) Summary
Runner resource settings and rendering
charts/github-app/values.yaml, charts/github-app/templates/configmap.yaml
The chart defines CPU, memory, and ephemeral-storage requests and limits. The ConfigMap emits corresponding environment variables when values are non-empty.
Chart release metadata
charts/github-app/Chart.yaml
The chart version changes to 0.22.0, the application version changes to 1.18.0, and Artifact Hub metadata records the synchronization. The security-update annotation is removed.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 9cfda

This updates github-app to v1.18.0 and adds optional workflow-runner resource configuration. The chart settings and rendered environment variables are aligned, with no current merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: synchronizing the github-app Helm chart to v1.18.0.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chrisleekr-bot

chrisleekr-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

bot workflow review, succeeded

🔍 Code review complete, 3 files, +39/-3.

Summary

Chart sync of github-app 1.17.2 → 1.18.0 (chart 0.21.1 → 0.22.0). The env-surface half of the sync is correct: the six new WORKFLOW_RUNNER_* resource keys match the v1.18.0 contract exactly, are rendered conditionally so an unset value stays undefined (zod .default() only fires on undefined; an empty string would fail the new regex and crash the app at startup), and the docs describing the canonical-quantity rules match upstream config.ts. The sync is incomplete, though: upstream #305 also rewrote examples/workflow-runner-admission.yaml, and the chart's copy of the ValidatingAdmissionPolicy plus its boundary ConfigMap were not updated. scripts/check-policy-parity.sh fails, which is why the required lint check and github-app / gate are red on 9cfda22. Verdict: do not merge until the policy/boundary half is ported.

What was checked

  • git diff origin/main...HEAD — all 3 files, then each read in full: charts/github-app/Chart.yaml, charts/github-app/templates/configmap.yaml, charts/github-app/values.yaml.
  • Cross-referenced the untouched runner templates: templates/workflow-runner-admission.yaml (13-key form, hardcoded 500m/1Gi/2Gi/2/4Gi/10Gi at lines 79, 128, 251-265) and templates/workflow-runner-boundary.yaml ("Exactly 13 keys", lines 25-53), plus the existing conflict guard at templates/configmap.yaml:127-131.
  • Upstream diffs at v1.17.2 vs v1.18.0: env-contract.json (added exactly the six WORKFLOW_RUNNER_{CPU,MEMORY,STORAGE}_{REQUEST,LIMIT} keys, 113 total, none removed), src/config.ts (cpuQuantity/byteQuantity schemas, defaults 500m/1Gi/2Gi/2/4Gi/10Gi, validateRunnerResourceOrdering superRefine), examples/workflow-runner-admission.yaml (13 → 19 boundary keys).
  • Validation runs (local, with a python3 jq shim since jq is absent in this image):
    • bash scripts/check-github-app-render.shall cases rendered
    • bash scripts/check-env-parity.shOK: github-app chart env surface matches contract v1.18.0
    • bash scripts/check-policy-parity.shFAIL, policy drift on three validations ❌
  • Live CI rollup for 9cfda22: lint FAILURE (required), github-app / gate FAILURE, ct lint SUCCESS.

Findings

  • [blocker] charts/github-app/templates/configmap.yaml:151 — the comment's premise ("boundary is locked at 13 keys and never pins these") is false at v1.18.0. Upstream's policy now asserts size(params.data) == 19 and compares Pod requests/limits and the workspace emptyDir.sizeLimit against params.data['runnerCpuRequest']runnerStorageLimit; the chart still ships the 13-key policy and boundary ConfigMap. Result: policy-parity gate fails, and with workflowRunner.enabled=true + admission.enabled=true any operator who sets one of the new knobs has every runner Pod denied at admission. Fix: port the v1.18.0 policy document into templates/workflow-runner-admission.yaml, add the six runner*Request/runner*Limit keys to templates/workflow-runner-boundary.yaml (update its "Exactly 13 keys" comment), re-run both parity gates.
  • [major] charts/github-app/values.yaml:498 — the new block documents the six quantities as unpinned passthrough "in both provisioning modes"; that is inverted at v1.18.0. They now belong to the same class as namespace/nodeLabel/nodeValue/imagePullSecret, which the chart protects with a render-time fail() at templates/configmap.yaml:127-131. No equivalent guard, no provisioning half, and no render-matrix case covers them. Fix: add a provisioning half (or feed the boundary from config.workflowRunner.* with explicit chart defaults), extend the dict-driven conflict guard, correct the comment, add a render-matrix assertion.

Reasoning

Things that could have looked sketchy but are correct:

  • Conditional {{- if $cr.X }} rendering rather than unconditional keys. Necessary, not stylistic: upstream's cpuQuantity/byteQuantity are z.string().trim().regex(...).default(fallback), and zod applies .default() only for undefined. Emitting WORKFLOW_RUNNER_CPU_REQUEST: "" would reach the schema as an empty string and fail the regex at startup. Omitting the key is the only way "empty inherits the app default" actually holds.
  • Documented defaults and validation rules. 500m/1Gi/2Gi/2/4Gi/10Gi match src/config.ts at v1.18.0 exactly. The rejected-spellings list in the values comment (4G, 1.5, 512Ki, 8192Mi, 2000m, 1000) matches CPU_SHAPE/BYTE_SHAPE plus the canonicality rules (cpuMillis rejects a digit run ending in 000; byteMebis rejects a mantissa divisible by 1024), and "request above its matching limit rejected at startup naming the variable" matches validateRunnerResourceOrdering.
  • Minor chart bump (0.21.1 → 0.22.0). Correct per .github/workflows/github-app-sync.yml: minor when the env-contract key diff is non-empty, which it is (six added).
  • Removal of artifacthub.io/containsSecurityUpdates: "true". Correct — v1.18.0 is three features with no security fix, and the annotation is a property of the release being published, not a historical record (the kind: security entries in artifacthub.io/changes retain the history).
  • kind: changed Artifact Hub entry. Consistent with the existing list style and links to the right upstream release.
  • Placement of the new env vars in configmap.yaml rather than the runner templates. Deliberate and required — the env-parity gate greps only configmap.yaml, the two Secrets and daemon-deployment.yaml; the gate passes because of it.
  • No values.schema.json exists for this chart, so no schema update is owed.

cost: $2.4737 · turns: 32 · duration: 236s

Comment thread charts/github-app/templates/configmap.yaml Outdated
Comment thread charts/github-app/values.yaml Outdated
…undary

Upstream #305 moved the runner resource quantities into the admission
boundary params: at v1.18.0 the policy asserts size(params.data) == 19,
runs isQuantity on runnerCpuRequest/MemoryRequest/StorageRequest and the
three matching limits, and compares the Pod's requests, limits and the
workspace emptyDir sizeLimit against those params instead of the old
hardcoded 500m/1Gi/2Gi/2/4Gi/10Gi.

The chart carried the stale 13-key policy, which failed
check-policy-parity.sh, and exposed the six knobs as config.workflowRunner.*
passthrough only. With workflowRunner.enabled=true that was a deny-all
trap: setting one rendered cleanly and then denied every runner Pod at
admission, with no signal from helm template.

The quantities now have a provisioning half under workflowRunner.*,
defaulting to the app defaults at appVersion, feeding both the boundary
ConfigMap and the controller env from one value. They join
namespace/nodeLabel/nodeValue/imagePullSecret in the config.workflowRunner.*
conflict guard, and the boundary refuses an empty quantity rather than
render a param isQuantity rejects.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016dn1eK9UKHn3hTSXEDkzH6
@chrisleekr
chrisleekr merged commit 843d50d into main Sep 9, 2026
9 checks passed
@chrisleekr
chrisleekr deleted the chore/github-app-1.18.0 branch September 9, 2026 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants