fix(repositories): pin hasDownloads so reconciliation stops 422-ing - #125
Merged
devantler merged 1 commit intoJul 27, 2026
Merged
Conversation
GitHub removed the downloads feature and no longer returns has_downloads, but the Terraform provider still defaults it to true. Repos that adopted that default through LateInitialize therefore hold a spec value the API can never report back, producing an unresolvable diff that makes the provider issue an update PATCH on every reconcile. Those PATCHes are rejected wholesale with 422 on the commit-signoff field, so no declared setting reaches GitHub at all. Pinning the field to the value the API actually reflects removes the diff. Measured on the live cluster: the correlation is exact across all 20 managed Repository resources - every resource with hasDownloads true in spec and absent in atProvider is Synced=False (17/17), and every resource where the two agree is Synced=True (3/3). Scope is deploy/repositories/ only. reusable-workflows lives in archived-repositories/ and deliberately gets no shared patch; its live value is true and already matches, so pinning it false would create the very diff this change removes. Signed-off-by: Nikolai Emil Damm <nikolaiemildamm@icloud.com>
Closed
4 tasks
Contributor
Author
CI is green at this head and the change is self-reviewed. Requesting a current-head review. @coderabbitai review |
|
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughUpdated the repository Kustomize configuration with a comment and JSON6902 patch that pins 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
Contributor
Author
Readiness evidence at
Programmatically tested. The repository's three required commands all pass: Exercised — what I actually observed, and its limit. This is configuration consumed by Flux and Crossplane in-cluster, so it has no locally-runnable surface. I verified the rendered artifact rather than reasoning about it: the build emits 20 I also confirmed on the live cluster that the field this change stops sending is genuinely the discriminator: agreement on it predicts sync state across all 20 resources with no exceptions, and no other field separates the healthy from the failing. What is NOT yet proven, stated plainly: that reconciliation actually recovers. The mirror these resources publish is demonstrably incomplete — one resource reports a field mismatch while sync succeeds — so the provider's internal comparison cannot be observed from outside. The correlation is exact and this is the best-supported explanation, but the mechanism is inferred. That is acceptable to merge because the failure mode is a no-op, not a regression: every update attempt is already rejected in full, so this cannot make any repository worse, and it is one line to revert. Note also that Verification is therefore post-merge and I will report it on #123: the failing resources returning to a healthy sync state, checked against the GitHub API rather than the mirror. |
devantler
marked this pull request as ready for review
July 27, 2026 04:32
4 tasks
This was referenced Jul 27, 2026
Closed
devantler
added a commit
that referenced
this pull request
Aug 6, 2026
Seven of the twenty Repository resources have been failing every update with 422 "Commit signoff is enforced by the organization and cannot be disabled", so declared repository configuration has not reached GitHub for them. The accepted explanation was that the field is rejected whenever it appears in an update, so #128 removed the declaration entirely. The live cluster shows the opposite. upjet builds the Terraform configuration from forProvider; an absent optional bool takes the provider's zero value of false; false against a live true is a permanent diff, so the payload carries web_commit_signoff_required: false and GitHub rejects it. The error names disabling, not presence. Declaring the live value leaves nothing to diff, so Terraform omits the field from the payload and the update applies. Measured, at two-minute resolution: 2026-07-27T04:33Z #125 merges, shared patch still declares the field 2026-07-27T04:34:5x nine write-enabled repos record LastAsyncOperation=Success 2026-07-27T14:42Z #128 removes the declaration 2026-07-27T14:44:4x the same repos begin recording AsyncUpdateFailure (422) Ten repositories still reconcile today only because the provider had already late-initialized the value into their spec, where Flux's removal could not reach it. The seven that never accumulated that residue are exactly the seven failing, and six of them differ from a working repository by this one field. So the write path currently depends on undeclared residue rather than on anything in this repository. platform-tenant-template is the one active resource holding no adopted values, so it cannot fall back on a previously observed visibility once updates apply again; its live value is pinned explicitly. tests/repository-update-policy.sh now pins the inverted invariant. Fixes #112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 tasks
devantler
added a commit
that referenced
this pull request
Aug 6, 2026
…136) * chore: claim #123 * fix(repositories): declare org-enforced signoff so updates can apply Seven of the twenty Repository resources have been failing every update with 422 "Commit signoff is enforced by the organization and cannot be disabled", so declared repository configuration has not reached GitHub for them. The accepted explanation was that the field is rejected whenever it appears in an update, so #128 removed the declaration entirely. The live cluster shows the opposite. upjet builds the Terraform configuration from forProvider; an absent optional bool takes the provider's zero value of false; false against a live true is a permanent diff, so the payload carries web_commit_signoff_required: false and GitHub rejects it. The error names disabling, not presence. Declaring the live value leaves nothing to diff, so Terraform omits the field from the payload and the update applies. Measured, at two-minute resolution: 2026-07-27T04:33Z #125 merges, shared patch still declares the field 2026-07-27T04:34:5x nine write-enabled repos record LastAsyncOperation=Success 2026-07-27T14:42Z #128 removes the declaration 2026-07-27T14:44:4x the same repos begin recording AsyncUpdateFailure (422) Ten repositories still reconcile today only because the provider had already late-initialized the value into their spec, where Flux's removal could not reach it. The seven that never accumulated that residue are exactly the seven failing, and six of them differ from a working repository by this one field. So the write path currently depends on undeclared residue rather than on anything in this repository. platform-tenant-template is the one active resource holding no adopted values, so it cannot fall back on a previously observed visibility once updates apply again; its live value is pinned explicitly. tests/repository-update-policy.sh now pins the inverted invariant. Fixes #112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(repositories): point signoff rationale at the tracking issue * docs(agents): bound LateInitialize residue to the adoption phase CodeRabbit: the previous wording let 'late-initialized values land in forProvider' read as an ongoing behaviour. Once LateInitialize is removed no newly observed field is copied in again, which is precisely why a resource adopted without a field can never acquire it and the config must supply it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 tasks
devantler
added a commit
that referenced
this pull request
Aug 6, 2026
…epos (#138) * chore: claim #123 * fix(repositories): declare org-enforced signoff so updates can apply Seven of the twenty Repository resources have been failing every update with 422 "Commit signoff is enforced by the organization and cannot be disabled", so declared repository configuration has not reached GitHub for them. The accepted explanation was that the field is rejected whenever it appears in an update, so #128 removed the declaration entirely. The live cluster shows the opposite. upjet builds the Terraform configuration from forProvider; an absent optional bool takes the provider's zero value of false; false against a live true is a permanent diff, so the payload carries web_commit_signoff_required: false and GitHub rejects it. The error names disabling, not presence. Declaring the live value leaves nothing to diff, so Terraform omits the field from the payload and the update applies. Measured, at two-minute resolution: 2026-07-27T04:33Z #125 merges, shared patch still declares the field 2026-07-27T04:34:5x nine write-enabled repos record LastAsyncOperation=Success 2026-07-27T14:42Z #128 removes the declaration 2026-07-27T14:44:4x the same repos begin recording AsyncUpdateFailure (422) Ten repositories still reconcile today only because the provider had already late-initialized the value into their spec, where Flux's removal could not reach it. The seven that never accumulated that residue are exactly the seven failing, and six of them differ from a working repository by this one field. So the write path currently depends on undeclared residue rather than on anything in this repository. platform-tenant-template is the one active resource holding no adopted values, so it cannot fall back on a previously observed visibility once updates apply again; its live value is pinned explicitly. tests/repository-update-policy.sh now pins the inverted invariant. Fixes #112 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(repositories): point signoff rationale at the tracking issue * docs(agents): bound LateInitialize residue to the adoption phase CodeRabbit: the previous wording let 'late-initialized values land in forProvider' read as an ongoing behaviour. Once LateInitialize is removed no newly observed field is copied in again, which is precisely why a resource adopted without a field can never acquire it and the config must supply it. * fix(repositories): correct stale private declarations before restoring writes ascoachingogvaner and wedding-app are public and serve live sites, but both declared visibility: private — carried in from the template they were written against, never true of either repository. While updates were write-blocked the divergence was inert; restoring the write path is what would make it act, so the declaration is corrected first and the two stay Observe-only until promoted separately. The kustomization grouped them under '# Private repos.', which described neither their visibility nor their management policy. fleet-gitops, which is genuinely private and fully managed, moves in with the actively-managed set. Part of #123 * docs(repositories): state the current management rationale, not the migration The two observe-only declarations described the change this PR makes rather than the state it leaves behind. Both now say why write management is still disabled and where enabling it is tracked. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 7, 2026
Open
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
1 participant
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.
Why
Declarative GitHub org management has not been applying anything. 17 of the 20 managed repositories are stuck in a failing sync state, and every one of them simultaneously reports itself healthy — which is why this ran for weeks without surfacing. While it lasts, no declared setting reaches GitHub for those repositories, including the visibility pins that two repositories' comments describe as the thing stopping management from ever exposing them. That guarantee has not existed.
The cause is not the commit-signoff setting the error message names. That one is already declared correctly and has been since June, so the obvious fix would have changed nothing. The real trigger is a setting GitHub has retired: it is still declared as on, GitHub no longer reports it at all, and the two can therefore never agree. Reconciliation keeps trying to correct it, and each attempt is rejected in full — taking every other declared setting down with it.
What
Declares that retired setting as off, matching what GitHub actually reflects, so the mismatch disappears.
Evidence: across all 20 managed repositories, agreement on this one setting predicts sync state exactly — all 17 that disagree are failing, all 3 that agree are healthy. No other setting divides the two groups.
Confidence, stated honestly: the correlation is exact, but the mechanism is inferred rather than observed — the live mirror these resources publish is demonstrably incomplete, so it cannot be used to prove which comparison reconciliation actually acts on. This is the best-supported single explanation, and it is cheap and reversible; if it is wrong it is a no-op rather than a regression. Confirmation is that the failing repositories return to a healthy sync state after this deploys, and I will verify that and report back on the issue.
Scope is the active repository set only. The archived repository is deliberately excluded — it is the one case where this value genuinely agrees today, so pinning it there would create the very mismatch this removes.
Needs your call, not fixed here: two repositories are declared private while live public. This change cannot flip them, but it does not resolve them either — and which of the two is the mistake is a judgement about intent on two live public sites. Evidence is on the issue.
Part of #123