From ac18e9b08568cca2c55a8878fa3ad9ca6d60ac61 Mon Sep 17 00:00:00 2001 From: Nic Cope Date: Wed, 20 May 2026 18:51:49 -0700 Subject: [PATCH 1/6] Add response.set_condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SDK has resource.get_condition and resource.Condition for reading conditions from observed resources, but no way to write conditions back to the response. Functions build fnv1.Condition protobuf messages by hand, mapping status strings to protobuf enums and setting the target each time. This commit adds response.set_condition, which accepts a resource.Condition — the same type returned by get_condition — and appends the corresponding fnv1.Condition to the response. Signed-off-by: Nic Cope --- crossplane/function/response.py | 36 ++++++++++++++++++++ tests/test_response.py | 60 +++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/crossplane/function/response.py b/crossplane/function/response.py index 5392d36..48413a8 100644 --- a/crossplane/function/response.py +++ b/crossplane/function/response.py @@ -81,6 +81,42 @@ def fatal(rsp: fnv1.RunFunctionResponse, message: str) -> None: ) +def set_condition( + rsp: fnv1.RunFunctionResponse, + condition: resource.Condition, +) -> None: + """Set a condition on the composite resource (XR). + + Args: + rsp: The RunFunctionResponse to update. + condition: The condition to set. + + The condition is appended to ``rsp.conditions``. Crossplane uses the + conditions returned by a function to set custom status conditions on + the composite resource. + + The ``last_transition_time`` field of the condition is ignored. + Crossplane sets the transition time itself. + + Do not set the ``Ready`` condition type. Crossplane manages it based + on resource readiness. + """ + status_map = { + "True": fnv1.STATUS_CONDITION_TRUE, + "False": fnv1.STATUS_CONDITION_FALSE, + "Unknown": fnv1.STATUS_CONDITION_UNKNOWN, + } + + rsp.conditions.append( + fnv1.Condition( + type=condition.typ, + status=status_map.get(condition.status, fnv1.STATUS_CONDITION_UNKNOWN), + reason=condition.reason or "", + message=condition.message or "", + ) + ) + + def set_output(rsp: fnv1.RunFunctionResponse, output: dict | structpb.Struct) -> None: """Set the output field in a RunFunctionResponse for operation functions. diff --git a/tests/test_response.py b/tests/test_response.py index 763a37f..d75e411 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -71,6 +71,66 @@ class TestCase: "-want, +got", ) + def test_set_condition(self) -> None: + @dataclasses.dataclass + class TestCase: + reason: str + condition: resource.Condition + want_type: str + want_status: fnv1.Status.ValueType + want_reason: str + want_message: str + + cases = [ + TestCase( + reason="A True condition should use STATUS_CONDITION_TRUE.", + condition=resource.Condition( + typ="DatabaseReady", + status="True", + reason="Available", + message="The database is ready", + ), + want_type="DatabaseReady", + want_status=fnv1.STATUS_CONDITION_TRUE, + want_reason="Available", + want_message="The database is ready", + ), + TestCase( + reason="A False condition should use STATUS_CONDITION_FALSE.", + condition=resource.Condition( + typ="DatabaseReady", + status="False", + reason="Creating", + ), + want_type="DatabaseReady", + want_status=fnv1.STATUS_CONDITION_FALSE, + want_reason="Creating", + want_message="", + ), + TestCase( + reason="An Unknown condition should use STATUS_CONDITION_UNKNOWN.", + condition=resource.Condition( + typ="DatabaseReady", + status="Unknown", + ), + want_type="DatabaseReady", + want_status=fnv1.STATUS_CONDITION_UNKNOWN, + want_reason="", + want_message="", + ), + ] + + for case in cases: + rsp = fnv1.RunFunctionResponse() + response.set_condition(rsp, case.condition) + + self.assertEqual(1, len(rsp.conditions), case.reason) + got = rsp.conditions[0] + self.assertEqual(case.want_type, got.type, case.reason) + self.assertEqual(case.want_status, got.status, case.reason) + self.assertEqual(case.want_reason, got.reason, case.reason) + self.assertEqual(case.want_message, got.message, case.reason) + def test_set_output(self) -> None: @dataclasses.dataclass class TestCase: From 629331fbb4506997d20f33578cdc15f5190b1838 Mon Sep 17 00:00:00 2001 From: Nic Cope Date: Wed, 20 May 2026 18:54:38 -0700 Subject: [PATCH 2/6] Add resource.child_name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Composition functions often derive child resource names by combining a parent name with a discriminator. The resulting name must be a valid DNS label — at most 63 characters. Names built from user-supplied components can exceed this limit, and functions end up writing ad-hoc truncation logic. This commit adds resource.child_name, which joins name parts, appends a deterministic 5-character SHA-256 hash suffix for uniqueness, and truncates the prefix to fit within the 63-character limit. child_name("my-xr", "bucket") # "my-xr-bucket-05ecb" child_name("my-very-long-deployment", "us-central1-a-gpu-cluster") # "my-very-long-deployment-us-central1-a-gpu-cluste-7e130" Signed-off-by: Nic Cope --- crossplane/function/resource.py | 35 ++++++++++++++++++++++ tests/test_resource.py | 53 +++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/crossplane/function/resource.py b/crossplane/function/resource.py index fae7be6..bc22098 100644 --- a/crossplane/function/resource.py +++ b/crossplane/function/resource.py @@ -16,6 +16,7 @@ import dataclasses import datetime +import hashlib import pydantic from google.protobuf import json_format @@ -140,3 +141,37 @@ def get_condition(resource: structpb.Struct, typ: str) -> Condition: return condition return unknown + + +_DNS_LABEL_MAX = 63 +_HASH_LEN = 5 + + +def child_name(*parts: str, sep: str = "-") -> str: + """Build a deterministic, DNS-label-safe name for a child resource. + + Args: + *parts: Name components to join (e.g. parent name, suffix). + sep: Separator between parts. Defaults to "-". + + Returns: + A name that is at most 63 characters long. + + Composition functions often derive child resource names from a parent + name and a discriminator. The resulting name must be a valid DNS label + (at most 63 characters). This function joins the parts, appends a + deterministic 5-character hash suffix for uniqueness, and truncates + the prefix to fit within the limit. + + The hash suffix is always appended, even for short names, so that + names are visually consistent regardless of length:: + + child_name("my-xr", "bucket") # "my-xr-bucket-a1b2c" + child_name("my-very-long-xr-name", + "with-a-very-long-suffix") # truncated to 63 chars + """ + full = sep.join(parts) + h = hashlib.sha256(full.encode()).hexdigest()[:_HASH_LEN] + max_prefix = _DNS_LABEL_MAX - _HASH_LEN - 1 + prefix = full[:max_prefix].rstrip(sep) + return f"{prefix}{sep}{h}" diff --git a/tests/test_resource.py b/tests/test_resource.py index 004295f..d651f66 100644 --- a/tests/test_resource.py +++ b/tests/test_resource.py @@ -324,6 +324,59 @@ class TestCase: got = resource.struct_to_dict(case.s) self.assertEqual(case.want, got, "-want, +got") + def test_child_name(self) -> None: + @dataclasses.dataclass + class TestCase: + reason: str + parts: list[str] + want: str + + cases = [ + TestCase( + reason="A short name should be joined with a hash suffix.", + parts=["my-xr", "bucket"], + want="my-xr-bucket-05ecb", + ), + TestCase( + reason="A single part should get a hash suffix.", + parts=["my-xr"], + want="my-xr-9d53f", + ), + TestCase( + reason="A long name should be truncated to fit within 63 characters.", + parts=["a" * 40, "b" * 40], + want="a" * 40 + "-" + "b" * 16 + "-" + "f5e42", + ), + TestCase( + reason="A name that would end with a trailing separator " + "after truncation should have the separator stripped.", + parts=["a" * 56 + "-", "x"], + # Without stripping, this would be "aaa..a--". + # The trailing separator from the truncation is stripped. + want="a" * 56 + "-" + "995eb", + ), + TestCase( + reason="The same inputs should always produce the same name.", + parts=["parent", "child"], + want="parent-child-2f0c9", + ), + ] + + for case in cases: + got = resource.child_name(*case.parts) + self.assertEqual(case.want, got, case.reason) + self.assertLessEqual(len(got), 63, case.reason) + + def test_child_name_deterministic(self) -> None: + a = resource.child_name("parent", "child") + b = resource.child_name("parent", "child") + self.assertEqual(a, b) + + def test_child_name_unique(self) -> None: + a = resource.child_name("parent", "child-a") + b = resource.child_name("parent", "child-b") + self.assertNotEqual(a, b) + if __name__ == "__main__": unittest.main() From c8bdf85c585d793bf611ef4adc0168b9c7ba24b9 Mon Sep 17 00:00:00 2001 From: Nic Cope Date: Wed, 20 May 2026 19:03:07 -0700 Subject: [PATCH 3/6] Add resource.update_status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Functions that set status on the composite resource have to wrap their status in {"status": ...} and call resource.update. This is a small but universal bit of boilerplate — nearly every composition function writes status. This commit adds resource.update_status, which accepts a dict or Pydantic model and nests it under the status key. Signed-off-by: Nic Cope --- crossplane/function/resource.py | 19 +++++++++++++ tests/test_resource.py | 50 +++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/crossplane/function/resource.py b/crossplane/function/resource.py index bc22098..2a14ca7 100644 --- a/crossplane/function/resource.py +++ b/crossplane/function/resource.py @@ -60,6 +60,25 @@ def update(r: fnv1.Resource, source: dict | structpb.Struct | pydantic.BaseModel raise TypeError(msg) +def update_status( + r: fnv1.Resource, + status: dict | pydantic.BaseModel, +) -> None: + """Update a resource's status. + + Args: + r: A composite or composed resource to update. + 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, + matching the behavior of :func:`update`. + """ + if isinstance(status, pydantic.BaseModel): + status = status.model_dump(exclude_defaults=True, warnings=False) + update(r, {"status": status}) + + def dict_to_struct(d: dict) -> structpb.Struct: """Create a Struct well-known type from the supplied dict. diff --git a/tests/test_resource.py b/tests/test_resource.py index d651f66..7717906 100644 --- a/tests/test_resource.py +++ b/tests/test_resource.py @@ -29,6 +29,56 @@ class TestResource(unittest.TestCase): def setUp(self) -> None: logging.configure(level=logging.Level.DISABLED) + def test_update_status(self) -> None: + @dataclasses.dataclass + class TestCase: + reason: str + r: fnv1.Resource + status: dict | pydantic.BaseModel + want: dict + + cases = [ + TestCase( + reason="Setting status from a dict should work.", + r=fnv1.Resource( + resource=resource.dict_to_struct( + {"apiVersion": "example.org", "kind": "XR"} + ), + ), + status={"ready": True}, + want={ + "apiVersion": "example.org", + "kind": "XR", + "status": {"ready": True}, + }, + ), + TestCase( + reason="Setting status from a Pydantic model should work.", + r=fnv1.Resource( + resource=resource.dict_to_struct( + {"apiVersion": "example.org", "kind": "XR"} + ), + ), + status=v1beta2.ForProvider(region="us-west-2"), + want={ + "apiVersion": "example.org", + "kind": "XR", + "status": {"region": "us-west-2"}, + }, + ), + TestCase( + reason="Setting status on an empty resource should work.", + r=fnv1.Resource(), + status={"replicas": 3}, + want={"status": {"replicas": 3}}, + ), + ] + + for case in cases: + resource.update_status(case.r, case.status) + got = resource.struct_to_dict(case.r.resource) + self.assertEqual(case.want, got, case.reason) + def test_add(self) -> None: @dataclasses.dataclass class TestCase: From fb3968281747d4ee1f15b4f1eb14fec2a62c2326 Mon Sep 17 00:00:00 2001 From: Nic Cope Date: Wed, 20 May 2026 19:09:16 -0700 Subject: [PATCH 4/6] Make set_condition variadic and rename to set_conditions This commit renames set_condition to set_conditions and changes the signature to accept variadic resource.Condition arguments. Functions that set multiple conditions can now pass them all in one call. Signed-off-by: Nic Cope --- crossplane/function/response.py | 40 +++++++------- tests/test_response.py | 96 +++++++++++++++++---------------- 2 files changed, 72 insertions(+), 64 deletions(-) diff --git a/crossplane/function/response.py b/crossplane/function/response.py index 48413a8..6f6ba67 100644 --- a/crossplane/function/response.py +++ b/crossplane/function/response.py @@ -81,40 +81,42 @@ def fatal(rsp: fnv1.RunFunctionResponse, message: str) -> None: ) -def set_condition( +_STATUS_MAP = { + "True": fnv1.STATUS_CONDITION_TRUE, + "False": fnv1.STATUS_CONDITION_FALSE, + "Unknown": fnv1.STATUS_CONDITION_UNKNOWN, +} + + +def set_conditions( rsp: fnv1.RunFunctionResponse, - condition: resource.Condition, + *conditions: resource.Condition, ) -> None: - """Set a condition on the composite resource (XR). + """Set one or more conditions on the composite resource (XR). Args: rsp: The RunFunctionResponse to update. - condition: The condition to set. + *conditions: The conditions to set. - The condition is appended to ``rsp.conditions``. Crossplane uses the + Each condition is appended to ``rsp.conditions``. Crossplane uses the conditions returned by a function to set custom status conditions on the composite resource. - The ``last_transition_time`` field of the condition is ignored. + The ``last_transition_time`` field of each condition is ignored. Crossplane sets the transition time itself. Do not set the ``Ready`` condition type. Crossplane manages it based on resource readiness. """ - status_map = { - "True": fnv1.STATUS_CONDITION_TRUE, - "False": fnv1.STATUS_CONDITION_FALSE, - "Unknown": fnv1.STATUS_CONDITION_UNKNOWN, - } - - rsp.conditions.append( - fnv1.Condition( - type=condition.typ, - status=status_map.get(condition.status, fnv1.STATUS_CONDITION_UNKNOWN), - reason=condition.reason or "", - message=condition.message or "", + for condition in conditions: + rsp.conditions.append( + fnv1.Condition( + type=condition.typ, + status=_STATUS_MAP.get(condition.status, fnv1.STATUS_CONDITION_UNKNOWN), + reason=condition.reason or "", + message=condition.message or "", + ) ) - ) def set_output(rsp: fnv1.RunFunctionResponse, output: dict | structpb.Struct) -> None: diff --git a/tests/test_response.py b/tests/test_response.py index d75e411..fd95089 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -71,65 +71,71 @@ class TestCase: "-want, +got", ) - def test_set_condition(self) -> None: + def test_set_conditions(self) -> None: @dataclasses.dataclass class TestCase: reason: str - condition: resource.Condition - want_type: str - want_status: fnv1.Status.ValueType - want_reason: str - want_message: str + conditions: list[resource.Condition] + want_types: list[str] + want_statuses: list[fnv1.Status.ValueType] + want_reasons: list[str] + want_messages: list[str] cases = [ TestCase( - reason="A True condition should use STATUS_CONDITION_TRUE.", - condition=resource.Condition( - typ="DatabaseReady", - status="True", - reason="Available", - message="The database is ready", - ), - want_type="DatabaseReady", - want_status=fnv1.STATUS_CONDITION_TRUE, - want_reason="Available", - want_message="The database is ready", - ), - TestCase( - reason="A False condition should use STATUS_CONDITION_FALSE.", - condition=resource.Condition( - typ="DatabaseReady", - status="False", - reason="Creating", - ), - want_type="DatabaseReady", - want_status=fnv1.STATUS_CONDITION_FALSE, - want_reason="Creating", - want_message="", + reason="A single True condition should work.", + conditions=[ + resource.Condition( + typ="DatabaseReady", + status="True", + reason="Available", + message="The database is ready", + ), + ], + want_types=["DatabaseReady"], + want_statuses=[fnv1.STATUS_CONDITION_TRUE], + want_reasons=["Available"], + want_messages=["The database is ready"], ), TestCase( - reason="An Unknown condition should use STATUS_CONDITION_UNKNOWN.", - condition=resource.Condition( - typ="DatabaseReady", - status="Unknown", - ), - want_type="DatabaseReady", - want_status=fnv1.STATUS_CONDITION_UNKNOWN, - want_reason="", - want_message="", + reason="Multiple conditions should all be appended.", + conditions=[ + resource.Condition( + typ="DatabaseReady", + status="True", + reason="Available", + ), + resource.Condition( + typ="CacheReady", + status="False", + reason="Creating", + ), + resource.Condition( + typ="NetworkReady", + status="Unknown", + ), + ], + want_types=["DatabaseReady", "CacheReady", "NetworkReady"], + want_statuses=[ + fnv1.STATUS_CONDITION_TRUE, + fnv1.STATUS_CONDITION_FALSE, + fnv1.STATUS_CONDITION_UNKNOWN, + ], + want_reasons=["Available", "Creating", ""], + want_messages=["", "", ""], ), ] for case in cases: rsp = fnv1.RunFunctionResponse() - response.set_condition(rsp, case.condition) + response.set_conditions(rsp, *case.conditions) - self.assertEqual(1, len(rsp.conditions), case.reason) - got = rsp.conditions[0] - self.assertEqual(case.want_type, got.type, case.reason) - self.assertEqual(case.want_status, got.status, case.reason) - self.assertEqual(case.want_reason, got.reason, case.reason) - self.assertEqual(case.want_message, got.message, case.reason) + self.assertEqual(len(case.conditions), len(rsp.conditions), case.reason) + for i, got in enumerate(rsp.conditions): + self.assertEqual(case.want_types[i], got.type, case.reason) + self.assertEqual(case.want_statuses[i], got.status, case.reason) + self.assertEqual(case.want_reasons[i], got.reason, case.reason) + self.assertEqual(case.want_messages[i], got.message, case.reason) def test_set_output(self) -> None: @dataclasses.dataclass From 1186b3c386fdf845d2215a5eb9fe7b6dc359dd5e Mon Sep 17 00:00:00 2001 From: Nic Cope Date: Wed, 20 May 2026 21:44:58 -0700 Subject: [PATCH 5/6] Only set condition message when non-empty When message is empty, omitting the field from the protobuf message is cleaner than explicitly setting it to an empty string. Protobuf includes explicitly-set zero values in serialization, so setting message="" causes it to appear in MessageToDict output and gRPC responses even when there is nothing to say. Signed-off-by: Nic Cope --- crossplane/function/response.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crossplane/function/response.py b/crossplane/function/response.py index 6f6ba67..c7b5b1d 100644 --- a/crossplane/function/response.py +++ b/crossplane/function/response.py @@ -109,14 +109,14 @@ def set_conditions( on resource readiness. """ for condition in conditions: - rsp.conditions.append( - fnv1.Condition( - type=condition.typ, - status=_STATUS_MAP.get(condition.status, fnv1.STATUS_CONDITION_UNKNOWN), - reason=condition.reason or "", - message=condition.message or "", - ) + c = fnv1.Condition( + type=condition.typ, + status=_STATUS_MAP.get(condition.status, fnv1.STATUS_CONDITION_UNKNOWN), + reason=condition.reason or "", ) + if condition.message: + c.message = condition.message + rsp.conditions.append(c) def set_output(rsp: fnv1.RunFunctionResponse, output: dict | structpb.Struct) -> None: From 8c644cfd761b6e97adce739bb889d0515614786b Mon Sep 17 00:00:00 2001 From: Nic Cope Date: Thu, 21 May 2026 11:52:32 -0700 Subject: [PATCH 6/6] Accept fnv1.Resource and None in get_condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protobuf map fields auto-vivify on bracket access: reading a missing key silently inserts a default-constructed entry into the map. This makes the obvious pattern for checking composed resource conditions unsafe: # Unsafe — auto-vivifies "bucket" if it does not exist. get_condition(req.observed.resources["bucket"].resource, "Ready") The safe alternative is .get(), which returns None without mutating the map. But get_condition only accepted structpb.Struct, forcing callers to add a None guard before extracting the .resource field. This commit widens get_condition to accept fnv1.Resource (extracting the Struct automatically) and None (returning an Unknown condition). Callers can now pass the result of a protobuf map .get() directly: get_condition(req.observed.resources.get("bucket"), "Ready") Signed-off-by: Nic Cope --- crossplane/function/resource.py | 24 ++++++++++++++++++++++-- tests/test_resource.py | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/crossplane/function/resource.py b/crossplane/function/resource.py index 2a14ca7..6b240d3 100644 --- a/crossplane/function/resource.py +++ b/crossplane/function/resource.py @@ -119,11 +119,17 @@ class Condition: last_transition_time: datetime.time | None = None -def get_condition(resource: structpb.Struct, typ: str) -> Condition: +def get_condition( + resource: structpb.Struct | fnv1.Resource | None, + typ: str, +) -> Condition: """Get the supplied status condition of the supplied resource. Args: - resource: A Crossplane resource. + resource: A Crossplane resource. Can be a protobuf Struct (the raw + resource), an fnv1.Resource wrapper, or None. When an + fnv1.Resource is supplied, the Struct is extracted automatically. + When None is supplied, an unknown condition is returned. typ: The type of status condition to get (e.g. Ready). Returns: @@ -131,9 +137,23 @@ def get_condition(resource: structpb.Struct, typ: str) -> Condition: A status condition is always returned. If the status condition isn't present in the supplied resource, a condition with status "Unknown" is returned. + + Accepting fnv1.Resource and None makes it safe to pass the result of a + protobuf map ``.get()`` call directly. This avoids auto-vivification, which + silently inserts a default entry when using bracket access on a missing + key:: + + # Safe — .get() returns None without mutating the map. + c = get_condition(req.observed.resources.get("bucket"), "Ready") + + # Unsafe — bracket access auto-vivifies an empty Resource. + c = get_condition(req.observed.resources["bucket"].resource, "Ready") """ unknown = Condition(typ=typ, status="Unknown") + if isinstance(resource, fnv1.Resource): + resource = resource.resource + if not resource or "status" not in resource: return unknown diff --git a/tests/test_resource.py b/tests/test_resource.py index 7717906..88bb37a 100644 --- a/tests/test_resource.py +++ b/tests/test_resource.py @@ -162,11 +162,17 @@ def test_get_condition(self) -> None: @dataclasses.dataclass class TestCase: reason: str - res: structpb.Struct + res: structpb.Struct | fnv1.Resource | None typ: str want: resource.Condition cases = [ + TestCase( + reason="Return an unknown condition if the resource is None.", + res=None, + typ="Ready", + want=resource.Condition(typ="Ready", status="Unknown"), + ), TestCase( reason="Return an unknown condition if the resource has no status.", res=resource.dict_to_struct({}), @@ -247,6 +253,31 @@ class TestCase: ), ), ), + TestCase( + reason="Unwrap an fnv1.Resource to get the condition from its Struct.", + res=fnv1.Resource( + resource=resource.dict_to_struct( + { + "status": { + "conditions": [ + { + "type": "Ready", + "status": "True", + } + ] + } + } + ), + ), + typ="Ready", + want=resource.Condition(typ="Ready", status="True"), + ), + TestCase( + reason="Return an unknown condition from an empty fnv1.Resource.", + res=fnv1.Resource(), + typ="Ready", + want=resource.Condition(typ="Ready", status="Unknown"), + ), ] for case in cases: