Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reduce HTTP client creations in the Keystone connector #2659

Merged
merged 2 commits into from
Sep 23, 2022
Merged

Reduce HTTP client creations in the Keystone connector #2659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5105ac8
Reduce HTTP client creations in Keystone connector
erwinvaneyk Sep 12, 2022
6a866ec
Fix tests
erwinvaneyk Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions connector/keystone/keystone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type conn struct {
Host string
AdminUsername string
AdminPassword string
client *http.Client
Logger log.Logger
}

Expand All @@ -35,6 +36,7 @@ type domainKeystone struct {
// Config holds the configuration parameters for Keystone connector.
// Keystone should expose API v3
// An example config:
//
// connectors:
// type: keystone
// id: keystone
Expand Down Expand Up @@ -111,11 +113,12 @@ var (
// Open returns an authentication strategy using Keystone.
func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) {
return &conn{
c.Domain,
c.Host,
c.AdminUsername,
c.AdminPassword,
logger,
Domain: c.Domain,
Host: c.Host,
AdminUsername: c.AdminUsername,
AdminPassword: c.AdminPassword,
Logger: logger,
client: http.DefaultClient,
nabokihms marked this conversation as resolved.
Show resolved Hide resolved
}, nil
}

Expand Down Expand Up @@ -192,7 +195,6 @@ func (p *conn) Refresh(
}

func (p *conn) getTokenResponse(ctx context.Context, username, pass string) (response *http.Response, err error) {
client := &http.Client{}
jsonData := loginRequestData{
auth: auth{
Identity: identity{
Expand Down Expand Up @@ -221,7 +223,7 @@ func (p *conn) getTokenResponse(ctx context.Context, username, pass string) (res
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(ctx)

return client.Do(req)
return p.client.Do(req)
}

func (p *conn) getAdminToken(ctx context.Context) (string, error) {
Expand All @@ -243,15 +245,14 @@ func (p *conn) checkIfUserExists(ctx context.Context, userID string, token strin
func (p *conn) getUser(ctx context.Context, userID string, token string) (*userResponse, error) {
// https://developer.openstack.org/api-ref/identity/v3/#show-user-details
userURL := p.Host + "/v3/users/" + userID
client := &http.Client{}
req, err := http.NewRequest("GET", userURL, nil)
if err != nil {
return nil, err
}

req.Header.Set("X-Auth-Token", token)
req = req.WithContext(ctx)
resp, err := client.Do(req)
resp, err := p.client.Do(req)
if err != nil {
return nil, err
}
Expand All @@ -276,7 +277,6 @@ func (p *conn) getUser(ctx context.Context, userID string, token string) (*userR
}

func (p *conn) getUserGroups(ctx context.Context, userID string, token string) ([]string, error) {
client := &http.Client{}
// https://developer.openstack.org/api-ref/identity/v3/#list-groups-to-which-a-user-belongs
groupsURL := p.Host + "/v3/users/" + userID + "/groups"
req, err := http.NewRequest("GET", groupsURL, nil)
Expand All @@ -285,7 +285,7 @@ func (p *conn) getUserGroups(ctx context.Context, userID string, token string) (
}
req.Header.Set("X-Auth-Token", token)
req = req.WithContext(ctx)
resp, err := client.Do(req)
resp, err := p.client.Do(req)
if err != nil {
p.Logger.Errorf("keystone: error while fetching user %q groups\n", userID)
return nil, err
Expand Down
34 changes: 17 additions & 17 deletions connector/keystone/keystone_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ type groupResponse struct {

func getAdminToken(t *testing.T, adminName, adminPass string) (token, id string) {
t.Helper()
client := &http.Client{}

jsonData := loginRequestData{
auth: auth{
Identity: identity{
Expand All @@ -70,7 +68,7 @@ func getAdminToken(t *testing.T, adminName, adminPass string) (token, id string)
}

req.Header.Set("Content-Type", "application/json")
resp, err := client.Do(req)
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatal(err)
}
Expand All @@ -93,7 +91,6 @@ func getAdminToken(t *testing.T, adminName, adminPass string) (token, id string)

func createUser(t *testing.T, token, userName, userEmail, userPass string) string {
t.Helper()
client := &http.Client{}

createUserData := map[string]interface{}{
"user": map[string]interface{}{
Expand All @@ -116,7 +113,7 @@ func createUser(t *testing.T, token, userName, userEmail, userPass string) strin
}
req.Header.Set("X-Auth-Token", token)
req.Header.Add("Content-Type", "application/json")
resp, err := client.Do(req)
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatal(err)
}
Expand All @@ -139,7 +136,6 @@ func createUser(t *testing.T, token, userName, userEmail, userPass string) strin
// delete group or user
func deleteResource(t *testing.T, token, id, uri string) {
t.Helper()
client := &http.Client{}

deleteURI := uri + id
req, err := http.NewRequest("DELETE", deleteURI, nil)
Expand All @@ -148,7 +144,7 @@ func deleteResource(t *testing.T, token, id, uri string) {
}
req.Header.Set("X-Auth-Token", token)

resp, err := client.Do(req)
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatalf("error: %v", err)
}
Expand All @@ -157,7 +153,6 @@ func deleteResource(t *testing.T, token, id, uri string) {

func createGroup(t *testing.T, token, description, name string) string {
t.Helper()
client := &http.Client{}

createGroupData := map[string]interface{}{
"group": map[string]interface{}{
Expand All @@ -177,7 +172,7 @@ func createGroup(t *testing.T, token, description, name string) string {
}
req.Header.Set("X-Auth-Token", token)
req.Header.Add("Content-Type", "application/json")
resp, err := client.Do(req)
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatal(err)
}
Expand All @@ -200,14 +195,13 @@ func createGroup(t *testing.T, token, description, name string) string {
func addUserToGroup(t *testing.T, token, groupID, userID string) error {
t.Helper()
uri := groupsURL + groupID + "/users/" + userID
client := &http.Client{}
req, err := http.NewRequest("PUT", uri, nil)
if err != nil {
return err
}
req.Header.Set("X-Auth-Token", token)

resp, err := client.Do(req)
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatalf("error: %v", err)
}
Expand All @@ -219,7 +213,8 @@ func addUserToGroup(t *testing.T, token, groupID, userID string) error {
func TestIncorrectCredentialsLogin(t *testing.T) {
setupVariables(t)
c := conn{
Host: keystoneURL, Domain: testDomain,
client: http.DefaultClient,
Host: keystoneURL, Domain: testDomain,
AdminUsername: adminUser, AdminPassword: adminPass,
}
s := connector.Scopes{OfflineAccess: true, Groups: true}
Expand Down Expand Up @@ -296,7 +291,8 @@ func TestValidUserLogin(t *testing.T) {
defer deleteResource(t, token, userID, usersURL)

c := conn{
Host: keystoneURL, Domain: tt.input.domain,
client: http.DefaultClient,
Host: keystoneURL, Domain: tt.input.domain,
AdminUsername: adminUser, AdminPassword: adminPass,
}
s := connector.Scopes{OfflineAccess: true, Groups: true}
Expand Down Expand Up @@ -333,7 +329,8 @@ func TestUseRefreshToken(t *testing.T) {
defer deleteResource(t, token, groupID, groupsURL)

c := conn{
Host: keystoneURL, Domain: testDomain,
client: http.DefaultClient,
Host: keystoneURL, Domain: testDomain,
AdminUsername: adminUser, AdminPassword: adminPass,
}
s := connector.Scopes{OfflineAccess: true, Groups: true}
Expand All @@ -358,7 +355,8 @@ func TestUseRefreshTokenUserDeleted(t *testing.T) {
userID := createUser(t, token, testUser, testEmail, testPass)

c := conn{
Host: keystoneURL, Domain: testDomain,
client: http.DefaultClient,
Host: keystoneURL, Domain: testDomain,
AdminUsername: adminUser, AdminPassword: adminPass,
}
s := connector.Scopes{OfflineAccess: true, Groups: true}
Expand Down Expand Up @@ -388,7 +386,8 @@ func TestUseRefreshTokenGroupsChanged(t *testing.T) {
defer deleteResource(t, token, userID, usersURL)

c := conn{
Host: keystoneURL, Domain: testDomain,
client: http.DefaultClient,
Host: keystoneURL, Domain: testDomain,
AdminUsername: adminUser, AdminPassword: adminPass,
}
s := connector.Scopes{OfflineAccess: true, Groups: true}
Expand Down Expand Up @@ -424,7 +423,8 @@ func TestNoGroupsInScope(t *testing.T) {
defer deleteResource(t, token, userID, usersURL)

c := conn{
Host: keystoneURL, Domain: testDomain,
client: http.DefaultClient,
Host: keystoneURL, Domain: testDomain,
AdminUsername: adminUser, AdminPassword: adminPass,
}
s := connector.Scopes{OfflineAccess: true, Groups: false}
Expand Down