ci: hold devnet auto-deploy on config changes#363
Merged
Conversation
The devnet deploy PR on infra-kubernetes is auto-merged only when no config paths (pkg/config, per-package config.go, embedded default YAMLs) changed between the commit devnet currently runs and the one being deployed. When config did change, the PR is opened without auto-merge, labeled config-change, and gets a comment with the added/removed schema keys, the raw config diff, and the commits involved; a human merges after checking the Helm values. Diffing against the deployed tag (recorded in the values file) rather than the push range keeps the hold sticky: a held config change cannot ship via a later unrelated commit. Unresolvable deployed tags fail safe to manual review. Job re-runs no longer re-enable auto-merge unconditionally.
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #363 +/- ##
=======================================
Coverage ? 32.02%
=======================================
Files ? 154
Lines ? 12250
Branches ? 0
=======================================
Hits ? 3923
Misses ? 8011
Partials ? 316
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Review follow-ups on the devnet config gate: - Move detection logic to .github/scripts/detect-config-changes.sh; the workflow step is now a one-line invocation. DIR is defined once at job level. - Read the deployed tag from all three service values files; per-service drift, a missing file/key, or a malformed tag all fail safe to a hold instead of diffing a wrong baseline or crashing the job. - Include pkg/indexer/types.go in the gated paths (yaml-tagged InstrumentKey schema embedded in indexer config). - Explicitly disable auto-merge on a hold: auto-merge armed by an earlier run no longer survives a later config-changed verdict. - Restructure the deploy-PR step so re-runs self-repair: the idempotent branch check only skips the commit; PR existence, hold label, and report comment are ensured on every run. PR-create and hold-comment failures are fatal instead of silently swallowed. - Report hardening: key scan uses the same pathspecs as the gate and allows - and . in key names; commits list capped at 100; oversized reports drop the inline diff to stay under the comment size limit; deployed tags are sanitized before interpolation; four-backtick fence so diff content cannot break out.
InstrumentKey is part of the deployed YAML config schema (Config.Instruments) but lived in types.go, forcing the CI config gate to carry a hardcoded exception for that file. Moving it to config.go restores the repo convention that yaml-tagged config structs live in config.go files, and the gate's pattern list drops the exception.
dhyaniarun1993
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Every merge to main auto-deploys to devnet: the workflow opens a PR on infra-kubernetes bumping the image tags and enables auto-merge. When a merge changes config (schema structs or the default YAMLs baked into the images), the new image can land on devnet before the Helm values are updated to match, breaking the deployment.
Change
A new
.github/scripts/detect-config-changes.sh(invoked byopen-devnet-pr) diffs config paths between the version devnet currently runs and the commit being deployed:pkg/config/(schema, loaders, embedded default YAMLs)pkg/*config.go,cmd/*config*.go(per-package config structs)config.gofiles (InstrumentKeymoved frompkg/indexer/types.gotopkg/indexer/config.goin this PR to close the one exception)*_test.goandpkg/config/tests/fixturesThe deployed baseline is read from all three service values files in infra-kubernetes; they must agree and parse as
main-<sha>.No config change → PR + auto-merge, unchanged behavior.
Config changed → the deploy PR is opened without auto-merge (and auto-merge armed by any earlier run is explicitly disabled), labeled
config-change, with a comment containing:yaml:"..."struct tags, best-effort headline)Merging that PR is the manual trigger — no separate workflow needed. Once merged, the recorded tag advances and the gate clears itself.
Why diff against the deployed tag instead of the push range
A held config change must stay held: if commit A (config) is blocked but unrelated commit B merges next, B's image contains A's change. Diffing against the deployed baseline keeps the hold sticky until a human ships it.
Fail-safe behavior
Every unresolvable state holds instead of auto-merging: missing values file or key, per-service tag drift (e.g. manual rollback of one service), non-
main-<sha>tag, sha not in history. Tags read from the infra repo are sanitized before being interpolated into the report.Reliability
gh pr createand hold-comment failures are fatal (red job) instead of silently swallowed — a missing deploy PR or an unexplained held PR can't go unnoticed.Testing
Dry-ran the script locally against real repo history (yq shimmed): baseline before #313 (admin whitelist API) holds and reports
api_keyas an added key with the full diff; clean baseline auto-merges; drift, missing-file,latest-tag, unknown-sha, and markdown-injection tags all hold with the stated reason. Workflow YAML parses; scripts passbash -n.Rollout note
If the infra values files currently hold non-
main-*tags (or disagree), the first deploy PR after this merges will be held (by design). Merging it seeds the baseline; subsequent runs resolve normally.Mainnet promotion is deliberately out of scope (discussed separately: draft promotion PR per release, never auto-merged).