feat(config): add mutually_exclusive for optional exclusive field groups - #26185
feat(config): add mutually_exclusive for optional exclusive field groups#26185Ash20pk wants to merge 9 commits into
mutually_exclusive for optional exclusive field groups#26185Conversation
…roups The example-config generator emits a value for every field with `docs::examples` metadata, with no notion of "at most one of these". For fields that are mutually exclusive but not required, that produces an example config the component's own validation rejects, failing `check-generated-docs`. `required_one_of` can't express this: it means "exactly one must be set", which is wrong when omitting the whole group is valid. It also emits a JSON Schema `oneOf` constraint that would make the group required. Add a `mutually_exclusive = "group"` field attribute alongside `required_one_of`. It shares the group collection and validation logic but emits no schema constraint, since JSON Schema can't express "at most one" without also requiring the group. Instead it annotates `docs::mutually_exclusive` metadata so the docs render an "At most one of ... can be set." note and the example generator emits only one member of the group. Unlike `required_one_of`, the chosen member is not forced into the example: it's subject to the variant's normal filter, so a minimal example correctly emits none of them. Apply it to the `axiom` sink's `url`/`region` and restore `url`'s `docs::examples`, which had to be stripped as a workaround.
The `url` option's examples are restored in the generated Cue, both endpoint fields carry the `mutually_exclusive` group annotation, and the advanced example now demonstrates `url` instead of `region`. The minimal example is unchanged: neither field is required, and "at most one" permits none.
`vector validate --no-environment` does not interpolate `${AXIOM_URL}`, and the
example generator emits the first example, so the concrete URL must come first to
satisfy the field's `uri` format validation.
|
All contributors have signed the CLA ✍️ ✅ |
github-actions
Bot
added
domain: sinks
|
I have read the CLA Document and I hereby sign the CLA |
![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: 59cda07037
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let (_, excluded) = split_exclusive_groups(params, "mutually_exclusive", |param| { | ||
| filter(param) && get_example_value(param, |_| false).is_some() | ||
| }); |
There was a problem hiding this comment.
When a mutually exclusive group belongs to a struct nested inside an array or map, params contains only the parent field, so this top-level call never sees the annotated members. get_example_value later includes each nested option that passes deep_filter, meaning an advanced example can still emit both members when both carry examples and produce a configuration that the component validation rejects. The group selection needs to be applied recursively while constructing nested objects.
Useful? React with 👍 / 👎.
The top-level scan in `make_example_params` only sees the parent field when an exclusive group lives in a struct nested inside an array or map, so the nested object was built from every option passing `deep_filter` and could set two members of one group. No component hits this today (every group is currently a top-level field, and axiom's are flattened), and `required_one_of` had the same gap, but nothing stops a future group from being declared in a nested struct. Filter both group kinds while building the nested object. This only drops members, never forces one, so a group whose chosen member yields no value contributes nothing rather than an invalid partial object.
|
Good catch — fixed in e574eb9. Two clarifications on the framing, since they affect how urgent this is:
Worth fixing regardless, since the point of this PR is to make future mutually-exclusive-but-optional fields safe and nothing stops a group from being declared in a nested struct. The fix filters both group kinds while building the nested object, via a shared Added
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e574eb925b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let group_pushes = groups | ||
| .iter() | ||
| .filter(|((kind, _), _)| *kind == ExclusiveKind::RequiredOneOf) |
There was a problem hiding this comment.
For consumers that validate configurations against Vector's generated JSON Schema, filtering these groups out makes {url: ..., region: ...} pass schema validation even though AxiomConfig::validate rejects it. JSON Schema can represent “at most one” without requiring any member—for example, by negating each pair of simultaneously required, non-null properties—so MutuallyExclusive groups should emit that constraint rather than remaining documentation-only.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
You're right that JSON Schema can express this, and my justification was wrong — I said "at most one" can't be expressed without making the group required, which isn't true. Negating each pair of simultaneously-set, non-null members does exactly that. I've corrected the code comments and the PR description in ed589f3 to state the real reason.
The real reason is scope: #26175 specifies this as docs-only metadata, "with no JSON-Schema oneOf constraint". So a schema constraint is a deliberate omission here, not an oversight about the format.
I've held off implementing it rather than expanding scope unilaterally, for two reasons:
- It changes what the published schema accepts, which is a call for a maintainer — today
{url: ..., region: ...}already passes schema validation foraxiom, so this PR isn't a regression there. schema_resolve.rsspecial-cases_required_one_of_constraintto skip constraint subschemas during enum detection. Anot/allOfsubschema pushed intoflattened_subschemaswould need equivalent handling, and getting it wrong risks shifting generated docs for every component with a flattened field. That deserves its own change with its own docs regeneration diff to review, rather than riding along here.
Happy to implement it in this PR if you'd prefer — the encoding is straightforward, and it'd need a matching skip in schema_resolve.rs plus a smoke test asserting the constraint doesn't make the group required. Otherwise I'd suggest a follow-up issue, since it applies to any future mutually_exclusive group rather than just axiom.
@thomasqueirozb your call, since you scoped the original issue.
There was a problem hiding this comment.
Implemented in 0103dd7 — you were right, and I've dropped my earlier scope argument.
The encoding negates the union of every pair of simultaneously-set members, which forbids setting two without requiring any:
not: { anyOf: [ { allOf: [ set(a), set(b) ] }, ... ] }
where set(x) is {required: [x], properties: {x: {not: {type: "null"}}}}, reusing the same present-and-non-null matching as the existing required_one_of entries so an explicit null counts as unset.
On the docs-pipeline risk I mentioned: it was real, but smaller than I thought. The constraint has no instance type of its own, so it doesn't hit enum resolution at all — it falls through to the unconstrained case and would have added a spurious * wildcard option. Fixed with a _mutually_exclusive_constraint marker. That skip now covers both constraint kinds and moved to the top of resolve_bare_schema, replacing the required_one_of check that only ran for one-of/any-of schemas.
Verification, against the actual output of vector generate-schema rather than just the emitted shape:
| config | before | after |
|---|---|---|
neither url nor region |
valid | valid |
url only |
valid | valid |
region only |
valid | valid |
| both | valid | rejected |
url: null + region |
valid | valid |
So the asymmetry you flagged is closed: the schema now agrees with AxiomConfig::validate, and the group is still not required. I also checked the pairwise encoding at n=3 — every pair is rejected, not just the first.
Two smoke tests cover it: one asserting the emitted constraint with no top-level required, and one asserting all three pairs are present for a three-member group. Note this changes a mutually_exclusive struct's schema from a plain object to an allOf, same as required_one_of already does.
Regenerating the docs produces no diff, and cargo vdev check component-examples still reports 248 validated / all passed.
| { | ||
| // Nested structs carry their own exclusive groups, so filter them here too: | ||
| // emitting two members of one group would produce an invalid config. | ||
| let excluded = excluded_group_members(options, &deep_filter); |
There was a problem hiding this comment.
When a nested required_one_of group has multiple members accepted by deep_filter, this call chooses the first one without checking whether get_value can actually render it. For example, if the first member is relevant/minimal but has no default, example, or enum while a later member has an example, the first produces no value and the later member is excluded; if another option keeps the parent object present, the generated configuration then violates the nested exactly-one constraint. Select the nested candidate using both deep_filter and renderability, as the top-level selection does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ed589f3. This one was a real hole in the nested filtering I added — selection used deep_filter alone, so an unrenderable first member would be chosen, excluding the rest and leaving a nested required_one_of group with nothing set. Invalid in the same way as setting two, and it made my earlier "only drops members, never forces one" claim not actually safe for the exactly-one case.
Selection now requires renderability too, mirroring the top-level logic. Nested object construction moved into build_nested_object to stay under the clippy::pedantic line limit.
Covered by nested_group_skips_unrenderable_member, where the first member has no default/example/enum and the second has an example. I confirmed it catches the bug by stubbing renders_nested_value to always return true — the test fails without the renderability check.
Nested group selection used `deep_filter` alone. If the chosen member had no default, example, or enum it rendered nothing while still excluding the others, leaving a nested `required_one_of` group with no member set — invalid in the same way as setting two. Require renderability as well, mirroring the top-level selection. The nested object construction moves into `build_nested_object` to keep `get_example_value` under the pedantic line limit. Also corrects the stated reason `mutually_exclusive` emits no schema constraint. "At most one" is expressible in JSON Schema by negating each pair of simultaneously-set members; omitting it is a deliberate scope decision, not a limitation of the format.
…clusive groups
Previously these groups were documentation-only, so a config setting two members
passed JSON Schema validation even though the component's own validation rejects
it. Consumers validating against Vector's published schema saw no constraint.
`oneOf` can't be reused here because it also makes the group required. Instead,
negate the union of every pair of simultaneously-set members:
not: { anyOf: [ { allOf: [ set(a), set(b) ] }, ... ] }
This forbids setting two without requiring any, so an empty object still
validates. Members are matched as present-and-non-null, so an explicit null is
treated as unset, matching the existing required_one_of entries and serde's
Option<T> semantics.
The constraint carries a _mutually_exclusive_constraint marker so the docs
pipeline skips it. It has no instance type of its own, so it would otherwise fall
through to the unconstrained case and add a spurious wildcard option. That skip
now covers both constraint kinds and moves to the top of resolve_bare_schema,
replacing the required_one_of check that only ran for one-of/any-of schemas.
Verified against the generated schema: for the axiom sink, setting both url and
region is now rejected while setting neither still validates. Regenerating the
docs produces no diff.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0103dd7866
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for (key, option) in options | ||
| .iter() | ||
| .filter(|(key, option)| deep_filter(option) && !excluded.contains(key.as_str())) |
There was a problem hiding this comment.
When a nested object is emitted in a minimal example but none of its required_one_of members passes deep_filter (for example, all members are optional and only carry examples), split_exclusive_groups chooses a fallback member, but this filter still removes that chosen member along with every other member. The resulting object therefore contains no member of an exactly-one group—or the required parent is omitted entirely—producing a generated configuration that fails its schema or component validation. Nested required_one_of selection needs to force its chosen renderable member just as the top-level path does, while mutually_exclusive should remain filter-dependent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7a67d67 — you're right, and this is the flip side of ed589f3.
build_nested_object only ever permitted the chosen member, never forced it, so the loop's own deep_filter could still drop it. In a minimal example deep_filter is selected_for_minimal, so a nested required_one_of group of plain Option members has nothing pass usable, split_exclusive_groups falls back to members[0], and that fallback gets filtered out alongside the rest. The group ends up with no member set — invalid in exactly the way setting two is.
In ed589f3 I made the nested path drop-only, reasoning it was "safe wherever a group cannot be forced in". That holds for mutually_exclusive, where none is a valid outcome, but it left the exactly-one case with no way to satisfy itself. The shared excluded_group_members helper was the wrong abstraction: it treated the two kinds as symmetric when the whole point of this PR is that they aren't.
The two are now handled separately in build_nested_object. required_one_of picks on renderability alone and forces its member past the filter, mirroring make_example_params; mutually_exclusive still requires deep_filter && renders, so a minimal example correctly emits none. excluded_group_members is gone.
One deliberate limit on the forcing, which I'd flag as the judgement call here: forcing does not create an object. If the nested object is empty on its own account, the chosen member is not added and get_example_value falls back to first_example(item_type) as it does today. Otherwise a nested struct whose only content is an unfilterable group would start emitting an object where the item's own example is used now, shifting output for fields that are already valid — a wider change than the bug warrants. So the forcing applies to the case you described (a nested object that is emitted anyway and would carry no group member), not to whether the object appears at all.
Same reachability note as the original nested finding: every required_one_of group in the tree (gcp_chronicle, azure_blob, elasticsearch, remap, sample) sits on a top-level component config rather than a struct nested inside an array or map field, so no generated example is wrong today. Worth fixing anyway, since the nested path exists to hold for groups declared anywhere.
Three tests added: nested_required_one_of_forces_a_member (minimal example, no member passes deep_filter — the case you described), nested_mutually_exclusive_stays_filter_dependent (same shape under at-most-one, asserting no member is forced), and forced_nested_member_does_not_create_an_object. I checked each fails without the fix — stubbing the forced list to empty fails the first, and disabling the empty-object guard fails the third. nested_group_skips_unrenderable_member from ed589f3 passes unmodified, which was the regression risk in splitting the two paths.
cargo test -p vdev 128 passed, cargo vdev check component-examples 248 validated / all passed, make check-generated-docs clean with no diff, clippy and fmt clean.
`build_nested_object` only permitted the chosen group member, never forced it, so the loop's own `deep_filter` could drop it. For a minimal example `deep_filter` is `selected_for_minimal`, so a nested `required_one_of` group of optional members had nothing pass, `split_exclusive_groups` fell back to the first member, and that fallback was filtered out alongside the rest — leaving the group with no member set, which its exactly-one constraint rejects just as setting two would. The two kinds are now handled separately rather than sharing `excluded_group_members`: `required_one_of` forces its chosen renderable member past the filter, mirroring `make_example_params`, while `mutually_exclusive` stays filter-dependent so a minimal example still correctly emits no member. Forcing is gated on the object being non-empty on its own account, so a nested struct whose sole content is an unfilterable group keeps falling back to the item's own example instead of materializing an object.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c87519bd1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if object.is_empty() { | ||
| return Some(object); |
There was a problem hiding this comment.
When a required array/object parameter contains a nested struct whose only renderable fields are optional required_one_of members, the minimal filter leaves object empty and this return runs before the selected member is forced. If the item schema has no explicit example to fall back to, get_example_value returns None, omitting the required parent and generating a configuration that violates the schema. The follow-up added this early return, which is fresh evidence that the earlier issue remains in the no-item-example branch; force the chosen member when no fallback example exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b9357e7 — you're right, and the early return I added in 7a67d67 was too blunt.
The guard was meant to stop forcing from creating an object where get_example_value would otherwise use the item's own example, preserving today's output. But that only holds when such an example exists. With no first_example(item_type) to fall back to, returning an empty object means get_example_value yields None and the field is dropped from the example altogether — so a required parent disappears, which is worse than the case the guard was protecting.
build_nested_object now takes has_item_example and prefers the fallback only when there is one. Without a fallback there is nothing to preserve, so the chosen member is forced in and the parent stays present. The caller already had item_type in hand, so this is just first_example hoisted above the call and reused for both the flag and the fallback branch.
One correction to the framing: the early return isn't what introduced this. Before 7a67d67 the object was empty in exactly this scenario too — no member passed deep_filter, nothing was forced, and get_example_value returned None the same way. The guard declined to fix that sub-case rather than creating it. Reachability is also unchanged: every required_one_of group in the tree sits on a top-level component config, not on a struct nested inside an array or map field, so no generated example is affected today.
forced_nested_member_keeps_a_required_parent covers it — same shape as forced_nested_member_does_not_create_an_object but with no example on the item, asserting the required parent survives with one member set. I confirmed it fails with the guard restored to its unconditional form, and that the sibling test still pins the fallback-preferring behaviour when an example does exist.
Verification: cargo test -p vdev 129 passed, cargo vdev build component-examples regenerates with no diff, clippy and fmt clean. I could not run cargo vdev check component-examples this time — building the vector binary for the validation sweep ran out of disk on my machine, so that one is down to CI rather than something I have confirmed locally.
The empty-object guard from 7a67d67 returned before forcing a `required_one_of` member whenever nothing else kept the nested object present. That is right when the item carries its own example, which is what `get_example_value` falls back to, but wrong when it does not: the fallback yields nothing, `get_example_value` returns `None`, and the field is dropped from the example entirely — omitting it even when it is required. `build_nested_object` now takes `has_item_example` and only prefers the fallback when one actually exists. Without a fallback there is nothing to preserve, so the chosen member is forced in and the parent stays present.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9357e7009
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let both_set = ::vector_config::schema::generate_any_of_schema(&[#(#pairs),*]); | ||
| let mut constraint = ::vector_config::schema::SchemaObject { | ||
| subschemas: Some(Box::new(::vector_config::schema::SubschemaValidation { | ||
| not: Some(Box::new(::vector_config::schema::Schema::Object(both_set))), |
There was a problem hiding this comment.
When an object sets two group members plus any unrelated property, the generated root schema accepts it. DisallowUnevaluatedPropertiesVisitor only unmarks allOf/oneOf/anyOf children, not not, so the inner both_set anyOf receives unevaluatedProperties: false; an unrelated property makes that inner schema fail and therefore makes this not succeed. This affects the actual Axiom shape because valid configurations also contain fields such as dataset and token, defeating the published schema constraint while runtime validation still rejects the configuration.
Useful? React with 👍 / 👎.
1 participant
Summary
The example-config generator emits a value for every field carrying
docs::examples, with no notion of "at most one of these". For fields that are mutually exclusive but not required, that produces an example config the component's own validation rejects — which is what brokecheck-generated-docsfor theaxiomsink in #26171.required_one_ofcan't express this. It means "exactly one must be set": it emits a JSON-SchemaoneOfconstraint and an "Exactly one of X or Y must be set." docs note, both wrong when omitting the whole group is valid.This implements option 2 from #26175 — the generator-level fix. Rather than adding a parallel mechanism, it generalizes the existing
required_one_ofmachinery so both flavors share the validation checks, group collection, and metadata injection, with anExclusiveKinddistinguishing them:oneOfcan't be reused, since it would also make the group required. Instead the constraint negates the union of every pair of simultaneously-set members (not: {anyOf: [{allOf: [set(a), set(b)]}, ...]}), forbidding two without requiring any. Verified against the realvector generate-schemaoutput: setting bothurlandregionis now rejected, setting neither still validates. It carries a_mutually_exclusive_constraintmarker so the docs pipeline skips it.urlorregioncan be set." alongside the existing "Exactly one … must be set."required_one_of, the chosen member is not forced into the example. It passes through the variant's normal filter, so a minimal example correctly emits neither field.required_one_ofalready had.Applied to the
axiomsink'surl/region, which letsurl'sdocs::examplesbe restored — the docs regression called out in the issue. Its examples are ordered concrete-URL-first, because the generator emits the first example andvector validate --no-environmentdoes not interpolate${AXIOM_URL}, which would fail that field'suriformat validation.One deviation from the issue: it framed this as docs-only metadata with no schema constraint. Codex review pointed out that "at most one" is expressible without requiring the group, and that leaving it out meant the published schema accepted configs
AxiomConfig::validaterejects. That was a good catch, so the constraint is included; see the review thread.Option 1 (deprecating the flattened fields for a non-flattened
endpointenum) is left for a follow-up. This change is what makes generated examples stay valid during that deprecation window, and for any future mutually-exclusive-but-optional field.References
Closes: #26175
Related: #26171
Vector configuration
The regenerated
axiomadvanced example, which previously set bothurlandregion:minimal.yamlis byte-identical to before: neither field is required, and "at most one" permits none.How did you test this PR?
Tests:
cargo test -p vdev— 123 passed, including 3 new cases incomponent_examples: one member emitted for an advanced example, none for a minimal example, and a regression test assertingrequired_one_ofstill forces a member.cargo test -p vector-config --test integration— 13 passed, including 2 new cases asserting nooneOfconstraint is generated and that both members carry the group metadata. The pre-existingrequired_one_of_generates_one_of_constrainttest passes unmodified, which was the main regression risk in sharing the code paths.cargo test -p vector --lib sinks::axiom— 12 passed.One note for reviewers:
region's hand-written "Cannot be used together withurl." now sits next to the auto-generated "At most one of …" note, so the rendered docs are mildly redundant. I left the prose alone to keep the diff tight, but happy to trim it if you'd prefer.Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.