Skip to content

Commit 58353f2

Browse files
Declare nothing for a scope that composes nothing, and keep what exists
Porting Modelplane to reference.composing turned up two gaps. A scope that read a reference but didn't compose its resource, because the value it needed wasn't there yet, still declared the dependency. Crossplane ignores a dependency whose resource it doesn't know about, and says so in an event on every reconcile. A scope now declares dependencies only for a resource that's composed or already exists. And a resource that exists, but that the block didn't compose because the value was missing, was left out of desired state, which deletes it. The fallback only covered a resource the block had written. It now keeps an existing resource either way: its current spec under whatever the block wrote, or its spec alone if the block wrote nothing, with a warning saying why. That's the natural way to write it - secrets = c.ref(cluster.status.secrets) if secrets: compose_backend(secrets) - and it shouldn't delete the backend when the cluster's secrets briefly disappear. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent b8ca8e0 commit 58353f2

2 files changed

Lines changed: 90 additions & 20 deletions

File tree

‎crossplane/function/reference.py‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -671,35 +671,51 @@ def _get(self, s: _Source, path: list, what: str) -> typing.Any:
671671
return v
672672

673673
def _close(self) -> None:
674-
_record(self.rsp, self.name, self._sources)
675-
676-
if not self._unresolved:
677-
return
678-
679674
exists = self.name in self.req.observed.resources
680-
ordered = request.has_capability(self.req, fnv1.CAPABILITY_DEPENDENCIES)
681675

682-
if not exists:
683-
# Crossplane won't create the resource until the dependency is
684-
# ready. One that doesn't enforce dependencies would create it
685-
# without the field, so hold it back by leaving it out instead.
686-
if not ordered:
676+
if self._unresolved:
677+
if exists:
678+
self._keep_current_spec()
679+
elif not request.has_capability(self.req, fnv1.CAPABILITY_DEPENDENCIES):
680+
# Crossplane won't create the resource until the dependency
681+
# is ready. One that doesn't enforce dependencies would create
682+
# it without the field, so hold it back by leaving it out.
687683
self.rsp.desired.resources.pop(self.name, None)
688-
return
689684

690-
# The resource exists, but something it refers to doesn't. The fields
691-
# that refer to it came back None and were left out, and applying
692-
# that would unset them. Keep what the resource already has for any
693-
# field this function no longer sets, and say so.
685+
# Declare dependencies only for a resource that's composed or exists.
686+
# A scope that composed nothing has nothing to order, and Crossplane
687+
# would ignore the dependencies, with an event saying so.
688+
if self.name in self.rsp.desired.resources or exists:
689+
_record(self.rsp, self.name, self._sources)
690+
691+
def _keep_current_spec(self) -> None:
692+
"""Keep an existing resource's spec while a reference it needs is gone.
693+
694+
The fields that refer to what's missing came back None and were left
695+
out, and applying that would unset them. A function that didn't
696+
compose the resource at all, because the value it needed wasn't
697+
there, would have it deleted. Either way, keep what the resource
698+
already has for anything this function doesn't set, and say so.
699+
"""
700+
observed = resource.struct_to_dict(
701+
self.req.observed.resources[self.name].resource
702+
)
694703
if self.name in self.rsp.desired.resources:
695704
r = self.rsp.desired.resources[self.name]
696-
observed = resource.struct_to_dict(
697-
self.req.observed.resources[self.name].resource
698-
)
699705
body = resource.struct_to_dict(r.resource)
700706
if "spec" in observed:
701707
body["spec"] = _overlay(observed["spec"], body.get("spec", {}))
702708
r.resource.CopyFrom(resource.dict_to_struct(body))
709+
else:
710+
meta = observed.get("metadata", {})
711+
body = {
712+
"apiVersion": observed.get("apiVersion"),
713+
"kind": observed.get("kind"),
714+
"metadata": {k: meta[k] for k in ("name", "namespace") if k in meta},
715+
}
716+
if "spec" in observed:
717+
body["spec"] = observed["spec"]
718+
resource.update(self.rsp.desired.resources[self.name], body)
703719

704720
response.warning(
705721
self.rsp,
@@ -761,7 +777,12 @@ def composing(
761777
left out. If the resource doesn't exist yet that's what ordering is for:
762778
Crossplane waits for the dependency before creating it. If it does exist,
763779
it keeps its current spec for the fields that were left out, and the
764-
response carries a warning saying why.
780+
response carries a warning saying why. That holds even if the block
781+
doesn't compose the resource at all because the value it needed is
782+
missing: an existing resource is kept rather than deleted.
783+
784+
Dependencies are declared only for a resource that's composed or already
785+
exists. A block that composes nothing declares nothing.
765786
"""
766787
scope = Scope(req, rsp, name)
767788
yield scope

‎tests/test_reference.py‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,52 @@ def test_existing_resource_keeps_its_spec(self) -> None:
581581
self.assertEqual(rsp.results[0].severity, fnv1.SEVERITY_WARNING)
582582
self.assertIn("vpc.status.atProvider.id", rsp.results[0].message)
583583

584+
def test_composing_nothing_declares_nothing(self) -> None:
585+
req = fnv1.RunFunctionRequest(meta=ORDERED)
586+
rsp = response.to(req)
587+
vpc = reference.named("vpc", VPC)
588+
589+
with reference.composing(req, rsp, "subnet") as c:
590+
vpc_id = c.ref(vpc.status.atProvider.id)
591+
if vpc_id:
592+
desired_subnet(rsp, vpc_id)
593+
594+
self.assertNotIn("subnet", rsp.desired.resources)
595+
self.assertEqual(edges(rsp), [])
596+
597+
def test_existing_resource_not_composed_is_kept(self) -> None:
598+
subnet = {
599+
"apiVersion": "ec2.aws.m.upbound.io/v1beta1",
600+
"kind": "Subnet",
601+
"metadata": {"name": "xr-subnet-1", "uid": "abc", "resourceVersion": "7"},
602+
"spec": {"forProvider": {"region": "us-east-1", "vpcId": "vpc-0123"}},
603+
"status": {"atProvider": {"id": "subnet-9"}},
604+
}
605+
req = fnv1.RunFunctionRequest(meta=ORDERED, observed=observed(subnet=subnet))
606+
rsp = response.to(req)
607+
vpc = reference.named("vpc", VPC)
608+
609+
with reference.composing(req, rsp, "subnet") as c:
610+
vpc_id = c.ref(vpc.status.atProvider.id)
611+
if vpc_id:
612+
desired_subnet(rsp, vpc_id)
613+
614+
# Kept rather than deleted, as its spec, without what the API server
615+
# owns: uid, resourceVersion and status.
616+
self.assertEqual(
617+
body(rsp, "subnet"),
618+
{
619+
"apiVersion": "ec2.aws.m.upbound.io/v1beta1",
620+
"kind": "Subnet",
621+
"metadata": {"name": "xr-subnet-1"},
622+
"spec": {"forProvider": {"region": "us-east-1", "vpcId": "vpc-0123"}},
623+
},
624+
)
625+
self.assertEqual(
626+
edges(rsp), [{"resource": "subnet", "composed_resource": "vpc"}]
627+
)
628+
self.assertEqual(rsp.results[0].severity, fnv1.SEVERITY_WARNING)
629+
584630
def test_without_capability_holds_back_by_omission(self) -> None:
585631
req = fnv1.RunFunctionRequest(meta=UNORDERED)
586632
rsp = response.to(req)
@@ -628,6 +674,9 @@ def test_required(self) -> None:
628674

629675
with reference.composing(req, rsp, "app-config") as c:
630676
host = c.ref(db.status.address)
677+
c.update(
678+
{"apiVersion": "v1", "kind": "ConfigMap", "data": {"DB_HOST": host}}
679+
)
631680

632681
self.assertEqual(host, "db.a")
633682
self.assertEqual(

0 commit comments

Comments
 (0)