Skip to content

Commit 07c7ec4

Browse files
committed
fix: make role creation configurable
1 parent 9eb033b commit 07c7ec4

8 files changed

Lines changed: 81 additions & 5 deletions

File tree

helm/fiftyone-teams-app/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,6 +1086,7 @@ If pods show unhealthy states (e.g., `0/1`, `CrashLoopBackOff`, `Pending`):
10861086
| teamsAppSettings.volumeMounts | list | `[]` | Volume mounts for `teams-app` pods. [Reference][volumes]. |
10871087
| teamsAppSettings.volumes | list | `[]` | Volumes for `teams-app` pods. [Reference][volumes]. |
10881088
| telemetry.enabled | bool | `true` | When `false`, no telemetry resources or sidecars are rendered and the FiftyOne UI's delegated-operator log viewer will be empty. |
1089+
| telemetry.rbac.create | bool | `true` | When `false`, the telemetry `Role`/`RoleBinding` granting the sidecar `pods/log` read access are not rendered. Set `false` only when the install identity cannot create namespaced RBAC AND `serviceAccounts` points at an existing account that already carries `pods/log` (e.g. the teams-api RBAC SA). On Kubernetes the sidecar tails container logs via the `pods/log` API; without this grant those calls return `403`, so the delegated-operator log viewer stays empty. The Settings → Metrics dashboard is unaffected — metrics use the pod's shared process namespace, not the Kubernetes API. |
10891090
| telemetry.redis.containerSecurityContext | object | `{"allowPrivilegeEscalation":false,"capabilities":{"drop":["ALL"]},"readOnlyRootFilesystem":true}` | Container-level security attributes for the telemetry Redis. `readOnlyRootFilesystem` is safe because writes go to the `/data` volume. [Reference][container-security-context]. |
10901091
| telemetry.redis.external.url | string | `""` | URL of an external Redis to use instead of the bundled one (e.g. `redis://my-redis.example.com:6379`). |
10911092
| telemetry.redis.image | string | `"redis:7-alpine"` | Container image for the telemetry Redis Deployment. |

helm/fiftyone-teams-app/templates/NOTES.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,16 @@
44
apiSettings.replicaCount will be set to 1 for this deployment. Please see
55
https://helm.fiftyone.ai for details.
66
{{ end }}
7+
{{- if and .Values.telemetry.enabled (not .Values.telemetry.rbac.create) }}
8+
[WARN] Telemetry is enabled but `telemetry.rbac.create` is false, so the chart
9+
is NOT creating the Role/RoleBinding that grants the sidecar `pods/log`
10+
access. On Kubernetes the sidecar tails container logs via the pods/log
11+
API, so unless you have separately provisioned a Role/RoleBinding
12+
granting `pods/log` to the sidecar's service account (or pointed
13+
`telemetry.serviceAccounts` at an account that already has it), the
14+
delegated-operator log viewer will be empty. The Settings → Metrics
15+
dashboard is unaffected. See https://helm.fiftyone.ai for details.
16+
{{ end }}
717
{{- if and .Values.telemetry.enabled (not .Values.telemetry.redis.external.url) (not .Values.telemetry.redis.persistence.enabled) (not .Values.telemetry.redis.persistence.existingClaim) }}
818
[INFO] Telemetry Redis is running on an emptyDir volume (no PersistentVolumeClaim).
919
Long-term telemetry archives in MongoDB are unaffected, but the in-window

helm/fiftyone-teams-app/templates/telemetry-role.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{- if .Values.telemetry.enabled }}
1+
{{- if and .Values.telemetry.enabled .Values.telemetry.rbac.create }}
22
apiVersion: rbac.authorization.k8s.io/v1
33
kind: Role
44
metadata:

helm/fiftyone-teams-app/templates/telemetry-rolebinding.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{- if .Values.telemetry.enabled }}
1+
{{- if and .Values.telemetry.enabled .Values.telemetry.rbac.create }}
22
apiVersion: rbac.authorization.k8s.io/v1
33
kind: RoleBinding
44
metadata:

helm/fiftyone-teams-app/values.schema.json

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4048,6 +4048,19 @@
40484048
"title": "enabled",
40494049
"type": "boolean"
40504050
},
4051+
"rbac": {
4052+
"properties": {
4053+
"create": {
4054+
"default": true,
4055+
"description": "When `false`, the telemetry `Role`/`RoleBinding` granting the sidecar\n`pods/log` read access are not rendered. Set `false` only when the\ninstall identity cannot create namespaced RBAC AND `serviceAccounts`\npoints at an existing account that already carries `pods/log` (e.g. the\nteams-api RBAC SA). On Kubernetes the sidecar tails container logs via the\n`pods/log` API; without this grant those calls return `403`, so the\ndelegated-operator log viewer stays empty. The Settings → Metrics\ndashboard is unaffected — metrics use the pod's shared process namespace,\nnot the Kubernetes API.",
4056+
"title": "create",
4057+
"type": "boolean"
4058+
}
4059+
},
4060+
"required": [],
4061+
"title": "rbac",
4062+
"type": "object"
4063+
},
40514064
"redis": {
40524065
"properties": {
40534066
"containerSecurityContext": {

helm/fiftyone-teams-app/values.yaml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1628,6 +1628,17 @@ telemetry:
16281628
# -- When `false`, no telemetry resources or sidecars are rendered and the
16291629
# FiftyOne UI's delegated-operator log viewer will be empty.
16301630
enabled: true
1631+
rbac:
1632+
# -- When `false`, the telemetry `Role`/`RoleBinding` granting the sidecar
1633+
# `pods/log` read access are not rendered. Set `false` only when the
1634+
# install identity cannot create namespaced RBAC AND `serviceAccounts`
1635+
# points at an existing account that already carries `pods/log` (e.g. the
1636+
# teams-api RBAC SA). On Kubernetes the sidecar tails container logs via the
1637+
# `pods/log` API; without this grant those calls return `403`, so the
1638+
# delegated-operator log viewer stays empty. The Settings → Metrics
1639+
# dashboard is unaffected — metrics use the pod's shared process namespace,
1640+
# not the Kubernetes API.
1641+
create: true
16311642
redis:
16321643
# -- Container image for the telemetry Redis Deployment.
16331644
image: redis:7-alpine

tests/unit/helm/telemetry-rolebinding_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,21 @@ func (s *telemetryRoleBindingTemplateTest) TestExplicitlyDisabled() {
6161
s.ErrorContains(err, "could not find template")
6262
}
6363

64+
// With telemetry enabled but rbac.create false, the Role/RoleBinding are
65+
// skipped so an install identity without namespaced RBAC permissions can still
66+
// deploy telemetry. Metrics keep working (shared process namespace), but the
67+
// sidecar loses pods/log API access, so the log viewer stays empty unless
68+
// serviceAccounts points at an SA that already carries pods/log.
69+
func (s *telemetryRoleBindingTemplateTest) TestRbacCreateDisabled() {
70+
options := &helm.Options{SetValues: map[string]string{
71+
"telemetry.enabled": "true",
72+
"telemetry.rbac.create": "false",
73+
}}
74+
75+
_, err := helm.RenderTemplateE(s.T(), options, s.chartPath, s.releaseName, s.templates)
76+
s.ErrorContains(err, "could not find template")
77+
}
78+
6479
// extractRole finds the Role document (not RoleBinding) in multi-doc render output.
6580
func (s *telemetryRoleBindingTemplateTest) extractRole(output string) (rbacv1.Role, bool) {
6681
for _, doc := range splitYAMLDocs(output) {

tests/unit/helm/telemetry-sidecar_test.go

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,12 @@ func findSidecar(containers []corev1.Container) *corev1.Container {
5050

5151
// TestShareProcessNamespaceEnabledByDefault asserts that the api, app,
5252
// plugins, and delegated-operator deployments all opt into PID-namespace
53-
// sharing when telemetry is enabled (the default). The sidecar relies on
54-
// /proc/<pid>/fd/1 access in the target container's PID namespace, so
55-
// dropping this would silently break log capture.
53+
// sharing when telemetry is enabled (the default). The sidecar samples the
54+
// target via the shared PID namespace — psutil reads /proc/<pid> for CPU,
55+
// memory, threads, and file descriptors, and py-spy attaches for stack
56+
// archives — so dropping this would silently break metrics capture. (Log
57+
// capture is separate: on Kubernetes the sidecar tails the container's logs
58+
// via the pods/log API, which the telemetry Role grants.)
5659
func (s *telemetrySidecarTemplateTest) TestShareProcessNamespaceEnabledByDefault() {
5760
cases := []struct {
5861
template string
@@ -132,6 +135,29 @@ func (s *telemetrySidecarTemplateTest) TestShareProcessNamespaceDisabledWithTele
132135
}
133136
}
134137

138+
// TestSidecarRenderedWithRbacCreateDisabled asserts that disabling the
139+
// telemetry RBAC escape hatch (telemetry.rbac.create=false) does not affect
140+
// sidecar injection. Metrics ride on the shared process namespace, not the
141+
// pods/log Role, so an install identity without namespaced RBAC permissions
142+
// still gets the sidecar and the Settings → Metrics dashboard.
143+
func (s *telemetrySidecarTemplateTest) TestSidecarRenderedWithRbacCreateDisabled() {
144+
options := &helm.Options{SetValues: map[string]string{
145+
"telemetry.rbac.create": "false",
146+
}}
147+
output := helm.RenderTemplate(s.T(), options, s.chartPath, s.releaseName,
148+
[]string{"templates/app-deployment.yaml"})
149+
150+
var deployment appsv1.Deployment
151+
helm.UnmarshalK8SYaml(s.T(), output, &deployment)
152+
153+
s.NotNil(findSidecar(deployment.Spec.Template.Spec.Containers),
154+
"telemetry-sidecar should still be injected when telemetry.rbac.create is false")
155+
s.Require().NotNil(deployment.Spec.Template.Spec.ShareProcessNamespace,
156+
"shareProcessNamespace should still be set when telemetry.rbac.create is false")
157+
s.True(*deployment.Spec.Template.Spec.ShareProcessNamespace,
158+
"shareProcessNamespace should still be true when telemetry.rbac.create is false")
159+
}
160+
135161
// TestSidecarSecurityContext asserts the sidecar drops all default caps,
136162
// disables privilege escalation, and only adds SYS_PTRACE on executor (DO)
137163
// sidecars where py-spy crash-stack archives are load-bearing.

0 commit comments

Comments
 (0)