feat(observability-pipelines): add WebSocket source#3855
Conversation
|
Implements the websocket source component: model structs, Expand/Flatten, schema, and registration in the observability_pipeline resource. Auth strategies none/basic/bearer/custom; mode-tagged TLS (enabled / with_client_cert with crt_file required); decoding bytes/gelf/json/syslog. Depends on the generated datadog-api-client-go websocket types from the API spec change; the build is blocked until the client is regenerated.
Jansen-w
force-pushed
the
jansen-w/websocket-source
branch
from
b53af57 to
bf5c214
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf5c21499f
ℹ️ 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".
| "crt_file": schema.StringAttribute{ | ||
| Optional: true, | ||
| Description: "Path to the client certificate file. Required when `mode` is `with_client_cert`.", |
There was a problem hiding this comment.
When users set tls.mode = "enabled" together with any certificate field, Terraform accepts the config but ExpandWebsocketSource serializes only the mode in that branch, so the API read drops those fields and subsequent plans keep trying to apply them. Conversely, with_client_cert is documented as requiring crt_file but the schema permits omitting it, allowing an invalid empty certificate path to reach the API. Please validate the TLS oneOf shape, or split the modes into mutually exclusive blocks, instead of accepting the same optional fields for both modes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the latest commit. Added WebsocketSourceTlsValidator (a validator.Object on the tls nested block, mirroring the existing SocketFramingValidator pattern):
mode = enabled: rejectscrt_file/ca_file/key_file/key_pass_keyif set, so no fields are silently dropped at expand time and there's no perma-diff.mode = with_client_cert: requires a non-emptycrt_file, matching the oneOf shape.
Enforced at plan time, so Expand never sees an invalid combination.
Add WebsocketSourceTlsValidator (validator.Object) on the tls nested block to address code-review feedback: - mode=enabled: reject crt_file, ca_file, key_file, key_pass_key if set, preventing silent field drops and perma-diffs in plans. - mode=with_client_cert: require crt_file to be non-empty, matching the OpenAPI oneOf shape. Mirrors the SocketFramingValidator pattern already used in this package (object-level validator on NestedBlockObject.Validators). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ae9535e2c
ℹ️ 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".
| if !exists || attr.IsNull() || attr.IsUnknown() { | ||
| return false |
There was a problem hiding this comment.
When mode = "with_client_cert" and crt_file is derived from another resource output, Terraform can pass it to validators as unknown during planning; treating unknown as unset here makes the later !isSet("crt_file") check fail the plan even though the value may be known by apply. Provider validators should return without diagnostics for unknown values, so this blocks valid configurations that compute the client certificate path dynamically.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. The validator no longer treats unknown values as unset — it now skips validation for any attribute that is unknown at plan time. with_client_cert only errors on a missing crt_file when it's known-and-empty/null (an unknown crt_file, e.g. from another resource's output, passes plan and is checked at apply); enabled only rejects cert fields that are known-and-non-empty. Mirrors the IsUnknown() skip pattern used by ExactlyOneProcessorValidator.
… planning When cert fields (e.g. crt_file) are derived from another resource's output they are UNKNOWN at plan time. The previous isSet helper treated unknown the same as null/missing, which caused the with_client_cert "crt_file required" check to fire during planning even though the value would be known at apply. Replace the single isSet helper with two explicit helpers: - isKnownAndSet: returns true only for known, non-empty values (used for the enabled mode "must not be set" checks — unchanged behaviour). - isKnownAndMissing: returns false for unknown values (skips the check), true for null/empty known values (used for the with_client_cert "required" check). This prevents false-positive plan errors on dynamic configs. Addresses code-review feedback about unknown values during planning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4d36c3cc9
ℹ️ 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".
| func ExpandWebsocketSource(src *WebsocketSourceModel, id string) (datadogV2.ObservabilityPipelineConfigSourceItem, diag.Diagnostics) { | ||
| var diags diag.Diagnostics | ||
|
|
||
| s := datadogV2.NewObservabilityPipelineWebsocketSourceWithDefaults() |
There was a problem hiding this comment.
This new source is wired directly to datadogV2.NewObservabilityPipelineWebsocketSourceWithDefaults() and related ObservabilityPipelineWebsocketSource* symbols, but the pinned github.com/DataDog/datadog-api-client-go/v2 in this repo does not export those symbols (go doc ...ObservabilityPipelineWebsocketSource reports no symbol). As committed, any build or test of the provider fails before users can use existing resources, so the provider needs the regenerated client/go.mod update in the same change or this code must be gated until that dependency lands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is expected and called out in the PR description. The WebSocket types (ObservabilityPipelineWebsocketSource*) are generated from the API specification, which is a separate change that hasn't been released yet — so the pinned datadog-api-client-go doesn't export them and the build won't pass until the client is regenerated and the go.mod bumped. That's why this PR is in draft: it's staged to go green once the generated client lands. The provider code here is written against the finalized type/field names so it compiles as soon as the client is updated. Not gating/removing in this PR, since the dependency is part of the planned rollout.
1 participant
Summary
Adds the Observability Pipelines WebSocket source — a new
websocketsource under thedatadog_observability_pipelineresource.Model/Expand/Flatten/Schemafollowing the existing source pattern.none,basic,bearer,custom.enabled(TLS without a client certificate) orwith_client_cert(mutual TLS;crt_filerequired, optionalca_file/key_file/key_pass_key).bytes/gelf/json/syslog.Dependency
This uses
datadog-api-client-gotypes for the WebSocket source that are generated from the API specification. Until that spec change is released and the client is regenerated,go buildfails on the not-yet-generated types — so this stays a draft until the client is updated, after which it compiles and the acceptance test can run.Test plan
go build ./...once the regenerated client lands🤖 Generated with Claude Code