diff --git a/crossplane/function/resource.py b/crossplane/function/resource.py index 6b240d3..7f1298c 100644 --- a/crossplane/function/resource.py +++ b/crossplane/function/resource.py @@ -40,12 +40,13 @@ 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. + data = source.model_dump(exclude_unset=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 +72,11 @@ 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, - matching the behavior of :func:`update`. + is a Pydantic model, fields the caller didn't explicitly set are + excluded, 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, warnings=False) update(r, {"status": status}) diff --git a/tests/test_resource.py b/tests/test_resource.py index 88bb37a..8124a5b 100644 --- a/tests/test_resource.py +++ b/tests/test_resource.py @@ -22,7 +22,10 @@ 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.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 +62,28 @@ 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 on an empty resource should work.", r=fnv1.Resource(), @@ -131,11 +149,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 +171,53 @@ 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( + # 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: diff --git a/tests/testdata/models/io/upbound/m/__init__.py b/tests/testdata/models/io/upbound/m/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/testdata/models/io/upbound/m/aws/__init__.py b/tests/testdata/models/io/upbound/m/aws/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/testdata/models/io/upbound/m/aws/iam/__init__.py b/tests/testdata/models/io/upbound/m/aws/iam/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/testdata/models/io/upbound/m/aws/iam/accountalias/__init__.py b/tests/testdata/models/io/upbound/m/aws/iam/accountalias/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/testdata/models/io/upbound/m/aws/iam/accountalias/v1beta1.py b/tests/testdata/models/io/upbound/m/aws/iam/accountalias/v1beta1.py new file mode 100644 index 0000000..3d1183a --- /dev/null +++ b/tests/testdata/models/io/upbound/m/aws/iam/accountalias/v1beta1.py @@ -0,0 +1,166 @@ +# generated by datamodel-codegen: +# filename: workdir/iam_aws_m_upbound_io_v1beta1_accountalias.yaml + +from __future__ import annotations + +from typing import Any, Literal + +from pydantic import AwareDatetime, BaseModel, Field + +from ......k8s.apimachinery.pkg.apis.meta import v1 + + +class ProviderConfigRef(BaseModel): + kind: str + """ + Kind of the referenced object. + """ + name: str + """ + Name of the referenced object. + """ + + +class WriteConnectionSecretToRef(BaseModel): + name: str + """ + Name of the secret. + """ + + +class Spec(BaseModel): + forProvider: dict[str, Any] + initProvider: dict[str, Any] | None = None + """ + THIS IS A BETA FIELD. It will be honored + unless the Management Policies feature flag is disabled. + InitProvider holds the same fields as ForProvider, with the exception + of Identifier and other resource reference fields. The fields that are + in InitProvider are merged into ForProvider when the resource is created. + The same fields are also added to the terraform ignore_changes hook, to + avoid updating them after creation. This is useful for fields that are + required on creation, but we do not desire to update them after creation, + for example because of an external controller is managing them, like an + autoscaler. + """ + managementPolicies: ( + list[Literal['Observe', 'Create', 'Update', 'Delete', 'LateInitialize', '*']] + | None + ) = ['*'] + """ + THIS IS A BETA FIELD. It is on by default but can be opted out + through a Crossplane feature flag. + ManagementPolicies specify the array of actions Crossplane is allowed to + take on the managed and external resources. + See the design doc for more information: https://github.com/crossplane/crossplane/blob/499895a25d1a1a0ba1604944ef98ac7a1a71f197/design/design-doc-observe-only-resources.md?plain=1#L223 + and this one: https://github.com/crossplane/crossplane/blob/444267e84783136daa93568b364a5f01228cacbe/design/one-pager-ignore-changes.md + """ + providerConfigRef: ProviderConfigRef | None = Field( + {'kind': 'ClusterProviderConfig', 'name': 'default'}, validate_default=True + ) + """ + ProviderConfigReference specifies how the provider that will be used to + create, observe, update, and delete this managed resource should be + configured. + """ + writeConnectionSecretToRef: WriteConnectionSecretToRef | None = None + """ + WriteConnectionSecretToReference specifies the namespace and name of a + Secret to which any connection details for this managed resource should + be written. Connection details frequently include the endpoint, username, + and password required to connect to the managed resource. + """ + + +class AtProvider(BaseModel): + id: str | None = None + + +class Condition(BaseModel): + lastTransitionTime: AwareDatetime + """ + LastTransitionTime is the last time this condition transitioned from one + status to another. + """ + message: str | None = None + """ + A Message containing details about this condition's last transition from + one status to another, if any. + """ + observedGeneration: int | None = None + """ + ObservedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + """ + reason: str + """ + A Reason for this condition's last transition from one status to another. + """ + status: str + """ + Status of this condition; is it currently True, False, or Unknown? + """ + type: str + """ + Type of this condition. At most one of each condition type may apply to + a resource at any point in time. + """ + + +class Status(BaseModel): + atProvider: AtProvider | None = None + conditions: list[Condition] | None = None + """ + Conditions of the resource. + """ + observedGeneration: int | None = None + """ + ObservedGeneration is the latest metadata.generation + which resulted in either a ready state, or stalled due to error + it can not recover from without human intervention. + """ + + +class AccountAlias(BaseModel): + apiVersion: Literal['iam.aws.m.upbound.io/v1beta1'] | None = ( + 'iam.aws.m.upbound.io/v1beta1' + ) + """ + APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + """ + kind: Literal['AccountAlias'] | None = 'AccountAlias' + """ + Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + """ + metadata: v1.ObjectMeta | None = None + """ + Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata + """ + spec: Spec + """ + AccountAliasSpec defines the desired state of AccountAlias + """ + status: Status | None = None + """ + AccountAliasStatus defines the observed state of AccountAlias. + """ + + +class AccountAliasList(BaseModel): + apiVersion: str | None = None + """ + APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + """ + items: list[AccountAlias] + """ + List of accountaliases. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md + """ + kind: str | None = None + """ + Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + """ + metadata: v1.ListMeta | None = None + """ + Standard list metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + """ \ No newline at end of file