Skip to content

test(chart): lock fake PV host access contract#7

Open
weicao wants to merge 2 commits into
fix/restore-populate-complete-contractfrom
derek/task84-b98-chart-contract
Open

test(chart): lock fake PV host access contract#7
weicao wants to merge 2 commits into
fix/restore-populate-complete-contractfrom
derek/task84-b98-chart-contract

Conversation

@weicao

@weicao weicao commented Jul 13, 2026

Copy link
Copy Markdown

Problem

The b98 syncer image was deployed with an older 0.34.0 chart. In fake-PV mode (persistentVolumeClaims.enabled=true, persistentVolumes.enabled=false), the custom external-populator handoff reads and patches host PV claimRefs, but the old rendered ClusterRole only granted get/list.

Commit 1b97603a7 already added the matching chart rule and auto-ClusterRole condition, but it did not update or add chart tests. The pre-existing test suite therefore had 20 stale exact rule counts and a stale assertion that disabling snapshot rules produces no ClusterRole.

Contract

Fake-PV external-populator handoff requires:

  • direct client calls: host PV get and patch
  • controller-runtime cached client bootstrap: host PV list and watch
  • no create/delete/update, which remain specific to full PV sync

This is expected fake-PV external-populator behavior, not an accidentally enabled full PV syncer. The runtime still registers the fake-PV syncer; only the PVC handoff accesses the host PV.

Declarative values:

sync:
  toHost:
    persistentVolumeClaims:
      enabled: true
    persistentVolumes:
      enabled: false
rbac:
  clusterRole:
    enabled: auto

The matching chart renders a persistentvolumes rule with exactly get/list/watch/patch for this path. Volume-snapshot RBAC remains independently controlled by rbac.enableVolumeSnapshotRules.

Changes

  • Add a focused source-side recording client test proving the fake-PV handoff performs host PV Get and Patch after the current terminal-populator gate is satisfied.
  • Replace the stale snapshot-only assertion with a fake-PV/PV-sync/disabled matrix.
  • Update exact rule counts affected by the new default fake-PV rule.

Source identity

  • Runtime b98 source/image commit: b98b1ae5a603fa504daf1e745d65784c5a0be0ea
  • Current PR fix(pvc): complete restore populate handoff #4 base head: 4b4091829b8f992b37e96356535c375441246ddf
  • This test PR head: d7255ec022952ab13a874ff928d2e32e6cfb50e2
  • chart/templates tree is identical at b98, current PR fix(pvc): complete restore populate handoff #4, and this PR: c87525f495ae20c351af3623669b7c92791503bb
  • chart/values.yaml blob is identical at all three: e03596b1ebf459bcfc3af075c3559f5b35aea45a

Verification

  • helm unittest chart: 217/217 PASS
  • helm lint chart: PASS
  • go test -count=1 ./pkg/controllers/resources/...: PASS
  • git diff --check: PASS
  • focused render with the values above yields persistentvolumes: [get,list,watch,patch]

This PR is stacked on the current PR #4 head and contains tests only. It does not authorize a live rollout; fresh-vcluster identity/readback remains a separate gate.

@weicao weicao force-pushed the derek/task84-b98-chart-contract branch from 7141a71 to d7255ec Compare July 13, 2026 18:07
@weicao

weicao commented Jul 13, 2026

Copy link
Copy Markdown
Author

Focused review: no code blocker on exact head d7255ec, stacked on exact PR #4 head 4b40918.\n\nVerified independently:\n- PR #7 is one test-only commit (2 files, +168/-21); parent equals the stated PR #4 head.\n- b98, PR #4, and PR #7 share chart/templates tree c87525f495ae20c351af3623669b7c92791503bb and chart/values.yaml blob e03596b1ebf459bcfc3af075c3559f5b35aea45a.\n- Source handoff performs host-PV Get then Patch; SyncController supplies HostManager.GetClient(), whose cache bootstrap accounts for List/Watch. The V13 runtime packet independently observed the missing Watch/Patch permissions.\n- Focused Helm render for PVC=true, PV=false, snapshots=false, clusterRole=auto produces exactly persistentvolumes [get,list,watch,patch]. PVC/PV/snapshot all disabled produces no ClusterRole; full PV sync retains lifecycle verbs.\n- Base RED: helm unittest on 4b40918 reports 21 failures / 194 pass from the stale assertion and 20 stale counts. Head GREEN: 217/217.\n- go test -count=1 ./pkg/controllers/resources/... PASS; focused recorder PASS; go test -race -count=1 ./pkg/controllers/resources/persistentvolumeclaims PASS; helm lint PASS; git diff --check PASS.\n\nScope boundary: this PR locks an already-present production chart contract; it does not repair an old deployed 0.34.0 chart by itself. Fresh validation must package the reviewed chart source with b98 exact image/values, prove rendered/live RBAC+config+image identity, then run a fresh create-only gate. No V13 reuse.\n\nGitHub would not accept an APPROVE review because the authenticated account is the PR author, so this is recorded as a focused-review comment; an independent account is still required if branch protection requires formal approval.

@weicao

weicao commented Jul 14, 2026

Copy link
Copy Markdown
Author

Fresh focused review: NO BLOCKER on the new exact head 7a8ab3968263801d33462c7a3fc069bcbf47c9c6 (parent d7255ec022952ab13a874ff928d2e32e6cfb50e2, base remains 4b4091829b8f992b37e96356535c375441246ddf).

Independent checks:

  • Remote PR head readback matches 7a8ab3968263801d33462c7a3fc069bcbf47c9c6; PR is OPEN/CLEAN/MERGEABLE.
  • Increment from the previously reviewed head is exactly one test file: chart/tests/statefulset_test.yaml, +9/-3. chart/templates tree remains c87525f495ae20c351af3623669b7c92791503bb; chart/values.yaml remains e03596b1ebf459bcfc3af075c3559f5b35aea45a. No production delta.
  • Source: helm lint PASS and Helm unittest 217/217 PASS.
  • New package vcluster-0.35.1-b98b1ae5a-7a8ab3968.tgz: independently downloaded, 72,948 bytes, SHA256 e4d8b86a67ec28e1a459270756d2f2fcc15ad02846bedaf186859879f95a7802; safe archive paths, no links. Package contents match exact source except normalized Chart.yaml metadata/version 0.35.1-b98b1ae5a-7a8ab3968.
  • Package: helm lint PASS and Helm unittest 217/217 PASS.
  • Explicit offline render with b98 image, pull Never, PVC sync=true, PV sync=false, ClusterRole=auto, snapshot rules=false produces image docker.io/local/vcluster-kb12-restore:b98b1ae5a-amd64, decoded config PVC=true/PV=false, and exactly one host-PV rule with get/list/watch/patch. No PV create/delete/update and no snapshot rule is introduced.

The prior d7255ec0 review/package is historical only and is not inherited as the new runtime credential. This verdict authorizes only the next fresh live identity/RBAC/config/readyz gate on the sealed 7a8ab3968 + e4d8b86a...7802 candidate; it is not runtime PASS, full-suite PASS, or release-ready. No V13 reuse or direct live RBAC patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant