Skip to content

Commit

Permalink
recreate the HTTP client to avoid deadline timeouts
Browse files Browse the repository at this point in the history
  • Loading branch information
jtarchie committed Dec 2, 2020
1 parent af5a646 commit e396ae7
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 9 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ can be found in [Pivotal Documentation](docs.pivotal.io/platform-automation).
This ensures that commands are not kept in `bash` history.
The environment variable `OM_PASSWORD` will overwrite the password value in `env.yml`.

## 7.1.1
### Bug Fixes
- The oauth handler created a client with a request and connection timeout.
When that timeout occurred, the and a retry happened (usually in the command logic)
the HTTP client would be reused.
The previous timeout would be propagated.
This will not create a new HTTP client each time.

## 7.1.0
### Features
- `om configure-opsman` can now set the UAA token expirations and timeouts.
Expand Down
25 changes: 17 additions & 8 deletions network/oauth_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ import (
)

type OAuthClient struct {
client *http.Client
caCert string
clientID string
clientSecret string
insecureSkipVerify bool
password string
target string
token *oauth2.Token
username string
connectTimeout time.Duration
requestTimeout time.Duration
}

func NewOAuthClient(
Expand All @@ -29,25 +31,21 @@ func NewOAuthClient(
connectTimeout time.Duration,
requestTimeout time.Duration,
) (*OAuthClient, error) {
httpclient, err := newHTTPClient(insecureSkipVerify, caCert, requestTimeout, connectTimeout)
if err != nil {
return nil, err
}

return &OAuthClient{
client: httpclient,
caCert: caCert,
clientID: clientID,
clientSecret: clientSecret,
insecureSkipVerify: insecureSkipVerify,
password: password,
target: target,
username: username,
connectTimeout: connectTimeout,
requestTimeout: requestTimeout,
}, nil
}

func (oc *OAuthClient) Do(request *http.Request) (*http.Response, error) {
token := oc.token
client := oc.client
target := oc.target

if !strings.HasPrefix(target, "http://") && !strings.HasPrefix(target, "https://") {
Expand All @@ -64,6 +62,17 @@ func (oc *OAuthClient) Do(request *http.Request) (*http.Response, error) {
request.URL.Scheme = targetURL.Scheme
request.URL.Host = targetURL.Host

client, err := newHTTPClient(
oc.insecureSkipVerify,
oc.caCert,
oc.requestTimeout,
oc.connectTimeout,
)

if err != nil {
return nil, err
}

if token != nil && token.Valid() {
request.Header.Set(
"Authorization",
Expand Down
105 changes: 104 additions & 1 deletion network/oauth_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,117 @@ import (

var _ = Describe("OAuthClient", func() {
var (
server *ghttp.Server
server *ghttp.Server
)

BeforeEach(func() {
server = ghttp.NewTLSServer()
})

Describe("Do", func() {
When("with a request timeout", func() {
It("use that timeout value", func() {
client, err := network.NewOAuthClient(server.URL(), "opsman-username", "opsman-password", "", "", true, "", time.Nanosecond, time.Nanosecond)
Expect(err).ToNot(HaveOccurred())

req, err := http.NewRequest("GET", "/some/path", strings.NewReader("request-body"))
Expect(err).ToNot(HaveOccurred())

_, err = client.Do(req)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("context deadline exceeded"))
})

It("retries with a new client", func() {
server.AppendHandlers(
func(http.ResponseWriter, *http.Request) {
time.Sleep(time.Duration(200) * time.Millisecond)
},
func(http.ResponseWriter, *http.Request) {
time.Sleep(time.Duration(200) * time.Millisecond)
},
ghttp.RespondWith(http.StatusOK, `{
"access_token": "some-opsman-token",
"token_type": "bearer",
"expires_in": 3600
}`, http.Header{
"Content-Type": []string{"application/json"},
}),
ghttp.RespondWith(http.StatusOK, nil),
)

client, err := network.NewOAuthClient(server.URL(), "opsman-username", "opsman-password", "", "", true, "", time.Duration(100)*time.Millisecond, time.Duration(100)*time.Millisecond)
Expect(err).ToNot(HaveOccurred())

req, err := http.NewRequest("GET", "/some/path", strings.NewReader("request-body"))
Expect(err).ToNot(HaveOccurred())

_, err = client.Do(req)
Expect(err).ToNot(HaveOccurred())
})
})

When("a token expires", func() {
It("will refresh it", func() {
server.AppendHandlers(
ghttp.RespondWith(http.StatusOK, `{
"access_token": "some-opsman-token",
"token_type": "bearer",
"expires_in": 1
}`, http.Header{
"Content-Type": []string{"application/json"},
}),
ghttp.RespondWith(http.StatusOK, nil),
ghttp.RespondWith(http.StatusOK, `{
"access_token": "some-opsman-token",
"token_type": "bearer",
"expires_in": 1
}`, http.Header{
"Content-Type": []string{"application/json"},
}),
ghttp.RespondWith(http.StatusOK, nil),
)

client, err := network.NewOAuthClient(server.URL(), "opsman-username", "opsman-password", "", "", true, "", time.Duration(100)*time.Millisecond, time.Duration(100)*time.Millisecond)
Expect(err).ToNot(HaveOccurred())

for i := 0; i < 2; i++ {
req, err := http.NewRequest("GET", "/some/path", strings.NewReader("request-body"))
Expect(err).ToNot(HaveOccurred())

_, err = client.Do(req)
Expect(err).ToNot(HaveOccurred())
}
})
})

When("a token has not expired", func() {
It("reuses it", func() {
server.AppendHandlers(
ghttp.RespondWith(http.StatusOK, `{
"access_token": "some-opsman-token",
"token_type": "bearer",
"expires_in": 3600
}`, http.Header{
"Content-Type": []string{"application/json"},
}),
ghttp.RespondWith(http.StatusOK, ""),
ghttp.RespondWith(http.StatusOK, ""),
)

client, err := network.NewOAuthClient(server.URL(), "opsman-username", "opsman-password", "", "", true, "", time.Duration(100)*time.Millisecond, time.Duration(100)*time.Millisecond)
Expect(err).ToNot(HaveOccurred())

for i := 0; i < 2; i++ {
req, err := http.NewRequest("GET", "/some/path", strings.NewReader("request-body"))
Expect(err).ToNot(HaveOccurred())

_, err = client.Do(req)
Expect(err).ToNot(HaveOccurred())
}
})
})

It("makes a request with authentication", func() {
server.RouteToHandler("POST", "/uaa/oauth/token", ghttp.CombineHandlers(
ghttp.VerifyBasicAuth("opsman", ""),
Expand Down

0 comments on commit e396ae7

Please sign in to comment.