Merge pull request #585 from voxel51/kacey/optional-role-creation

fix: make auto rbac create optional

Author: kaixi-wang

helm/fiftyone-teams-app/README.md

@@ -1086,6 +1086,7 @@ If pods show unhealthy states (e.g., `0/1`, `CrashLoopBackOff`, `Pending`):
 | teamsAppSettings.volumeMounts | list | `[]` | Volume mounts for `teams-app` pods. [Reference][volumes]. |
 | teamsAppSettings.volumes | list | `[]` | Volumes for `teams-app` pods. [Reference][volumes]. |
 | 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. |
+| 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. |
 | 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]. |
 | 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`). |
 | telemetry.redis.image | string | `"redis:7-alpine"` | Container image for the telemetry Redis Deployment. |

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

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

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

@@ -1,4 +1,4 @@
-{{- if .Values.telemetry.enabled }}
+{{- if and .Values.telemetry.enabled .Values.telemetry.rbac.create }}
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:

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

@@ -1,4 +1,4 @@
-{{- if .Values.telemetry.enabled }}
+{{- if and .Values.telemetry.enabled .Values.telemetry.rbac.create }}
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
 metadata:

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

@@ -4048,6 +4048,19 @@
           "title": "enabled",
           "type": "boolean"
         },
+        "rbac": {
+          "properties": {
+            "create": {
+              "default": true,
+              "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.",
+              "title": "create",
+              "type": "boolean"
+            }
+          },
+          "required": [],
+          "title": "rbac",
+          "type": "object"
+        },
         "redis": {
           "properties": {
             "containerSecurityContext": {

helm/fiftyone-teams-app/values.yaml

@@ -1628,6 +1628,17 @@ telemetry:
   # -- When `false`, no telemetry resources or sidecars are rendered and the
   # FiftyOne UI's delegated-operator log viewer will be empty.
   enabled: true
+  rbac:
+    # -- 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.
+    create: true
   redis:
     # -- Container image for the telemetry Redis Deployment.
     image: redis:7-alpine

tests/unit/helm/telemetry-rolebinding_test.go

@@ -61,6 +61,22 @@ func (s *telemetryRoleBindingTemplateTest) TestExplicitlyDisabled() {
 	s.ErrorContains(err, "could not find template")
 }
 
+// With telemetry enabled but rbac.create false, the Role/RoleBinding are
+// skipped so an install identity without namespaced RBAC permissions can still
+// deploy telemetry. Metrics keep working (shared process namespace), but the
+// sidecar loses pods/log API access, so the log viewer stays empty unless
+// serviceAccounts points at an SA that already carries pods/log.
+func (s *telemetryRoleBindingTemplateTest) TestRbacCreateDisabled() {
+	options := &helm.Options{SetValues: map[string]string{
+		"telemetry.enabled":     "true",
+		"telemetry.rbac.create": "false",
+	}}
+	for _, tmpl := range s.templates {
+		_, err := helm.RenderTemplateE(s.T(), options, s.chartPath, s.releaseName, []string{tmpl})
+		s.ErrorContains(err, "could not find template", "%s should be skipped", tmpl)
+	}
+}
+
 // extractRole finds the Role document (not RoleBinding) in multi-doc render output.
 func (s *telemetryRoleBindingTemplateTest) extractRole(output string) (rbacv1.Role, bool) {
 	for _, doc := range splitYAMLDocs(output) {

tests/unit/helm/telemetry-sidecar_test.go

@@ -147,9 +147,12 @@ func (s *telemetrySidecarTemplateTest) TestSidecarExtraVolumes() {
 
 // TestShareProcessNamespaceEnabledByDefault asserts that the api, app,
 // plugins, and delegated-operator deployments all opt into PID-namespace
-// sharing when telemetry is enabled (the default). The sidecar relies on
-// /proc/<pid>/fd/1 access in the target container's PID namespace, so
-// dropping this would silently break log capture.
+// sharing when telemetry is enabled (the default). The sidecar samples the
+// target via the shared PID namespace — psutil reads /proc/<pid> for CPU,
+// memory, threads, and file descriptors, and py-spy attaches for stack
+// archives — so dropping this would silently break metrics capture. (Log
+// capture is separate: on Kubernetes the sidecar tails the container's logs
+// via the pods/log API, which the telemetry Role grants.)
 func (s *telemetrySidecarTemplateTest) TestShareProcessNamespaceEnabledByDefault() {
 	cases := []struct {
 		template string
@@ -229,6 +232,29 @@ func (s *telemetrySidecarTemplateTest) TestShareProcessNamespaceDisabledWithTele
 	}
 }
 
+// TestSidecarRenderedWithRbacCreateDisabled asserts that disabling the
+// telemetry RBAC escape hatch (telemetry.rbac.create=false) does not affect
+// sidecar injection. Metrics ride on the shared process namespace, not the
+// pods/log Role, so an install identity without namespaced RBAC permissions
+// still gets the sidecar and the Settings → Metrics dashboard.
+func (s *telemetrySidecarTemplateTest) TestSidecarRenderedWithRbacCreateDisabled() {
+	options := &helm.Options{SetValues: map[string]string{
+		"telemetry.rbac.create": "false",
+	}}
+	output := helm.RenderTemplate(s.T(), options, s.chartPath, s.releaseName,
+		[]string{"templates/app-deployment.yaml"})
+
+	var deployment appsv1.Deployment
+	helm.UnmarshalK8SYaml(s.T(), output, &deployment)
+
+	s.NotNil(findSidecar(deployment.Spec.Template.Spec.Containers),
+		"telemetry-sidecar should still be injected when telemetry.rbac.create is false")
+	s.Require().NotNil(deployment.Spec.Template.Spec.ShareProcessNamespace,
+		"shareProcessNamespace should still be set when telemetry.rbac.create is false")
+	s.True(*deployment.Spec.Template.Spec.ShareProcessNamespace,
+		"shareProcessNamespace should still be true when telemetry.rbac.create is false")
+}
+
 // TestSidecarSecurityContext asserts the sidecar drops all default caps,
 // disables privilege escalation, and only adds SYS_PTRACE on executor (DO)
 // sidecars where py-spy crash-stack archives are load-bearing.