diff --git a/.github/workflows/jobs.yaml b/.github/workflows/jobs.yaml index f83b79faa..0bb161d86 100644 --- a/.github/workflows/jobs.yaml +++ b/.github/workflows/jobs.yaml @@ -1542,7 +1542,7 @@ jobs: go tool cover -func=all.out | grep total > tmp2 result=`cat tmp2 | awk 'END {print $3}'` result=${result%\%} - threshold=64.9 + threshold=65.4 echo "Result:" echo "$result%" if (( $(echo "$result >= $threshold" |bc -l) )); then diff --git a/operatorapi/tenant_add.go b/operatorapi/tenant_add.go index 86152cd36..a54afdc12 100644 --- a/operatorapi/tenant_add.go +++ b/operatorapi/tenant_add.go @@ -34,7 +34,6 @@ import ( miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "k8s.io/client-go/kubernetes/typed/core/v1" ) 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 return nil, restapi.ErrorWithContext(ctx, err) } - return createTenant(ctx, params, k8sClient, k8sClient.client.CoreV1(), session) + return createTenant(ctx, params, k8sClient, session) } -func createTenant(ctx context.Context, params operator_api.CreateTenantParams, clientSet K8sClientI, cv1 v1.CoreV1Interface, session *models.Principal) (response *models.CreateTenantResponse, mError *models.Error) { +func createTenant(ctx context.Context, params operator_api.CreateTenantParams, clientSet K8sClientI, session *models.Principal) (response *models.CreateTenantResponse, mError *models.Error) { tenantReq := params.Body minioImage := getTenantMinIOImage(tenantReq.Image) @@ -240,7 +239,7 @@ func createTenant(ctx context.Context, params operator_api.CreateTenantParams, c if tenantReq.ImagePullSecret != "" { imagePullSecret = tenantReq.ImagePullSecret - } else if imagePullSecret, err = setImageRegistry(ctx, tenantReq.ImageRegistry, cv1, ns, tenantName); err != nil { + } else if imagePullSecret, err = setImageRegistry(ctx, tenantReq.ImageRegistry, clientSet, ns, tenantName); err != nil { return nil, restapi.ErrorWithContext(ctx, err) } // pass the image pull secret to the Tenant diff --git a/operatorapi/tenant_update.go b/operatorapi/tenant_update.go index f08b0d401..7e37420bd 100644 --- a/operatorapi/tenant_update.go +++ b/operatorapi/tenant_update.go @@ -31,11 +31,10 @@ import ( miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - v1 "k8s.io/client-go/kubernetes/typed/core/v1" ) // updateTenantAction does an update on the minioTenant by patching the desired changes -func updateTenantAction(ctx context.Context, operatorClient OperatorClientI, clientset v1.CoreV1Interface, httpCl http.ClientI, namespace string, params operator_api.UpdateTenantParams) error { +func updateTenantAction(ctx context.Context, operatorClient OperatorClientI, clientset K8sClientI, httpCl http.ClientI, namespace string, params operator_api.UpdateTenantParams) error { imageToUpdate := params.Body.Image imageRegistryReq := params.Body.ImageRegistry diff --git a/operatorapi/tenants.go b/operatorapi/tenants.go index 59abdfd20..6830ff26e 100644 --- a/operatorapi/tenants.go +++ b/operatorapi/tenants.go @@ -924,6 +924,10 @@ func getSetTenantAdministratorsResponse(session *models.Principal, params operat if err != nil { return restapi.ErrorWithContext(ctx, err) } + return setTenantAdministrators(ctx, minTenant, k8sClient, params) +} + +func setTenantAdministrators(ctx context.Context, minTenant *miniov2.Tenant, k8sClient K8sClientI, params operator_api.SetTenantAdministratorsParams) *models.Error { minTenant.EnsureDefaults() svcURL := GetTenantServiceURL(minTenant) @@ -963,19 +967,23 @@ func getTenantConfigurationResponse(session *models.Principal, params operator_a opClient := &operatorClient{ client: opClientClientSet, } + // get Kubernetes Client + clientSet, err := cluster.K8sClient(session.STSSessionToken) + if err != nil { + return nil, restapi.ErrorWithContext(ctx, err) + } + k8sClient := &k8sClient{ + client: clientSet, + } minTenant, err := getTenant(ctx, opClient, params.Namespace, params.Tenant) if err != nil { return nil, restapi.ErrorWithContext(ctx, err) } - // get Kubernetes Client - clientSet, err := cluster.K8sClient(session.STSSessionToken) - k8sClient := k8sClient{ - client: clientSet, - } - if err != nil { - return nil, restapi.ErrorWithContext(ctx, err) - } - tenantConfiguration, err := GetTenantConfiguration(ctx, &k8sClient, minTenant) + return parseTenantConfiguration(ctx, k8sClient, minTenant) +} + +func parseTenantConfiguration(ctx context.Context, k8sClient K8sClientI, minTenant *miniov2.Tenant) (*models.TenantConfigurationResponse, *models.Error) { + tenantConfiguration, err := GetTenantConfiguration(ctx, k8sClient, minTenant) if err != nil { return nil, restapi.ErrorWithContext(ctx, err) } @@ -1335,7 +1343,7 @@ func getListTenantsResponse(session *models.Principal, params operator_api.ListT // setImageRegistry creates a secret to store the private registry credentials, if one exist it updates the existing one // returns the name of the secret created/updated -func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset v1.CoreV1Interface, namespace, tenantName string) (string, error) { +func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset K8sClientI, namespace, tenantName string) (string, error) { if req == nil || req.Registry == nil || req.Username == nil || req.Password == nil { return "", nil } @@ -1363,7 +1371,7 @@ func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset corev1.DockerConfigJsonKey: []byte(string(imRegistryJSON)), } // Get or Create secret if it doesn't exist - currentSecret, err := clientset.Secrets(namespace).Get(ctx, pullSecretName, metav1.GetOptions{}) + currentSecret, err := clientset.getSecret(ctx, namespace, pullSecretName, metav1.GetOptions{}) if err != nil { if k8sErrors.IsNotFound(err) { instanceSecret := corev1.Secret{ @@ -1376,7 +1384,7 @@ func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset Data: secretCredentials, Type: corev1.SecretTypeDockerConfigJson, } - _, err = clientset.Secrets(namespace).Create(ctx, &instanceSecret, metav1.CreateOptions{}) + _, err = clientset.createSecret(ctx, namespace, &instanceSecret, metav1.CreateOptions{}) if err != nil { return "", err } @@ -1385,7 +1393,7 @@ func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset return "", err } currentSecret.Data = secretCredentials - _, err = clientset.Secrets(namespace).Update(ctx, currentSecret, metav1.UpdateOptions{}) + _, err = clientset.updateSecret(ctx, namespace, currentSecret, metav1.UpdateOptions{}) if err != nil { return "", err } @@ -1426,6 +1434,9 @@ func getUpdateTenantResponse(session *models.Principal, params operator_api.Upda if err != nil { return restapi.ErrorWithContext(ctx, err) } + k8sClient := &k8sClient{ + client: clientSet, + } opClient := &operatorClient{ client: opClientClientSet, } @@ -1434,7 +1445,7 @@ func getUpdateTenantResponse(session *models.Principal, params operator_api.Upda Timeout: 4 * time.Second, }, } - if err := updateTenantAction(ctx, opClient, clientSet.CoreV1(), httpC, params.Namespace, params); err != nil { + if err := updateTenantAction(ctx, opClient, k8sClient, httpC, params.Namespace, params); err != nil { return restapi.ErrorWithContext(ctx, err, errors.New("unable to update tenant")) } return nil diff --git a/operatorapi/tenants_2_test.go b/operatorapi/tenants_2_test.go index dead84383..1c56dde08 100644 --- a/operatorapi/tenants_2_test.go +++ b/operatorapi/tenants_2_test.go @@ -32,7 +32,9 @@ import ( "github.com/stretchr/testify/suite" corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" ) type TenantTestSuite struct { @@ -148,7 +150,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongECP() { k8sClientCreateSecretMock = func(ctx context.Context, namespace string, secret *v1.Secret, opts metav1.CreateOptions) (*v1.Secret, error) { return nil, nil } - _, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{}) + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) suite.assert.NotNil(err) } @@ -174,7 +176,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongActiveDirectoryConfig() { return nil, nil } - _, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{}) + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) suite.assert.NotNil(err) } @@ -196,7 +198,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongBuiltInUsers() { } return nil, nil } - _, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{}) + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) suite.assert.NotNil(err) } @@ -227,7 +229,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithOIDCAndWrongServerCertificates k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error { return nil } - _, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{}) + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) suite.assert.NotNil(err) } @@ -246,7 +248,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongClientCertificates() { k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error { return nil } - _, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{}) + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) suite.assert.NotNil(err) } @@ -264,7 +266,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongCAsCertificates() { } return nil, nil } - _, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{}) + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) suite.assert.NotNil(err) } @@ -283,7 +285,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongMtlsCertificates() { k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error { return nil } - _, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{}) + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) suite.assert.NotNil(err) } @@ -304,7 +306,7 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongKESConfig() { k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error { return nil } - _, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{}) + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) suite.assert.NotNil(err) } @@ -318,7 +320,55 @@ func (suite *TenantTestSuite) TestCreateTenantWithWrongPool() { k8sClientDeleteSecretMock = func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error { return nil } - _, err := createTenant(context.Background(), params, suite.k8sclient, nil, &models.Principal{}) + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) + suite.assert.NotNil(err) +} + +func (suite *TenantTestSuite) TestCreateTenantWithImageRegistryCreateError() { + params, _ := suite.initCreateTenantRequest() + params.Body.MountPath = "/mock-path" + registry := "mock-registry" + username := "mock-username" + password := "mock-password" + params.Body.ImageRegistry = &models.ImageRegistry{ + Registry: ®istry, + Username: &username, + Password: &password, + } + + k8sClientCreateSecretMock = func(ctx context.Context, namespace string, secret *v1.Secret, opts metav1.CreateOptions) (*v1.Secret, error) { + if strings.HasPrefix(secret.Name, fmt.Sprintf("%s-secret", *params.Body.Name)) { + return nil, nil + } + return nil, errors.New("mock-create-error") + } + k8sclientGetSecretMock = func(ctx context.Context, namespace, secretName string, opts metav1.GetOptions) (*corev1.Secret, error) { + return nil, k8sErrors.NewNotFound(schema.GroupResource{}, "") + } + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) + suite.assert.NotNil(err) +} + +func (suite *TenantTestSuite) TestCreateTenantWithImageRegistryUpdateError() { + params, _ := suite.initCreateTenantRequest() + registry := "mock-registry" + username := "mock-username" + password := "mock-password" + params.Body.ImageRegistry = &models.ImageRegistry{ + Registry: ®istry, + Username: &username, + Password: &password, + } + k8sClientCreateSecretMock = func(ctx context.Context, namespace string, secret *v1.Secret, opts metav1.CreateOptions) (*v1.Secret, error) { + return nil, nil + } + k8sClientUpdateSecretMock = func(ctx context.Context, namespace string, secret *v1.Secret, opts metav1.UpdateOptions) (*v1.Secret, error) { + return nil, errors.New("mock-update-error") + } + k8sclientGetSecretMock = func(ctx context.Context, namespace, secretName string, opts metav1.GetOptions) (*corev1.Secret, error) { + return &v1.Secret{}, nil + } + _, err := createTenant(context.Background(), params, suite.k8sclient, &models.Principal{}) suite.assert.NotNil(err) } @@ -392,6 +442,20 @@ func (suite *TenantTestSuite) initTenantConfigurationRequest() (params operator_ return params, api } +func (suite *TenantTestSuite) TestParseTenantConfigurationWithoutError() { + tenant := &miniov2.Tenant{ + Spec: miniov2.TenantSpec{ + Env: []corev1.EnvVar{ + {Name: "mock", Value: "mock-env"}, + {Name: "mock", Value: "mock-env-2"}, + }, + }, + } + config, err := parseTenantConfiguration(context.Background(), suite.k8sclient, tenant) + suite.assert.NotNil(config) + suite.assert.Nil(err) +} + func (suite *TenantTestSuite) TestUpdateTenantConfigurationHandlerWithError() { params, api := suite.initUpdateTenantConfigurationRequest() response := api.OperatorAPIUpdateTenantConfigurationHandler.Handle(params, &models.Principal{}) @@ -629,11 +693,63 @@ func (suite *TenantTestSuite) TestSetTenantAdministratorsHandlerWithError() { suite.assert.True(ok) } +func (suite *TenantTestSuite) TestSetTenantAdministratorsWithAdminClientError() { + params, _ := suite.initSetTenantAdministratorsRequest() + tenant := &miniov2.Tenant{} + err := setTenantAdministrators(context.Background(), tenant, suite.k8sclient, params) + suite.assert.NotNil(err) +} + +func (suite *TenantTestSuite) TestSetTenantAdministratorsWithUserPolicyError() { + params, _ := suite.initSetTenantAdministratorsRequest() + tenant := &miniov2.Tenant{ + Spec: miniov2.TenantSpec{ + Env: []corev1.EnvVar{ + {Name: "accesskey", Value: "mock-access"}, + {Name: "secretkey", Value: "mock-secret"}, + }, + }, + } + params.Body.UserDNS = []string{"mock-user"} + err := setTenantAdministrators(context.Background(), tenant, suite.k8sclient, params) + suite.assert.NotNil(err) +} + +func (suite *TenantTestSuite) TestSetTenantAdministratorsWithGroupPolicyError() { + params, _ := suite.initSetTenantAdministratorsRequest() + tenant := &miniov2.Tenant{ + Spec: miniov2.TenantSpec{ + Env: []corev1.EnvVar{ + {Name: "accesskey", Value: "mock-access"}, + {Name: "secretkey", Value: "mock-secret"}, + }, + }, + } + params.Body.GroupDNS = []string{"mock-user"} + err := setTenantAdministrators(context.Background(), tenant, suite.k8sclient, params) + suite.assert.NotNil(err) +} + +func (suite *TenantTestSuite) TestSetTenantAdministratorsWithoutError() { + params, _ := suite.initSetTenantAdministratorsRequest() + tenant := &miniov2.Tenant{ + Spec: miniov2.TenantSpec{ + Env: []corev1.EnvVar{ + {Name: "accesskey", Value: "mock-access"}, + {Name: "secretkey", Value: "mock-secret"}, + }, + }, + } + err := setTenantAdministrators(context.Background(), tenant, suite.k8sclient, params) + suite.assert.Nil(err) +} + func (suite *TenantTestSuite) initSetTenantAdministratorsRequest() (params operator_api.SetTenantAdministratorsParams, api operations.OperatorAPI) { registerTenantHandlers(&api) params.HTTPRequest = &http.Request{} params.Namespace = "mock-namespace" params.Tenant = "mock-tenant" + params.Body = &models.SetAdministratorsRequest{} return params, api } diff --git a/operatorapi/tenants_test.go b/operatorapi/tenants_test.go index 4347d233a..4edd0310a 100644 --- a/operatorapi/tenants_test.go +++ b/operatorapi/tenants_test.go @@ -1166,9 +1166,9 @@ func Test_UpdateTenantAction(t *testing.T) { opClientTenantGetMock = tt.args.mockTenantGet opClientTenantPatchMock = tt.args.mockTenantPatch httpClientGetMock = tt.args.mockHTTPClientGet - cnsClient := fake.NewSimpleClientset(tt.objs...) + cnsClient := k8sClientMock{} t.Run(tt.name, func(t *testing.T) { - if err := updateTenantAction(tt.args.ctx, tt.args.operatorClient, cnsClient.CoreV1(), tt.args.httpCl, tt.args.nameSpace, tt.args.params); (err != nil) != tt.wantErr { + if err := updateTenantAction(tt.args.ctx, tt.args.operatorClient, cnsClient, tt.args.httpCl, tt.args.nameSpace, tt.args.params); (err != nil) != tt.wantErr { t.Errorf("updateTenantAction() error = %v, wantErr %v", err, tt.wantErr) } })