From 735dbfaff15ea0af5ed7851c3d36e6fc2248c89a Mon Sep 17 00:00:00 2001 From: pjuarezd Date: Tue, 8 Aug 2023 19:05:55 -0600 Subject: [PATCH 1/3] Bugfix reload CA cert in secret: When the CA certificate in the secret `operator-ca-tls` changes, Operator recreates this secret on the Tenants namespace, but is not using the newly provided cert for healthcheck, this will force reload the certificates on secret update --- pkg/controller/main-controller.go | 6 +-- pkg/controller/minio.go | 64 +++++++++++++++++++++++++------ pkg/controller/monitoring.go | 4 +- pkg/controller/operator.go | 10 +++-- pkg/controller/pools.go | 2 +- pkg/controller/sts.go | 2 +- pkg/controller/sts_handlers.go | 2 +- 7 files changed, 66 insertions(+), 24 deletions(-) diff --git a/pkg/controller/main-controller.go b/pkg/controller/main-controller.go index 76220966f0d..373775a41c3 100644 --- a/pkg/controller/main-controller.go +++ b/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()) + adminClnt, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport(false)) 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()) { + if tenant.MinIOHealthCheck(c.getTransport(false)) { 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()) { + if !tenant.MinIOHealthCheck(c.getTransport(false)) { klog.Infof("%s is not running can't update image online", key) return WrapResult(Result{}, ErrMinIONotReady) } diff --git a/pkg/controller/minio.go b/pkg/controller/minio.go index 215fdd5fbaa..5bc9308a23d 100644 --- a/pkg/controller/minio.go +++ b/pkg/controller/minio.go @@ -27,6 +27,7 @@ import ( "fmt" "math/big" "net" + "slices" "time" "k8s.io/apimachinery/pkg/runtime/schema" @@ -110,23 +111,25 @@ func (c *Controller) getTLSSecret(ctx context.Context, nsName string, secretName return c.kubeClientSet.CoreV1().Secrets(nsName).Get(ctx, secretName, metav1.GetOptions{}) } -func getOperatorCACert(secretData map[string][]byte) ([]byte, error) { - for _, key := range []string{ +func getOperatorCertFromSecret(secretData map[string][]byte, key string) ([]byte, error) { + keys := []string{ "tls.crt", "ca.crt", "public.crt", - } { + } + if slices.Contains(keys, key) { data, ok := secretData[key] if ok { return data, nil } } - return nil, fmt.Errorf("missing 'public.crt' in %s/%s secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName) + return nil, fmt.Errorf("missing '%s' in %s/%s secret", key, miniov2.GetNSFromFile(), OperatorCATLSSecretName) } // checkOperatorCaForTenant create or updates the operator-ca-tls secret for tenant if need it func (c *Controller) checkOperatorCaForTenant(ctx context.Context, tenant *miniov2.Tenant) (operatorCATLSExists bool, err error) { - var tenantCaCert []byte + var certsData map[string][]byte + // get operator-ca-tls in minio-operator namespace operatorCaSecret, err := c.kubeClientSet.CoreV1().Secrets(miniov2.GetNSFromFile()).Get(ctx, OperatorCATLSSecretName, metav1.GetOptions{}) if err != nil { @@ -137,11 +140,24 @@ func (c *Controller) checkOperatorCaForTenant(ctx context.Context, tenant *minio return false, err } - operatorCaCert, err := getOperatorCACert(operatorCaSecret.Data) + operatorPublicCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, "public.crt") if err != nil { + // If no public.crt is present we error, other certs are optional return false, err } + certsData["public.crt"] = operatorPublicCert + + operatorTLSCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, "tls.crt") + if err == nil { + certsData["tls.crt"] = operatorTLSCert + } + + operatorCACert, err := getOperatorCertFromSecret(operatorCaSecret.Data, "ca.crt") + if err == nil { + certsData["ca.crt"] = operatorCACert + } + var tenantCaSecret *corev1.Secret createTenantCASecret := func() error { @@ -160,9 +176,7 @@ func (c *Controller) checkOperatorCaForTenant(ctx context.Context, tenant *minio }), }, }, - Data: map[string][]byte{ - "public.crt": operatorCaCert, - }, + Data: certsData, } _, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Create(ctx, tenantCaSecret, metav1.CreateOptions{}) return err @@ -179,16 +193,42 @@ func (c *Controller) checkOperatorCaForTenant(ctx context.Context, tenant *minio } } - tenantCaCert = tenantCaSecret.Data["public.crt"] - if !bytes.Equal(tenantCaCert, operatorCaCert) { - tenantCaSecret.Data["public.crt"] = operatorCaCert + publicCert, ok := tenantCaSecret.Data["public.crt"] + if ok && !bytes.Equal(publicCert, operatorPublicCert) { + tenantCaSecret.Data["public.crt"] = operatorPublicCert _, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Update(ctx, tenantCaSecret, metav1.UpdateOptions{}) if err != nil { return false, err } + // Reload certificates + c.getTransport(true) 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"] + if ok && !bytes.Equal(tlsCert, operatorTLSCert) { + tenantCaSecret.Data["tls.crt"] = tlsCert + _, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Update(ctx, tenantCaSecret, metav1.UpdateOptions{}) + if err != nil { + return false, err + } + // Reload certificates + c.getTransport(true) + 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"] + if ok && !bytes.Equal(caCert, operatorCACert) { + tenantCaSecret.Data["ca.crt"] = caCert + _, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Update(ctx, tenantCaSecret, metav1.UpdateOptions{}) + if err != nil { + return false, err + } + // Reload certificates + c.getTransport(true) + return false, fmt.Errorf("'ca.crt' in '%s/%s' secret changed, updating '%s/%s' secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName, tenant.Namespace, OperatorCATLSSecretName) + } + return true, nil } diff --git a/pkg/controller/monitoring.go b/pkg/controller/monitoring.go index 439696361a7..ae4acd56518 100644 --- a/pkg/controller/monitoring.go +++ b/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()) + adminClnt, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport(false)) 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()) + aClnt.SetCustomTransport(c.getTransport(false)) hctx, hcancel := context.WithTimeout(context.Background(), 60*time.Second) defer hcancel() diff --git a/pkg/controller/operator.go b/pkg/controller/operator.go index f9bf83979a5..771de40e6a5 100644 --- a/pkg/controller/operator.go +++ b/pkg/controller/operator.go @@ -89,8 +89,10 @@ func (c *Controller) fetchUserCredentials(ctx context.Context, tenant *miniov2.T return userCredentials } -func (c *Controller) getTransport() *http.Transport { - if c.transport != nil { +// 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 { return c.transport } rootCAs := miniov2.MustGetSystemCertPool() @@ -187,7 +189,7 @@ func (c *Controller) createUsers(ctx context.Context, tenant *miniov2.Tenant, te } // get a new admin client - adminClient, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport()) + adminClient, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport(false)) if err != nil { klog.Errorf("Error instantiating adminClnt: %v", err) return err @@ -221,7 +223,7 @@ func (c *Controller) createBuckets(ctx context.Context, tenant *miniov2.Tenant, } // get a new admin client - minioClient, err := tenant.NewMinIOUser(tenantConfiguration, c.getTransport()) + minioClient, err := tenant.NewMinIOUser(tenantConfiguration, c.getTransport(false)) if err != nil { // show the error and continue klog.Errorf("Error instantiating minio Client: %v ", err) diff --git a/pkg/controller/pools.go b/pkg/controller/pools.go index 4d8f643daa7..f051f136c0d 100644 --- a/pkg/controller/pools.go +++ b/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()) + livePodAdminClnt, err := tenant.NewMinIOAdminForAddress(livePodAddress, tenantConfiguration, c.getTransport(false)) if err != nil { return err } diff --git a/pkg/controller/sts.go b/pkg/controller/sts.go index 920d482c7de..46dc96520cb 100644 --- a/pkg/controller/sts.go +++ b/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() + transport := c.getTransport(false) if err != nil { return nil, "", "", err } diff --git a/pkg/controller/sts_handlers.go b/pkg/controller/sts_handlers.go index 3a0c6d05087..5ade8f867bf 100644 --- a/pkg/controller/sts_handlers.go +++ b/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()) + adminClient, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport(false)) if err != nil { writeSTSErrorResponse(w, true, ErrSTSInternalError, fmt.Errorf("Error communicating with tenant '%s': %s", tenant.Name, err)) return From a850f072ef043e2d80df38a751017ef6c56cbade Mon Sep 17 00:00:00 2001 From: pjuarezd Date: Fri, 11 Aug 2023 15:25:50 -0600 Subject: [PATCH 2/3] refactor for suggestions Signed-off-by: pjuarezd --- pkg/common/const.go | 9 ++++ pkg/controller/main-controller.go | 6 +-- pkg/controller/minio.go | 38 ++++++++-------- pkg/controller/monitoring.go | 4 +- pkg/controller/operator.go | 75 ++++++++++++++++++------------- pkg/controller/pools.go | 2 +- pkg/controller/sts.go | 2 +- pkg/controller/sts_handlers.go | 2 +- 8 files changed, 81 insertions(+), 57 deletions(-) diff --git a/pkg/common/const.go b/pkg/common/const.go index 5ec6eda675d..1a94e9c42b0 100644 --- a/pkg/common/const.go +++ b/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 diff --git a/pkg/controller/main-controller.go b/pkg/controller/main-controller.go index 373775a41c3..76220966f0d 100644 --- a/pkg/controller/main-controller.go +++ b/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) } diff --git a/pkg/controller/minio.go b/pkg/controller/minio.go index 5bc9308a23d..8ac9dbd364c 100644 --- a/pkg/controller/minio.go +++ b/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) } diff --git a/pkg/controller/monitoring.go b/pkg/controller/monitoring.go index ae4acd56518..439696361a7 100644 --- a/pkg/controller/monitoring.go +++ b/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() diff --git a/pkg/controller/operator.go b/pkg/controller/operator.go index 771de40e6a5..c472efa41ad 100644 --- a/pkg/controller/operator.go +++ b/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) diff --git a/pkg/controller/pools.go b/pkg/controller/pools.go index f051f136c0d..4d8f643daa7 100644 --- a/pkg/controller/pools.go +++ b/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 } diff --git a/pkg/controller/sts.go b/pkg/controller/sts.go index 46dc96520cb..920d482c7de 100644 --- a/pkg/controller/sts.go +++ b/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 } diff --git a/pkg/controller/sts_handlers.go b/pkg/controller/sts_handlers.go index 5ade8f867bf..3a0c6d05087 100644 --- a/pkg/controller/sts_handlers.go +++ b/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 From 1aacfab20ca5f09ecb906590c814fd5421c243ee Mon Sep 17 00:00:00 2001 From: pjuarezd Date: Fri, 11 Aug 2023 15:43:08 -0600 Subject: [PATCH 3/3] no need to update several times the secret Signed-off-by: pjuarezd --- pkg/controller/minio.go | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/pkg/controller/minio.go b/pkg/controller/minio.go index 8ac9dbd364c..74b9e0b21f8 100644 --- a/pkg/controller/minio.go +++ b/pkg/controller/minio.go @@ -195,40 +195,31 @@ func (c *Controller) checkOperatorCaForTenant(ctx context.Context, tenant *minio } } - publicCert, ok := tenantCaSecret.Data[common.PublicCRT] - if ok && !bytes.Equal(publicCert, operatorPublicCert) { + update := false + + if publicCert, ok := tenantCaSecret.Data[common.PublicCRT]; ok && !bytes.Equal(publicCert, 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.createTransport() - return false, fmt.Errorf("'public.crt' in '%s/%s' secret changed, updating '%s/%s' secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName, tenant.Namespace, OperatorCATLSSecretName) + update = true } - tlsCert, ok := tenantCaSecret.Data[common.TLSCRT] - if ok && !bytes.Equal(tlsCert, operatorTLSCert) { + if tlsCert, ok := tenantCaSecret.Data[common.TLSCRT]; ok && !bytes.Equal(tlsCert, operatorTLSCert) { 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.createTransport() - return false, fmt.Errorf("'tls.crt' in '%s/%s' secret changed, updating '%s/%s' secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName, tenant.Namespace, OperatorCATLSSecretName) + update = true } - caCert, ok := tenantCaSecret.Data[common.CACRT] - if ok && !bytes.Equal(caCert, operatorCACert) { + if caCert, ok := tenantCaSecret.Data[common.CACRT]; ok && !bytes.Equal(caCert, operatorCACert) { tenantCaSecret.Data[common.CACRT] = caCert + update = true + } + + if update { _, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Update(ctx, tenantCaSecret, metav1.UpdateOptions{}) if err != nil { return false, err } // Reload certificates c.createTransport() - return false, fmt.Errorf("'ca.crt' in '%s/%s' secret changed, updating '%s/%s' secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName, tenant.Namespace, OperatorCATLSSecretName) + return false, fmt.Errorf("'%s/%s' secret changed, updating '%s/%s' secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName, tenant.Namespace, OperatorCATLSSecretName) } return true, nil