Skip to content

fix(config): accept omitted flattened optional internally-tagged enums - #26248

Open
bruceg wants to merge 1 commit into
masterfrom
bruceg/configurable-correctness
Open

fix(config): accept omitted flattened optional internally-tagged enums#26248
bruceg wants to merge 1 commit into
masterfrom
bruceg/configurable-correctness

Conversation

@bruceg

@bruceg bruceg commented Aug 27, 2026

Copy link
Copy Markdown
Member

Summary

Generated JSON Schema treated #[serde(flatten)] Option<InternallyTaggedEnum> as required. generate_optional_schema encodes optionality as oneOf: [null, T], which is correct for a nullable property, but flatten merges that subschema into the parent object via allOf. The value under validation is never JSON null, so the null branch was dead and the present branch was the only reachable one.

Rewrite that branch after single-use $ref inlining, scoped to allOf members only: replace null with not: { required: [<tag field>] } so omission matches serde. Shared optional $refs are cloned into the flatten site so a plain Option<T> property still accepts JSON null.

References

Vector configuration

N/A

How did you test this PR?

PR includes a set of snapshots showing correct schema outputs.

Is this a breaking change?

  • Yes
  • No

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on our guidelines.
  • No. A maintainer will apply the no-changelog label to this PR.

Contributor Guidelines

  • Please read our Vector contributor resources.
  • Do not hesitate to use @vectordotdev/vector to reach out to us regarding this PR.
  • Some CI checks run only after we manually approve them. To catch issues early, add a pre-push hook (template) or run the following locally before pushing:
    • make fmt
    • make check-clippy (auto-fix with make clippy-fix)
    • make test
  • After a review is requested, please avoid force pushes to help us review incrementally.
    • Feel free to push as many commits as you want. They will be squashed into one before merging.
    • For example, you can run git merge origin master and git push.
  • If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
    run make build-licenses to regenerate the license inventory and commit the changes (if any). More details on the dd-rust-license-tool.

@bruceg
bruceg requested a review from a team as a code owner August 27, 2026 14:59
@bruceg bruceg added domain: config Anything related to configuring Vector type: bug A bug fix labels Aug 27, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b0f8e1f75b

ℹ️ 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".

Comment thread lib/vector-config/src/schema/visitors/flattened_optional.rs Outdated
Comment thread lib/vector-config/src/schema/visitors/flattened_optional.rs Outdated
Comment thread lib/vector-config/src/schema/visitors/flattened_optional.rs Outdated
@bruceg
bruceg force-pushed the bruceg/configurable-correctness branch from b0f8e1f to 0b007d8 Compare August 27, 2026 19:06
@github-actions github-actions Bot added the domain: vdev Anything related to the vdev tooling label Aug 27, 2026
@bruceg

bruceg commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b007d86cf

ℹ️ 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".

Comment thread lib/vector-config/tests/integration/schema_snapshots.rs
Generated JSON Schema treated `#[serde(flatten)] Option<InternallyTaggedEnum>`
as required. `generate_optional_schema` encodes optionality as `oneOf: [null, T]`,
which is correct for a nullable property, but flatten merges that subschema into
the parent object via `allOf`. The value under validation is never JSON `null`,
so the null branch was dead and the present branch was the only reachable one.

Rewrite that branch after single-use `$ref` inlining, scoped to `allOf` members
only: replace `null` with `not: { required: [<tag field>] }` so omission matches
serde. Shared optional `$ref`s are cloned into the flatten site so a plain
`Option<T>` property still accepts JSON `null`.
@bruceg
bruceg force-pushed the bruceg/configurable-correctness branch from 0b007d8 to 7b65721 Compare August 31, 2026 17:14
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-31T17:19:39.740016Z 7b65721 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: config Anything related to configuring Vector domain: vdev Anything related to the vdev tooling type: bug A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant