From 4df07c1e1f3bf16fe851e5cdbced0134721e0774 Mon Sep 17 00:00:00 2001 From: Pedro Juarez Date: Mon, 27 May 2024 23:26:15 -0700 Subject: [PATCH] Reload certificates in `operator-ca-tls` secrets (#2133) * Listen for secret changes in the operator namespace and trust TLS certificates stored in secrets with the prefix "operator-ca-tls." * No longer copy the secret `operator-ca-tls` from the operator namespace to the tenants namespace: Since [PR #1847](https://github.com/minio/operator/pull/1847), the secret `operator-ca-tls` is no longer mounted in the tenant, so there is no need to keep a copy. * `queue.NewNamedRateLimitingQueue` is deprecated and has been replaced with the recommended `queue.NewRateLimitingQueueWithConfig`. * Remove the duplicated method `getTLSSecret` and invoke `getCertificateSecret` instead. * Rename [generateTLSCert](https://github.com/minio/operator/blob/1c2fa4f402cc2c91c9903e6da6e9a693c92b65e4/pkg/controller/tls.go#L108) to `generateTLSCertificateForService` for better understanding. * Remove duplicated constants for 'public.crt', 'tls.crt', and 'ca.crt' in the `github.com/minio/operator/pkg/common` namespace. * Replace hardcoded strings 'public.crt', 'tls.crt', and 'ca.crt' with constants in the `github.com/minio/operator/pkg/certs` namespace. Signed-off-by: pjuarezd --------- Signed-off-by: pjuarezd --- pkg/apis/minio.min.io/v2/helper.go | 10 +- pkg/certs/const.go | 3 + pkg/common/const.go | 9 - pkg/controller/console.go | 4 +- pkg/controller/controller.go | 13 +- pkg/controller/csr.go | 6 +- pkg/controller/custom.go | 10 +- pkg/controller/kes.go | 6 +- pkg/controller/main-controller.go | 100 +++++++---- pkg/controller/minio.go | 125 +------------ pkg/controller/monitoring.go | 12 +- pkg/controller/operator.go | 170 ++++++++++++++---- pkg/controller/sts.go | 2 +- pkg/controller/tenants.go | 46 +++-- pkg/controller/tls.go | 24 +-- pkg/resources/statefulsets/kes-statefulset.go | 10 +- .../statefulsets/minio-statefulset.go | 81 +++++---- 17 files changed, 322 insertions(+), 309 deletions(-) diff --git a/pkg/apis/minio.min.io/v2/helper.go b/pkg/apis/minio.min.io/v2/helper.go index 8e49c874bc9..5aee5092118 100644 --- a/pkg/apis/minio.min.io/v2/helper.go +++ b/pkg/apis/minio.min.io/v2/helper.go @@ -37,6 +37,8 @@ import ( "text/template" "time" + "github.com/minio/operator/pkg/certs" + "github.com/miekg/dns" appsv1 "k8s.io/api/apps/v1" @@ -102,7 +104,7 @@ var ( // GetPodCAFromFile assumes the operator is running inside a k8s pod and extract the // current ca certificate from /var/run/secrets/kubernetes.io/serviceaccount/ca.crt func GetPodCAFromFile() []byte { - cert, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/ca.crt") + cert, err := os.ReadFile(fmt.Sprintf("/var/run/secrets/kubernetes.io/serviceaccount/%s", certs.CAPublicCertFile)) if err != nil { return nil } @@ -120,7 +122,7 @@ func GetOpenshiftServiceCAFromFile() []byte { // GetOpenshiftCSRSignerCAFromFile extracts the tls.crt certificate in Openshift deployments coming from the mounted secret openshift-csr-signer-ca func GetOpenshiftCSRSignerCAFromFile() []byte { - cert, err := os.ReadFile("/tmp/csr-signer-ca/tls.crt") + cert, err := os.ReadFile(fmt.Sprintf("/tmp/csr-signer-ca/%s", certs.TLSCertFile)) if err != nil { return nil } @@ -129,13 +131,13 @@ func GetOpenshiftCSRSignerCAFromFile() []byte { // GetPublicCertFilePath return the path to the certificate file based for the serviceName func GetPublicCertFilePath(serviceName string) string { - publicCertPath := fmt.Sprintf("/tmp/%s/public.crt", serviceName) + publicCertPath := fmt.Sprintf("/tmp/%s/%s", serviceName, certs.PublicCertFile) return publicCertPath } // GetPrivateKeyFilePath return the path to the key file based for the serviceName func GetPrivateKeyFilePath(serviceName string) string { - privateKey := fmt.Sprintf("/tmp/%s/private.key", serviceName) + privateKey := fmt.Sprintf("/tmp/%s/%s", serviceName, certs.PrivateKeyFile) return privateKey } diff --git a/pkg/certs/const.go b/pkg/certs/const.go index 9e116b93ad9..8ee995ad464 100644 --- a/pkg/certs/const.go +++ b/pkg/certs/const.go @@ -35,6 +35,9 @@ const ( // PrivateKeyFile Private key file for HTTPS. PrivateKeyFile = "private.key" + // CAPublicCertFile Public certificate file for Certificate authority. + CAPublicCertFile = "ca.crt" + // TLSKeyFile Private key file for HTTPS. TLSKeyFile = "tls.key" ) diff --git a/pkg/common/const.go b/pkg/common/const.go index 1a94e9c42b0..5ec6eda675d 100644 --- a/pkg/common/const.go +++ b/pkg/common/const.go @@ -36,15 +36,6 @@ 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/console.go b/pkg/controller/console.go index 8c1b4d99621..e54bdbf5eb5 100644 --- a/pkg/controller/console.go +++ b/pkg/controller/console.go @@ -100,12 +100,12 @@ func (c *Controller) checkConsoleSvc(ctx context.Context, tenant *miniov2.Tenant // generateConsoleTLSCert Issues the Operator Console TLS Certificate func (c *Controller) generateConsoleTLSCert() (*string, *string) { - return c.generateTLSCert("console", OperatorConsoleTLSSecretName, getConsoleDeploymentName()) + return c.generateTLSCertificateForService("console", OperatorConsoleTLSSecretName, getConsoleDeploymentName()) } func (c *Controller) recreateOperatorConsoleCertsIfRequired(ctx context.Context) error { namespace := miniov2.GetNSFromFile() - operatorConsoleTLSSecret, err := c.getTLSSecret(ctx, namespace, OperatorConsoleTLSSecretName) + operatorConsoleTLSSecret, err := c.getCertificateSecret(ctx, namespace, OperatorConsoleTLSSecretName) if err != nil { if k8serrors.IsNotFound(err) { klog.V(2).Info("TLS certificate not found. Generating one.") diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 2db7d2cf1a6..2bfc6542533 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -148,6 +148,7 @@ func StartOperator(kubeconfig string) { klog.Infof("Watching only namespaces: %s", strings.Join(namespaces.ToSlice(), ",")) } + kubeInformerFactoryInOperatorNamespace := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, time.Hour*1, kubeinformers.WithNamespace(v2.GetNSFromFile())) kubeInformerFactory := kubeinformers.NewSharedInformerFactory(kubeClient, time.Second*30) minioInformerFactory := informers.NewSharedInformerFactory(controllerClient, time.Second*30) podName := os.Getenv(HostnameEnv) @@ -163,20 +164,18 @@ func StartOperator(kubeconfig string) { k8sClient, controllerClient, promClient, - kubeInformerFactory.Apps().V1().StatefulSets(), - kubeInformerFactory.Apps().V1().Deployments(), - kubeInformerFactory.Core().V1().Pods(), - minioInformerFactory.Minio().V2().Tenants(), - minioInformerFactory.Sts().V1beta1().PolicyBindings(), - kubeInformerFactory.Core().V1().Services(), hostsTemplate, pkg.Version, + kubeInformerFactory, + minioInformerFactory.Minio().V2().Tenants(), + minioInformerFactory.Sts().V1beta1().PolicyBindings(), minioInformerFactory.Job().V1alpha1().MinIOJobs(), - kubeInformerFactory.Batch().V1().Jobs(), + kubeInformerFactoryInOperatorNamespace, ) go kubeInformerFactory.Start(stopCh) go minioInformerFactory.Start(stopCh) + go kubeInformerFactoryInOperatorNamespace.Start(stopCh) if err = mainController.Start(2, stopCh); err != nil { klog.Fatalf("Error running mainController: %s", err.Error()) diff --git a/pkg/controller/csr.go b/pkg/controller/csr.go index 92c2776afc5..e627a187ebb 100644 --- a/pkg/controller/csr.go +++ b/pkg/controller/csr.go @@ -29,6 +29,8 @@ import ( "syscall" "time" + "github.com/minio/operator/pkg/certs" + "github.com/minio/operator/pkg/controller/certificates" certificatesV1 "k8s.io/api/certificates/v1" @@ -257,8 +259,8 @@ func (c *Controller) createSecret(ctx context.Context, tenant *miniov2.Tenant, l }, }, Data: map[string][]byte{ - "private.key": pkBytes, - "public.crt": certBytes, + certs.PrivateKeyFile: pkBytes, + certs.PublicCertFile: certBytes, }, } _, err := c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Create(ctx, secret, metav1.CreateOptions{}) diff --git a/pkg/controller/custom.go b/pkg/controller/custom.go index 579c3648cba..8b079039235 100644 --- a/pkg/controller/custom.go +++ b/pkg/controller/custom.go @@ -24,15 +24,17 @@ import ( "math" "time" + "github.com/minio/operator/pkg/certs" + miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var secretTypePublicKeyNameMap = map[string]string{ - "kubernetes.io/tls": "tls.crt", - "cert-manager.io/v1": "tls.crt", - "cert-manager.io/v1alpha2": "tls.crt", + "kubernetes.io/tls": certs.TLSCertFile, + "cert-manager.io/v1": certs.TLSCertFile, + "cert-manager.io/v1alpha2": certs.TLSCertFile, // Add newer secretTypes and their corresponding values in future } @@ -51,7 +53,7 @@ func (c *Controller) getCustomCertificates(ctx context.Context, tenant *miniov2. for certType, secrets := range secretsMap { certificates = nil - publicKey := "public.crt" + publicKey := certs.PublicCertFile // Iterate over TLS secrets and build array of CertificateInfo structure // that will be used to display information about certs for _, secret := range secrets { diff --git a/pkg/controller/kes.go b/pkg/controller/kes.go index 52d312d8074..85a37585bb0 100644 --- a/pkg/controller/kes.go +++ b/pkg/controller/kes.go @@ -26,6 +26,8 @@ import ( "errors" "fmt" + "github.com/minio/operator/pkg/certs" + "github.com/minio/operator/pkg/controller/certificates" corev1 "k8s.io/api/core/v1" @@ -326,9 +328,9 @@ func (c *Controller) getCertIdentity(ns string, cert *miniov2.LocalCertificateRe } // Store the Identity to be used later during KES container creation if secret.Type == "kubernetes.io/tls" || secret.Type == "cert-manager.io/v1alpha2" || secret.Type == "cert-manager.io/v1" { - certbytes = secret.Data["tls.crt"] + certbytes = secret.Data[certs.TLSCertFile] } else { - certbytes = secret.Data["public.crt"] + certbytes = secret.Data[certs.PublicCertFile] } // parse the certificate here to generate the identity for this certifcate. diff --git a/pkg/controller/main-controller.go b/pkg/controller/main-controller.go index 3e535494209..afac5743e72 100644 --- a/pkg/controller/main-controller.go +++ b/pkg/controller/main-controller.go @@ -26,6 +26,8 @@ import ( "syscall" "time" + kubeinformers "k8s.io/client-go/informers" + "github.com/minio/operator/pkg/utils" "github.com/minio/madmin-go/v3" @@ -54,9 +56,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" - appsinformers "k8s.io/client-go/informers/apps/v1" - batchv1 "k8s.io/client-go/informers/batch/v1" - coreinformers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -138,25 +137,25 @@ type Controller struct { // statefulSetListerSynced returns true if the StatefulSet shared informer // has synced at least once. statefulSetListerSynced cache.InformerSynced - + // secretLister returns list/get secrets from a shared informer + secretLister corelisters.SecretLister + // secretListerSynced returns true if Secret shared informer has synced at least once + secretListerSynced cache.InformerSynced // deploymentLister is able to list/get Deployments from a shared // informer's store. deploymentLister appslisters.DeploymentLister // deploymentListerSynced returns true if the Deployment shared informer // has synced at least once. deploymentListerSynced cache.InformerSynced - // tenantsSynced returns true if the StatefulSet shared informer // has synced at least once. tenantsSynced cache.InformerSynced - // serviceLister is able to list/get Services from a shared informer's // store. serviceLister corelisters.ServiceLister // serviceListerSynced returns true if the Service shared informer // has synced at least once. serviceListerSynced cache.InformerSynced - // queue is a rate limited work queue. This is used to queue work to be // processed instead of performing it as soon as a change happens. This // means we can ensure we only process a fixed amount of resources at a @@ -227,17 +226,21 @@ func NewController( k8sClient client.Client, minioClientSet clientset.Interface, promClient promclientset.Interface, - statefulSetInformer appsinformers.StatefulSetInformer, - deploymentInformer appsinformers.DeploymentInformer, - podInformer coreinformers.PodInformer, - tenantInformer informers.TenantInformer, - policyBindingInformer stsInformers.PolicyBindingInformer, - serviceInformer coreinformers.ServiceInformer, hostsTemplate, operatorVersion string, - minioJobinformer jobinformers.MinIOJobInformer, - jobInformer batchv1.JobInformer, + kubeInformerFactory kubeinformers.SharedInformerFactory, + tenantInformer informers.TenantInformer, + policyBindingInformer stsInformers.PolicyBindingInformer, + minioJobInformer jobinformers.MinIOJobInformer, + kubeInformerFactoryInOperatorNamespace kubeinformers.SharedInformerFactory, ) *Controller { + statefulSetInformer := kubeInformerFactory.Apps().V1().StatefulSets() + deploymentInformer := kubeInformerFactory.Apps().V1().Deployments() + podInformer := kubeInformerFactory.Core().V1().Pods() + serviceInformer := kubeInformerFactory.Core().V1().Services() + jobInformer := kubeInformerFactory.Batch().V1().Jobs() + secretInformer := kubeInformerFactoryInOperatorNamespace.Core().V1().Secrets() + // Create event broadcaster // Add minio-controller types to the default Kubernetes Scheme so Events can be // logged for minio-controller types. @@ -263,20 +266,22 @@ func NewController( tenantsSynced: tenantInformer.Informer().HasSynced, serviceLister: serviceInformer.Lister(), serviceListerSynced: serviceInformer.Informer().HasSynced, - workqueue: queue.NewNamedRateLimitingQueue(MinIOControllerRateLimiter(), "Tenants"), - healthCheckQueue: queue.NewNamedRateLimitingQueue(MinIOControllerRateLimiter(), "TenantsHealth"), + secretLister: secretInformer.Lister(), + secretListerSynced: secretInformer.Informer().HasSynced, + workqueue: queue.NewRateLimitingQueueWithConfig(MinIOControllerRateLimiter(), queue.RateLimitingQueueConfig{Name: "Tenants"}), + healthCheckQueue: queue.NewRateLimitingQueueWithConfig(MinIOControllerRateLimiter(), queue.RateLimitingQueueConfig{Name: "TenantsHealth"}), recorder: recorder, hostsTemplate: hostsTemplate, operatorVersion: operatorVersion, policyBindingListerSynced: policyBindingInformer.Informer().HasSynced, controllers: []*JobController{ NewJobController( - minioJobinformer, + minioJobInformer, jobInformer, namespacesToWatch, kubeClientSet, recorder, - queue.NewNamedRateLimitingQueue(MinIOControllerRateLimiter(), "MinioJobs"), + queue.NewRateLimitingQueueWithConfig(MinIOControllerRateLimiter(), queue.RateLimitingQueueConfig{Name: "MinioJobs"}), k8sClient, ), }, @@ -347,7 +352,7 @@ func NewController( oldDepl := old.(*corev1.Pod) if newDepl.ResourceVersion == oldDepl.ResourceVersion { // Periodic resync will send update events for all known Deployments. - // Two different versions of the same Deployments will always have different RVs. + // Two different versions of the same Pods will always have different RVs. return } controller.handlePodChange(new) @@ -355,6 +360,22 @@ func NewController( DeleteFunc: controller.handlePodChange, }) + secretInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + controller.handleSecret(obj, nil) + }, + UpdateFunc: func(old, new interface{}) { + newSecret := new.(*corev1.Secret) + oldSecret := old.(*corev1.Secret) + if newSecret.ResourceVersion == oldSecret.ResourceVersion { + // Periodic resync will send update events for all known Deployments. + // Two different versions of the same secret will always have different RVs. + return + } + controller.handleSecret(new, old) + }, + }) + return controller } @@ -415,7 +436,7 @@ func leaderRun(ctx context.Context, c *Controller, threadiness int, stopCh <-cha // Wait for the caches to be synced before starting workers klog.Info("Waiting for informer caches to sync") - if ok := cache.WaitForCacheSync(stopCh, c.statefulSetListerSynced, c.deploymentListerSynced, c.tenantsSynced, c.policyBindingListerSynced); !ok { + if ok := cache.WaitForCacheSync(stopCh, c.statefulSetListerSynced, c.deploymentListerSynced, c.tenantsSynced, c.policyBindingListerSynced, c.secretListerSynced); !ok { panic("failed to wait for caches to sync") } // Wait for the caches to be synced before starting workers @@ -896,13 +917,6 @@ func (c *Controller) syncHandler(key string) (Result, error) { return WrapResult(Result{}, err) } - // check if operator-ca-tls has to be updated or re-created in the tenant namespace - operatorCATLSExists, err := c.checkOperatorCAForTenant(ctx, tenant) - if err != nil { - // Don't return here as we get stuck when recreating the stateful set - klog.Infof("There was an error while updating the certificate %s", err) - } - // consolidate the status of all pools. this is meant to cover for legacy tenants // this status value is zero only for new tenants or legacy tenants if len(tenant.Status.Pools) == 0 { @@ -974,7 +988,6 @@ func (c *Controller) syncHandler(key string) (Result, error) { ServiceName: tenant.MinIOHLServiceName(), HostsTemplate: c.hostsTemplate, OperatorVersion: c.operatorVersion, - OperatorCATLS: operatorCATLSExists, }) ss, err = c.kubeClientSet.AppsV1().StatefulSets(tenant.Namespace).Create(ctx, ss, cOpts) if err != nil { @@ -1183,7 +1196,6 @@ func (c *Controller) syncHandler(key string) (Result, error) { ServiceName: tenant.MinIOHLServiceName(), HostsTemplate: c.hostsTemplate, OperatorVersion: c.operatorVersion, - OperatorCATLS: operatorCATLSExists, }) if _, err = c.kubeClientSet.AppsV1().StatefulSets(tenant.Namespace).Update(ctx, ss, uOpts); err != nil { return WrapResult(Result{}, err) @@ -1232,7 +1244,6 @@ func (c *Controller) syncHandler(key string) (Result, error) { ServiceName: tenant.MinIOHLServiceName(), HostsTemplate: c.hostsTemplate, OperatorVersion: c.operatorVersion, - OperatorCATLS: operatorCATLSExists, }) // Verify if this pool matches the spec on the tenant (resources, affinity, sidecars, etc) poolMatchesSS, err := poolSSMatchesSpec(expectedStatefulSet, existingStatefulSet) @@ -1390,6 +1401,33 @@ func (c *Controller) handleObject(obj interface{}) { } } +func (c *Controller) handleSecret(obj interface{}, oldObj interface{}) { + ns := miniov2.GetNSFromFile() + var secret *corev1.Secret + var ok bool + if secret, ok = obj.(*corev1.Secret); !ok { + runtime.HandleError(fmt.Errorf("error decoding object, invalid type")) + return + } + // Observe secrets in the Operator namespace + if secret.Namespace == ns { + // a secret with prefix "operator-ca-tls" changed, reload all trusted CA certificates + if strings.HasPrefix(secret.Name, OperatorCATLSSecretName) { + klog.Infof("Secret '%s/%s' changed", secret.Namespace, secret.Name) + var oldSecret *corev1.Secret + if oldObj != nil { + if oldCasted, ok := oldObj.(*corev1.Secret); ok { + oldSecret = oldCasted + } + } + // Add new certificates to Transport Certs if any changed + if !c.TrustTLSCertificatesInSecretIfChanged(secret, oldSecret) { + klog.Infof("No new certificate was added from secret '%s/%s'", secret.Name, secret.Name) + } + } + } +} + // MinIOControllerRateLimiter is a no-arg constructor for a default rate limiter for a workqueue for our controller. // both overall and per-item rate limiting. The overall is a token bucket and the per-item is exponential func MinIOControllerRateLimiter() queue.RateLimiter { diff --git a/pkg/controller/minio.go b/pkg/controller/minio.go index 6f182e96d7c..a06b60a5c53 100644 --- a/pkg/controller/minio.go +++ b/pkg/controller/minio.go @@ -15,7 +15,6 @@ package controller import ( - "bytes" "context" "crypto/ed25519" "crypto/rand" @@ -27,13 +26,8 @@ import ( "fmt" "math/big" "net" - "slices" "time" - "github.com/minio/operator/pkg/common" - - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/minio/operator/pkg/controller/certificates" corev1 "k8s.io/api/core/v1" @@ -109,128 +103,11 @@ func (c *Controller) recreateMinIOCertsOnTenant(ctx context.Context, tenant *min return c.checkAndCreateMinIOCSR(ctx, nsName, tenant) } -func (c *Controller) getTLSSecret(ctx context.Context, nsName string, secretName string) (*corev1.Secret, error) { - return c.kubeClientSet.CoreV1().Secrets(nsName).Get(ctx, secretName, metav1.GetOptions{}) -} - -func getOperatorCertFromSecret(secretData map[string][]byte, key string) ([]byte, error) { - keys := []string{ - common.TLSCRT, - common.CACRT, - common.PublicCRT, - } - if slices.Contains(keys, key) { - data, ok := secretData[key] - if ok { - return data, nil - } - } - 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) { - certsData := make(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 { - // if operator-ca-tls doesnt exists continue - if k8serrors.IsNotFound(err) { - return false, nil - } - return false, err - } - - operatorPublicCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, common.PublicCRT) - if err == nil { - certsData[common.PublicCRT] = operatorPublicCert - } - - operatorTLSCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, common.TLSCRT) - if err == nil { - certsData[common.TLSCRT] = operatorTLSCert - } - - operatorCACert, err := getOperatorCertFromSecret(operatorCaSecret.Data, common.CACRT) - if err == nil { - certsData[common.CACRT] = operatorCACert - } - - if len(certsData) == 0 { - return false, fmt.Errorf("'%s' secret exists but is missing public.crt, tls.crt and ca.crt, please fix it manually", OperatorCATLSSecretName) - } - - var tenantCaSecret *corev1.Secret - - createTenantCASecret := func() error { - // create tenant operator-ca-tls secret - tenantCaSecret = &corev1.Secret{ - Type: "Opaque", - ObjectMeta: metav1.ObjectMeta{ - Name: OperatorCATLSSecretName, - Namespace: tenant.Namespace, - Labels: tenant.MinIOPodLabels(), - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(tenant, schema.GroupVersionKind{ - Group: miniov2.SchemeGroupVersion.Group, - Version: miniov2.SchemeGroupVersion.Version, - Kind: miniov2.MinIOCRDResourceKind, - }), - }, - }, - Data: certsData, - } - _, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Create(ctx, tenantCaSecret, metav1.CreateOptions{}) - return err - } - - tenantCaSecret, err = c.kubeClientSet.CoreV1().Secrets(tenant.Namespace).Get(ctx, OperatorCATLSSecretName, metav1.GetOptions{}) - if err != nil { - if !k8serrors.IsNotFound(err) { - return false, err - } - klog.Infof("'%s/%s' secret not found, creating one now", tenant.Namespace, OperatorCATLSSecretName) - if err = createTenantCASecret(); err != nil { - return false, err - } - } - - update := false - - if publicCert, ok := tenantCaSecret.Data[common.PublicCRT]; ok && !bytes.Equal(publicCert, operatorPublicCert) { - tenantCaSecret.Data[common.PublicCRT] = operatorPublicCert - update = true - } - - if tlsCert, ok := tenantCaSecret.Data[common.TLSCRT]; ok && !bytes.Equal(tlsCert, operatorTLSCert) { - tenantCaSecret.Data[common.TLSCRT] = tlsCert - update = true - } - - 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("'%s/%s' secret changed, updating '%s/%s' secret", miniov2.GetNSFromFile(), OperatorCATLSSecretName, tenant.Namespace, OperatorCATLSSecretName) - } - - return true, nil -} - // checkMinIOCertificatesStatus checks for the current status of MinIO and it's service func (c *Controller) checkMinIOCertificatesStatus(ctx context.Context, tenant *miniov2.Tenant, nsName types.NamespacedName) error { if tenant.AutoCert() { // check if there's already a TLS secret for MinIO - tlsSecret, err := c.getTLSSecret(ctx, tenant.Namespace, tenant.MinIOTLSSecretName()) + tlsSecret, err := c.getCertificateSecret(ctx, tenant.Namespace, tenant.MinIOTLSSecretName()) if err != nil { if k8serrors.IsNotFound(err) { if err := c.checkAndCreateMinIOCSR(ctx, nsName, tenant); err != nil { diff --git a/pkg/controller/monitoring.go b/pkg/controller/monitoring.go index cd3360d73d6..f404ab237b9 100644 --- a/pkg/controller/monitoring.go +++ b/pkg/controller/monitoring.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "log" + "strings" "time" "github.com/minio/madmin-go/v3" @@ -118,13 +119,14 @@ func (c *Controller) updateHealthStatusForTenant(tenant *miniov2.Tenant) error { // get cluster health for tenant healthResult, err := aClnt.Healthy(hctx, madmin.HealthOpts{}) if err != nil { + if strings.Contains(err.Error(), "failed to verify certificate") { + err := c.reloadTenantExternalCerts(tenant) + if err != nil { + return err + } + } // show the error and continue klog.Infof("'%s/%s' Failed to get cluster health: %v", tenant.Namespace, tenant.Name, err) - err = c.renewExternalCerts(context.Background(), tenant, err) - if err != nil { - klog.Errorf("There was an error on certificate renewal %s", err) - return err - } return nil } diff --git a/pkg/controller/operator.go b/pkg/controller/operator.go index a8f19b95eb4..cb5cd042e08 100644 --- a/pkg/controller/operator.go +++ b/pkg/controller/operator.go @@ -21,25 +21,29 @@ import ( "context" "crypto/tls" "crypto/x509" + "encoding/pem" "fmt" "net" "net/http" + "slices" "strings" "time" + "github.com/minio/operator/pkg/certs" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" "github.com/minio/operator/pkg/common" "github.com/minio/operator/pkg/utils" + xcerts "github.com/minio/pkg/certs" + "github.com/minio/pkg/env" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - - miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" - xcerts "github.com/minio/pkg/certs" - "github.com/minio/pkg/env" "k8s.io/klog/v2" k8sscheme "k8s.io/kubectl/pkg/scheme" ) @@ -51,6 +55,8 @@ const ( OperatorDeploymentNameEnv = "MINIO_OPERATOR_DEPLOYMENT_NAME" // OperatorCATLSSecretName is the name of the secret for the operator CA OperatorCATLSSecretName = "operator-ca-tls" + // OperatorCATLSSecretPrefix is the name of the multi tenant secret for the operator CA + OperatorCATLSSecretPrefix = OperatorCATLSSecretName + "-" // OperatorCSRSignerCASecretName is the name of the secret for the signer-ca certificate // this is a copy of the secret signer-ca in namespace OperatorCSRSignerCASecretName = "openshift-csr-signer-ca" @@ -160,38 +166,22 @@ func (c *Controller) fetchTransportCACertificates() (pool *x509.CertPool) { } } - // Custom ca certificate to be used by operator - operatorCATLSCert, err := c.kubeClientSet.CoreV1().Secrets(miniov2.GetNSFromFile()).Get(context.Background(), OperatorCATLSSecretName, metav1.GetOptions{}) - if err == nil && operatorCATLSCert != nil { - if val, ok := operatorCATLSCert.Data["tls.crt"]; ok { - rootCAs.AppendCertsFromPEM(val) - } - if val, ok := operatorCATLSCert.Data["ca.crt"]; ok { - rootCAs.AppendCertsFromPEM(val) - } - if val, ok := operatorCATLSCert.Data["public.crt"]; ok { - rootCAs.AppendCertsFromPEM(val) - } - } - - // Multi-tenancy support for external certificates - // One secret per tenant to allow for the automatic appending and renewal of certificates upon expiration. + // Append all external Certificate Authorities added to Operator, secrets with prefix "operator-ca-tls" secretsAvailableAtOperatorNS, _ := c.kubeClientSet.CoreV1().Secrets(miniov2.GetNSFromFile()).List(context.Background(), metav1.ListOptions{}) for _, secret := range secretsAvailableAtOperatorNS.Items { // Check if secret starts with "operator-ca-tls-" - secretName := OperatorCATLSSecretName + "-" - if strings.HasPrefix(secret.Name, secretName) { - klog.Infof("External secret found: %s", secret.Name) + if strings.HasPrefix(secret.Name, OperatorCATLSSecretName) { operatorCATLSCert, err := c.kubeClientSet.CoreV1().Secrets(miniov2.GetNSFromFile()).Get(context.Background(), secret.Name, metav1.GetOptions{}) if err == nil && operatorCATLSCert != nil { - if val, ok := operatorCATLSCert.Data["ca.crt"]; ok { - klog.Infof("Appending cert from %s secret", secret.Name) - rootCAs.AppendCertsFromPEM(val) - } else { - klog.Errorf("NOT appending %s secret, ok: %t", secret.Name, ok) + if newPublicCert, err := getFileFromSecretDataField(operatorCATLSCert.Data, certs.PublicCertFile); err == nil { + rootCAs.AppendCertsFromPEM(newPublicCert) + } + if newTLSCert, err := getFileFromSecretDataField(operatorCATLSCert.Data, certs.TLSCertFile); err == nil { + rootCAs.AppendCertsFromPEM(newTLSCert) + } + if newCACert, err := getFileFromSecretDataField(operatorCATLSCert.Data, certs.CAPublicCertFile); err == nil { + rootCAs.AppendCertsFromPEM(newCACert) } - } else { - klog.Errorf("NOT appending %s secret, err: %s operatorCATLSCert: %s", secret.Name, err, operatorCATLSCert) } } } @@ -199,6 +189,115 @@ func (c *Controller) fetchTransportCACertificates() (pool *x509.CertPool) { return rootCAs } +// getFileFromSecretDataField Get the value of a secret field +// limiting the field key name to public TLS certificate file names +func getFileFromSecretDataField(secretData map[string][]byte, key string) ([]byte, error) { + keys := []string{ + certs.TLSCertFile, + certs.CAPublicCertFile, + certs.PublicCertFile, + } + if slices.Contains(keys, key) { + data, ok := secretData[key] + if ok { + return data, nil + } + } else { + return nil, fmt.Errorf("unknow TLS key '%s'", key) + } + return nil, fmt.Errorf("key '%s' not found in secret", key) +} + +// TrustTLSCertificatesInSecretIfChanged Compares old and new secret content and trusts TLS certificates if field +// content is different, looks for the fields public.crt, tls.crt and ca.crt +func (c *Controller) TrustTLSCertificatesInSecretIfChanged(newSecret *corev1.Secret, oldSecret *corev1.Secret) bool { + added := false + if oldSecret == nil { + // secret did not exist before, we trust all certs in it + if c.trustPEMInSecretField(newSecret, certs.PublicCertFile) { + added = true + } + if c.trustPEMInSecretField(newSecret, certs.TLSCertFile) { + added = true + } + if c.trustPEMInSecretField(newSecret, certs.CAPublicCertFile) { + added = true + } + } else { + // compare to add to trust only certs that changed + if c.trustIfChanged(newSecret, oldSecret, certs.PublicCertFile) { + added = true + } + if c.trustIfChanged(newSecret, oldSecret, certs.TLSCertFile) { + added = true + } + if c.trustIfChanged(newSecret, oldSecret, certs.CAPublicCertFile) { + added = true + } + } + return added +} + +func (c *Controller) trustIfChanged(newSecret *corev1.Secret, oldSecret *corev1.Secret, fieldToCompare string) bool { + if newPublicCert, err := getFileFromSecretDataField(newSecret.Data, fieldToCompare); err == nil { + if oldPublicCert, err := getFileFromSecretDataField(oldSecret.Data, fieldToCompare); err == nil { + newPublicCert = bytes.TrimSpace(newPublicCert) + oldPublicCert = bytes.TrimSpace(oldPublicCert) + // add to trust only if cert changed + if !bytes.Equal(oldPublicCert, newPublicCert) { + if err := c.addTLSCertificatesToTrustInTransport(newPublicCert); err == nil { + klog.Infof("Added certificates in field '%s' of '%s/%s' secret to trusted RootCA's", fieldToCompare, newSecret.Namespace, newSecret.Name) + return true + } + klog.Errorf("Failed adding certs in field '%s' of '%s/%s' secret: %v", fieldToCompare, newSecret.Namespace, newSecret.Name, err) + } + } else { + // If field was not present in old secret but is in new secret then is an addition, we trust it + if err := c.addTLSCertificatesToTrustInTransport(newPublicCert); err == nil { + klog.Infof("Added certificates in field '%s' of '%s/%s' secret to trusted RootCA's", fieldToCompare, newSecret.Namespace, newSecret.Name) + return true + } + klog.Errorf("Failed adding certificates in field %s of '%s/%s' secret: %v", fieldToCompare, newSecret.Namespace, newSecret.Name, err) + } + } + return false +} + +func (c *Controller) trustPEMInSecretField(secret *corev1.Secret, fieldToCompare string) bool { + newPublicCert, err := getFileFromSecretDataField(secret.Data, fieldToCompare) + if err == nil { + if err := c.addTLSCertificatesToTrustInTransport(newPublicCert); err == nil { + klog.Infof("Added certificates in field '%s' of '%s/%s' secret to trusted RootCA's", fieldToCompare, secret.Namespace, secret.Name) + return true + } + klog.Errorf("Failed adding certificates in field '%s' of '%s/%s' secret: %v", fieldToCompare, secret.Namespace, secret.Name, err) + } + return false +} + +func (c *Controller) addTLSCertificatesToTrustInTransport(certificateData []byte) error { + var x509Certs []*x509.Certificate + current := certificateData + // A single PEM file could contain more than one certificates, keeping track of the index to help debugging + certIndex := 1 + for len(current) > 0 { + var pemBlock *pem.Block + if pemBlock, current = pem.Decode(current); pemBlock == nil { + return fmt.Errorf("invalid PEM in file in index %d", certIndex) + } + x509Cert, err := x509.ParseCertificate(pemBlock.Bytes) + if err != nil { + return fmt.Errorf("error parsing x509 certificate from PEM in index, %d: %v", certIndex, err) + } + x509Certs = append(x509Certs, x509Cert) + certIndex++ + } + for _, cert := range x509Certs { + c.getTransport().TLSClientConfig.RootCAs.AddCert(cert) + } + return nil +} + // GetSignerCAFromSecret Retrieves the CA certificate for Openshift CSR signed certificates from // openshift-kube-controller-manager-operator namespace func (c *Controller) GetSignerCAFromSecret() ([]byte, error) { @@ -211,7 +310,7 @@ func (c *Controller) GetSignerCAFromSecret() ([]byte, error) { } return nil, fmt.Errorf("%s secret was found but we failed to load the secret: %#v", OpenshiftCATLSSecretName, err) } else if openShiftCATLSCertSecret != nil { - if val, ok := openShiftCATLSCertSecret.Data[common.TLSCRT]; ok { + if val, ok := openShiftCATLSCertSecret.Data[certs.TLSCertFile]; ok { caCertificate = val } } @@ -232,7 +331,7 @@ func (c *Controller) GetOpenshiftCSRSignerCAFromSecret() ([]byte, error) { return nil, fmt.Errorf("%s secret was found but we failed to get certificate: %#v", OperatorCSRSignerCASecretName, err) } else if openShifCSRSignerCATLSCertSecret != nil { // When secret was obtained with no errors - if val, ok := openShifCSRSignerCATLSCertSecret.Data[common.TLSCRT]; ok { + if val, ok := openShifCSRSignerCATLSCertSecret.Data[certs.TLSCertFile]; ok { // OpenShift csr-signer secret has tls.crt certificates that we need to append in order // to trust the signer. If we append the val, Operator will be able to provisioning the // initial users and get Tenant Health, so tenant can be properly initialized and in @@ -281,7 +380,7 @@ func (c *Controller) checkOpenshiftSignerCACertInOperatorNamespace(ctx context.C OwnerReferences: []metav1.OwnerReference{ownerReference}, }, Data: map[string][]byte{ - common.TLSCRT: csrSignerCertificate, + certs.TLSCertFile: csrSignerCertificate, }, } _, err = c.kubeClientSet.CoreV1().Secrets(namespace).Create(ctx, csrSignerSecret, metav1.CreateOptions{}) @@ -292,14 +391,13 @@ func (c *Controller) checkOpenshiftSignerCACertInOperatorNamespace(ctx context.C return err } - if caCert, ok := csrSignerSecret.Data[common.TLSCRT]; ok && !bytes.Equal(caCert, csrSignerCertificate) { - csrSignerSecret.Data[common.TLSCRT] = csrSignerCertificate + if caCert, ok := csrSignerSecret.Data[certs.TLSCertFile]; ok && !bytes.Equal(caCert, csrSignerCertificate) { + csrSignerSecret.Data[certs.TLSCertFile] = csrSignerCertificate _, err = c.kubeClientSet.CoreV1().Secrets(namespace).Update(ctx, csrSignerSecret, metav1.UpdateOptions{}) if err != nil { return err } klog.Infof("'%s/%s' secret changed, updating '%s/%s' secret", OpenshiftKubeControllerNamespace, OpenshiftCATLSSecretName, namespace, OperatorCSRSignerCASecretName) - c.fetchTransportCACertificates() // Reload CA certificates c.createTransport() } diff --git a/pkg/controller/sts.go b/pkg/controller/sts.go index 5bf3beab17e..1c9f1ac4472 100644 --- a/pkg/controller/sts.go +++ b/pkg/controller/sts.go @@ -400,7 +400,7 @@ func IsSTSEnabled() bool { // generateConsoleTLSCert Issues the Operator Console TLS Certificate func (c *Controller) generateSTSTLSCert() (*string, *string) { - return c.generateTLSCert("sts", STSTLSSecretName, getOperatorDeploymentName()) + return c.generateTLSCertificateForService("sts", STSTLSSecretName, getOperatorDeploymentName()) } // waitSTSTLSCert Waits for the Operator leader to issue the TLS Certificate for STS diff --git a/pkg/controller/tenants.go b/pkg/controller/tenants.go index d869a167bdc..b0b33a3a6f6 100644 --- a/pkg/controller/tenants.go +++ b/pkg/controller/tenants.go @@ -21,6 +21,8 @@ import ( "errors" "strings" + "github.com/minio/operator/pkg/certs" + corev1 "k8s.io/api/core/v1" "k8s.io/klog/v2" @@ -51,11 +53,8 @@ func (c *Controller) getTenantConfiguration(ctx context.Context, tenant *miniov2 // renewCert will renew one certificate at a time func (c *Controller) renewCert(secret corev1.Secret, index int, tenant *miniov2.Tenant) error { - // Check if secret starts with "operator-ca-tls-" - secretName := OperatorCATLSSecretName + "-" // If the secret does not start with "operator-ca-tls-" then no need to continue - if !strings.HasPrefix(secret.Name, secretName) { - klog.Info("No secret found for multi-tenancy architecture of external certificates") + if !strings.HasPrefix(secret.Name, OperatorCATLSSecretPrefix) { return nil } klog.Infof("%d external secret found: %s", index, secret.Name) @@ -71,7 +70,7 @@ func (c *Controller) renewCert(secret corev1.Secret, index int, tenant *miniov2. klog.Errorf("certificate's data can't be empty: %s", data) return errors.New("empty cert data") } - CACertificate := data.Data["ca.crt"] + CACertificate := data.Data[certs.CAPublicCertFile] if CACertificate == nil || len(CACertificate) <= 0 { klog.Errorf("ca.crt certificate data can't be empty: %s", CACertificate) return errors.New("empty cert ca data") @@ -91,7 +90,7 @@ func (c *Controller) renewCert(secret corev1.Secret, index int, tenant *miniov2. Namespace: miniov2.GetNSFromFile(), }, Data: map[string][]byte{ - "ca.crt": CACertificate, + certs.CAPublicCertFile: CACertificate, }, } _, err = c.kubeClientSet.CoreV1().Secrets(miniov2.GetNSFromFile()).Create(context.Background(), newSecret, metav1.CreateOptions{}) @@ -99,8 +98,6 @@ func (c *Controller) renewCert(secret corev1.Secret, index int, tenant *miniov2. klog.Errorf("Secret not created %s", err) return err } - // Append it - c.fetchTransportCACertificates() // Reload CA certificates c.createTransport() // Rollout the Operator Deployment to use new certificate and trust the tenant. @@ -119,25 +116,22 @@ func (c *Controller) renewCert(secret corev1.Secret, index int, tenant *miniov2. return nil } -// renewExternalCerts renews external certificates when they expire, ensuring that the Operator trusts its tenants. -func (c *Controller) renewExternalCerts(ctx context.Context, tenant *miniov2.Tenant, err error) error { - if strings.Contains(err.Error(), "failed to verify certificate") { - externalCertSecret := tenant.Spec.ExternalCertSecret - klog.Info("Let's check if there is an external cert for the tenant...") - if externalCertSecret != nil { - // Check that there is a secret that starts with "operator-ca-tls-" to proceed with the renewal - secretsAvailableAtOperatorNS, err := c.kubeClientSet.CoreV1().Secrets(miniov2.GetNSFromFile()).List(context.Background(), metav1.ListOptions{}) +// reloadTenantExternalCerts reloads Tenant external certificates +func (c *Controller) reloadTenantExternalCerts(tenant *miniov2.Tenant) error { + externalCertSecret := tenant.Spec.ExternalCertSecret + if externalCertSecret != nil { + // Check that there is a secret that starts with "operator-ca-tls-" to proceed with the renewal + secretsAvailableAtOperatorNS, err := c.kubeClientSet.CoreV1().Secrets(miniov2.GetNSFromFile()).List(context.Background(), metav1.ListOptions{}) + if err != nil { + klog.Info("No external certificates are found under the multi-tenancy architecture to handle.") + return nil + } + klog.Info("there are secret(s) for the operator") + for index, secret := range secretsAvailableAtOperatorNS.Items { + err = c.renewCert(secret, index, tenant) if err != nil { - klog.Info("No external certificates are found under the multi-tenancy architecture to handle.") - return nil - } - klog.Info("there are secret(s) for the operator") - for index, secret := range secretsAvailableAtOperatorNS.Items { - err = c.renewCert(secret, index, tenant) - if err != nil { - klog.Errorf("There was an error while renewing the cert: %s", err) - return err - } + klog.Errorf("There was an error while renewing the cert: %s", err) + return err } } } diff --git a/pkg/controller/tls.go b/pkg/controller/tls.go index c29521fe23d..b491e2b7be7 100644 --- a/pkg/controller/tls.go +++ b/pkg/controller/tls.go @@ -28,6 +28,8 @@ import ( "os" "time" + "github.com/minio/operator/pkg/certs" + miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" "github.com/minio/operator/pkg/controller/certificates" xcerts "github.com/minio/pkg/certs" @@ -104,8 +106,8 @@ func (c *Controller) writeCertSecretToFile(tlsCertSecret *corev1.Secret, service return publicCertPath, privateKeyPath } -// generateTLSCert Generic method to generate TLS Certificartes for different services -func (c *Controller) generateTLSCert(serviceName string, secretName string, deploymentName string) (*string, *string) { +// generateTLSCertificateForService Generic method to generate TLS Certificates for a service +func (c *Controller) generateTLSCertificateForService(serviceName, secretName, deploymentName string) (*string, *string) { ctx := context.Background() namespace := miniov2.GetNSFromFile() csrName := getCSRName(serviceName) @@ -172,12 +174,12 @@ func (c *Controller) checkAndCreateCSR(ctx context.Context, deployment metav1.Ob // getKeyNames Identify the K8s secret keys containing the public and private TLS certificate keys func (c *Controller) getKeyNames(tlsCertificateSecret *corev1.Secret) (string, string) { // default secret keys for Opaque k8s secret - publicCertKey := "public.crt" - privateKeyKey := "private.key" + publicCertKey := certs.PublicCertFile + privateKeyKey := certs.PrivateKeyFile // if secret type is k8s tls or cert-manager use the right secret keys if tlsCertificateSecret.Type == "kubernetes.io/tls" || tlsCertificateSecret.Type == "cert-manager.io/v1alpha2" || tlsCertificateSecret.Type == "cert-manager.io/v1" { - publicCertKey = "tls.crt" - privateKeyKey = "tls.key" + publicCertKey = certs.TLSCertFile + privateKeyKey = certs.TLSKeyFile } return publicCertKey, privateKeyKey } @@ -199,8 +201,8 @@ func (c *Controller) createCertificateSecret(ctx context.Context, deployment met }, }, Data: map[string][]byte{ - "private.key": pkBytes, - "public.crt": certBytes, + certs.PrivateKeyFile: pkBytes, + certs.PublicCertFile: certBytes, }, } _, err := c.kubeClientSet.CoreV1().Secrets(miniov2.GetNSFromFile()).Create(ctx, secret, metav1.CreateOptions{}) @@ -211,7 +213,7 @@ func (c *Controller) createCertificateSecret(ctx context.Context, deployment met // finally creating a secret storing private key and certificate for TLS // This Method Blocks till the CSR Request is approved via kubectl approve func (c *Controller) createAndStoreCSR(ctx context.Context, deployment metav1.Object, serviceName string, csrName string, secretName string) error { - privKeysBytes, csrBytes, err := generateCSRCryptoData(serviceName) + privKeysBytes, csrBytes, err := generateServiceCSRCryptoData(serviceName) if err != nil { klog.Errorf("Private Key and CSR generation failed with error: %v", err) return err @@ -299,8 +301,8 @@ func getCSRName(serviceName string) string { return fmt.Sprintf("%s-%s-csr", serviceName, namespace) } -// generateCSRCryptoData Internal func Creates the private Key material -func generateCSRCryptoData(serviceName string) ([]byte, []byte, error) { +// generateServiceCSRCryptoData Creates the private Key material +func generateServiceCSRCryptoData(serviceName string) ([]byte, []byte, error) { privateKey, err := newPrivateKey(miniov2.DefaultEllipticCurve) if err != nil { klog.Errorf("Unexpected error during the ECDSA Key generation: %v", err) diff --git a/pkg/resources/statefulsets/kes-statefulset.go b/pkg/resources/statefulsets/kes-statefulset.go index 49b04091602..54b4c3fa6cd 100644 --- a/pkg/resources/statefulsets/kes-statefulset.go +++ b/pkg/resources/statefulsets/kes-statefulset.go @@ -17,6 +17,8 @@ package statefulsets import ( "sort" + "github.com/minio/operator/pkg/certs" + operatorApi "github.com/minio/operator/api" miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" appsv1 "k8s.io/api/apps/v1" @@ -201,8 +203,8 @@ func NewForKES(t *miniov2.Tenant, serviceName string) *appsv1.StatefulSet { var clientCertSecret string serverCertPaths := []corev1.KeyToPath{ - {Key: "public.crt", Path: certPath}, - {Key: "private.key", Path: keyPath}, + {Key: certs.PublicCertFile, Path: certPath}, + {Key: certs.PrivateKeyFile, Path: keyPath}, } configPath := []corev1.KeyToPath{ @@ -216,8 +218,8 @@ func NewForKES(t *miniov2.Tenant, serviceName string) *appsv1.StatefulSet { // "cert-manager.io/v1alpha2" because of same keys in both. if t.Spec.KES.ExternalCertSecret.Type == "kubernetes.io/tls" || t.Spec.KES.ExternalCertSecret.Type == "cert-manager.io/v1alpha2" || t.Spec.KES.ExternalCertSecret.Type == "cert-manager.io/v1" { serverCertPaths = []corev1.KeyToPath{ - {Key: "tls.crt", Path: certPath}, - {Key: "tls.key", Path: keyPath}, + {Key: certs.TLSCertFile, Path: certPath}, + {Key: certs.TLSKeyFile, Path: keyPath}, } } } else { diff --git a/pkg/resources/statefulsets/minio-statefulset.go b/pkg/resources/statefulsets/minio-statefulset.go index 636030f55f0..159367dba4c 100644 --- a/pkg/resources/statefulsets/minio-statefulset.go +++ b/pkg/resources/statefulsets/minio-statefulset.go @@ -467,7 +467,6 @@ type NewPoolArgs struct { ServiceName string HostsTemplate string OperatorVersion string - OperatorCATLS bool } // NewPool creates a new StatefulSet for the given Cluster. @@ -486,12 +485,12 @@ func NewPool(args *NewPoolArgs) *appsv1.StatefulSet { var clientCertSecret string clientCertPaths := []corev1.KeyToPath{ - {Key: "public.crt", Path: "client.crt"}, - {Key: "private.key", Path: "client.key"}, + {Key: certs.PublicCertFile, Path: "client.crt"}, + {Key: certs.PrivateKeyFile, Path: "client.key"}, } var kesCertSecret string KESCertPath := []corev1.KeyToPath{ - {Key: "public.crt", Path: "CAs/kes.crt"}, + {Key: certs.PublicCertFile, Path: "CAs/kes.crt"}, } // Create an empty dir volume to share the configuration between the main container and side-car @@ -526,8 +525,8 @@ func NewPool(args *NewPoolArgs) *appsv1.StatefulSet { // // Iterate over all provided TLS certificates and store them on the list of Volumes that will be mounted to the Pod for index, secret := range t.Spec.ExternalCertSecret { - crtMountPath := fmt.Sprintf("hostname-%d/public.crt", index) - keyMountPath := fmt.Sprintf("hostname-%d/private.key", index) + crtMountPath := fmt.Sprintf("hostname-%d/%s", index, certs.PublicCertFile) + keyMountPath := fmt.Sprintf("hostname-%d/%s", index, certs.PrivateKeyFile) caMountPath := fmt.Sprintf("CAs/hostname-%d.crt", index) // MinIO requires to have at least 1 certificate keyPair under the `certs` folder, by default // we will take the first secret as the default certificate @@ -536,29 +535,29 @@ func NewPool(args *NewPoolArgs) *appsv1.StatefulSet { // + public.crt // + private.key if index == 0 { - crtMountPath = "public.crt" - keyMountPath = "private.key" - caMountPath = "CAs/public.crt" + crtMountPath = certs.PublicCertFile + keyMountPath = certs.PrivateKeyFile + caMountPath = fmt.Sprintf("%s/%s", certs.CertsCADir, certs.PublicCertFile) } var serverCertPaths []corev1.KeyToPath if secret.Type == "kubernetes.io/tls" { serverCertPaths = []corev1.KeyToPath{ - {Key: "tls.crt", Path: crtMountPath}, - {Key: "tls.key", Path: keyMountPath}, - {Key: "tls.crt", Path: caMountPath}, + {Key: certs.TLSCertFile, Path: crtMountPath}, + {Key: certs.TLSKeyFile, Path: keyMountPath}, + {Key: certs.TLSCertFile, Path: caMountPath}, } } else if secret.Type == "cert-manager.io/v1alpha2" || secret.Type == "cert-manager.io/v1" { serverCertPaths = []corev1.KeyToPath{ - {Key: "tls.crt", Path: crtMountPath}, - {Key: "tls.key", Path: keyMountPath}, - {Key: "ca.crt", Path: caMountPath}, + {Key: certs.TLSCertFile, Path: crtMountPath}, + {Key: certs.TLSKeyFile, Path: keyMountPath}, + {Key: certs.CAPublicCertFile, Path: caMountPath}, } } else { serverCertPaths = []corev1.KeyToPath{ - {Key: "public.crt", Path: crtMountPath}, - {Key: "private.key", Path: keyMountPath}, - {Key: "public.crt", Path: caMountPath}, + {Key: certs.PublicCertFile, Path: crtMountPath}, + {Key: certs.PrivateKeyFile, Path: keyMountPath}, + {Key: certs.PublicCertFile, Path: caMountPath}, } } certVolumeSources = append(certVolumeSources, corev1.VolumeProjection{ @@ -572,13 +571,13 @@ func NewPool(args *NewPoolArgs) *appsv1.StatefulSet { } // AutoCert certificates will be used for internal communication if requested if t.AutoCert() { - crtMountPath := "public.crt" - keyMountPath := "private.key" - caMountPath := "CAs/public.crt" + crtMountPath := certs.PublicCertFile + keyMountPath := certs.PrivateKeyFile + caMountPath := fmt.Sprintf("%s/%s", certs.CertsCADir, certs.PublicCertFile) if len(t.Spec.ExternalCertSecret) > 0 { index := len(t.Spec.ExternalCertSecret) - crtMountPath = fmt.Sprintf("hostname-%d/public.crt", index) - keyMountPath = fmt.Sprintf("hostname-%d/private.key", index) + crtMountPath = fmt.Sprintf("hostname-%d/%s", index, certs.PublicCertFile) + keyMountPath = fmt.Sprintf("hostname-%d/%s", index, certs.PrivateKeyFile) caMountPath = fmt.Sprintf("CAs/hostname-%d.crt", index) } certVolumeSources = append(certVolumeSources, corev1.VolumeProjection{ @@ -587,9 +586,9 @@ func NewPool(args *NewPoolArgs) *appsv1.StatefulSet { Name: t.MinIOTLSSecretName(), }, Items: []corev1.KeyToPath{ - {Key: "public.crt", Path: crtMountPath}, - {Key: "private.key", Path: keyMountPath}, - {Key: "public.crt", Path: caMountPath}, + {Key: certs.PublicCertFile, Path: crtMountPath}, + {Key: certs.PrivateKeyFile, Path: keyMountPath}, + {Key: certs.PublicCertFile, Path: caMountPath}, }, }, }) @@ -615,19 +614,19 @@ func NewPool(args *NewPoolArgs) *appsv1.StatefulSet { var clientKeyPairPaths []corev1.KeyToPath if secret.Type == "kubernetes.io/tls" { clientKeyPairPaths = []corev1.KeyToPath{ - {Key: "tls.crt", Path: crtMountPath}, - {Key: "tls.key", Path: keyMountPath}, + {Key: certs.TLSCertFile, Path: crtMountPath}, + {Key: certs.TLSKeyFile, Path: keyMountPath}, } } else if secret.Type == "cert-manager.io/v1alpha2" || secret.Type == "cert-manager.io/v1" { clientKeyPairPaths = []corev1.KeyToPath{ - {Key: "tls.crt", Path: crtMountPath}, - {Key: "tls.key", Path: keyMountPath}, - {Key: "ca.crt", Path: fmt.Sprintf("CAs/client-ca-%d.crt", index)}, + {Key: certs.TLSCertFile, Path: crtMountPath}, + {Key: certs.TLSKeyFile, Path: keyMountPath}, + {Key: certs.CAPublicCertFile, Path: fmt.Sprintf("%s/client-ca-%d.crt", certs.CertsCADir, index)}, } } else { clientKeyPairPaths = []corev1.KeyToPath{ - {Key: "public.crt", Path: crtMountPath}, - {Key: "private.key", Path: keyMountPath}, + {Key: certs.PublicCertFile, Path: crtMountPath}, + {Key: certs.PrivateKeyFile, Path: keyMountPath}, } } certVolumeSources = append(certVolumeSources, corev1.VolumeProjection{ @@ -653,15 +652,15 @@ func NewPool(args *NewPoolArgs) *appsv1.StatefulSet { // "cert-manager.io/v1alpha2" because of same keys in both. if secret.Type == "kubernetes.io/tls" { caCertPaths = []corev1.KeyToPath{ - {Key: "tls.crt", Path: fmt.Sprintf("CAs/ca-%d.crt", index)}, + {Key: certs.TLSCertFile, Path: fmt.Sprintf("%s/ca-%d.crt", certs.CertsCADir, index)}, } } else if secret.Type == "cert-manager.io/v1alpha2" || secret.Type == "cert-manager.io/v1" { caCertPaths = []corev1.KeyToPath{ - {Key: "ca.crt", Path: fmt.Sprintf("CAs/ca-%d.crt", index)}, + {Key: certs.CAPublicCertFile, Path: fmt.Sprintf("%s/ca-%d.crt", certs.CertsCADir, index)}, } } else { caCertPaths = []corev1.KeyToPath{ - {Key: "public.crt", Path: fmt.Sprintf("CAs/ca-%d.crt", index)}, + {Key: certs.PublicCertFile, Path: fmt.Sprintf("%s/ca-%d.crt", certs.CertsCADir, index)}, } } certVolumeSources = append(certVolumeSources, corev1.VolumeProjection{ @@ -683,13 +682,13 @@ func NewPool(args *NewPoolArgs) *appsv1.StatefulSet { // "cert-manager.io/v1alpha2" / cert-manager.io/v1 because of same keys in both. if t.Spec.ExternalClientCertSecret.Type == "kubernetes.io/tls" || t.Spec.ExternalClientCertSecret.Type == "cert-manager.io/v1alpha2" || t.Spec.KES.ExternalCertSecret.Type == "cert-manager.io/v1" { clientCertPaths = []corev1.KeyToPath{ - {Key: "tls.crt", Path: "client.crt"}, - {Key: "tls.key", Path: "client.key"}, + {Key: certs.TLSCertFile, Path: "client.crt"}, + {Key: certs.TLSKeyFile, Path: "client.key"}, } } else { clientCertPaths = []corev1.KeyToPath{ - {Key: "public.crt", Path: "client.crt"}, - {Key: "private.key", Path: "client.key"}, + {Key: certs.PublicCertFile, Path: "client.crt"}, + {Key: certs.PrivateKeyFile, Path: "client.key"}, } } } else { @@ -703,7 +702,7 @@ func NewPool(args *NewPoolArgs) *appsv1.StatefulSet { // "cert-manager.io/v1alpha2" because of same keys in both. if t.Spec.KES.ExternalCertSecret.Type == "kubernetes.io/tls" || t.Spec.KES.ExternalCertSecret.Type == "cert-manager.io/v1alpha2" || t.Spec.KES.ExternalCertSecret.Type == "cert-manager.io/v1" { KESCertPath = []corev1.KeyToPath{ - {Key: "tls.crt", Path: "CAs/kes.crt"}, + {Key: certs.TLSCertFile, Path: fmt.Sprintf("%s/kes.crt", certs.CertsCADir)}, } } } else {