From 4d359753acb6da8e9fd52856ac80da9d9f213d7f Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Wed, 19 Apr 2023 17:19:55 +0200 Subject: [PATCH] update logr to 1.2.4 and fix Stash client In version 1.2.4 a [change](https://github.com/go-logr/logr/pull/166) was introduced that significantly changed the assumptions about logr's API, i.e. that `logr.Logger{} != logr.Discard()`. This condition doesn't evaluate to true with 1.2.4, anymore, and the default value of the `logr.Logger` type is now usable as a replacement for `logr.Discard()`. Therefore we do not need to check if the value has been provided, anymore: If it is not provided, the default one is used which is effectively a discarding logger. closes #209 Signed-off-by: Max Jonas Werner --- go.mod | 3 ++- go.sum | 6 ++++-- stash/auth.go | 9 +++++---- stash/client.go | 4 ---- stash/client_test.go | 26 ++++++++++++++++---------- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index 8689fe66..99e965dd 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/ProtonMail/go-crypto v0.0.0-20220714114130-e85cedf506cd github.com/go-git/go-billy/v5 v5.3.1 github.com/go-git/go-git/v5 v5.4.2 - github.com/go-logr/logr v1.2.3 + github.com/go-logr/logr v1.2.4 github.com/go-logr/zapr v1.2.3 github.com/google/go-cmp v0.5.9 github.com/google/go-github/v49 v49.1.0 @@ -22,6 +22,7 @@ require ( golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa golang.org/x/oauth2 v0.6.0 golang.org/x/time v0.3.0 + k8s.io/utils v0.0.0-20230406110748-d93618cff8a2 ) // Fix CVE-2022-28948 diff --git a/go.sum b/go.sum index 918a6f6c..c752fda3 100644 --- a/go.sum +++ b/go.sum @@ -34,8 +34,8 @@ github.com/go-git/go-git-fixtures/v4 v4.2.1/go.mod h1:K8zd3kDUAykwTdDCr+I0per6Y6 github.com/go-git/go-git/v5 v5.4.2 h1:BXyZu9t0VkbiHtqrsvdq39UDhGJTl1h55VW6CSC4aY4= github.com/go-git/go-git/v5 v5.4.2/go.mod h1:gQ1kArt6d+n+BGd+/B/I74HwRTLhth2+zti4ihgckDc= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0= -github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/logr v1.2.4 h1:g01GSCwiDw2xSZfjJ2/T9M+S6pFdcNtFYsp+Y43HYDQ= +github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/zapr v1.2.3 h1:a9vnzlIBPQBBkeaR9IuMUfmVOrQlkoC4YfPoFkX3T7A= github.com/go-logr/zapr v1.2.3/go.mod h1:eIauM6P8qSvTw5o2ez6UEAfGjQKrxQTl5EoK+Qa2oG4= github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= @@ -253,3 +253,5 @@ gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +k8s.io/utils v0.0.0-20230406110748-d93618cff8a2 h1:qY1Ad8PODbnymg2pRbkyMT/ylpTrCM8P2RJ0yroCyIk= +k8s.io/utils v0.0.0-20230406110748-d93618cff8a2/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= diff --git a/stash/auth.go b/stash/auth.go index 3a20b9b7..d5599022 100644 --- a/stash/auth.go +++ b/stash/auth.go @@ -18,6 +18,7 @@ package stash import ( "errors" + "fmt" "net/url" "github.com/fluxcd/go-git-providers/gitprovider" @@ -33,13 +34,13 @@ func NewStashClient(username, token string, optFns ...gitprovider.ClientOption) opts, err := gitprovider.MakeClientOptions(optFns...) if err != nil { - return nil, err + return nil, fmt.Errorf("failed making client options: %w", err) } // Create a *http.Client using the transport chain client, err := gitprovider.BuildClientFromTransportChain(opts.GetTransportChain()) if err != nil { - return nil, err + return nil, fmt.Errorf("failed building client: %w", err) } if opts.Domain == nil { @@ -55,7 +56,7 @@ func NewStashClient(username, token string, optFns ...gitprovider.ClientOption) url, err = url.Parse(host) if err != nil { - return nil, err + return nil, fmt.Errorf("failed parsing host URL %q: %w", host, err) } var stashClient *Client @@ -66,7 +67,7 @@ func NewStashClient(username, token string, optFns ...gitprovider.ClientOption) } if err != nil { - return nil, err + return nil, fmt.Errorf("failed creating client: %w", err) } // By default, turn destructive actions off. But allow overrides. diff --git a/stash/client.go b/stash/client.go index e5d35af8..edf20de4 100644 --- a/stash/client.go +++ b/stash/client.go @@ -173,10 +173,6 @@ func NewClient(httpClient *http.Client, host string, header *http.Header, logger return nil, errors.New("host is required") } - if (logr.Logger{} == logger) { - return nil, errors.New("logger is required") - } - if httpClient == nil { httpClient = &http.Client{ Transport: defaultTransport, diff --git a/stash/client_test.go b/stash/client_test.go index 1d5958bd..d351112b 100644 --- a/stash/client_test.go +++ b/stash/client_test.go @@ -33,6 +33,7 @@ import ( "github.com/go-logr/logr" "github.com/go-logr/zapr" "go.uber.org/zap/zaptest" + "k8s.io/utils/pointer" ) func Test_NewClient(t *testing.T) { @@ -45,6 +46,7 @@ func Test_NewClient(t *testing.T) { transport http.RoundTripper log logr.Logger output string + err *string }{ { name: "no host", @@ -54,7 +56,7 @@ func Test_NewClient(t *testing.T) { client: &http.Client{}, transport: defaultTransport, log: logr.Discard(), - output: "host is required", + err: pointer.String("host is required"), }, { name: "wrong host", @@ -64,7 +66,7 @@ func Test_NewClient(t *testing.T) { client: &http.Client{}, transport: defaultTransport, log: logr.Discard(), - output: `failed to parse host https://local#.@*dev to url, parse "https://local#.@*dev": net/url: invalid userinfo`, + err: pointer.String(`failed to parse host https://local#.@*dev to url, parse "https://local#.@*dev": net/url: invalid userinfo`), }, { name: "default host", @@ -85,7 +87,6 @@ func Test_NewClient(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var val string c, err := NewClient(&http.Client{}, tt.host, tt.header, tt.log, func(c *Client) error { c.Client.HTTPClient = &http.Client{ Transport: tt.transport, @@ -94,13 +95,18 @@ func Test_NewClient(t *testing.T) { return nil }) if err != nil { - val = fmt.Sprintf("%s", err) - } - if c != nil { - val = c.BaseURL.String() - } - if val != tt.output { - t.Errorf("Expected %s, got %s", tt.output, val) + if tt.err == nil { + t.Errorf("did not expect an error but got %q", err) + } else if *tt.err != err.Error() { + t.Errorf("unexpected error: expected %q but got %q", *tt.err, err) + } + } else { + if tt.err != nil { + t.Errorf("expected error %q but got none", *tt.err) + } + if tt.output != c.BaseURL.String() { + t.Errorf("expected output %q but got %q", tt.output, c.BaseURL.String()) + } } }) }