Skip to content

Commit bacbe55

Browse files
committed
fix(cozy-lib): delete the Secrets cert-manager writes when an app is deleted
cert-manager runs without --enable-certificate-owner-ref, so the Secret a Certificate writes has no owner and outlives the release. qdrant, redis, rabbitmq, nats and opensearch mint their own CA and leaf certificate and had no cleanup for them, so deleting one of these apps left Secrets holding tls.key in the tenant namespace, and a later app of the same name could pick up a key it did not create. The cleanup is one post-delete hook in cozy-lib that an app feeds with the Secret names its Certificates can write, so every app gets the same least-privilege delete by name and the same non-zero exit on a failed delete instead of a hand-written copy. Turning the cert-manager flag on would change the same behaviour for every Certificate the platform itself issues, including the CA Certificates its own components trust. Assisted-by: LLM Signed-off-by: Yan Bondarenko <[email protected]>
1 parent 9a348ee commit bacbe55

12 files changed

Lines changed: 715 additions & 0 deletions

File tree

‎docs/agents/overview.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ A chart also declares the limits on its own resource name there, with `## @name
117117
- Include upstream charts in `charts/` directory (vendored, not referenced)
118118
- Override configuration in root `values.yaml`
119119
- Use `values.schema.json` for input validation and dashboard UI rendering
120+
- An app that issues Certificates through cert-manager removes the Secrets they write when the release is deleted, with the `cozy-lib.hooks.deleteSecrets` post-delete hook listing every `secretName` the chart can render. cert-manager runs without `--enable-certificate-owner-ref`, so nothing else reclaims those Secrets or the keys in them
120121

121122
### Go Code
122123
- Follow standard **Go conventions** and idioms
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{{- /* Secrets cert-manager writes for the Certificates in certmanager.yaml. */}}
2+
{{- include "cozy-lib.hooks.deleteSecrets" (dict
3+
"root" .
4+
"secrets" (list
5+
(printf "%s-ca" .Release.Name)
6+
(printf "%s-tls" .Release.Name))) }}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
suite: nats removes the Secrets cert-manager wrote once the release is deleted
2+
3+
release:
4+
name: nats-test
5+
namespace: tenant-test
6+
7+
set:
8+
_cluster:
9+
cluster-domain: cozy.local
10+
11+
tests:
12+
- it: grants delete on exactly the Secrets its Certificates write
13+
templates:
14+
- templates/hooks/cleanup-secrets.yaml
15+
documentSelector:
16+
path: kind
17+
value: Role
18+
asserts:
19+
- equal:
20+
path: rules[0].resourceNames
21+
value:
22+
- nats-test-ca
23+
- nats-test-tls
24+
25+
- it: deletes them from a post-delete Job that fails when a delete fails
26+
templates:
27+
- templates/hooks/cleanup-secrets.yaml
28+
documentSelector:
29+
path: kind
30+
value: Job
31+
asserts:
32+
- equal:
33+
path: metadata.annotations["helm.sh/hook"]
34+
value: post-delete
35+
- equal:
36+
path: metadata.annotations["helm.sh/hook-delete-policy"]
37+
value: before-hook-creation,hook-succeeded
38+
- equal:
39+
path: spec.template.spec.serviceAccountName
40+
value: nats-test-secrets-cleanup
41+
- matchRegex:
42+
path: spec.template.spec.containers[0].command[2]
43+
pattern: 'kubectl delete secret --namespace tenant-test --ignore-not-found "nats-test-ca" "nats-test-tls"; then'
44+
- matchRegex:
45+
path: spec.template.spec.containers[0].command[2]
46+
pattern: 'exit 1'
47+
- matchRegex:
48+
path: spec.template.spec.containers[0].image
49+
pattern: '^docker\.io/clastix/kubectl:v[0-9]+\.[0-9]+(\.[0-9]+)?@sha256:[0-9a-f]{64}$'
50+
51+
- it: still renders when the release no longer issues certificates, since an earlier revision may have
52+
templates:
53+
- templates/hooks/cleanup-secrets.yaml
54+
set:
55+
tls:
56+
enabled: false
57+
asserts:
58+
- hasDocuments:
59+
count: 4
60+
61+
- it: lists the Secret of Certificate 1 of 2 among those it deletes
62+
templates:
63+
- templates/certmanager.yaml
64+
set:
65+
tls:
66+
enabled: true
67+
documentSelector:
68+
path: spec.secretName
69+
value: nats-test-ca
70+
asserts:
71+
- isKind:
72+
of: Certificate
73+
74+
- it: lists the Secret of Certificate 2 of 2 among those it deletes
75+
templates:
76+
- templates/certmanager.yaml
77+
set:
78+
tls:
79+
enabled: true
80+
documentSelector:
81+
path: spec.secretName
82+
value: nats-test-tls
83+
asserts:
84+
- isKind:
85+
of: Certificate
86+
87+
- it: issues no Certificate beyond the 2 the cleanup covers
88+
templates:
89+
- templates/certmanager.yaml
90+
set:
91+
tls:
92+
enabled: true
93+
documentSelector:
94+
path: kind
95+
value: Certificate
96+
matchMany: true
97+
asserts:
98+
- hasDocuments:
99+
count: 2
100+
filterAware: true
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{{- /*
2+
Secrets cert-manager writes for the Certificates in certmanager.yaml. The
3+
Dashboards one is listed even when dashboards.enabled is false, since it may
4+
have been true earlier in the life of the release.
5+
*/}}
6+
{{- include "cozy-lib.hooks.deleteSecrets" (dict
7+
"root" .
8+
"secrets" (list
9+
(printf "%s.http-ca" .Release.Name)
10+
(printf "%s-http-server" .Release.Name)
11+
(printf "%s-dashboards-server" .Release.Name))) }}
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
suite: opensearch removes the Secrets cert-manager wrote once the release is deleted
2+
3+
release:
4+
name: opensearch-test
5+
namespace: tenant-test
6+
7+
set:
8+
_cluster:
9+
cluster-domain: cozy.local
10+
11+
tests:
12+
- it: grants delete on exactly the Secrets its Certificates write
13+
templates:
14+
- templates/hooks/cleanup-secrets.yaml
15+
documentSelector:
16+
path: kind
17+
value: Role
18+
asserts:
19+
- equal:
20+
path: rules[0].resourceNames
21+
value:
22+
- opensearch-test.http-ca
23+
- opensearch-test-http-server
24+
- opensearch-test-dashboards-server
25+
26+
- it: deletes them from a post-delete Job that fails when a delete fails
27+
templates:
28+
- templates/hooks/cleanup-secrets.yaml
29+
documentSelector:
30+
path: kind
31+
value: Job
32+
asserts:
33+
- equal:
34+
path: metadata.annotations["helm.sh/hook"]
35+
value: post-delete
36+
- equal:
37+
path: metadata.annotations["helm.sh/hook-delete-policy"]
38+
value: before-hook-creation,hook-succeeded
39+
- equal:
40+
path: spec.template.spec.serviceAccountName
41+
value: opensearch-test-secrets-cleanup
42+
- matchRegex:
43+
path: spec.template.spec.containers[0].command[2]
44+
pattern: 'kubectl delete secret --namespace tenant-test --ignore-not-found "opensearch-test\.http-ca" "opensearch-test-http-server" "opensearch-test-dashboards-server"; then'
45+
- matchRegex:
46+
path: spec.template.spec.containers[0].command[2]
47+
pattern: 'exit 1'
48+
- matchRegex:
49+
path: spec.template.spec.containers[0].image
50+
pattern: '^docker\.io/clastix/kubectl:v[0-9]+\.[0-9]+(\.[0-9]+)?@sha256:[0-9a-f]{64}$'
51+
52+
- it: still renders when the release no longer issues certificates, since an earlier revision may have
53+
templates:
54+
- templates/hooks/cleanup-secrets.yaml
55+
set:
56+
tls:
57+
issuer: operator
58+
asserts:
59+
- hasDocuments:
60+
count: 4
61+
62+
- it: lists the Secret of Certificate 1 of 3 among those it deletes
63+
templates:
64+
- templates/certmanager.yaml
65+
set:
66+
tls:
67+
issuer: cert-manager
68+
dashboards:
69+
enabled: true
70+
documentSelector:
71+
path: spec.secretName
72+
value: opensearch-test.http-ca
73+
asserts:
74+
- isKind:
75+
of: Certificate
76+
77+
- it: lists the Secret of Certificate 2 of 3 among those it deletes
78+
templates:
79+
- templates/certmanager.yaml
80+
set:
81+
tls:
82+
issuer: cert-manager
83+
dashboards:
84+
enabled: true
85+
documentSelector:
86+
path: spec.secretName
87+
value: opensearch-test-http-server
88+
asserts:
89+
- isKind:
90+
of: Certificate
91+
92+
- it: lists the Secret of Certificate 3 of 3 among those it deletes
93+
templates:
94+
- templates/certmanager.yaml
95+
set:
96+
tls:
97+
issuer: cert-manager
98+
dashboards:
99+
enabled: true
100+
documentSelector:
101+
path: spec.secretName
102+
value: opensearch-test-dashboards-server
103+
asserts:
104+
- isKind:
105+
of: Certificate
106+
107+
- it: issues no Certificate beyond the 3 the cleanup covers
108+
templates:
109+
- templates/certmanager.yaml
110+
set:
111+
tls:
112+
issuer: cert-manager
113+
dashboards:
114+
enabled: true
115+
documentSelector:
116+
path: kind
117+
value: Certificate
118+
matchMany: true
119+
asserts:
120+
- hasDocuments:
121+
count: 3
122+
filterAware: true
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{{- /* Secrets cert-manager writes for the Certificates in certmanager.yaml. */}}
2+
{{- include "cozy-lib.hooks.deleteSecrets" (dict
3+
"root" .
4+
"secrets" (list
5+
(printf "%s-ca" .Release.Name)
6+
(printf "%s-tls" .Release.Name))) }}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
suite: qdrant removes the Secrets cert-manager wrote once the release is deleted
2+
3+
release:
4+
name: qdrant-test
5+
namespace: tenant-test
6+
7+
set:
8+
_cluster:
9+
cluster-domain: cozy.local
10+
11+
tests:
12+
- it: grants delete on exactly the Secrets its Certificates write
13+
templates:
14+
- templates/hooks/cleanup-secrets.yaml
15+
documentSelector:
16+
path: kind
17+
value: Role
18+
asserts:
19+
- equal:
20+
path: rules[0].resourceNames
21+
value:
22+
- qdrant-test-ca
23+
- qdrant-test-tls
24+
25+
- it: deletes them from a post-delete Job that fails when a delete fails
26+
templates:
27+
- templates/hooks/cleanup-secrets.yaml
28+
documentSelector:
29+
path: kind
30+
value: Job
31+
asserts:
32+
- equal:
33+
path: metadata.annotations["helm.sh/hook"]
34+
value: post-delete
35+
- equal:
36+
path: metadata.annotations["helm.sh/hook-delete-policy"]
37+
value: before-hook-creation,hook-succeeded
38+
- equal:
39+
path: spec.template.spec.serviceAccountName
40+
value: qdrant-test-secrets-cleanup
41+
- matchRegex:
42+
path: spec.template.spec.containers[0].command[2]
43+
pattern: 'kubectl delete secret --namespace tenant-test --ignore-not-found "qdrant-test-ca" "qdrant-test-tls"; then'
44+
- matchRegex:
45+
path: spec.template.spec.containers[0].command[2]
46+
pattern: 'exit 1'
47+
- matchRegex:
48+
path: spec.template.spec.containers[0].image
49+
pattern: '^docker\.io/clastix/kubectl:v[0-9]+\.[0-9]+(\.[0-9]+)?@sha256:[0-9a-f]{64}$'
50+
51+
- it: still renders when the release no longer issues certificates, since an earlier revision may have
52+
templates:
53+
- templates/hooks/cleanup-secrets.yaml
54+
set:
55+
tls:
56+
enabled: false
57+
asserts:
58+
- hasDocuments:
59+
count: 4
60+
61+
- it: lists the Secret of Certificate 1 of 2 among those it deletes
62+
templates:
63+
- templates/certmanager.yaml
64+
set:
65+
tls:
66+
enabled: true
67+
documentSelector:
68+
path: spec.secretName
69+
value: qdrant-test-ca
70+
asserts:
71+
- isKind:
72+
of: Certificate
73+
74+
- it: lists the Secret of Certificate 2 of 2 among those it deletes
75+
templates:
76+
- templates/certmanager.yaml
77+
set:
78+
tls:
79+
enabled: true
80+
documentSelector:
81+
path: spec.secretName
82+
value: qdrant-test-tls
83+
asserts:
84+
- isKind:
85+
of: Certificate
86+
87+
- it: issues no Certificate beyond the 2 the cleanup covers
88+
templates:
89+
- templates/certmanager.yaml
90+
set:
91+
tls:
92+
enabled: true
93+
documentSelector:
94+
path: kind
95+
value: Certificate
96+
matchMany: true
97+
asserts:
98+
- hasDocuments:
99+
count: 2
100+
filterAware: true
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{{- /* Secrets cert-manager writes for the Certificates in certmanager.yaml. */}}
2+
{{- include "cozy-lib.hooks.deleteSecrets" (dict
3+
"root" .
4+
"secrets" (list
5+
(printf "%s-ca" .Release.Name)
6+
(printf "%s-tls" .Release.Name))) }}

0 commit comments

Comments
 (0)