Skip to content

Commit

Permalink
chore: relinting, continued
Browse files Browse the repository at this point in the history
This phase applies trivial relinting fixes, to keep the amount of
changes reviewable.

Non-trivial fixes are deferred to a follow-up PR.

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
  • Loading branch information
fredbi committed Dec 8, 2023
1 parent 1af6e90 commit ac3f5e8
Show file tree
Hide file tree
Showing 35 changed files with 196 additions and 154 deletions.
18 changes: 10 additions & 8 deletions bytestream.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,15 @@ func ByteStreamConsumer(opts ...byteStreamOpt) Consumer {
return errors.New("ByteStreamConsumer requires a reader") // early exit
}

close := defaultCloser //nolint:revive,predeclared
closer := defaultCloser
if vals.Close {
if cl, ok := reader.(io.Closer); ok {
close = cl.Close //nolint:revive
closer = cl.Close
}
}
//nolint:errcheck // closing a reader wouldn't fail.
defer close()
defer func() {
_ = closer()
}()

if wrtr, ok := data.(io.Writer); ok {
_, err := io.Copy(wrtr, reader)
Expand Down Expand Up @@ -109,14 +110,15 @@ func ByteStreamProducer(opts ...byteStreamOpt) Producer {
if writer == nil {
return errors.New("ByteStreamProducer requires a writer") // early exit
}
close := defaultCloser //nolint:revive,predeclared
closer := defaultCloser
if vals.Close {
if cl, ok := writer.(io.Closer); ok {
close = cl.Close //nolint:revive
closer = cl.Close
}
}
//nolint:errcheck // TODO: closing a writer would fail.
defer close()
defer func() {
_ = closer()
}()

if rc, ok := data.(io.ReadCloser); ok {
defer rc.Close()
Expand Down
3 changes: 1 addition & 2 deletions client/keepalive.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ func (d *drainingReadCloser) Close() error {
// If the reader side (a HTTP server) is misbehaving, it still may send
// some bytes, but the closer ignores them to keep the underling
// connection open.
//nolint:errcheck
io.Copy(io.Discard, d.rdr)
_, _ = io.Copy(io.Discard, d.rdr)
}
return d.rdr.Close()
}
2 changes: 1 addition & 1 deletion client/opentelemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func assertOpenTelemetrySubmit(t *testing.T, operation *runtime.ClientOperation,
attribute.String("http.route", "/kubernetes-clusters/{cluster_id}"),
attribute.String("http.method", http.MethodGet),
attribute.String("span.kind", trace.SpanKindClient.String()),
attribute.String("http.scheme", "https"),
attribute.String("http.scheme", schemeHTTPS),
// NOTE: this becomes http.response.status_code with semconv v1.21
attribute.Int("http.status_code", 490),
}, span.Attributes)
Expand Down
4 changes: 2 additions & 2 deletions client/opentracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type mockRuntime struct {
func (m *mockRuntime) Submit(operation *runtime.ClientOperation) (interface{}, error) {
_ = operation.Params.WriteToRequest(&m.req, nil)
_, _ = operation.Reader.ReadResponse(&tres{}, nil)
return nil, nil //nolint:nilnil
return map[string]interface{}{}, nil
}

func testOperation(ctx context.Context) *runtime.ClientOperation {
Expand All @@ -52,7 +52,7 @@ func testOperation(ctx context.Context) *runtime.ClientOperation {
PathPattern: "/kubernetes-clusters/{cluster_id}",
ProducesMediaTypes: []string{"application/json"},
ConsumesMediaTypes: []string{"application/json"},
Schemes: []string{"https"},
Schemes: []string{schemeHTTPS},
Reader: runtime.ClientResponseReaderFunc(func(runtime.ClientResponse, runtime.Consumer) (interface{}, error) {
return nil, nil
}),
Expand Down
9 changes: 5 additions & 4 deletions client/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package client

import (
"bytes"
"context"
"fmt"
"io"
"log"
Expand Down Expand Up @@ -317,13 +318,13 @@ DoneChoosingBodySource:

urlPath := path.Join(basePathURL.Path, pathPatternURL.Path)
for k, v := range r.pathParams {
urlPath = strings.Replace(urlPath, "{"+k+"}", url.PathEscape(v), -1) //nolint:gocritic
urlPath = strings.ReplaceAll(urlPath, "{"+k+"}", url.PathEscape(v))
}
if reinstateSlash {
urlPath = urlPath + "/" //nolint:gocritic
urlPath += "/"
}

req, err := http.NewRequest(r.method, urlPath, body) //nolint:noctx
req, err := http.NewRequestWithContext(context.Background(), r.method, urlPath, body)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -361,7 +362,7 @@ func (r *request) GetMethod() string {
func (r *request) GetPath() string {
path := r.pathPattern
for k, v := range r.pathParams {
path = strings.Replace(path, "{"+k+"}", v, -1) //nolint:gocritic
path = strings.ReplaceAll(path, "{"+k+"}", v)

Check warning on line 365 in client/request.go

View check run for this annotation

Codecov / codecov/patch

client/request.go#L365

Added line #L365 was not covered by tests
}
return path
}
Expand Down
2 changes: 1 addition & 1 deletion client/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ func TestGetBodyCallsBeforeRoundTrip(t *testing.T) {
}),
}

openAPIClient := New(hu.Host, "/", []string{"http"})
openAPIClient := New(hu.Host, "/", []string{schemeHTTP})
res, err := openAPIClient.Submit(operation)
require.NoError(t, err)

Expand Down
25 changes: 16 additions & 9 deletions client/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//nolint:goconst
package client

import (
Expand Down Expand Up @@ -40,6 +39,11 @@ import (
"github.com/opentracing/opentracing-go"
)

const (
schemeHTTP = "http"
schemeHTTPS = "https"
)

// TLSClientOptions to configure client authentication with mutual TLS
type TLSClientOptions struct {
// Certificate is the path to a PEM-encoded certificate to be used for
Expand Down Expand Up @@ -112,7 +116,9 @@ type TLSClientOptions struct {
// TLSClientAuth creates a tls.Config for mutual auth
func TLSClientAuth(opts TLSClientOptions) (*tls.Config, error) {
// create client tls config
cfg := &tls.Config{} //nolint:gosec
cfg := &tls.Config{
MinVersion: tls.VersionTLS12,
}

// load client cert if specified
if opts.Certificate != "" {
Expand Down Expand Up @@ -158,11 +164,12 @@ func TLSClientAuth(opts TLSClientOptions) (*tls.Config, error) {
// When no CA certificate is provided, default to the system cert pool
// that way when a request is made to a server known by the system trust store,
// the name is still verified
if opts.LoadedCA != nil { //nolint:gocritic
switch {
case opts.LoadedCA != nil:
caCertPool := basePool(opts.LoadedCAPool)
caCertPool.AddCert(opts.LoadedCA)
cfg.RootCAs = caCertPool
} else if opts.CA != "" {
case opts.CA != "":
// load ca cert
caCert, err := os.ReadFile(opts.CA)
if err != nil {
Expand All @@ -171,7 +178,7 @@ func TLSClientAuth(opts TLSClientOptions) (*tls.Config, error) {
caCertPool := basePool(opts.LoadedCAPool)
caCertPool.AppendCertsFromPEM(caCert)
cfg.RootCAs = caCertPool
} else if opts.LoadedCAPool != nil {
case opts.LoadedCAPool != nil:
cfg.RootCAs = opts.LoadedCAPool
}

Expand Down Expand Up @@ -227,7 +234,7 @@ type Runtime struct {
Host string
BasePath string
Formats strfmt.Registry
Context context.Context //nolint:containedctx
Context context.Context //nolint:containedctx // we precisely want this type to contain the request context

Debug bool
logger logger.Logger
Expand Down Expand Up @@ -316,7 +323,7 @@ func (r *Runtime) pickScheme(schemes []string) string {
if v := r.selectScheme(schemes); v != "" {
return v
}
return "http"
return schemeHTTP

Check warning on line 326 in client/runtime.go

View check run for this annotation

Codecov / codecov/patch

client/runtime.go#L326

Added line #L326 was not covered by tests
}

func (r *Runtime) selectScheme(schemes []string) string {
Expand All @@ -327,9 +334,9 @@ func (r *Runtime) selectScheme(schemes []string) string {

scheme := schemes[0]
// prefer https, but skip when not possible
if scheme != "https" && schLen > 1 {
if scheme != schemeHTTPS && schLen > 1 {
for _, sch := range schemes {
if sch == "https" {
if sch == schemeHTTPS {
scheme = sch
break
}
Expand Down
Loading

0 comments on commit ac3f5e8

Please sign in to comment.