From 81b21d47fed2acb194546cbb3524d321139b174c Mon Sep 17 00:00:00 2001 From: Predrag Knezevic Date: Wed, 20 Sep 2023 14:19:39 +0200 Subject: [PATCH] fix(pkg/resource/composite) drop usage of ObjectReference in SetClaimReference `spec.claimRef` schema is just subset of corev1.ObjectReference, and hence `SetClaimReference` might get a reference that have more fields set, e.g. UID. The fields that do not exist in claimRef schema must be not set on the underlying object, otherwise K8s API server complains about non-existing field when client sends a patch request. * Introduced `claim.Reference` type that corresponds to `spec.claimRef` schema. * `composite.SetClaimReference` signature changed to accept an instance of `claim.Reference` * appropriate tests updated/new tests added Signed-off-by: Predrag Knezevic --- pkg/resource/fake/mocks.go | 7 +++--- pkg/resource/interfaces.go | 5 ++-- pkg/resource/unstructured/claim/claim.go | 25 +++++++++++++++++++ pkg/resource/unstructured/claim/claim_test.go | 13 ++++++++++ .../unstructured/composite/composite.go | 7 +++--- .../unstructured/composite/composite_test.go | 7 +++--- 6 files changed, 53 insertions(+), 11 deletions(-) diff --git a/pkg/resource/fake/mocks.go b/pkg/resource/fake/mocks.go index 29cc9d5e6..200f68f62 100644 --- a/pkg/resource/fake/mocks.go +++ b/pkg/resource/fake/mocks.go @@ -32,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim" ) // Conditioned is a mock that implements Conditioned interface. @@ -46,13 +47,13 @@ func (m *Conditioned) GetCondition(ct xpv1.ConditionType) xpv1.Condition { } // ClaimReferencer is a mock that implements ClaimReferencer interface. -type ClaimReferencer struct{ Ref *corev1.ObjectReference } +type ClaimReferencer struct{ Ref *claim.Reference } // SetClaimReference sets the ClaimReference. -func (m *ClaimReferencer) SetClaimReference(r *corev1.ObjectReference) { m.Ref = r } +func (m *ClaimReferencer) SetClaimReference(r *claim.Reference) { m.Ref = r } // GetClaimReference gets the ClaimReference. -func (m *ClaimReferencer) GetClaimReference() *corev1.ObjectReference { return m.Ref } +func (m *ClaimReferencer) GetClaimReference() *claim.Reference { return m.Ref } // ManagedResourceReferencer is a mock that implements ManagedResourceReferencer interface. type ManagedResourceReferencer struct{ Ref *corev1.ObjectReference } diff --git a/pkg/resource/interfaces.go b/pkg/resource/interfaces.go index 9561d19e4..a0a0603fa 100644 --- a/pkg/resource/interfaces.go +++ b/pkg/resource/interfaces.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim" ) // A Conditioned may have conditions set or retrieved. Conditions are typically @@ -36,8 +37,8 @@ type Conditioned interface { // A ClaimReferencer may reference a resource claim. type ClaimReferencer interface { - SetClaimReference(r *corev1.ObjectReference) - GetClaimReference() *corev1.ObjectReference + SetClaimReference(r *claim.Reference) + GetClaimReference() *claim.Reference } // A ManagedResourceReferencer may reference a concrete managed resource. diff --git a/pkg/resource/unstructured/claim/claim.go b/pkg/resource/unstructured/claim/claim.go index dd4de5d50..c9bc20040 100644 --- a/pkg/resource/unstructured/claim/claim.go +++ b/pkg/resource/unstructured/claim/claim.go @@ -60,6 +60,21 @@ type Unstructured struct { unstructured.Unstructured } +// Reference to a claim +type Reference struct { + // APIVersion of the referenced claim. + APIVersion string `json:"apiVersion"` + + // Kind of the referenced claim. + Kind string `json:"kind"` + + // Name of the referenced claim. + Name string `json:"name"` + + // Namespace of the referenced claim. + Namespace string `json:"namespace"` +} + // GetUnstructured returns the underlying *unstructured.Unstructured. func (c *Unstructured) GetUnstructured() *unstructured.Unstructured { return &c.Unstructured @@ -165,6 +180,16 @@ func (c *Unstructured) SetResourceReference(ref *corev1.ObjectReference) { _ = fieldpath.Pave(c.Object).SetValue("spec.resourceRef", ref) } +// GetReference returns reference to this claim +func (c *Unstructured) GetReference() *Reference { + return &Reference{ + APIVersion: c.GetAPIVersion(), + Kind: c.GetKind(), + Name: c.GetName(), + Namespace: c.GetNamespace(), + } +} + // GetWriteConnectionSecretToReference of this composite resource claim. func (c *Unstructured) GetWriteConnectionSecretToReference() *xpv1.LocalSecretReference { out := &xpv1.LocalSecretReference{} diff --git a/pkg/resource/unstructured/claim/claim_test.go b/pkg/resource/unstructured/claim/claim_test.go index 37cb3258e..1fd0938b0 100644 --- a/pkg/resource/unstructured/claim/claim_test.go +++ b/pkg/resource/unstructured/claim/claim_test.go @@ -291,6 +291,19 @@ func TestResourceReference(t *testing.T) { } } +func TestClaimReference(t *testing.T) { + ref := &Reference{Namespace: "ns", Name: "cool", APIVersion: "foo.com/v1", Kind: "Foo"} + u := &Unstructured{} + u.SetName(ref.Name) + u.SetNamespace(ref.Namespace) + u.SetAPIVersion(ref.APIVersion) + u.SetKind(ref.Kind) + got := u.GetReference() + if diff := cmp.Diff(ref, got); diff != "" { + t.Errorf("\nu.GetClaimReference(): -want, +got:\n%s", diff) + } +} + func TestWriteConnectionSecretToReference(t *testing.T) { ref := &xpv1.LocalSecretReference{Name: "cool"} cases := map[string]struct { diff --git a/pkg/resource/unstructured/composite/composite.go b/pkg/resource/unstructured/composite/composite.go index 97b8995be..e279586f4 100644 --- a/pkg/resource/unstructured/composite/composite.go +++ b/pkg/resource/unstructured/composite/composite.go @@ -25,6 +25,7 @@ import ( xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" + "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim" ) // An Option modifies an unstructured composite resource. @@ -137,8 +138,8 @@ func (c *Unstructured) GetCompositionUpdatePolicy() *xpv1.UpdatePolicy { } // GetClaimReference of this Composite resource. -func (c *Unstructured) GetClaimReference() *corev1.ObjectReference { - out := &corev1.ObjectReference{} +func (c *Unstructured) GetClaimReference() *claim.Reference { + out := &claim.Reference{} if err := fieldpath.Pave(c.Object).GetValueInto("spec.claimRef", out); err != nil { return nil } @@ -146,7 +147,7 @@ func (c *Unstructured) GetClaimReference() *corev1.ObjectReference { } // SetClaimReference of this Composite resource. -func (c *Unstructured) SetClaimReference(ref *corev1.ObjectReference) { +func (c *Unstructured) SetClaimReference(ref *claim.Reference) { _ = fieldpath.Pave(c.Object).SetValue("spec.claimRef", ref) } diff --git a/pkg/resource/unstructured/composite/composite_test.go b/pkg/resource/unstructured/composite/composite_test.go index c74ffdbca..f6d1f67c5 100644 --- a/pkg/resource/unstructured/composite/composite_test.go +++ b/pkg/resource/unstructured/composite/composite_test.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim" ) func TestWithGroupVersionKind(t *testing.T) { @@ -241,11 +242,11 @@ func TestCompositionUpdatePolicy(t *testing.T) { } func TestClaimReference(t *testing.T) { - ref := &corev1.ObjectReference{Namespace: "ns", Name: "cool"} + ref := &claim.Reference{Namespace: "ns", Name: "cool", APIVersion: "acme.com/v1", Kind: "Foo"} cases := map[string]struct { u *Unstructured - set *corev1.ObjectReference - want *corev1.ObjectReference + set *claim.Reference + want *claim.Reference }{ "NewRef": { u: New(),