feat: add support for extraContainers and nativeSidecarContainers

[kaixi-wang]

Rationale

Review Priority

• high
• medium
• low

Changes

Checklist

• This PR maintains parity between Docker Compose and Helm

Testing

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 16c54ce3-4d1d-4e86-875d-ff4b9b76472d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This pull request adds telemetry overlay support to Docker deployments by documenting the feature, extending environment templates with telemetry configuration variables, and introducing comprehensive test coverage for telemetry Compose configurations. Additionally, it extends RBAC permissions to allow pod log access.

Changes

Cohort / File(s)	Summary
Telemetry Documentation & Configuration docker/README.md, docker/internal-auth/env.template, docker/legacy-auth/env.template	Adds telemetry overlay documentation explaining the feature enables live per-service metrics. Introduces commented-out telemetry environment variables for both auth variants, covering Redis connectivity, sidecar images, namespace labeling, and process target configuration.
Telemetry Compose Tests tests/unit/compose/docker-compose-internal-auth_test.go, tests/unit/compose/docker-compose-legacy-auth_test.go	Expands test coverage with new scenarios for telemetry Compose overlays. Adds assertions for telemetry-specific services (Redis, telemetry sidecars) and verifies expected container images (redis:7-alpine, voxel51/telemetry-sidecar:latest) across FiftyOne app and Teams services, including delegated-operators scenarios.
RBAC Policy Updates tests/unit/helm/api-role_test.go	Updates RBAC Role test to include a new PolicyRule granting get access to pods/log resources in the core API group.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Telemetry whispers now echo so clear,
Metrics and logs dancing year after year,
Redis and sidecars join the embrace,
With tests to ensure they all find their place!
Pod logs can peek now—what joy, what delight! 📊✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is largely empty with template sections present but no substantive content filling the Rationale, Changes, or Testing sections.	Complete the Rationale section explaining why telemetry sidecars are being added, detail all changes made across Docker and Helm configurations, and document how the changes were tested.
Title check	⚠️ Warning	The PR title claims to add support for 'extraContainers and nativeSidecarContainers', but the actual changes focus on telemetry configuration (documentation, environment templates, and telemetry test scenarios) rather than implementing container support features.	Update the title to accurately reflect the primary changes, such as 'docs: add telemetry overlay configuration' or 'feat: add telemetry configuration documentation and tests'. The current title is misleading about what the PR actually implements.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch kacey/sidecar

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

docker/README.md (1)

649-658: Add one concrete compose command example for overlay order.

Line 653-Line 654 says to layer compose.telemetry.yaml, but a command snippet would reduce operator guesswork.

Suggested README snippet

 ### Telemetry

 FiftyOne Enterprise ships an optional telemetry overlay that exposes live
 per-service metrics (CPU, memory, FDs, thread counts) and tailed logs on
 the Settings → Metrics page. Enable by layering `compose.telemetry.yaml`
 onto your existing compose invocation.
+
+Example:
+
+```shell
+docker compose \
+  -f compose.yaml \
+  -f compose.telemetry.yaml \
+  -f compose.override.yaml \
+  up -d
+```

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docker/README.md` around lines 649 - 658, The Telemetry section in
docker/README.md instructs users to layer compose.telemetry.yaml but lacks a
concrete docker compose command; add a short example command showing overlay
order (using compose.yaml then compose.telemetry.yaml then
compose.override.yaml) under the "Telemetry" heading so operators can copy-paste
a working invocation; place the snippet immediately after the sentence
mentioning `compose.telemetry.yaml` and label it as a shell example for clarity.

tests/unit/compose/docker-compose-internal-auth_test.go (1)

227-259: Add override-image test cases for telemetry env customization.

Current cases validate defaults (redis:7-alpine, voxel51/telemetry-sidecar:latest) but not env-driven overrides. Adding one override case for TELEMETRY_REDIS_IMAGE and TELEMETRY_SIDECAR_IMAGE would harden regression coverage.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/unit/compose/docker-compose-internal-auth_test.go` around lines 227 -
259, Add test cases that verify env-driven image overrides by duplicating the
existing telemetry cases ("telemetryRedis", "telemetrySidecarFiftyoneApp",
"telemetrySidecarTeamsApi", "telemetrySidecarTeamsDo") but include an override
.env file in the dotEnvFiles slice and change the expected image strings to the
custom values from TELEMETRY_REDIS_IMAGE and TELEMETRY_SIDECAR_IMAGE; update the
table entries that currently use internalAuthComposeTelemetryFile and
s.dotEnvFiles to append the new override env file (the file that sets
TELEMETRY_REDIS_IMAGE / TELEMETRY_SIDECAR_IMAGE) and set the expected image to
the override value so the test asserts env-driven customization.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docker/README.md`:
- Around line 649-658: The Telemetry section in docker/README.md instructs users
to layer compose.telemetry.yaml but lacks a concrete docker compose command; add
a short example command showing overlay order (using compose.yaml then
compose.telemetry.yaml then compose.override.yaml) under the "Telemetry" heading
so operators can copy-paste a working invocation; place the snippet immediately
after the sentence mentioning `compose.telemetry.yaml` and label it as a shell
example for clarity.

In `@tests/unit/compose/docker-compose-internal-auth_test.go`:
- Around line 227-259: Add test cases that verify env-driven image overrides by
duplicating the existing telemetry cases ("telemetryRedis",
"telemetrySidecarFiftyoneApp", "telemetrySidecarTeamsApi",
"telemetrySidecarTeamsDo") but include an override .env file in the dotEnvFiles
slice and change the expected image strings to the custom values from
TELEMETRY_REDIS_IMAGE and TELEMETRY_SIDECAR_IMAGE; update the table entries that
currently use internalAuthComposeTelemetryFile and s.dotEnvFiles to append the
new override env file (the file that sets TELEMETRY_REDIS_IMAGE /
TELEMETRY_SIDECAR_IMAGE) and set the expected image to the override value so the
test asserts env-driven customization.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: da97c372-0939-4bd2-b515-cdb3ec767500

📥 Commits

Reviewing files that changed from the base of the PR and between 7b0c11d and 0a182bf.

⛔ Files ignored due to path filters (5)

• helm/fiftyone-teams-app/templates/api-deployment.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/api-role.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/app-deployment.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/delegated-operator-instance-deployment.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/plugins-deployment.yaml is excluded by !**/*.yaml

📒 Files selected for processing (6)

• docker/README.md
• docker/internal-auth/env.template
• docker/legacy-auth/env.template
• tests/unit/compose/docker-compose-internal-auth_test.go
• tests/unit/compose/docker-compose-legacy-auth_test.go
• tests/unit/helm/api-role_test.go

[mo-getter]

It's looking good! Please note, we'll want to target the next release branch (which doesn't exist yet), and I'd also like to do a bit of hands-on testing before we merge.

But this is awesome stuff! Thanks for doing all of this.

[kaixi-wang]

closing in favor of #566