Skip to content

Commit

Permalink
Use NewClient, SecureTLSConfig, BasicAuthHeader from http instead of …
Browse files Browse the repository at this point in the history 
…utils.

Bring in improvements to proxy settings and debugging.  Update
tests according.

A few tests have been reworked to use httptest instead of updating the
DefaultTransport.

Some compiler warnings have been resolved.
  • Loading branch information
@hmlanigan
hmlanigan committed Jul 28, 2020
1 parent ba70c2d commit cf452a1
Show file tree
Hide file tree
Showing 34 changed files with 314 additions and 259 deletions.
6 changes: 4 additions & 2 deletions api/apiclient.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/gorilla/websocket"
"github.com/juju/clock"
"github.com/juju/errors"
jujuhttp "github.com/juju/http"
"github.com/juju/loggo"
"github.com/juju/names/v4"
"github.com/juju/utils"
Expand All @@ -36,6 +37,7 @@ import (
"github.com/juju/juju/api/base"
"github.com/juju/juju/apiserver/params"
"github.com/juju/juju/charmstore"

"github.com/juju/juju/core/network"
"github.com/juju/juju/rpc"
"github.com/juju/juju/rpc/jsoncodec"
Expand Down Expand Up @@ -413,7 +415,7 @@ func (st *state) connectStream(path string, attrs url.Values, extraHeaders http.
}
var requestHeader http.Header
if st.tag != "" {
requestHeader = utils.BasicAuthHeader(st.tag, st.password)
requestHeader = jujuhttp.BasicAuthHeader(st.tag, st.password)
} else {
requestHeader = make(http.Header)
}
Expand Down Expand Up @@ -1140,7 +1142,7 @@ func (d dialer) dial1() (jsoncodec.JSONConn, *tls.Config, error) {
// API server. If certPool is non-nil, we use it as the config's RootCAs,
// and the server name is set to "juju-apiserver".
func NewTLSConfig(certPool *x509.CertPool) *tls.Config {
tlsConfig := utils.SecureTLSConfig()
tlsConfig := jujuhttp.SecureTLSConfig()
if certPool != nil {
// We want to be specific here (rather than just using "anything").
// See commit 7fc118f015d8480dfad7831788e4b8c0432205e8 (PR 899).
Expand Down
3 changes: 2 additions & 1 deletion apiserver/apiserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/gorilla/websocket"
"github.com/juju/clock"
jujuhttp "github.com/juju/http"
"github.com/juju/loggo"
"github.com/juju/names/v4"
jc "github.com/juju/testing/checkers"
Expand Down Expand Up @@ -285,7 +286,7 @@ func dialWebsocketFromURL(c *gc.C, server string, header http.Header) (*websocke
header.Set("Origin", "http://localhost/")
caCerts := x509.NewCertPool()
c.Assert(caCerts.AppendCertsFromPEM([]byte(coretesting.CACert)), jc.IsTrue)
tlsConfig := utils.SecureTLSConfig()
tlsConfig := jujuhttp.SecureTLSConfig()
tlsConfig.RootCAs = caCerts
tlsConfig.ServerName = "juju-apiserver"

Expand Down
10 changes: 5 additions & 5 deletions apiserver/debuglog_db_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"net/url"

"github.com/gorilla/websocket"
jujuhttp "github.com/juju/http"
jc "github.com/juju/testing/checkers"
"github.com/juju/utils"
gc "gopkg.in/check.v1"

"github.com/juju/juju/apiserver/params"
Expand Down Expand Up @@ -54,7 +54,7 @@ func (s *debugLogDBSuite) TestNoAuth(c *gc.C) {

func (s *debugLogDBSuite) TestUnitLoginsRejected(c *gc.C) {
u, password := s.Factory.MakeUnitReturningPassword(c, nil)
header := utils.BasicAuthHeader(u.Tag().String(), password)
header := jujuhttp.BasicAuthHeader(u.Tag().String(), password)

conn, _, err := s.dialWebsocketInternal(c, nil, header)
c.Assert(err, jc.ErrorIsNil)
Expand All @@ -70,7 +70,7 @@ func (s *debugLogDBSuite) TestUserLoginsAccepted(c *gc.C) {
Name: "oryx",
Password: "gardener",
})
header := utils.BasicAuthHeader(u.Tag().String(), "gardener")
header := jujuhttp.BasicAuthHeader(u.Tag().String(), "gardener")
conn, _, err := s.dialWebsocketInternal(c, noResultsPlease, header)
c.Assert(err, jc.ErrorIsNil)
c.Assert(conn, gc.NotNil)
Expand All @@ -84,7 +84,7 @@ func (s *debugLogDBSuite) TestMachineLoginsAccepted(c *gc.C) {
m, password := s.Factory.MakeMachineReturningPassword(c, &factory.MachineParams{
Nonce: "foo-nonce",
})
header := utils.BasicAuthHeader(m.Tag().String(), password)
header := jujuhttp.BasicAuthHeader(m.Tag().String(), password)
header.Add(params.MachineNonceHeader, "foo-nonce")
conn, _, err := s.dialWebsocketInternal(c, noResultsPlease, header)
c.Assert(err, jc.ErrorIsNil)
Expand All @@ -101,7 +101,7 @@ func (s *debugLogDBSuite) logURL(scheme string, queryParams url.Values) *url.URL
}

func (s *debugLogDBSuite) dialWebsocket(c *gc.C, queryParams url.Values) *websocket.Conn {
header := utils.BasicAuthHeader(s.Owner.String(), ownerPassword)
header := jujuhttp.BasicAuthHeader(s.Owner.String(), ownerPassword)
conn, _, err := s.dialWebsocketInternal(c, queryParams, header)
c.Assert(err, jc.ErrorIsNil)
return conn
Expand Down
2 changes: 1 addition & 1 deletion apiserver/facades/schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28589,7 +28589,7 @@
"$ref": "#/definitions/MachineNetworkConfigResults"
}
},
"description": "GetContainerInterfaceInfo returns information to configure networking for a\ncontainer. It accepts container tags as arguments."
"description": "GetContainerInterfaceInfo returns information to configure networking for a\ncontainer. It accepts container tags as arguments.\nTODO (manadart 2020-07-23): This method is not used and can be removed when\nnext this facade version is bumped.\nWe then don't need the parameterised prepareOrGet..."
},
"GetContainerProfileInfo": {
"type": "object",
Expand Down
5 changes: 3 additions & 2 deletions apiserver/logsink_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"time"

"github.com/gorilla/websocket"
jujuhttp "github.com/juju/http"
"github.com/juju/loggo"
"github.com/juju/names/v4"
jc "github.com/juju/testing/checkers"
Expand Down Expand Up @@ -68,7 +69,7 @@ func (s *logsinkSuite) TestNoAuth(c *gc.C) {

func (s *logsinkSuite) TestRejectsUserLogins(c *gc.C) {
user := s.Factory.MakeUser(c, &factory.UserParams{Password: "sekrit"})
header := utils.BasicAuthHeader(user.Tag().String(), "sekrit")
header := jujuhttp.BasicAuthHeader(user.Tag().String(), "sekrit")
s.checkAuthFails(c, header, http.StatusForbidden, "authorization failed: tag kind user not valid")
}

Expand Down Expand Up @@ -232,7 +233,7 @@ func (s *logsinkSuite) dialWebsocket(c *gc.C) *websocket.Conn {
}

func (s *logsinkSuite) makeAuthHeader() http.Header {
header := utils.BasicAuthHeader(s.machineTag.String(), s.password)
header := jujuhttp.BasicAuthHeader(s.machineTag.String(), s.password)
header.Add(params.MachineNonceHeader, s.nonce)
return header
}
11 changes: 6 additions & 5 deletions apiserver/logtransfer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/gorilla/websocket"
"github.com/juju/errors"
jujuhttp "github.com/juju/http"
"github.com/juju/loggo"
"github.com/juju/names/v4"
jc "github.com/juju/testing/checkers"
Expand Down Expand Up @@ -63,7 +64,7 @@ func (s *logtransferSuite) SetUpTest(c *gc.C) {
}

func (s *logtransferSuite) makeAuthHeader() http.Header {
header := utils.BasicAuthHeader(s.userTag.String(), s.password)
header := jujuhttp.BasicAuthHeader(s.userTag.String(), s.password)
header.Add(params.MigrationModelHTTPHeader, s.State.ModelUUID())
return header
}
Expand Down Expand Up @@ -91,14 +92,14 @@ func (s *logtransferSuite) checkAuthFails(c *gc.C, header http.Header, code int,
}

func (s *logtransferSuite) TestRejectsMissingModelHeader(c *gc.C) {
header := utils.BasicAuthHeader(s.userTag.String(), s.password)
header := jujuhttp.BasicAuthHeader(s.userTag.String(), s.password)
ws := s.dialWebsocketInternal(c, header)
websockettest.AssertJSONError(c, ws, `initialising migration logsink session: unknown model: ""`)
websockettest.AssertWebsocketClosed(c, ws)
}

func (s *logtransferSuite) TestRejectsBadMigratingModelUUID(c *gc.C) {
header := utils.BasicAuthHeader(s.userTag.String(), s.password)
header := jujuhttp.BasicAuthHeader(s.userTag.String(), s.password)
header.Add(params.MigrationModelHTTPHeader, "does-not-exist")
ws := s.dialWebsocketInternal(c, header)
websockettest.AssertJSONError(c, ws, `initialising migration logsink session: unknown model: "does-not-exist"`)
Expand All @@ -116,13 +117,13 @@ func (s *logtransferSuite) TestRejectsInvalidVersion(c *gc.C) {
}

func (s *logtransferSuite) TestRejectsMachineLogins(c *gc.C) {
header := utils.BasicAuthHeader(s.machineTag.String(), s.machinePassword)
header := jujuhttp.BasicAuthHeader(s.machineTag.String(), s.machinePassword)
header.Add(params.MachineNonceHeader, "nonce")
s.checkAuthFails(c, header, http.StatusForbidden, "authorization failed: machine 0 is not a user")
}

func (s *logtransferSuite) TestRejectsBadPasword(c *gc.C) {
header := utils.BasicAuthHeader(s.userTag.String(), "wrong")
header := jujuhttp.BasicAuthHeader(s.userTag.String(), "wrong")
header.Add(params.MigrationModelHTTPHeader, s.State.ModelUUID())
s.checkAuthFails(c, header, http.StatusUnauthorized, "authentication failed: invalid entity name or password")
}
Expand Down
12 changes: 6 additions & 6 deletions apiserver/pubsub_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import (
"time"

"github.com/gorilla/websocket"
jujuhttp "github.com/juju/http"
"github.com/juju/loggo"
"github.com/juju/names/v4"
"github.com/juju/pubsub"
jc "github.com/juju/testing/checkers"
"github.com/juju/utils"
gc "gopkg.in/check.v1"

"github.com/juju/juju/apiserver/params"
Expand Down Expand Up @@ -63,7 +63,7 @@ func (s *pubsubSuite) TestNoAuth(c *gc.C) {

func (s *pubsubSuite) TestRejectsUserLogins(c *gc.C) {
user := s.Factory.MakeUser(c, &factory.UserParams{Password: "sekrit"})
header := utils.BasicAuthHeader(user.Tag().String(), "sekrit")
header := jujuhttp.BasicAuthHeader(user.Tag().String(), "sekrit")
s.checkAuthFails(c, header, http.StatusForbidden, "authorization failed: user username-.* is not a controller")
}

Expand All @@ -72,19 +72,19 @@ func (s *pubsubSuite) TestRejectsNonServerMachineLogins(c *gc.C) {
Nonce: "a-nonce",
Jobs: []state.MachineJob{state.JobHostUnits},
})
header := utils.BasicAuthHeader(m.Tag().String(), password)
header := jujuhttp.BasicAuthHeader(m.Tag().String(), password)
header.Add(params.MachineNonceHeader, "a-nonce")
s.checkAuthFails(c, header, http.StatusForbidden, "authorization failed: machine .* is not a controller")
}

func (s *pubsubSuite) TestRejectsBadPassword(c *gc.C) {
header := utils.BasicAuthHeader(s.machineTag.String(), "wrong")
header := jujuhttp.BasicAuthHeader(s.machineTag.String(), "wrong")
header.Add(params.MachineNonceHeader, s.nonce)
s.checkAuthFails(c, header, http.StatusUnauthorized, "authentication failed: invalid entity name or password")
}

func (s *pubsubSuite) TestRejectsIncorrectNonce(c *gc.C) {
header := utils.BasicAuthHeader(s.machineTag.String(), s.password)
header := jujuhttp.BasicAuthHeader(s.machineTag.String(), s.password)
header.Add(params.MachineNonceHeader, "wrong")
s.checkAuthFails(c, header, http.StatusUnauthorized, "authentication failed: machine 0 not provisioned")
}
Expand Down Expand Up @@ -170,7 +170,7 @@ func (s *pubsubSuite) dialWebsocketInternal(c *gc.C, header http.Header) (*webso
}

func (s *pubsubSuite) makeAuthHeader() http.Header {
header := utils.BasicAuthHeader(s.machineTag.String(), s.password)
header := jujuhttp.BasicAuthHeader(s.machineTag.String(), s.password)
header.Add(params.MachineNonceHeader, s.nonce)
return header
}
11 changes: 7 additions & 4 deletions apiserver/registration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
"net/http"
"strings"

jujuhttp "github.com/juju/http"
jc "github.com/juju/testing/checkers"
"github.com/juju/testing/httptesting"
"github.com/juju/utils"
"golang.org/x/crypto/nacl/secretbox"
gc "gopkg.in/check.v1"

Expand Down Expand Up @@ -48,8 +48,9 @@ func (s *registrationSuite) TestRegister(c *gc.C) {
ciphertext := s.sealBox(
c, validNonce, secretKey, fmt.Sprintf(`{"password": "%s"}`, password),
)
client := jujuhttp.NewClient(jujuhttp.Config{SkipHostnameVerification: true})
resp := httptesting.Do(c, httptesting.DoRequestParams{
Do: utils.GetNonValidatingHTTPClient().Do,
Do: client.Do,
URL: s.registrationURL,
Method: "POST",
JSONBody: &params.SecretKeyLoginRequest{
Expand Down Expand Up @@ -87,8 +88,9 @@ func (s *registrationSuite) TestRegister(c *gc.C) {
}

func (s *registrationSuite) TestRegisterInvalidMethod(c *gc.C) {
client := jujuhttp.NewClient(jujuhttp.Config{SkipHostnameVerification: true})
httptesting.AssertJSONCall(c, httptesting.JSONCallParams{
Do: utils.GetNonValidatingHTTPClient().Do,
Do: client.Do,
URL: s.registrationURL,
Method: "GET",
ExpectStatus: http.StatusMethodNotAllowed,
Expand Down Expand Up @@ -161,8 +163,9 @@ func (s *registrationSuite) TestRegisterInvalidRequestPayload(c *gc.C) {
}

func (s *registrationSuite) testInvalidRequest(c *gc.C, requestBody, errorMessage, errorCode string, statusCode int) {
client := jujuhttp.NewClient(jujuhttp.Config{SkipHostnameVerification: true})
httptesting.AssertJSONCall(c, httptesting.JSONCallParams{
Do: utils.GetNonValidatingHTTPClient().Do,
Do: client.Do,
URL: s.registrationURL,
Method: "POST",
Body: strings.NewReader(requestBody),
Expand Down
3 changes: 2 additions & 1 deletion apiserver/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/gorilla/websocket"
"github.com/juju/errors"
jujuhttp "github.com/juju/http"
"github.com/juju/loggo"
"github.com/juju/names/v4"
jc "github.com/juju/testing/checkers"
Expand Down Expand Up @@ -179,7 +180,7 @@ func dialWebsocket(c *gc.C, addr, path string) (*websocket.Conn, error) {
header.Set("Origin", "http://localhost/")
caCerts := x509.NewCertPool()
c.Assert(caCerts.AppendCertsFromPEM([]byte(coretesting.CACert)), jc.IsTrue)
tlsConfig := utils.SecureTLSConfig()
tlsConfig := jujuhttp.SecureTLSConfig()
tlsConfig.RootCAs = caCerts
tlsConfig.ServerName = "anything"

Expand Down
4 changes: 2 additions & 2 deletions apiserver/testing/fakeapi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (

"github.com/bmizerany/pat"
"github.com/gorilla/websocket"
jujuhttp "github.com/juju/http"
"github.com/juju/rpcreflect"
"github.com/juju/utils"

"github.com/juju/juju/apiserver/observer"
"github.com/juju/juju/apiserver/observer/fakeobserver"
Expand Down Expand Up @@ -61,7 +61,7 @@ func NewAPIServer(newRoot func(modelUUID string) interface{}) *Server {

srv.Server = httptest.NewUnstartedServer(pmux)

tlsConfig := utils.SecureTLSConfig()
tlsConfig := jujuhttp.SecureTLSConfig()
tlsConfig.Certificates = []tls.Certificate{tlsCert}
srv.Server.TLS = tlsConfig

Expand Down
7 changes: 4 additions & 3 deletions apiserver/testing/http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import (
"io/ioutil"
"net/http"

jujuhttp "github.com/juju/http"
jc "github.com/juju/testing/checkers"
"github.com/juju/testing/httptesting"
"github.com/juju/utils"
gc "gopkg.in/check.v1"

"github.com/juju/juju/apiserver/params"
Expand Down Expand Up @@ -89,14 +89,15 @@ func SendHTTPRequest(c *gc.C, p HTTPRequestParams) *http.Response {
hp.Header.Set(params.MachineNonceHeader, p.Nonce)
}
if hp.Do == nil {
hp.Do = utils.GetNonValidatingHTTPClient().Do
client := jujuhttp.NewClient(jujuhttp.Config{SkipHostnameVerification: true})
hp.Do = client.Do
}
return httptesting.Do(c, hp)
}

func AssertResponse(c *gc.C, resp *http.Response, expHTTPStatus int, expContentType string) []byte {
body, err := ioutil.ReadAll(resp.Body)
resp.Body.Close()
_ = resp.Body.Close()
c.Assert(err, jc.ErrorIsNil)
c.Check(resp.StatusCode, gc.Equals, expHTTPStatus, gc.Commentf("body: %s", body))
ctype := resp.Header.Get("Content-Type")
Expand Down
Loading

0 comments on commit cf452a1

Please sign in to comment.