Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix synchronization between claim and the counterpart composite #4896

Merged
merged 2 commits into from
Dec 13, 2023
Merged

Fix synchronization between claim and the counterpart composite #4896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf8023f
Fix synchronization between claim and the counterpart composite
pedjak Oct 16, 2023
b390a75
Addressed reviewers comments
pedjak Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 0 additions & 36 deletions internal/controller/apiextensions/claim/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,53 +28,17 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/crossplane/crossplane-runtime/pkg/errors"
"github.com/crossplane/crossplane-runtime/pkg/meta"
"github.com/crossplane/crossplane-runtime/pkg/resource"
)

// Error strings.
const (
errUpdateClaim = "cannot update composite resource claim"
errBindClaimConflict = "cannot bind claim that references a different composite resource"
errGetSecret = "cannot get composite resource's connection secret"
errGetXRD = "cannot get composite resource definition"
errSecretConflict = "cannot establish control of existing connection secret"
errCreateOrUpdateSecret = "cannot create or update connection secret"
)

// An APIBinder binds claims to composites by updating them in a Kubernetes API
// server.
type APIBinder struct {
client client.Client
}

// NewAPIBinder returns a new APIBinder.
func NewAPIBinder(c client.Client) *APIBinder {
return &APIBinder{client: c}
}

// Bind the supplied claim to the supplied composite.
func (a *APIBinder) Bind(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error {
existing := cm.GetResourceReference()
proposed := meta.ReferenceTo(cp, cp.GetObjectKind().GroupVersionKind())
equal := cmp.Equal(existing, proposed, cmpopts.IgnoreFields(corev1.ObjectReference{}, "UID"))

// We refuse to 're-bind' a claim that is already bound to a different
// composite resource.
if existing != nil && !equal {
return errors.New(errBindClaimConflict)
}

// There's no need to call update if the claim already references this
// composite resource.
if equal {
return nil
}

cm.SetResourceReference(proposed)
return errors.Wrap(a.client.Update(ctx, cm), errUpdateClaim)
}

// An APIConnectionPropagator propagates connection details by reading
// them from and writing them to a Kubernetes API server.
type APIConnectionPropagator struct {
Expand Down
111 changes: 0 additions & 111 deletions internal/controller/apiextensions/claim/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,120 +35,9 @@ import (
)

var (
_ Binder = &APIBinder{}
_ ConnectionPropagator = &APIConnectionPropagator{}
)

func TestBind(t *testing.T) {
errBoom := errors.New("boom")

type fields struct {
c client.Client
}

type args struct {
ctx context.Context
cm resource.CompositeClaim
cp resource.Composite
}

type want struct {
cm resource.CompositeClaim
err error
}

cases := map[string]struct {
reason string
fields fields
args args
want want
}{
"CompositeRefConflict": {
reason: "An error should be returned if the claim is bound to another composite resource",
args: args{
cm: &fake.CompositeClaim{
CompositeResourceReferencer: fake.CompositeResourceReferencer{
Ref: &corev1.ObjectReference{
Name: "who",
},
},
},
cp: &fake.Composite{
ObjectMeta: metav1.ObjectMeta{
Name: "wat",
},
},
},
want: want{
cm: &fake.CompositeClaim{
CompositeResourceReferencer: fake.CompositeResourceReferencer{
Ref: &corev1.ObjectReference{
Name: "who",
},
},
},
err: errors.New(errBindClaimConflict),
},
},
"UpdateClaimError": {
reason: "Errors updating the claim should be returned",
fields: fields{
c: &test.MockClient{
MockUpdate: test.NewMockUpdateFn(errBoom),
},
},
args: args{
cm: &fake.CompositeClaim{
CompositeResourceReferencer: fake.CompositeResourceReferencer{},
},
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Name: "who"}},
},
want: want{
cm: &fake.CompositeClaim{
CompositeResourceReferencer: fake.CompositeResourceReferencer{
Ref: &corev1.ObjectReference{
Name: "who",
},
},
},
err: errors.Wrap(errBoom, errUpdateClaim),
},
},
"NoOp": {
reason: "We should return without calling Update if the claim already references the composite",
args: args{
cm: &fake.CompositeClaim{
CompositeResourceReferencer: fake.CompositeResourceReferencer{
Ref: &corev1.ObjectReference{Name: "coolXR"},
},
},
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Name: "coolXR"}},
},
want: want{
cm: &fake.CompositeClaim{
CompositeResourceReferencer: fake.CompositeResourceReferencer{
Ref: &corev1.ObjectReference{Name: "coolXR"},
},
},
},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
b := NewAPIBinder(tc.fields.c)
err := b.Bind(tc.args.ctx, tc.args.cm, tc.args.cp)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("b.Bind(...): %s\n-want error, +got error:\n%s\n", tc.reason, diff)
}
if diff := cmp.Diff(tc.want.cm, tc.args.cm); diff != "" {
t.Errorf("b.Bind(...): %s\n-want, +got:\n%s\n", tc.reason, diff)
}
})
}

}

func TestPropagateConnection(t *testing.T) {
errBoom := errors.New("boom")

Expand Down