Skip to content

Commit 6930fd1

Browse files
authored
Add tests for setImageRegistry tenantAdministrators and tenantConfiguration (#2584)
1 parent 62e822d commit 6930fd1

File tree

6 files changed

+153
-28
lines changed

6 files changed

+153
-28
lines changed

.github/workflows/jobs.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1542,7 +1542,7 @@ jobs:
15421542
go tool cover -func=all.out | grep total > tmp2
15431543
result=`cat tmp2 | awk 'END {print $3}'`
15441544
result=${result%\%}
1545-
threshold=64.9
1545+
threshold=65.4
15461546
echo "Result:"
15471547
echo "$result%"
15481548
if (( $(echo "$result >= $threshold" |bc -l) )); then

operatorapi/tenant_add.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import (
3434
miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2"
3535
"k8s.io/apimachinery/pkg/api/resource"
3636
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
37-
v1 "k8s.io/client-go/kubernetes/typed/core/v1"
3837
)
3938

4039
func getTenantCreatedResponse(session *models.Principal, params operator_api.CreateTenantParams) (response *models.CreateTenantResponse, mError *models.Error) {
@@ -49,10 +48,10 @@ func getTenantCreatedResponse(session *models.Principal, params operator_api.Cre
4948
return nil, restapi.ErrorWithContext(ctx, err)
5049
}
5150

52-
return createTenant(ctx, params, k8sClient, k8sClient.client.CoreV1(), session)
51+
return createTenant(ctx, params, k8sClient, session)
5352
}
5453

55-
func createTenant(ctx context.Context, params operator_api.CreateTenantParams, clientSet K8sClientI, cv1 v1.CoreV1Interface, session *models.Principal) (response *models.CreateTenantResponse, mError *models.Error) {
54+
func createTenant(ctx context.Context, params operator_api.CreateTenantParams, clientSet K8sClientI, session *models.Principal) (response *models.CreateTenantResponse, mError *models.Error) {
5655
tenantReq := params.Body
5756
minioImage := getTenantMinIOImage(tenantReq.Image)
5857

@@ -240,7 +239,7 @@ func createTenant(ctx context.Context, params operator_api.CreateTenantParams, c
240239

241240
if tenantReq.ImagePullSecret != "" {
242241
imagePullSecret = tenantReq.ImagePullSecret
243-
} else if imagePullSecret, err = setImageRegistry(ctx, tenantReq.ImageRegistry, cv1, ns, tenantName); err != nil {
242+
} else if imagePullSecret, err = setImageRegistry(ctx, tenantReq.ImageRegistry, clientSet, ns, tenantName); err != nil {
244243
return nil, restapi.ErrorWithContext(ctx, err)
245244
}
246245
// pass the image pull secret to the Tenant

operatorapi/tenant_update.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,10 @@ import (
3131
miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2"
3232
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3333
"k8s.io/apimachinery/pkg/types"
34-
v1 "k8s.io/client-go/kubernetes/typed/core/v1"
3534
)
3635

3736
// updateTenantAction does an update on the minioTenant by patching the desired changes
38-
func updateTenantAction(ctx context.Context, operatorClient OperatorClientI, clientset v1.CoreV1Interface, httpCl http.ClientI, namespace string, params operator_api.UpdateTenantParams) error {
37+
func updateTenantAction(ctx context.Context, operatorClient OperatorClientI, clientset K8sClientI, httpCl http.ClientI, namespace string, params operator_api.UpdateTenantParams) error {
3938
imageToUpdate := params.Body.Image
4039
imageRegistryReq := params.Body.ImageRegistry
4140

operatorapi/tenants.go

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,10 @@ func getSetTenantAdministratorsResponse(session *models.Principal, params operat
924924
if err != nil {
925925
return restapi.ErrorWithContext(ctx, err)
926926
}
927+
return setTenantAdministrators(ctx, minTenant, k8sClient, params)
928+
}
929+
930+
func setTenantAdministrators(ctx context.Context, minTenant *miniov2.Tenant, k8sClient K8sClientI, params operator_api.SetTenantAdministratorsParams) *models.Error {
927931
minTenant.EnsureDefaults()
928932

929933
svcURL := GetTenantServiceURL(minTenant)
@@ -963,19 +967,23 @@ func getTenantConfigurationResponse(session *models.Principal, params operator_a
963967
opClient := &operatorClient{
964968
client: opClientClientSet,
965969
}
966-
minTenant, err := getTenant(ctx, opClient, params.Namespace, params.Tenant)
970+
// get Kubernetes Client
971+
clientSet, err := cluster.K8sClient(session.STSSessionToken)
967972
if err != nil {
968973
return nil, restapi.ErrorWithContext(ctx, err)
969974
}
970-
// get Kubernetes Client
971-
clientSet, err := cluster.K8sClient(session.STSSessionToken)
972-
k8sClient := k8sClient{
975+
k8sClient := &k8sClient{
973976
client: clientSet,
974977
}
978+
minTenant, err := getTenant(ctx, opClient, params.Namespace, params.Tenant)
975979
if err != nil {
976980
return nil, restapi.ErrorWithContext(ctx, err)
977981
}
978-
tenantConfiguration, err := GetTenantConfiguration(ctx, &k8sClient, minTenant)
982+
return parseTenantConfiguration(ctx, k8sClient, minTenant)
983+
}
984+
985+
func parseTenantConfiguration(ctx context.Context, k8sClient K8sClientI, minTenant *miniov2.Tenant) (*models.TenantConfigurationResponse, *models.Error) {
986+
tenantConfiguration, err := GetTenantConfiguration(ctx, k8sClient, minTenant)
979987
if err != nil {
980988
return nil, restapi.ErrorWithContext(ctx, err)
981989
}
@@ -1335,7 +1343,7 @@ func getListTenantsResponse(session *models.Principal, params operator_api.ListT
13351343

13361344
// setImageRegistry creates a secret to store the private registry credentials, if one exist it updates the existing one
13371345
// returns the name of the secret created/updated
1338-
func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset v1.CoreV1Interface, namespace, tenantName string) (string, error) {
1346+
func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset K8sClientI, namespace, tenantName string) (string, error) {
13391347
if req == nil || req.Registry == nil || req.Username == nil || req.Password == nil {
13401348
return "", nil
13411349
}
@@ -1363,7 +1371,7 @@ func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset
13631371
corev1.DockerConfigJsonKey: []byte(string(imRegistryJSON)),
13641372
}
13651373
// Get or Create secret if it doesn't exist
1366-
currentSecret, err := clientset.Secrets(namespace).Get(ctx, pullSecretName, metav1.GetOptions{})
1374+
currentSecret, err := clientset.getSecret(ctx, namespace, pullSecretName, metav1.GetOptions{})
13671375
if err != nil {
13681376
if k8sErrors.IsNotFound(err) {
13691377
instanceSecret := corev1.Secret{
@@ -1376,7 +1384,7 @@ func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset
13761384
Data: secretCredentials,
13771385
Type: corev1.SecretTypeDockerConfigJson,
13781386
}
1379-
_, err = clientset.Secrets(namespace).Create(ctx, &instanceSecret, metav1.CreateOptions{})
1387+
_, err = clientset.createSecret(ctx, namespace, &instanceSecret, metav1.CreateOptions{})
13801388
if err != nil {
13811389
return "", err
13821390
}
@@ -1385,7 +1393,7 @@ func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset
13851393
return "", err
13861394
}
13871395
currentSecret.Data = secretCredentials
1388-
_, err = clientset.Secrets(namespace).Update(ctx, currentSecret, metav1.UpdateOptions{})
1396+
_, err = clientset.updateSecret(ctx, namespace, currentSecret, metav1.UpdateOptions{})
13891397
if err != nil {
13901398
return "", err
13911399
}
@@ -1426,6 +1434,9 @@ func getUpdateTenantResponse(session *models.Principal, params operator_api.Upda
14261434
if err != nil {
14271435
return restapi.ErrorWithContext(ctx, err)
14281436
}
1437+
k8sClient := &k8sClient{
1438+
client: clientSet,
1439+
}
14291440
opClient := &operatorClient{
14301441
client: opClientClientSet,
14311442
}
@@ -1434,7 +1445,7 @@ func getUpdateTenantResponse(session *models.Principal, params operator_api.Upda
14341445
Timeout: 4 * time.Second,
14351446
},
14361447
}
1437-
if err := updateTenantAction(ctx, opClient, clientSet.CoreV1(), httpC, params.Namespace, params); err != nil {
1448+
if err := updateTenantAction(ctx, opClient, k8sClient, httpC, params.Namespace, params); err != nil {
14381449
return restapi.ErrorWithContext(ctx, err, errors.New("unable to update tenant"))
14391450
}
14401451
return nil

operatorapi/tenants_2_test.go

Lines changed: 125 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ import (
3232
"github.com/stretchr/testify/suite"
3333
corev1 "k8s.io/api/core/v1"
3434
v1 "k8s.io/api/core/v1"
35+
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
3536
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
37+
"k8s.io/apimachinery/pkg/runtime/schema"
3638
)
3739

3840
type TenantTestSuite struct {
@@ -148,7 +150,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongECP() {
148150
k8sClientCreateSecretMock = func(ctx context.Context, namespace string, secret *v1.Secret, opts metav1.CreateOptions) (*v1.Secret, error) {
149151
return nil, nil
150152
}
151-
_, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{})
153+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
152154
suite.assert.NotNil(err)
153155
}
154156

@@ -174,7 +176,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongActiveDirectoryConfig() {
174176

175177
return nil, nil
176178
}
177-
_, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{})
179+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
178180
suite.assert.NotNil(err)
179181
}
180182

@@ -196,7 +198,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongBuiltInUsers() {
196198
}
197199
return nil, nil
198200
}
199-
_, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{})
201+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
200202
suite.assert.NotNil(err)
201203
}
202204

@@ -227,7 +229,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithOIDCAndWrongServerCertificates
227229
k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error {
228230
return nil
229231
}
230-
_, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{})
232+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
231233
suite.assert.NotNil(err)
232234
}
233235

@@ -246,7 +248,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongClientCertificates() {
246248
k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error {
247249
return nil
248250
}
249-
_, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{})
251+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
250252
suite.assert.NotNil(err)
251253
}
252254

@@ -264,7 +266,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongCAsCertificates() {
264266
}
265267
return nil, nil
266268
}
267-
_, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{})
269+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
268270
suite.assert.NotNil(err)
269271
}
270272

@@ -283,7 +285,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongMtlsCertificates() {
283285
k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error {
284286
return nil
285287
}
286-
_, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{})
288+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
287289
suite.assert.NotNil(err)
288290
}
289291

@@ -304,7 +306,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongKESConfig() {
304306
k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error {
305307
return nil
306308
}
307-
_, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{})
309+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
308310
suite.assert.NotNil(err)
309311
}
310312

@@ -318,7 +320,55 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongPool() {
318320
k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error {
319321
return nil
320322
}
321-
_, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{})
323+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
324+
suite.assert.NotNil(err)
325+
}
326+
327+
func (suite *TenantTestSuite) TestCreateTenantWithImageRegistryCreateError() {
328+
params, _ := suite.initCreateTenantRequest()
329+
params.Body.MountPath = "/mock-path"
330+
registry := "mock-registry"
331+
username := "mock-username"
332+
password := "mock-password"
333+
params.Body.ImageRegistry = &models.ImageRegistry{
334+
Registry: &registry,
335+
Username: &username,
336+
Password: &password,
337+
}
338+
339+
k8sClientCreateSecretMock = func(ctx context.Context, namespace string, secret *v1.Secret, opts metav1.CreateOptions) (*v1.Secret, error) {
340+
if strings.HasPrefix(secret.Name, fmt.Sprintf("%s-secret", *params.Body.Name)) {
341+
return nil, nil
342+
}
343+
return nil, errors.New("mock-create-error")
344+
}
345+
k8sclientGetSecretMock = func(ctx context.Context, namespace, secretName string, opts metav1.GetOptions) (*corev1.Secret, error) {
346+
return nil, k8sErrors.NewNotFound(schema.GroupResource{}, "")
347+
}
348+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
349+
suite.assert.NotNil(err)
350+
}
351+
352+
func (suite *TenantTestSuite) TestCreateTenantWithImageRegistryUpdateError() {
353+
params, _ := suite.initCreateTenantRequest()
354+
registry := "mock-registry"
355+
username := "mock-username"
356+
password := "mock-password"
357+
params.Body.ImageRegistry = &models.ImageRegistry{
358+
Registry: &registry,
359+
Username: &username,
360+
Password: &password,
361+
}
362+
k8sClientCreateSecretMock = func(ctx context.Context, namespace string, secret *v1.Secret, opts metav1.CreateOptions) (*v1.Secret, error) {
363+
return nil, nil
364+
}
365+
k8sClientUpdateSecretMock = func(ctx context.Context, namespace string, secret *v1.Secret, opts metav1.UpdateOptions) (*v1.Secret, error) {
366+
return nil, errors.New("mock-update-error")
367+
}
368+
k8sclientGetSecretMock = func(ctx context.Context, namespace, secretName string, opts metav1.GetOptions) (*corev1.Secret, error) {
369+
return &v1.Secret{}, nil
370+
}
371+
_, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{})
322372
suite.assert.NotNil(err)
323373
}
324374

@@ -392,6 +442,20 @@ func (suite *TenantTestSuite) initTenantConfigurationRequest() (params operator_
392442
return params, api
393443
}
394444

445+
func (suite *TenantTestSuite) TestParseTenantConfigurationWithoutError() {
446+
tenant := &miniov2.Tenant{
447+
Spec: miniov2.TenantSpec{
448+
Env: []corev1.EnvVar{
449+
{Name: "mock", Value: "mock-env"},
450+
{Name: "mock", Value: "mock-env-2"},
451+
},
452+
},
453+
}
454+
config, err := parseTenantConfiguration(context.Background(), suite.k8sclient, tenant)
455+
suite.assert.NotNil(config)
456+
suite.assert.Nil(err)
457+
}
458+
395459
func (suite *TenantTestSuite) TestUpdateTenantConfigurationHandlerWithError() {
396460
params, api := suite.initUpdateTenantConfigurationRequest()
397461
response := api.OperatorAPIUpdateTenantConfigurationHandler.Handle(params, &models.Principal{})
@@ -629,11 +693,63 @@ func (suite *TenantTestSuite) TestSetTenantAdministratorsHandlerWithError() {
629693
suite.assert.True(ok)
630694
}
631695

696+
func (suite *TenantTestSuite) TestSetTenantAdministratorsWithAdminClientError() {
697+
params, _ := suite.initSetTenantAdministratorsRequest()
698+
tenant := &miniov2.Tenant{}
699+
err := setTenantAdministrators(context.Background(), tenant, suite.k8sclient, params)
700+
suite.assert.NotNil(err)
701+
}
702+
703+
func (suite *TenantTestSuite) TestSetTenantAdministratorsWithUserPolicyError() {
704+
params, _ := suite.initSetTenantAdministratorsRequest()
705+
tenant := &miniov2.Tenant{
706+
Spec: miniov2.TenantSpec{
707+
Env: []corev1.EnvVar{
708+
{Name: "accesskey", Value: "mock-access"},
709+
{Name: "secretkey", Value: "mock-secret"},
710+
},
711+
},
712+
}
713+
params.Body.UserDNS = []string{"mock-user"}
714+
err := setTenantAdministrators(context.Background(), tenant, suite.k8sclient, params)
715+
suite.assert.NotNil(err)
716+
}
717+
718+
func (suite *TenantTestSuite) TestSetTenantAdministratorsWithGroupPolicyError() {
719+
params, _ := suite.initSetTenantAdministratorsRequest()
720+
tenant := &miniov2.Tenant{
721+
Spec: miniov2.TenantSpec{
722+
Env: []corev1.EnvVar{
723+
{Name: "accesskey", Value: "mock-access"},
724+
{Name: "secretkey", Value: "mock-secret"},
725+
},
726+
},
727+
}
728+
params.Body.GroupDNS = []string{"mock-user"}
729+
err := setTenantAdministrators(context.Background(), tenant, suite.k8sclient, params)
730+
suite.assert.NotNil(err)
731+
}
732+
733+
func (suite *TenantTestSuite) TestSetTenantAdministratorsWithoutError() {
734+
params, _ := suite.initSetTenantAdministratorsRequest()
735+
tenant := &miniov2.Tenant{
736+
Spec: miniov2.TenantSpec{
737+
Env: []corev1.EnvVar{
738+
{Name: "accesskey", Value: "mock-access"},
739+
{Name: "secretkey", Value: "mock-secret"},
740+
},
741+
},
742+
}
743+
err := setTenantAdministrators(context.Background(), tenant, suite.k8sclient, params)
744+
suite.assert.Nil(err)
745+
}
746+
632747
func (suite *TenantTestSuite) initSetTenantAdministratorsRequest() (params operator_api.SetTenantAdministratorsParams, api operations.OperatorAPI) {
633748
registerTenantHandlers(&api)
634749
params.HTTPRequest = &http.Request{}
635750
params.Namespace = "mock-namespace"
636751
params.Tenant = "mock-tenant"
752+
params.Body = &models.SetAdministratorsRequest{}
637753
return params, api
638754
}
639755

operatorapi/tenants_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,9 +1166,9 @@ func Test_UpdateTenantAction(t *testing.T) {
11661166
opClientTenantGetMock = tt.args.mockTenantGet
11671167
opClientTenantPatchMock = tt.args.mockTenantPatch
11681168
httpClientGetMock = tt.args.mockHTTPClientGet
1169-
cnsClient := fake.NewSimpleClientset(tt.objs...)
1169+
cnsClient := k8sClientMock{}
11701170
t.Run(tt.name, func(t *testing.T) {
1171-
if err := updateTenantAction(tt.args.ctx, tt.args.operatorClient, cnsClient.CoreV1(), tt.args.httpCl, tt.args.nameSpace, tt.args.params); (err != nil) != tt.wantErr {
1171+
if err := updateTenantAction(tt.args.ctx, tt.args.operatorClient, cnsClient, tt.args.httpCl, tt.args.nameSpace, tt.args.params); (err != nil) != tt.wantErr {
11721172
t.Errorf("updateTenantAction() error = %v, wantErr %v", err, tt.wantErr)
11731173
}
11741174
})

0 commit comments

Comments
 (0)