Skip to content

Commit c3b6d8d

Browse files
simontesaraaronschweignexus49
authored
feat: adding improved support for custom account types (#118)
* feat: allow configurable custom account types in webhook On-behalf-of: @SAP <simon@simontesar.com> * feat: support custom account types with pre-provisioned workspace types On-behalf-of: @SAP <simon@simontesar.com> * feat: remove "account" from default accounttypes On-behalf-of: @SAP <simon@simontesar.com> * feat: remove enum from account.spec.accounttype On-behalf-of: @SAP <simon@simontesar.com> * feat: treat all non-org accounts the same("account" accountType is not special) On-behalf-of: @SAP <simon@simontesar.com> * fix: use right workspace type * fix: correct additional account types cli flag binding On-behalf-of: @SAP <simon@simontesar.com> * fix: update test setup to match custom account types changes - Update APIExport schema reference to use correct version - Fix test expectation for workspace type naming Signed-off-by: Bastian Echterhölter <bastian.echterhoelter@sap.com> On-behalf-of: @SAP <bastian.echterhoelter@sap.com> * feat: add org label to generated workspace types - Add core.platform-mesh.io/org label to org and account workspace types - Rename subroutine constants to shorter names Signed-off-by: Bastian Echterhölter <bastian.echterhoelter@sap.com> On-behalf-of: @SAP <bastian.echterhoelter@sap.com> --------- Signed-off-by: Bastian Echterhölter <bastian.echterhoelter@sap.com> Co-authored-by: I547291 <aaron.schweig@sap.com> Co-authored-by: Bastian Echterhölter <bastian.echterhoelter@sap.com>
1 parent 04dd403 commit c3b6d8d

17 files changed

Lines changed: 100 additions & 75 deletions

api/v1alpha1/account_types.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ const (
3131
// AccountSpec defines the desired state of Account
3232
type AccountSpec struct {
3333
// Type specifies the intended type for this Account object.
34-
// +kubebuilder:validation:Enum=org;account
3534
Type AccountType `json:"type"`
3635

3736
// The display name for this account

api/v1alpha1/account_webhook.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ import (
1212
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
1313
)
1414

15-
func SetupAccountWebhookWithManager(mgr ctrl.Manager, denyList []string) error {
15+
func SetupAccountWebhookWithManager(mgr ctrl.Manager, organizationNameDenyList []string, accountTypeAllowList []AccountType) error {
1616
return ctrl.NewWebhookManagedBy(mgr).
1717
For(&Account{}).
1818
WithDefaulter(&AccountDefaulter{}).
19-
WithValidator(&AccountValidator{DenyList: denyList}).
19+
WithValidator(&AccountValidator{OrganizationNameDenyList: organizationNameDenyList, AccountTypeAllowList: accountTypeAllowList}).
2020
Complete()
2121
}
2222

@@ -40,11 +40,17 @@ var _ webhook.CustomDefaulter = &AccountDefaulter{}
4040
var _ webhook.CustomValidator = &AccountValidator{}
4141

4242
type AccountValidator struct {
43-
DenyList []string
43+
OrganizationNameDenyList []string
44+
AccountTypeAllowList []AccountType
4445
}
4546

4647
func (v *AccountValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
4748
account := obj.(*Account)
49+
50+
if !slices.Contains(v.AccountTypeAllowList, account.Spec.Type) {
51+
return nil, fmt.Errorf("account type %s not in allowlist", account.Spec.Type)
52+
}
53+
4854
if account.Spec.Type != AccountTypeOrg {
4955
return nil, nil
5056
}
@@ -53,7 +59,7 @@ func (v *AccountValidator) ValidateCreate(ctx context.Context, obj runtime.Objec
5359
return nil, fmt.Errorf("organization name %q is too short, must be at least 3 characters", account.Name)
5460
}
5561

56-
if slices.Contains(v.DenyList, account.Name) {
62+
if slices.Contains(v.OrganizationNameDenyList, account.Name) {
5763
return nil, fmt.Errorf("organization name %q is not allowed", account.Name)
5864
}
5965

@@ -63,6 +69,10 @@ func (v *AccountValidator) ValidateCreate(ctx context.Context, obj runtime.Objec
6369
func (v *AccountValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
6470
account := newObj.(*Account)
6571

72+
if !slices.Contains(v.AccountTypeAllowList, account.Spec.Type) {
73+
return nil, fmt.Errorf("account type %s not in allowlist", account.Spec.Type)
74+
}
75+
6676
if account.Spec.Type != AccountTypeOrg {
6777
return nil, nil
6878
}
@@ -71,7 +81,7 @@ func (v *AccountValidator) ValidateUpdate(ctx context.Context, oldObj, newObj ru
7181
return nil, fmt.Errorf("organization name %q is too short, must be at least 3 characters", account.Name)
7282
}
7383

74-
if slices.Contains(v.DenyList, account.Name) {
84+
if slices.Contains(v.OrganizationNameDenyList, account.Name) {
7585
return nil, fmt.Errorf("organization name %q is not allowed", account.Name)
7686
}
7787

api/v1alpha1/account_webhook_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestAccountValidator_ValidateCreate(t *testing.T) {
3939
name: "non-organization account with denied name",
4040
account: &Account{
4141
ObjectMeta: metav1.ObjectMeta{Name: "admin"},
42-
Spec: AccountSpec{Type: "personal"},
42+
Spec: AccountSpec{Type: "account"},
4343
},
4444
denyList: []string{"admin", "root", "system"},
4545
expectError: false,
@@ -67,7 +67,7 @@ func TestAccountValidator_ValidateCreate(t *testing.T) {
6767

6868
for _, tt := range tests {
6969
t.Run(tt.name, func(t *testing.T) {
70-
validator := &AccountValidator{DenyList: tt.denyList}
70+
validator := &AccountValidator{OrganizationNameDenyList: tt.denyList, AccountTypeAllowList: []AccountType{AccountTypeAccount, AccountTypeOrg}}
7171
_, err := validator.ValidateCreate(context.Background(), tt.account)
7272

7373
if tt.expectError {
@@ -117,10 +117,10 @@ func TestAccountValidator_ValidateUpdate(t *testing.T) {
117117
expectError: false,
118118
},
119119
{
120-
name: "changing from personal to organization with denied name",
120+
name: "changing from account to organization with denied name",
121121
oldAccount: &Account{
122122
ObjectMeta: metav1.ObjectMeta{Name: "admin"},
123-
Spec: AccountSpec{Type: "personal"},
123+
Spec: AccountSpec{Type: "account"},
124124
},
125125
newAccount: &Account{
126126
ObjectMeta: metav1.ObjectMeta{Name: "admin"},
@@ -131,10 +131,10 @@ func TestAccountValidator_ValidateUpdate(t *testing.T) {
131131
errorMsg: `organization name "admin" is not allowed`,
132132
},
133133
{
134-
name: "changing from personal to organization with allowed name",
134+
name: "changing from account to organization with allowed name",
135135
oldAccount: &Account{
136136
ObjectMeta: metav1.ObjectMeta{Name: "my-account"},
137-
Spec: AccountSpec{Type: "personal"},
137+
Spec: AccountSpec{Type: "account"},
138138
},
139139
newAccount: &Account{
140140
ObjectMeta: metav1.ObjectMeta{Name: "my-account"},
@@ -144,27 +144,27 @@ func TestAccountValidator_ValidateUpdate(t *testing.T) {
144144
expectError: false,
145145
},
146146
{
147-
name: "changing from organization to personal with denied name",
147+
name: "changing from organization to account with denied name",
148148
oldAccount: &Account{
149149
ObjectMeta: metav1.ObjectMeta{Name: "admin"},
150150
Spec: AccountSpec{Type: "org"},
151151
},
152152
newAccount: &Account{
153153
ObjectMeta: metav1.ObjectMeta{Name: "admin"},
154-
Spec: AccountSpec{Type: "personal"},
154+
Spec: AccountSpec{Type: "account"},
155155
},
156156
denyList: []string{"admin", "root", "system"},
157157
expectError: false,
158158
},
159159
{
160-
name: "updating personal account with denied name",
160+
name: "updating account account with denied name",
161161
oldAccount: &Account{
162162
ObjectMeta: metav1.ObjectMeta{Name: "admin"},
163-
Spec: AccountSpec{Type: "personal"},
163+
Spec: AccountSpec{Type: "account"},
164164
},
165165
newAccount: &Account{
166166
ObjectMeta: metav1.ObjectMeta{Name: "admin"},
167-
Spec: AccountSpec{Type: "personal"},
167+
Spec: AccountSpec{Type: "account"},
168168
},
169169
denyList: []string{"admin", "root", "system"},
170170
expectError: false,
@@ -200,7 +200,7 @@ func TestAccountValidator_ValidateUpdate(t *testing.T) {
200200

201201
for _, tt := range tests {
202202
t.Run(tt.name, func(t *testing.T) {
203-
validator := &AccountValidator{DenyList: tt.denyList}
203+
validator := &AccountValidator{OrganizationNameDenyList: tt.denyList, AccountTypeAllowList: []AccountType{AccountTypeAccount, AccountTypeOrg}}
204204
_, err := validator.ValidateUpdate(context.Background(), tt.oldAccount, tt.newAccount)
205205

206206
if tt.expectError {

api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cmd/operator.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,13 @@ func RunController(_ *cobra.Command, _ []string) { // coverage-ignore
169169
}
170170
}
171171

172+
accountTypeAllowList := []v1alpha1.AccountType{v1alpha1.AccountTypeOrg}
173+
for _, additionalType := range operatorCfg.Webhooks.AdditionalAccountTypes {
174+
accountTypeAllowList = append(accountTypeAllowList, v1alpha1.AccountType(additionalType))
175+
}
176+
172177
log.Info().Strs("deniedNames", denyList).Msg("webhooks are enabled")
173-
if err := v1alpha1.SetupAccountWebhookWithManager(mgr.GetLocalManager(), denyList); err != nil {
178+
if err := v1alpha1.SetupAccountWebhookWithManager(mgr.GetLocalManager(), denyList, accountTypeAllowList); err != nil {
174179
log.Fatal().Err(err).Str("webhook", "Account").Msg("unable to create webhook")
175180
}
176181
}

config/crd/core.platform-mesh.io_accounts.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,6 @@ spec:
9797
type: array
9898
type:
9999
description: Type specifies the intended type for this Account object.
100-
enum:
101-
- org
102-
- account
103100
type: string
104101
required:
105102
- displayName

config/resources/apiresourceschema-accounts.core.platform-mesh.io.yaml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ apiVersion: apis.kcp.io/v1alpha1
22
kind: APIResourceSchema
33
metadata:
44
creationTimestamp: null
5-
name: v250915-1185c0b.accounts.core.platform-mesh.io
5+
name: v260109-82344be.accounts.core.platform-mesh.io
66
spec:
77
group: core.platform-mesh.io
88
names:
@@ -93,9 +93,6 @@ spec:
9393
type: array
9494
type:
9595
description: Type specifies the intended type for this Account object.
96-
enum:
97-
- org
98-
- account
9996
type: string
10097
required:
10198
- displayName

internal/config/config.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ package config
33
// OperatorConfig struct to hold the app config
44
type OperatorConfig struct {
55
Webhooks struct {
6-
Enabled bool `mapstructure:"webhooks-enabled" default:"false"`
7-
CertDir string `mapstructure:"webhooks-cert-dir" default:"certs"`
8-
Port int `mapstructure:"webhooks-port" default:"9443"`
9-
DenyList string `mapstructure:"webhooks-deny-list"`
6+
Enabled bool `mapstructure:"webhooks-enabled" default:"false"`
7+
CertDir string `mapstructure:"webhooks-cert-dir" default:"certs"`
8+
Port int `mapstructure:"webhooks-port" default:"9443"`
9+
DenyList string `mapstructure:"webhooks-deny-list"`
10+
AdditionalAccountTypes []string `mapstructure:"webhooks-additional-account-types"`
1011
} `mapstructure:",squash"`
1112
Subroutines struct {
1213
WorkspaceType struct {

internal/controller/account_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ func (s *AccountTestSuite) verifyWorkspace(ctx context.Context, orgName, account
241241
s.Require().NoError(s.rootOrgsDefaultClient.Get(ctx, types.NamespacedName{Name: accountName}, workspace))
242242
s.Equal(accountName, workspace.Name)
243243
s.NotNil(workspace.Spec.Type)
244-
expectedType := kcptenancyv1alpha.WorkspaceTypeName(fmt.Sprintf("%s-acc", orgName))
244+
expectedType := kcptenancyv1alpha.WorkspaceTypeName(fmt.Sprintf("%s-%s", orgName, v1alpha1.AccountTypeAccount))
245245
s.Equal(expectedType, workspace.Spec.Type.Name)
246246
}
247247

internal/controller/test/setup/01-platform-mesh-system/apiexport-core.platform-mesh.io.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ metadata:
66
spec:
77
latestResourceSchemas:
88
- v260126-7c674ee.accountinfos.core.platform-mesh.io
9-
- v250915-1185c0b.accounts.core.platform-mesh.io
9+
- v260109-82344be.accounts.core.platform-mesh.io
1010
permissionClaims:
1111
- resource: apibindings
1212
group: apis.kcp.io

0 commit comments

Comments
 (0)