diff --git a/apis/generate.go b/apis/generate.go index ad201672..baeffd27 100644 --- a/apis/generate.go +++ b/apis/generate.go @@ -32,7 +32,6 @@ limitations under the License. package apis import ( - _ "sigs.k8s.io/controller-tools/cmd/controller-gen" //nolint:typecheck - _ "github.com/crossplane/crossplane-tools/cmd/angryjet" //nolint:typecheck + _ "sigs.k8s.io/controller-tools/cmd/controller-gen" //nolint:typecheck ) diff --git a/apis/projects/v1alpha1/hook_types.go b/apis/projects/v1alpha1/hook_types.go index 03108a4f..7f6bbf98 100644 --- a/apis/projects/v1alpha1/hook_types.go +++ b/apis/projects/v1alpha1/hook_types.go @@ -89,8 +89,11 @@ type HookParameters struct { EnableSSLVerification *bool `json:"enableSslVerification,omitempty"` // Token is the secret token to validate received payloads. - // +optional - Token *string `json:"token,omitempty"` + Token *Token `json:"token"` +} + +type Token struct { + SecretRef *xpv1.SecretKeySelector `json:"secretRef"` } // HookObservation represents a project hook. diff --git a/apis/projects/v1alpha1/zz_generated.deepcopy.go b/apis/projects/v1alpha1/zz_generated.deepcopy.go index 773ffed1..886776ce 100644 --- a/apis/projects/v1alpha1/zz_generated.deepcopy.go +++ b/apis/projects/v1alpha1/zz_generated.deepcopy.go @@ -768,8 +768,8 @@ func (in *HookParameters) DeepCopyInto(out *HookParameters) { } if in.Token != nil { in, out := &in.Token, &out.Token - *out = new(string) - **out = **in + *out = new(Token) + (*in).DeepCopyInto(*out) } } @@ -1812,6 +1812,26 @@ func (in *StorageStatistics) DeepCopy() *StorageStatistics { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Token) DeepCopyInto(out *Token) { + *out = *in + if in.SecretRef != nil { + in, out := &in.SecretRef, &out.SecretRef + *out = new(v1.SecretKeySelector) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Token. +func (in *Token) DeepCopy() *Token { + if in == nil { + return nil + } + out := new(Token) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *User) DeepCopyInto(out *User) { *out = *in diff --git a/go.mod b/go.mod index 388cf877..414d7eca 100644 --- a/go.mod +++ b/go.mod @@ -71,7 +71,7 @@ require ( go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.26.0 golang.org/x/mod v0.14.0 // indirect - golang.org/x/net v0.23.0 // indirect + golang.org/x/net v0.23.0 golang.org/x/oauth2 v0.21.0 // indirect golang.org/x/sys v0.20.0 // indirect golang.org/x/term v0.18.0 // indirect diff --git a/package/crds/projects.gitlab.crossplane.io_hooks.yaml b/package/crds/projects.gitlab.crossplane.io_hooks.yaml index 59893133..56c00ce6 100644 --- a/package/crds/projects.gitlab.crossplane.io_hooks.yaml +++ b/package/crds/projects.gitlab.crossplane.io_hooks.yaml @@ -192,7 +192,28 @@ spec: type: boolean token: description: Token is the secret token to validate received payloads. - type: string + properties: + secretRef: + description: A SecretKeySelector is a reference to a secret + key in an arbitrary namespace. + properties: + key: + description: The key to select. + type: string + name: + description: Name of the secret. + type: string + namespace: + description: Namespace of the secret. + type: string + required: + - key + - name + - namespace + type: object + required: + - secretRef + type: object url: description: URL is the hook URL. type: string @@ -200,6 +221,7 @@ spec: description: WikiPageEvents triggers hook on wiki events. type: boolean required: + - token - url type: object managementPolicies: diff --git a/pkg/clients/projects/hook.go b/pkg/clients/projects/hook.go index 980c26a8..0845041c 100644 --- a/pkg/clients/projects/hook.go +++ b/pkg/clients/projects/hook.go @@ -20,8 +20,13 @@ import ( "strings" "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" "github.com/xanzy/go-gitlab" + "golang.org/x/net/context" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane-contrib/provider-gitlab/apis/projects/v1alpha1" "github.com/crossplane-contrib/provider-gitlab/pkg/clients" @@ -114,7 +119,13 @@ func GenerateHookObservation(hook *gitlab.ProjectHook) v1alpha1.HookObservation } // GenerateCreateHookOptions generates project creation options -func GenerateCreateHookOptions(p *v1alpha1.HookParameters) *gitlab.AddProjectHookOptions { +func GenerateCreateHookOptions(p *v1alpha1.HookParameters, client client.Client, ctx context.Context) (*gitlab.AddProjectHookOptions, error) { + token, err := getTokenValueFromSecret(p, client, ctx) + + if err != nil { + return nil, err + } + hook := &gitlab.AddProjectHookOptions{ URL: p.URL, ConfidentialNoteEvents: p.ConfidentialNoteEvents, @@ -129,14 +140,39 @@ func GenerateCreateHookOptions(p *v1alpha1.HookParameters) *gitlab.AddProjectHoo PipelineEvents: p.PipelineEvents, WikiPageEvents: p.WikiPageEvents, EnableSSLVerification: p.EnableSSLVerification, - Token: p.Token, + Token: token, } - return hook + return hook, nil +} + +func getTokenValueFromSecret(p *v1alpha1.HookParameters, client client.Client, ctx context.Context) (*string, error) { + secret := &v1.Secret{} + + if err := client.Get(ctx, types.NamespacedName{Name: p.Token.SecretRef.Name, Namespace: p.Token.SecretRef.Namespace}, secret); err != nil { + return nil, errors.Wrap(err, "Cannot get referenced Secret") + + } + + value := secret.Data[p.Token.SecretRef.Key] + + if value == nil { + return nil, errors.Errorf("Could not find key %v in the referenced secret", p.Token.SecretRef.Key) + } + + data := string(value) + + return &data, nil } // GenerateEditHookOptions generates project edit options -func GenerateEditHookOptions(p *v1alpha1.HookParameters) *gitlab.EditProjectHookOptions { +func GenerateEditHookOptions(p *v1alpha1.HookParameters, client client.Client, ctx context.Context) (*gitlab.EditProjectHookOptions, error) { + token, err := getTokenValueFromSecret(p, client, ctx) + + if err != nil { + return nil, err + } + o := &gitlab.EditProjectHookOptions{ URL: p.URL, ConfidentialNoteEvents: p.ConfidentialNoteEvents, @@ -151,10 +187,10 @@ func GenerateEditHookOptions(p *v1alpha1.HookParameters) *gitlab.EditProjectHook PipelineEvents: p.PipelineEvents, WikiPageEvents: p.WikiPageEvents, EnableSSLVerification: p.EnableSSLVerification, - Token: p.Token, + Token: token, } - return o + return o, nil } // IsHookUpToDate checks whether there is a change in any of the modifiable fields. diff --git a/pkg/clients/projects/hook_test.go b/pkg/clients/projects/hook_test.go index 29c5cc66..a6adb065 100644 --- a/pkg/clients/projects/hook_test.go +++ b/pkg/clients/projects/hook_test.go @@ -17,12 +17,18 @@ limitations under the License. package projects import ( + "context" "testing" "time" + v1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/errors" + "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/google/go-cmp/cmp" "github.com/xanzy/go-gitlab" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/crossplane-contrib/provider-gitlab/apis/projects/v1alpha1" ) @@ -41,7 +47,13 @@ var ( pipelineEvents = true wikiPageEvents = true enableSSLVerification = true - token = "84B9C651-9025-47D2-9124-DD951BD268E8" + token = v1alpha1.Token{ + SecretRef: &v1.SecretKeySelector{ + Key: "token", SecretReference: v1.SecretReference{Name: "test", Namespace: "test"}, + }, + } + + tokenValue = "84B9C651-9025-47D2-9124-DD951BD268E8" ) func TestGenerateHookObservation(t *testing.T) { @@ -128,10 +140,15 @@ func TestLateInitializeHook(t *testing.T) { func TestGenerateCreateHookOptions(t *testing.T) { type args struct { parameters *v1alpha1.HookParameters + secret *corev1.Secret + } + type want struct { + addProjectHookOptions *gitlab.AddProjectHookOptions + err error } cases := map[string]struct { args args - want *gitlab.AddProjectHookOptions + want want }{ "AllFields": { args: args{ @@ -149,24 +166,32 @@ func TestGenerateCreateHookOptions(t *testing.T) { PipelineEvents: &pipelineEvents, WikiPageEvents: &wikiPageEvents, EnableSSLVerification: &enableSSLVerification, - Token: &token, + Token: &token}, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "test"}, + Data: map[string][]byte{ + "token": []byte(tokenValue), + }, }, }, - want: &gitlab.AddProjectHookOptions{ - URL: &url, - ConfidentialNoteEvents: &confidentialNoteEvents, - PushEvents: &pushEvents, - PushEventsBranchFilter: &pushEventsBranchFilter, - IssuesEvents: &issuesEvents, - ConfidentialIssuesEvents: &confidentialIssuesEvents, - MergeRequestsEvents: &mergeRequestsEvents, - TagPushEvents: &tagPushEvents, - NoteEvents: ¬eEvents, - JobEvents: &jobEvents, - PipelineEvents: &pipelineEvents, - WikiPageEvents: &wikiPageEvents, - EnableSSLVerification: &enableSSLVerification, - Token: &token, + want: want{ + err: nil, + addProjectHookOptions: &gitlab.AddProjectHookOptions{ + URL: &url, + ConfidentialNoteEvents: &confidentialNoteEvents, + PushEvents: &pushEvents, + PushEventsBranchFilter: &pushEventsBranchFilter, + IssuesEvents: &issuesEvents, + ConfidentialIssuesEvents: &confidentialIssuesEvents, + MergeRequestsEvents: &mergeRequestsEvents, + TagPushEvents: &tagPushEvents, + NoteEvents: ¬eEvents, + JobEvents: &jobEvents, + PipelineEvents: &pipelineEvents, + WikiPageEvents: &wikiPageEvents, + EnableSSLVerification: &enableSSLVerification, + Token: &tokenValue, + }, }, }, "SomeFields": { @@ -175,19 +200,78 @@ func TestGenerateCreateHookOptions(t *testing.T) { PushEvents: &pushEvents, PushEventsBranchFilter: &pushEventsBranchFilter, IssuesEvents: &issuesEvents, + Token: &token, }, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "test"}, + Data: map[string][]byte{ + "token": []byte(tokenValue), + }, + }, + }, + want: want{ + err: nil, + addProjectHookOptions: &gitlab.AddProjectHookOptions{ + PushEvents: &pushEvents, + PushEventsBranchFilter: &pushEventsBranchFilter, + IssuesEvents: &issuesEvents, + Token: &tokenValue, + }, + }, + }, + "FailNoSecret": { + args: args{ + parameters: &v1alpha1.HookParameters{ + PushEvents: &pushEvents, + PushEventsBranchFilter: &pushEventsBranchFilter, + IssuesEvents: &issuesEvents, + Token: &token, + }, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "other", Name: "other"}, + Data: map[string][]byte{ + "token": []byte(tokenValue), + }, + }, + }, + want: want{ + err: errors.New(`Cannot get referenced Secret: secrets "test" not found`), + addProjectHookOptions: nil, }, - want: &gitlab.AddProjectHookOptions{ - PushEvents: &pushEvents, - PushEventsBranchFilter: &pushEventsBranchFilter, - IssuesEvents: &issuesEvents, + }, + "FailWrongKey": { + args: args{ + parameters: &v1alpha1.HookParameters{ + PushEvents: &pushEvents, + PushEventsBranchFilter: &pushEventsBranchFilter, + IssuesEvents: &issuesEvents, + Token: &token, + }, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "test"}, + Data: map[string][]byte{ + "wrongKey": []byte(tokenValue), + }, + }, + }, + want: want{ + err: errors.New("Could not find key token in the referenced secret"), + addProjectHookOptions: nil, }, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - got := GenerateCreateHookOptions(tc.args.parameters) - if diff := cmp.Diff(tc.want, got); diff != "" { + + client := fake.NewClientBuilder().WithObjects(tc.args.secret).Build() + + got, err := GenerateCreateHookOptions(tc.args.parameters, client, context.Background()) + if err != nil && tc.want.err != nil { + if diff := cmp.Diff(tc.want.err.Error(), err.Error(), test.EquateErrors()); diff != "" { + t.Errorf("r: -want, +got:\n%s", diff) + } + } + if diff := cmp.Diff(tc.want.addProjectHookOptions, got); diff != "" { t.Errorf("r: -want, +got:\n%s", diff) } }) @@ -234,13 +318,21 @@ func TestGenerateEditHookOptions(t *testing.T) { PipelineEvents: &pipelineEvents, WikiPageEvents: &wikiPageEvents, EnableSSLVerification: &enableSSLVerification, - Token: &token, + Token: &tokenValue, }, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - got := GenerateEditHookOptions(tc.args.parameters) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "test"}, + Data: map[string][]byte{ + "token": []byte(tokenValue), + }, + } + client := fake.NewClientBuilder().WithObjects(secret).Build() + got, _ := GenerateEditHookOptions(tc.args.parameters, client, context.Background()) + if diff := cmp.Diff(tc.want, got); diff != "" { t.Errorf("r: -want, +got:\n%s", diff) } diff --git a/pkg/controller/projects/hooks/controller.go b/pkg/controller/projects/hooks/controller.go index fb5b1543..9d0b93e1 100644 --- a/pkg/controller/projects/hooks/controller.go +++ b/pkg/controller/projects/hooks/controller.go @@ -48,6 +48,7 @@ const ( errCreateFailed = "cannot create Gitlab project hook" errUpdateFailed = "cannot update Gitlab project hook" errDeleteFailed = "cannot delete Gitlab project hook" + errSecretRefInvalid = "invalid token reference" ) // SetupHook adds a controller that reconciles Hooks. @@ -152,7 +153,14 @@ func (e *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext } cr.Status.SetConditions(xpv1.Creating()) - hook, _, err := e.client.AddProjectHook(*cr.Spec.ForProvider.ProjectID, projects.GenerateCreateHookOptions(&cr.Spec.ForProvider), gitlab.WithContext(ctx)) + hookOptions, err := projects.GenerateCreateHookOptions(&cr.Spec.ForProvider, e.kube, ctx) + + if err != nil { + return managed.ExternalCreation{}, errors.Wrap(err, errSecretRefInvalid) + } + + hook, _, err := e.client.AddProjectHook(*cr.Spec.ForProvider.ProjectID, hookOptions, gitlab.WithContext(ctx)) + if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, errCreateFailed) } @@ -170,11 +178,18 @@ func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext if err != nil { return managed.ExternalUpdate{}, errors.New(errNotHook) } + if cr.Spec.ForProvider.ProjectID == nil { return managed.ExternalUpdate{}, errors.New(errProjectIDMissing) } - _, _, err = e.client.EditProjectHook(*cr.Spec.ForProvider.ProjectID, hookid, projects.GenerateEditHookOptions(&cr.Spec.ForProvider), gitlab.WithContext(ctx)) + editHookOptions, err := projects.GenerateEditHookOptions(&cr.Spec.ForProvider, e.kube, ctx) + + if err != nil { + return managed.ExternalUpdate{}, errors.New(errSecretRefInvalid) + } + + _, _, err = e.client.EditProjectHook(*cr.Spec.ForProvider.ProjectID, hookid, editHookOptions, gitlab.WithContext(ctx)) if err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateFailed) } diff --git a/pkg/controller/projects/hooks/controller_test.go b/pkg/controller/projects/hooks/controller_test.go index 24cf1b82..c1dabf92 100644 --- a/pkg/controller/projects/hooks/controller_test.go +++ b/pkg/controller/projects/hooks/controller_test.go @@ -23,6 +23,7 @@ import ( "testing" "time" + v1 "github.com/crossplane/crossplane-runtime/apis/common/v1" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" @@ -30,6 +31,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/pkg/errors" "github.com/xanzy/go-gitlab" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -43,6 +45,13 @@ var ( createTime = time.Now() projectID = 5678 projectHookID = 1234 + tokenValue = "test" + tokenSecret = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "test"}, + Data: map[string][]byte{ + "token": []byte(tokenValue), + }, + } ) type args struct { @@ -75,7 +84,11 @@ func withDefaultValues() projectHookModifier { PipelineEvents: &f, WikiPageEvents: &f, EnableSSLVerification: &f, - Token: nil, + Token: &v1alpha1.Token{ + SecretRef: &v1.SecretKeySelector{ + Key: "token", SecretReference: v1.SecretReference{Name: "test", Namespace: "test"}, + }, + }, } } } @@ -86,6 +99,16 @@ func withProjectID(pid int) projectHookModifier { } } +func withTokenRef() projectHookModifier { + return func(r *v1alpha1.Hook) { + r.Spec.ForProvider.Token = &v1alpha1.Token{ + SecretRef: &v1.SecretKeySelector{ + Key: "token", SecretReference: v1.SecretReference{Name: "test", Namespace: "test"}, + }, + } + } +} + func withStatus(s v1alpha1.HookObservation) projectHookModifier { return func(r *v1alpha1.Hook) { r.Status.AtProvider = s } } @@ -180,6 +203,7 @@ func TestObserve(t *testing.T) { }, cr: projecthook( withProjectID(projectID), + withTokenRef(), withExternalName(projectHookID), withStatus(v1alpha1.HookObservation{ ID: projectHookID, @@ -256,6 +280,10 @@ func TestCreate(t *testing.T) { args: args{ kube: &test.MockClient{ MockUpdate: test.NewMockUpdateFn(nil), + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + *obj.(*corev1.Secret) = tokenSecret + return nil + }), }, projecthook: &fake.MockClient{ MockAddHook: func(pid interface{}, opt *gitlab.AddProjectHookOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectHook, *gitlab.Response, error) { @@ -277,6 +305,12 @@ func TestCreate(t *testing.T) { }, "FailedCreation": { args: args{ + kube: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + *obj.(*corev1.Secret) = tokenSecret + return nil + }), + }, projecthook: &fake.MockClient{ MockAddHook: func(pid interface{}, opt *gitlab.AddProjectHookOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectHook, *gitlab.Response, error) { return &gitlab.ProjectHook{}, &gitlab.Response{}, errBoom @@ -324,8 +358,14 @@ func TestUpdate(t *testing.T) { args want }{ - "SuccessfulEditProject": { + "SuccessfulEditHook": { args: args{ + kube: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + *obj.(*corev1.Secret) = tokenSecret + return nil + }), + }, projecthook: &fake.MockClient{ MockEditHook: func(pid interface{}, hook int, opt *gitlab.EditProjectHookOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectHook, *gitlab.Response, error) { return &gitlab.ProjectHook{}, &gitlab.Response{}, nil @@ -334,12 +374,14 @@ func TestUpdate(t *testing.T) { cr: projecthook( withExternalName(projectHookID), withProjectID(projectID), + withTokenRef(), withStatus(v1alpha1.HookObservation{ID: projectHookID}), ), }, want: want{ cr: projecthook( withExternalName(projectHookID), + withTokenRef(), withProjectID(projectID), withStatus(v1alpha1.HookObservation{ID: projectHookID}), ), @@ -347,6 +389,13 @@ func TestUpdate(t *testing.T) { }, "FailedEdit": { args: args{ + kube: &test.MockClient{ + MockUpdate: test.NewMockUpdateFn(nil), + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + *obj.(*corev1.Secret) = tokenSecret + return nil + }), + }, projecthook: &fake.MockClient{ MockEditHook: func(pid interface{}, hook int, opt *gitlab.EditProjectHookOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectHook, *gitlab.Response, error) { return &gitlab.ProjectHook{}, &gitlab.Response{}, errBoom @@ -355,6 +404,7 @@ func TestUpdate(t *testing.T) { cr: projecthook( withExternalName(projectHookID), withProjectID(projectID), + withTokenRef(), withStatus(v1alpha1.HookObservation{ID: projectHookID}), ), }, @@ -362,6 +412,7 @@ func TestUpdate(t *testing.T) { cr: projecthook( withExternalName(projectHookID), withProjectID(projectID), + withTokenRef(), withStatus(v1alpha1.HookObservation{ID: projectHookID}), ), err: errors.Wrap(errBoom, errUpdateFailed),