Skip to content

Commit

Permalink
Bugfix reload CA cert in secret:
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pjuarezd committed Aug 10, 2023
1 parent 02aa055 commit 735dbfa
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 24 deletions.
6 changes: 3 additions & 3 deletions pkg/controller/main-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down
64 changes: 52 additions & 12 deletions pkg/controller/minio.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"fmt"
"math/big"
"net"
"slices"
"time"

"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down
10 changes: 6 additions & 4 deletions pkg/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/pools.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/sts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/sts_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 735dbfa

Please sign in to comment.