Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: relinting, continued #274

Merged
merged 1 commit into from
Dec 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@

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

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) 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 @@
"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 @@
// 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 @@
// 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 @@
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 @@
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 @@
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 @@

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