Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions deploy/repositories/data-product-controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
15 changes: 12 additions & 3 deletions khoist-pages-bootstrap.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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

Expand Down
19 changes: 15 additions & 4 deletions scripts/check-repository-drift.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
25 changes: 25 additions & 0 deletions tests/repository-drift.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
15 changes: 11 additions & 4 deletions tests/repository-update-policy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -71,26 +71,31 @@ 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"
fi
[[ "$(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" ]] ||
Expand All @@ -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
Expand Down
Loading