Skip to content

Serialize models with exclude_unset, not exclude_defaults#208

Open
negz wants to merge 1 commit into
crossplane:mainfrom
negz:best-left-unset
Open

Serialize models with exclude_unset, not exclude_defaults#208
negz wants to merge 1 commit into
crossplane:mainfrom
negz:best-left-unset

Conversation

@negz
Copy link
Copy Markdown
Member

@negz negz commented Jun 4, 2026

Fixes #207

resource.update and resource.update_status serialized Pydantic models with model_dump(exclude_defaults=True), which asks "is this field different from its default?" and includes it if it is. The correct question for server-side apply is "did the caller set this field?", which exclude_unset answers.

exclude_defaults also regressed with newer datamodel-code-generator, which emits object defaults as a raw dict with validate_default=True instead of a default_factory. The default is validated into a model instance at construction, which doesn't compare equal to the declared dict default, so exclude_defaults fails to exclude it. Unset fields like spec.providerConfigRef then leaked into every composed resource.

exclude_unset is immune to how a default is represented (see crossplane/cli#64 (comment)). A field the caller didn't touch is absent from model_fields_set. It also keeps fields the caller explicitly set to their default value, which is more correct under server-side apply, where setting a field claims ownership of it.

The tests cover both datamodel-code-generator styles: the existing S3 Bucket model (old default_factory object defaults) and a new IAM AccountAlias model (newer validate_default=True object defaults). The AccountAlias case is the one that fails under exclude_defaults.

I have:

@negz negz requested a review from bobh66 as a code owner June 4, 2026 00:33
resource.update and update_status serialized Pydantic models with
model_dump(exclude_defaults=True), which asks "is this field different
from its default?". The correct question for server-side apply is "did
the caller set this field?", which exclude_unset answers.

exclude_defaults also regressed with newer datamodel-code-generator. It
emits object defaults as a raw dict with validate_default=True instead
of a default_factory. The default is validated into a model instance at
construction, which doesn't compare equal to the declared dict default,
so exclude_defaults fails to exclude it. Unset fields like
spec.providerConfigRef then leaked into every composed resource.

exclude_unset is immune to how a default is represented: a field the
caller didn't touch is absent from model_fields_set. It also keeps
fields the caller explicitly set to their default value, which is more
correct under server-side apply, where setting a field claims ownership
of it.

The apiVersion and kind workaround stays. Functions build models with
kwargs and rarely pass these, so they're unset and excluded either way.

See crossplane#207 for
more detail.

Signed-off-by: Nic Cope <[email protected]>
@negz negz force-pushed the best-left-unset branch from 8b04926 to fa72dd0 Compare June 4, 2026 00:39
@negz
Copy link
Copy Markdown
Member Author

negz commented Jun 4, 2026

I just pinned a fairly huge Python function project to use this commit and I'm not seeing anything concerning. As expected my tests need a few updates for cases where the function code explicitly set a field to its defaults. Before this change those fields would be omitted on serialization - now they're emitted.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Serialize models with exclude_unset, not exclude_defaults

1 participant