Skip to content

Commit

Permalink
Merge pull request #101 from lvvorovi/issue/11
Browse files Browse the repository at this point in the history
  • Loading branch information
@MisterMX
MisterMX authored Aug 10, 2023
2 parents 4dbdfdb + e69095f commit 9ec1f36
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 81 deletions.
32 changes: 16 additions & 16 deletions pkg/controller/projects/members/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package members

import (
"context"
"strconv"

"github.com/xanzy/go-gitlab"

Expand All @@ -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"

Expand All @@ -38,11 +36,11 @@ import (

const (
errNotMember = "managed resource is not a Gitlab Project Member custom resource"
errProjectIDMissing = "Project ID missing"
errCreateFailed = "cannot create Gitlab Project Member"
errUpdateFailed = "cannot update Gitlab Project Member"
errDeleteFailed = "cannot delete Gitlab Project Member"
errObserveFailed = "cannot observe Gitlab Project Member"
errProjectIDMissing = "ProjectID is missing"
)

// SetupMember adds a controller that reconciles Project Members.
Expand Down Expand Up @@ -87,18 +85,15 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
if !ok {
return managed.ExternalObservation{}, errors.New(errNotMember)
}

externalName := meta.GetExternalName(cr)
if externalName == "" {
return managed.ExternalObservation{ResourceExists: false}, nil
if cr.Spec.ForProvider.ProjectID == nil {
return managed.ExternalObservation{}, errors.New(errProjectIDMissing)
}

projectMemberID, err := strconv.Atoi(externalName)
if err != nil {
return managed.ExternalObservation{}, errors.New(errNotMember)
}
projectMember, res, err := e.client.GetProjectMember(
*cr.Spec.ForProvider.ProjectID,
cr.Spec.ForProvider.UserID,
)

projectMember, res, err := e.client.GetProjectMember(projectMemberID, cr.Spec.ForProvider.UserID)
if err != nil {
if clients.IsResponseNotFound(res) {
return managed.ExternalObservation{}, nil
Expand Down Expand Up @@ -134,7 +129,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.ProjectID))
return managed.ExternalCreation{ExternalNameAssigned: true}, nil
}

Expand All @@ -143,8 +137,12 @@ func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
if !ok {
return managed.ExternalUpdate{}, errors.New(errNotMember)
}
if cr.Spec.ForProvider.ProjectID == nil {
return managed.ExternalUpdate{}, errors.New(errProjectIDMissing)
}

_, _, err := e.client.EditProjectMember(
meta.GetExternalName(cr),
*cr.Spec.ForProvider.ProjectID,
cr.Spec.ForProvider.UserID,
projects.GenerateEditMemberOptions(&cr.Spec.ForProvider),
gitlab.WithContext(ctx),
Expand All @@ -157,9 +155,12 @@ func (e *external) Delete(ctx context.Context, mg resource.Managed) error {
if !ok {
return errors.New(errNotMember)
}
if cr.Spec.ForProvider.ProjectID == nil {
return errors.New(errProjectIDMissing)
}

_, err := e.client.DeleteProjectMember(
meta.GetExternalName(cr),
*cr.Spec.ForProvider.ProjectID,
cr.Spec.ForProvider.UserID,
gitlab.WithContext(ctx),
)
Expand All @@ -168,7 +169,6 @@ func (e *external) Delete(ctx context.Context, mg resource.Managed) error {

// isMemberUpToDate checks whether there is a change in any of the modifiable fields.
func isMemberUpToDate(p *v1alpha1.MemberParameters, g *gitlab.ProjectMember) bool { // nolint:gocyclo

if !cmp.Equal(int(p.AccessLevel), int(g.AccessLevel)) {
return false
}
Expand Down
110 changes: 45 additions & 65 deletions pkg/controller/projects/members/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package members
import (
"context"
"net/http"
"strconv"
"testing"
"time"

Expand All @@ -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"
Expand All @@ -37,21 +35,20 @@ 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"
email = "[email protected]"
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"
email = "[email protected]"
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))
projectID = 1234
)

type args struct {
Expand All @@ -74,8 +71,8 @@ func withConditions(c ...xpv1.Condition) projectModifier {
return func(cr *v1alpha1.Member) { cr.Status.ConditionedStatus.Conditions = c }
}

func withExternalName(n string) projectModifier {
return func(r *v1alpha1.Member) { meta.SetExternalName(r, n) }
func withProjectID() projectModifier {
return func(r *v1alpha1.Member) { r.Spec.ForProvider.ProjectID = &projectID }
}

func withStatus(s v1alpha1.MemberObservation) projectModifier {
Expand All @@ -86,10 +83,6 @@ func withSpec(s v1alpha1.MemberParameters) projectModifier {
return func(r *v1alpha1.Member) { r.Spec.ForProvider = s }
}

func withAnnotations(a map[string]string) projectModifier {
return func(p *v1alpha1.Member) { meta.AddAnnotations(p, a) }
}

func projectMember(m ...projectModifier) *v1alpha1.Member {
cr := &v1alpha1.Member{}
for _, f := range m {
Expand Down Expand Up @@ -166,7 +159,7 @@ func TestObserve(t *testing.T) {
err: errors.New(errNotMember),
},
},
"NoExternalName": {
"ErrProjectIDMissing": {
args: args{
cr: projectMember(),
},
Expand All @@ -177,20 +170,7 @@ func TestObserve(t *testing.T) {
ResourceUpToDate: false,
ResourceLateInitialized: false,
},
},
},
"NotIDExternalName": {
args: args{
projectMember: &fake.MockClient{
MockGetMember: func(pid interface{}, user int, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectMember, *gitlab.Response, error) {
return &gitlab.ProjectMember{}, &gitlab.Response{}, nil
},
},
cr: projectMember(withExternalName("fr")),
},
want: want{
cr: projectMember(withExternalName("fr")),
err: errors.New(errNotMember),
err: errors.New(errProjectIDMissing),
},
},
"ErrGet404": {
Expand All @@ -200,10 +180,10 @@ func TestObserve(t *testing.T) {
return nil, &gitlab.Response{Response: &http.Response{StatusCode: 404}}, errBoom
},
},
cr: projectMember(withExternalName(extName)),
cr: projectMember(withProjectID()),
},
want: want{
cr: projectMember(withAnnotations(extNameAnnotation)),
cr: projectMember(withProjectID()),
result: managed.ExternalObservation{ResourceExists: false},
err: nil,
},
Expand All @@ -215,10 +195,10 @@ func TestObserve(t *testing.T) {
return nil, nil, errBoom
},
},
cr: projectMember(withExternalName(extName)),
cr: projectMember(withProjectID()),
},
want: want{
cr: projectMember(withAnnotations(extNameAnnotation)),
cr: projectMember(withProjectID()),
result: managed.ExternalObservation{ResourceExists: false},
err: errors.Wrap(errBoom, errObserveFailed),
},
Expand All @@ -231,14 +211,14 @@ func TestObserve(t *testing.T) {
},
},
cr: projectMember(
withExternalName(extName),
withProjectID(),
),
},
want: want{
cr: projectMember(
withConditions(xpv1.Available()),
withAnnotations(extNameAnnotation),
withExternalName(extName),
withProjectID(),
withProjectID(),
),
result: managed.ExternalObservation{
ResourceExists: true,
Expand All @@ -257,15 +237,15 @@ func TestObserve(t *testing.T) {
},
},
cr: projectMember(
withExternalName(extName),
withProjectID(),
withAccessLevel(10),
),
},
want: want{
cr: projectMember(
withConditions(xpv1.Available()),
withAnnotations(extNameAnnotation),
withExternalName(extName),
withProjectID(),
withProjectID(),
withAccessLevel(10),
),
result: managed.ExternalObservation{
Expand All @@ -285,15 +265,15 @@ func TestObserve(t *testing.T) {
},
},
cr: projectMember(
withExternalName(extName),
withProjectID(),
withExpiresAt(expiresAtNew.String()),
),
},
want: want{
cr: projectMember(
withConditions(xpv1.Available()),
withAnnotations(extNameAnnotation),
withExternalName(extName),
withProjectID(),
withProjectID(),
withExpiresAt(expiresAtNew.String()),
),
result: managed.ExternalObservation{
Expand Down Expand Up @@ -363,13 +343,13 @@ func TestCreate(t *testing.T) {
},
},
cr: projectMember(
withAnnotations(extNameAnnotation),
withProjectID(),
withSpec(v1alpha1.MemberParameters{ProjectID: &ID}),
),
},
want: want{
cr: projectMember(
withExternalName(extName),
withProjectID(),
withSpec(v1alpha1.MemberParameters{ProjectID: &ID}),
),
result: managed.ExternalCreation{ExternalNameAssigned: true},
Expand All @@ -396,13 +376,13 @@ func TestCreate(t *testing.T) {
},
},
cr: projectMember(
withAnnotations(extNameAnnotation),
withProjectID(),
withSpec(v1alpha1.MemberParameters{ProjectID: &ID}),
),
},
want: want{
cr: projectMember(
withExternalName(extName),
withProjectID(),
withSpec(v1alpha1.MemberParameters{ProjectID: &ID}),
),
result: managed.ExternalCreation{ExternalNameAssigned: true},
Expand All @@ -416,13 +396,13 @@ func TestCreate(t *testing.T) {
},
},
cr: projectMember(
withAnnotations(extNameAnnotation),
withProjectID(),
withSpec(v1alpha1.MemberParameters{ProjectID: &ID}),
),
},
want: want{
cr: projectMember(
withExternalName(extName),
withProjectID(),
withSpec(v1alpha1.MemberParameters{ProjectID: &ID}),
),
err: errors.Wrap(errBoom, errCreateFailed),
Expand Down Expand Up @@ -485,13 +465,13 @@ func TestUpdate(t *testing.T) {
},
},
cr: projectMember(
withExternalName(extName),
withProjectID(),
withStatus(v1alpha1.MemberObservation{Username: "new username"}),
),
},
want: want{
cr: projectMember(
withExternalName(extName),
withProjectID(),
withStatus(v1alpha1.MemberObservation{Username: "new username"}),
),
},
Expand All @@ -503,10 +483,10 @@ func TestUpdate(t *testing.T) {
return &gitlab.ProjectMember{}, &gitlab.Response{}, errBoom
},
},
cr: projectMember(),
cr: projectMember(withProjectID()),
},
want: want{
cr: projectMember(),
cr: projectMember(withProjectID()),
err: errors.Wrap(errBoom, errUpdateFailed),
},
},
Expand Down Expand Up @@ -555,10 +535,10 @@ func TestDelete(t *testing.T) {
return &gitlab.Response{}, nil
},
},
cr: projectMember(withExternalName(extName)),
cr: projectMember(withProjectID()),
},
want: want{
cr: projectMember(withExternalName(extName)),
cr: projectMember(withProjectID()),
err: nil,
},
},
Expand All @@ -569,10 +549,10 @@ func TestDelete(t *testing.T) {
return &gitlab.Response{}, errBoom
},
},
cr: projectMember(),
cr: projectMember(withProjectID()),
},
want: want{
cr: projectMember(),
cr: projectMember(withProjectID()),
err: errors.Wrap(errBoom, errDeleteFailed),
},
},
Expand Down

0 comments on commit 9ec1f36

Please sign in to comment.