diff --git a/pkg/controller/groups/members/controller.go b/pkg/controller/groups/members/controller.go index a403a17d..54b5462a 100644 --- a/pkg/controller/groups/members/controller.go +++ b/pkg/controller/groups/members/controller.go @@ -15,7 +15,6 @@ package members import ( "context" - "strconv" "github.com/xanzy/go-gitlab" @@ -27,7 +26,6 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/event" "github.com/crossplane/crossplane-runtime/pkg/logging" - "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" "github.com/crossplane/crossplane-runtime/pkg/resource" @@ -89,17 +87,13 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex return managed.ExternalObservation{}, errors.New(errNotMember) } - externalName := meta.GetExternalName(cr) - if externalName == "" { - return managed.ExternalObservation{ResourceExists: false}, nil - } - - groupID, err := strconv.Atoi(externalName) - if err != nil { - return managed.ExternalObservation{}, errors.New(errIDNotInt) + if cr.Spec.ForProvider.GroupID == nil { + return managed.ExternalObservation{}, errors.New(errMissingGroupID) } - - groupMember, res, err := e.client.GetGroupMember(groupID, cr.Spec.ForProvider.UserID) + groupMember, res, err := e.client.GetGroupMember( + *cr.Spec.ForProvider.GroupID, + cr.Spec.ForProvider.UserID, + ) if err != nil { if clients.IsResponseNotFound(res) { return managed.ExternalObservation{}, nil @@ -136,7 +130,6 @@ func (e *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext return managed.ExternalCreation{}, errors.Wrap(err, errCreateFailed) } - meta.SetExternalName(cr, strconv.Itoa(*cr.Spec.ForProvider.GroupID)) return managed.ExternalCreation{ExternalNameAssigned: true}, nil } @@ -145,9 +138,11 @@ func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext if !ok { return managed.ExternalUpdate{}, errors.New(errNotMember) } - + if cr.Spec.ForProvider.GroupID == nil { + return managed.ExternalUpdate{}, errors.New(errMissingGroupID) + } _, _, err := e.client.EditGroupMember( - meta.GetExternalName(cr), + *cr.Spec.ForProvider.GroupID, cr.Spec.ForProvider.UserID, groups.GenerateEditMemberOptions(&cr.Spec.ForProvider), gitlab.WithContext(ctx), @@ -160,9 +155,11 @@ func (e *external) Delete(ctx context.Context, mg resource.Managed) error { if !ok { return errors.New(errNotMember) } - + if cr.Spec.ForProvider.GroupID == nil { + return errors.New(errMissingGroupID) + } _, err := e.client.RemoveGroupMember( - meta.GetExternalName(cr), + *cr.Spec.ForProvider.GroupID, cr.Spec.ForProvider.UserID, nil, gitlab.WithContext(ctx), diff --git a/pkg/controller/groups/members/controller_test.go b/pkg/controller/groups/members/controller_test.go index 2be025a5..a7f965ea 100644 --- a/pkg/controller/groups/members/controller_test.go +++ b/pkg/controller/groups/members/controller_test.go @@ -16,7 +16,6 @@ package members import ( "context" "net/http" - "strconv" "testing" "time" @@ -26,7 +25,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/crossplane/crossplane-runtime/pkg/test" @@ -37,20 +35,19 @@ import ( ) var ( - unexpecedItem resource.Managed - errBoom = errors.New("boom") - ID = 0 - extName = strconv.Itoa(ID) - extNameAnnotation = map[string]string{meta.AnnotationKeyExternalName: extName} - username = "username" - name = "name" - state = "state" - avatarURL = "http://avatarURL" - webURL = "http://webURL" - accessLevel = gitlab.AccessLevelValue(30) - now = time.Now() - expiresAt = gitlab.ISOTime(now.AddDate(0, 0, 7*3)) - expiresAtNew = gitlab.ISOTime(now.AddDate(0, 0, 7*4)) + unexpecedItem resource.Managed + errBoom = errors.New("boom") + ID = 0 + username = "username" + name = "name" + state = "state" + avatarURL = "http://avatarURL" + webURL = "http://webURL" + accessLevel = gitlab.AccessLevelValue(30) + now = time.Now() + expiresAt = gitlab.ISOTime(now.AddDate(0, 0, 7*3)) + expiresAtNew = gitlab.ISOTime(now.AddDate(0, 0, 7*4)) + groupID = 1234 ) type args struct { @@ -73,8 +70,8 @@ func withConditions(c ...xpv1.Condition) groupModifier { return func(cr *v1alpha1.Member) { cr.Status.ConditionedStatus.Conditions = c } } -func withExternalName(n string) groupModifier { - return func(r *v1alpha1.Member) { meta.SetExternalName(r, n) } +func withGroupID() groupModifier { + return func(r *v1alpha1.Member) { r.Spec.ForProvider.GroupID = &groupID } } func withStatus(s v1alpha1.MemberObservation) groupModifier { @@ -85,10 +82,6 @@ func withSpec(s v1alpha1.MemberParameters) groupModifier { return func(r *v1alpha1.Member) { r.Spec.ForProvider = s } } -func withAnnotations(a map[string]string) groupModifier { - return func(p *v1alpha1.Member) { meta.AddAnnotations(p, a) } -} - func groupMember(m ...groupModifier) *v1alpha1.Member { cr := &v1alpha1.Member{} for _, f := range m { @@ -165,33 +158,6 @@ func TestObserve(t *testing.T) { err: errors.New(errNotMember), }, }, - "NoExternalName": { - args: args{ - cr: groupMember(), - }, - want: want{ - cr: groupMember(), - result: managed.ExternalObservation{ - ResourceExists: false, - ResourceUpToDate: false, - ResourceLateInitialized: false, - }, - }, - }, - "NotIDExternalName": { - args: args{ - groupMember: &fake.MockClient{ - MockGetMember: func(gid interface{}, user int, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { - return &gitlab.GroupMember{}, &gitlab.Response{}, nil - }, - }, - cr: groupMember(withExternalName("fr")), - }, - want: want{ - cr: groupMember(withExternalName("fr")), - err: errors.New(errIDNotInt), - }, - }, "FailedGetRequest": { args: args{ groupMember: &fake.MockClient{ @@ -199,10 +165,10 @@ func TestObserve(t *testing.T) { return nil, &gitlab.Response{Response: &http.Response{StatusCode: 400}}, errBoom }, }, - cr: groupMember(withExternalName(extName)), + cr: groupMember(withGroupID()), }, want: want{ - cr: groupMember(withAnnotations(extNameAnnotation)), + cr: groupMember(withGroupID()), result: managed.ExternalObservation{ResourceExists: false}, err: errors.Wrap(errBoom, errGetFailed), }, @@ -214,10 +180,10 @@ func TestObserve(t *testing.T) { return nil, &gitlab.Response{Response: &http.Response{StatusCode: 404}}, errBoom }, }, - cr: groupMember(withExternalName(extName)), + cr: groupMember(withGroupID()), }, want: want{ - cr: groupMember(withAnnotations(extNameAnnotation)), + cr: groupMember(withGroupID()), result: managed.ExternalObservation{ResourceExists: false}, err: nil, }, @@ -230,14 +196,13 @@ func TestObserve(t *testing.T) { }, }, cr: groupMember( - withExternalName(extName), + withGroupID(), ), }, want: want{ cr: groupMember( withConditions(xpv1.Available()), - withAnnotations(extNameAnnotation), - withExternalName(extName), + withGroupID(), ), result: managed.ExternalObservation{ ResourceExists: true, @@ -256,15 +221,14 @@ func TestObserve(t *testing.T) { }, }, cr: groupMember( - withExternalName(extName), + withGroupID(), withAccessLevel(10), ), }, want: want{ cr: groupMember( withConditions(xpv1.Available()), - withAnnotations(extNameAnnotation), - withExternalName(extName), + withGroupID(), withAccessLevel(10), ), result: managed.ExternalObservation{ @@ -284,15 +248,14 @@ func TestObserve(t *testing.T) { }, }, cr: groupMember( - withExternalName(extName), + withGroupID(), withExpiresAt(expiresAtNew.String()), ), }, want: want{ cr: groupMember( withConditions(xpv1.Available()), - withAnnotations(extNameAnnotation), - withExternalName(extName), + withGroupID(), withExpiresAt(expiresAtNew.String()), ), result: managed.ExternalObservation{ @@ -361,13 +324,12 @@ func TestCreate(t *testing.T) { }, }, cr: groupMember( - withAnnotations(extNameAnnotation), withSpec(v1alpha1.MemberParameters{GroupID: &ID}), ), }, want: want{ cr: groupMember( - withExternalName(extName), + withGroupID(), withSpec(v1alpha1.MemberParameters{GroupID: &ID}), ), result: managed.ExternalCreation{ExternalNameAssigned: true}, @@ -393,13 +355,12 @@ func TestCreate(t *testing.T) { }, }, cr: groupMember( - withAnnotations(extNameAnnotation), withSpec(v1alpha1.MemberParameters{GroupID: &ID}), ), }, want: want{ cr: groupMember( - withExternalName(extName), + withGroupID(), withSpec(v1alpha1.MemberParameters{GroupID: &ID}), ), result: managed.ExternalCreation{ExternalNameAssigned: true}, @@ -413,13 +374,12 @@ func TestCreate(t *testing.T) { }, }, cr: groupMember( - withAnnotations(extNameAnnotation), withSpec(v1alpha1.MemberParameters{GroupID: &ID}), ), }, want: want{ cr: groupMember( - withExternalName(extName), + withGroupID(), withSpec(v1alpha1.MemberParameters{GroupID: &ID}), ), err: errors.Wrap(errBoom, errCreateFailed), @@ -481,13 +441,13 @@ func TestUpdate(t *testing.T) { }, }, cr: groupMember( - withExternalName(extName), + withGroupID(), withStatus(v1alpha1.MemberObservation{Username: "new username"}), ), }, want: want{ cr: groupMember( - withExternalName(extName), + withGroupID(), withStatus(v1alpha1.MemberObservation{Username: "new username"}), ), }, @@ -499,10 +459,10 @@ func TestUpdate(t *testing.T) { return &gitlab.GroupMember{}, &gitlab.Response{}, errBoom }, }, - cr: groupMember(), + cr: groupMember(withGroupID()), }, want: want{ - cr: groupMember(), + cr: groupMember(withGroupID()), err: errors.Wrap(errBoom, errUpdateFailed), }, }, @@ -551,10 +511,10 @@ func TestDelete(t *testing.T) { return &gitlab.Response{}, nil }, }, - cr: groupMember(withExternalName(extName)), + cr: groupMember(withGroupID()), }, want: want{ - cr: groupMember(withExternalName(extName)), + cr: groupMember(withGroupID()), err: nil, }, }, @@ -565,10 +525,10 @@ func TestDelete(t *testing.T) { return &gitlab.Response{}, errBoom }, }, - cr: groupMember(), + cr: groupMember(withGroupID()), }, want: want{ - cr: groupMember(), + cr: groupMember(withGroupID()), err: errors.Wrap(errBoom, errDeleteFailed), }, },