Fix Get Latest MinIO Image on Tenant Create (#1703)

* Fix Get Latest MinIO Image on Tenant Create

Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>

* remove `operator_` prefix on files in operatorapi

Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>

cluster/config.go

@@ -61,13 +61,6 @@ func GetNsFromFile() string {
 // Namespace will run only once at console startup
 var Namespace = GetNsFromFile()
 
-var latestMinIOImage, errLatestMinIOImage = utils.GetLatestMinIOImage(
-	&utils.HTTPClient{
-		Client: &http.Client{
-			Timeout: 15 * time.Second,
-		},
-	})
-
 // GetMinioImage returns the image URL to be used when deploying a MinIO instance, if there is
 // a preferred image to be used (configured via ENVIRONMENT VARIABLES) GetMinioImage will return that
 // if not, GetMinioImage will try to obtain the image URL for the latest version of MinIO and return that
@@ -77,17 +70,15 @@ func GetMinioImage() (*string, error) {
 	if image != "" {
 		return &image, nil
 	}
+	latestMinIOImage, errLatestMinIOImage := utils.GetLatestMinIOImage(
+		&utils.HTTPClient{
+			Client: &http.Client{
+				Timeout: 5 * time.Second,
+			},
+		})
+
 	if errLatestMinIOImage != nil {
 		return nil, errLatestMinIOImage
 	}
 	return latestMinIOImage, nil
 }
-
-// GetLatestMinioImage returns the latest image URL on minio repository
-func GetLatestMinioImage(client utils.HTTPClientI) (*string, error) {
-	latestMinIOImage, err := utils.GetLatestMinIOImage(client)
-	if err != nil {
-		return nil, err
-	}
-	return latestMinIOImage, nil
-}

operatorapi/tenant_get.go

@@ -0,0 +1,156 @@
+// This file is part of MinIO Console Server
+// Copyright (c) 2022 MinIO, Inc.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package operatorapi
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/minio/console/cluster"
+	"github.com/minio/console/models"
+	"github.com/minio/console/operatorapi/operations/operator_api"
+	"github.com/minio/console/restapi"
+	miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func getTenantDetailsResponse(session *models.Principal, params operator_api.TenantDetailsParams) (*models.Tenant, *models.Error) {
+	// 5 seconds timeout
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	opClientClientSet, err := cluster.OperatorClient(session.STSSessionToken)
+	if err != nil {
+		return nil, prepareError(err)
+	}
+
+	opClient := &operatorClient{
+		client: opClientClientSet,
+	}
+
+	minTenant, err := getTenant(ctx, opClient, params.Namespace, params.Tenant)
+	if err != nil {
+		return nil, prepareError(err)
+	}
+
+	info := getTenantInfo(minTenant)
+
+	// get Kubernetes Client
+	clientSet, err := cluster.K8sClient(session.STSSessionToken)
+	if err != nil {
+		return nil, prepareError(err)
+	}
+
+	k8sClient := k8sClient{
+		client: clientSet,
+	}
+
+	tenantConfiguration, err := GetTenantConfiguration(ctx, &k8sClient, minTenant)
+	if err != nil {
+		restapi.LogError("unable to fetch configuration for tenant %s: %v", minTenant.Name, err)
+	}
+
+	// detect if AD/LDAP is enabled
+	ldapEnabled := false
+	if string(tenantConfiguration["MINIO_IDENTITY_LDAP_SERVER_ADDR"]) != "" {
+		ldapEnabled = true
+	}
+
+	// detect if OpenID is enabled
+	oidcEnabled := false
+	if string(tenantConfiguration["MINIO_IDENTITY_OPENID_CONFIG_URL"]) != "" {
+		oidcEnabled = true
+	}
+
+	// detect if encryption is enabled
+	if minTenant.HasKESEnabled() || string(tenantConfiguration["MINIO_KMS_SECRET_KEY"]) != "" {
+		info.EncryptionEnabled = true
+	}
+
+	info.LogEnabled = minTenant.HasLogEnabled()
+	info.MonitoringEnabled = minTenant.HasPrometheusEnabled()
+	info.IdpAdEnabled = ldapEnabled
+	info.IdpOidcEnabled = oidcEnabled
+	info.MinioTLS = minTenant.TLS()
+
+	// attach status information
+	info.Status = &models.TenantStatus{
+		HealthStatus:  string(minTenant.Status.HealthStatus),
+		DrivesHealing: minTenant.Status.DrivesHealing,
+		DrivesOffline: minTenant.Status.DrivesOffline,
+		DrivesOnline:  minTenant.Status.DrivesOnline,
+		WriteQuorum:   minTenant.Status.WriteQuorum,
+		Usage: &models.TenantStatusUsage{
+			Raw:           minTenant.Status.Usage.RawCapacity,
+			RawUsage:      minTenant.Status.Usage.RawUsage,
+			Capacity:      minTenant.Status.Usage.Capacity,
+			CapacityUsage: minTenant.Status.Usage.Usage,
+		},
+	}
+
+	// get tenant service
+	minTenant.EnsureDefaults()
+	//minio service
+	minSvc, err := k8sClient.getService(ctx, minTenant.Namespace, minTenant.MinIOCIServiceName(), metav1.GetOptions{})
+	if err != nil {
+		// we can tolerate this error
+		restapi.LogError("Unable to get MinIO service name: %v, continuing", err)
+	}
+	//console service
+	conSvc, err := k8sClient.getService(ctx, minTenant.Namespace, minTenant.ConsoleCIServiceName(), metav1.GetOptions{})
+	if err != nil {
+		// we can tolerate this error
+		restapi.LogError("Unable to get MinIO console service name: %v, continuing", err)
+	}
+
+	schema := "http"
+	consoleSchema := "http"
+	consolePort := fmt.Sprintf(":%d", miniov2.ConsolePort)
+	if minTenant.TLS() {
+		schema = "https"
+		consoleSchema = "https"
+		consolePort = fmt.Sprintf(":%d", miniov2.ConsoleTLSPort)
+	}
+	var minioEndpoint string
+	var consoleEndpoint string
+	if minSvc != nil && len(minSvc.Status.LoadBalancer.Ingress) > 0 {
+		if minSvc.Status.LoadBalancer.Ingress[0].IP != "" {
+			minioEndpoint = fmt.Sprintf("%s://%s", schema, minSvc.Status.LoadBalancer.Ingress[0].IP)
+		}
+
+		if minSvc.Status.LoadBalancer.Ingress[0].Hostname != "" {
+			minioEndpoint = fmt.Sprintf("%s://%s", schema, minSvc.Status.LoadBalancer.Ingress[0].Hostname)
+		}
+
+	}
+	if conSvc != nil && len(conSvc.Status.LoadBalancer.Ingress) > 0 {
+		if conSvc.Status.LoadBalancer.Ingress[0].IP != "" {
+			consoleEndpoint = fmt.Sprintf("%s://%s%s", consoleSchema, conSvc.Status.LoadBalancer.Ingress[0].IP, consolePort)
+		}
+		if conSvc.Status.LoadBalancer.Ingress[0].Hostname != "" {
+			consoleEndpoint = fmt.Sprintf("%s://%s%s", consoleSchema, conSvc.Status.LoadBalancer.Ingress[0].Hostname, consolePort)
+		}
+	}
+	if minioEndpoint != "" || consoleEndpoint != "" {
+		info.Endpoints = &models.TenantEndpoints{
+			Console: consoleEndpoint,
+			Minio:   minioEndpoint,
+		}
+	}
+
+	return info, nil
+}

operatorapi/tenant_update.go

@@ -0,0 +1,103 @@
+// This file is part of MinIO Console Server
+// Copyright (c) 2022 MinIO, Inc.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package operatorapi
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"strings"
+
+	"github.com/minio/console/operatorapi/operations/operator_api"
+	utils2 "github.com/minio/console/pkg/utils"
+	"github.com/minio/console/restapi"
+	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 utils2.HTTPClientI, namespace string, params operator_api.UpdateTenantParams) error {
+	imageToUpdate := params.Body.Image
+	imageRegistryReq := params.Body.ImageRegistry
+
+	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, imageRegistryReq, clientset, namespace, params.Tenant); err != nil {
+			restapi.LogError("error setting image registry secret: %v", err)
+			return err
+		}
+	}
+
+	// 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 := utils2.GetLatestMinIOImage(httpCl)
+		// 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
+		}
+	}
+
+	// Prometheus Annotations
+	currentAnnotations := minInst.Annotations
+	prometheusAnnotations := map[string]string{
+		prometheusPath:   "/minio/prometheus/metrics",
+		prometheusPort:   fmt.Sprint(miniov2.MinIOPort),
+		prometheusScrape: "true",
+	}
+	if params.Body.EnablePrometheus && currentAnnotations != nil {
+		// add prometheus annotations to the tenant
+		minInst.Annotations = addAnnotations(currentAnnotations, prometheusAnnotations)
+		// add prometheus annotations to the each pool
+		if minInst.Spec.Pools != nil {
+			for _, pool := range minInst.Spec.Pools {
+				poolAnnotations := pool.VolumeClaimTemplate.GetObjectMeta().GetAnnotations()
+				pool.VolumeClaimTemplate.GetObjectMeta().SetAnnotations(addAnnotations(poolAnnotations, prometheusAnnotations))
+			}
+		}
+	} else {
+		// remove prometheus annotations to the tenant
+		minInst.Annotations = removeAnnotations(currentAnnotations, prometheusAnnotations)
+		// add prometheus annotations from each pool
+		if minInst.Spec.Pools != nil {
+			for _, pool := range minInst.Spec.Pools {
+				poolAnnotations := pool.VolumeClaimTemplate.GetObjectMeta().GetAnnotations()
+				pool.VolumeClaimTemplate.GetObjectMeta().SetAnnotations(removeAnnotations(poolAnnotations, prometheusAnnotations))
+			}
+		}
+	}
+
+	payloadBytes, err := json.Marshal(minInst)
+	if err != nil {
+		return err
+	}
+	_, err = operatorClient.TenantPatch(ctx, namespace, minInst.Name, types.MergePatchType, payloadBytes, metav1.PatchOptions{})
+	if err != nil {
+		return err
+	}
+	return nil
+}

operatorapi/tenants.go

@@ -528,132 +528,6 @@ func getTenantInfo(tenant *miniov2.Tenant) *models.Tenant {
 	}
 }
 
-func getTenantDetailsResponse(session *models.Principal, params operator_api.TenantDetailsParams) (*models.Tenant, *models.Error) {
-	// 5 seconds timeout
-	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-	defer cancel()
-	opClientClientSet, err := cluster.OperatorClient(session.STSSessionToken)
-	if err != nil {
-		return nil, prepareError(err)
-	}
-
-	opClient := &operatorClient{
-		client: opClientClientSet,
-	}
-
-	minTenant, err := getTenant(ctx, opClient, params.Namespace, params.Tenant)
-	if err != nil {
-		return nil, prepareError(err)
-	}
-
-	info := getTenantInfo(minTenant)
-
-	// get Kubernetes Client
-	clientSet, err := cluster.K8sClient(session.STSSessionToken)
-	if err != nil {
-		return nil, prepareError(err)
-	}
-
-	k8sClient := k8sClient{
-		client: clientSet,
-	}
-
-	tenantConfiguration, err := GetTenantConfiguration(ctx, &k8sClient, minTenant)
-	if err != nil {
-		restapi.LogError("unable to fetch configuration for tenant %s: %v", minTenant.Name, err)
-	}
-
-	// detect if AD/LDAP is enabled
-	ldapEnabled := false
-	if string(tenantConfiguration["MINIO_IDENTITY_LDAP_SERVER_ADDR"]) != "" {
-		ldapEnabled = true
-	}
-
-	// detect if OpenID is enabled
-	oidcEnabled := false
-	if string(tenantConfiguration["MINIO_IDENTITY_OPENID_CONFIG_URL"]) != "" {
-		oidcEnabled = true
-	}
-
-	// detect if encryption is enabled
-	if minTenant.HasKESEnabled() || string(tenantConfiguration["MINIO_KMS_SECRET_KEY"]) != "" {
-		info.EncryptionEnabled = true
-	}
-
-	info.LogEnabled = minTenant.HasLogEnabled()
-	info.MonitoringEnabled = minTenant.HasPrometheusEnabled()
-	info.IdpAdEnabled = ldapEnabled
-	info.IdpOidcEnabled = oidcEnabled
-	info.MinioTLS = minTenant.TLS()
-
-	// attach status information
-	info.Status = &models.TenantStatus{
-		HealthStatus:  string(minTenant.Status.HealthStatus),
-		DrivesHealing: minTenant.Status.DrivesHealing,
-		DrivesOffline: minTenant.Status.DrivesOffline,
-		DrivesOnline:  minTenant.Status.DrivesOnline,
-		WriteQuorum:   minTenant.Status.WriteQuorum,
-		Usage: &models.TenantStatusUsage{
-			Raw:           minTenant.Status.Usage.RawCapacity,
-			RawUsage:      minTenant.Status.Usage.RawUsage,
-			Capacity:      minTenant.Status.Usage.Capacity,
-			CapacityUsage: minTenant.Status.Usage.Usage,
-		},
-	}
-
-	// get tenant service
-	minTenant.EnsureDefaults()
-	//minio service
-	minSvc, err := k8sClient.getService(ctx, minTenant.Namespace, minTenant.MinIOCIServiceName(), metav1.GetOptions{})
-	if err != nil {
-		// we can tolerate this error
-		restapi.LogError("Unable to get MinIO service name: %v, continuing", err)
-	}
-	//console service
-	conSvc, err := k8sClient.getService(ctx, minTenant.Namespace, minTenant.ConsoleCIServiceName(), metav1.GetOptions{})
-	if err != nil {
-		// we can tolerate this error
-		restapi.LogError("Unable to get MinIO console service name: %v, continuing", err)
-	}
-
-	schema := "http"
-	consoleSchema := "http"
-	consolePort := fmt.Sprintf(":%d", miniov2.ConsolePort)
-	if minTenant.TLS() {
-		schema = "https"
-		consoleSchema = "https"
-		consolePort = fmt.Sprintf(":%d", miniov2.ConsoleTLSPort)
-	}
-	var minioEndpoint string
-	var consoleEndpoint string
-	if minSvc != nil && len(minSvc.Status.LoadBalancer.Ingress) > 0 {
-		if minSvc.Status.LoadBalancer.Ingress[0].IP != "" {
-			minioEndpoint = fmt.Sprintf("%s://%s", schema, minSvc.Status.LoadBalancer.Ingress[0].IP)
-		}
-
-		if minSvc.Status.LoadBalancer.Ingress[0].Hostname != "" {
-			minioEndpoint = fmt.Sprintf("%s://%s", schema, minSvc.Status.LoadBalancer.Ingress[0].Hostname)
-		}
-
-	}
-	if conSvc != nil && len(conSvc.Status.LoadBalancer.Ingress) > 0 {
-		if conSvc.Status.LoadBalancer.Ingress[0].IP != "" {
-			consoleEndpoint = fmt.Sprintf("%s://%s%s", consoleSchema, conSvc.Status.LoadBalancer.Ingress[0].IP, consolePort)
-		}
-		if conSvc.Status.LoadBalancer.Ingress[0].Hostname != "" {
-			consoleEndpoint = fmt.Sprintf("%s://%s%s", consoleSchema, conSvc.Status.LoadBalancer.Ingress[0].Hostname, consolePort)
-		}
-	}
-	if minioEndpoint != "" || consoleEndpoint != "" {
-		info.Endpoints = &models.TenantEndpoints{
-			Console: consoleEndpoint,
-			Minio:   minioEndpoint,
-		}
-	}
-
-	return info, nil
-}
-
 func parseCertificate(name string, rawCert []byte) (*models.CertificateInfo, error) {
 	block, _ := pem.Decode(rawCert)
 	if block == nil {
@@ -1040,77 +914,6 @@ func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset
 	return pullSecretName, nil
 }
 
-// updateTenantAction does an update on the minioTenant by patching the desired changes
-func updateTenantAction(ctx context.Context, operatorClient OperatorClientI, clientset v1.CoreV1Interface, httpCl utils2.HTTPClientI, namespace string, params operator_api.UpdateTenantParams) error {
-	imageToUpdate := params.Body.Image
-	imageRegistryReq := params.Body.ImageRegistry
-
-	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, imageRegistryReq, clientset, namespace, params.Tenant); err != nil {
-			restapi.LogError("error setting image registry secret: %v", err)
-			return err
-		}
-	}
-
-	// 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 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
-		}
-	}
-
-	// Prometheus Annotations
-	currentAnnotations := minInst.Annotations
-	prometheusAnnotations := map[string]string{
-		prometheusPath:   "/minio/prometheus/metrics",
-		prometheusPort:   fmt.Sprint(miniov2.MinIOPort),
-		prometheusScrape: "true",
-	}
-	if params.Body.EnablePrometheus && currentAnnotations != nil {
-		// add prometheus annotations to the tenant
-		minInst.Annotations = addAnnotations(currentAnnotations, prometheusAnnotations)
-		// add prometheus annotations to the each pool
-		if minInst.Spec.Pools != nil {
-			for _, pool := range minInst.Spec.Pools {
-				poolAnnotations := pool.VolumeClaimTemplate.GetObjectMeta().GetAnnotations()
-				pool.VolumeClaimTemplate.GetObjectMeta().SetAnnotations(addAnnotations(poolAnnotations, prometheusAnnotations))
-			}
-		}
-	} else {
-		// remove prometheus annotations to the tenant
-		minInst.Annotations = removeAnnotations(currentAnnotations, prometheusAnnotations)
-		// add prometheus annotations from each pool
-		if minInst.Spec.Pools != nil {
-			for _, pool := range minInst.Spec.Pools {
-				poolAnnotations := pool.VolumeClaimTemplate.GetObjectMeta().GetAnnotations()
-				pool.VolumeClaimTemplate.GetObjectMeta().SetAnnotations(removeAnnotations(poolAnnotations, prometheusAnnotations))
-			}
-		}
-	}
-
-	payloadBytes, err := json.Marshal(minInst)
-	if err != nil {
-		return err
-	}
-	_, err = operatorClient.TenantPatch(ctx, namespace, minInst.Name, types.MergePatchType, payloadBytes, metav1.PatchOptions{})
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
 // addAnnotations will merge two annotation maps
 func addAnnotations(annotationsOne, annotationsTwo map[string]string) map[string]string {
 	if annotationsOne == nil {