feat: add authenticated OpenTelemetry gateway - #231
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an authenticated OpenTelemetry Collector with SAK-to-JWT enrollment, optional Helm deployment and agent routing, integration validation, multi-architecture image publishing, and nested-module build checks. ChangesOpenTelemetry gateway
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Agent as fleet-intelligence-agent
participant Gateway as fleetint-otelcol
participant Enrollment as enrollment endpoint
participant Backend as OTLP backend
Agent->>Gateway: Send OTLP metrics and logs
Gateway->>Enrollment: Enroll with SAK bearer token
Enrollment-->>Gateway: Return JWT assertion
Gateway->>Backend: Export OTLP with JWT and Nv-Actor-Id
Backend-->>Gateway: Return response or refreshed JWT
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 188-189: Update the actions/checkout@v4 step in the build job to
set persist-credentials to false, ensuring the checkout action does not store
the GITHUB_TOKEN in git configuration before third-party tooling runs.
- Around line 191-197: Update the “Set up Go” workflow step to use
actions/setup-go@v6 instead of v4, keeping its existing go-version configuration
unchanged.
In `@cmd/fleetint/run.go`:
- Around line 225-227: Update the FLEETINT_COLLECTOR_ENDPOINT handling in the
environment override block so Infow does not log the raw endpoint value. Log
only that the collector endpoint override was enabled, or pass a safely redacted
endpoint while preserving assignment of the full value to he.CollectorEndpoint.
In `@internal/config/config.go`:
- Around line 181-190: Update the collector_endpoint validation around
parsed.ForceQuery and the raw endpoint string to reject bare trailing ? and #
characters, while preserving the existing credential, query, fragment, and
HTTP(S) checks. Add regression tests covering http://collector.example? and
http://collector.example#.
In `@otelcol/auth/sakauth/config.go`:
- Around line 57-59: Update the SAK enrollment TLS validation in the
configuration validation method containing the c.TLS.Insecure check to also
reject c.TLS.InsecureSkipVerify. Return the same validation error for either
insecure TLS setting, ensuring enrollment requires HTTPS certificate
verification.
In `@otelcol/auth/sakauth/extension.go`:
- Around line 246-252: Guard the response-token update in the request flow
around the jwt_assertion handling so it only applies when the shared rt.ext.jwt
still equals the request’s usedJWT; otherwise leave the newer token and
customerID unchanged. Add a regression test covering concurrent or out-of-order
responses, including a 401 refresh, to verify stale responses cannot overwrite
the newest JWT.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9c891f07-c5ab-4125-b79f-f13e6e4bb1f6
⛔ Files ignored due to path filters (1)
otelcol/auth/sakauth/go.sumis excluded by!**/*.sum
📒 Files selected for processing (29)
.dockerignore.github/workflows/ci.yml.github/workflows/release.yml.gitignoreMakefilecmd/fleetint/gateway_test.gocmd/fleetint/metadata.gocmd/fleetint/run.godeployments/helm/fleet-intelligence-agent/templates/daemonset.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-configmap.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-deployment.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-networkpolicy.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-pdb.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-secret.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-service.yamldeployments/helm/fleet-intelligence-agent/values.yamldeployments/otel-collector/config.yamlinternal/config/config.gointernal/config/config_test.gointernal/exporter/exporter.gointernal/exporter/exporter_test.gootelcol/Dockerfileotelcol/auth/sakauth/collector_integration_test.gootelcol/auth/sakauth/config.gootelcol/auth/sakauth/extension.gootelcol/auth/sakauth/extension_test.gootelcol/auth/sakauth/factory.gootelcol/auth/sakauth/go.modotelcol/otelcol-builder.yaml
| - name: Set up Go | ||
| uses: actions/setup-go@v4 | ||
| with: | ||
| go-version: ${{ env.GO_VERSION }} | ||
|
|
||
| - name: Install OTel Collector Builder | ||
| run: go install go.opentelemetry.io/collector/cmd/builder@v0.156.0 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the workflow trigger and relevant job section.
file=".github/workflows/ci.yml"
echo "== File lines around trigger =="
nl -ba "$file" | sed -n '1,120p'
echo
echo "== File lines around setup-go step =="
nl -ba "$file" | sed -n '170,220p'Repository: NVIDIA/fleet-intelligence-agent
Length of output: 241
🌐 Web query:
actions/setup-go v6 cache default enabled cache behavior workflow cache poisoning GitHub Actions
💡 Result:
The actions/setup-go action (including v6) has caching enabled by default [1][2]. The action supports caching for Go modules and build outputs to improve performance, using the cache input which defaults to true [1][3]. Regarding cache poisoning, this is a known security concern in GitHub Actions environments [4][5]. Because actions/setup-go historically enables read-write caching by default, untrusted workflows—such as those triggered by pull requests from forks or external contributors—can potentially write malicious artifacts to the shared cache [4][6]. These tainted artifacts can then be restored by subsequent, trusted workflow runs on the main branch, leading to a supply chain vulnerability [4][5]. To mitigate this risk, several strategies are employed: 1. Manual Disabling: In sensitive workflows, such as release or deployment pipelines, it is standard practice to explicitly set cache: false to prevent the action from writing to the cache [5][6]. 2. Read-Only Cache Support: Recent updates to actions/setup-go include the addition of a cache-write input [4]. By setting cache-write: false, users can allow the action to restore existing caches while skipping the post-step cache save, effectively implementing a read-only cache mode [4]. This allows workflows (like PR checks) to benefit from existing dependencies without risking the integrity of the cache for future runs [4]. When designing workflows, it is recommended to evaluate whether full read-write caching is necessary or if a read-only configuration provides sufficient protection against cache poisoning [4][5].
Citations:
- 1: https://github.com/actions/setup-go?tab=readme-ov-file
- 2: https://github.com/actions/setup-go/tree/v6.3.0
- 3: https://github.com/actions/setup-go/tree/v6.0.0
- 4: Add cache-write input for read-only cache mode actions/setup-go#732
- 5: Disable Go module caching in release workflow to prevent cache poisoning github/gh-aw#5872
- 6: fix: disable Go module cache in agentic setup actions to prevent cache poisoning github/gh-aw#19865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/ci.yml"
echo "== Trigger and top-level workflow lines =="
sed -n '1,90p' "$file" | cat -n
echo
echo "== Step around actions/setup-go =="
sed -n '180,205p' "$file" | cat -nRepository: NVIDIA/fleet-intelligence-agent
Length of output: 4316
🌐 Web query:
actions/setup-go v4 deprecated Node runtime current major v6 release notes
💡 Result:
The actions/setup-go action has progressed through several major versions, primarily driven by the deprecation and end-of-life cycles of Node.js runtimes on GitHub Actions runners [1][2][3]. Key points regarding the transition and v6 release: 1. Node.js Runtime Updates: Older versions of the action used outdated Node.js runtimes. Specifically, v5 upgraded the runtime from Node 16 to Node 20 [2]. The current major release, v6, further upgrades the Node.js runtime from Node 20 to Node 24 [3]. 2. Deprecation Context: The deprecation of Node 20 on GitHub Actions runners was announced in September 2025, as Node 20 reached its end-of-life (EOL) in April 2026 [1]. Users are encouraged to update to actions that support newer Node.js runtimes, such as v6 of setup-go [4][3]. 3. v6 Release Notes: The v6.0.0 release (published September 4, 2025) includes the upgrade to Node 24 and improvements to toolchain handling [3]. Because of the runtime change, users must ensure their runners are on version v2.327.1 or later to maintain compatibility [4][3]. Additional enhancements in the v6.x series have included updated download sources for Go distributions and support for.tool-versions files [5]. In summary, the transition from v4 to v6 involves significant updates to the underlying Node.js runtime (from Node 16 in v4/early versions to Node 24 in v6) to maintain compliance with GitHub Actions runner lifecycle requirements [2][3].
Citations:
- 1: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
- 2: https://github.com/actions/setup-go/releases/tag/v5.0.0
- 3: https://github.com/actions/setup-go/releases/tag/v6.0.0
- 4: https://github.com/actions/setup-go?tab=readme-ov-file
- 5: https://github.com/actions/setup-go/releases/tag/v6.1.0
🌐 Web query:
GitHub Actions pull_request fork cache save actions/setup-go default cache poisoning trusted push restore
💡 Result:
When using GitHub Actions, specifically with pull_request events from forks or pull_request_target workflows, understanding cache access and security risks is critical. Cache Access Limitations By default, GitHub Actions enforces cache isolation [1][2]. A workflow run triggered by a pull request from a fork has restricted access to the cache: - It can restore existing cache entries available to the base branch or default branch [1]. - It is generally restricted from creating or overwriting cache entries to prevent unauthorized cache modifications [3][1]. When a cache write is denied in these scenarios, the actions/cache step reports a warning but does not fail the job [3][4]. Security Risks: Cache Poisoning "Cache poisoning" is a security concern where an attacker attempts to inject malicious data into the cache [5][6][7]. - Risk with pull_request_target: This trigger runs in the context of the base repository and has access to secrets and write permissions [8][4][6]. If a workflow using pull_request_target is misconfigured to check out and execute code from an untrusted fork (the "Pwn Request" pattern), that code runs with the privileges of the base repository [5][6][7]. - Mechanism: Even if you restrict the job's GITHUB_TOKEN permissions (e.g., contents: read), the cache save step uses an internal, runner-specific token to authenticate with the cache service [5][6][7]. Consequently, if malicious code runs during the workflow, it can write arbitrary data to the cache under a deterministic key that a future, trusted workflow might restore [6][7]. This can lead to downstream supply-chain compromises [5][7]. Best Practices for actions/setup-go and Caching The actions/setup-go action uses actions/cache internally [9][10]. To maintain security: - Avoid Unsafe Saves: Do not allow cache writes in workflows that execute untrusted fork code (such as those using pull_request_target that check out the PR's head ref) [5][11]. - Use Restore-Only Logic: In untrusted contexts, configure workflows to be "restore-only" [5]. You can implement custom logic to detect if a PR is from a fork and modify your cache keys to prevent collision with trusted branches [12]. - Rely on Trusted Triggers: Only perform cache saves in workflows triggered by trusted events, such as push to your default branch [8][4]. For further implementation guidance, refer to the documentation on securing pull_request_target workflows and the specific behavior of the actions/cache action [8][3][4].
Citations:
- 1: Cache not caching on pull_request actions/cache#633
- 2: https://github.com/orgs/community/discussions/194493
- 3: https://github.com/actions/cache/?tab=readme-ov-file
- 4: https://docs.github.com/en/enterprise-cloud@latest/actions/reference/security/securely-using-pull_request_target
- 5: https://saschb2b.com/blog/pull-request-target-trap
- 6: https://www.trackr.live/2026/06/27/github-actions-cache-poisoning-fork-trust-boundary/
- 7: https://www.endorlabs.com/learn/how-a-misconfigured-ci-workflow-became-an-npm-supply-chain-compromise
- 8: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target
- 9: https://github.com/actions/setup-go/blob/main/README.md
- 10: https://github.com/actions/setup-go
- 11: Cache/save should not run automatically from a pull_request_trigger actions/cache#1756
- 12: https://github.com/ag-ui-protocol/ag-ui/blob/d53c4ef6/.github/workflows/unit-go-sdk.yml
🌐 Web query:
GitHub Actions cache forked pull requests read-only save disabled official docs
💡 Result:
GitHub Actions restricts cache access for workflows triggered by pull requests from forks to ensure security and prevent cache poisoning [1][2]. In these scenarios, workflows have read-only access to the cache, meaning they can restore existing cache entries but cannot save or create new ones [1][3]. When a workflow attempts to save a cache while in a read-only state: - The job does not fail [1][3]. - The actions/cache action reports a warning (e.g., "cache write denied") in the workflow logs [1][3]. - The step completes successfully without writing any data to the cache [1][3]. This behavior is a security measure applied to untrusted triggers, including pull requests from forks, to prevent unauthorized modification of the shared cache [4][2]. For workflows that require populating the cache, GitHub recommends using trusted triggers, such as events on the base repository's default branch (e.g., push events) [2]. As of June 2026, GitHub has further reinforced read-only cache protections for workflow events that can be triggered without explicit write permissions [4].
Citations:
- 1: https://github.com/actions/cache/?tab=readme-ov-file
- 2: https://docs.github.com/en/enterprise-server@3.19/actions/reference/security/securely-using-pull_request_target
- 3: https://github.com/actions/cache/blob/main/README.md
- 4: https://github.blog/changelog/2026-06-26-read-only-actions-cache-for-untrusted-triggers/
Update actions/setup-go to v6. actions/setup-go@v4 is behind the current major and should be bumped to avoid Node-runtime deprecation issues.
🧰 Tools
🪛 actionlint (1.7.12)
[error] 192-192: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 zizmor (1.26.1)
[error] 192-192: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 191 - 197, Update the “Set up Go”
workflow step to use actions/setup-go@v6 instead of v4, keeping its existing
go-version configuration unchanged.
Source: Linters/SAST tools
| @@ -0,0 +1,30 @@ | |||
| # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |||
There was a problem hiding this comment.
Wondering why do we need a pdb here?
There was a problem hiding this comment.
The gateway is 2 replicas on the metrics/logs path and the sending queue is in-memory, so if a drain evicts both at once we drop whatever is in flight. minAvailable: 1 keeps one up.
It only renders when the gateway is enabled and there is a config for that, we can default to false if that is less confusing
| processors: | ||
| memory_limiter: | ||
| check_interval: 5s | ||
| limit_mib: 400 | ||
| spike_limit_mib: 100 | ||
|
|
||
| batch: | ||
| timeout: 10s | ||
| send_batch_size: 1000 |
There was a problem hiding this comment.
Can this memory limiter be configured through helm values? What if we change the OTLP pod memory configuration to a large value say 2GB, do we also need to change this memory limiter?
There was a problem hiding this comment.
Good catch! There were two configs disagreeing. The one you're looking at wasn't read by Helm, so I deleted it, the ConfigMap is the only one now.
It's configurable via otelGateway.memoryLimiter.* and it's a percentage, so it follows the pod limit.
| } | ||
| e.mu.Lock() | ||
| e.jwt = jwt | ||
| e.customerID = extractCustomerID(jwt) |
There was a problem hiding this comment.
Do we need the customer id from the backend?
| path: fleetint-${{ matrix.goos }}-${{ matrix.goarch }} | ||
| retention-days: 7 | ||
|
|
||
| build_check_otelcol: |
There was a problem hiding this comment.
otelcol/auth/sakauth is a separate Go module (it has its own go.mod). In .github/workflows/ci.yml, unit_test runs make test → go test ./... from the root module, which does not descend into nested modules. similar to the lint runs golangci-lint at the root and vuln runs govulncheck ./... at the root. build_check_otelcol only runs the single integration test: run ^TestCollectorGatewayEndToEnd$ and only for amd64). so extension_test.go and config tests are never executed in CI. Should we add something like following in ci to include that?
- name: Test sakauth module
working-directory: otelcol/auth/sakauth
run: go test -race ./...
There was a problem hiding this comment.
Good catch! I added to the Makefile so the ci and local tests will include the otelcol directory!
| endpoint: 0.0.0.0:{{ .Values.otelGateway.service.port }} | ||
|
|
||
| processors: | ||
| memory_limiter: |
There was a problem hiding this comment.
from default values.yaml,
memory_limiter:
limit_percentage: 80
spike_limit_percentage: 20
is this limiter computed from the total node mem or the pod limit? if it's the former, there could possibly be OOM issue if the pod is capped at a smaller mem e.g. resources.limits.memory: 512Mi while the node itself is large, say 512G
There was a problem hiding this comment.
Pod limit memorylimiter reads the cgroup quota, falling back to node memory only when no limit is set. We default to limits.memory: 512Mi, so 80% is ~410Mi.
The fallback is a real risk with resources: {}, so I added a fail guard plus a note in values.yaml.
Add an optional custom OpenTelemetry Collector that centralizes SAK-to-JWT authentication and forwards agent metrics and logs while preserving direct enrollment for inventory and attestation. Harden the gateway for production with current Collector dependencies, secure token and TLS handling, highly available Helm resources, GHCR publishing, and end-to-end OTLP integration coverage. Signed-off-by: Rodrigo Sampaio Vaz <rvaz@nvidia.com>
otelcol/auth/sakauth has its own go.mod, so root-module `./...` targets never descended into it: its unit and config tests, golangci-lint and govulncheck were all silently skipped. Add an OTELCOL_MODULE variable and extend the test, lint and vuln targets to run there too. Keeping this in the Makefile means `make docker-test` covers the module as well, since it runs those three targets. The CI lint job invokes golangci-lint directly rather than through make, so it gets a matching step. Also set persist-credentials: false on the build_check_otelcol checkout, which installs and runs the OTel Collector Builder and its dependencies. Signed-off-by: Rodrigo Sampaio Vaz <rvaz@nvidia.com>
Validate() rejected tls.insecure but not tls.insecure_skip_verify, which kept the https:// scheme and so passed every other check while disabling certificate verification. The jwt_assertion handler also overwrote the stored token unconditionally, so an out-of-order response could replace a newer token with an older one. Install the refreshed token only while the stored one is unchanged. Signed-off-by: Rodrigo Sampaio Vaz <rvaz@nvidia.com>
The FLEETINT_COLLECTOR_ENDPOINT override logged the raw value, which can carry credentials in userinfo or a token in the path. Validation rejects those but runs later, so log only the host. Validation also missed a bare trailing "?" and "#": url.Parse reports the first as ForceQuery with an empty RawQuery and drops the second entirely, so both reached the point where "/v1/metrics" is appended. Signed-off-by: Rodrigo Sampaio Vaz <rvaz@nvidia.com>
The collector memory_limiter is configured by percentage and derives that percentage from the container's cgroup memory limit, falling back to total node memory when none is set. On a large node an unlimited pod would size the limiter against the node and be OOM-killed before it ever engages, so reject that at render time. Document the relationship in values.yaml: the percentages track resources.limits.memory automatically, so raising the pod limit needs no second edit. Delete deployments/otel-collector/config.yaml. It was unreferenced by the Dockerfile, the builder manifest and the tests, and its hardcoded limit_mib contradicted the ConfigMap the chart actually deploys. Signed-off-by: Rodrigo Sampaio Vaz <rvaz@nvidia.com>
0ac718d to
4e2bb61
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/release.yml (1)
142-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider embedding version metadata into the otelcol image.
Unlike the agent image build (Lines 128-141), this step passes no
build-args/OCI labels, so the publishedfleetint-otelcolimage carries no embedded version/revision/build-timestamp for traceability. Would need correspondingARG/LABELadditions inotelcol/Dockerfile.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 142 - 150, Update the “Build and push fleetint-otelcol image” step to pass the same version, revision, and build-timestamp metadata build arguments and OCI labels used by the agent image, and add the corresponding ARG/LABEL declarations in otelcol/Dockerfile so the published image embeds that metadata.otelcol/Dockerfile (1)
9-9: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider pinning base images by digest.
golang:1.26.5andgcr.io/distroless/static:nonrootare tag-referenced rather than digest-pinned, so the same tag can resolve to different content over time, weakening build reproducibility for a multi-arch, publicly-published image.Also applies to: 27-27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@otelcol/Dockerfile` at line 9, Update the Dockerfile’s builder and runtime base image references, including golang:1.26.5 and gcr.io/distroless/static:nonroot, to use immutable image digests while retaining their current tags and multi-architecture support.deployments/helm/fleet-intelligence-agent/templates/otel-gateway-deployment.yaml (1)
85-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded secret key name for
existingSecret.When
otelGateway.enroll.existingSecretis set, the key is hardcoded tosak-token. Operators bringing an existing Secret (the recommended production path per the PR's objectives) may already use a different key name, forcing them to either rename/duplicate the secret or fail silently.♻️ Suggested addition of a configurable key field
- name: SAK_TOKEN valueFrom: secretKeyRef: name: {{ .Values.otelGateway.enroll.existingSecret | default (printf "%s-otel-gateway-sak" (include "fleet-intelligence-agent.fullname" .)) }} - key: sak-token + key: {{ .Values.otelGateway.enroll.existingSecretKey | default "sak-token" }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployments/helm/fleet-intelligence-agent/templates/otel-gateway-deployment.yaml` around lines 85 - 88, Make the secret key reference in the otel gateway deployment configurable for existing secrets instead of always using sak-token. Add or reuse an otelGateway enrollment values field for the key, use it in the secretKeyRef under the existingSecret path, and preserve sak-token as the default when no override is provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@otelcol/auth/sakauth/extension.go`:
- Around line 148-155: Update refreshJWT’s post-performEnrollment write to
compare e.jwt with the previously captured staleJWT while holding e.mu; install
the enrollment JWT and customer ID only when they still match, otherwise return
the current e.jwt without overwriting it. Add a regression test covering an
in-flight enrollment refresh completing after a newer response-header refresh,
asserting the newer token remains active.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 142-150: Update the “Build and push fleetint-otelcol image” step
to pass the same version, revision, and build-timestamp metadata build arguments
and OCI labels used by the agent image, and add the corresponding ARG/LABEL
declarations in otelcol/Dockerfile so the published image embeds that metadata.
In
`@deployments/helm/fleet-intelligence-agent/templates/otel-gateway-deployment.yaml`:
- Around line 85-88: Make the secret key reference in the otel gateway
deployment configurable for existing secrets instead of always using sak-token.
Add or reuse an otelGateway enrollment values field for the key, use it in the
secretKeyRef under the existingSecret path, and preserve sak-token as the
default when no override is provided.
In `@otelcol/Dockerfile`:
- Line 9: Update the Dockerfile’s builder and runtime base image references,
including golang:1.26.5 and gcr.io/distroless/static:nonroot, to use immutable
image digests while retaining their current tags and multi-architecture support.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d4079e01-ec41-48da-bf03-7f384b296552
⛔ Files ignored due to path filters (1)
otelcol/auth/sakauth/go.sumis excluded by!**/*.sum
📒 Files selected for processing (28)
.dockerignore.github/workflows/ci.yml.github/workflows/release.yml.gitignoreMakefilecmd/fleetint/gateway_test.gocmd/fleetint/metadata.gocmd/fleetint/run.godeployments/helm/fleet-intelligence-agent/templates/daemonset.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-configmap.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-deployment.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-networkpolicy.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-pdb.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-secret.yamldeployments/helm/fleet-intelligence-agent/templates/otel-gateway-service.yamldeployments/helm/fleet-intelligence-agent/values.yamlinternal/config/config.gointernal/config/config_test.gointernal/exporter/exporter.gointernal/exporter/exporter_test.gootelcol/Dockerfileotelcol/auth/sakauth/collector_integration_test.gootelcol/auth/sakauth/config.gootelcol/auth/sakauth/extension.gootelcol/auth/sakauth/extension_test.gootelcol/auth/sakauth/factory.gootelcol/auth/sakauth/go.modotelcol/otelcol-builder.yaml
🚧 Files skipped from review as they are similar to previous changes (15)
- .gitignore
- cmd/fleetint/metadata.go
- deployments/helm/fleet-intelligence-agent/templates/daemonset.yaml
- internal/config/config_test.go
- otelcol/otelcol-builder.yaml
- otelcol/auth/sakauth/config.go
- cmd/fleetint/run.go
- internal/exporter/exporter_test.go
- internal/config/config.go
- internal/exporter/exporter.go
- otelcol/auth/sakauth/factory.go
- deployments/helm/fleet-intelligence-agent/values.yaml
- .dockerignore
- Makefile
- otelcol/auth/sakauth/extension_test.go
| jwt, err := e.performEnrollment(ctx) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| e.mu.Lock() | ||
| e.jwt = jwt | ||
| e.customerID = extractCustomerID(jwt) | ||
| e.mu.Unlock() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Prevent a delayed enrollment refresh from replacing a newer JWT.
refreshJWT checks staleJWT before the network call, but unconditionally writes afterward. A concurrent response-header refresh can install a newer token while enrollment is pending, and this path then regresses it. Only install the enrollment result if e.jwt still equals staleJWT; otherwise return the current token. Add an out-of-order refresh/header regression test.
Proposed fix
jwt, err := e.performEnrollment(ctx)
if err != nil {
return "", err
}
+ newCustomerID := extractCustomerID(jwt)
e.mu.Lock()
- e.jwt = jwt
- e.customerID = extractCustomerID(jwt)
+ if e.jwt == staleJWT {
+ e.jwt = jwt
+ e.customerID = newCustomerID
+ } else {
+ jwt = e.jwt
+ }
e.mu.Unlock()
return jwt, nil📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jwt, err := e.performEnrollment(ctx) | |
| if err != nil { | |
| return "", err | |
| } | |
| e.mu.Lock() | |
| e.jwt = jwt | |
| e.customerID = extractCustomerID(jwt) | |
| e.mu.Unlock() | |
| jwt, err := e.performEnrollment(ctx) | |
| if err != nil { | |
| return "", err | |
| } | |
| newCustomerID := extractCustomerID(jwt) | |
| e.mu.Lock() | |
| if e.jwt == staleJWT { | |
| e.jwt = jwt | |
| e.customerID = newCustomerID | |
| } else { | |
| jwt = e.jwt | |
| } | |
| e.mu.Unlock() | |
| return jwt, nil |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@otelcol/auth/sakauth/extension.go` around lines 148 - 155, Update
refreshJWT’s post-performEnrollment write to compare e.jwt with the previously
captured staleJWT while holding e.mu; install the enrollment JWT and customer ID
only when they still match, otherwise return the current e.jwt without
overwriting it. Add a regression test covering an in-flight enrollment refresh
completing after a newer response-header refresh, asserting the newer token
remains active.
Signed-off-by: Rodrigo Sampaio Vaz <rvaz@nvidia.com>
e9306b6 to
d679bcc
Compare
3 participants
Description
Adds an optional, production-hardened OpenTelemetry gateway deployment for Fleet Intelligence. Agents route OTLP metrics and logs through a per-cluster custom Collector, which owns backend SAK-to-JWT authentication; direct agent enrollment remains unchanged for inventory and attestation.
The gateway is disabled by default and does not change bare-metal or existing Kubernetes behavior unless
otelGateway.enabled=true.Security and reliability
Review guide
cmd/fleetint/run.go,internal/config/, andinternal/exporter/establish collector precedence without forwarding node credentials.otelcol/auth/sakauth/contains enrollment, token refresh, TLS, and request authentication.deployments/helm/fleet-intelligence-agent/templates/otel-gateway-*contains availability and security controls.otelcol/Dockerfile,otelcol/otelcol-builder.yaml, and GitHub workflows build, test, and publish the collector.collector_integration_test.gostarts the built Collector and verifies real OTLP metrics/logs forwarding, identity attributes, and backend authentication.Operational notes
otelGateway.enroll.existingSecretrather than passing a SAK through Helm values.Test plan
90.8%statement coverage).go vetand fullgolangci-lint(0 issues).Checklist
Summary by CodeRabbit
fleetint-otelcolimage, including automated publishing/tagging.