From 369663e4be6c1916881081e1ec171bbecdb7668e Mon Sep 17 00:00:00 2001 From: Milos Gajdos Date: Tue, 1 Apr 2025 08:15:26 -0700 Subject: [PATCH] Fix S3 driver loglevel param Unfortunately YAML struck us hard in this one. It interprets "off" as a truthy value so setting loglevel to off sets it to false. This commit makes sure we set the loglevel to off if the param is marshalled into false and if it's not a string. Signed-off-by: Milos Gajdos --- registry/storage/driver/s3-aws/s3.go | 13 ++- registry/storage/driver/s3-aws/s3_test.go | 115 ++++++++++++++++++++++ 2 files changed, 126 insertions(+), 2 deletions(-) diff --git a/registry/storage/driver/s3-aws/s3.go b/registry/storage/driver/s3-aws/s3.go index 1550ddb4b..39cbed3f2 100644 --- a/registry/storage/driver/s3-aws/s3.go +++ b/registry/storage/driver/s3-aws/s3.go @@ -448,11 +448,20 @@ func FromParameters(ctx context.Context, parameters map[string]interface{}) (*Dr return New(ctx, params) } -func getS3LogLevelFromParam(param interface{}) aws.LogLevelType { +func getS3LogLevelFromParam(param any) aws.LogLevelType { if param == nil { return aws.LogOff } - logLevelParam := param.(string) + // YAML 1.X interprets "off" as false + if b, ok := param.(bool); ok && !b { + return aws.LogOff + } + // if it's not a string, return off + logLevelParam, ok := param.(string) + if !ok { + return aws.LogOff + } + var logLevel aws.LogLevelType switch strings.ToLower(logLevelParam) { case "off": diff --git a/registry/storage/driver/s3-aws/s3_test.go b/registry/storage/driver/s3-aws/s3_test.go index ff61ef1ad..407cd4ac6 100644 --- a/registry/storage/driver/s3-aws/s3_test.go +++ b/registry/storage/driver/s3-aws/s3_test.go @@ -166,6 +166,121 @@ func BenchmarkS3DriverSuite(b *testing.B) { testsuites.BenchDriver(b, newDriverConstructor(b)) } +func TestGetS3LogLevelFromParam(t *testing.T) { + tests := []struct { + name string + param any + expected aws.LogLevelType + }{ + // Nil case + { + name: "nil parameter", + param: nil, + expected: aws.LogOff, + }, + + // Boolean cases - YAML converts "off" to false + { + name: "boolean false (from YAML off)", + param: false, + expected: aws.LogOff, + }, + { + name: "boolean true", + param: true, + expected: aws.LogOff, + }, + + // String cases - valid values + { + name: "string off", + param: "off", + expected: aws.LogOff, + }, + { + name: "string OFF uppercase", + param: "OFF", + expected: aws.LogOff, + }, + { + name: "string debug", + param: "debug", + expected: aws.LogDebug, + }, + { + name: "string DEBUG uppercase", + param: "DEBUG", + expected: aws.LogDebug, + }, + { + name: "string debugwithsigning", + param: "debugwithsigning", + expected: aws.LogDebugWithSigning, + }, + { + name: "string debugwithhttpbody", + param: "debugwithhttpbody", + expected: aws.LogDebugWithHTTPBody, + }, + { + name: "string debugwithrequestretries", + param: "debugwithrequestretries", + expected: aws.LogDebugWithRequestRetries, + }, + { + name: "string debugwithrequesterrors", + param: "debugwithrequesterrors", + expected: aws.LogDebugWithRequestErrors, + }, + { + name: "string debugwitheventstreambody", + param: "debugwitheventstreambody", + expected: aws.LogDebugWithEventStreamBody, + }, + + // Invalid cases + { + name: "invalid string value", + param: "invalid", + expected: aws.LogOff, + }, + { + name: "empty string", + param: "", + expected: aws.LogOff, + }, + { + name: "integer value", + param: 42, + expected: aws.LogOff, + }, + { + name: "float value", + param: 42.0, + expected: aws.LogOff, + }, + { + name: "slice value", + param: []string{"off"}, + expected: aws.LogOff, + }, + { + name: "map value", + param: map[string]string{"level": "off"}, + expected: aws.LogOff, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getS3LogLevelFromParam(tt.param) + if result != tt.expected { + t.Errorf("getS3LogLevelFromParam(%v) = %v, want %v", tt.param, result, tt.expected) + } + }) + } +} + func TestEmptyRootList(t *testing.T) { skipCheck(t)