From 3d1e11d1a73e40cd863007eec2d42bd0ccd4ea40 Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Fri, 14 Jan 2022 16:41:43 +0100 Subject: [PATCH 1/6] Add User and OrganizationMembers Resources --- apis/v1/organizationmembers_type.go | 43 ++++ apis/v1/user_types.go | 44 ++++ apis/v1/zz_generated.deepcopy.go | 219 ++++++++++++++++++ .../base/appuio.io_organizationmembers.yaml | 70 ++++++ .../v1/base/appuio.io_users.yaml | 65 ++++++ 5 files changed, 441 insertions(+) create mode 100644 apis/v1/organizationmembers_type.go create mode 100644 apis/v1/user_types.go create mode 100644 config/crd/apiextensions.k8s.io/v1/base/appuio.io_organizationmembers.yaml create mode 100644 config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml diff --git a/apis/v1/organizationmembers_type.go b/apis/v1/organizationmembers_type.go new file mode 100644 index 00000000..23e18e2c --- /dev/null +++ b/apis/v1/organizationmembers_type.go @@ -0,0 +1,43 @@ +package v1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status + +// OrganizationMembers is the collection of members of an organization +type OrganizationMembers struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec OrganizationMembersSpec `json:"spec,omitempty"` + Status OrganizationMembersStatus `json:"status,omitempty"` +} + +type OrganizationMembersSpec struct { + UserRefs []UserRef `json:"userRefs,omitempty"` +} +type OrganizationMembersStatus struct { + UserRefs []UserRef `json:"resolvedUserRefs,omitempty"` +} + +type UserRef struct { + ID string `json:"id,omitempty"` + Username string `json:"username,omitempty"` +} + +// +kubebuilder:object:root=true + +// OrganizationMembersList contains a list of OrganizationMembers resources +type OrganizationMembersList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + + Items []OrganizationMembers `json:"items"` +} + +func init() { + SchemeBuilder.Register(&OrganizationMembers{}, &OrganizationMembersList{}) +} diff --git a/apis/v1/user_types.go b/apis/v1/user_types.go new file mode 100644 index 00000000..1d0ac77f --- /dev/null +++ b/apis/v1/user_types.go @@ -0,0 +1,44 @@ +package v1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// +kubebuilder:object:root=true +// +kubebuilder:resource:scope=Cluster +// +kubebuilder:subresource:status + +// User is a representation of a APPUiO Cloud user +type User struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec UserSpec `json:"spec,omitempty"` + Status UserStatus `json:"status,omitempty"` +} +type UserSpec struct { + Preferences UserPreferences `json:"Preferences,omitempty"` +} +type UserPreferences struct { + DefaultOrganizationRef string `json:"defaultOrganizationRef,omitempty"` +} +type UserStatus struct { + DefaultOrganizationRef string `json:"defaultOrganization,omitempty"` + DisplayName string `json:"displayName,omitempty"` + Username string `json:"username,omitempty"` + Email string `json:"email,omitempty"` +} + +// +kubebuilder:object:root=true + +// UserList contains a list of Users. +type UserList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + + Items []User `json:"items"` +} + +func init() { + SchemeBuilder.Register(&User{}, &UserList{}) +} diff --git a/apis/v1/zz_generated.deepcopy.go b/apis/v1/zz_generated.deepcopy.go index 7a4ca881..505e2153 100644 --- a/apis/v1/zz_generated.deepcopy.go +++ b/apis/v1/zz_generated.deepcopy.go @@ -50,6 +50,105 @@ func (in Features) DeepCopy() Features { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OrganizationMembers) DeepCopyInto(out *OrganizationMembers) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OrganizationMembers. +func (in *OrganizationMembers) DeepCopy() *OrganizationMembers { + if in == nil { + return nil + } + out := new(OrganizationMembers) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *OrganizationMembers) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OrganizationMembersList) DeepCopyInto(out *OrganizationMembersList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]OrganizationMembers, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OrganizationMembersList. +func (in *OrganizationMembersList) DeepCopy() *OrganizationMembersList { + if in == nil { + return nil + } + out := new(OrganizationMembersList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *OrganizationMembersList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OrganizationMembersSpec) DeepCopyInto(out *OrganizationMembersSpec) { + *out = *in + if in.UserRefs != nil { + in, out := &in.UserRefs, &out.UserRefs + *out = make([]UserRef, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OrganizationMembersSpec. +func (in *OrganizationMembersSpec) DeepCopy() *OrganizationMembersSpec { + if in == nil { + return nil + } + out := new(OrganizationMembersSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OrganizationMembersStatus) DeepCopyInto(out *OrganizationMembersStatus) { + *out = *in + if in.UserRefs != nil { + in, out := &in.UserRefs, &out.UserRefs + *out = make([]UserRef, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OrganizationMembersStatus. +func (in *OrganizationMembersStatus) DeepCopy() *OrganizationMembersStatus { + if in == nil { + return nil + } + out := new(OrganizationMembersStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in URLMap) DeepCopyInto(out *URLMap) { { @@ -71,6 +170,126 @@ func (in URLMap) DeepCopy() URLMap { 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 + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new User. +func (in *User) DeepCopy() *User { + if in == nil { + return nil + } + out := new(User) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *User) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *UserList) DeepCopyInto(out *UserList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]User, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserList. +func (in *UserList) DeepCopy() *UserList { + if in == nil { + return nil + } + out := new(UserList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *UserList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *UserPreferences) DeepCopyInto(out *UserPreferences) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserPreferences. +func (in *UserPreferences) DeepCopy() *UserPreferences { + if in == nil { + return nil + } + out := new(UserPreferences) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *UserRef) DeepCopyInto(out *UserRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserRef. +func (in *UserRef) DeepCopy() *UserRef { + if in == nil { + return nil + } + out := new(UserRef) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *UserSpec) DeepCopyInto(out *UserSpec) { + *out = *in + out.Preferences = in.Preferences +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserSpec. +func (in *UserSpec) DeepCopy() *UserSpec { + if in == nil { + return nil + } + out := new(UserSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *UserStatus) DeepCopyInto(out *UserStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserStatus. +func (in *UserStatus) DeepCopy() *UserStatus { + if in == nil { + return nil + } + out := new(UserStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Zone) DeepCopyInto(out *Zone) { *out = *in diff --git a/config/crd/apiextensions.k8s.io/v1/base/appuio.io_organizationmembers.yaml b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_organizationmembers.yaml new file mode 100644 index 00000000..662b61da --- /dev/null +++ b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_organizationmembers.yaml @@ -0,0 +1,70 @@ + +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.7.0 + creationTimestamp: null + name: organizationmembers.appuio.io +spec: + group: appuio.io + names: + kind: OrganizationMembers + listKind: OrganizationMembersList + plural: organizationmembers + singular: organizationmembers + scope: Namespaced + versions: + - name: v1 + schema: + openAPIV3Schema: + description: OrganizationMembers is the collection of members of an organization + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + properties: + userRefs: + items: + properties: + id: + type: string + username: + type: string + type: object + type: array + type: object + status: + properties: + resolvedUserRefs: + items: + properties: + id: + type: string + username: + type: string + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml new file mode 100644 index 00000000..9ec48c3f --- /dev/null +++ b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml @@ -0,0 +1,65 @@ + +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.7.0 + creationTimestamp: null + name: users.appuio.io +spec: + group: appuio.io + names: + kind: User + listKind: UserList + plural: users + singular: user + scope: Cluster + versions: + - name: v1 + schema: + openAPIV3Schema: + description: User is a representation of a APPUiO Cloud user + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + properties: + Preferences: + properties: + defaultOrganizationRef: + type: string + type: object + type: object + status: + properties: + defaultOrganization: + type: string + displayName: + type: string + email: + type: string + username: + type: string + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] From bce87194272ce8d75235679cb6f375231a2d621a Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Tue, 18 Jan 2022 15:41:38 +0100 Subject: [PATCH 2/6] Add organization members resource on organization creation --- apiserver/organization/create.go | 35 ++++++++++++++++++ apiserver/organization/create_test.go | 6 ++++ apiserver/organization/members.go | 24 +++++++++++++ apiserver/organization/mock/members.go | 50 ++++++++++++++++++++++++++ apiserver/organization/organization.go | 9 +++++ 5 files changed, 124 insertions(+) create mode 100644 apiserver/organization/members.go create mode 100644 apiserver/organization/mock/members.go diff --git a/apiserver/organization/create.go b/apiserver/organization/create.go index 16efdb87..48ca0ac1 100644 --- a/apiserver/organization/create.go +++ b/apiserver/organization/create.go @@ -3,11 +3,14 @@ package organization import ( "context" "fmt" + "strings" orgv1 "github.com/appuio/control-api/apis/organization/v1" + controlv1 "github.com/appuio/control-api/apis/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" ) @@ -45,5 +48,37 @@ func (s *organizationStorage) create(ctx context.Context, org *orgv1.Organizatio return nil, fmt.Errorf("failed to create organization: %w", err) } + orgMembers := newOrganizationMembers(ctx, org.Name, "") + + if err := s.members.CreateMembers(ctx, orgMembers); err != nil { + // rollback + _, deleteErr := s.namepaces.DeleteNamespace(ctx, org.Name, nil) + if deleteErr != nil { + err = fmt.Errorf("%w and failed to clean up namespace: %s", err, deleteErr.Error()) + } + return nil, fmt.Errorf("failed to create organization: %w", err) + + } + return org, nil } + +func newOrganizationMembers(ctx context.Context, organization, usernamePrefix string) *controlv1.OrganizationMembers { + userRefs := []controlv1.UserRef{} + user, ok := request.UserFrom(ctx) + if ok { + userRefs = append(userRefs, controlv1.UserRef{ + ID: strings.TrimPrefix(user.GetName(), usernamePrefix), + }) + } + + return &controlv1.OrganizationMembers{ + ObjectMeta: metav1.ObjectMeta{ + Name: "members", + Namespace: organization, + }, + Spec: controlv1.OrganizationMembersSpec{ + UserRefs: userRefs, + }, + } +} diff --git a/apiserver/organization/create_test.go b/apiserver/organization/create_test.go index 1cbc0483..3a938d0e 100644 --- a/apiserver/organization/create_test.go +++ b/apiserver/organization/create_test.go @@ -77,6 +77,8 @@ func TestOrganizationStorage_Create(t *testing.T) { os, mnp, mauth := newMockedOrganizationStorage(ctrl) mrb := mock.NewMockroleBindingCreator(ctrl) os.rbac = mrb + mmemb := mock.NewMockmemberProvider(ctrl) + os.members = mmemb mauth.EXPECT(). Authorize(gomock.Any(), isAuthRequest("create")). Return(tc.authDecision.decision, tc.authDecision.reason, tc.authDecision.err). @@ -89,6 +91,10 @@ func TestOrganizationStorage_Create(t *testing.T) { CreateRoleBindings(gomock.Any(), gomock.Any()). Return(nil). AnyTimes() + mmemb.EXPECT(). + CreateMembers(gomock.Any(), gomock.Any()). + Return(nil). + AnyTimes() nopValidate := func(ctx context.Context, obj runtime.Object) error { return nil diff --git a/apiserver/organization/members.go b/apiserver/organization/members.go new file mode 100644 index 00000000..371bcbc3 --- /dev/null +++ b/apiserver/organization/members.go @@ -0,0 +1,24 @@ +package organization + +import ( + "context" + + controlv1 "github.com/appuio/control-api/apis/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// memberProvider is an abstraction for interacting with the OrganizationMembers Object +//go:generate go run github.com/golang/mock/mockgen -source=$GOFILE -destination=./mock/$GOFILE +type memberProvider interface { + CreateMembers(ctx context.Context, members *controlv1.OrganizationMembers) error +} + +type kubeMemberProvider struct { + Client client.Client + + usernamePrefix string +} + +func (k kubeMemberProvider) CreateMembers(ctx context.Context, members *controlv1.OrganizationMembers) error { + return k.Client.Create(ctx, members) +} diff --git a/apiserver/organization/mock/members.go b/apiserver/organization/mock/members.go new file mode 100644 index 00000000..aac276c7 --- /dev/null +++ b/apiserver/organization/mock/members.go @@ -0,0 +1,50 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: members.go + +// Package mock_organization is a generated GoMock package. +package mock_organization + +import ( + context "context" + reflect "reflect" + + v1 "github.com/appuio/control-api/apis/v1" + gomock "github.com/golang/mock/gomock" +) + +// MockmemberProvider is a mock of memberProvider interface. +type MockmemberProvider struct { + ctrl *gomock.Controller + recorder *MockmemberProviderMockRecorder +} + +// MockmemberProviderMockRecorder is the mock recorder for MockmemberProvider. +type MockmemberProviderMockRecorder struct { + mock *MockmemberProvider +} + +// NewMockmemberProvider creates a new mock instance. +func NewMockmemberProvider(ctrl *gomock.Controller) *MockmemberProvider { + mock := &MockmemberProvider{ctrl: ctrl} + mock.recorder = &MockmemberProviderMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockmemberProvider) EXPECT() *MockmemberProviderMockRecorder { + return m.recorder +} + +// CreateMembers mocks base method. +func (m *MockmemberProvider) CreateMembers(ctx context.Context, members *v1.OrganizationMembers) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateMembers", ctx, members) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateMembers indicates an expected call of CreateMembers. +func (mr *MockmemberProviderMockRecorder) CreateMembers(ctx, members interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateMembers", reflect.TypeOf((*MockmemberProvider)(nil).CreateMembers), ctx, members) +} diff --git a/apiserver/organization/organization.go b/apiserver/organization/organization.go index 393821c4..c1eb84ce 100644 --- a/apiserver/organization/organization.go +++ b/apiserver/organization/organization.go @@ -4,6 +4,7 @@ import ( "errors" orgv1 "github.com/appuio/control-api/apis/organization/v1" + controlv1 "github.com/appuio/control-api/apis/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -26,6 +27,10 @@ func New(clusterRoles *[]string) restbuilder.ResourceHandlerProvider { if err != nil { return nil, err } + err = controlv1.AddToScheme(c.Scheme()) + if err != nil { + return nil, err + } return &organizationStorage{ namepaces: &kubeNamespaceProvider{ Client: c, @@ -37,12 +42,16 @@ func New(clusterRoles *[]string) restbuilder.ResourceHandlerProvider { Client: c, ClusterRoles: *clusterRoles, }, + members: kubeMemberProvider{ + Client: c, + }, }, nil } } type organizationStorage struct { namepaces namespaceProvider + members memberProvider authorizer rbacAuthorizer rbac roleBindingCreator From 666dbd9bf3b070a2867bcd4e96117f02798d9139 Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Tue, 18 Jan 2022 17:21:39 +0100 Subject: [PATCH 3/6] Add test for abort on failure during OrganizationMember creation --- apiserver/organization/create_test.go | 102 +++++++++++++++++--------- 1 file changed, 68 insertions(+), 34 deletions(-) diff --git a/apiserver/organization/create_test.go b/apiserver/organization/create_test.go index 3a938d0e..edf8c254 100644 --- a/apiserver/organization/create_test.go +++ b/apiserver/organization/create_test.go @@ -119,39 +119,73 @@ func TestOrganizationStorage_Create(t *testing.T) { } func TestOrganizationStorage_Create_Abort(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - os, mnp, mauth := newMockedOrganizationStorage(ctrl) - mrb := mock.NewMockroleBindingCreator(ctrl) - os.rbac = mrb - mauth.EXPECT(). - Authorize(gomock.Any(), isAuthRequest("create")). - Return(authorizer.DecisionAllow, "", nil). - Times(1) - mnp.EXPECT(). - CreateNamespace(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil). - Times(1) - mrb.EXPECT(). - CreateRoleBindings(gomock.Any(), gomock.Any()). - Return(errors.New("")). - Times(1) - mnp.EXPECT(). - DeleteNamespace(gomock.Any(), gomock.Any(), gomock.Any()). - Return(fooNs, nil). - Times(1) - - nopValidate := func(ctx context.Context, obj runtime.Object) error { - return nil + + tests := map[string]struct { + failRoleBinding bool + failMembers bool + }{ + "GivenRolebindingFails_ThenAbort": { + failRoleBinding: true, + }, + "GivenMembersFails_ThenAbort": { + failMembers: true, + }, + } + for n, tc := range tests { + t.Run(n, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + os, mnp, mauth := newMockedOrganizationStorage(ctrl) + mrb := mock.NewMockroleBindingCreator(ctrl) + os.rbac = mrb + mmemb := mock.NewMockmemberProvider(ctrl) + os.members = mmemb + + mauth.EXPECT(). + Authorize(gomock.Any(), isAuthRequest("create")). + Return(authorizer.DecisionAllow, "", nil). + Times(1) + mnp.EXPECT(). + CreateNamespace(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil). + Times(1) + + if tc.failRoleBinding { + mrb.EXPECT(). + CreateRoleBindings(gomock.Any(), gomock.Any()). + Return(errors.New("")). + Times(1) + } else { + mrb.EXPECT(). + CreateRoleBindings(gomock.Any(), gomock.Any()). + Return(nil). + Times(1) + if tc.failMembers { + mmemb.EXPECT(). + CreateMembers(gomock.Any(), gomock.Any()). + Return(errors.New("")). + Times(1) + } + } + + mnp.EXPECT(). + DeleteNamespace(gomock.Any(), gomock.Any(), gomock.Any()). + Return(fooNs, nil). + Times(1) + + nopValidate := func(ctx context.Context, obj runtime.Object) error { + return nil + } + _, err := os.Create(request.WithRequestInfo(request.NewContext(), + &request.RequestInfo{ + Verb: "create", + APIGroup: orgv1.GroupVersion.Group, + Resource: "organizations", + Name: "foo", + }), + fooOrg, nopValidate, nil) + + require.Error(t, err) + }) } - _, err := os.Create(request.WithRequestInfo(request.NewContext(), - &request.RequestInfo{ - Verb: "create", - APIGroup: orgv1.GroupVersion.Group, - Resource: "organizations", - Name: "foo", - }), - fooOrg, nopValidate, nil) - - require.Error(t, err) } From 02e554cec2a0578bebb33a08c8f948ec26ba6519 Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Wed, 19 Jan 2022 09:56:54 +0100 Subject: [PATCH 4/6] Add comments for exported structs --- apis/v1/organizationmembers_type.go | 4 ++++ apis/v1/user_types.go | 6 ++++++ .../v1/base/appuio.io_organizationmembers.yaml | 6 ++++++ .../crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml | 3 +++ 4 files changed, 19 insertions(+) diff --git a/apis/v1/organizationmembers_type.go b/apis/v1/organizationmembers_type.go index 23e18e2c..70cf0e0b 100644 --- a/apis/v1/organizationmembers_type.go +++ b/apis/v1/organizationmembers_type.go @@ -16,13 +16,17 @@ type OrganizationMembers struct { Status OrganizationMembersStatus `json:"status,omitempty"` } +// OrganizationMembersSpec contains the desired members of the organization type OrganizationMembersSpec struct { UserRefs []UserRef `json:"userRefs,omitempty"` } + +// OrganizationMembersStatus contains the actual members of the organization type OrganizationMembersStatus struct { UserRefs []UserRef `json:"resolvedUserRefs,omitempty"` } +// UserRef points to a user type UserRef struct { ID string `json:"id,omitempty"` Username string `json:"username,omitempty"` diff --git a/apis/v1/user_types.go b/apis/v1/user_types.go index 1d0ac77f..4ca42f91 100644 --- a/apis/v1/user_types.go +++ b/apis/v1/user_types.go @@ -16,12 +16,18 @@ type User struct { Spec UserSpec `json:"spec,omitempty"` Status UserStatus `json:"status,omitempty"` } + +// UserSpec contains the desired state of the user type UserSpec struct { Preferences UserPreferences `json:"Preferences,omitempty"` } + +// UserPreferences contains the Preferences of the user type UserPreferences struct { DefaultOrganizationRef string `json:"defaultOrganizationRef,omitempty"` } + +// UserStatus contains the acutal state of the user type UserStatus struct { DefaultOrganizationRef string `json:"defaultOrganization,omitempty"` DisplayName string `json:"displayName,omitempty"` diff --git a/config/crd/apiextensions.k8s.io/v1/base/appuio.io_organizationmembers.yaml b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_organizationmembers.yaml index 662b61da..eee6739b 100644 --- a/config/crd/apiextensions.k8s.io/v1/base/appuio.io_organizationmembers.yaml +++ b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_organizationmembers.yaml @@ -34,9 +34,12 @@ spec: metadata: type: object spec: + description: OrganizationMembersSpec contains the desired members of the + organization properties: userRefs: items: + description: UserRef points to a user properties: id: type: string @@ -46,9 +49,12 @@ spec: type: array type: object status: + description: OrganizationMembersStatus contains the actual members of + the organization properties: resolvedUserRefs: items: + description: UserRef points to a user properties: id: type: string diff --git a/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml index 9ec48c3f..6f0bf15b 100644 --- a/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml +++ b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml @@ -34,14 +34,17 @@ spec: metadata: type: object spec: + description: UserSpec contains the desired state of the user properties: Preferences: + description: UserPreferences contains the Preferences of the user properties: defaultOrganizationRef: type: string type: object type: object status: + description: UserStatus contains the acutal state of the user properties: defaultOrganization: type: string From 5ce81330cb3325db483d28fb0e75c4fb1531975d Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Wed, 19 Jan 2022 10:47:01 +0100 Subject: [PATCH 5/6] Test and correctly integrate username prefix --- Makefile | 2 +- apiserver/organization/create.go | 2 +- apiserver/organization/create_test.go | 37 +++++++++++++++++++++++--- apiserver/organization/organization.go | 10 ++++--- main.go | 4 ++- 5 files changed, 46 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index f0b4fc04..17f8d457 100644 --- a/Makefile +++ b/Makefile @@ -62,7 +62,7 @@ clean: ## Cleans up the generated resources .PHONY: run KUBECONFIG ?= ~/.kube/config run: build ## Starts control api against the configured kuberentes cluster - $(BIN_FILENAME) --secure-port 9443 --kubeconfig $(KUBECONFIG) --authentication-kubeconfig $(KUBECONFIG) --authorization-kubeconfig $(KUBECONFIG) --cluster-roles appuio-organization-viewer,appuio-organization-admin + $(BIN_FILENAME) --secure-port 9443 --kubeconfig $(KUBECONFIG) --authentication-kubeconfig $(KUBECONFIG) --authorization-kubeconfig $(KUBECONFIG) --cluster-roles appuio-organization-viewer,appuio-organization-admin --username-prefix "appuio#" .PHONY: local-env local-env-setup: ## Setup local kind-based dev environment diff --git a/apiserver/organization/create.go b/apiserver/organization/create.go index 48ca0ac1..e2f63fa7 100644 --- a/apiserver/organization/create.go +++ b/apiserver/organization/create.go @@ -48,7 +48,7 @@ func (s *organizationStorage) create(ctx context.Context, org *orgv1.Organizatio return nil, fmt.Errorf("failed to create organization: %w", err) } - orgMembers := newOrganizationMembers(ctx, org.Name, "") + orgMembers := newOrganizationMembers(ctx, org.Name, s.usernamePrefix) if err := s.members.CreateMembers(ctx, orgMembers); err != nil { // rollback diff --git a/apiserver/organization/create_test.go b/apiserver/organization/create_test.go index edf8c254..6f96eff1 100644 --- a/apiserver/organization/create_test.go +++ b/apiserver/organization/create_test.go @@ -3,6 +3,7 @@ package organization import ( "context" "errors" + "fmt" "testing" "github.com/golang/mock/gomock" @@ -10,31 +11,38 @@ import ( "github.com/stretchr/testify/require" orgv1 "github.com/appuio/control-api/apis/organization/v1" + controlv1 "github.com/appuio/control-api/apis/v1" mock "github.com/appuio/control-api/apiserver/organization/mock" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/request" ) func TestOrganizationStorage_Create(t *testing.T) { tests := map[string]struct { + userID string organizationIn *orgv1.Organization namespaceErr error authDecision authResponse + memberName string + organizationOut *orgv1.Organization err error }{ "GivenCreateOrg_ThenSuccess": { + userID: "appuio#smith", organizationIn: fooOrg, authDecision: authResponse{ decision: authorizer.DecisionAllow, }, + memberName: "smith", organizationOut: fooOrg, }, "GivenNsExists_ThenFail": { @@ -79,6 +87,7 @@ func TestOrganizationStorage_Create(t *testing.T) { os.rbac = mrb mmemb := mock.NewMockmemberProvider(ctrl) os.members = mmemb + os.usernamePrefix = "appuio#" mauth.EXPECT(). Authorize(gomock.Any(), isAuthRequest("create")). Return(tc.authDecision.decision, tc.authDecision.reason, tc.authDecision.err). @@ -92,20 +101,22 @@ func TestOrganizationStorage_Create(t *testing.T) { Return(nil). AnyTimes() mmemb.EXPECT(). - CreateMembers(gomock.Any(), gomock.Any()). + CreateMembers(gomock.Any(), containsMember(tc.memberName)). Return(nil). AnyTimes() nopValidate := func(ctx context.Context, obj runtime.Object) error { return nil } - org, err := os.Create(request.WithRequestInfo(request.NewContext(), + org, err := os.Create(request.WithUser(request.WithRequestInfo(request.NewContext(), &request.RequestInfo{ Verb: "create", APIGroup: orgv1.GroupVersion.Group, Resource: "organizations", Name: tc.organizationIn.Name, - }), + }), &user.DefaultInfo{ + Name: tc.userID, + }), tc.organizationIn, nopValidate, nil) if tc.err != nil { @@ -189,3 +200,23 @@ func TestOrganizationStorage_Create_Abort(t *testing.T) { }) } } + +type memberMatcher struct { + user string +} + +func (m memberMatcher) Matches(x interface{}) bool { + mem, ok := x.(*controlv1.OrganizationMembers) + if !ok { + return ok + } + return len(mem.Spec.UserRefs) > 0 && mem.Spec.UserRefs[0].ID == m.user +} + +func (m memberMatcher) String() string { + return fmt.Sprintf("contains %s", m.user) +} + +func containsMember(user string) memberMatcher { + return memberMatcher{user: user} +} diff --git a/apiserver/organization/organization.go b/apiserver/organization/organization.go index c1eb84ce..74fa04bb 100644 --- a/apiserver/organization/organization.go +++ b/apiserver/organization/organization.go @@ -21,7 +21,7 @@ import ( // +kubebuilder:rbac:groups="flowcontrol.apiserver.k8s.io",resources=prioritylevelconfigurations;flowschemas,verbs=get;list;watch // New returns a new storage provider for Organizations -func New(clusterRoles *[]string) restbuilder.ResourceHandlerProvider { +func New(clusterRoles *[]string, usernamePrefix *string) restbuilder.ResourceHandlerProvider { return func(s *runtime.Scheme, g genericregistry.RESTOptionsGetter) (rest.Storage, error) { c, err := client.NewWithWatch(loopback.GetLoopbackMasterClientConfig(), client.Options{}) if err != nil { @@ -45,13 +45,17 @@ func New(clusterRoles *[]string) restbuilder.ResourceHandlerProvider { members: kubeMemberProvider{ Client: c, }, + usernamePrefix: *usernamePrefix, }, nil } } type organizationStorage struct { - namepaces namespaceProvider - members memberProvider + namepaces namespaceProvider + + members memberProvider + usernamePrefix string + authorizer rbacAuthorizer rbac roleBindingCreator diff --git a/main.go b/main.go index 78e54de0..cfe19fb9 100644 --- a/main.go +++ b/main.go @@ -35,8 +35,9 @@ func main() { ).Info("Starting control-api…") roles := []string{} + usernamePrefix := "" cmd, err := builder.APIServer. - WithResourceAndHandler(&orgv1.Organization{}, orgStore.New(&roles)). + WithResourceAndHandler(&orgv1.Organization{}, orgStore.New(&roles, &usernamePrefix)). WithoutEtcd(). ExposeLoopbackAuthorizer(). ExposeLoopbackMasterClientConfig(). @@ -46,6 +47,7 @@ func main() { } cmd.Flags().StringSliceVar(&roles, "cluster-roles", []string{}, "Cluster Roles to bind when creating an organization") + cmd.Flags().StringVar(&usernamePrefix, "username-prefix", "", "Prefix prepended to username claims. Usually the same as \"--oidc-username-prefix\" of the Kubernetes API server") err = cmd.Execute() if err != nil { logger.Error(err, "API server stopped unexpectedly") From ec6e44b09d1d026ed862d9185a89c5c2674af17d Mon Sep 17 00:00:00 2001 From: Fabian Fischer <10788152+glrf@users.noreply.github.com> Date: Wed, 19 Jan 2022 15:33:33 +0100 Subject: [PATCH 6/6] Fix CRD definition Co-authored-by: Chris --- apis/v1/organizationmembers_type.go | 2 +- apis/v1/user_types.go | 2 +- apis/v1/zz_generated.deepcopy.go | 4 ++-- config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apis/v1/organizationmembers_type.go b/apis/v1/organizationmembers_type.go index 70cf0e0b..66c0274d 100644 --- a/apis/v1/organizationmembers_type.go +++ b/apis/v1/organizationmembers_type.go @@ -23,7 +23,7 @@ type OrganizationMembersSpec struct { // OrganizationMembersStatus contains the actual members of the organization type OrganizationMembersStatus struct { - UserRefs []UserRef `json:"resolvedUserRefs,omitempty"` + ResolvedUserRefs []UserRef `json:"resolvedUserRefs,omitempty"` } // UserRef points to a user diff --git a/apis/v1/user_types.go b/apis/v1/user_types.go index 4ca42f91..626689ca 100644 --- a/apis/v1/user_types.go +++ b/apis/v1/user_types.go @@ -19,7 +19,7 @@ type User struct { // UserSpec contains the desired state of the user type UserSpec struct { - Preferences UserPreferences `json:"Preferences,omitempty"` + Preferences UserPreferences `json:"preferences,omitempty"` } // UserPreferences contains the Preferences of the user diff --git a/apis/v1/zz_generated.deepcopy.go b/apis/v1/zz_generated.deepcopy.go index 505e2153..342d12fb 100644 --- a/apis/v1/zz_generated.deepcopy.go +++ b/apis/v1/zz_generated.deepcopy.go @@ -132,8 +132,8 @@ func (in *OrganizationMembersSpec) DeepCopy() *OrganizationMembersSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OrganizationMembersStatus) DeepCopyInto(out *OrganizationMembersStatus) { *out = *in - if in.UserRefs != nil { - in, out := &in.UserRefs, &out.UserRefs + if in.ResolvedUserRefs != nil { + in, out := &in.ResolvedUserRefs, &out.ResolvedUserRefs *out = make([]UserRef, len(*in)) copy(*out, *in) } diff --git a/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml index 6f0bf15b..f0921360 100644 --- a/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml +++ b/config/crd/apiextensions.k8s.io/v1/base/appuio.io_users.yaml @@ -36,7 +36,7 @@ spec: spec: description: UserSpec contains the desired state of the user properties: - Preferences: + preferences: description: UserPreferences contains the Preferences of the user properties: defaultOrganizationRef: