Merge pull request juju#18314 from SimonRichardson/http-client-worker

juju#18314

The http client worker inverts the dependency tree for http clients. There will now be a worker for a http client. If a provider requests one, then we can apply the proxies correctly in just one location.

This is step one in doing so.

<!-- 
The PR title should match: <type>(optional <scope>): <description>.

Please also ensure all commits in this PR comply with our conventional commits specification:
https://docs.google.com/document/d/1SYUo9G7qZ_jdoVXpUVamS5VCgHmtZ0QA-wZxKoMS-C0 
-->

<!-- Why this change is needed and what it does. -->


## QA steps

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

api/package_test.go

@@ -34,6 +34,7 @@ func (*ImportSuite) TestImports(c *gc.C) {
 		"core/credential",
 		"core/devices",
 		"core/facades",
+		"core/http",
 		"core/instance",
 		"core/life",
 		"core/logger",

apiserver/facade/facadetest/context.go

@@ -10,6 +10,7 @@ import (
 	"github.com/juju/names/v5"
 
 	"github.com/juju/juju/apiserver/facade"
+	corehttp "github.com/juju/juju/core/http"
 	"github.com/juju/juju/core/leadership"
 	"github.com/juju/juju/core/lease"
 	"github.com/juju/juju/core/logger"
@@ -174,11 +175,11 @@ func (c ModelContext) SingularClaimer() (lease.Claimer, error) {
 // understood by the context.
 // - [ErrorHTTPClientForPurposeNotFound] when no http client can be found for
 // the requested [HTTPClientPurpose].
-func (c ModelContext) HTTPClient(purpose facade.HTTPClientPurpose) (facade.HTTPClient, error) {
+func (c ModelContext) HTTPClient(purpose corehttp.Purpose) (facade.HTTPClient, error) {
 	var client facade.HTTPClient
 
 	switch purpose {
-	case facade.CharmhubHTTPClient:
+	case corehttp.CharmhubPurpose:
 		client = c.CharmhubHTTPClient_
 	default:
 		return nil, fmt.Errorf(

apiserver/facade/interface.go

@@ -12,6 +12,7 @@ import (
 	"github.com/juju/description/v8"
 	"github.com/juju/names/v5"
 
+	corehttp "github.com/juju/juju/core/http"
 	"github.com/juju/juju/core/leadership"
 	"github.com/juju/juju/core/lease"
 	corelogger "github.com/juju/juju/core/logger"
@@ -164,7 +165,7 @@ type ModelContext interface {
 	// understood by the context.
 	// - [ErrorHTTPClientForPurposeNotFound] when no http client can be found
 	// for the requested [HTTPClientPurpose].
-	HTTPClient(HTTPClientPurpose) (HTTPClient, error)
+	HTTPClient(corehttp.Purpose) (HTTPClient, error)
 
 	// MachineTag returns the current machine tag.
 	MachineTag() names.Tag
@@ -308,10 +309,3 @@ type Hub interface {
 type HTTPClient interface {
 	Do(*http.Request) (*http.Response, error)
 }
-
-// HTTPClientPurpose describes a specific purpose for an HTTP client.
-type HTTPClientPurpose string
-
-const (
-	CharmhubHTTPClient HTTPClientPurpose = "charmhub"
-)

apiserver/facades/client/application/application.go

@@ -30,6 +30,7 @@ import (
 	"github.com/juju/juju/core/config"
 	"github.com/juju/juju/core/constraints"
 	"github.com/juju/juju/core/crossmodel"
+	corehttp "github.com/juju/juju/core/http"
 	"github.com/juju/juju/core/instance"
 	"github.com/juju/juju/core/leadership"
 	corelogger "github.com/juju/juju/core/logger"
@@ -152,7 +153,7 @@ func newFacadeBase(stdCtx context.Context, ctx facade.ModelContext) (*APIBase, e
 		State: ctx.State(),
 	}
 
-	charmhubHTTPClient, err := ctx.HTTPClient(facade.CharmhubHTTPClient)
+	charmhubHTTPClient, err := ctx.HTTPClient(corehttp.CharmhubPurpose)
 	if err != nil {
 		return nil, fmt.Errorf(
 			"getting charm hub http client: %w",

apiserver/facades/client/charms/register.go

@@ -15,6 +15,7 @@ import (
 	apiservererrors "github.com/juju/juju/apiserver/errors"
 	"github.com/juju/juju/apiserver/facade"
 	corecharm "github.com/juju/juju/core/charm"
+	corehttp "github.com/juju/juju/core/http"
 	"github.com/juju/juju/internal/charm/services"
 )
 
@@ -54,7 +55,7 @@ func makeFacadeBase(stdCtx context.Context, ctx facade.ModelContext) (*API, erro
 	if err != nil {
 		return nil, fmt.Errorf("getting model info: %w", err)
 	}
-	charmhubHTTPClient, err := ctx.HTTPClient(facade.CharmhubHTTPClient)
+	charmhubHTTPClient, err := ctx.HTTPClient(corehttp.CharmhubPurpose)
 	if err != nil {
 		return nil, fmt.Errorf(
 			"getting charm hub http client: %w",

apiserver/facades/client/resources/facade.go

@@ -15,6 +15,7 @@ import (
 	"github.com/juju/juju/apiserver/facade"
 	"github.com/juju/juju/apiserver/facades/client/charms"
 	corecharm "github.com/juju/juju/core/charm"
+	corehttp "github.com/juju/juju/core/http"
 	corelogger "github.com/juju/juju/core/logger"
 	"github.com/juju/juju/core/resources"
 	"github.com/juju/juju/internal/charm"
@@ -57,7 +58,7 @@ func NewFacade(ctx facade.ModelContext) (*API, error) {
 	rst := st.Resources(ctx.ObjectStore())
 	modelConfigService := ctx.DomainServices().Config()
 
-	charmhubHTTPClient, err := ctx.HTTPClient(facade.CharmhubHTTPClient)
+	charmhubHTTPClient, err := ctx.HTTPClient(corehttp.CharmhubPurpose)
 	if err != nil {
 		return nil, fmt.Errorf(
 			"getting charm hub http client: %w",

apiserver/facades/controller/charmdownloader/charmdownloader.go

@@ -12,10 +12,10 @@ import (
 	"github.com/juju/names/v5"
 
 	apiservererrors "github.com/juju/juju/apiserver/errors"
+	corehttp "github.com/juju/juju/core/http"
 	corelogger "github.com/juju/juju/core/logger"
 	"github.com/juju/juju/internal/charm"
 	"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"
 )
@@ -28,7 +28,7 @@ type CharmDownloaderAPI struct {
 	resourcesBackend   ResourcesBackend
 	stateBackend       StateBackend
 	clock              clock.Clock
-	charmhubHTTPClient http.HTTPClient
+	charmhubHTTPClient corehttp.HTTPClient
 
 	objectStore   services.Storage
 	newDownloader func(services.CharmDownloaderConfig) (Downloader, error)
@@ -48,7 +48,7 @@ func newAPI(
 	stateBackend StateBackend,
 	modelConfigService ModelConfigService,
 	clk clock.Clock,
-	httpClient http.HTTPClient,
+	httpClient corehttp.HTTPClient,
 	objectStore services.Storage,
 	newDownloader func(services.CharmDownloaderConfig) (Downloader, error),
 	logger corelogger.Logger,

apiserver/facades/controller/charmdownloader/register.go

@@ -11,6 +11,7 @@ import (
 	"github.com/juju/clock"
 
 	"github.com/juju/juju/apiserver/facade"
+	corehttp "github.com/juju/juju/core/http"
 	"github.com/juju/juju/internal/charm/services"
 )
 
@@ -26,9 +27,9 @@ func newFacadeV1(ctx facade.ModelContext) (*CharmDownloaderAPI, error) {
 	authorizer := ctx.Auth()
 	rawState := ctx.State()
 	stateBackend := stateShim{rawState}
-	resourcesBackend := resourcesShim{ctx.Resources()}
+	resourcesBackend := resourcesShim{Resources: ctx.Resources()}
 
-	charmhubHTTPClient, err := ctx.HTTPClient(facade.CharmhubHTTPClient)
+	charmhubHTTPClient, err := ctx.HTTPClient(corehttp.CharmhubPurpose)
 	if err != nil {
 		return nil, fmt.Errorf(
 			"getting charm hub http client: %w",

apiserver/facades/controller/charmrevisionupdater/interface_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/juju/juju/apiserver/facades/controller/charmrevisionupdater"
 	"github.com/juju/juju/apiserver/facades/controller/charmrevisionupdater/mocks"
 	charmmetrics "github.com/juju/juju/core/charm/metrics"
+	corehttp "github.com/juju/juju/core/http"
 	corelogger "github.com/juju/juju/core/logger"
 	"github.com/juju/juju/core/objectstore"
 	"github.com/juju/juju/internal/charm"
@@ -181,7 +182,7 @@ type facadeContextShim struct {
 	domainServices      services.DomainServices
 }
 
-func (s facadeContextShim) HTTPClient(_ facade.HTTPClientPurpose) (facade.HTTPClient, error) {
+func (s facadeContextShim) HTTPClient(_ corehttp.Purpose) (facade.HTTPClient, error) {
 	return s.httpClient, nil
 }
 

apiserver/facades/controller/charmrevisionupdater/register.go

@@ -12,6 +12,7 @@ import (
 
 	apiservererrors "github.com/juju/juju/apiserver/errors"
 	"github.com/juju/juju/apiserver/facade"
+	corehttp "github.com/juju/juju/core/http"
 	"github.com/juju/juju/internal/charmhub"
 )
 
@@ -28,7 +29,7 @@ func newCharmRevisionUpdaterAPI(ctx facade.ModelContext) (*CharmRevisionUpdaterA
 		return nil, apiservererrors.ErrPerm
 	}
 
-	charmhubHTTPClient, err := ctx.HTTPClient(facade.CharmhubHTTPClient)
+	charmhubHTTPClient, err := ctx.HTTPClient(corehttp.CharmhubPurpose)
 	if err != nil {
 		return nil, fmt.Errorf(
 			"getting charm hub http client: %w",

apiserver/root.go

@@ -20,6 +20,7 @@ import (
 	"github.com/juju/juju/apiserver/facade"
 	"github.com/juju/juju/core/changestream"
 	coredatabase "github.com/juju/juju/core/database"
+	corehttp "github.com/juju/juju/core/http"
 	"github.com/juju/juju/core/leadership"
 	"github.com/juju/juju/core/lease"
 	corelogger "github.com/juju/juju/core/logger"
@@ -890,11 +891,11 @@ func (ctx *facadeContext) LeadershipReader() (leadership.Reader, error) {
 // understood by the context.
 // - [ErrorHTTPClientForPurposeNotFound] when no http client can be found for
 // the requested [HTTPClientPurpose].
-func (ctx *facadeContext) HTTPClient(purpose facade.HTTPClientPurpose) (facade.HTTPClient, error) {
+func (ctx *facadeContext) HTTPClient(purpose corehttp.Purpose) (facade.HTTPClient, error) {
 	var client facade.HTTPClient
 
 	switch purpose {
-	case facade.CharmhubHTTPClient:
+	case corehttp.CharmhubPurpose:
 		client = ctx.r.shared.charmhubHTTPClient
 	default:
 		return nil, fmt.Errorf(

cmd/containeragent/initialize/package_test.go

@@ -51,6 +51,7 @@ func (*importSuite) TestImports(c *gc.C) {
 		"core/credential",
 		"core/devices",
 		"core/facades",
+		"core/http",
 		"core/instance",
 		"core/leadership",
 		"core/lease",
@@ -73,13 +74,13 @@ func (*importSuite) TestImports(c *gc.C) {
 		"core/secrets",
 		"core/status",
 		"core/trace",
-		"core/upgrade",
 		"core/unit",
+		"core/upgrade",
 		"core/user",
 		"core/version",
 		"core/watcher",
-		"domain/secret/errors",
 		"domain/model/errors",
+		"domain/secret/errors",
 		"domain/secretbackend/errors",
 		"environs/cloudspec",
 		"environs/config",

cmd/jujud-controller/agent/machine.go

@@ -4,7 +4,6 @@
 package agent
 
 import (
-	"context"
 	stdcontext "context"
 	"fmt"
 	"net/http"
@@ -37,7 +36,6 @@ import (
 	"github.com/juju/juju/agent"
 	"github.com/juju/juju/agent/addons"
 	agentconfig "github.com/juju/juju/agent/config"
-	"github.com/juju/juju/agent/engine"
 	agentengine "github.com/juju/juju/agent/engine"
 	agenterrors "github.com/juju/juju/agent/errors"
 	"github.com/juju/juju/agent/tools"
@@ -65,17 +63,14 @@ import (
 	"github.com/juju/juju/core/status"
 	jujuversion "github.com/juju/juju/core/version"
 	"github.com/juju/juju/environs"
-	"github.com/juju/juju/internal/charmhub"
 	"github.com/juju/juju/internal/container/broker"
 	internallogger "github.com/juju/juju/internal/logger"
 	"github.com/juju/juju/internal/mongo"
 	"github.com/juju/juju/internal/mongo/mongometrics"
 	"github.com/juju/juju/internal/pki"
 	"github.com/juju/juju/internal/pubsub/centralhub"
-	"github.com/juju/juju/internal/s3client"
 	"github.com/juju/juju/internal/service"
 	"github.com/juju/juju/internal/services"
-	sshimporter "github.com/juju/juju/internal/ssh/importer"
 	"github.com/juju/juju/internal/storage/looputil"
 	internalupgrade "github.com/juju/juju/internal/upgrade"
 	"github.com/juju/juju/internal/upgrades"
@@ -584,7 +579,7 @@ func (a *MachineAgent) makeEngineCreator(
 ) func() (worker.Worker, error) {
 	return func() (worker.Worker, error) {
 		agentConfig := a.CurrentConfig()
-		engineConfigFunc := engine.DependencyEngineConfig
+		engineConfigFunc := agentengine.DependencyEngineConfig
 		metrics := agentengine.NewMetrics()
 		controllerMetricsSink := metrics.ForModel(agentConfig.Model())
 		eng, err := dependency.NewEngine(engineConfigFunc(
@@ -614,17 +609,6 @@ func (a *MachineAgent) makeEngineCreator(
 			handle("/metrics/", promhttp.HandlerFor(a.prometheusRegistry, promhttp.HandlerOpts{}))
 		}
 
-		// Create a single HTTP client so we can reuse HTTP connections, for
-		// example across the various Charmhub API requests required for deploy.
-		charmhubLogger := internallogger.GetLogger("juju.charmhub", corelogger.CHARMHUB)
-		charmhubHTTPClient := charmhub.DefaultHTTPClient(charmhubLogger)
-
-		s3Logger := internallogger.GetLogger("juju.objectstore.s3", corelogger.OBJECTSTORE)
-		s3HTTPClient := s3client.DefaultHTTPClient(s3Logger)
-
-		sshImporterLogger := internallogger.GetLogger("juju.ssh.importer", corelogger.SSHIMPORTER)
-		sshImporterClient := sshimporter.DefaultHTTPClient(sshImporterLogger)
-
 		manifoldsCfg := machine.ManifoldsConfig{
 			PreviousAgentVersion:              previousAgentVersion,
 			AgentName:                         agentName,
@@ -668,11 +652,8 @@ func (a *MachineAgent) makeEngineCreator(
 			},
 			SetupLogging:            agentconf.SetupAgentLogging,
 			DependencyEngineMetrics: metrics,
-			CharmhubHTTPClient:      charmhubHTTPClient,
-			S3HTTPClient:            s3HTTPClient,
 			NewEnvironFunc:          newEnvirons,
 			NewCAASBrokerFunc:       newCAASBroker,
-			SSHImporterHTTPClient:   sshImporterClient,
 		}
 		manifolds := iaasMachineManifolds(manifoldsCfg)
 		if a.isCaasAgent {
@@ -803,7 +784,7 @@ func (a *MachineAgent) machineStartup(ctx stdcontext.Context, apiConn api.Connec
 type noopStatusSetter struct{}
 
 // SetStatus implements upgradesteps.StatusSetter
-func (a *noopStatusSetter) SetStatus(_ context.Context, _ status.Status, _ string, _ map[string]interface{}) error {
+func (a *noopStatusSetter) SetStatus(_ stdcontext.Context, _ status.Status, _ string, _ map[string]interface{}) error {
 	return nil
 }