Skip to content

Commit 4a06df9

Browse files
scooby87claude
scooby87
and
claude
committed
fix(etcd): scope cleanup Role to the data-etcd PVC names and assert hook metadata
Address CodeRabbit review on #3170: - Tighten the cleanup Role's PVC rule from a namespace-wide get/list/delete to get/delete by explicit resourceNames (data-etcd-<ordinal>, one per replica), mirroring the least-privilege pattern already used for the TLS secrets. The Job only ever deletes those deterministic names, so list is unnecessary and a regression can no longer reach a sibling workload's PVC. - Extend the Role and RoleBinding unit tests to assert helm.sh/hook and helm.sh/hook-delete-policy (not just the weight), so the RBAC cannot regress into normal release resources, and assert the PVC resourceNames. Assisted-By: Claude <[email protected]> Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Signed-off-by: Алексей Артамонов <[email protected]>
1 parent a92f060 commit 4a06df9

2 files changed

Lines changed: 27 additions & 2 deletions

File tree

‎packages/extra/etcd/templates/hooks/cleanup.yaml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,16 @@ metadata:
2121
"helm.sh/hook-weight": "5"
2222
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
2323
rules:
24+
# The Job deletes the data PVCs by their deterministic per-replica names
25+
# (data-etcd-<ordinal>), so we grant delete by explicit resourceNames rather
26+
# than a namespace-wide PVC delete that could hit a sibling workload's PVC.
2427
- apiGroups: [""]
2528
resources: ["persistentvolumeclaims"]
26-
verbs: ["get", "list", "delete"]
29+
verbs: ["get", "delete"]
30+
resourceNames:
31+
{{- range $i := until (int .Values.replicas) }}
32+
- data-etcd-{{ $i }}
33+
{{- end }}
2734
# cert-manager TLS secrets carry only the cluster-wide
2835
# controller.cert-manager.io/fao label, and the two pre-install "keep"
2936
# secrets carry none, so we grant delete by explicit resourceNames rather

‎packages/extra/etcd/tests/cleanup_test.yaml‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,27 @@ tests:
4343
path: kind
4444
value: Role
4545
asserts:
46+
- equal:
47+
path: metadata.annotations["helm.sh/hook"]
48+
value: post-delete
4649
- equal:
4750
path: metadata.annotations["helm.sh/hook-weight"]
4851
value: "5"
52+
- equal:
53+
path: metadata.annotations["helm.sh/hook-delete-policy"]
54+
value: before-hook-creation,hook-succeeded
55+
# PVC deletes are scoped to the deterministic per-replica names, not a
56+
# namespace-wide delete, so a regression cannot reach a sibling's PVC.
4957
- contains:
5058
path: rules
5159
content:
5260
apiGroups: [""]
5361
resources: ["persistentvolumeclaims"]
54-
verbs: ["get", "list", "delete"]
62+
verbs: ["get", "delete"]
63+
resourceNames:
64+
- data-etcd-0
65+
- data-etcd-1
66+
- data-etcd-2
5567
- contains:
5668
path: rules
5769
content:
@@ -70,9 +82,15 @@ tests:
7082
path: kind
7183
value: RoleBinding
7284
asserts:
85+
- equal:
86+
path: metadata.annotations["helm.sh/hook"]
87+
value: post-delete
7388
- equal:
7489
path: metadata.annotations["helm.sh/hook-weight"]
7590
value: "5"
91+
- equal:
92+
path: metadata.annotations["helm.sh/hook-delete-policy"]
93+
value: before-hook-creation,hook-succeeded
7694
- equal:
7795
path: roleRef.name
7896
value: etcd-cleanup

0 commit comments

Comments
 (0)