From a2e7259ccb8d5acea3bcf6dea082045ba8716a46 Mon Sep 17 00:00:00 2001 From: Daniel Valdivia Date: Tue, 11 Aug 2020 18:20:43 -0700 Subject: [PATCH] =?UTF-8?q?Allow=20to=20Specify=20the=20Tenant=20Console?= =?UTF-8?q?=20Image.=20Support=20Image=20Pull=20Secrets=E2=80=A6=20(#245)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Allow to Specify the Tenant Console Image. Support Image Pull Secrets by Name. This PR adds support for `console_image` on create tenant and update tenant so the console image can be set by the caller. This is in case the image used is hosted in a private registry. Also adds support to specify the Image Pull Secret, if it's not specified, the individual image registry credentials can still be specified. * Add tests for new fields. --- models/create_tenant_request.go | 6 +++ models/update_tenant_request.go | 24 +++++++++++ restapi/admin_tenants.go | 74 +++++++++++++++++++++------------ restapi/admin_tenants_test.go | 57 ++++++++++++++++++++++++- restapi/embedded_spec.go | 26 ++++++++++++ swagger.yml | 9 ++++ 6 files changed, 168 insertions(+), 28 deletions(-) diff --git a/models/create_tenant_request.go b/models/create_tenant_request.go index 876b2db0b..0ab097599 100644 --- a/models/create_tenant_request.go +++ b/models/create_tenant_request.go @@ -42,6 +42,9 @@ type CreateTenantRequest struct { // annotations Annotations map[string]string `json:"annotations,omitempty"` + // console image + ConsoleImage string `json:"console_image,omitempty"` + // enable console EnableConsole *bool `json:"enable_console,omitempty"` @@ -60,6 +63,9 @@ type CreateTenantRequest struct { // image Image string `json:"image,omitempty"` + // image pull secret + ImagePullSecret string `json:"image_pull_secret,omitempty"` + // image registry ImageRegistry *ImageRegistry `json:"image_registry,omitempty"` diff --git a/models/update_tenant_request.go b/models/update_tenant_request.go index e3f5c260d..324e22595 100644 --- a/models/update_tenant_request.go +++ b/models/update_tenant_request.go @@ -34,10 +34,17 @@ import ( // swagger:model updateTenantRequest type UpdateTenantRequest struct { + // console image + // Pattern: ^((.*?)/(.*?):(.+))$ + ConsoleImage string `json:"console_image,omitempty"` + // image // Pattern: ^((.*?)/(.*?):(.+))$ Image string `json:"image,omitempty"` + // image pull secret + ImagePullSecret string `json:"image_pull_secret,omitempty"` + // image registry ImageRegistry *ImageRegistry `json:"image_registry,omitempty"` } @@ -46,6 +53,10 @@ type UpdateTenantRequest struct { func (m *UpdateTenantRequest) Validate(formats strfmt.Registry) error { var res []error + if err := m.validateConsoleImage(formats); err != nil { + res = append(res, err) + } + if err := m.validateImage(formats); err != nil { res = append(res, err) } @@ -60,6 +71,19 @@ func (m *UpdateTenantRequest) Validate(formats strfmt.Registry) error { return nil } +func (m *UpdateTenantRequest) validateConsoleImage(formats strfmt.Registry) error { + + if swag.IsZero(m.ConsoleImage) { // not required + return nil + } + + if err := validate.Pattern("console_image", "body", string(m.ConsoleImage), `^((.*?)/(.*?):(.+))$`); err != nil { + return err + } + + return nil +} + func (m *UpdateTenantRequest) validateImage(formats strfmt.Registry) error { if swag.IsZero(m.Image) { // not required diff --git a/restapi/admin_tenants.go b/restapi/admin_tenants.go index cda9f85a0..15fd2d5ba 100644 --- a/restapi/admin_tenants.go +++ b/restapi/admin_tenants.go @@ -54,10 +54,6 @@ import ( v1 "k8s.io/client-go/kubernetes/typed/core/v1" ) -const ( - minioRegCred = "minio-regcred-secret" -) - type imageRegistry struct { Auths map[string]imageRegistryCredentials `json:"auths"` } @@ -589,7 +585,7 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create const consoleVersion = "minio/console:v0.3.11" minInst.Spec.Console = &operator.ConsoleConfiguration{ - Replicas: 2, + Replicas: 1, Image: consoleVersion, ConsoleSecret: &corev1.LocalObjectReference{Name: consoleSecretName}, Resources: corev1.ResourceRequirements{ @@ -660,13 +656,25 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create minInst.Spec.Mountpath = tenantReq.MounthPath } - if err := setImageRegistry(ctx, tenantReq.ImageRegistry, clientset.CoreV1(), ns); err != nil { + // We accept either `image_pull_secret` or the individual details of the `image_registry` but not both + var imagePullSecret string + + if tenantReq.ImagePullSecret != "" { + imagePullSecret = tenantReq.ImagePullSecret + } else if imagePullSecret, err = setImageRegistry(ctx, *tenantReq.Name, tenantReq.ImageRegistry, clientset.CoreV1(), ns); err != nil { log.Println("error setting image registry secret:", err) return nil, err } + // pass the image pull secret to the Tenant + if imagePullSecret != "" { + minInst.Spec.ImagePullSecret = corev1.LocalObjectReference{ + Name: imagePullSecret, + } + } - minInst.Spec.ImagePullSecret = corev1.LocalObjectReference{ - Name: minioRegCred, + // set console image if provided + if tenantReq.ConsoleImage != "" { + minInst.Spec.Console.Image = tenantReq.ConsoleImage } opClient, err := cluster.OperatorClient(session.SessionToken) @@ -700,9 +708,11 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create return response, nil } -func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset v1.CoreV1Interface, namespace string) error { +// 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, tenantName string, req *models.ImageRegistry, clientset v1.CoreV1Interface, namespace string) (string, error) { if req == nil || req.Registry == nil || req.Username == nil || req.Password == nil { - return nil + return "", nil } credentials := make(map[string]imageRegistryCredentials) @@ -720,12 +730,14 @@ func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset } imRegistryJSON, err := json.Marshal(imRegistry) if err != nil { - return err + return "", err } + pullSecretName := fmt.Sprintf("%s-regcred", tenantName) + instanceSecret := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: minioRegCred, + Name: pullSecretName, }, Data: map[string][]byte{ corev1.DockerConfigJsonKey: []byte(string(imRegistryJSON)), @@ -734,22 +746,22 @@ func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset } // Get or Create secret if it doesn't exist - _, err = clientset.Secrets(namespace).Get(ctx, minioRegCred, metav1.GetOptions{}) + _, err = clientset.Secrets(namespace).Get(ctx, pullSecretName, metav1.GetOptions{}) if err != nil { if k8sErrors.IsNotFound(err) { _, err = clientset.Secrets(namespace).Create(ctx, &instanceSecret, metav1.CreateOptions{}) if err != nil { - return err + return "", err } - return nil + return "", nil } - return err + return "", err } _, err = clientset.Secrets(namespace).Update(ctx, &instanceSecret, metav1.UpdateOptions{}) if err != nil { - return err + return "", err } - return nil + return pullSecretName, nil } // updateTenantAction does an update on the minioTenant by patching the desired changes @@ -757,25 +769,35 @@ func updateTenantAction(ctx context.Context, operatorClient OperatorClient, clie imageToUpdate := params.Body.Image imageRegistryReq := params.Body.ImageRegistry - if err := setImageRegistry(ctx, imageRegistryReq, clientset, namespace); err != nil { - log.Println("error setting image registry secret:", err) - return err - } - minInst, err := operatorClient.TenantGet(ctx, namespace, params.Tenant, metav1.GetOptions{}) if err != nil { return err } + // we can take either the `image_pull_secret` of the `image_registry` but not both + if params.Body.ImagePullSecret != "" { + minInst.Spec.ImagePullSecret.Name = params.Body.ImagePullSecret + } else { + // update the image pull secret content + if _, err := setImageRegistry(ctx, params.Tenant, imageRegistryReq, clientset, namespace); err != nil { + log.Println("error setting image registry secret:", err) + return err + } + } + + // update the console image + if strings.TrimSpace(params.Body.ConsoleImage) != "" && minInst.Spec.Console != nil { + minInst.Spec.Console.Image = params.Body.ConsoleImage + } // if image to update is empty we'll use the latest image by default if strings.TrimSpace(imageToUpdate) != "" { minInst.Spec.Image = imageToUpdate } else { im, err := cluster.GetLatestMinioImage(httpCl) - if err != nil { - return err + // if we can't get the MinIO image, we won' auto-update it unless it's explicit by name + if err == nil { + minInst.Spec.Image = *im } - minInst.Spec.Image = *im } payloadBytes, err := json.Marshal(minInst) diff --git a/restapi/admin_tenants_test.go b/restapi/admin_tenants_test.go index ed28b2c6b..9d22285ea 100644 --- a/restapi/admin_tenants_test.go +++ b/restapi/admin_tenants_test.go @@ -647,6 +647,7 @@ func Test_UpdateTenantAction(t *testing.T) { return &http.Response{}, nil }, params: admin_api.UpdateTenantParams{ + Tenant: "minio-tenant", Body: &models.UpdateTenantRequest{ Image: "minio/minio:RELEASE.2020-06-03T22-13-49Z", }, @@ -675,6 +676,7 @@ func Test_UpdateTenantAction(t *testing.T) { }, nil }, params: admin_api.UpdateTenantParams{ + Tenant: "minio-tenant", Body: &models.UpdateTenantRequest{ Image: "", }, @@ -683,7 +685,7 @@ func Test_UpdateTenantAction(t *testing.T) { wantErr: false, }, { - name: "Empty image input Error retrieving latest image", + name: "Empty image input Error retrieving latest image, nothing happens", args: args{ ctx: context.Background(), operatorClient: opClient, @@ -700,12 +702,63 @@ func Test_UpdateTenantAction(t *testing.T) { return nil, errors.New("error") }, params: admin_api.UpdateTenantParams{ + Tenant: "minio-tenant", Body: &models.UpdateTenantRequest{ Image: "", }, }, }, - wantErr: true, + wantErr: false, + }, + { + name: "Update minio console version no errors", + args: args{ + ctx: context.Background(), + operatorClient: opClient, + httpCl: httpClientM, + nameSpace: "default", + tenantName: "minio-tenant", + mockTenantPatch: func(ctx context.Context, namespace string, tenantName string, pt types.PatchType, data []byte, options metav1.PatchOptions) (*v1.Tenant, error) { + return &v1.Tenant{}, nil + }, + mockTenantGet: func(ctx context.Context, namespace string, tenantName string, options metav1.GetOptions) (*v1.Tenant, error) { + return &v1.Tenant{}, nil + }, + mockHTTPClientGet: func(url string) (resp *http.Response, err error) { + return nil, errors.New("use default minio") + }, + params: admin_api.UpdateTenantParams{ + Body: &models.UpdateTenantRequest{ + ConsoleImage: "minio/console:v0.3.11", + }, + }, + }, + wantErr: false, + }, + { + name: "Update minio image pull secrets no errors", + args: args{ + ctx: context.Background(), + operatorClient: opClient, + httpCl: httpClientM, + nameSpace: "default", + tenantName: "minio-tenant", + mockTenantPatch: func(ctx context.Context, namespace string, tenantName string, pt types.PatchType, data []byte, options metav1.PatchOptions) (*v1.Tenant, error) { + return &v1.Tenant{}, nil + }, + mockTenantGet: func(ctx context.Context, namespace string, tenantName string, options metav1.GetOptions) (*v1.Tenant, error) { + return &v1.Tenant{}, nil + }, + mockHTTPClientGet: func(url string) (resp *http.Response, err error) { + return nil, errors.New("use default minio") + }, + params: admin_api.UpdateTenantParams{ + Body: &models.UpdateTenantRequest{ + ImagePullSecret: "minio-regcred", + }, + }, + }, + wantErr: false, }, } for _, tt := range tests { diff --git a/restapi/embedded_spec.go b/restapi/embedded_spec.go index 1b8ecaaba..64f6aa45e 100644 --- a/restapi/embedded_spec.go +++ b/restapi/embedded_spec.go @@ -2024,6 +2024,9 @@ func init() { "type": "string" } }, + "console_image": { + "type": "string" + }, "enable_console": { "type": "boolean", "default": true @@ -2046,6 +2049,9 @@ func init() { "image": { "type": "string" }, + "image_pull_secret": { + "type": "string" + }, "image_registry": { "$ref": "#/definitions/imageRegistry" }, @@ -3059,10 +3065,17 @@ func init() { "updateTenantRequest": { "type": "object", "properties": { + "console_image": { + "type": "string", + "pattern": "^((.*?)/(.*?):(.+))$" + }, "image": { "type": "string", "pattern": "^((.*?)/(.*?):(.+))$" }, + "image_pull_secret": { + "type": "string" + }, "image_registry": { "$ref": "#/definitions/imageRegistry" } @@ -5938,6 +5951,9 @@ func init() { "type": "string" } }, + "console_image": { + "type": "string" + }, "enable_console": { "type": "boolean", "default": true @@ -5960,6 +5976,9 @@ func init() { "image": { "type": "string" }, + "image_pull_secret": { + "type": "string" + }, "image_registry": { "$ref": "#/definitions/imageRegistry" }, @@ -6907,10 +6926,17 @@ func init() { "updateTenantRequest": { "type": "object", "properties": { + "console_image": { + "type": "string", + "pattern": "^((.*?)/(.*?):(.+))$" + }, "image": { "type": "string", "pattern": "^((.*?)/(.*?):(.+))$" }, + "image_pull_secret": { + "type": "string" + }, "image_registry": { "$ref": "#/definitions/imageRegistry" } diff --git a/swagger.yml b/swagger.yml index b570546be..251ccb9a0 100644 --- a/swagger.yml +++ b/swagger.yml @@ -1778,8 +1778,13 @@ definitions: image: type: string pattern: "^((.*?)/(.*?):(.+))$" + console_image: + type: string + pattern: "^((.*?)/(.*?):(.+))$" image_registry: $ref: "#/definitions/imageRegistry" + image_pull_secret: + type: string imageRegistry: type: object @@ -1807,6 +1812,8 @@ definitions: pattern: "^[a-z0-9-]{3,63}$" image: type: string + console_image: + type: string service_name: type: string zones: @@ -1835,6 +1842,8 @@ definitions: type: string image_registry: $ref: "#/definitions/imageRegistry" + image_pull_secret: + type: string idp: type: object $ref: "#/definitions/idpConfiguration"