From fdb7232d25825ca9fdeeb1832bc2e02ee464426a Mon Sep 17 00:00:00 2001 From: Pedro Juarez Date: Fri, 24 May 2024 08:55:05 -0700 Subject: [PATCH] Bugfix sidecar credentials validation (#2134) * Bugfix sidecar credentials validation Sidecar allways return `Missing root credentials in the configuration.` causing tenant to not start on the modification of the tenant configuration secret that sidecar observes * Adds little `Config secret '%s' sync` log line to know when a secret sync event has triggered in sidecar. * Remove `pkg/validator.go` file, it got moved to `sidecar/pkg/validator/validator.go` and this one is no longer needed. * Fix: "Struct Controller has methods on both value and pointer receivers. Such usage is not recommended by the Go Documentation." * Run Informer factories in goroutines to do not block the process and proceed to wait for caches to sync. Signed-off-by: pjuarezd * lint Signed-off-by: pjuarezd --------- Signed-off-by: pjuarezd --- pkg/validator/validator.go | 140 --------------------------- sidecar/pkg/sidecar/sidecar_utils.go | 47 +++++---- 2 files changed, 23 insertions(+), 164 deletions(-) delete mode 100644 pkg/validator/validator.go diff --git a/pkg/validator/validator.go b/pkg/validator/validator.go deleted file mode 100644 index d46b51e6862..00000000000 --- a/pkg/validator/validator.go +++ /dev/null @@ -1,140 +0,0 @@ -// This file is part of MinIO Operator -// Copyright (c) 2023 MinIO, Inc. -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -package validator - -import ( - "bufio" - "context" - "fmt" - "log" - "os" - "strings" - - miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" - clientset "github.com/minio/operator/pkg/client/clientset/versioned" - "github.com/minio/operator/pkg/resources/statefulsets" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/rest" - "k8s.io/klog/v2" -) - -// Validate checks the configuration on the seeded configuration and issues a valid one for MinIO to -// start, however if root credentials are missing, it will exit with error -func Validate(tenantName string) { - rootUserFound, rootPwdFound, fileContents, err := ReadTmpConfig() - if err != nil { - panic(err) - } - - namespace := miniov2.GetNSFromFile() - - cfg, err := rest.InClusterConfig() - // If config is passed as a flag use that instead - //if kubeconfig != "" { - // cfg, err = clientcmd.BuildConfigFromFlags(masterURL, kubeconfig) - //} - if err != nil { - panic(err) - } - - controllerClient, err := clientset.NewForConfig(cfg) - if err != nil { - klog.Fatalf("Error building MinIO clientset: %s", err.Error()) - } - - ctx := context.Background() - - args, err := GetTenantArgs(ctx, controllerClient, tenantName, namespace) - if err != nil { - log.Println(err) - os.Exit(1) - } - - fileContents = fileContents + fmt.Sprintf("export MINIO_ARGS=\"%s\"\n", args) - - if !rootUserFound || !rootPwdFound { - log.Println("Missing root credentials in the configuration.") - log.Println("MinIO won't start") - os.Exit(1) - } - - err = os.WriteFile(miniov2.CfgFile, []byte(fileContents), 0o644) - if err != nil { - log.Println(err) - } -} - -// GetTenantArgs returns the arguments for the tenant based on the tenants they have -func GetTenantArgs(ctx context.Context, controllerClient *clientset.Clientset, tenantName string, namespace string) (string, error) { - // get the only tenant in this namespace - tenant, err := controllerClient.MinioV2().Tenants(namespace).Get(ctx, tenantName, metav1.GetOptions{}) - if err != nil { - log.Println(err) - return "", err - } - - tenant.EnsureDefaults() - - // Validate the MinIO Tenant - if err = tenant.Validate(); err != nil { - log.Println(err) - return "", err - } - - args := strings.Join(statefulsets.GetContainerArgs(tenant, ""), " ") - return args, err -} - -// ReadTmpConfig reads the seeded configuration from a tmp location -func ReadTmpConfig() (bool, bool, string, error) { - file, err := os.Open("/tmp/minio-config/config.env") - if err != nil { - log.Fatal(err) - } - defer file.Close() - - rootUserFound := false - rootPwdFound := false - - scanner := bufio.NewScanner(file) - newFile := "" - for scanner.Scan() { - line := scanner.Text() - if strings.Contains(line, "MINIO_ROOT_USER") { - rootUserFound = true - } - if strings.Contains(line, "MINIO_ACCESS_KEY") { - rootUserFound = true - } - if strings.Contains(line, "MINIO_ROOT_PASSWORD") { - rootPwdFound = true - } - if strings.Contains(line, "MINIO_SECRET_KEY") { - rootPwdFound = true - } - // We don't allow users to set MINIO_ARGS - if strings.Contains(line, "MINIO_ARGS") { - log.Println("MINIO_ARGS in config file found. It will be ignored.") - continue - } - newFile = newFile + line + "\n" - } - if err := scanner.Err(); err != nil { - log.Fatal(err) - } - return rootUserFound, rootPwdFound, newFile, nil -} diff --git a/sidecar/pkg/sidecar/sidecar_utils.go b/sidecar/pkg/sidecar/sidecar_utils.go index 365988d5a7e..746cde2130f 100644 --- a/sidecar/pkg/sidecar/sidecar_utils.go +++ b/sidecar/pkg/sidecar/sidecar_utils.go @@ -143,6 +143,7 @@ func NewSideCarController(kubeClient *kubernetes.Clientset, controllerClient *cl if oldSecret.Name != secretName { return } + log.Printf("Config secret '%s' sync", secretName) newSecret := new.(*corev1.Secret) if newSecret.ResourceVersion == oldSecret.ResourceVersion { // Periodic resync will send update events for all known Tenants. @@ -151,36 +152,39 @@ func NewSideCarController(kubeClient *kubernetes.Clientset, controllerClient *cl } data := newSecret.Data["config.env"] // validate root creds in string - rootUserMissing := true - rootPassMissing := false + rootUserFound := false + rootPwdFound := false dataStr := string(data) - if !strings.Contains(dataStr, "MINIO_ROOT_USER") { - rootUserMissing = true + if strings.Contains(dataStr, "MINIO_ROOT_USER") { + rootUserFound = true } - if !strings.Contains(dataStr, "MINIO_ACCESS_KEY") { - rootUserMissing = true + if strings.Contains(dataStr, "MINIO_ACCESS_KEY") { + rootUserFound = true } - if !strings.Contains(dataStr, "MINIO_ROOT_PASSWORD") { - rootPassMissing = true + if strings.Contains(dataStr, "MINIO_ROOT_PASSWORD") { + rootPwdFound = true } - if !strings.Contains(dataStr, "MINIO_SECRET_KEY") { - rootPassMissing = true + if strings.Contains(dataStr, "MINIO_SECRET_KEY") { + rootPwdFound = true } - if rootUserMissing || rootPassMissing { + if !rootUserFound || !rootPwdFound { log.Println("Missing root credentials in the configuration.") log.Println("MinIO won't start") os.Exit(1) } - c.regenCfgWithCfg(tenantName, namespace, string(data)) + if !strings.HasSuffix(dataStr, "\n") { + dataStr = dataStr + "\n" + } + c.regenCfgWithCfg(tenantName, namespace, dataStr) }, }) return c } -func (c Controller) regenCfg(tenantName string, namespace string) { +func (c *Controller) regenCfg(tenantName string, namespace string) { rootUserFound, rootPwdFound, fileContents, err := validator.ReadTmpConfig() if err != nil { log.Println(err) @@ -194,7 +198,7 @@ func (c Controller) regenCfg(tenantName string, namespace string) { c.regenCfgWithCfg(tenantName, namespace, fileContents) } -func (c Controller) regenCfgWithCfg(tenantName string, namespace string, fileContents string) { +func (c *Controller) regenCfgWithCfg(tenantName string, namespace string, fileContents string) { ctx := context.Background() args, err := validator.GetTenantArgs(ctx, c.controllerClient, tenantName, namespace) @@ -213,18 +217,13 @@ func (c Controller) regenCfgWithCfg(tenantName string, namespace string, fileCon // Run starts the informers func (c *Controller) Run(stopCh chan struct{}) error { - // Starts all the shared minioInformers that have been created by the factory so - // far. - c.minInformerFactory.Start(stopCh) - c.informerFactory.Start(stopCh) + // Starts all the shared minioInformers that have been created by the factory so far. + go c.minInformerFactory.Start(stopCh) + go c.informerFactory.Start(stopCh) // wait for the initial synchronization of the local cache. - if !cache.WaitForCacheSync(stopCh, c.tenantInformer.Informer().HasSynced) { - return fmt.Errorf("Failed to sync") - } - // wait for the initial synchronization of the local cache. - if !cache.WaitForCacheSync(stopCh, c.secretInformer.Informer().HasSynced) { - return fmt.Errorf("Failed to sync") + if !cache.WaitForCacheSync(stopCh, c.tenantInformer.Informer().HasSynced, c.secretInformer.Informer().HasSynced) { + return fmt.Errorf("failed to sync") } return nil }