Serialize models with exclude_unset, not exclude_defaults

crossplane/function/resource.py

@@ -40,12 +40,21 @@ def update(r: fnv1.Resource, source: dict | structpb.Struct | pydantic.BaseModel
     """
     match source:
         case pydantic.BaseModel():
-            data = source.model_dump(exclude_defaults=True, warnings=False)
-            # In Pydantic, exclude_defaults=True in model_dump excludes fields
-            # that have their value equal to the default. If a field like
-            # apiVersion is set to its default value 's3.aws.upbound.io/v1beta2'
-            # (and not explicitly provided during initialization), it will be
-            # excluded from the serialized output.
+            # exclude_unset emits only the fields the caller explicitly set.
+            # Crossplane treats desired resources as server-side apply intent,
+            # so a function should own exactly the fields it has an opinion
+            # about and leave the rest to the API server.
+            #
+            # by_alias emits each field under its alias, which is its real
+            # wire name. datamodel-code-generator aliases fields whose KRM
+            # name collides with a Python keyword or builtin (e.g. it emits a
+            # bool_ attribute aliased to bool, continue_ aliased to continue).
+            # Without by_alias those fields serialize under the Python name and
+            # don't match the resource's schema. It's a no-op for ordinary
+            # fields, which have no alias.
+            data = source.model_dump(exclude_unset=True, by_alias=True, warnings=False)
+            # apiVersion and kind identify the resource but are rarely passed
+            # as kwargs, so they're usually unset. Add them back explicitly.
             data["apiVersion"] = source.apiVersion
             data["kind"] = source.kind
             r.resource.update(data)
@@ -71,11 +80,12 @@ def update_status(
         status: The status to set, as a dictionary or Pydantic model.
 
     Sets ``r.resource.status`` from the supplied status. When the status
-    is a Pydantic model, fields set to their default value are excluded,
+    is a Pydantic model, fields the caller didn't explicitly set are
+    excluded and aliased fields are emitted under their wire names,
     matching the behavior of :func:`update`.
     """
     if isinstance(status, pydantic.BaseModel):
-        status = status.model_dump(exclude_defaults=True, warnings=False)
+        status = status.model_dump(exclude_unset=True, by_alias=True, warnings=False)
     update(r, {"status": status})
 
 

tests/test_resource.py

@@ -22,7 +22,11 @@
 
 import crossplane.function.proto.v1.run_function_pb2 as fnv1
 from crossplane.function import logging, resource
-from tests.testdata.models.io.upbound.aws.s3 import v1beta2
+from tests.testdata.models.io.k8s.api.resource import v1 as resourcev1
+from tests.testdata.models.io.upbound.aws.s3 import v1beta2 as s3v1beta2
+from tests.testdata.models.io.upbound.m.aws.iam.accountalias import (
+    v1beta1 as accountaliasv1beta1,
+)
 
 
 class TestResource(unittest.TestCase):
@@ -59,13 +63,43 @@ class TestCase:
                         {"apiVersion": "example.org", "kind": "XR"}
                     ),
                 ),
-                status=v1beta2.ForProvider(region="us-west-2"),
+                status=s3v1beta2.ForProvider(region="us-west-2"),
                 want={
                     "apiVersion": "example.org",
                     "kind": "XR",
                     "status": {"region": "us-west-2"},
                 },
             ),
+            TestCase(
+                reason="Fields the caller set should be kept, while unset "
+                "fields are omitted.",
+                r=fnv1.Resource(
+                    resource=resource.dict_to_struct(
+                        {"apiVersion": "example.org", "kind": "XR"}
+                    ),
+                ),
+                status=s3v1beta2.ForProvider(region="us-west-2", forceDestroy=False),
+                want={
+                    "apiVersion": "example.org",
+                    "kind": "XR",
+                    "status": {"region": "us-west-2", "forceDestroy": False},
+                },
+            ),
+            TestCase(
+                reason="Setting status from a Pydantic model with keyword-"
+                "aliased fields should emit the fields under their aliases.",
+                r=fnv1.Resource(
+                    resource=resource.dict_to_struct(
+                        {"apiVersion": "example.org", "kind": "XR"}
+                    ),
+                ),
+                status=resourcev1.DeviceAttribute(**{"bool": True}),
+                want={
+                    "apiVersion": "example.org",
+                    "kind": "XR",
+                    "status": {"bool": True},
+                },
+            ),
             TestCase(
                 reason="Setting status on an empty resource should work.",
                 r=fnv1.Resource(),
@@ -131,11 +165,16 @@ class TestCase:
                 ),
             ),
             TestCase(
-                reason="Updating from a Pydantic model should work.",
+                # This model uses the default_factory form that older
+                # datamodel-code-generator emits for fields with an object
+                # default. providerConfigRef has such a default but isn't set
+                # here, so it must not be emitted.
+                reason="Updating from a Pydantic model with default_factory "
+                "object defaults should omit unset fields.",
                 r=fnv1.Resource(),
-                source=v1beta2.Bucket(
-                    spec=v1beta2.Spec(
-                        forProvider=v1beta2.ForProvider(region="us-west-2"),
+                source=s3v1beta2.Bucket(
+                    spec=s3v1beta2.Spec(
+                        forProvider=s3v1beta2.ForProvider(region="us-west-2"),
                     ),
                 ),
                 want=fnv1.Resource(
@@ -148,6 +187,98 @@ class TestCase:
                     ),
                 ),
             ),
+            TestCase(
+                # This model uses the validate_default=True form that newer
+                # datamodel-code-generator emits for fields with an object
+                # default. providerConfigRef has such a default but isn't set
+                # here, so it must not be emitted.
+                reason="Updating from a Pydantic model with validate_default "
+                "object defaults should omit unset fields.",
+                r=fnv1.Resource(),
+                source=accountaliasv1beta1.AccountAlias(
+                    spec=accountaliasv1beta1.Spec(forProvider={"x": "y"}),
+                ),
+                want=fnv1.Resource(
+                    resource=resource.dict_to_struct(
+                        {
+                            "apiVersion": "iam.aws.m.upbound.io/v1beta1",
+                            "kind": "AccountAlias",
+                            "spec": {"forProvider": {"x": "y"}},
+                        }
+                    ),
+                ),
+            ),
+            TestCase(
+                # datamodel-code-generator can't name a field bool or int, so
+                # it emits bool_ aliased to bool and int_ aliased to int. The
+                # alias is the resource's real wire name, so update must emit
+                # fields under their aliases.
+                reason="Updating from a Pydantic model with keyword-aliased "
+                "fields should emit the fields under their aliases.",
+                r=fnv1.Resource(),
+                source=resourcev1.ResourceSlice(
+                    spec=resourcev1.Spec(
+                        devices=[
+                            resourcev1.Device(
+                                name="gpu",
+                                attributes={
+                                    "powered": resourcev1.DeviceAttribute(
+                                        **{"bool": True},
+                                    ),
+                                    "lanes": resourcev1.DeviceAttribute(
+                                        **{"int": 16},
+                                    ),
+                                },
+                            ),
+                        ],
+                    ),
+                ),
+                want=fnv1.Resource(
+                    resource=resource.dict_to_struct(
+                        {
+                            "apiVersion": "resource.k8s.io/v1",
+                            "kind": "ResourceSlice",
+                            "spec": {
+                                "devices": [
+                                    {
+                                        "name": "gpu",
+                                        "attributes": {
+                                            "powered": {"bool": True},
+                                            "lanes": {"int": 16},
+                                        },
+                                    },
+                                ],
+                            },
+                        }
+                    ),
+                ),
+            ),
+            TestCase(
+                # managementPolicies defaults to ["*"] and is set to ["*"]
+                # here. A field the caller sets is one it has an opinion about
+                # and should own, even when the value equals the default.
+                reason="A field the caller explicitly set to its default value "
+                "should be emitted.",
+                r=fnv1.Resource(),
+                source=accountaliasv1beta1.AccountAlias(
+                    spec=accountaliasv1beta1.Spec(
+                        forProvider={"x": "y"},
+                        managementPolicies=["*"],
+                    ),
+                ),
+                want=fnv1.Resource(
+                    resource=resource.dict_to_struct(
+                        {
+                            "apiVersion": "iam.aws.m.upbound.io/v1beta1",
+                            "kind": "AccountAlias",
+                            "spec": {
+                                "forProvider": {"x": "y"},
+                                "managementPolicies": ["*"],
+                            },
+                        }
+                    ),
+                ),
+            ),
         ]
 
         for case in cases:
@@ -158,6 +289,66 @@ class TestCase:
                 "-want, +got",
             )
 
+    def test_model_round_trip(self) -> None:
+        # A function reads an observed resource (wire names), validates it into
+        # a model, then writes it back via update. A field that goes in under
+        # its wire name must come back out under the same wire name. This pins
+        # the property the by_alias fix exists to guarantee: validation accepts
+        # the alias, and serialization must emit the alias, not the Python
+        # attribute name. It does not assert anything about fields the model
+        # doesn't define (pydantic drops them) or value types (Struct coerces
+        # numbers to float).
+        @dataclasses.dataclass
+        class TestCase:
+            reason: str
+            # The resource as it arrives from Crossplane, using wire names.
+            observed: dict
+            # The model type to validate the observed resource into.
+            model: type[pydantic.BaseModel]
+
+        cases = [
+            TestCase(
+                reason="A model with keyword-aliased fields should round-trip "
+                "through validation and update with its fields under the same "
+                "wire names (bool, int) they arrived under.",
+                observed={
+                    "apiVersion": "resource.k8s.io/v1",
+                    "kind": "ResourceSlice",
+                    "spec": {
+                        "devices": [
+                            {
+                                "name": "gpu",
+                                "attributes": {
+                                    "powered": {"bool": True},
+                                    "lanes": {"int": 16},
+                                    "model": {"string": "h100"},
+                                },
+                            },
+                        ],
+                    },
+                },
+                model=resourcev1.ResourceSlice,
+            ),
+            TestCase(
+                reason="A model with only ordinary fields should round-trip unchanged.",
+                observed={
+                    "apiVersion": "s3.aws.upbound.io/v1beta2",
+                    "kind": "Bucket",
+                    "spec": {"forProvider": {"region": "us-west-2"}},
+                },
+                model=s3v1beta2.Bucket,
+            ),
+        ]
+
+        for case in cases:
+            # Mimic the SDK flow: a function reads an observed resource (wire
+            # names), validates it into a model, then writes it back out.
+            m = case.model.model_validate(case.observed)
+            r = fnv1.Resource()
+            resource.update(r, m)
+            got = resource.struct_to_dict(r.resource)
+            self.assertEqual(case.observed, got, case.reason)
+
     def test_get_condition(self) -> None:
         @dataclasses.dataclass
         class TestCase:

tests/testdata/models/io/k8s/api/__init__.py

@@ -0,0 +1,2 @@
+# generated by datamodel-codegen:
+#   filename:  <stdin>

tests/testdata/models/io/k8s/api/resource/__init__.py

@@ -0,0 +1,2 @@
+# generated by datamodel-codegen:
+#   filename:  <stdin>

tests/testdata/models/io/k8s/api/resource/v1.py

@@ -0,0 +1,66 @@
+# generated by datamodel-codegen:
+#   filename:  <stdin>
+
+from __future__ import annotations
+
+from typing import Literal
+
+from pydantic import BaseModel, Field
+
+from ...apimachinery.pkg.apis.meta import v1
+
+
+class DeviceAttribute(BaseModel):
+    bool_: bool | None = Field(None, alias='bool')
+    """
+    BoolValue is a true/false value.
+    """
+    int_: int | None = Field(None, alias='int')
+    """
+    IntValue is a number.
+    """
+    string: str | None = None
+    """
+    StringValue is a string.
+    """
+    version: str | None = None
+    """
+    VersionValue is a semantic version according to semver.org spec 2.0.0.
+    """
+
+
+class Device(BaseModel):
+    name: str
+    """
+    Name is a unique identifier among all devices managed by the driver.
+    """
+    attributes: dict[str, DeviceAttribute] | None = None
+    """
+    Attributes defines the set of attributes for this device.
+    """
+
+
+class Spec(BaseModel):
+    devices: list[Device] | None = None
+    """
+    Devices lists the devices in this resource slice.
+    """
+
+
+class ResourceSlice(BaseModel):
+    apiVersion: Literal['resource.k8s.io/v1'] | None = 'resource.k8s.io/v1'
+    """
+    APIVersion defines the versioned schema of this representation of an object.
+    """
+    kind: Literal['ResourceSlice'] | None = 'ResourceSlice'
+    """
+    Kind is a string value representing the REST resource this object represents.
+    """
+    metadata: v1.ObjectMeta | None = None
+    """
+    Standard object's metadata.
+    """
+    spec: Spec
+    """
+    Contents of the ResourceSlice.
+    """