diff --git a/AGENTS.md b/AGENTS.md index 86233d9a..181164e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,7 +68,12 @@ for the architecture, the GitHub App credential setup, and the Observe-first ado private metadata into this repository. The `platform` and `ksail` Repository resources host workflow-built Pages sites that the GitHub App cannot write, so each declares the deprecated `pages` field exactly as GitHub reports it. Declaring the whole list replaces stale - provider state and leaves no Pages diff for an update to send (issue #232). After a repository's one-time + provider state and leaves no Pages diff for an update to send (issue #232). KHoist's newly + bootstrapped site has no observed Pages block, so an enforced declaration instead retries a + forbidden Pages PUT. Keep only its Pages bootstrap expectation in `initProvider.pages`, retain + the homepage and repository settings in `forProvider`, and verify the Pages expectation through + the read-only repository drift check (#614). Do not expand the managed App's Pages permissions. + After a repository's one-time `archived: true` update lands, move it to exactly `Observe`: GitHub makes archived repositories read-only, and retaining `Update` or `LateInitialize` lets newly exposed provider fields create a permanent update loop. `tests/repository-update-policy.sh` pins both invariants. Verify diff --git a/deploy/repositories/data-product-controller.yaml b/deploy/repositories/data-product-controller.yaml index c5fd106d..a701b4a0 100644 --- a/deploy/repositories/data-product-controller.yaml +++ b/deploy/repositories/data-product-controller.yaml @@ -5,19 +5,20 @@ metadata: annotations: crossplane.io/external-name: data-product-controller spec: - # The adopted repository is Synced/Ready. Apply the shared repository policy. + # Keep ordinary settings management on the adopted repository. managementPolicies: ["Observe", "Create", "Update"] + # Pages was bootstrapped separately. Preserve its initial declaration without + # asking the managed App to write Pages; the drift audit still verifies it. + initProvider: + pages: + - buildType: workflow + cname: khoist.devantler.tech forProvider: name: data-product-controller visibility: public archived: false description: "A cloud-native controller for composable, self-describing data products." homepageUrl: https://khoist.devantler.tech/ - # Bootstrap is a maintainer prerequisite; see khoist-pages-bootstrap.md. - # Match the workflow site's live shape exactly, without a branch source (#232). - pages: - - buildType: workflow - cname: khoist.devantler.tech # Issues are used here; an undeclared bool takes the provider default (false) on update (#311). hasIssues: true providerConfigRef: diff --git a/khoist-pages-bootstrap.md b/khoist-pages-bootstrap.md index 5877fa30..f65f7d85 100644 --- a/khoist-pages-bootstrap.md +++ b/khoist-pages-bootstrap.md @@ -27,6 +27,13 @@ The intended settings are: | Custom domain | `khoist.devantler.tech` | | Homepage | `https://khoist.devantler.tech/` | +The bootstrap expectation is declared in `initProvider.pages`. The managed App +does not enforce it after adoption; the read-only repository drift check still +compares its build type and domain with the Pages API and fails on a mismatch or +an unreadable site. The homepage and ordinary repository settings remain enforced +in `forProvider`. Existing KSail/platform Pages adoptions retain their separate +observed-state declarations from #232. + There is no branch `source` block for a workflow-built site. A checked-in `CNAME` file does not configure an Actions-built site's custom domain. @@ -35,9 +42,11 @@ Confirm `build_type: workflow` and the exact `cname`. The REST response may reta a legacy `source` branch even for workflow builds. The pinned provider's [`flattenPages` implementation](https://github.com/integrations/terraform-provider-github/blob/v6.13.0/github/resource_github_repository.go) normalizes that field to absent for workflow builds, so the declaration must omit it. -Once those settings match, the reviewed declaration preserves the provider's shape while retaining -the repository's existing management policies. Verify successful reconciliation -without Pages permission errors before treating configuration adoption as complete. +The initial forProvider declaration in #615 still retried a Pages PUT: this new +site had no observed provider Pages state to match. The bootstrap-only declaration +avoids taking ongoing Pages write ownership. Verify successful repository +reconciliation and the independent Pages drift check before treating adoption as +complete; a matching manifest alone does not prove either result. ## Publication ownership diff --git a/scripts/check-repository-drift.sh b/scripts/check-repository-drift.sh index ffe6cb45..c9aabd71 100755 --- a/scripts/check-repository-drift.sh +++ b/scripts/check-repository-drift.sh @@ -67,9 +67,16 @@ yq -N -o=json -I=0 ' { "repo": (.metadata.annotations["crossplane.io/external-name"] // .metadata.name), "resource": .metadata.name, - "declared": .spec.forProvider + "declared": .spec.forProvider, + "bootstrapPages": .spec.initProvider.pages } -' "$render" >"$work/declared.jsonl" || abort "failed to read Repository resources from the render" +' "$render" | jq -c ' + if .bootstrapPages != null then + if .declared | has("pages") then + error("Pages must have one declaration owner") + else .declared.pages = .bootstrapPages end + else . end | del(.bootstrapPages) +' >"$work/declared.jsonl" || abort "failed to read Repository resources from the render" declared_count="$(grep -c . "$work/declared.jsonl" || true)" # deploy/ manages the whole org; a render that yields almost nothing means the @@ -91,7 +98,8 @@ readonly KEY_PROGRAM=' readonly SETTINGS_KEYS='["allow_auto_merge", "allow_squash_merge", "allow_merge_commit", "allow_rebase_merge", "allow_update_branch", "delete_branch_on_merge", "web_commit_signoff_required"]' -readonly SETTINGS_QUERY='query RepositorySettings($owner: String!, $name: String!) { +SETTINGS_QUERY=$(cat <<'GRAPHQL' +query RepositorySettings($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { id nameWithOwner isPrivate allow_auto_merge: autoMergeAllowed @@ -102,7 +110,10 @@ readonly SETTINGS_QUERY='query RepositorySettings($owner: String!, $name: String delete_branch_on_merge: deleteBranchOnMerge web_commit_signoff_required: webCommitSignoffRequired } -}' +} +GRAPHQL +) +readonly SETTINGS_QUERY complete_repository_settings() { local entry="$1" live="$2" repo="$3" declared_settings missing settings diff --git a/tests/repository-drift.sh b/tests/repository-drift.sh index d802e324..b96bc385 100644 --- a/tests/repository-drift.sh +++ b/tests/repository-drift.sh @@ -263,6 +263,31 @@ expect_status "$work/pages-unmapped" 2 "a declared Pages key this check cannot c grep -Fq 'pages.source' "$work/pages-unmapped/stderr" || fail "the uncomparable Pages key must be named" +# Bootstrap-only Pages settings are still audited, although Crossplane must not +# attempt to enforce them through an App that cannot write Pages. +seed_bootstrap_pages() { + DECLARED=$' initProvider:\n pages:\n - buildType: workflow\n cname: pages.example.invalid\n' \ + perl -0pi -e 's/^(spec:\n)/$1$ENV{DECLARED}/m or die "no fixture spec\n"' "$1/render.yaml" +} +bootstrap_pages_fixture() { + local dir="$1" live_pages="$2" + pages_fixture "$dir" "" "$live_pages" + seed_bootstrap_pages "$dir" +} +bootstrap_pages_fixture "$work/bootstrap-pages" "$live_cname" +expect_status "$work/bootstrap-pages" 0 "matching bootstrap-only Pages settings" +bootstrap_pages_fixture "$work/bootstrap-pages-drift" '{"build_type":"workflow","cname":"other.example.invalid"}' +expect_status "$work/bootstrap-pages-drift" 1 "a changed bootstrap-only Pages domain" +grep -Fq 'DRIFT fixture-repo-1.pages' "$work/bootstrap-pages-drift/stdout" || + fail "bootstrap-only Pages drift must still name the pages field" +bootstrap_pages_fixture "$work/bootstrap-pages-missing" "" +expect_status "$work/bootstrap-pages-missing" 2 "an unreadable bootstrap-only Pages site" + +# Two owners for Pages are ambiguous; do not silently hide either declaration. +pages_fixture "$work/pages-two-owners" "$cname_pages" "$live_cname" +seed_bootstrap_pages "$work/pages-two-owners" +expect_status "$work/pages-two-owners" 2 "both managed and bootstrap-only Pages settings" + # Exercise the real gh transport path, including the REST omissions observed # with an installation token. The fake accepts only the expected read requests. transport="$work/transport" diff --git a/tests/repository-update-policy.sh b/tests/repository-update-policy.sh index 2b809b70..2fbfb14d 100755 --- a/tests/repository-update-policy.sh +++ b/tests/repository-update-policy.sh @@ -71,18 +71,23 @@ active_count="$(printf '%s\n' "$active_repositories" | grep -c . || true)" # With `pages` matching GitHub exactly, an update carries no Pages change, so both # are managed with Observe/Create/Update like every other active repository. declare_pages() { - local name="$1" want_cname="$2" want_homepage="${3:-}" got + local name="$1" want_cname="$2" want_homepage="${3:-}" owner="${4:-forProvider}" got got="$( yq -N -o=json -I=0 " select(.kind == \"Repository\" and .metadata.name == \"$name\") | { \"policies\": (.spec.managementPolicies | sort | join(\",\")), - \"pages\": .spec.forProvider.pages, + \"pages\": .spec.$owner.pages, + \"managedPages\": (.spec.forProvider | has(\"pages\")), \"homepage\": .spec.forProvider.homepageUrl } " "$render" )" [[ -n "$got" ]] || fail "$name Repository resource is missing" + if [[ "$owner" == initProvider ]]; then + [[ "$(yq -N '.managedPages' <<<"$got")" == "false" ]] || + fail "$name bootstrap-only Pages must not be enforced through forProvider: $got" + fi if [[ -n "$want_homepage" ]]; then [[ "$(yq -N '.homepage' <<<"$got")" == "$want_homepage" ]] || fail "$name must declare homepageUrl: $want_homepage, as GitHub reports it: $got" @@ -90,7 +95,7 @@ declare_pages() { [[ "$(yq -N '.policies' <<<"$got")" == "Create,Observe,Update" ]] || fail "$name must be managed with Observe/Create/Update now that its Pages state is declared: $got" [[ "$(yq -N '.pages | length' <<<"$got")" == "1" ]] || - fail "$name must declare exactly one forProvider.pages entry: $got" + fail "$name must declare exactly one $owner.pages entry: $got" [[ "$(yq -N '.pages[0].buildType' <<<"$got")" == "workflow" ]] || fail "$name Pages must declare buildType: workflow, as GitHub reports it: $got" [[ "$(yq -N '.pages[0] | has("source")' <<<"$got")" == "false" ]] || @@ -105,7 +110,9 @@ declare_pages() { } declare_pages ksail ksail.devantler.tech declare_pages platform "" -declare_pages data-product-controller khoist.devantler.tech https://khoist.devantler.tech/ +# A first Pages site has no observed provider block to adopt (#614). Enforcing +# it would retry an unsupported Pages PUT. Keep its seed explicit and audit it. +declare_pages data-product-controller khoist.devantler.tech https://khoist.devantler.tech/ initProvider # LateInitialize copies live-only values into forProvider, and everything in # forProvider is sent on update PATCHes, so LateInitialize is what turns a