fix(observability-pipelines-worker): correct sgc_path in bootstrap template - #2883
Conversation
…mplate The bootstrap.yaml template hardcodes sgc_path as `datadog-secret-backend` when secretFileContents is used, but the container image only has the binary as `sgc` (renamed from `secret-generic-connector` in the Dockerfile). This causes OPW to fail to load pipeline config with "No such file or directory" when resolving pipeline identifiers via the secrets backend. Also prevents emitting a duplicate `secret:` YAML block when both `bootstrap.config.secret` and `secretFileContents` are provided, which previously caused "duplicate field 'secret'" deserialization errors. Fixes: - sgc_path: .../datadog-secret-backend -> .../sgc - Skip auto-generated secret block when bootstrap.config already defines one Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccb904388e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
| {{- if and .Values.datadog.bootstrap.secretFileContents (not (dig "secret" nil .Values.datadog.bootstrap.config)) }} | ||
| secret: | ||
| sgc_path: /opt/datadog/observability-pipelines-worker/bin/datadog-secret-backend | ||
| sgc_path: /opt/datadog/observability-pipelines-worker/bin/sgc |
There was a problem hiding this comment.
looks like OPW defaults secret.sgc_path when it's omitted -- see the default setting to /sgc in 2.21.1 & this test
That default matches the sgc binary since 2.13.0, whereas the change to /sgc happened in 2.15.1. Should we simply omit secret.sgc_path from the template and defer to the worker default?
(note i haven't seen the google doc yet)
There was a problem hiding this comment.
ok yeah I stand by this after looking at the doc. Changing the hardcoded path works too but omitting it fully to defer to the default feels a tad nicer
Address review feedback: - Remove sgc_path entirely instead of changing the value; OPW defaults to the correct path (/opt/.../bin/sgc) since 2.13.0 - Add CHANGELOG.md entry for 2.21.2 - Bump chart version to 2.21.2 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes edge case where bootstrap.config.secret is set to an empty or null value: dig returns a falsy value, not evaluates to true, and the template emits a duplicate secret: block. hasKey checks key presence regardless of the value. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
9a17bea
into
main
Summary
sgc_pathfrombootstrap.yamltemplate, deferring to OPW's built-in default (/opt/.../bin/sgcsince 2.13.0)secret:YAML block when bothbootstrap.configandsecretFileContentsare sethasKeyinstead ofdigtruthiness to correctly detect key presence regardless of valueProblem
The
bootstrap.yamltemplate hardcodedsgc_pathas/opt/datadog/observability-pipelines-worker/bin/datadog-secret-backend, but the binary was renamed tosgcin 2.15.1:# Dockerfile:32 cp /opt/datadog-agent/embedded/bin/secret-generic-connector \ /opt/datadog/observability-pipelines-worker/bin/sgcOPW already defaults to the correct path when
sgc_pathis omitted (lib/config/src/bootstrap/mod.rs:35):When
secretFileContentswas used, OPW failed to load pipeline config:Additionally, providing
bootstrap.config.secretalongsidesecretFileContentsto override the path produced a duplicatesecret:YAML key, which OPW rejects.Fix
Three changes to
templates/bootstrap.yaml:sgc_pathline entirely, deferring to OPW's built-in defaultsecret:block whenbootstrap.configalready defines onehasKeyinstead ofdigto detect key presence (handles empty/null values correctly)Test plan - helm template render results
All 6 permutations verified locally via
helm template:Case 1:
secretFileContentsonly (most common path)Expected: single
secret:block withbackend_type/backend_config, nosgc_pathResult: PASS - single block, no
sgc_path, OPW uses defaultCase 2:
bootstrap.config.secret(vault) +secretFileContentsExpected: only user's vault config, no duplicate auto-generated block
Result: PASS - user config wins, no duplicate
Case 3:
bootstrap.config.secret=""+secretFileContents(codex edge case)Expected: user's
secret: ""rendered, no duplicate auto blockResult: PASS -
hasKeydetects key presence despite empty value, no duplicateCase 4:
bootstrap.config(non-secret keys) +secretFileContentsExpected: user's non-secret config + auto
secret:block (secret key absent from config)Result: PASS - auto block correctly added when user config has no
secretkeyCase 5:
bootstrap.config.secretonly (nosecretFileContents)Expected: user's config only, no auto block
Result: PASS - unchanged behavior
Case 6: Neither
bootstrap.confignorsecretFileContentsExpected: no bootstrap configmap rendered at all
Result: PASS - 0 matches for "bootstrap" in rendered output
🤖 Generated with Claude Code