refactor for suggestions

Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>

pkg/common/const.go

@@ -36,6 +36,15 @@ const (
 	OperatorRuntimeOpenshift Runtime = "OPENSHIFT"
 	// OperatorRuntimeRancher is the Rancher runtime flag
 	OperatorRuntimeRancher Runtime = "RANCHER"
+
+	// TLSCRT is  name of the field containing tls certificate in secret
+	TLSCRT = "tls.crt"
+
+	// CACRT name of the field containing ca certificate in secret
+	CACRT = "ca.crt"
+
+	// PublicCRT name of the field containing public certificate in secret
+	PublicCRT = "public.crt"
 )
 
 // Runtimes is a map of the supported Kubernetes runtimes

pkg/controller/main-controller.go

@@ -904,7 +904,7 @@ func (c *Controller) syncHandler(key string) (Result, error) {
 		return WrapResult(Result{}, err)
 	}
 
-	adminClnt, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport(false))
+	adminClnt, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport())
 	if err != nil {
 		if _, uerr := c.updateTenantStatus(ctx, tenant, StatusTenantCredentialsNotSet, 0); uerr != nil {
 			return WrapResult(Result{}, uerr)
@@ -1083,7 +1083,7 @@ func (c *Controller) syncHandler(key string) (Result, error) {
 			}
 		} else {
 			// check if MinIO is already online after the previous restart
-			if tenant.MinIOHealthCheck(c.getTransport(false)) {
+			if tenant.MinIOHealthCheck(c.getTransport()) {
 				tenant.Status.WaitingOnReady = nil
 				if _, err = c.updatePoolStatus(ctx, tenant); err != nil {
 					klog.Infof("'%s' Can't update tenant status: %v", key, err)
@@ -1120,7 +1120,7 @@ func (c *Controller) syncHandler(key string) (Result, error) {
 		ssImage = ssImages[1]
 	}
 	if specImage != ssImage && tenant.Status.CurrentState != StatusUpdatingMinIOVersion {
-		if !tenant.MinIOHealthCheck(c.getTransport(false)) {
+		if !tenant.MinIOHealthCheck(c.getTransport()) {
 			klog.Infof("%s is not running can't update image online", key)
 			return WrapResult(Result{}, ErrMinIONotReady)
 		}

pkg/controller/minio.go

@@ -30,6 +30,8 @@ import (
 	"slices"
 	"time"
 
+	"github.com/minio/operator/pkg/common"
+
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"github.com/minio/operator/pkg/controller/certificates"
@@ -113,9 +115,9 @@ func (c *Controller) getTLSSecret(ctx context.Context, nsName string, secretName
 
 func getOperatorCertFromSecret(secretData map[string][]byte, key string) ([]byte, error) {
 	keys := []string{
-		"tls.crt",
-		"ca.crt",
-		"public.crt",
+		common.TLSCRT,
+		common.CACRT,
+		common.PublicCRT,
 	}
 	if slices.Contains(keys, key) {
 		data, ok := secretData[key]
@@ -140,22 +142,22 @@ func (c *Controller) checkOperatorCaForTenant(ctx context.Context, tenant *minio
 		return false, err
 	}
 
-	operatorPublicCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, "public.crt")
+	operatorPublicCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, common.PublicCRT)
 	if err != nil {
 		// If no public.crt is present we error, other certs are optional
 		return false, err
 	}
 
-	certsData["public.crt"] = operatorPublicCert
+	certsData[common.PublicCRT] = operatorPublicCert
 
-	operatorTLSCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, "tls.crt")
+	operatorTLSCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, common.TLSCRT)
 	if err == nil {
-		certsData["tls.crt"] = operatorTLSCert
+		certsData[common.TLSCRT] = operatorTLSCert
 	}
 
-	operatorCACert, err := getOperatorCertFromSecret(operatorCaSecret.Data, "ca.crt")
+	operatorCACert, err := getOperatorCertFromSecret(operatorCaSecret.Data, common.CACRT)
 	if err == nil {
-		certsData["ca.crt"] = operatorCACert
+		certsData[common.CACRT] = operatorCACert
 	}
 
 	var tenantCaSecret *corev1.Secret
@@ -193,39 +195,39 @@ func (c *Controller) checkOperatorCaForTenant(ctx context.Context, tenant *minio
 		}
 	}
 
-	publicCert, ok := tenantCaSecret.Data["public.crt"]
+	publicCert, ok := tenantCaSecret.Data[common.PublicCRT]
 	if ok && !bytes.Equal(publicCert, operatorPublicCert) {
-		tenantCaSecret.Data["public.crt"] = operatorPublicCert
+		tenantCaSecret.Data[common.PublicCRT] = operatorPublicCert
 		_, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Update(ctx, tenantCaSecret, metav1.UpdateOptions{})
 		if err != nil {
 			return false, err
 		}
 		// Reload certificates
-		c.getTransport(true)
+		c.createTransport()
 		return false, fmt.Errorf("'public.crt' in '%s/%s' secret changed, updating '%s/%s' secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName, tenant.Namespace, OperatorCATLSSecretName)
 	}
 
-	tlsCert, ok := tenantCaSecret.Data["tls.crt"]
+	tlsCert, ok := tenantCaSecret.Data[common.TLSCRT]
 	if ok && !bytes.Equal(tlsCert, operatorTLSCert) {
-		tenantCaSecret.Data["tls.crt"] = tlsCert
+		tenantCaSecret.Data[common.TLSCRT] = tlsCert
 		_, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Update(ctx, tenantCaSecret, metav1.UpdateOptions{})
 		if err != nil {
 			return false, err
 		}
 		// Reload certificates
-		c.getTransport(true)
+		c.createTransport()
 		return false, fmt.Errorf("'tls.crt' in '%s/%s' secret changed, updating '%s/%s' secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName, tenant.Namespace, OperatorCATLSSecretName)
 	}
 
-	caCert, ok := tenantCaSecret.Data["ca.crt"]
+	caCert, ok := tenantCaSecret.Data[common.CACRT]
 	if ok && !bytes.Equal(caCert, operatorCACert) {
-		tenantCaSecret.Data["ca.crt"] = caCert
+		tenantCaSecret.Data[common.CACRT] = caCert
 		_, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Update(ctx, tenantCaSecret, metav1.UpdateOptions{})
 		if err != nil {
 			return false, err
 		}
 		// Reload certificates
-		c.getTransport(true)
+		c.createTransport()
 		return false, fmt.Errorf("'ca.crt' in '%s/%s' secret changed, updating '%s/%s' secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName, tenant.Namespace, OperatorCATLSSecretName)
 	}
 

pkg/controller/monitoring.go

@@ -100,7 +100,7 @@ func (c *Controller) updateHealthStatusForTenant(tenant *miniov2.Tenant) error {
 		return err
 	}
 
-	adminClnt, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport(false))
+	adminClnt, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport())
 	if err != nil {
 		klog.Errorf("Error instantiating adminClnt '%s/%s': %v", tenant.Namespace, tenant.Name, err)
 		return err
@@ -112,7 +112,7 @@ func (c *Controller) updateHealthStatusForTenant(tenant *miniov2.Tenant) error {
 		klog.Infof("'%s/%s': %v", tenant.Namespace, tenant.Name, err)
 		return nil
 	}
-	aClnt.SetCustomTransport(c.getTransport(false))
+	aClnt.SetCustomTransport(c.getTransport())
 
 	hctx, hcancel := context.WithTimeout(context.Background(), 60*time.Second)
 	defer hcancel()

pkg/controller/operator.go

@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"crypto/tls"
+	"crypto/x509"
 	"net"
 	"net/http"
 	"time"
@@ -90,11 +91,48 @@ func (c *Controller) fetchUserCredentials(ctx context.Context, tenant *miniov2.T
 }
 
 // getTransport returns a *http.Transport with the collection of the trusted CA certificates
-// reload=true forces to reload the CA certificates instead of return the cached transport
-func (c *Controller) getTransport(reload bool) *http.Transport {
-	if c.transport != nil && !reload {
+// returns a cached transport if already available
+func (c *Controller) getTransport() *http.Transport {
+	if c.transport != nil {
 		return c.transport
 	}
+	c.transport = c.createTransport()
+	return c.transport
+}
+
+// createTransport returns a *http.Transport with the collection of the trusted CA certificates
+func (c *Controller) createTransport() *http.Transport {
+	rootCAs := c.fetchTransportCACertificates()
+	dialer := &net.Dialer{
+		Timeout:   15 * time.Second,
+		KeepAlive: 15 * time.Second,
+	}
+	c.transport = &http.Transport{
+		Proxy:                 http.ProxyFromEnvironment,
+		DialContext:           dialer.DialContext,
+		MaxIdleConnsPerHost:   1024,
+		IdleConnTimeout:       15 * time.Second,
+		ResponseHeaderTimeout: 15 * time.Minute,
+		TLSHandshakeTimeout:   15 * time.Second,
+		ExpectContinueTimeout: 15 * time.Second,
+		// Go net/http automatically unzip if content-type is
+		// gzip disable this feature, as we are always interested
+		// in raw stream.
+		DisableCompression: true,
+		TLSClientConfig: &tls.Config{
+			// Can't use SSLv3 because of POODLE and BEAST
+			// Can't use TLSv1.0 because of POODLE and BEAST using CBC cipher
+			// Can't use TLSv1.1 because of RC4 cipher usage
+			MinVersion: tls.VersionTLS12,
+			RootCAs:    rootCAs,
+		},
+	}
+
+	return c.transport
+}
+
+// fetchTransportCACertificates retrieves a *x509.CertPool with all CA that operator will trust
+func (c *Controller) fetchTransportCACertificates() (pool *x509.CertPool) {
 	rootCAs := miniov2.MustGetSystemCertPool()
 	// Default kubernetes CA certificate
 	rootCAs.AppendCertsFromPEM(miniov2.GetPodCAFromFile())
@@ -142,32 +180,7 @@ func (c *Controller) getTransport(reload bool) *http.Transport {
 			}
 		}
 	}
-	dialer := &net.Dialer{
-		Timeout:   15 * time.Second,
-		KeepAlive: 15 * time.Second,
-	}
-	c.transport = &http.Transport{
-		Proxy:                 http.ProxyFromEnvironment,
-		DialContext:           dialer.DialContext,
-		MaxIdleConnsPerHost:   1024,
-		IdleConnTimeout:       15 * time.Second,
-		ResponseHeaderTimeout: 15 * time.Minute,
-		TLSHandshakeTimeout:   15 * time.Second,
-		ExpectContinueTimeout: 15 * time.Second,
-		// Go net/http automatically unzip if content-type is
-		// gzip disable this feature, as we are always interested
-		// in raw stream.
-		DisableCompression: true,
-		TLSClientConfig: &tls.Config{
-			// Can't use SSLv3 because of POODLE and BEAST
-			// Can't use TLSv1.0 because of POODLE and BEAST using CBC cipher
-			// Can't use TLSv1.1 because of RC4 cipher usage
-			MinVersion: tls.VersionTLS12,
-			RootCAs:    rootCAs,
-		},
-	}
-
-	return c.transport
+	return rootCAs
 }
 
 func (c *Controller) createUsers(ctx context.Context, tenant *miniov2.Tenant, tenantConfiguration map[string][]byte) (err error) {
@@ -189,7 +202,7 @@ func (c *Controller) createUsers(ctx context.Context, tenant *miniov2.Tenant, te
 	}
 
 	// get a new admin client
-	adminClient, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport(false))
+	adminClient, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport())
 	if err != nil {
 		klog.Errorf("Error instantiating adminClnt: %v", err)
 		return err
@@ -223,7 +236,7 @@ func (c *Controller) createBuckets(ctx context.Context, tenant *miniov2.Tenant,
 	}
 
 	// get a new admin client
-	minioClient, err := tenant.NewMinIOUser(tenantConfiguration, c.getTransport(false))
+	minioClient, err := tenant.NewMinIOUser(tenantConfiguration, c.getTransport())
 	if err != nil {
 		// show the error and continue
 		klog.Errorf("Error instantiating minio Client: %v ", err)

pkg/controller/pools.go

@@ -119,7 +119,7 @@ func (c *Controller) restartInitializedPool(ctx context.Context, tenant *miniov2
 	}
 
 	livePodAddress := fmt.Sprintf("%s:9000", tenant.MinIOHLPodHostname(livePod.Name))
-	livePodAdminClnt, err := tenant.NewMinIOAdminForAddress(livePodAddress, tenantConfiguration, c.getTransport(false))
+	livePodAdminClnt, err := tenant.NewMinIOAdminForAddress(livePodAddress, tenantConfiguration, c.getTransport())
 	if err != nil {
 		return err
 	}

pkg/controller/sts.go

@@ -348,7 +348,7 @@ func AssumeRole(ctx context.Context, c *Controller, tenant *miniov2.Tenant, sess
 // getTenantClient returns an http client that can be used to connect with the tenant
 func getTenantClient(ctx context.Context, c *Controller, tenant *miniov2.Tenant) (*http.Client, string, string, error) {
 	tenantConfiguration, err := c.getTenantCredentials(ctx, tenant)
-	transport := c.getTransport(false)
+	transport := c.getTransport()
 	if err != nil {
 		return nil, "", "", err
 	}

pkg/controller/sts_handlers.go

@@ -172,7 +172,7 @@ func (c *Controller) AssumeRoleWithWebIdentityHandler(w http.ResponseWriter, r *
 		writeSTSErrorResponse(w, true, ErrSTSInternalError, fmt.Errorf("Error getting tenant '%s' root credentials: %s", tenant.Name, err))
 		return
 	}
-	adminClient, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport(false))
+	adminClient, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport())
 	if err != nil {
 		writeSTSErrorResponse(w, true, ErrSTSInternalError, fmt.Errorf("Error communicating with tenant '%s': %s", tenant.Name, err))
 		return