fix(pkg/resource/composite) drop usage of ObjectReference in SetClaim…

…Reference `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 <[email protected]>
crossplane · Oct 9, 2023 · 81b21d4 · 81b21d4
1 parent b537456
commit 81b21d4
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,16 +138,16 @@ 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
 	}
 	return out
 }
 
 // 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
@@ -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(),