Skip to content

Serialize models with exclude_unset, not exclude_defaults #207

@negz

Description

@negz

What happened?

resource.update (and resource.update_status) serialize Pydantic models with model_dump(exclude_defaults=True). I think exclude_unset=True is more correct, and a recent change to the generated models has turned this from an academic distinction into a behavioural regression.

Some background on how we got here. When I added resource.update in #98 I reached for exclude_defaults to stop model_dump serializing every optional field the function didn't set as an explicit None. That mostly worked, but it was too aggressive: in #114 we found it also strips apiVersion and kind when they equal their default, which for a generated model they always do, so we re-add them by hand. I floated switching to exclude_none and dropping the defaults from the generated models instead, but @haarchri pointed out that'd cost you the default values in editor/LSP completion, so we kept exclude_defaults plus the workaround. We didn't consider exclude_unset as an option.

The Crossplane CLI generates these models with datamodel-code-generator, and we're bumping the pinned image from a very old 0.31.2 to 0.59.0 to fix crossplane/cli#63. Along the way the generator changed how it emits a field with an object default. Before:

providerConfigRef: Optional[ProviderConfigRef] = Field(
    default_factory=lambda: ProviderConfigRef.model_validate(
        {'kind': 'ClusterProviderConfig', 'name': 'default'}
    )
)

After:

providerConfigRef: ProviderConfigRef | None = Field(
    {'kind': 'ClusterProviderConfig', 'name': 'default'}, validate_default=True
)

That change is deliberate upstream: see koxudaxi/datamodel-code-generator#2228.

The trouble is how that interacts with exclude_defaults. The two forms behave differently:

  • default_factory: an unset field is recognized as equal to its default, so exclude_defaults=True omits it. The composed resource has no spec.providerConfigRef.
  • raw default + validate_default=True: the default is validated into a ProviderConfigRef instance at construction. exclude_defaults compares that instance against the field's declared default, a plain dict. They're not equal, so the field is not excluded.

So every composed MR now carries an explicit spec.providerConfigRef: {kind: ClusterProviderConfig, name: default} that it previously omitted. Same goes for any field with an object default the function didn't set.

In most cases this isn't a problem. We generate the Pydantic models from the same OpenAPI schemas the Kubernetes API server uses, so if a function emits the default providerConfigRef it's just explicitly specifying what the API server would've defaulted anyway. So it's unlikely merging crossplane/cli#64 would break anyone.

Semantically though, I think exclude_unset is the correct option. We serialize these models into a RunFunctionResponse as desired resources, and Crossplane treats desired resources as server-side apply fully-specified intent. Under SSA the caller should only specify the fields it has an opinion about and wants to own. If a function doesn't explicitly want providerConfigRef.name=default, it shouldn't include it. Emitting it claims ownership of a field the function doesn't actually care about. That could mean spurious field-manager conflicts if some other entity does care about it, and it pins the field to the schema default even where the effective server-side default might differ (dynamic defaulting, admission webhooks, version skew between the model and the API server).

The distinction is:

  • exclude_defaults asks "is this field's value different from its default?"
  • exclude_unset asks "did the caller set this field?"

exclude_unset matches the SSA intent: emit exactly the fields the function set, own those, leave the rest to the API server. It's also immune to the codegen change, because it doesn't care how a default is represented. A field the function didn't touch is absent from model_fields_set. That fixes the providerConfigRef regression for object defaults, scalar defaults, and any future representation the generator produces.

It is a behaviour change though, so we should think through the implications.

First, apiVersion and kind. exclude_unset does not let us drop the manual re-add from #114. When a function builds a model with kwargs (the common case) it doesn't pass apiVersion/kind, so they're not in model_fields_set and exclude_unset omits them too. The workaround stays necessary either way.

Second, fields a function sets to their default value. Say a function writes autoCreate=False where False is the schema default. Today exclude_defaults drops it; exclude_unset keeps it. I'd argue that's more correct. If you set it, you have an opinion and should own it.

Third, snapshot tests. Any function whose tests assert the exact RunFunctionResponse could see diffs if fields start appearing that exclude_defaults used to strip. (i.e. Fields the user explicitly set to the default value.)

How can we reproduce it?

Hand-write a model with an object-typed field that has a default, mirroring what the newer datamodel-code-generator emits, and dump it without setting that field:

from pydantic import BaseModel, Field

class ProviderConfigRef(BaseModel):
    kind: str | None = None
    name: str

class Spec(BaseModel):
    providerConfigRef: ProviderConfigRef | None = Field(
        {"kind": "ClusterProviderConfig", "name": "default"}, validate_default=True
    )

print(Spec().model_dump(exclude_defaults=True))
# {'providerConfigRef': {'kind': 'ClusterProviderConfig', 'name': 'default'}}
print(Spec().model_dump(exclude_unset=True))
# {}

The exclude_defaults dump includes a field the caller never set; exclude_unset doesn't.

What environment did it happen in?

Function version: n/a — this is about resource.update/update_status in the SDK itself. The regression surfaces for anyone using models generated by datamodel-code-generator >= 0.54.0, where the validate_default=True change landed, which the Crossplane CLI will produce once crossplane/cli#64 merges.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions