From 309cc0dccf4ae076cb3b2d227742f72eb303eccf Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 21 May 2024 09:15:40 +0100 Subject: [PATCH 1/4] Interns the github.com/juju/http/v2 package This brings in the http/v2 package into the internal package. This is a long path to audit and intern external juju packages so we can keep them upto date easily. --- api/apiclient.go | 2 +- .../charmdownloader/charmdownloader.go | 2 +- apiserver/pubsub_test.go | 2 +- apiserver/registration_test.go | 2 +- apiserver/server_test.go | 2 +- apiserver/testing/fakeapi.go | 2 +- apiserver/testing/http.go | 2 +- apiserver/tools.go | 2 +- apiserver/tools_integration_test.go | 2 +- cmd/juju/cloud/updatepublicclouds.go | 2 +- cmd/juju/controller/register.go | 2 +- environs/simplestreams/datasource.go | 2 +- environs/simplestreams/export_test.go | 2 +- environs/simplestreams/testing/testing.go | 2 +- environs/sync/sync.go | 2 +- environs/sync/sync_test.go | 2 +- environs/testing/tools.go | 2 +- go.mod | 1 - go.sum | 2 - internal/charmhub/http.go | 2 +- internal/charmhub/http_test.go | 2 +- internal/downloader/utils.go | 2 +- internal/http/client.go | 334 ++++++++++++++++ internal/http/client_mock_test.go | 319 ++++++++++++++++ internal/http/client_test.go | 252 +++++++++++++ internal/http/clock_mock_test.go | 193 ++++++++++ internal/http/http.go | 49 +++ internal/http/http_mock_test.go | 79 ++++ internal/http/http_test.go | 90 +++++ internal/http/middleware.go | 305 +++++++++++++++ internal/http/middleware_test.go | 355 ++++++++++++++++++ internal/http/package_test.go | 18 + internal/http/tls.go | 92 +++++ internal/http/tls_test.go | 148 ++++++++ internal/mongo/open.go | 2 +- internal/provider/ec2/environ.go | 2 +- internal/provider/gce/environ.go | 2 +- internal/provider/gce/google/auth.go | 2 +- internal/provider/gce/google/auth_test.go | 2 +- internal/provider/gce/google/config.go | 2 +- .../gce/google/config_connection_test.go | 2 +- internal/provider/gce/google/conn.go | 2 +- internal/provider/gce/google/conn_test.go | 2 +- internal/provider/gce/google/testing_test.go | 2 +- internal/provider/lxd/provider.go | 2 +- internal/provider/openstack/client.go | 2 +- internal/provider/openstack/provider.go | 2 +- internal/s3client/http.go | 2 +- .../upgrades/upgradevalidation/validation.go | 2 +- internal/worker/httpserver/tls.go | 2 +- internal/worker/upgrader/upgrader.go | 2 +- 51 files changed, 2271 insertions(+), 40 deletions(-) create mode 100644 internal/http/client.go create mode 100644 internal/http/client_mock_test.go create mode 100644 internal/http/client_test.go create mode 100644 internal/http/clock_mock_test.go create mode 100644 internal/http/http.go create mode 100644 internal/http/http_mock_test.go create mode 100644 internal/http/http_test.go create mode 100644 internal/http/middleware.go create mode 100644 internal/http/middleware_test.go create mode 100644 internal/http/package_test.go create mode 100644 internal/http/tls.go create mode 100644 internal/http/tls_test.go diff --git a/api/apiclient.go b/api/apiclient.go index ae9495aadef..9cfa4c946ea 100644 --- a/api/apiclient.go +++ b/api/apiclient.go @@ -25,7 +25,7 @@ import ( "github.com/gorilla/websocket" "github.com/juju/clock" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "github.com/juju/utils/v4" "github.com/juju/utils/v4/parallel" diff --git a/apiserver/facades/controller/charmdownloader/charmdownloader.go b/apiserver/facades/controller/charmdownloader/charmdownloader.go index 6f52de3986f..be60e18de56 100644 --- a/apiserver/facades/controller/charmdownloader/charmdownloader.go +++ b/apiserver/facades/controller/charmdownloader/charmdownloader.go @@ -10,7 +10,7 @@ import ( "github.com/juju/charm/v13" "github.com/juju/clock" "github.com/juju/errors" - "github.com/juju/http/v2" + "github.com/juju/juju/internal/http" "github.com/juju/names/v5" apiservererrors "github.com/juju/juju/apiserver/errors" diff --git a/apiserver/pubsub_test.go b/apiserver/pubsub_test.go index 4f164c022e6..bd4a8a441ec 100644 --- a/apiserver/pubsub_test.go +++ b/apiserver/pubsub_test.go @@ -12,7 +12,7 @@ import ( "time" "github.com/gorilla/websocket" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/loggo/v2" "github.com/juju/names/v5" "github.com/juju/pubsub/v2" diff --git a/apiserver/registration_test.go b/apiserver/registration_test.go index 509a8ecbd5d..50089da3c43 100644 --- a/apiserver/registration_test.go +++ b/apiserver/registration_test.go @@ -14,7 +14,7 @@ import ( "strings" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" "github.com/juju/testing/httptesting" "go.uber.org/mock/gomock" diff --git a/apiserver/server_test.go b/apiserver/server_test.go index c5cbf9669f5..8f918900595 100644 --- a/apiserver/server_test.go +++ b/apiserver/server_test.go @@ -12,7 +12,7 @@ import ( "github.com/gorilla/websocket" jujuerrors "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/loggo/v2" "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" diff --git a/apiserver/testing/fakeapi.go b/apiserver/testing/fakeapi.go index 092743d1cfc..a7b45def495 100644 --- a/apiserver/testing/fakeapi.go +++ b/apiserver/testing/fakeapi.go @@ -13,7 +13,7 @@ import ( "github.com/bmizerany/pat" "github.com/gorilla/websocket" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/observer" diff --git a/apiserver/testing/http.go b/apiserver/testing/http.go index 2b8f8095c66..9741fb58dbb 100644 --- a/apiserver/testing/http.go +++ b/apiserver/testing/http.go @@ -7,7 +7,7 @@ import ( "io" "net/http" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" "github.com/juju/testing/httptesting" gc "gopkg.in/check.v1" diff --git a/apiserver/tools.go b/apiserver/tools.go index c70a0cefa96..af5dbfa21cc 100644 --- a/apiserver/tools.go +++ b/apiserver/tools.go @@ -16,7 +16,7 @@ import ( "github.com/im7mortal/kmutex" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/version/v2" "github.com/juju/juju/apiserver/common" diff --git a/apiserver/tools_integration_test.go b/apiserver/tools_integration_test.go index b3237a4c6e8..cd710202d3b 100644 --- a/apiserver/tools_integration_test.go +++ b/apiserver/tools_integration_test.go @@ -14,7 +14,7 @@ import ( "github.com/go-macaroon-bakery/macaroon-bakery/v3/bakery" "github.com/go-macaroon-bakery/macaroon-bakery/v3/httpbakery" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" diff --git a/cmd/juju/cloud/updatepublicclouds.go b/cmd/juju/cloud/updatepublicclouds.go index 3b0b4bd42bd..ae416667aa0 100644 --- a/cmd/juju/cloud/updatepublicclouds.go +++ b/cmd/juju/cloud/updatepublicclouds.go @@ -15,7 +15,7 @@ import ( "github.com/juju/cmd/v4" "github.com/juju/collections/set" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "golang.org/x/crypto/openpgp" "golang.org/x/crypto/openpgp/clearsign" diff --git a/cmd/juju/controller/register.go b/cmd/juju/controller/register.go index 12abe1bbdf1..94f43e36d69 100644 --- a/cmd/juju/controller/register.go +++ b/cmd/juju/controller/register.go @@ -25,7 +25,7 @@ import ( "github.com/juju/collections/set" "github.com/juju/errors" "github.com/juju/gnuflag" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "golang.org/x/crypto/nacl/secretbox" "golang.org/x/crypto/ssh/terminal" diff --git a/environs/simplestreams/datasource.go b/environs/simplestreams/datasource.go index edff2f8be33..d1afce4beca 100644 --- a/environs/simplestreams/datasource.go +++ b/environs/simplestreams/datasource.go @@ -14,7 +14,7 @@ import ( "github.com/juju/clock" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/retry" "github.com/juju/utils/v4" diff --git a/environs/simplestreams/export_test.go b/environs/simplestreams/export_test.go index 989cf350c84..cf943dad409 100644 --- a/environs/simplestreams/export_test.go +++ b/environs/simplestreams/export_test.go @@ -3,7 +3,7 @@ package simplestreams -import jujuhttp "github.com/juju/http/v2" +import jujuhttp "github.com/juju/juju/internal/http" func ExtractCatalogsForProducts(metadata CloudMetadata, productIds []string) []MetadataCatalog { return metadata.extractCatalogsForProducts(productIds) diff --git a/environs/simplestreams/testing/testing.go b/environs/simplestreams/testing/testing.go index db486150366..94d038be18e 100644 --- a/environs/simplestreams/testing/testing.go +++ b/environs/simplestreams/testing/testing.go @@ -12,7 +12,7 @@ import ( "net/http" "strings" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" diff --git a/environs/sync/sync.go b/environs/sync/sync.go index faf512417d7..f769b3615c6 100644 --- a/environs/sync/sync.go +++ b/environs/sync/sync.go @@ -11,7 +11,7 @@ import ( "path/filepath" "github.com/juju/errors" - "github.com/juju/http/v2" + "github.com/juju/juju/internal/http" "github.com/juju/utils/v4" "github.com/juju/version/v2" diff --git a/environs/sync/sync_test.go b/environs/sync/sync_test.go index 23fc668ab38..3e08620ffd1 100644 --- a/environs/sync/sync_test.go +++ b/environs/sync/sync_test.go @@ -16,7 +16,7 @@ import ( "path/filepath" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" jujutesting "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/utils/v4/tar" diff --git a/environs/testing/tools.go b/environs/testing/tools.go index b73caa71435..34a0bfbb54f 100644 --- a/environs/testing/tools.go +++ b/environs/testing/tools.go @@ -13,7 +13,7 @@ import ( "strings" "github.com/juju/collections/set" - "github.com/juju/http/v2" + "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" "github.com/juju/version/v2" gc "gopkg.in/check.v1" diff --git a/go.mod b/go.mod index ba0dd2046ac..9c7ece87b75 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,6 @@ require ( github.com/juju/gnuflag v1.0.0 github.com/juju/gojsonschema v1.0.0 github.com/juju/gomaasapi/v2 v2.2.0 - github.com/juju/http/v2 v2.0.1 github.com/juju/idmclient/v2 v2.0.0 github.com/juju/jsonschema v1.0.0 github.com/juju/loggo v1.0.0 diff --git a/go.sum b/go.sum index c77e64bd085..1c2305dbeef 100644 --- a/go.sum +++ b/go.sum @@ -519,8 +519,6 @@ github.com/juju/gojsonschema v1.0.0 h1:v0hVxinRWko/SwRYCZ99fBH20Q1JtDqKtplcovXew github.com/juju/gojsonschema v1.0.0/go.mod h1:w3BDUH3gGgrcrAyvEbBy3lNb1vmdqG31UMn3njL8oGc= github.com/juju/gomaasapi/v2 v2.2.0 h1:vaYeEKr0mQXsM38/zfWqCrYDG8cYhHRkfTQMgkJGHdU= github.com/juju/gomaasapi/v2 v2.2.0/go.mod h1:ZsohFbU4xShV1aSQYQ21hR1lKj7naNGY0SPuyelcUmk= -github.com/juju/http/v2 v2.0.1 h1:cTt8RHvdVv9uZFmAb/7zQrRitT1bgywEDOf8nLpYd8M= -github.com/juju/http/v2 v2.0.1/go.mod h1:V9dcD7DkT4xkwWjXpFr1C0ZNq7gG+eN9f+Mi2c7GnCs= github.com/juju/httpprof v0.0.0-20141217160036-14bf14c30767/go.mod h1:+MaLYz4PumRkkyHYeXJ2G5g5cIW0sli2bOfpmbaMV/g= github.com/juju/idmclient/v2 v2.0.0 h1:PsGa092JGy6iFNHZCcao+bigVsTyz1C+tHNRdYmKvuE= github.com/juju/idmclient/v2 v2.0.0/go.mod h1:EOiFbPmnkqKvCUS/DHpDRWhL7eKF0AJaTvMjIYlIUak= diff --git a/internal/charmhub/http.go b/internal/charmhub/http.go index 3d7c11456a7..fd43f3482d5 100644 --- a/internal/charmhub/http.go +++ b/internal/charmhub/http.go @@ -16,7 +16,7 @@ import ( "github.com/juju/clock" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/retry" "gopkg.in/httprequest.v1" diff --git a/internal/charmhub/http_test.go b/internal/charmhub/http_test.go index b94f18ab014..b8f61be8bf8 100644 --- a/internal/charmhub/http_test.go +++ b/internal/charmhub/http_test.go @@ -14,7 +14,7 @@ import ( "time" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/testing" jc "github.com/juju/testing/checkers" "go.uber.org/mock/gomock" diff --git a/internal/downloader/utils.go b/internal/downloader/utils.go index e23af8de896..feb768649c2 100644 --- a/internal/downloader/utils.go +++ b/internal/downloader/utils.go @@ -10,7 +10,7 @@ import ( "os" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/utils/v4" corelogger "github.com/juju/juju/core/logger" diff --git a/internal/http/client.go b/internal/http/client.go new file mode 100644 index 00000000000..3497029d061 --- /dev/null +++ b/internal/http/client.go @@ -0,0 +1,334 @@ +// Copyright 2020 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package http + +import ( + "context" + "crypto/tls" + "crypto/x509" + "net/http" + "net/http/httptrace" + "net/http/httputil" + "time" + + "github.com/juju/clock" + "github.com/juju/errors" + + "github.com/juju/juju/core/logger" + internallogger "github.com/juju/juju/internal/logger" +) + +// NOTE: Once we refactor the juju tests enough that they do not use +// a RoundTripper on the DefaultTransport, NewClient can always return +// a Client with a locally constructed Transport via NewHttpTLSTransport +// and init() will no longer be needed. +// +// https://bugs.launchpad.net/juju/+bug/1888888 +// +// TODO (stickupkid) This is terrible, I'm not kidding! This isn't yours to +// touch! +func init() { + defaultTransport := http.DefaultTransport.(*http.Transport) + // Call the DialContextMiddleware for the DefaultTransport to + // facilitate testing use of allowOutgoingAccess. + defaultTransport = DialContextMiddleware(NewLocalDialBreaker(true))(defaultTransport) + // Call our own proxy function with the DefaultTransport. + http.DefaultTransport = ProxyMiddleware(defaultTransport) +} + +// HTTPClient represents an http.Client. +type HTTPClient interface { + Do(req *http.Request) (*http.Response, error) +} + +// Option to be passed into the transport construction to customize the +// default transport. +type Option func(*options) + +type options struct { + caCertificates []string + cookieJar http.CookieJar + disableKeepAlives bool + skipHostnameVerification bool + tlsHandshakeTimeout time.Duration + middlewares []TransportMiddleware + httpClient *http.Client + logger logger.Logger + requestRecorder RequestRecorder + retryPolicy *RetryPolicy +} + +// WithCACertificates contains Authority certificates to be used to validate +// certificates of cloud infrastructure components. +// The contents are Base64 encoded x.509 certs. +func WithCACertificates(value ...string) Option { + return func(opt *options) { + opt.caCertificates = value + } +} + +// WithCookieJar is used to insert relevant cookies into every +// outbound Request and is updated with the cookie values +// of every inbound Response. The Jar is consulted for every +// redirect that the Client follows. +// +// If Jar is nil, cookies are only sent if they are explicitly +// set on the Request. +func WithCookieJar(value http.CookieJar) Option { + return func(opt *options) { + opt.cookieJar = value + } +} + +// WithDisableKeepAlives will disable HTTP keep alives, not TCP keep alives. +// Disabling HTTP keep alives will only use the connection to the server for a +// single HTTP request, slowing down subsequent requests and creating a lot of +// garbage for the collector. +func WithDisableKeepAlives(value bool) Option { + return func(opt *options) { + opt.disableKeepAlives = value + } +} + +// WithSkipHostnameVerification will skip hostname verification on the TLS/SSL +// certificates. +func WithSkipHostnameVerification(value bool) Option { + return func(opt *options) { + opt.skipHostnameVerification = value + } +} + +// WithTLSHandshakeTimeout will modify how long a TLS handshake should take. +// Setting the value to zero will mean that no timeout will occur. +func WithTLSHandshakeTimeout(value time.Duration) Option { + return func(opt *options) { + opt.tlsHandshakeTimeout = value + } +} + +// WithTransportMiddlewares allows the wrapping or modification of the existing +// transport for a given client. +// In an ideal world, all transports should be cloned to prevent the +// modification of an existing client transport. +func WithTransportMiddlewares(middlewares ...TransportMiddleware) Option { + return func(opt *options) { + opt.middlewares = middlewares + } +} + +// WithHTTPClient allows to define the http.Client to use. +func WithHTTPClient(value *http.Client) Option { + return func(opt *options) { + opt.httpClient = value + } +} + +// WithLogger defines a logger to use with the client. +// +// It is recommended that you create a child logger to allow disabling of the +// trace logging to prevent log flooding. +func WithLogger(value logger.Logger) Option { + return func(opt *options) { + opt.logger = value + } +} + +// WithRequestRecorder specifies a RequestRecorder used for recording outgoing +// http requests regardless of whether they succeeded or failed. +func WithRequestRecorder(value RequestRecorder) Option { + return func(opt *options) { + opt.requestRecorder = value + } +} + +// WithRequestRetrier specifies a request retrying policy. +func WithRequestRetrier(value RetryPolicy) Option { + return func(opt *options) { + opt.retryPolicy = &value + } +} + +// Create a options instance with default values. +func newOptions() *options { + // In this case, use a default http.Client. + // Ideally we should always use the NewHTTPTLSTransport, + // however test suites such as JujuConnSuite and some facade + // tests rely on settings to the http.DefaultTransport for + // tests to run with different protocol scheme such as "test" + // and some replace the RoundTripper to answer test scenarios. + // + // https://bugs.launchpad.net/juju/+bug/1888888 + defaultCopy := *http.DefaultClient + + return &options{ + tlsHandshakeTimeout: 20 * time.Second, + skipHostnameVerification: false, + middlewares: []TransportMiddleware{ + DialContextMiddleware(NewLocalDialBreaker(true)), + FileProtocolMiddleware, + ProxyMiddleware, + }, + httpClient: &defaultCopy, + logger: internallogger.GetLogger("http"), + } +} + +// Client represents an http client. +type Client struct { + HTTPClient + + logger logger.Logger +} + +// NewClient returns a new juju http client defined +// by the given config. +func NewClient(options ...Option) *Client { + opts := newOptions() + for _, option := range options { + option(opts) + } + + client := opts.httpClient + transport := NewHTTPTLSTransport(TransportConfig{ + DisableKeepAlives: opts.disableKeepAlives, + TLSHandshakeTimeout: opts.tlsHandshakeTimeout, + Middlewares: opts.middlewares, + }) + switch { + case len(opts.caCertificates) > 0: + transport = transportWithCerts(transport, opts.caCertificates, opts.skipHostnameVerification) + case opts.skipHostnameVerification: + transport = transportWithSkipVerify(transport, opts.skipHostnameVerification) + } + + if opts.requestRecorder != nil { + client.Transport = roundTripRecorder{ + requestRecorder: opts.requestRecorder, + wrappedRoundTripper: transport, + } + } else { + client.Transport = transport + } + + // Ensure we add the retry middleware after request recorder if there is + // one, to ensure that we get all the logging at the right level. + if opts.retryPolicy != nil { + client.Transport = makeRetryMiddleware( + client.Transport, + *opts.retryPolicy, + clock.WallClock, + opts.logger, + ) + } + + if opts.cookieJar != nil { + client.Jar = opts.cookieJar + } + return &Client{ + HTTPClient: client, + logger: opts.logger, + } +} + +func transportWithSkipVerify(defaultTransport *http.Transport, skipHostnameVerify bool) *http.Transport { + transport := defaultTransport + // We know that the DefaultHTTPTransport doesn't create a tls.Config here + // so we can safely do that here. + transport.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: skipHostnameVerify, + } + // We're creating a new tls.Config, HTTP/2 requests will not work, force the + // client to create a HTTP/2 requests. + transport.ForceAttemptHTTP2 = true + return transport +} + +func transportWithCerts(defaultTransport *http.Transport, caCerts []string, skipHostnameVerify bool) *http.Transport { + pool := x509.NewCertPool() + for _, cert := range caCerts { + pool.AppendCertsFromPEM([]byte(cert)) + } + + tlsConfig := SecureTLSConfig() + tlsConfig.RootCAs = pool + tlsConfig.InsecureSkipVerify = skipHostnameVerify + + transport := defaultTransport + transport.TLSClientConfig = tlsConfig + + // We're creating a new tls.Config, HTTP/2 requests will not work, force the + // client to create a HTTP/2 requests. + transport.ForceAttemptHTTP2 = true + return transport +} + +// Client returns the underlying http.Client. Used in testing +// only. +func (c *Client) Client() *http.Client { + return c.HTTPClient.(*http.Client) +} + +// Get issues a GET to the specified URL. It mimics the net/http Get, +// but allows for enhanced debugging. +// +// When err is nil, resp always contains a non-nil resp.Body. +// Caller should close resp.Body when done reading from it. +func (c *Client) Get(ctx context.Context, path string) (resp *http.Response, err error) { + req, err := http.NewRequestWithContext(ctx, "GET", path, nil) + if err != nil { + return nil, errors.Trace(err) + } + + if err := c.traceRequest(req, path); err != nil { + // No need to fail, but let user know we're + // not tracing the client GET. + err = errors.Annotatef(err, "setup of http client tracing failed") + c.logger.Tracef("%s", err) + } + return c.Do(req) +} + +// traceRequest enabled debugging on the http request if +// log level for ths package is set to Trace. Otherwise it +// returns with no change to the request. +func (c *Client) traceRequest(req *http.Request, url string) error { + if !c.logger.IsLevelEnabled(logger.TRACE) { + return nil + } + + dump, err := httputil.DumpRequestOut(req, true) + if err != nil { + return errors.Trace(err) + } + c.logger.Tracef("request for %q: %q", url, dump) + trace := &httptrace.ClientTrace{ + DNSStart: func(info httptrace.DNSStartInfo) { + c.logger.Tracef("%s DNS Start: %q", url, info.Host) + }, + DNSDone: func(dnsInfo httptrace.DNSDoneInfo) { + c.logger.Tracef("%s DNS Info: %+v\n", url, dnsInfo) + }, + ConnectDone: func(network, addr string, err error) { + c.logger.Tracef("%s Connection Done: network %q, addr %q, err %q", url, network, addr, err) + }, + GetConn: func(hostPort string) { + c.logger.Tracef("%s Get Conn: %q", url, hostPort) + }, + GotConn: func(connInfo httptrace.GotConnInfo) { + c.logger.Tracef("%s Got Conn: %+v", url, connInfo) + }, + TLSHandshakeStart: func() { + c.logger.Tracef("%s TLS Handshake Start", url) + }, + TLSHandshakeDone: func(st tls.ConnectionState, err error) { + c.logger.Tracef("%s TLS Handshake Done: complete %t, verified chains %d, server name %q", + url, + st.HandshakeComplete, + len(st.VerifiedChains), + st.ServerName) + }, + } + *req = *req.WithContext(httptrace.WithClientTrace(req.Context(), trace)) + return nil +} diff --git a/internal/http/client_mock_test.go b/internal/http/client_mock_test.go new file mode 100644 index 00000000000..a46fa3d5c94 --- /dev/null +++ b/internal/http/client_mock_test.go @@ -0,0 +1,319 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/internal/http (interfaces: HTTPClient,RequestRecorder,Logger) +// +// Generated by this command: +// +// mockgen -typed -package http -destination client_mock_test.go github.com/juju/juju/internal/http HTTPClient,RequestRecorder,Logger +// + +// Package http is a generated GoMock package. +package http + +import ( + http "net/http" + url "net/url" + reflect "reflect" + time "time" + + gomock "go.uber.org/mock/gomock" +) + +// MockHTTPClient is a mock of HTTPClient interface. +type MockHTTPClient struct { + ctrl *gomock.Controller + recorder *MockHTTPClientMockRecorder +} + +// MockHTTPClientMockRecorder is the mock recorder for MockHTTPClient. +type MockHTTPClientMockRecorder struct { + mock *MockHTTPClient +} + +// NewMockHTTPClient creates a new mock instance. +func NewMockHTTPClient(ctrl *gomock.Controller) *MockHTTPClient { + mock := &MockHTTPClient{ctrl: ctrl} + mock.recorder = &MockHTTPClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockHTTPClient) EXPECT() *MockHTTPClientMockRecorder { + return m.recorder +} + +// Do mocks base method. +func (m *MockHTTPClient) Do(arg0 *http.Request) (*http.Response, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Do", arg0) + ret0, _ := ret[0].(*http.Response) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Do indicates an expected call of Do. +func (mr *MockHTTPClientMockRecorder) Do(arg0 any) *MockHTTPClientDoCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Do", reflect.TypeOf((*MockHTTPClient)(nil).Do), arg0) + return &MockHTTPClientDoCall{Call: call} +} + +// MockHTTPClientDoCall wrap *gomock.Call +type MockHTTPClientDoCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockHTTPClientDoCall) Return(arg0 *http.Response, arg1 error) *MockHTTPClientDoCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockHTTPClientDoCall) Do(f func(*http.Request) (*http.Response, error)) *MockHTTPClientDoCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockHTTPClientDoCall) DoAndReturn(f func(*http.Request) (*http.Response, error)) *MockHTTPClientDoCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockRequestRecorder is a mock of RequestRecorder interface. +type MockRequestRecorder struct { + ctrl *gomock.Controller + recorder *MockRequestRecorderMockRecorder +} + +// MockRequestRecorderMockRecorder is the mock recorder for MockRequestRecorder. +type MockRequestRecorderMockRecorder struct { + mock *MockRequestRecorder +} + +// NewMockRequestRecorder creates a new mock instance. +func NewMockRequestRecorder(ctrl *gomock.Controller) *MockRequestRecorder { + mock := &MockRequestRecorder{ctrl: ctrl} + mock.recorder = &MockRequestRecorderMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockRequestRecorder) EXPECT() *MockRequestRecorderMockRecorder { + return m.recorder +} + +// Record mocks base method. +func (m *MockRequestRecorder) Record(arg0 string, arg1 *url.URL, arg2 *http.Response, arg3 time.Duration) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Record", arg0, arg1, arg2, arg3) +} + +// Record indicates an expected call of Record. +func (mr *MockRequestRecorderMockRecorder) Record(arg0, arg1, arg2, arg3 any) *MockRequestRecorderRecordCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Record", reflect.TypeOf((*MockRequestRecorder)(nil).Record), arg0, arg1, arg2, arg3) + return &MockRequestRecorderRecordCall{Call: call} +} + +// MockRequestRecorderRecordCall wrap *gomock.Call +type MockRequestRecorderRecordCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockRequestRecorderRecordCall) Return() *MockRequestRecorderRecordCall { + c.Call = c.Call.Return() + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockRequestRecorderRecordCall) Do(f func(string, *url.URL, *http.Response, time.Duration)) *MockRequestRecorderRecordCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockRequestRecorderRecordCall) DoAndReturn(f func(string, *url.URL, *http.Response, time.Duration)) *MockRequestRecorderRecordCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// RecordError mocks base method. +func (m *MockRequestRecorder) RecordError(arg0 string, arg1 *url.URL, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RecordError", arg0, arg1, arg2) +} + +// RecordError indicates an expected call of RecordError. +func (mr *MockRequestRecorderMockRecorder) RecordError(arg0, arg1, arg2 any) *MockRequestRecorderRecordErrorCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RecordError", reflect.TypeOf((*MockRequestRecorder)(nil).RecordError), arg0, arg1, arg2) + return &MockRequestRecorderRecordErrorCall{Call: call} +} + +// MockRequestRecorderRecordErrorCall wrap *gomock.Call +type MockRequestRecorderRecordErrorCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockRequestRecorderRecordErrorCall) Return() *MockRequestRecorderRecordErrorCall { + c.Call = c.Call.Return() + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockRequestRecorderRecordErrorCall) Do(f func(string, *url.URL, error)) *MockRequestRecorderRecordErrorCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockRequestRecorderRecordErrorCall) DoAndReturn(f func(string, *url.URL, error)) *MockRequestRecorderRecordErrorCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockLogger is a mock of Logger interface. +type MockLogger struct { + ctrl *gomock.Controller + recorder *MockLoggerMockRecorder +} + +// MockLoggerMockRecorder is the mock recorder for MockLogger. +type MockLoggerMockRecorder struct { + mock *MockLogger +} + +// NewMockLogger creates a new mock instance. +func NewMockLogger(ctrl *gomock.Controller) *MockLogger { + mock := &MockLogger{ctrl: ctrl} + mock.recorder = &MockLoggerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockLogger) EXPECT() *MockLoggerMockRecorder { + return m.recorder +} + +// Errorf mocks base method. +func (m *MockLogger) Errorf(arg0 string, arg1 ...any) { + m.ctrl.T.Helper() + varargs := []any{arg0} + for _, a := range arg1 { + varargs = append(varargs, a) + } + m.ctrl.Call(m, "Errorf", varargs...) +} + +// Errorf indicates an expected call of Errorf. +func (mr *MockLoggerMockRecorder) Errorf(arg0 any, arg1 ...any) *MockLoggerErrorfCall { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0}, arg1...) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Errorf", reflect.TypeOf((*MockLogger)(nil).Errorf), varargs...) + return &MockLoggerErrorfCall{Call: call} +} + +// MockLoggerErrorfCall wrap *gomock.Call +type MockLoggerErrorfCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockLoggerErrorfCall) Return() *MockLoggerErrorfCall { + c.Call = c.Call.Return() + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockLoggerErrorfCall) Do(f func(string, ...any)) *MockLoggerErrorfCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockLoggerErrorfCall) DoAndReturn(f func(string, ...any)) *MockLoggerErrorfCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// IsTraceEnabled mocks base method. +func (m *MockLogger) IsTraceEnabled() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsTraceEnabled") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsTraceEnabled indicates an expected call of IsTraceEnabled. +func (mr *MockLoggerMockRecorder) IsTraceEnabled() *MockLoggerIsTraceEnabledCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsTraceEnabled", reflect.TypeOf((*MockLogger)(nil).IsTraceEnabled)) + return &MockLoggerIsTraceEnabledCall{Call: call} +} + +// MockLoggerIsTraceEnabledCall wrap *gomock.Call +type MockLoggerIsTraceEnabledCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockLoggerIsTraceEnabledCall) Return(arg0 bool) *MockLoggerIsTraceEnabledCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockLoggerIsTraceEnabledCall) Do(f func() bool) *MockLoggerIsTraceEnabledCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockLoggerIsTraceEnabledCall) DoAndReturn(f func() bool) *MockLoggerIsTraceEnabledCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// Tracef mocks base method. +func (m *MockLogger) Tracef(arg0 string, arg1 ...any) { + m.ctrl.T.Helper() + varargs := []any{arg0} + for _, a := range arg1 { + varargs = append(varargs, a) + } + m.ctrl.Call(m, "Tracef", varargs...) +} + +// Tracef indicates an expected call of Tracef. +func (mr *MockLoggerMockRecorder) Tracef(arg0 any, arg1 ...any) *MockLoggerTracefCall { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0}, arg1...) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Tracef", reflect.TypeOf((*MockLogger)(nil).Tracef), varargs...) + return &MockLoggerTracefCall{Call: call} +} + +// MockLoggerTracefCall wrap *gomock.Call +type MockLoggerTracefCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockLoggerTracefCall) Return() *MockLoggerTracefCall { + c.Call = c.Call.Return() + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockLoggerTracefCall) Do(f func(string, ...any)) *MockLoggerTracefCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockLoggerTracefCall) DoAndReturn(f func(string, ...any)) *MockLoggerTracefCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/internal/http/client_test.go b/internal/http/client_test.go new file mode 100644 index 00000000000..507e838fb74 --- /dev/null +++ b/internal/http/client_test.go @@ -0,0 +1,252 @@ +// Copyright 2020 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package http + +import ( + "bytes" + "context" + "encoding/pem" + "fmt" + "net/http" + "net/http/cookiejar" + "net/http/httptest" + "net/url" + "time" + + "github.com/juju/testing" + jc "github.com/juju/testing/checkers" + "go.uber.org/mock/gomock" + gc "gopkg.in/check.v1" +) + +type clientSuite struct{} + +var _ = gc.Suite(&clientSuite{}) + +func (s *clientSuite) TestNewClient(c *gc.C) { + client := NewClient() + c.Assert(client, gc.NotNil) +} + +type httpSuite struct { + testing.IsolationSuite + server *httptest.Server +} + +var _ = gc.Suite(&httpSuite{}) + +func (s *httpSuite) SetUpTest(c *gc.C) { + s.IsolationSuite.SetUpTest(c) + s.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) +} + +func (s *httpSuite) TestInsecureClientAllowAccess(c *gc.C) { + client := NewClient(WithSkipHostnameVerification(true)) + _, err := client.Get(context.TODO(), s.server.URL) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *httpSuite) TestSecureClientAllowAccess(c *gc.C) { + client := NewClient() + _, err := client.Get(context.TODO(), s.server.URL) + c.Assert(err, jc.ErrorIsNil) +} + +// NewClient with a default config used to overwrite http.DefaultClient.Jar +// field; add a regression test for that. +func (s *httpSuite) TestDefaultClientJarNotOverwritten(c *gc.C) { + oldJar := http.DefaultClient.Jar + + jar, err := cookiejar.New(nil) + c.Assert(err, jc.ErrorIsNil) + + client := NewClient(WithCookieJar(jar)) + + hc := client.HTTPClient.(*http.Client) + c.Assert(hc.Jar, gc.Equals, jar) + c.Assert(http.DefaultClient.Jar, gc.Not(gc.Equals), jar) + c.Assert(http.DefaultClient.Jar, gc.Equals, oldJar) + + http.DefaultClient.Jar = oldJar +} + +func (s *httpSuite) TestRequestRecorder(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + dummyServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + _, _ = fmt.Fprintln(res, "they are listening...") + })) + defer dummyServer.Close() + + validTarget := fmt.Sprintf("%s/tin/foil", dummyServer.URL) + validTargetURL, err := url.Parse(validTarget) + c.Assert(err, jc.ErrorIsNil) + + invalidTarget := "btc://secret/wallet" + invalidTargetURL, err := url.Parse(invalidTarget) + c.Assert(err, jc.ErrorIsNil) + + recorder := NewMockRequestRecorder(ctrl) + recorder.EXPECT().Record("GET", validTargetURL, gomock.AssignableToTypeOf(&http.Response{}), gomock.AssignableToTypeOf(time.Duration(42))) + recorder.EXPECT().RecordError("PUT", invalidTargetURL, gomock.Any()) + + client := NewClient(WithRequestRecorder(recorder)) + res, err := client.Get(context.TODO(), validTarget) + c.Assert(err, jc.ErrorIsNil) + defer res.Body.Close() + + req, err := http.NewRequestWithContext(context.TODO(), "PUT", invalidTarget, nil) + c.Assert(err, jc.ErrorIsNil) + _, err = client.Do(req) + c.Assert(err, gc.Not(jc.ErrorIsNil)) +} + +func (s *httpSuite) TestRetry(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + attempts := 0 + retries := 3 + dummyServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + if attempts < retries-1 { + res.WriteHeader(http.StatusBadGateway) + } else { + res.WriteHeader(http.StatusOK) + } + attempts++ + _, _ = fmt.Fprintln(res, "they are listening...") + })) + defer dummyServer.Close() + + validTarget := fmt.Sprintf("%s/tin/foil", dummyServer.URL) + validTargetURL, err := url.Parse(validTarget) + c.Assert(err, jc.ErrorIsNil) + + recorder := NewMockRequestRecorder(ctrl) + recorder.EXPECT().Record("GET", validTargetURL, gomock.AssignableToTypeOf(&http.Response{}), gomock.AssignableToTypeOf(time.Duration(42))).Times(retries) + + client := NewClient( + // We can use the request recorder to monitor how many retries have been + // made. + WithRequestRecorder(recorder), + WithRequestRetrier(RetryPolicy{ + Delay: time.Nanosecond, + Attempts: retries, + MaxDelay: time.Minute, + }), + ) + res, err := client.Get(context.TODO(), validTarget) + c.Assert(err, jc.ErrorIsNil) + defer res.Body.Close() +} + +func (s *httpSuite) TestRetryExceeded(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + retries := 3 + dummyServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + res.WriteHeader(http.StatusBadGateway) + _, _ = fmt.Fprintln(res, "they are listening...") + })) + defer dummyServer.Close() + + validTarget := fmt.Sprintf("%s/tin/foil", dummyServer.URL) + validTargetURL, err := url.Parse(validTarget) + c.Assert(err, jc.ErrorIsNil) + + recorder := NewMockRequestRecorder(ctrl) + recorder.EXPECT().Record("GET", validTargetURL, gomock.AssignableToTypeOf(&http.Response{}), gomock.AssignableToTypeOf(time.Duration(42))).Times(retries) + + client := NewClient( + // We can use the request recorder to monitor how many retries have been + // made. + WithRequestRecorder(recorder), + WithRequestRetrier(RetryPolicy{ + Delay: time.Nanosecond, + Attempts: retries, + MaxDelay: time.Minute, + }), + ) + _, err = client.Get(context.TODO(), validTarget) + c.Assert(err, gc.ErrorMatches, `.*attempt count exceeded: retryable error`) +} + +type httpTLSServerSuite struct { + testing.IsolationSuite + server *httptest.Server +} + +var _ = gc.Suite(&httpTLSServerSuite{}) + +func (s *httpTLSServerSuite) SetUpTest(c *gc.C) { + s.IsolationSuite.SetUpTest(c) + // NewTLSServer returns a server which serves TLS, but + // its certificates are not validated by the default + // OS certificates, so any HTTPS request will fail + // unless a non-validating client is used. + s.server = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) +} + +func (s *httpTLSServerSuite) TearDownTest(c *gc.C) { + if s.server != nil { + s.server.Close() + } + s.IsolationSuite.TearDownTest(c) +} + +func (s *httpTLSServerSuite) TestValidatingClientGetter(c *gc.C) { + client := NewClient() + _, err := client.Get(context.TODO(), s.server.URL) + c.Assert(err, gc.ErrorMatches, "(.|\n)*x509: certificate signed by unknown authority") +} + +func (s *httpTLSServerSuite) TestNonValidatingClientGetter(c *gc.C) { + client := NewClient(WithSkipHostnameVerification(true)) + resp, err := client.Get(context.TODO(), s.server.URL) + c.Assert(err, gc.IsNil) + _ = resp.Body.Close() + c.Assert(resp.StatusCode, gc.Equals, http.StatusOK) +} + +func (s *httpTLSServerSuite) TestGetHTTPClientWithCertsVerify(c *gc.C) { + s.testGetHTTPClientWithCerts(c, true) +} + +func (s *httpTLSServerSuite) TestGetHTTPClientWithCertsNoVerify(c *gc.C) { + s.testGetHTTPClientWithCerts(c, false) +} + +func (s *httpTLSServerSuite) testGetHTTPClientWithCerts(c *gc.C, skip bool) { + caPEM := new(bytes.Buffer) + err := pem.Encode(caPEM, &pem.Block{ + Type: "CERTIFICATE", + Bytes: s.server.Certificate().Raw, + }) + c.Assert(err, gc.IsNil) + + client := NewClient( + WithCACertificates(caPEM.String()), + WithSkipHostnameVerification(skip), + ) + resp, err := client.Get(context.TODO(), s.server.URL) + c.Assert(err, gc.IsNil) + c.Assert(resp.Body.Close(), gc.IsNil) + c.Assert(resp.StatusCode, gc.Equals, http.StatusOK) +} + +func (s *clientSuite) TestDisableKeepAlives(c *gc.C) { + client := NewClient() + transport := client.Client().Transport.(*http.Transport) + c.Assert(transport.DisableKeepAlives, gc.Equals, false) + + client = NewClient(WithDisableKeepAlives(false)) + transport = client.Client().Transport.(*http.Transport) + c.Assert(transport.DisableKeepAlives, gc.Equals, false) + + client = NewClient(WithDisableKeepAlives(true)) + transport = client.Client().Transport.(*http.Transport) + c.Assert(transport.DisableKeepAlives, gc.Equals, true) +} diff --git a/internal/http/clock_mock_test.go b/internal/http/clock_mock_test.go new file mode 100644 index 00000000000..f9972b69e94 --- /dev/null +++ b/internal/http/clock_mock_test.go @@ -0,0 +1,193 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/clock (interfaces: Clock) +// +// Generated by this command: +// +// mockgen -typed -package http -destination clock_mock_test.go github.com/juju/clock Clock +// + +// Package http is a generated GoMock package. +package http + +import ( + reflect "reflect" + time "time" + + clock "github.com/juju/clock" + gomock "go.uber.org/mock/gomock" +) + +// MockClock is a mock of Clock interface. +type MockClock struct { + ctrl *gomock.Controller + recorder *MockClockMockRecorder +} + +// MockClockMockRecorder is the mock recorder for MockClock. +type MockClockMockRecorder struct { + mock *MockClock +} + +// NewMockClock creates a new mock instance. +func NewMockClock(ctrl *gomock.Controller) *MockClock { + mock := &MockClock{ctrl: ctrl} + mock.recorder = &MockClockMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockClock) EXPECT() *MockClockMockRecorder { + return m.recorder +} + +// After mocks base method. +func (m *MockClock) After(arg0 time.Duration) <-chan time.Time { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "After", arg0) + ret0, _ := ret[0].(<-chan time.Time) + return ret0 +} + +// After indicates an expected call of After. +func (mr *MockClockMockRecorder) After(arg0 any) *MockClockAfterCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "After", reflect.TypeOf((*MockClock)(nil).After), arg0) + return &MockClockAfterCall{Call: call} +} + +// MockClockAfterCall wrap *gomock.Call +type MockClockAfterCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockClockAfterCall) Return(arg0 <-chan time.Time) *MockClockAfterCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockClockAfterCall) Do(f func(time.Duration) <-chan time.Time) *MockClockAfterCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockClockAfterCall) DoAndReturn(f func(time.Duration) <-chan time.Time) *MockClockAfterCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// AfterFunc mocks base method. +func (m *MockClock) AfterFunc(arg0 time.Duration, arg1 func()) clock.Timer { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AfterFunc", arg0, arg1) + ret0, _ := ret[0].(clock.Timer) + return ret0 +} + +// AfterFunc indicates an expected call of AfterFunc. +func (mr *MockClockMockRecorder) AfterFunc(arg0, arg1 any) *MockClockAfterFuncCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AfterFunc", reflect.TypeOf((*MockClock)(nil).AfterFunc), arg0, arg1) + return &MockClockAfterFuncCall{Call: call} +} + +// MockClockAfterFuncCall wrap *gomock.Call +type MockClockAfterFuncCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockClockAfterFuncCall) Return(arg0 clock.Timer) *MockClockAfterFuncCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockClockAfterFuncCall) Do(f func(time.Duration, func()) clock.Timer) *MockClockAfterFuncCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockClockAfterFuncCall) DoAndReturn(f func(time.Duration, func()) clock.Timer) *MockClockAfterFuncCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// NewTimer mocks base method. +func (m *MockClock) NewTimer(arg0 time.Duration) clock.Timer { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewTimer", arg0) + ret0, _ := ret[0].(clock.Timer) + return ret0 +} + +// NewTimer indicates an expected call of NewTimer. +func (mr *MockClockMockRecorder) NewTimer(arg0 any) *MockClockNewTimerCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewTimer", reflect.TypeOf((*MockClock)(nil).NewTimer), arg0) + return &MockClockNewTimerCall{Call: call} +} + +// MockClockNewTimerCall wrap *gomock.Call +type MockClockNewTimerCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockClockNewTimerCall) Return(arg0 clock.Timer) *MockClockNewTimerCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockClockNewTimerCall) Do(f func(time.Duration) clock.Timer) *MockClockNewTimerCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockClockNewTimerCall) DoAndReturn(f func(time.Duration) clock.Timer) *MockClockNewTimerCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// Now mocks base method. +func (m *MockClock) Now() time.Time { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Now") + ret0, _ := ret[0].(time.Time) + return ret0 +} + +// Now indicates an expected call of Now. +func (mr *MockClockMockRecorder) Now() *MockClockNowCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Now", reflect.TypeOf((*MockClock)(nil).Now)) + return &MockClockNowCall{Call: call} +} + +// MockClockNowCall wrap *gomock.Call +type MockClockNowCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockClockNowCall) Return(arg0 time.Time) *MockClockNowCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockClockNowCall) Do(f func() time.Time) *MockClockNowCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockClockNowCall) DoAndReturn(f func() time.Time) *MockClockNowCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/internal/http/http.go b/internal/http/http.go new file mode 100644 index 00000000000..454788ac72c --- /dev/null +++ b/internal/http/http.go @@ -0,0 +1,49 @@ +// Copyright 2020 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package http + +import ( + "encoding/base64" + "fmt" + "net/http" + "strings" +) + +// BasicAuthHeader creates a header that contains just the "Authorization" +// entry. The implementation was originally taked from net/http but this is +// needed externally from the http request object in order to use this with +// our websockets. See 2 (end of page 4) http://www.ietf.org/rfc/rfc2617.txt +// "To receive authorization, the client sends the userid and password, +// separated by a single colon (":") character, within a base64 encoded string +// in the credentials." +func BasicAuthHeader(username, password string) http.Header { + auth := username + ":" + password + encoded := "Basic " + base64.StdEncoding.EncodeToString([]byte(auth)) + return http.Header{ + "Authorization": {encoded}, + } +} + +// ParseBasicAuth attempts to find an Authorization header in the supplied +// http.Header and if found parses it as a Basic header. See 2 (end of page 4) +// http://www.ietf.org/rfc/rfc2617.txt "To receive authorization, the client +// sends the userid and password, separated by a single colon (":") character, +// within a base64 encoded string in the credentials." +func ParseBasicAuthHeader(h http.Header) (userid, password string, err error) { + parts := strings.Fields(h.Get("Authorization")) + if len(parts) != 2 || parts[0] != "Basic" { + return "", "", fmt.Errorf("invalid or missing HTTP auth header") + } + // Challenge is a base64-encoded "tag:pass" string. + // See RFC 2617, Section 2. + challenge, err := base64.StdEncoding.DecodeString(parts[1]) + if err != nil { + return "", "", fmt.Errorf("invalid HTTP auth encoding") + } + tokens := strings.SplitN(string(challenge), ":", 2) + if len(tokens) != 2 { + return "", "", fmt.Errorf("invalid HTTP auth contents") + } + return tokens[0], tokens[1], nil +} diff --git a/internal/http/http_mock_test.go b/internal/http/http_mock_test.go new file mode 100644 index 00000000000..6ad7c4dba6c --- /dev/null +++ b/internal/http/http_mock_test.go @@ -0,0 +1,79 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/internal/http (interfaces: RoundTripper) +// +// Generated by this command: +// +// mockgen -typed -package http -destination http_mock_test.go github.com/juju/juju/internal/http RoundTripper +// + +// Package http is a generated GoMock package. +package http + +import ( + http "net/http" + reflect "reflect" + + gomock "go.uber.org/mock/gomock" +) + +// MockRoundTripper is a mock of RoundTripper interface. +type MockRoundTripper struct { + ctrl *gomock.Controller + recorder *MockRoundTripperMockRecorder +} + +// MockRoundTripperMockRecorder is the mock recorder for MockRoundTripper. +type MockRoundTripperMockRecorder struct { + mock *MockRoundTripper +} + +// NewMockRoundTripper creates a new mock instance. +func NewMockRoundTripper(ctrl *gomock.Controller) *MockRoundTripper { + mock := &MockRoundTripper{ctrl: ctrl} + mock.recorder = &MockRoundTripperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockRoundTripper) EXPECT() *MockRoundTripperMockRecorder { + return m.recorder +} + +// RoundTrip mocks base method. +func (m *MockRoundTripper) RoundTrip(arg0 *http.Request) (*http.Response, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RoundTrip", arg0) + ret0, _ := ret[0].(*http.Response) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// RoundTrip indicates an expected call of RoundTrip. +func (mr *MockRoundTripperMockRecorder) RoundTrip(arg0 any) *MockRoundTripperRoundTripCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RoundTrip", reflect.TypeOf((*MockRoundTripper)(nil).RoundTrip), arg0) + return &MockRoundTripperRoundTripCall{Call: call} +} + +// MockRoundTripperRoundTripCall wrap *gomock.Call +type MockRoundTripperRoundTripCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockRoundTripperRoundTripCall) Return(arg0 *http.Response, arg1 error) *MockRoundTripperRoundTripCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockRoundTripperRoundTripCall) Do(f func(*http.Request) (*http.Response, error)) *MockRoundTripperRoundTripCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockRoundTripperRoundTripCall) DoAndReturn(f func(*http.Request) (*http.Response, error)) *MockRoundTripperRoundTripCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/internal/http/http_test.go b/internal/http/http_test.go new file mode 100644 index 00000000000..d9be3b16fce --- /dev/null +++ b/internal/http/http_test.go @@ -0,0 +1,90 @@ +// Copyright 2020 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package http_test + +import ( + "encoding/base64" + "net/http" + "strings" + + "github.com/juju/testing" + gc "gopkg.in/check.v1" + + jujuhttp "github.com/juju/juju/internal/http" +) + +type httpSuite struct { + testing.IsolationSuite +} + +var _ = gc.Suite(&httpSuite{}) + +func (s *httpSuite) TestBasicAuthHeader(c *gc.C) { + header := jujuhttp.BasicAuthHeader("eric", "sekrit") + c.Assert(len(header), gc.Equals, 1) + auth := header.Get("Authorization") + fields := strings.Fields(auth) + c.Assert(len(fields), gc.Equals, 2) + basic, encoded := fields[0], fields[1] + c.Assert(basic, gc.Equals, "Basic") + decoded, err := base64.StdEncoding.DecodeString(encoded) + c.Assert(err, gc.IsNil) + c.Assert(string(decoded), gc.Equals, "eric:sekrit") +} + +func (s *httpSuite) TestParseBasicAuthHeader(c *gc.C) { + tests := []struct { + about string + h http.Header + expectUserid string + expectPassword string + expectError string + }{{ + about: "no Authorization header", + h: http.Header{}, + expectError: "invalid or missing HTTP auth header", + }, { + about: "empty Authorization header", + h: http.Header{ + "Authorization": {""}, + }, + expectError: "invalid or missing HTTP auth header", + }, { + about: "Not basic encoding", + h: http.Header{ + "Authorization": {"NotBasic stuff"}, + }, + expectError: "invalid or missing HTTP auth header", + }, { + about: "invalid base64", + h: http.Header{ + "Authorization": {"Basic not-base64"}, + }, + expectError: "invalid HTTP auth encoding", + }, { + about: "no ':'", + h: http.Header{ + "Authorization": {"Basic " + base64.StdEncoding.EncodeToString([]byte("aladdin"))}, + }, + expectError: "invalid HTTP auth contents", + }, { + about: "valid credentials", + h: http.Header{ + "Authorization": {"Basic " + base64.StdEncoding.EncodeToString([]byte("aladdin:open sesame"))}, + }, + expectUserid: "aladdin", + expectPassword: "open sesame", + }} + for i, test := range tests { + c.Logf("test %d: %s", i, test.about) + u, p, err := jujuhttp.ParseBasicAuthHeader(test.h) + c.Assert(u, gc.Equals, test.expectUserid) + c.Assert(p, gc.Equals, test.expectPassword) + if test.expectError != "" { + c.Assert(err.Error(), gc.Equals, test.expectError) + } else { + c.Assert(err, gc.IsNil) + } + } +} diff --git a/internal/http/middleware.go b/internal/http/middleware.go new file mode 100644 index 00000000000..9f6dfad7750 --- /dev/null +++ b/internal/http/middleware.go @@ -0,0 +1,305 @@ +// Copyright 2021 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. +package http + +import ( + "context" + "net" + "net/http" + "net/url" + "strconv" + "time" + + "github.com/juju/clock" + "github.com/juju/errors" + "github.com/juju/juju/core/logger" + "github.com/juju/loggo/v2" + "github.com/juju/retry" + "golang.org/x/net/http/httpproxy" +) + +// FileProtocolMiddleware registers support for file:// URLs on the given transport. +func FileProtocolMiddleware(transport *http.Transport) *http.Transport { + transport.RegisterProtocol("file", http.NewFileTransport(http.Dir("/"))) + return transport +} + +// DialBreaker replicates a highly specialized CircuitBreaker pattern, which +// takes into account the current address. +type DialBreaker interface { + // Allowed checks to see if a given address is allowed. + Allowed(string) bool + // Trip will cause the DialBreaker to change the breaker state + Trip() +} + +func isLocalAddr(addr string) bool { + host, _, err := net.SplitHostPort(addr) + if err != nil { + return false + } + return host == "localhost" || net.ParseIP(host).IsLoopback() +} + +// DialContextMiddleware patches the default HTTP transport so +// that it fails when an attempt is made to dial a non-local +// host. +func DialContextMiddleware(breaker DialBreaker) TransportMiddleware { + return func(transport *http.Transport) *http.Transport { + dialer := &net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + } + transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + if !breaker.Allowed(addr) { + return nil, errors.Errorf("access to address %q not allowed", addr) + } + + return dialer.DialContext(ctx, network, addr) + } + return transport + } +} + +// LocalDialBreaker defines a DialBreaker that when tripped only allows local +// dials, anything else is prevented. +type LocalDialBreaker struct { + allowOutgoingAccess bool +} + +// NewLocalDialBreaker creates a new LocalDialBreaker with a default value. +func NewLocalDialBreaker(allowOutgoingAccess bool) *LocalDialBreaker { + return &LocalDialBreaker{ + allowOutgoingAccess: allowOutgoingAccess, + } +} + +// Allowed checks to see if a dial is allowed to happen, or returns an error +// stating why. +func (b *LocalDialBreaker) Allowed(addr string) bool { + if b.allowOutgoingAccess { + return true + } + // If we're not allowing outgoing access, then only local addresses are + // allowed to be dialed. Check for local only addresses. + return isLocalAddr(addr) +} + +// Trip inverts the local state of the DialBreaker. +func (b *LocalDialBreaker) Trip() { + b.allowOutgoingAccess = !b.allowOutgoingAccess +} + +// ProxyMiddleware adds a Proxy to the given transport. This implementation +// uses the http.ProxyFromEnvironment. +func ProxyMiddleware(transport *http.Transport) *http.Transport { + transport.Proxy = getProxy + return transport +} + +var midLogger = loggo.GetLoggerWithTags("juju.http.middleware", "http") + +func getProxy(req *http.Request) (*url.URL, error) { + // Get proxy config new for each client. Go will cache the proxy + // settings for a process, this is a problem for long running programs. + // And caused changes in proxy settings via model-config not to + // be used. + cfg := httpproxy.FromEnvironment() + midLogger.Tracef("proxy config http(%s), https(%s), no-proxy(%s)", + cfg.HTTPProxy, cfg.HTTPSProxy, cfg.NoProxy) + return cfg.ProxyFunc()(req.URL) +} + +// ForceAttemptHTTP2Middleware forces a HTTP/2 connection if a non-zero +// Dial, DialTLS, or DialContext func or TLSClientConfig is provided to the +// Transport. Using any of these will render HTTP/2 disabled, so force the +// client to use it for requests. +func ForceAttemptHTTP2Middleware(transport *http.Transport) *http.Transport { + transport.ForceAttemptHTTP2 = true + return transport +} + +// RequestRecorder is implemented by types that can record information about +// successful and unsuccessful http requests. +type RequestRecorder interface { + // Record an outgoing request which produced an http.Response. + Record(method string, url *url.URL, res *http.Response, rtt time.Duration) + + // Record an outgoing request which returned back an error. + RecordError(method string, url *url.URL, err error) +} + +// RoundTripper allows us to generate mocks for the http.RoundTripper because +// we're already in a http package. +type RoundTripper = http.RoundTripper + +type roundTripRecorder struct { + requestRecorder RequestRecorder + wrappedRoundTripper http.RoundTripper +} + +// RoundTrip implements http.RoundTripper. If delegates the request to the +// wrapped RoundTripper and invokes the appropriate RequestRecorder methods +// depending on the outcome. +func (lr roundTripRecorder) RoundTrip(req *http.Request) (*http.Response, error) { + start := time.Now() + res, err := lr.wrappedRoundTripper.RoundTrip(req) + rtt := time.Since(start) + + if err != nil { + lr.requestRecorder.RecordError(req.Method, req.URL, err) + } else { + lr.requestRecorder.Record(req.Method, req.URL, res, rtt) + } + + return res, err +} + +// RetryMiddleware allows retrying of certain retryable http errors. +// This only handles very specific status codes, ones that are deemed retryable: +// +// - 502 Bad Gateway +// - 503 Service Unavailable +// - 504 Gateway Timeout +type retryMiddleware struct { + policy RetryPolicy + wrappedRoundTripper http.RoundTripper + clock clock.Clock + logger logger.Logger +} + +type RetryPolicy struct { + Delay time.Duration + MaxDelay time.Duration + Attempts int +} + +// Validate validates the RetryPolicy for any issues. +func (p RetryPolicy) Validate() error { + if p.Attempts < 1 { + return errors.Errorf("expected at least one attempt") + } + if p.MaxDelay < 1 { + return errors.Errorf("expected max delay to be a valid time") + } + return nil +} + +// makeRetryMiddleware creates a retry transport. +func makeRetryMiddleware(transport http.RoundTripper, policy RetryPolicy, clock clock.Clock, logger logger.Logger) http.RoundTripper { + return retryMiddleware{ + policy: policy, + wrappedRoundTripper: transport, + clock: clock, + logger: logger, + } +} + +type retryableErr struct{} + +func (retryableErr) Error() string { + return "retryable error" +} + +// RoundTrip defines a strategy for handling retries based on the status code. +func (m retryMiddleware) RoundTrip(req *http.Request) (*http.Response, error) { + var ( + res *http.Response + backOffErr error + ) + err := retry.Call(retry.CallArgs{ + Clock: m.clock, + Func: func() error { + if err := req.Context().Err(); err != nil { + return err + } + if backOffErr != nil { + return backOffErr + } + + var retryable bool + var err error + res, retryable, err = m.roundTrip(req) + if err != nil { + return err + } + if retryable { + return retryableErr{} + } + return nil + }, + IsFatalError: func(err error) bool { + // Work out if it's not a retryable error. + _, ok := errors.Cause(err).(retryableErr) + return !ok + }, + Attempts: m.policy.Attempts, + Delay: m.policy.Delay, + BackoffFunc: func(delay time.Duration, attempts int) time.Duration { + var duration time.Duration + duration, backOffErr = m.defaultBackoff(res, delay) + return duration + }, + }) + + return res, err +} + +func (m retryMiddleware) roundTrip(req *http.Request) (*http.Response, bool, error) { + res, err := m.wrappedRoundTripper.RoundTrip(req) + if err != nil { + return nil, false, err + } + + switch res.StatusCode { + case http.StatusBadGateway, http.StatusGatewayTimeout: + // The request should be retryable. + fallthrough + case http.StatusServiceUnavailable, http.StatusTooManyRequests: + // The request should be retryable, but additionally should contain + // a potential Retry-After header. + return res, true, nil + default: + // Don't handle any of the following status codes. + return res, false, nil + } +} + +// defaultBackoff attempts to workout a good backoff strategy based on the +// backoff policy or the status code from the response. +// +// RFC7231 states that the retry-after header can look like the following: +// +// - Retry-After: +// - Retry-After: +func (m retryMiddleware) defaultBackoff(resp *http.Response, backoff time.Duration) (time.Duration, error) { + if header := resp.Header.Get("Retry-After"); header != "" { + // Attempt to parse the header from the request. + // + // Check for delay in seconds first, before checking for a http-date + seconds, err := strconv.ParseInt(header, 10, 64) + if err == nil { + return m.clampBackoff(time.Second * time.Duration(seconds)) + } + // Check for http-date. + date, err := time.Parse(time.RFC1123, header) + if err == nil { + return m.clampBackoff(m.clock.Now().Sub(date)) + } + url := "" + if resp.Request != nil { + url = resp.Request.URL.String() + } + m.logger.Errorf("unable to parse Retry-After header %s from %s", header, url) + } + + return m.clampBackoff(backoff) +} + +func (m retryMiddleware) clampBackoff(duration time.Duration) (time.Duration, error) { + if m.policy.MaxDelay > 0 && duration > m.policy.MaxDelay { + future := m.clock.Now().Add(duration) + return duration, errors.Errorf("API request retry is not accepting further requests until %s", future.Format(time.RFC3339)) + } + return duration, nil +} diff --git a/internal/http/middleware_test.go b/internal/http/middleware_test.go new file mode 100644 index 00000000000..6aab6e3bbd6 --- /dev/null +++ b/internal/http/middleware_test.go @@ -0,0 +1,355 @@ +// Copyright 2021 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. +package http + +import ( + "context" + "net/http" + "time" + + "github.com/juju/clock" + "github.com/juju/testing" + "go.uber.org/mock/gomock" + gc "gopkg.in/check.v1" + + loggertesting "github.com/juju/juju/internal/logger/testing" +) + +type DialContextMiddlewareSuite struct { + testing.IsolationSuite +} + +var _ = gc.Suite(&DialContextMiddlewareSuite{}) + +var isLocalAddrTests = []struct { + addr string + isLocal bool +}{ + {addr: "localhost:456", isLocal: true}, + {addr: "127.0.0.1:1234", isLocal: true}, + {addr: "[::1]:4567", isLocal: true}, + {addr: "localhost:smtp", isLocal: true}, + {addr: "123.45.67.5", isLocal: false}, + {addr: "0.1.2.3", isLocal: false}, + {addr: "10.0.43.6:12345", isLocal: false}, + {addr: ":456", isLocal: false}, + {addr: "12xz4.5.6", isLocal: false}, +} + +func (s *DialContextMiddlewareSuite) TestIsLocalAddr(c *gc.C) { + for i, test := range isLocalAddrTests { + c.Logf("test %d: %v", i, test.addr) + c.Assert(isLocalAddr(test.addr), gc.Equals, test.isLocal) + } +} + +func (s *DialContextMiddlewareSuite) TestInsecureClientNoAccess(c *gc.C) { + client := NewClient( + WithTransportMiddlewares( + DialContextMiddleware(NewLocalDialBreaker(false)), + ), + WithSkipHostnameVerification(true), + ) + _, err := client.Get(context.TODO(), "http://0.1.2.3:1234") + c.Assert(err, gc.ErrorMatches, `.*access to address "0.1.2.3:1234" not allowed`) +} + +func (s *DialContextMiddlewareSuite) TestSecureClientNoAccess(c *gc.C) { + client := NewClient( + WithTransportMiddlewares( + DialContextMiddleware(NewLocalDialBreaker(false)), + ), + ) + _, err := client.Get(context.TODO(), "http://0.1.2.3:1234") + c.Assert(err, gc.ErrorMatches, `.*access to address "0.1.2.3:1234" not allowed`) +} + +type LocalDialBreakerSuite struct { + testing.IsolationSuite +} + +var _ = gc.Suite(&LocalDialBreakerSuite{}) + +func (s *LocalDialBreakerSuite) TestAllowed(c *gc.C) { + breaker := NewLocalDialBreaker(true) + + for i, test := range isLocalAddrTests { + c.Logf("test %d: %v", i, test.addr) + allowed := breaker.Allowed(test.addr) + c.Assert(allowed, gc.Equals, true) + } +} + +func (s *LocalDialBreakerSuite) TestLocalAllowed(c *gc.C) { + breaker := NewLocalDialBreaker(false) + + for i, test := range isLocalAddrTests { + c.Logf("test %d: %v", i, test.addr) + allowed := breaker.Allowed(test.addr) + c.Assert(allowed, gc.Equals, test.isLocal) + } +} + +func (s *LocalDialBreakerSuite) TestLocalAllowedAfterTrip(c *gc.C) { + breaker := NewLocalDialBreaker(true) + + for i, test := range isLocalAddrTests { + c.Logf("test %d: %v", i, test.addr) + allowed := breaker.Allowed(test.addr) + c.Assert(allowed, gc.Equals, true) + + breaker.Trip() + + allowed = breaker.Allowed(test.addr) + c.Assert(allowed, gc.Equals, test.isLocal) + + // Reset the breaker. + breaker.Trip() + } +} + +type RetrySuite struct { + testing.IsolationSuite +} + +var _ = gc.Suite(&RetrySuite{}) + +func (s *RetrySuite) TestRetryNotRequired(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + req, err := http.NewRequest("GET", "http://meshuggah.rocks", nil) + c.Assert(err, gc.IsNil) + + transport := NewMockRoundTripper(ctrl) + transport.EXPECT().RoundTrip(req).Return(&http.Response{ + StatusCode: http.StatusOK, + }, nil) + + middleware := makeRetryMiddleware(transport, RetryPolicy{ + Attempts: 3, + Delay: time.Second, + MaxDelay: time.Minute, + }, clock.WallClock, loggertesting.WrapCheckLog(c)) + + resp, err := middleware.RoundTrip(req) + c.Assert(err, gc.IsNil) + c.Assert(resp.StatusCode, gc.Equals, http.StatusOK) +} + +func (s *RetrySuite) TestRetryRequired(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + req, err := http.NewRequest("GET", "http://meshuggah.rocks", nil) + c.Assert(err, gc.IsNil) + + transport := NewMockRoundTripper(ctrl) + transport.EXPECT().RoundTrip(req).Return(&http.Response{ + StatusCode: http.StatusBadGateway, + }, nil).Times(2) + transport.EXPECT().RoundTrip(req).Return(&http.Response{ + StatusCode: http.StatusOK, + }, nil) + + ch := make(chan time.Time) + + clock := NewMockClock(ctrl) + clock.EXPECT().Now().Return(time.Now()).AnyTimes() + clock.EXPECT().After(gomock.Any()).Return(ch).AnyTimes() + + retries := 3 + go func() { + for i := 0; i < retries; i++ { + ch <- time.Now() + } + }() + + middleware := makeRetryMiddleware(transport, RetryPolicy{ + Attempts: retries, + Delay: time.Second, + MaxDelay: time.Minute, + }, clock, loggertesting.WrapCheckLog(c)) + + resp, err := middleware.RoundTrip(req) + c.Assert(err, gc.IsNil) + c.Assert(resp.StatusCode, gc.Equals, http.StatusOK) +} + +func (s *RetrySuite) TestRetryRequiredUsingBackoff(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + req, err := http.NewRequest("GET", "http://meshuggah.rocks", nil) + c.Assert(err, gc.IsNil) + + header := make(http.Header) + header.Add("Retry-After", "42") + + transport := NewMockRoundTripper(ctrl) + transport.EXPECT().RoundTrip(req).Return(&http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: header, + }, nil).Times(2) + transport.EXPECT().RoundTrip(req).Return(&http.Response{ + StatusCode: http.StatusOK, + }, nil) + + ch := make(chan time.Time) + + clock := NewMockClock(ctrl) + clock.EXPECT().Now().Return(time.Now()).AnyTimes() + clock.EXPECT().After(time.Second * 42).Return(ch).Times(2) + + retries := 3 + go func() { + for i := 0; i < retries; i++ { + ch <- time.Now() + } + }() + + middleware := makeRetryMiddleware(transport, RetryPolicy{ + Attempts: retries, + Delay: time.Second, + MaxDelay: time.Minute, + }, clock, loggertesting.WrapCheckLog(c)) + + resp, err := middleware.RoundTrip(req) + c.Assert(err, gc.IsNil) + c.Assert(resp.StatusCode, gc.Equals, http.StatusOK) +} + +func (s *RetrySuite) TestRetryRequiredUsingBackoffFailure(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + req, err := http.NewRequest("GET", "http://meshuggah.rocks", nil) + c.Assert(err, gc.IsNil) + + header := make(http.Header) + header.Add("Retry-After", "2520") + + transport := NewMockRoundTripper(ctrl) + transport.EXPECT().RoundTrip(req).Return(&http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: header, + }, nil) + + ch := make(chan time.Time) + + clock := NewMockClock(ctrl) + clock.EXPECT().Now().Return(time.Now()).AnyTimes() + clock.EXPECT().After(time.Minute * 42).Return(ch) + + retries := 3 + go func() { + ch <- time.Now() + }() + + middleware := makeRetryMiddleware(transport, RetryPolicy{ + Attempts: retries, + Delay: time.Minute, + MaxDelay: time.Second, + }, clock, loggertesting.WrapCheckLog(c)) + + _, err = middleware.RoundTrip(req) + c.Assert(err, gc.ErrorMatches, `API request retry is not accepting further requests until .*`) +} + +func (s *RetrySuite) TestRetryRequiredUsingBackoffError(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + req, err := http.NewRequest("GET", "http://meshuggah.rocks", nil) + c.Assert(err, gc.IsNil) + + header := make(http.Header) + header.Add("Retry-After", "!@1234391asd--\\123") + + transport := NewMockRoundTripper(ctrl) + transport.EXPECT().RoundTrip(req).Return(&http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: header, + }, nil) + + ch := make(chan time.Time) + + clock := NewMockClock(ctrl) + clock.EXPECT().Now().Return(time.Now()).AnyTimes() + clock.EXPECT().After(time.Minute * 1).Return(ch) + + retries := 3 + go func() { + ch <- time.Now() + }() + + middleware := makeRetryMiddleware(transport, RetryPolicy{ + Attempts: retries, + Delay: time.Minute, + MaxDelay: time.Second, + }, clock, loggertesting.WrapCheckLog(c)) + + _, err = middleware.RoundTrip(req) + c.Assert(err, gc.ErrorMatches, `API request retry is not accepting further requests until .*`) +} + +func (s *RetrySuite) TestRetryRequiredAndExceeded(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + req, err := http.NewRequest("GET", "http://meshuggah.rocks", nil) + c.Assert(err, gc.IsNil) + + transport := NewMockRoundTripper(ctrl) + transport.EXPECT().RoundTrip(req).Return(&http.Response{ + StatusCode: http.StatusBadGateway, + }, nil).Times(3) + + ch := make(chan time.Time) + + clock := NewMockClock(ctrl) + clock.EXPECT().Now().Return(time.Now()).AnyTimes() + clock.EXPECT().After(gomock.Any()).Return(ch).AnyTimes() + + retries := 3 + go func() { + for i := 0; i < retries; i++ { + ch <- time.Now() + } + }() + + middleware := makeRetryMiddleware(transport, RetryPolicy{ + Attempts: retries, + Delay: time.Second, + MaxDelay: time.Minute, + }, clock, loggertesting.WrapCheckLog(c)) + + _, err = middleware.RoundTrip(req) + c.Assert(err, gc.ErrorMatches, `attempt count exceeded: retryable error`) +} + +func (s *RetrySuite) TestRetryRequiredContextKilled(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + ctx, cancel := context.WithCancel(context.Background()) + + req, err := http.NewRequestWithContext(ctx, "GET", "http://meshuggah.rocks", nil) + c.Assert(err, gc.IsNil) + + transport := NewMockRoundTripper(ctrl) + + clock := NewMockClock(ctrl) + clock.EXPECT().Now().Return(time.Now()) + + middleware := makeRetryMiddleware(transport, RetryPolicy{ + Attempts: 3, + Delay: time.Second, + }, clock, loggertesting.WrapCheckLog(c)) + + // Nothing should run, the context has been cancelled. + cancel() + + _, err = middleware.RoundTrip(req) + c.Assert(err, gc.ErrorMatches, `context canceled`) +} diff --git a/internal/http/package_test.go b/internal/http/package_test.go new file mode 100644 index 00000000000..b0da07d052d --- /dev/null +++ b/internal/http/package_test.go @@ -0,0 +1,18 @@ +// Copyright 2020 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package http + +import ( + "testing" + + gc "gopkg.in/check.v1" +) + +//go:generate go run go.uber.org/mock/mockgen -typed -package http -destination client_mock_test.go github.com/juju/juju/internal/http HTTPClient,RequestRecorder,Logger +//go:generate go run go.uber.org/mock/mockgen -typed -package http -destination http_mock_test.go github.com/juju/juju/internal/http RoundTripper +//go:generate go run go.uber.org/mock/mockgen -typed -package http -destination clock_mock_test.go github.com/juju/clock Clock + +func Test(t *testing.T) { + gc.TestingT(t) +} diff --git a/internal/http/tls.go b/internal/http/tls.go new file mode 100644 index 00000000000..cf38e783f33 --- /dev/null +++ b/internal/http/tls.go @@ -0,0 +1,92 @@ +// Copyright 2020 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package http + +import ( + "crypto/tls" + "net/http" + "time" +) + +// TransportMiddleware represents a way to add an adapter to the existing transport. +type TransportMiddleware func(*http.Transport) *http.Transport + +// TransportConfig holds the configurable values for setting up a http +// transport. +type TransportConfig struct { + TLSConfig *tls.Config + DisableKeepAlives bool + TLSHandshakeTimeout time.Duration + Middlewares []TransportMiddleware +} + +// NewHTTPTLSTransport returns a new http.Transport constructed with the TLS config +// and the necessary parameters for Juju. +func NewHTTPTLSTransport(config TransportConfig) *http.Transport { + transport := &http.Transport{ + TLSClientConfig: config.TLSConfig, + DisableKeepAlives: config.DisableKeepAlives, + TLSHandshakeTimeout: config.TLSHandshakeTimeout, + } + for _, middlewareFn := range config.Middlewares { + transport = middlewareFn(transport) + } + return transport +} + +// DefaultHTTPTransport creates a default transport with proxy middleware +// enabled. +func DefaultHTTPTransport() *http.Transport { + return NewHTTPTLSTransport(TransportConfig{ + TLSHandshakeTimeout: 20 * time.Second, + Middlewares: []TransportMiddleware{ + ProxyMiddleware, + }, + }) +} + +// knownGoodCipherSuites contains the list of secure cipher suites to use +// with tls.Config. This list matches those that Go 1.6 implements from +// https://wiki.mozilla.org/Security/Server_Side_TLS#Recommended_configurations. +// +// https://tools.ietf.org/html/rfc7525#section-4.2 excludes RSA exchange completely +// so we could be more strict if all our clients will support +// TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256/384. Unfortunately Go's crypto library +// is limited and doesn't support DHE-RSA-AES256-GCM-SHA384 and +// DHE-RSA-AES256-SHA256, which are part of the recommended set. +// +// Unfortunately we can't drop the RSA algorithms because our servers aren't +// generating ECDHE keys. +var knownGoodCipherSuites = []uint16{ + // These are technically useless for Juju, since we use an RSA certificate, + // but they also don't hurt anything, and supporting an ECDSA certificate + // could be useful in the future. + tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + + // Windows doesn't support GCM currently, so we need these for RSA support. + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + + // We need this so that we have at least one suite in common + // with the default gnutls installed for precise and trusty. + tls.TLS_RSA_WITH_AES_256_CBC_SHA, +} + +// SecureTLSConfig returns a tls.Config that conforms to Juju's security +// standards, so as to avoid known security vulnerabilities in certain +// configurations. +// +// Currently it excludes RC4 implementations from the available ciphersuites, +// requires ciphersuites that provide forward secrecy, and sets the minimum TLS +// version to 1.2. +func SecureTLSConfig() *tls.Config { + return &tls.Config{ + CipherSuites: knownGoodCipherSuites, + MinVersion: tls.VersionTLS12, + } +} diff --git a/internal/http/tls_test.go b/internal/http/tls_test.go new file mode 100644 index 00000000000..92e9f0cc586 --- /dev/null +++ b/internal/http/tls_test.go @@ -0,0 +1,148 @@ +// Copyright 2020 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package http + +import ( + "fmt" + "io/ioutil" + "net/http" + "os/exec" + "path/filepath" + "runtime" + + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" +) + +type TLSSuite struct{} + +var _ = gc.Suite(TLSSuite{}) + +func (TLSSuite) TestWinCipher(c *gc.C) { + if runtime.GOOS != "windows" { + c.Skip("Windows-specific test.") + } + + d := c.MkDir() + go runServer(d, c) + + out := filepath.Join(d, "out.txt") + + // this script enables TLS 1.2, accepts whatever cert the server has (since + // it's self-signed), then tries to connect to the web server. + script := fmt.Sprintf(`[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12 +[System.Net.ServicePointManager]::ServerCertificateValidationCallback = {$true} +(New-Object System.Net.WebClient).DownloadFile("https://127.0.0.1:10443", "%s") +`, out) + err := runPS(d, script) + c.Assert(err, jc.ErrorIsNil) + b, err := ioutil.ReadFile(out) + c.Assert(err, jc.ErrorIsNil) + c.Assert(string(b), gc.Equals, "This is an example server.\n") +} + +func runServer(dir string, c *gc.C) { + handler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.Header().Set("Content-Type", "text/plain") + w.Write([]byte("This is an example server.\n")) + }) + + s := http.Server{ + Addr: ":10443", + TLSConfig: SecureTLSConfig(), + Handler: handler, + } + + certFile := filepath.Join(dir, "cert.pem") + err := ioutil.WriteFile(certFile, []byte(cert), 0600) + c.Assert(err, jc.ErrorIsNil) + keyFile := filepath.Join(dir, "key.pem") + err = ioutil.WriteFile(keyFile, []byte(key), 0600) + c.Assert(err, jc.ErrorIsNil) + + err = s.ListenAndServeTLS(certFile, keyFile) + c.Assert(err, jc.ErrorIsNil) +} + +func runPS(dir, script string) error { + scriptFile := filepath.Join(dir, "script.ps1") + args := []string{ + "-NoProfile", + "-NonInteractive", + "-ExecutionPolicy", "RemoteSigned", + "-File", scriptFile, + } + // Exceptions don't result in a non-zero exit code by default + // when using -File. The exit code of an explicit "exit" when + // using -Command is ignored and results in an exit code of 1. + // We use -File and trap exceptions to cover both. + script = "trap {Write-Error $_; exit 1}\n" + script + if err := ioutil.WriteFile(scriptFile, []byte(script), 0600); err != nil { + return err + } + cmd := exec.Command("powershell.exe", args...) + return cmd.Run() +} + +const ( + cert = `-----BEGIN CERTIFICATE----- +MIIC9TCCAd2gAwIBAgIRALhL8rNhi3x29T8g/AwK9bAwDQYJKoZIhvcNAQELBQAw +EjEQMA4GA1UEChMHQWNtZSBDbzAeFw0xNjA3MjYxNjI4MzRaFw0xNzA3MjYxNjI4 +MzRaMBIxEDAOBgNVBAoTB0FjbWUgQ28wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw +ggEKAoIBAQCaVIZUmQdBTXYATbTmMhscCTUSNt+Dn3OP8w2v/2QJyUz3s1eiiuec +ymD+6TC7lNjzIXhFJnHTyuo/p2d2lHNvbQmUh0kMjPxnIDaCqWZXcjR+vnFo4jgl +VtxCqPG2zi62kZxB0Pu9DzJ7AlqF9BTbpu0INDyFzLJtj73RIv00kRDTpFzHQSNN +tzi9ZzKY7ZS6urftqXc4pqoaSyFXqw7uSNcBcr7Cc8oXIz5tQoVU5m0uKBGOQvwC +b+ICd+RIYS09L1E76UGpDcrJ0LQlysQ/ZMmSsA5YHGf5KE+N0WnWdQCADq3voQra +q47HBpH+ByA1F1REMwgMoFNZRNrEHdXFAgMBAAGjRjBEMA4GA1UdDwEB/wQEAwIF +oDATBgNVHSUEDDAKBggrBgEFBQcDATAMBgNVHRMBAf8EAjAAMA8GA1UdEQQIMAaH +BH8AAAEwDQYJKoZIhvcNAQELBQADggEBAJPwxR3fhEpZz2JB2dAUuj0KqFD7uPQp +m30Slu3cihqQkoaGiSMQdGSZ/VnieHbS/XaZo8JqixU8RucYjVT2eM5YRgcGxU91 +L4yJfPm7qPwGIvwpfqlZK5GcpC/qk3joNqL43gGfn6vbtqw+wF33yfcyTlTO1hwN +vZSU4HC3Hz+FoFnmqkW5lXiuggm/jsdWqPIDA0NJHrws/wjqu3T+wQcfTvIwIPMG +WFmUP5hvWD/9HpizJqROhRZwfsJHDpHDu0nKgSDnV1gX2S5XaUsUWu53V/Hczbo0 +fSD4wg+Zd/x3fh+EpOd1qbHmXrDWSs4z/T61yKzrgENd/kSncJC38pg= +-----END CERTIFICATE----- +` + key = `-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEAmlSGVJkHQU12AE205jIbHAk1Ejbfg59zj/MNr/9kCclM97NX +oornnMpg/ukwu5TY8yF4RSZx08rqP6dndpRzb20JlIdJDIz8ZyA2gqlmV3I0fr5x +aOI4JVbcQqjxts4utpGcQdD7vQ8yewJahfQU26btCDQ8hcyybY+90SL9NJEQ06Rc +x0EjTbc4vWcymO2Uurq37al3OKaqGkshV6sO7kjXAXK+wnPKFyM+bUKFVOZtLigR +jkL8Am/iAnfkSGEtPS9RO+lBqQ3KydC0JcrEP2TJkrAOWBxn+ShPjdFp1nUAgA6t +76EK2quOxwaR/gcgNRdURDMIDKBTWUTaxB3VxQIDAQABAoIBAQCBktX10UW2HkMk +nhlz7D22nERito+TAx0Tjw2+5r4nOUvV7E13uwgbLA+j9kVkOOStvTwtUsne+E8U +gojrllgVBYc1nSBH2VdRfkpGCdRTNx+8CklNtiFNuE/V5+KJiTLPNhHrcHrrkQbh +IGjAbt3UTaJVcQYfkG1+b2D/ZlERAC0s0lnqidoqeAaiXDmDesIz+gXkpfbt5mHa +f/LRFRvjtBDjCOTkZ3OdFeSyW+z4zs75vvk3amQNixGW74obFUZFBvF81yUZH7kf +bWBMJMIo024oo4Rpi5k279gx2pWNLHQ68AWF/zLbu32xGrSQuTelVU5MNgEDVB9W +3T01iHwBAoGBAMGGslxNYcf2lg0pW6II4EmOSvbdZ5z9kmV92wkN4zTP3Tzr/Kzf +UMALczvCBYplo6Q6nR+TvRukl8Mr1e5m7Ophfv21vZfprs2YXigL9vTZKRsis8Fk +QSK2kO9CVnWjFu11jYCDN9nUD+9lB+ry9grdY0744a8dTsxmZ1m1ZwA1AoGBAMwm +nF0+OnMkILfsnaK6PVsJBUI5N/j05P/mDQcDZdQMVOBSh/kceQ4LWHXdL0lMVLBY +pGPXqwsO8Q/d2R2oI1acgIFcl73FTchrQd1YaHmnyfqInhKt9QOXj1c0ii4BL3ff +iGVf4gqQVH0B2nK7pjkBlwvpjsYFVDHP9/xkXlFRAoGAC9mgoFBItYLe601mBAUB +Ht/srTMffhh012wedm54RCqaRHm6zicafbf1xWn7Bt90ZsEEEAPu53tro5LSlbeN +uEhiC00On/e6MXKsCU26QIHvp263jRcDegmt1Ei+nJNw+vdgw8bFK7x1gVYxZuyb +rkyiIRrSTvO/eHqox3B5LyUCgYAmKZWTTJ2qhndjSmURVVVA3kfQYFfZPxZLy9pl +lDoF0KRRJrxqUetDN9W6erVrM0ylhnx8eYVs1Mc1WxhKFfM9LpZLGF75R5fJvlsa +oHsvOrFkFwPNpB0oJb3S5GxsOyZ/dxbNNIZRyTcyAxWt2uwwvd5ZiLh6xeY+RY0q +7iw/cQKBgQCaWJ8bSNNhQeaBSW5IVHFctYtLPv9aPHagBdJkKmeb06HWQHi+AvkY +nd0dgM/TfgtnuhbVS4ISkT4vZoSn84hOE7BG5rSPE+/q24Wv5gG0PI1sky8tmXzX +juAEWSJVCSE0TK/mvBVdlyKOJoEgtfMcRfDQfA1rI9My0rU+/Y5A0w== +-----END RSA PRIVATE KEY-----` +) + +func (TLSSuite) TestDisableKeepAlives(c *gc.C) { + transport := DefaultHTTPTransport() + c.Assert(transport.DisableKeepAlives, gc.Equals, false) + + transport = NewHTTPTLSTransport(TransportConfig{}) + c.Assert(transport.DisableKeepAlives, gc.Equals, false) + + transport = NewHTTPTLSTransport(TransportConfig{ + DisableKeepAlives: true, + }) + c.Assert(transport.DisableKeepAlives, gc.Equals, true) +} diff --git a/internal/mongo/open.go b/internal/mongo/open.go index 25fb07c2ad9..ebfaf735c0e 100644 --- a/internal/mongo/open.go +++ b/internal/mongo/open.go @@ -13,7 +13,7 @@ import ( "time" "github.com/juju/errors" - "github.com/juju/http/v2" + "github.com/juju/juju/internal/http" "github.com/juju/mgo/v3" "github.com/juju/names/v5" "github.com/juju/utils/v4/cert" diff --git a/internal/provider/ec2/environ.go b/internal/provider/ec2/environ.go index c3bbf5ca3b3..525d8df0135 100644 --- a/internal/provider/ec2/environ.go +++ b/internal/provider/ec2/environ.go @@ -23,7 +23,7 @@ import ( "github.com/juju/clock" "github.com/juju/collections/set" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "github.com/juju/retry" "github.com/juju/version/v2" diff --git a/internal/provider/gce/environ.go b/internal/provider/gce/environ.go index da93fa559d5..dcf3a7b8662 100644 --- a/internal/provider/gce/environ.go +++ b/internal/provider/gce/environ.go @@ -11,7 +11,7 @@ import ( "time" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "google.golang.org/api/compute/v1" jujucloud "github.com/juju/juju/cloud" diff --git a/internal/provider/gce/google/auth.go b/internal/provider/gce/google/auth.go index 8898bf52899..8a10b0897a0 100644 --- a/internal/provider/gce/google/auth.go +++ b/internal/provider/gce/google/auth.go @@ -7,7 +7,7 @@ import ( "context" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "golang.org/x/oauth2/google" "golang.org/x/oauth2/jwt" "google.golang.org/api/compute/v1" diff --git a/internal/provider/gce/google/auth_test.go b/internal/provider/gce/google/auth_test.go index 8a365e0b500..639e74ca11b 100644 --- a/internal/provider/gce/google/auth_test.go +++ b/internal/provider/gce/google/auth_test.go @@ -6,7 +6,7 @@ package google import ( "context" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" ) diff --git a/internal/provider/gce/google/config.go b/internal/provider/gce/google/config.go index 16b022450d3..eeac394affd 100644 --- a/internal/provider/gce/google/config.go +++ b/internal/provider/gce/google/config.go @@ -9,7 +9,7 @@ import ( "net/mail" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" ) // The names of OS environment variables related to GCE. diff --git a/internal/provider/gce/google/config_connection_test.go b/internal/provider/gce/google/config_connection_test.go index 832eab6ed19..5a104e43779 100644 --- a/internal/provider/gce/google/config_connection_test.go +++ b/internal/provider/gce/google/config_connection_test.go @@ -4,7 +4,7 @@ package google_test import ( - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" diff --git a/internal/provider/gce/google/conn.go b/internal/provider/gce/google/conn.go index e9db80bdfca..d1fd888f0dc 100644 --- a/internal/provider/gce/google/conn.go +++ b/internal/provider/gce/google/conn.go @@ -7,7 +7,7 @@ import ( "context" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "google.golang.org/api/compute/v1" ) diff --git a/internal/provider/gce/google/conn_test.go b/internal/provider/gce/google/conn_test.go index f109a03fc89..fd55569b6d9 100644 --- a/internal/provider/gce/google/conn_test.go +++ b/internal/provider/gce/google/conn_test.go @@ -7,7 +7,7 @@ import ( "context" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" "google.golang.org/api/compute/v1" gc "gopkg.in/check.v1" diff --git a/internal/provider/gce/google/testing_test.go b/internal/provider/gce/google/testing_test.go index 111de4e50b6..b1baa0e9945 100644 --- a/internal/provider/gce/google/testing_test.go +++ b/internal/provider/gce/google/testing_test.go @@ -4,7 +4,7 @@ package google import ( - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "google.golang.org/api/compute/v1" gc "gopkg.in/check.v1" diff --git a/internal/provider/lxd/provider.go b/internal/provider/lxd/provider.go index b194474a98f..07339dc692f 100644 --- a/internal/provider/lxd/provider.go +++ b/internal/provider/lxd/provider.go @@ -13,8 +13,8 @@ import ( "github.com/juju/clock" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" "github.com/juju/jsonschema" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/schema" "gopkg.in/juju/environschema.v1" "gopkg.in/yaml.v2" diff --git a/internal/provider/openstack/client.go b/internal/provider/openstack/client.go index 36828598552..59693a2af16 100644 --- a/internal/provider/openstack/client.go +++ b/internal/provider/openstack/client.go @@ -13,7 +13,7 @@ import ( "github.com/go-goose/goose/v5/neutron" "github.com/go-goose/goose/v5/nova" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" corelogger "github.com/juju/juju/core/logger" environscloudspec "github.com/juju/juju/environs/cloudspec" diff --git a/internal/provider/openstack/provider.go b/internal/provider/openstack/provider.go index f85b6a10c6e..ae68995830e 100644 --- a/internal/provider/openstack/provider.go +++ b/internal/provider/openstack/provider.go @@ -30,8 +30,8 @@ import ( "github.com/juju/collections/set" "github.com/juju/collections/transform" "github.com/juju/errors" - "github.com/juju/http/v2" "github.com/juju/jsonschema" + "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "github.com/juju/retry" "github.com/juju/utils/v4" diff --git a/internal/s3client/http.go b/internal/s3client/http.go index 9f9cdfc2c8f..2d66c7d4fe9 100644 --- a/internal/s3client/http.go +++ b/internal/s3client/http.go @@ -4,7 +4,7 @@ package s3client import ( - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/core/logger" ) diff --git a/internal/upgrades/upgradevalidation/validation.go b/internal/upgrades/upgradevalidation/validation.go index 9d7a2f39d6b..dd93243ab24 100644 --- a/internal/upgrades/upgradevalidation/validation.go +++ b/internal/upgrades/upgradevalidation/validation.go @@ -10,7 +10,7 @@ import ( "strings" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/replicaset/v3" "github.com/juju/version/v2" diff --git a/internal/worker/httpserver/tls.go b/internal/worker/httpserver/tls.go index 47891e9a1f3..8f3e0e6669b 100644 --- a/internal/worker/httpserver/tls.go +++ b/internal/worker/httpserver/tls.go @@ -10,7 +10,7 @@ import ( "strings" "github.com/juju/errors" - "github.com/juju/http/v2" + "github.com/juju/juju/internal/http" "golang.org/x/crypto/acme" "golang.org/x/crypto/acme/autocert" diff --git a/internal/worker/upgrader/upgrader.go b/internal/worker/upgrader/upgrader.go index fd0be1c9f34..0fc7ffc7c61 100644 --- a/internal/worker/upgrader/upgrader.go +++ b/internal/worker/upgrader/upgrader.go @@ -12,7 +12,7 @@ import ( "github.com/juju/clock" "github.com/juju/errors" - jujuhttp "github.com/juju/http/v2" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "github.com/juju/version/v2" "github.com/juju/worker/v4/catacomb" From 6ac6178a20de3a9a370ea65dc6df20e1cf11a439 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 21 May 2024 09:30:56 +0100 Subject: [PATCH 2/4] Remove the httpLogger shim The http logger shim was required when we needed to massage the core logger interface into the http logger interface. --- cmd/juju/cloud/updatepublicclouds.go | 14 ++------------ cmd/juju/controller/register.go | 12 +----------- environs/simplestreams/datasource.go | 12 +----------- internal/charmhub/http.go | 12 +----------- internal/downloader/utils.go | 14 ++------------ internal/provider/ec2/environ.go | 12 +----------- internal/provider/gce/environ.go | 12 +----------- internal/provider/lxd/provider.go | 12 +----------- internal/provider/openstack/client.go | 12 +----------- internal/s3client/http.go | 12 +----------- internal/upgrades/upgradevalidation/validation.go | 12 +----------- 11 files changed, 13 insertions(+), 123 deletions(-) diff --git a/cmd/juju/cloud/updatepublicclouds.go b/cmd/juju/cloud/updatepublicclouds.go index ae416667aa0..9b8d4215334 100644 --- a/cmd/juju/cloud/updatepublicclouds.go +++ b/cmd/juju/cloud/updatepublicclouds.go @@ -15,7 +15,6 @@ import ( "github.com/juju/cmd/v4" "github.com/juju/collections/set" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "golang.org/x/crypto/openpgp" "golang.org/x/crypto/openpgp/clearsign" @@ -25,6 +24,7 @@ import ( jujucmd "github.com/juju/juju/cmd" "github.com/juju/juju/cmd/modelcmd" corelogger "github.com/juju/juju/core/logger" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/juju/keys" "github.com/juju/juju/jujuclient" ) @@ -108,20 +108,10 @@ func (c *updatePublicCloudsCommand) Init(args []string) error { return cmd.CheckEmpty(args) } -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} - // PublishedPublicClouds retrieves public cloud information from the given URL. func PublishedPublicClouds(ctx context.Context, url, key string) (map[string]jujucloud.Cloud, error) { client := jujuhttp.NewClient( - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("http", corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("http", corelogger.HTTP)), ) resp, err := client.Get(ctx, url) if err != nil { diff --git a/cmd/juju/controller/register.go b/cmd/juju/controller/register.go index 94f43e36d69..42d7a638879 100644 --- a/cmd/juju/controller/register.go +++ b/cmd/juju/controller/register.go @@ -636,9 +636,7 @@ func (c *registerCommand) secretKeyLogin( httpClient := jujuhttp.NewClient( jujuhttp.WithSkipHostnameVerification(true), jujuhttp.WithCookieJar(cookieJar), - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("http", corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("http", corelogger.HTTP)), ) httpResp, err := httpClient.Do(httpReq) if err != nil { @@ -797,11 +795,3 @@ func genAlreadyRegisteredError(controller, user string) error { } return errors.New(buf.String()) } - -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} diff --git a/environs/simplestreams/datasource.go b/environs/simplestreams/datasource.go index d1afce4beca..e2350ceb1b4 100644 --- a/environs/simplestreams/datasource.go +++ b/environs/simplestreams/datasource.go @@ -134,9 +134,7 @@ func NewDataSource(cfg Config) DataSource { client := jujuhttp.NewClient( jujuhttp.WithSkipHostnameVerification(!cfg.HostnameVerification), jujuhttp.WithCACertificates(cfg.CACertificates...), - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("http", corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("http", corelogger.HTTP)), ) return NewDataSourceWithClient(cfg, client) } @@ -246,11 +244,3 @@ func (h *urlDataSource) Priority() int { func (h *urlDataSource) RequireSigned() bool { return h.requireSigned } - -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} diff --git a/internal/charmhub/http.go b/internal/charmhub/http.go index fd43f3482d5..2f3a1f3df7b 100644 --- a/internal/charmhub/http.go +++ b/internal/charmhub/http.go @@ -104,9 +104,7 @@ func requestHTTPClient(recorder jujuhttp.RequestRecorder, policy jujuhttp.RetryP return jujuhttp.NewClient( jujuhttp.WithRequestRecorder(recorder), jujuhttp.WithRequestRetrier(policy), - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("transport", corelogger.CHARMHUB, corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("transport", corelogger.CHARMHUB, corelogger.HTTP)), ) } } @@ -363,11 +361,3 @@ func (c *httpRESTClient) Post(ctx context.Context, path path.Path, headers http. StatusCode: resp.StatusCode, }, nil } - -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} diff --git a/internal/downloader/utils.go b/internal/downloader/utils.go index feb768649c2..53def1d0247 100644 --- a/internal/downloader/utils.go +++ b/internal/downloader/utils.go @@ -10,10 +10,10 @@ import ( "os" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/utils/v4" corelogger "github.com/juju/juju/core/logger" + jujuhttp "github.com/juju/juju/internal/http" ) // NewHTTPBlobOpener returns a blob opener func suitable for use with @@ -24,9 +24,7 @@ func NewHTTPBlobOpener(hostnameVerification bool) func(Request) (io.ReadCloser, // TODO(rog) make the download operation interruptible. client := jujuhttp.NewClient( jujuhttp.WithSkipHostnameVerification(!hostnameVerification), - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("http", corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("http", corelogger.HTTP)), ) resp, err := client.Get(context.TODO(), req.URL.String()) @@ -63,11 +61,3 @@ func NewSha256Verifier(expected string) func(*os.File) error { return nil } } - -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} diff --git a/internal/provider/ec2/environ.go b/internal/provider/ec2/environ.go index 525d8df0135..5cb3f5ebe30 100644 --- a/internal/provider/ec2/environ.go +++ b/internal/provider/ec2/environ.go @@ -2767,9 +2767,7 @@ func (e *environ) SetCloudSpec(ctx stdcontext.Context, spec environscloudspec.Cl } httpClient := jujuhttp.NewClient( - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("http", corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("http", corelogger.HTTP)), ) var err error @@ -2813,11 +2811,3 @@ func CreateTagSpecification(resourceType types.ResourceType, tags map[string]str return spec } - -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} diff --git a/internal/provider/gce/environ.go b/internal/provider/gce/environ.go index dcf3a7b8662..365db7f074a 100644 --- a/internal/provider/gce/environ.go +++ b/internal/provider/gce/environ.go @@ -165,9 +165,7 @@ func (e *environ) SetCloudSpec(_ stdcontext.Context, spec environscloudspec.Clou // of the environ. HTTPClient: jujuhttp.NewClient( jujuhttp.WithSkipHostnameVerification(spec.SkipTLSVerify), - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("http", corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("http", corelogger.HTTP)), ), } @@ -289,11 +287,3 @@ func (env *environ) DestroyController(ctx envcontext.ProviderCallContext, contro // TODO(wallyworld): destroy hosted model resources return env.Destroy(ctx) } - -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} diff --git a/internal/provider/lxd/provider.go b/internal/provider/lxd/provider.go index 07339dc692f..c566159b4bb 100644 --- a/internal/provider/lxd/provider.go +++ b/internal/provider/lxd/provider.go @@ -120,9 +120,7 @@ func NewProvider() environs.CloudEnvironProvider { configReader := lxcConfigReader{} factory := NewServerFactory(NewHTTPClientFunc(func() *http.Client { return jujuhttp.NewClient( - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("http", corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("http", corelogger.HTTP)), ).Client() })) @@ -433,11 +431,3 @@ func (lxcConfigReader) ReadCert(path string) ([]byte, error) { certFile, err := os.ReadFile(path) return certFile, errors.Trace(err) } - -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} diff --git a/internal/provider/openstack/client.go b/internal/provider/openstack/client.go index 59693a2af16..3760de33e6d 100644 --- a/internal/provider/openstack/client.go +++ b/internal/provider/openstack/client.go @@ -254,20 +254,10 @@ func newClient( httpClient := jujuhttp.NewClient( jujuhttp.WithSkipHostnameVerification(opts.skipHostnameVerification), jujuhttp.WithCACertificates(opts.caCertificates...), - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("http", corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("http", corelogger.HTTP)), ) return client.NewClient(&cred, authMode, gooseLogger, client.WithHTTPClient(httpClient.Client()), client.WithHTTPHeadersFunc(opts.httpHeadersFunc), ), nil } - -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} diff --git a/internal/s3client/http.go b/internal/s3client/http.go index 2d66c7d4fe9..8928edf27b9 100644 --- a/internal/s3client/http.go +++ b/internal/s3client/http.go @@ -13,16 +13,6 @@ import ( // store. func DefaultHTTPClient(logger logger.Logger) HTTPClient { return jujuhttp.NewClient( - jujuhttp.WithLogger(httpLogger{ - Logger: logger, - }), + jujuhttp.WithLogger(logger), ) } - -type httpLogger struct { - logger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(logger.TRACE) -} diff --git a/internal/upgrades/upgradevalidation/validation.go b/internal/upgrades/upgradevalidation/validation.go index dd93243ab24..3cec6ce41bb 100644 --- a/internal/upgrades/upgradevalidation/validation.go +++ b/internal/upgrades/upgradevalidation/validation.go @@ -292,9 +292,7 @@ func getCheckForLXDVersion(cloudspec environscloudspec.CloudSpec) Validator { } server, err := NewServerFactory(lxd.NewHTTPClientFunc(func() *http.Client { return jujuhttp.NewClient( - jujuhttp.WithLogger(httpLogger{ - Logger: logger.Child("http", corelogger.HTTP), - }), + jujuhttp.WithLogger(logger.Child("http", corelogger.HTTP)), ).Client() })).RemoteServer(lxd.CloudSpec{CloudSpec: cloudspec}) if err != nil { @@ -307,11 +305,3 @@ func getCheckForLXDVersion(cloudspec environscloudspec.CloudSpec) Validator { return nil, errors.Trace(err) } } - -type httpLogger struct { - corelogger.Logger -} - -func (l httpLogger) IsTraceEnabled() bool { - return l.IsLevelEnabled(corelogger.TRACE) -} From f0a9513635a5bee4727bd650c4e88e486a7d5705 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 21 May 2024 11:11:48 +0100 Subject: [PATCH 3/4] Remove the local logger interface We no longer need the local logger interface, instead we have the central core/logger interface. --- internal/http/client_mock_test.go | 147 +----------------------------- internal/http/package_test.go | 2 +- 2 files changed, 3 insertions(+), 146 deletions(-) diff --git a/internal/http/client_mock_test.go b/internal/http/client_mock_test.go index a46fa3d5c94..2e974a899fe 100644 --- a/internal/http/client_mock_test.go +++ b/internal/http/client_mock_test.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/internal/http (interfaces: HTTPClient,RequestRecorder,Logger) +// Source: github.com/juju/juju/internal/http (interfaces: HTTPClient,RequestRecorder) // // Generated by this command: // -// mockgen -typed -package http -destination client_mock_test.go github.com/juju/juju/internal/http HTTPClient,RequestRecorder,Logger +// mockgen -typed -package http -destination client_mock_test.go github.com/juju/juju/internal/http HTTPClient,RequestRecorder // // Package http is a generated GoMock package. @@ -174,146 +174,3 @@ func (c *MockRequestRecorderRecordErrorCall) DoAndReturn(f func(string, *url.URL c.Call = c.Call.DoAndReturn(f) return c } - -// MockLogger is a mock of Logger interface. -type MockLogger struct { - ctrl *gomock.Controller - recorder *MockLoggerMockRecorder -} - -// MockLoggerMockRecorder is the mock recorder for MockLogger. -type MockLoggerMockRecorder struct { - mock *MockLogger -} - -// NewMockLogger creates a new mock instance. -func NewMockLogger(ctrl *gomock.Controller) *MockLogger { - mock := &MockLogger{ctrl: ctrl} - mock.recorder = &MockLoggerMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockLogger) EXPECT() *MockLoggerMockRecorder { - return m.recorder -} - -// Errorf mocks base method. -func (m *MockLogger) Errorf(arg0 string, arg1 ...any) { - m.ctrl.T.Helper() - varargs := []any{arg0} - for _, a := range arg1 { - varargs = append(varargs, a) - } - m.ctrl.Call(m, "Errorf", varargs...) -} - -// Errorf indicates an expected call of Errorf. -func (mr *MockLoggerMockRecorder) Errorf(arg0 any, arg1 ...any) *MockLoggerErrorfCall { - mr.mock.ctrl.T.Helper() - varargs := append([]any{arg0}, arg1...) - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Errorf", reflect.TypeOf((*MockLogger)(nil).Errorf), varargs...) - return &MockLoggerErrorfCall{Call: call} -} - -// MockLoggerErrorfCall wrap *gomock.Call -type MockLoggerErrorfCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockLoggerErrorfCall) Return() *MockLoggerErrorfCall { - c.Call = c.Call.Return() - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockLoggerErrorfCall) Do(f func(string, ...any)) *MockLoggerErrorfCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockLoggerErrorfCall) DoAndReturn(f func(string, ...any)) *MockLoggerErrorfCall { - c.Call = c.Call.DoAndReturn(f) - return c -} - -// IsTraceEnabled mocks base method. -func (m *MockLogger) IsTraceEnabled() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsTraceEnabled") - ret0, _ := ret[0].(bool) - return ret0 -} - -// IsTraceEnabled indicates an expected call of IsTraceEnabled. -func (mr *MockLoggerMockRecorder) IsTraceEnabled() *MockLoggerIsTraceEnabledCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsTraceEnabled", reflect.TypeOf((*MockLogger)(nil).IsTraceEnabled)) - return &MockLoggerIsTraceEnabledCall{Call: call} -} - -// MockLoggerIsTraceEnabledCall wrap *gomock.Call -type MockLoggerIsTraceEnabledCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockLoggerIsTraceEnabledCall) Return(arg0 bool) *MockLoggerIsTraceEnabledCall { - c.Call = c.Call.Return(arg0) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockLoggerIsTraceEnabledCall) Do(f func() bool) *MockLoggerIsTraceEnabledCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockLoggerIsTraceEnabledCall) DoAndReturn(f func() bool) *MockLoggerIsTraceEnabledCall { - c.Call = c.Call.DoAndReturn(f) - return c -} - -// Tracef mocks base method. -func (m *MockLogger) Tracef(arg0 string, arg1 ...any) { - m.ctrl.T.Helper() - varargs := []any{arg0} - for _, a := range arg1 { - varargs = append(varargs, a) - } - m.ctrl.Call(m, "Tracef", varargs...) -} - -// Tracef indicates an expected call of Tracef. -func (mr *MockLoggerMockRecorder) Tracef(arg0 any, arg1 ...any) *MockLoggerTracefCall { - mr.mock.ctrl.T.Helper() - varargs := append([]any{arg0}, arg1...) - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Tracef", reflect.TypeOf((*MockLogger)(nil).Tracef), varargs...) - return &MockLoggerTracefCall{Call: call} -} - -// MockLoggerTracefCall wrap *gomock.Call -type MockLoggerTracefCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockLoggerTracefCall) Return() *MockLoggerTracefCall { - c.Call = c.Call.Return() - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockLoggerTracefCall) Do(f func(string, ...any)) *MockLoggerTracefCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockLoggerTracefCall) DoAndReturn(f func(string, ...any)) *MockLoggerTracefCall { - c.Call = c.Call.DoAndReturn(f) - return c -} diff --git a/internal/http/package_test.go b/internal/http/package_test.go index b0da07d052d..5e3380a1fcb 100644 --- a/internal/http/package_test.go +++ b/internal/http/package_test.go @@ -9,7 +9,7 @@ import ( gc "gopkg.in/check.v1" ) -//go:generate go run go.uber.org/mock/mockgen -typed -package http -destination client_mock_test.go github.com/juju/juju/internal/http HTTPClient,RequestRecorder,Logger +//go:generate go run go.uber.org/mock/mockgen -typed -package http -destination client_mock_test.go github.com/juju/juju/internal/http HTTPClient,RequestRecorder //go:generate go run go.uber.org/mock/mockgen -typed -package http -destination http_mock_test.go github.com/juju/juju/internal/http RoundTripper //go:generate go run go.uber.org/mock/mockgen -typed -package http -destination clock_mock_test.go github.com/juju/clock Clock From 79c063b82425b44ac8bfdd98506a1cc90807696d Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 21 May 2024 11:23:25 +0100 Subject: [PATCH 4/4] Format imports based on package location --- api/apiclient.go | 2 +- api/package_test.go | 1 + .../facades/controller/charmdownloader/charmdownloader.go | 2 +- apiserver/pubsub_test.go | 2 +- apiserver/registration_test.go | 2 +- apiserver/server_test.go | 2 +- apiserver/testing/fakeapi.go | 2 +- apiserver/testing/http.go | 2 +- apiserver/tools.go | 2 +- apiserver/tools_integration_test.go | 2 +- cmd/containeragent/initialize/package_test.go | 1 + cmd/juju/controller/register.go | 2 +- environs/simplestreams/datasource.go | 2 +- environs/simplestreams/testing/testing.go | 2 +- environs/sync/sync.go | 2 +- environs/sync/sync_test.go | 2 +- environs/testing/tools.go | 2 +- internal/charmhub/http.go | 2 +- internal/charmhub/http_test.go | 3 ++- internal/http/middleware.go | 3 ++- internal/mongo/open.go | 3 ++- internal/provider/ec2/environ.go | 2 +- internal/provider/gce/environ.go | 5 ++--- internal/provider/gce/google/auth.go | 3 ++- internal/provider/gce/google/auth_test.go | 3 ++- internal/provider/gce/google/config.go | 1 + internal/provider/gce/google/config_connection_test.go | 2 +- internal/provider/gce/google/conn.go | 3 ++- internal/provider/gce/google/conn_test.go | 2 +- internal/provider/gce/google/testing_test.go | 2 +- internal/provider/lxd/provider.go | 7 +++---- internal/provider/openstack/client.go | 2 +- internal/provider/openstack/provider.go | 2 +- internal/s3client/http.go | 3 +-- internal/upgrades/upgradevalidation/validation.go | 2 +- internal/worker/httpserver/tls.go | 2 +- internal/worker/upgrader/upgrader.go | 2 +- 37 files changed, 46 insertions(+), 40 deletions(-) diff --git a/api/apiclient.go b/api/apiclient.go index 9cfa4c946ea..0c45e9e7a4d 100644 --- a/api/apiclient.go +++ b/api/apiclient.go @@ -25,7 +25,6 @@ import ( "github.com/gorilla/websocket" "github.com/juju/clock" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "github.com/juju/utils/v4" "github.com/juju/utils/v4/parallel" @@ -35,6 +34,7 @@ import ( "github.com/juju/juju/core/facades" coremacaroon "github.com/juju/juju/core/macaroon" "github.com/juju/juju/core/network" + jujuhttp "github.com/juju/juju/internal/http" internallogger "github.com/juju/juju/internal/logger" jujuproxy "github.com/juju/juju/internal/proxy" proxy "github.com/juju/juju/internal/proxy/config" diff --git a/api/package_test.go b/api/package_test.go index a6558f9188a..11bbaa2ed44 100644 --- a/api/package_test.go +++ b/api/package_test.go @@ -56,6 +56,7 @@ func (*ImportSuite) TestImports(c *gc.C) { "domain/secretbackend/errors", "environs/envcontext", "internal/feature", + "internal/http", "internal/logger", "internal/proxy", "internal/proxy/config", diff --git a/apiserver/facades/controller/charmdownloader/charmdownloader.go b/apiserver/facades/controller/charmdownloader/charmdownloader.go index be60e18de56..9c71679b736 100644 --- a/apiserver/facades/controller/charmdownloader/charmdownloader.go +++ b/apiserver/facades/controller/charmdownloader/charmdownloader.go @@ -10,12 +10,12 @@ import ( "github.com/juju/charm/v13" "github.com/juju/clock" "github.com/juju/errors" - "github.com/juju/juju/internal/http" "github.com/juju/names/v5" apiservererrors "github.com/juju/juju/apiserver/errors" corelogger "github.com/juju/juju/core/logger" "github.com/juju/juju/internal/charm/services" + "github.com/juju/juju/internal/http" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state/watcher" ) diff --git a/apiserver/pubsub_test.go b/apiserver/pubsub_test.go index bd4a8a441ec..d4f017982f9 100644 --- a/apiserver/pubsub_test.go +++ b/apiserver/pubsub_test.go @@ -12,7 +12,6 @@ import ( "time" "github.com/gorilla/websocket" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/loggo/v2" "github.com/juju/names/v5" "github.com/juju/pubsub/v2" @@ -24,6 +23,7 @@ import ( "github.com/juju/juju/core/permission" "github.com/juju/juju/domain/access/service" "github.com/juju/juju/internal/auth" + jujuhttp "github.com/juju/juju/internal/http" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" diff --git a/apiserver/registration_test.go b/apiserver/registration_test.go index 50089da3c43..f3971879b91 100644 --- a/apiserver/registration_test.go +++ b/apiserver/registration_test.go @@ -14,7 +14,6 @@ import ( "strings" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" "github.com/juju/testing/httptesting" "go.uber.org/mock/gomock" @@ -29,6 +28,7 @@ import ( "github.com/juju/juju/domain/access/service" "github.com/juju/juju/environs" "github.com/juju/juju/internal/auth" + jujuhttp "github.com/juju/juju/internal/http" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state/stateenvirons" diff --git a/apiserver/server_test.go b/apiserver/server_test.go index 8f918900595..61e8ebd757f 100644 --- a/apiserver/server_test.go +++ b/apiserver/server_test.go @@ -12,7 +12,6 @@ import ( "github.com/gorilla/websocket" jujuerrors "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/loggo/v2" "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" @@ -27,6 +26,7 @@ import ( apitesting "github.com/juju/juju/apiserver/testing" "github.com/juju/juju/core/network" "github.com/juju/juju/core/permission" + jujuhttp "github.com/juju/juju/internal/http" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" diff --git a/apiserver/testing/fakeapi.go b/apiserver/testing/fakeapi.go index a7b45def495..ee7fed55b63 100644 --- a/apiserver/testing/fakeapi.go +++ b/apiserver/testing/fakeapi.go @@ -13,13 +13,13 @@ import ( "github.com/bmizerany/pat" "github.com/gorilla/websocket" - jujuhttp "github.com/juju/juju/internal/http" apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/observer" "github.com/juju/juju/apiserver/observer/fakeobserver" apiwebsocket "github.com/juju/juju/apiserver/websocket" "github.com/juju/juju/core/trace" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/internal/rpcreflect" "github.com/juju/juju/rpc" "github.com/juju/juju/rpc/jsoncodec" diff --git a/apiserver/testing/http.go b/apiserver/testing/http.go index 9741fb58dbb..d9b9587ab3a 100644 --- a/apiserver/testing/http.go +++ b/apiserver/testing/http.go @@ -7,11 +7,11 @@ import ( "io" "net/http" - jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" "github.com/juju/testing/httptesting" gc "gopkg.in/check.v1" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/rpc/params" jujuversion "github.com/juju/juju/version" ) diff --git a/apiserver/tools.go b/apiserver/tools.go index af5dbfa21cc..6e191de573d 100644 --- a/apiserver/tools.go +++ b/apiserver/tools.go @@ -16,7 +16,6 @@ import ( "github.com/im7mortal/kmutex" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/version/v2" "github.com/juju/juju/apiserver/common" @@ -25,6 +24,7 @@ import ( "github.com/juju/juju/environs" "github.com/juju/juju/environs/simplestreams" envtools "github.com/juju/juju/environs/tools" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/internal/tools" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" diff --git a/apiserver/tools_integration_test.go b/apiserver/tools_integration_test.go index cd710202d3b..8c509545848 100644 --- a/apiserver/tools_integration_test.go +++ b/apiserver/tools_integration_test.go @@ -14,7 +14,6 @@ import ( "github.com/go-macaroon-bakery/macaroon-bakery/v3/bakery" "github.com/go-macaroon-bakery/macaroon-bakery/v3/httpbakery" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" @@ -24,6 +23,7 @@ import ( "github.com/juju/juju/core/permission" "github.com/juju/juju/domain/access/service" "github.com/juju/juju/internal/auth" + jujuhttp "github.com/juju/juju/internal/http" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/testing/factory" diff --git a/cmd/containeragent/initialize/package_test.go b/cmd/containeragent/initialize/package_test.go index 19f6e03205a..51e351df95e 100644 --- a/cmd/containeragent/initialize/package_test.go +++ b/cmd/containeragent/initialize/package_test.go @@ -86,6 +86,7 @@ func (*importSuite) TestImports(c *gc.C) { "internal/charmhub/path", "internal/charmhub/transport", "internal/feature", + "internal/http", "internal/logger", "internal/mongo", "internal/network", diff --git a/cmd/juju/controller/register.go b/cmd/juju/controller/register.go index 42d7a638879..01781aea515 100644 --- a/cmd/juju/controller/register.go +++ b/cmd/juju/controller/register.go @@ -25,7 +25,6 @@ import ( "github.com/juju/collections/set" "github.com/juju/errors" "github.com/juju/gnuflag" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "golang.org/x/crypto/nacl/secretbox" "golang.org/x/crypto/ssh/terminal" @@ -39,6 +38,7 @@ import ( "github.com/juju/juju/cmd/modelcmd" corelogger "github.com/juju/juju/core/logger" "github.com/juju/juju/core/permission" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/internal/proxy/factory" "github.com/juju/juju/jujuclient" "github.com/juju/juju/rpc/params" diff --git a/environs/simplestreams/datasource.go b/environs/simplestreams/datasource.go index e2350ceb1b4..e2b89d82199 100644 --- a/environs/simplestreams/datasource.go +++ b/environs/simplestreams/datasource.go @@ -14,11 +14,11 @@ import ( "github.com/juju/clock" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/retry" "github.com/juju/utils/v4" corelogger "github.com/juju/juju/core/logger" + jujuhttp "github.com/juju/juju/internal/http" ) // A DataSource retrieves simplestreams metadata. diff --git a/environs/simplestreams/testing/testing.go b/environs/simplestreams/testing/testing.go index 94d038be18e..644bc699cf0 100644 --- a/environs/simplestreams/testing/testing.go +++ b/environs/simplestreams/testing/testing.go @@ -12,11 +12,11 @@ import ( "net/http" "strings" - jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" "github.com/juju/juju/environs/simplestreams" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/testing" ) diff --git a/environs/sync/sync.go b/environs/sync/sync.go index f769b3615c6..64cd9a518c2 100644 --- a/environs/sync/sync.go +++ b/environs/sync/sync.go @@ -11,7 +11,6 @@ import ( "path/filepath" "github.com/juju/errors" - "github.com/juju/juju/internal/http" "github.com/juju/utils/v4" "github.com/juju/version/v2" @@ -19,6 +18,7 @@ import ( "github.com/juju/juju/environs/simplestreams" "github.com/juju/juju/environs/storage" envtools "github.com/juju/juju/environs/tools" + "github.com/juju/juju/internal/http" internallogger "github.com/juju/juju/internal/logger" coretools "github.com/juju/juju/internal/tools" "github.com/juju/juju/juju/keys" diff --git a/environs/sync/sync_test.go b/environs/sync/sync_test.go index 3e08620ffd1..aebbb80b1b2 100644 --- a/environs/sync/sync_test.go +++ b/environs/sync/sync_test.go @@ -16,7 +16,6 @@ import ( "path/filepath" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" jujutesting "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/utils/v4/tar" @@ -35,6 +34,7 @@ import ( envtesting "github.com/juju/juju/environs/testing" envtools "github.com/juju/juju/environs/tools" toolstesting "github.com/juju/juju/environs/tools/testing" + jujuhttp "github.com/juju/juju/internal/http" coretools "github.com/juju/juju/internal/tools" "github.com/juju/juju/juju/names" coretesting "github.com/juju/juju/testing" diff --git a/environs/testing/tools.go b/environs/testing/tools.go index 34a0bfbb54f..68d86d31bdf 100644 --- a/environs/testing/tools.go +++ b/environs/testing/tools.go @@ -13,7 +13,6 @@ import ( "strings" "github.com/juju/collections/set" - "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" "github.com/juju/version/v2" gc "gopkg.in/check.v1" @@ -27,6 +26,7 @@ import ( sstesting "github.com/juju/juju/environs/simplestreams/testing" "github.com/juju/juju/environs/storage" envtools "github.com/juju/juju/environs/tools" + "github.com/juju/juju/internal/http" coretools "github.com/juju/juju/internal/tools" "github.com/juju/juju/juju/names" coretesting "github.com/juju/juju/testing" diff --git a/internal/charmhub/http.go b/internal/charmhub/http.go index 2f3a1f3df7b..dacd4d51c91 100644 --- a/internal/charmhub/http.go +++ b/internal/charmhub/http.go @@ -16,12 +16,12 @@ import ( "github.com/juju/clock" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/retry" "gopkg.in/httprequest.v1" corelogger "github.com/juju/juju/core/logger" "github.com/juju/juju/internal/charmhub/path" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/version" ) diff --git a/internal/charmhub/http_test.go b/internal/charmhub/http_test.go index b8f61be8bf8..ff2f1a255f9 100644 --- a/internal/charmhub/http_test.go +++ b/internal/charmhub/http_test.go @@ -14,11 +14,12 @@ import ( "time" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/testing" jc "github.com/juju/testing/checkers" "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" + + jujuhttp "github.com/juju/juju/internal/http" ) type APIRequesterSuite struct { diff --git a/internal/http/middleware.go b/internal/http/middleware.go index 9f6dfad7750..dc55afdeed2 100644 --- a/internal/http/middleware.go +++ b/internal/http/middleware.go @@ -12,10 +12,11 @@ import ( "github.com/juju/clock" "github.com/juju/errors" - "github.com/juju/juju/core/logger" "github.com/juju/loggo/v2" "github.com/juju/retry" "golang.org/x/net/http/httpproxy" + + "github.com/juju/juju/core/logger" ) // FileProtocolMiddleware registers support for file:// URLs on the given transport. diff --git a/internal/mongo/open.go b/internal/mongo/open.go index ebfaf735c0e..3dd8383b0be 100644 --- a/internal/mongo/open.go +++ b/internal/mongo/open.go @@ -13,10 +13,11 @@ import ( "time" "github.com/juju/errors" - "github.com/juju/juju/internal/http" "github.com/juju/mgo/v3" "github.com/juju/names/v5" "github.com/juju/utils/v4/cert" + + "github.com/juju/juju/internal/http" ) // SocketTimeout should be long enough that even a slow mongo server diff --git a/internal/provider/ec2/environ.go b/internal/provider/ec2/environ.go index 5cb3f5ebe30..6d585ddb17b 100644 --- a/internal/provider/ec2/environ.go +++ b/internal/provider/ec2/environ.go @@ -23,7 +23,6 @@ import ( "github.com/juju/clock" "github.com/juju/collections/set" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "github.com/juju/retry" "github.com/juju/version/v2" @@ -49,6 +48,7 @@ import ( "github.com/juju/juju/environs/tags" "github.com/juju/juju/internal/cloudconfig/instancecfg" "github.com/juju/juju/internal/cloudconfig/providerinit" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/internal/provider/common" "github.com/juju/juju/internal/storage" ) diff --git a/internal/provider/gce/environ.go b/internal/provider/gce/environ.go index 365db7f074a..7fd6f71f682 100644 --- a/internal/provider/gce/environ.go +++ b/internal/provider/gce/environ.go @@ -4,14 +4,12 @@ package gce import ( - "context" stdcontext "context" "strings" "sync" "time" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "google.golang.org/api/compute/v1" jujucloud "github.com/juju/juju/cloud" @@ -25,6 +23,7 @@ import ( "github.com/juju/juju/environs/envcontext" "github.com/juju/juju/environs/instances" "github.com/juju/juju/environs/simplestreams" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/internal/provider/common" "github.com/juju/juju/internal/provider/gce/google" ) @@ -200,7 +199,7 @@ func (env *environ) Region() (simplestreams.CloudSpec, error) { } // SetConfig updates the env's configuration. -func (env *environ) SetConfig(ctx context.Context, cfg *config.Config) error { +func (env *environ) SetConfig(ctx stdcontext.Context, cfg *config.Config) error { env.lock.Lock() defer env.lock.Unlock() diff --git a/internal/provider/gce/google/auth.go b/internal/provider/gce/google/auth.go index 8a10b0897a0..1b47f3f8055 100644 --- a/internal/provider/gce/google/auth.go +++ b/internal/provider/gce/google/auth.go @@ -7,12 +7,13 @@ import ( "context" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "golang.org/x/oauth2/google" "golang.org/x/oauth2/jwt" "google.golang.org/api/compute/v1" "google.golang.org/api/option" transporthttp "google.golang.org/api/transport/http" + + jujuhttp "github.com/juju/juju/internal/http" ) var scopes = []string{ diff --git a/internal/provider/gce/google/auth_test.go b/internal/provider/gce/google/auth_test.go index 639e74ca11b..4b19b78fbd3 100644 --- a/internal/provider/gce/google/auth_test.go +++ b/internal/provider/gce/google/auth_test.go @@ -6,9 +6,10 @@ package google import ( "context" - jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" + + jujuhttp "github.com/juju/juju/internal/http" ) type authSuite struct { diff --git a/internal/provider/gce/google/config.go b/internal/provider/gce/google/config.go index eeac394affd..dccfa3862fb 100644 --- a/internal/provider/gce/google/config.go +++ b/internal/provider/gce/google/config.go @@ -9,6 +9,7 @@ import ( "net/mail" "github.com/juju/errors" + jujuhttp "github.com/juju/juju/internal/http" ) diff --git a/internal/provider/gce/google/config_connection_test.go b/internal/provider/gce/google/config_connection_test.go index 5a104e43779..f7b14448620 100644 --- a/internal/provider/gce/google/config_connection_test.go +++ b/internal/provider/gce/google/config_connection_test.go @@ -4,10 +4,10 @@ package google_test import ( - jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/internal/provider/gce/google" ) diff --git a/internal/provider/gce/google/conn.go b/internal/provider/gce/google/conn.go index d1fd888f0dc..ed7912a5a70 100644 --- a/internal/provider/gce/google/conn.go +++ b/internal/provider/gce/google/conn.go @@ -7,8 +7,9 @@ import ( "context" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "google.golang.org/api/compute/v1" + + jujuhttp "github.com/juju/juju/internal/http" ) // service facilitates mocking out the GCE API during tests. diff --git a/internal/provider/gce/google/conn_test.go b/internal/provider/gce/google/conn_test.go index fd55569b6d9..575868033a9 100644 --- a/internal/provider/gce/google/conn_test.go +++ b/internal/provider/gce/google/conn_test.go @@ -7,11 +7,11 @@ import ( "context" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" jc "github.com/juju/testing/checkers" "google.golang.org/api/compute/v1" gc "gopkg.in/check.v1" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/internal/provider/gce/google" ) diff --git a/internal/provider/gce/google/testing_test.go b/internal/provider/gce/google/testing_test.go index b1baa0e9945..74834f0c457 100644 --- a/internal/provider/gce/google/testing_test.go +++ b/internal/provider/gce/google/testing_test.go @@ -4,11 +4,11 @@ package google import ( - jujuhttp "github.com/juju/juju/internal/http" "google.golang.org/api/compute/v1" gc "gopkg.in/check.v1" "github.com/juju/juju/core/network" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/testing" ) diff --git a/internal/provider/lxd/provider.go b/internal/provider/lxd/provider.go index c566159b4bb..935abe1e53e 100644 --- a/internal/provider/lxd/provider.go +++ b/internal/provider/lxd/provider.go @@ -4,7 +4,6 @@ package lxd import ( - "context" stdcontext "context" "net/http" "os" @@ -14,7 +13,6 @@ import ( "github.com/juju/clock" "github.com/juju/errors" "github.com/juju/jsonschema" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/schema" "gopkg.in/juju/environschema.v1" "gopkg.in/yaml.v2" @@ -26,6 +24,7 @@ import ( "github.com/juju/juju/environs/config" "github.com/juju/juju/environs/envcontext" "github.com/juju/juju/internal/container/lxd" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/internal/provider/lxd/lxdnames" ) @@ -189,7 +188,7 @@ func (p *environProvider) Ping(ctx envcontext.ProviderCallContext, endpoint stri } // PrepareConfig implements environs.EnvironProvider. -func (p *environProvider) PrepareConfig(ctx context.Context, args environs.PrepareConfigParams) (*config.Config, error) { +func (p *environProvider) PrepareConfig(ctx stdcontext.Context, args environs.PrepareConfigParams) (*config.Config, error) { if err := p.validateCloudSpec(args.Cloud); err != nil { return nil, errors.Annotate(err, "validating cloud spec") } @@ -206,7 +205,7 @@ func (p *environProvider) PrepareConfig(ctx context.Context, args environs.Prepa } // Validate implements environs.EnvironProvider. -func (*environProvider) Validate(ctx context.Context, cfg, old *config.Config) (valid *config.Config, err error) { +func (*environProvider) Validate(ctx stdcontext.Context, cfg, old *config.Config) (valid *config.Config, err error) { if _, err := newValidConfig(ctx, cfg); err != nil { return nil, errors.Annotate(err, "invalid base config") } diff --git a/internal/provider/openstack/client.go b/internal/provider/openstack/client.go index 3760de33e6d..6dd49299359 100644 --- a/internal/provider/openstack/client.go +++ b/internal/provider/openstack/client.go @@ -13,10 +13,10 @@ import ( "github.com/go-goose/goose/v5/neutron" "github.com/go-goose/goose/v5/nova" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" corelogger "github.com/juju/juju/core/logger" environscloudspec "github.com/juju/juju/environs/cloudspec" + jujuhttp "github.com/juju/juju/internal/http" internallogger "github.com/juju/juju/internal/logger" ) diff --git a/internal/provider/openstack/provider.go b/internal/provider/openstack/provider.go index ae68995830e..a3469b082d9 100644 --- a/internal/provider/openstack/provider.go +++ b/internal/provider/openstack/provider.go @@ -31,7 +31,6 @@ import ( "github.com/juju/collections/transform" "github.com/juju/errors" "github.com/juju/jsonschema" - "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "github.com/juju/retry" "github.com/juju/utils/v4" @@ -55,6 +54,7 @@ import ( "github.com/juju/juju/internal/cloudconfig/cloudinit" "github.com/juju/juju/internal/cloudconfig/instancecfg" "github.com/juju/juju/internal/cloudconfig/providerinit" + "github.com/juju/juju/internal/http" internallogger "github.com/juju/juju/internal/logger" "github.com/juju/juju/internal/provider/common" "github.com/juju/juju/internal/storage" diff --git a/internal/s3client/http.go b/internal/s3client/http.go index 8928edf27b9..ce59e2bd3a8 100644 --- a/internal/s3client/http.go +++ b/internal/s3client/http.go @@ -4,9 +4,8 @@ package s3client import ( - jujuhttp "github.com/juju/juju/internal/http" - "github.com/juju/juju/core/logger" + jujuhttp "github.com/juju/juju/internal/http" ) // DefaultHTTPClient returns the default http client used to access the object diff --git a/internal/upgrades/upgradevalidation/validation.go b/internal/upgrades/upgradevalidation/validation.go index 3cec6ce41bb..575873bde92 100644 --- a/internal/upgrades/upgradevalidation/validation.go +++ b/internal/upgrades/upgradevalidation/validation.go @@ -10,13 +10,13 @@ import ( "strings" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/replicaset/v3" "github.com/juju/version/v2" corebase "github.com/juju/juju/core/base" corelogger "github.com/juju/juju/core/logger" environscloudspec "github.com/juju/juju/environs/cloudspec" + jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/juju/internal/provider/lxd" "github.com/juju/juju/internal/provider/lxd/lxdnames" "github.com/juju/juju/state" diff --git a/internal/worker/httpserver/tls.go b/internal/worker/httpserver/tls.go index 8f3e0e6669b..ff8bc576e2e 100644 --- a/internal/worker/httpserver/tls.go +++ b/internal/worker/httpserver/tls.go @@ -10,11 +10,11 @@ import ( "strings" "github.com/juju/errors" - "github.com/juju/juju/internal/http" "golang.org/x/crypto/acme" "golang.org/x/crypto/acme/autocert" "github.com/juju/juju/core/logger" + "github.com/juju/juju/internal/http" ) // SNIGetterFunc is a helper function that aids the TLS SNI process by working diff --git a/internal/worker/upgrader/upgrader.go b/internal/worker/upgrader/upgrader.go index 0fc7ffc7c61..634b76c895b 100644 --- a/internal/worker/upgrader/upgrader.go +++ b/internal/worker/upgrader/upgrader.go @@ -12,7 +12,6 @@ import ( "github.com/juju/clock" "github.com/juju/errors" - jujuhttp "github.com/juju/juju/internal/http" "github.com/juju/names/v5" "github.com/juju/version/v2" "github.com/juju/worker/v4/catacomb" @@ -24,6 +23,7 @@ import ( "github.com/juju/juju/core/logger" coreos "github.com/juju/juju/core/os" "github.com/juju/juju/core/watcher" + jujuhttp "github.com/juju/juju/internal/http" coretools "github.com/juju/juju/internal/tools" "github.com/juju/juju/internal/upgrades" "github.com/juju/juju/internal/worker/gate"