Pass by_alias when serializing models in update and update_status

resource.update and resource.update_status serialize Pydantic models
with model_dump(exclude_unset=True), which doesn't pass by_alias=True.
A model field that carries a Pydantic alias is then serialized under its
Python attribute name rather than its alias, which is the resource's real
wire name.

This bites fields whose KRM name is a Python keyword or builtin.
datamodel-code-generator can't name a field bool, int, from, continue, or
schema, so it emits a bool_ attribute aliased to bool, int_ aliased to
int, and so on. When a function composes a resource that sets such a
field, update wrote the Python name into the desired resource:

  data = source.model_dump(exclude_unset=True)
  # -> {"bool_": True}, but the resource's field is "bool"

The composed resource then carried bool_: true instead of bool: true,
which doesn't match the resource's schema. The API server rejects it or
silently drops the unknown field, and the field the function set never
takes effect. This surfaced with Kubernetes Dynamic Resource Allocation,
whose device attribute value is a one-of over string, version, bool, and
int.

This change passes by_alias=True alongside exclude_unset=True in both
functions. It's a no-op for ordinary fields, which have no alias, and
corrects only the keyword-collision cases. It's also symmetric with how
pydantic deserializes these models: by default an aliased field is
populated only by its alias, so reads and writes now both speak wire
names.

Fixes #210.

Signed-off-by: Nic Cope <nicc@rk0n.org>

Author: negz

crossplane/function/resource.py

@@ -44,7 +44,15 @@ def update(r: fnv1.Resource, source: dict | structpb.Struct | pydantic.BaseModel
             # 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.
-            data = source.model_dump(exclude_unset=True, warnings=False)
+            #
+            # 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
@@ -73,10 +81,11 @@ def update_status(
 
     Sets ``r.resource.status`` from the supplied status. When the status
     is a Pydantic model, fields the caller didn't explicitly set are
-    excluded, matching the behavior of :func:`update`.
+    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_unset=True, warnings=False)
+        status = status.model_dump(exclude_unset=True, by_alias=True, warnings=False)
     update(r, {"status": status})
 
 

tests/test_resource.py

@@ -22,6 +22,7 @@
 
 import crossplane.function.proto.v1.run_function_pb2 as fnv1
 from crossplane.function import logging, resource
+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,
@@ -84,6 +85,21 @@ class TestCase:
                     "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(),
@@ -192,6 +208,51 @@ class TestCase:
                     ),
                 ),
             ),
+            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
@@ -228,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.
+    """