Merge pull request juju#17393 from SimonRichardson/intern-http-package

juju#17393

This brings the http/v2 package into the internal package. Packages in the github.com/juju namespace tend to be left to rot, this is less than ideal when there is a list of known ciphers in the library. These should be audited to ensure that they do indeed cover what we want to set as a project.

```go
// 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,
}
```

As an aside, interning the library also removes the logger shim that I had to put in last month. Again, we're striving for a much easier and more manageable project, having shims is a roadblock for that to succeed.

## Checklist

<!-- If an item is not applicable, use `~strikethrough~`. -->

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing

## QA steps

### Bootstrap + Deploy

Ensure that we can still bootstrap and deploy the controller charm and ubuntu.

```sh
$ juju bootstrap lxd test
$ juju add-model default
$ juju deploy ubuntu
```

### Charmhub

```sh
$ juju find postres
$ juju download ubuntu
$ juju info ubuntu
```

## Links

**Jira card:** JUJU-

api/apiclient.go

@@ -25,7 +25,6 @@ import (
 	"github.com/gorilla/websocket"
 	"github.com/juju/clock"
 	"github.com/juju/errors"
-	jujuhttp "github.com/juju/http/v2"
 	"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"

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",

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/http/v2"
 	"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"
 )

apiserver/pubsub_test.go

@@ -12,7 +12,6 @@ import (
 	"time"
 
 	"github.com/gorilla/websocket"
-	jujuhttp "github.com/juju/http/v2"
 	"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"

apiserver/registration_test.go

@@ -14,7 +14,6 @@ import (
 	"strings"
 
 	"github.com/juju/errors"
-	jujuhttp "github.com/juju/http/v2"
 	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"

apiserver/server_test.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/gorilla/websocket"
 	jujuerrors "github.com/juju/errors"
-	jujuhttp "github.com/juju/http/v2"
 	"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"

apiserver/testing/fakeapi.go

@@ -13,13 +13,13 @@ import (
 
 	"github.com/bmizerany/pat"
 	"github.com/gorilla/websocket"
-	jujuhttp "github.com/juju/http/v2"
 
 	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"

apiserver/testing/http.go

@@ -7,11 +7,11 @@ import (
 	"io"
 	"net/http"
 
-	jujuhttp "github.com/juju/http/v2"
 	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"
 )

apiserver/tools.go

@@ -16,7 +16,6 @@ import (
 
 	"github.com/im7mortal/kmutex"
 	"github.com/juju/errors"
-	jujuhttp "github.com/juju/http/v2"
 	"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"

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/http/v2"
 	"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"

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",

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/http/v2"
 	"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 {

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

environs/simplestreams/datasource.go

@@ -14,11 +14,11 @@ import (
 
 	"github.com/juju/clock"
 	"github.com/juju/errors"
-	jujuhttp "github.com/juju/http/v2"
 	"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.
@@ -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)
-}

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)

environs/simplestreams/testing/testing.go

@@ -12,11 +12,11 @@ import (
 	"net/http"
 	"strings"
 
-	jujuhttp "github.com/juju/http/v2"
 	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"
 )
 

environs/sync/sync.go

@@ -11,14 +11,14 @@ import (
 	"path/filepath"
 
 	"github.com/juju/errors"
-	"github.com/juju/http/v2"
 	"github.com/juju/utils/v4"
 	"github.com/juju/version/v2"
 
 	"github.com/juju/juju/environs/filestorage"
 	"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"

environs/sync/sync_test.go

@@ -16,7 +16,6 @@ import (
 	"path/filepath"
 
 	"github.com/juju/errors"
-	jujuhttp "github.com/juju/http/v2"
 	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"

environs/testing/tools.go

@@ -13,7 +13,6 @@ import (
 	"strings"
 
 	"github.com/juju/collections/set"
-	"github.com/juju/http/v2"
 	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"

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

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=

internal/charmhub/http.go

@@ -16,12 +16,12 @@ import (
 
 	"github.com/juju/clock"
 	"github.com/juju/errors"
-	jujuhttp "github.com/juju/http/v2"
 	"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"
 )
 
@@ -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)
-}