From 7c1f9667d14703c1a606106e355a188216c8064c Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Tue, 16 Apr 2024 08:43:39 -0700 Subject: [PATCH] Use GetDuration() helper for MINIO_KMS_KEY_CACHE_INTERVAL as time.Duration (#19512) Bonus: Use default duration of 10 seconds if invalid input < time.Second is specified --- cmd/common-main.go | 3 +-- cmd/logging.go | 13 +++++++++---- go.mod | 2 +- go.sum | 4 ++-- internal/kms/kes.go | 27 ++++++++++++++++----------- 5 files changed, 29 insertions(+), 20 deletions(-) diff --git a/cmd/common-main.go b/cmd/common-main.go index b57dcf7d7..3adc2fc68 100644 --- a/cmd/common-main.go +++ b/cmd/common-main.go @@ -958,8 +958,7 @@ func handleKMSConfig() { } } - kmsLogger := Logger{} - KMS, err := kms.NewWithConfig(kmsConf, kmsLogger) + KMS, err := kms.NewWithConfig(kmsConf, KMSLogger{}) if err != nil { logger.Fatal(err, "Unable to initialize a connection to KES as specified by the shell environment") } diff --git a/cmd/logging.go b/cmd/logging.go index 79c3e93eb..456c469d3 100644 --- a/cmd/logging.go +++ b/cmd/logging.go @@ -194,10 +194,15 @@ func kmsLogIf(ctx context.Context, err error, errKind ...interface{}) { logger.LogIf(ctx, "kms", err, errKind...) } -// Logger permits access to module specific logging -type Logger struct{} +// KMSLogger permits access to kms module specific logging +type KMSLogger struct{} // LogOnceIf is the implementation of LogOnceIf, accessible using the Logger interface -func (l Logger) LogOnceIf(ctx context.Context, subsystem string, err error, id string, errKind ...interface{}) { - logger.LogOnceIf(ctx, subsystem, err, id, errKind...) +func (l KMSLogger) LogOnceIf(ctx context.Context, err error, id string, errKind ...interface{}) { + logger.LogOnceIf(ctx, "kms", err, id, errKind...) +} + +// LogIf is the implementation of LogIf, accessible using the Logger interface +func (l KMSLogger) LogIf(ctx context.Context, err error, errKind ...interface{}) { + logger.LogIf(ctx, "kms", err, errKind...) } diff --git a/go.mod b/go.mod index e2022389f..26237c0d2 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,7 @@ require ( github.com/minio/madmin-go/v3 v3.0.50 github.com/minio/minio-go/v7 v7.0.69 github.com/minio/mux v1.9.0 - github.com/minio/pkg/v2 v2.0.16 + github.com/minio/pkg/v2 v2.0.17 github.com/minio/selfupdate v0.6.0 github.com/minio/sha256-simd v1.0.1 github.com/minio/simdjson-go v0.4.5 diff --git a/go.sum b/go.sum index a0655da2c..a2d3065c5 100644 --- a/go.sum +++ b/go.sum @@ -455,8 +455,8 @@ github.com/minio/minio-go/v7 v7.0.69 h1:l8AnsQFyY1xiwa/DaQskY4NXSLA2yrGsW5iD9nRP github.com/minio/minio-go/v7 v7.0.69/go.mod h1:XAvOPJQ5Xlzk5o3o/ArO2NMbhSGkimC+bpW/ngRKDmQ= github.com/minio/mux v1.9.0 h1:dWafQFyEfGhJvK6AwLOt83bIG5bxKxKJnKMCi0XAaoA= github.com/minio/mux v1.9.0/go.mod h1:1pAare17ZRL5GpmNL+9YmqHoWnLmMZF9C/ioUCfy0BQ= -github.com/minio/pkg/v2 v2.0.16 h1:qBw2D08JE7fu4UORIxx0O4L09NM0wtMrw9sJRU5R1u0= -github.com/minio/pkg/v2 v2.0.16/go.mod h1:V+OP/fKRD/qhJMQpdXXrCXcLYjGMpHKEE26zslthm5k= +github.com/minio/pkg/v2 v2.0.17 h1:ndmGlitUj/eCVRPmfsAw3KlbtVNxqk0lQIvDXlcTHiQ= +github.com/minio/pkg/v2 v2.0.17/go.mod h1:V+OP/fKRD/qhJMQpdXXrCXcLYjGMpHKEE26zslthm5k= github.com/minio/selfupdate v0.6.0 h1:i76PgT0K5xO9+hjzKcacQtO7+MjJ4JKA8Ak8XQ9DDwU= github.com/minio/selfupdate v0.6.0/go.mod h1:bO02GTIPCMQFTEvE5h4DjYB58bCoZ35XLeBf0buTDdM= github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM= diff --git a/internal/kms/kes.go b/internal/kms/kes.go index 3ab564dc8..bce1b123c 100644 --- a/internal/kms/kes.go +++ b/internal/kms/kes.go @@ -72,7 +72,7 @@ type Config struct { // NewWithConfig returns a new KMS using the given // configuration. -func NewWithConfig(config Config, kmsLogger Logger) (KMS, error) { +func NewWithConfig(config Config, logger Logger) (KMS, error) { if len(config.Endpoints) == 0 { return nil, errors.New("kms: no server endpoints") } @@ -141,7 +141,7 @@ func NewWithConfig(config Config, kmsLogger Logger) (KMS, error) { } }() - go c.refreshKMSMasterKeyCache(kmsLogger) + go c.refreshKMSMasterKeyCache(logger) return c, nil } @@ -150,13 +150,17 @@ func NewWithConfig(config Config, kmsLogger Logger) (KMS, error) { func (c *kesClient) refreshKMSMasterKeyCache(logger Logger) { ctx := context.Background() - defaultCacheInterval := 10 - cacheInterval, err := env.GetInt("EnvKESKeyCacheInterval", defaultCacheInterval) + defaultCacheDuration := 10 * time.Second + cacheDuration, err := env.GetDuration(EnvKESKeyCacheInterval, defaultCacheDuration) if err != nil { - cacheInterval = defaultCacheInterval + logger.LogOnceIf(ctx, fmt.Errorf("%s, using default of 10s", err.Error()), "refresh-kms-master-key") + cacheDuration = defaultCacheDuration } - - timer := time.NewTimer(time.Duration(cacheInterval) * time.Second) + if cacheDuration < time.Second { + logger.LogOnceIf(ctx, errors.New("cache duration is less than 1s, using default of 10s"), "refresh-kms-master-key") + cacheDuration = defaultCacheDuration + } + timer := time.NewTimer(cacheDuration) defer timer.Stop() for { @@ -167,7 +171,7 @@ func (c *kesClient) refreshKMSMasterKeyCache(logger Logger) { c.RefreshKey(ctx, logger) // Reset for the next interval - timer.Reset(time.Duration(cacheInterval) * time.Second) + timer.Reset(cacheDuration) } } } @@ -484,7 +488,8 @@ func (c *kesClient) Verify(ctx context.Context) []VerifyResult { // Logger interface permits access to module specific logging, in this case, for KMS type Logger interface { - LogOnceIf(ctx context.Context, subsystem string, err error, id string, errKind ...interface{}) + LogOnceIf(ctx context.Context, err error, id string, errKind ...interface{}) + LogIf(ctx context.Context, err error, errKind ...interface{}) } // RefreshKey checks the validity of the KMS Master Key @@ -503,13 +508,13 @@ func (c *kesClient) RefreshKey(ctx context.Context, logger Logger) bool { // 1. Generate a new key using the KMS. kmsCtx, err := kmsContext.MarshalText() if err != nil { - logger.LogOnceIf(ctx, "kms", err, "refresh-kms-master-key") + logger.LogOnceIf(ctx, err, "refresh-kms-master-key") validKey = false break } _, err = client.GenerateKey(ctx, env.Get(EnvKESKeyName, ""), kmsCtx) if err != nil { - logger.LogOnceIf(ctx, "kms", err, "refresh-kms-master-key") + logger.LogOnceIf(ctx, err, "refresh-kms-master-key") validKey = false break }