GCE: Allow inject of global http client

The following code changes starts to lift the http client out from the edge and into the constructor of the environ. Future commits should allow us to pass in a http client on the construction of the environ from the dependency engine. This is just the first step of many to do that.
sdg0919 · May 28, 2021 · bb8520c · bb8520c
1 parent f2388c9
commit bb8520c
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 33 deletions.
diff --git a/provider/gce/environ.go b/provider/gce/environ.go
@@ -4,11 +4,13 @@
 package gce
 
 import (
+	stdcontext "context"
 	"strings"
 	"sync"
 	"time"
 
 	"github.com/juju/errors"
+	"github.com/juju/utils"
 	"google.golang.org/api/compute/v1"
 
 	jujucloud "github.com/juju/juju/cloud"
@@ -98,8 +100,8 @@ var _ environs.NetworkingEnviron = (*environ)(nil)
 // Function entry points defined as variables so they can be overridden
 // for testing purposes.
 var (
-	newConnection = func(conn google.ConnectionConfig, creds *google.Credentials) (gceConnection, error) {
-		return google.Connect(conn, creds)
+	newConnection = func(ctx stdcontext.Context, conn google.ConnectionConfig, creds *google.Credentials) (gceConnection, error) {
+		return google.Connect(ctx, conn, creds)
 	}
 	destroyEnv = common.Destroy
 	bootstrap  = common.Bootstrap
@@ -150,14 +152,27 @@ func (e *environ) SetCloudSpec(spec environscloudspec.CloudSpec) error {
 		ClientEmail: credAttrs[credAttrClientEmail],
 		PrivateKey:  []byte(credAttrs[credAttrPrivateKey]),
 	}
+
+	sslVerification := utils.VerifySSLHostnames
+	if spec.SkipTLSVerify {
+		sslVerification = utils.NoVerifySSLHostnames
+	}
+
 	connectionConfig := google.ConnectionConfig{
 		Region:    spec.Region,
 		ProjectID: credential.ProjectID,
+
+		// TODO (Stickupkid): Pass the http.Client through on the construction
+		// of the environ.
+		HTTPClient: utils.GetHTTPClient(sslVerification),
 	}
 
+	// TODO (stickupkid): Pass the context through the method call.
+	ctx := stdcontext.Background()
+
 	// Connect and authenticate.
 	var err error
-	e.gce, err = newConnection(connectionConfig, credential)
+	e.gce, err = newConnection(ctx, connectionConfig, credential)
 	if err != nil {
 		return errors.Trace(err)
 	}
@@ -220,7 +235,7 @@ func (env *environ) Create(ctx context.ProviderCallContext, p environs.CreatePar
 	return nil
 }
 
-// Bootstrap creates a new instance, chosing the series and arch out of
+// Bootstrap creates a new instance, choosing the series and arch out of
 // available tools. The series and arch are returned along with a func
 // that must be called to finalize the bootstrap process by transferring
 // the tools and installing the initial juju controller.

diff --git a/provider/gce/google/auth.go b/provider/gce/google/auth.go
@@ -6,7 +6,6 @@ package google
 import (
 	"context"
 	"net/http"
-	"time"
 
 	"github.com/juju/errors"
 	"golang.org/x/oauth2/google"
@@ -30,31 +29,24 @@ var scopes = []string{
 // which is then used by the Environ.
 // This should also be relocated alongside its wrapper,
 // rather than this "auth.go" file.
-func newComputeService(creds *Credentials) (*compute.Service, error) {
+func newComputeService(ctx context.Context, creds *Credentials, httpClient *http.Client) (*compute.Service, error) {
 	cfg, err := newJWTConfig(creds)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 
-	ctx := context.Background()
+	// We're substituting the transport, with a wrapped GCE specific version of
+	// the original http.Client.
+	newClient := *httpClient
 
-	ts := cfg.TokenSource(ctx)
-	tsOpt := option.WithTokenSource(ts)
-
-	newTransport := http.DefaultTransport.(*http.Transport).Clone()
-	newTransport.TLSHandshakeTimeout = 20 * time.Second
-
-	httpTransport, err := transporthttp.NewTransport(ctx, newTransport, tsOpt)
-	if err != nil {
+	tsOpt := option.WithTokenSource(cfg.TokenSource(ctx))
+	if newClient.Transport, err = transporthttp.NewTransport(ctx, httpClient.Transport, tsOpt); err != nil {
 		return nil, errors.Trace(err)
 	}
 
-	httpClient := &http.Client{}
-	httpClient.Transport = httpTransport
-
 	service, err := compute.NewService(ctx,
 		tsOpt,
-		option.WithHTTPClient(httpClient),
+		option.WithHTTPClient(&newClient),
 	)
 	return service, errors.Trace(err)
 }

diff --git a/provider/gce/google/auth_test.go b/provider/gce/google/auth_test.go
@@ -4,6 +4,9 @@
 package google
 
 import (
+	"context"
+	"net/http"
+
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 )
@@ -15,7 +18,7 @@ type authSuite struct {
 var _ = gc.Suite(&authSuite{})
 
 func (s *authSuite) TestNewComputeService(c *gc.C) {
-	_, err := newComputeService(s.Credentials)
+	_, err := newComputeService(context.TODO(), s.Credentials, http.DefaultClient)
 	c.Assert(err, jc.ErrorIsNil)
 }
 

diff --git a/provider/gce/google/config.go b/provider/gce/google/config.go
@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"io"
 	"io/ioutil"
+	"net/http"
 	"net/mail"
 
 	"github.com/juju/errors"
@@ -188,6 +189,9 @@ type ConnectionConfig struct {
 	// ProjectID is the project ID to use in all GCE API requests for
 	// the connection.
 	ProjectID string
+
+	// HTTPClient is the client to use for all GCE connections.
+	HTTPClient *http.Client
 }
 
 // Validate checks the connection's fields for invalid values.
@@ -204,5 +208,8 @@ func (gc ConnectionConfig) Validate() error {
 	if gc.ProjectID == "" {
 		return NewMissingConfigValue(OSEnvProjectID, "ProjectID")
 	}
+	if gc.HTTPClient == nil {
+		return errors.NotFoundf("connection config http.Client")
+	}
 	return nil
 }
diff --git a/provider/gce/google/config_connection_test.go b/provider/gce/google/config_connection_test.go
@@ -4,6 +4,8 @@
 package google_test
 
 import (
+	"net/http"
+
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 
@@ -18,8 +20,9 @@ var _ = gc.Suite(&connConfigSuite{})
 
 func (*connConfigSuite) TestValidateValid(c *gc.C) {
 	cfg := google.ConnectionConfig{
-		Region:    "spam",
-		ProjectID: "eggs",
+		Region:     "spam",
+		ProjectID:  "eggs",
+		HTTPClient: http.DefaultClient,
 	}
 	err := cfg.Validate()
 

diff --git a/provider/gce/google/conn.go b/provider/gce/google/conn.go
@@ -4,6 +4,9 @@
 package google
 
 import (
+	"context"
+	"net/http"
+
 	"github.com/juju/errors"
 	"google.golang.org/api/compute/v1"
 )
@@ -137,8 +140,8 @@ type Connection struct {
 // Connect after a successful connection has already been made will
 // result in an error. All errors that happen while authenticating and
 // connecting are returned by Connect.
-func Connect(connCfg ConnectionConfig, creds *Credentials) (*Connection, error) {
-	raw, err := newService(creds)
+func Connect(ctx context.Context, connCfg ConnectionConfig, creds *Credentials) (*Connection, error) {
+	raw, err := newService(ctx, creds, connCfg.HTTPClient)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
@@ -151,8 +154,8 @@ func Connect(connCfg ConnectionConfig, creds *Credentials) (*Connection, error)
 	return conn, nil
 }
 
-var newService = func(creds *Credentials) (*compute.Service, error) {
-	return newComputeService(creds)
+var newService = func(ctx context.Context, creds *Credentials, httpClient *http.Client) (*compute.Service, error) {
+	return newComputeService(ctx, creds, httpClient)
 }
 
 // VerifyCredentials ensures that the authentication credentials used

diff --git a/provider/gce/google/conn_test.go b/provider/gce/google/conn_test.go
@@ -4,6 +4,9 @@
 package google_test
 
 import (
+	"context"
+	"net/http"
+
 	"github.com/juju/errors"
 	jc "github.com/juju/testing/checkers"
 	"google.golang.org/api/compute/v1"
@@ -14,20 +17,18 @@ import (
 
 type connSuite struct {
 	google.BaseSuite
-
-	conn *google.Connection
 }
 
 var _ = gc.Suite(&connSuite{})
 
 func (s *connSuite) TestConnect(c *gc.C) {
 	google.SetRawConn(s.Conn, nil)
 	service := &compute.Service{}
-	s.PatchValue(google.NewService, func(auth *google.Credentials) (*compute.Service, error) {
+	s.PatchValue(google.NewService, func(ctx context.Context, creds *google.Credentials, httpClient *http.Client) (*compute.Service, error) {
 		return service, nil
 	})
 
-	conn, err := google.Connect(s.ConnCfg, s.Credentials)
+	conn, err := google.Connect(context.TODO(), s.ConnCfg, s.Credentials)
 	c.Assert(err, jc.ErrorIsNil)
 
 	c.Check(google.ExposeRawService(conn), gc.Equals, service)

diff --git a/provider/gce/google/testing_test.go b/provider/gce/google/testing_test.go
@@ -4,6 +4,8 @@
 package google
 
 import (
+	"net/http"
+
 	"google.golang.org/api/compute/v1"
 	gc "gopkg.in/check.v1"
 
@@ -51,8 +53,9 @@ func (s *BaseSuite) SetUpTest(c *gc.C) {
 }`[1:]),
 	}
 	s.ConnCfg = ConnectionConfig{
-		Region:    "a",
-		ProjectID: "spam",
+		Region:     "a",
+		ProjectID:  "spam",
+		HTTPClient: http.DefaultClient,
 	}
 	fake := &fakeConn{}
 	s.Conn = &Connection{

diff --git a/provider/gce/testing_test.go b/provider/gce/testing_test.go
@@ -4,6 +4,7 @@
 package gce
 
 import (
+	stdcontext "context"
 	"fmt"
 	"net/url"
 	"strings"
@@ -328,7 +329,7 @@ func (s *BaseSuite) SetUpTest(c *gc.C) {
 
 	// Patch out all expensive external deps.
 	s.Env.gce = s.FakeConn
-	s.PatchValue(&newConnection, func(google.ConnectionConfig, *google.Credentials) (gceConnection, error) {
+	s.PatchValue(&newConnection, func(stdcontext.Context, google.ConnectionConfig, *google.Credentials) (gceConnection, error) {
 		return s.FakeConn, nil
 	})
 	s.PatchValue(&bootstrap, s.FakeCommon.Bootstrap)