Skip to content

Commit

Permalink
fix client caching (#54)
Browse files Browse the repository at this point in the history 
* fix client caching

* support passord changes in client caches

* re-activate test inside docker build

* same indentation as before
  • Loading branch information
@RalfHammer
RalfHammer authored Jun 20, 2024
1 parent ba1d2d0 commit ccfa83d
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 67 deletions.
34 changes: 22 additions & 12 deletions .vscode/launch.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,28 @@
]
},
{
"name": "Run integration test",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/internal/controllers",
"console": "internalConsole",
"env": {
"KUBEBUILDER_ASSETS": "${workspaceFolder}/bin/k8s/current",
"TEST_TIMEOUT": "1200",
},
"args": [],
"showLog": true
"name": "Run client test",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/internal/cf",
"console": "internalConsole",
"args": [],
"showLog": true
},
{
"name": "Run integration test",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/internal/controllers",
"console": "internalConsole",
"env": {
"KUBEBUILDER_ASSETS": "${workspaceFolder}/bin/k8s/current",
"TEST_TIMEOUT": "1200",
},
"args": [],
"showLog": true
},
]
}
134 changes: 85 additions & 49 deletions internal/cf/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,32 @@ const (
)

type organizationClient struct {
url string
username string
password string
organizationName string
client cfclient.Client
}

type clientIdentifier struct {
URL string
Username string
}

type spaceClient struct {
url string
username string
password string
spaceGuid string
client cfclient.Client
}

type clientIdentifier struct {
url string
username string
}

type clientCacheEntry struct {
url string
username string
password string
client cfclient.Client
}

var (
cacheMutex = &sync.Mutex{}
clientCache = make(map[clientIdentifier]*clientCacheEntry)
)

func newOrganizationClient(organizationName string, url string, username string, password string) (*organizationClient, error) {
if organizationName == "" {
return nil, fmt.Errorf("missing or empty organization name")
Expand All @@ -68,7 +74,7 @@ func newOrganizationClient(organizationName string, url string, username string,
if err != nil {
return nil, err
}
return &organizationClient{url: url, username: username, password: password, organizationName: organizationName, client: *c}, nil
return &organizationClient{organizationName: organizationName, client: *c}, nil
}

func newSpaceClient(spaceGuid string, url string, username string, password string) (*spaceClient, error) {
Expand All @@ -92,71 +98,101 @@ func newSpaceClient(spaceGuid string, url string, username string, password stri
if err != nil {
return nil, err
}
return &spaceClient{url: url, username: username, password: password, spaceGuid: spaceGuid, client: *c}, nil
return &spaceClient{spaceGuid: spaceGuid, client: *c}, nil
}

var (
spaceClientCache = make(map[clientIdentifier]*spaceClient)
orgClientCache = make(map[clientIdentifier]*organizationClient)
cacheMutex = &sync.Mutex{}
)

func NewOrganizationClient(organizationName string, url string, username string, password string) (facade.OrganizationClient, error) {
cacheMutex.Lock()
defer cacheMutex.Unlock()
identifier := clientIdentifier{URL: url, Username: username}
client, cached := orgClientCache[identifier]
var err error
if !cached {

// look up CF client in cache
identifier := clientIdentifier{url: url, username: username}
cacheEntry, isInCache := clientCache[identifier]

var err error = nil
var client *organizationClient = nil
if isInCache {
// re-use CF client and wrap it as organizationClient
client = &organizationClient{organizationName: organizationName, client: cacheEntry.client}
if cacheEntry.password != password {
// password was rotated => delete client from cache and create a new one below
delete(clientCache, identifier)
isInCache = false
}
}

if !isInCache {
// create new CF client and wrap it as organizationClient
client, err = newOrganizationClient(organizationName, url, username, password)
if err == nil {
orgClientCache[identifier] = client
}
} else {
// If the password has changed since we cached the client, we want to update it to the new one
if client.password != password {
client.password = password
// add CF client to cache
clientCache[identifier] = &clientCacheEntry{url: url, username: username, password: password, client: client.client}
}
}

return client, err
}

func NewSpaceClient(spaceGuid string, url string, username string, password string) (facade.SpaceClient, error) {
cacheMutex.Lock()
defer cacheMutex.Unlock()
identifier := clientIdentifier{URL: url, Username: username}
client, cached := spaceClientCache[identifier]
var err error
if !cached {

// look up CF client in cache
identifier := clientIdentifier{url: url, username: username}
cacheEntry, isInCache := clientCache[identifier]

var err error = nil
var client *spaceClient = nil
if isInCache {
// re-use CF client from cache and wrap it as spaceClient
client = &spaceClient{spaceGuid: spaceGuid, client: cacheEntry.client}
if cacheEntry.password != password {
// password was rotated => delete client from cache and create a new one below
delete(clientCache, identifier)
isInCache = false
}
}

if !isInCache {
// create new CF client and wrap it as spaceClient
client, err = newSpaceClient(spaceGuid, url, username, password)
if err == nil {
spaceClientCache[identifier] = client
}
} else {
// If the password has changed since we cached the client, we want to update it to the new one
if client.password != password {
client.password = password
// add CF client to cache
clientCache[identifier] = &clientCacheEntry{url: url, username: username, password: password, client: client.client}
}
}

return client, err
}

func NewSpaceHealthChecker(spaceGuid string, url string, username string, password string) (facade.SpaceHealthChecker, error) {
cacheMutex.Lock()
defer cacheMutex.Unlock()
identifier := clientIdentifier{URL: url, Username: username}
client, cached := spaceClientCache[identifier]
var err error
if !cached {

// look up CF client in cache
identifier := clientIdentifier{url: url, username: username}
cacheEntry, isInCache := clientCache[identifier]

var err error = nil
var client *spaceClient = nil
if isInCache {
// re-use CF client from cache and wrap it as spaceClient
client = &spaceClient{spaceGuid: spaceGuid, client: cacheEntry.client}
if cacheEntry.password != password {
// password was rotated => delete client from cache and create a new one below
delete(clientCache, identifier)
isInCache = false
}
}

if !isInCache {
// create new CF client and wrap it as spaceClient
client, err = newSpaceClient(spaceGuid, url, username, password)
if err == nil {
spaceClientCache[identifier] = client
}
} else {
// If the password has changed since we cached the client, we want to update it to the new one
if client.password != password {
client.password = password
// add CF client to cache
clientCache[identifier] = &clientCacheEntry{url: url, username: username, password: password, client: client.client}
}
}

return client, err
}
68 changes: 62 additions & 6 deletions internal/cf/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@ import (
// - use separate resource names to prevent collisions between tests

const (
OrgName = "test-org"
Username = "testUser"
Password = "testPass"
Owner = "testOwner"
OrgName = "test-org"
OrgName2 = "test-org-2"
SpaceName = "test-space"
SpaceName2 = "test-space-2"
Username = "testUser"
Password = "testPass"
Owner = "testOwner"
Owner2 = "testOwner2"

spacesURI = "/v3/spaces"
serviceInstancesURI = "/v3/service_instances"
Expand Down Expand Up @@ -87,7 +91,7 @@ var _ = Describe("CF Client tests", Ordered, func() {
Describe("NewOrganizationClient", func() {
BeforeEach(func() {
// Reset the cache so tests can be run independently
orgClientCache = make(map[clientIdentifier]*organizationClient)
clientCache = make(map[clientIdentifier]*clientCacheEntry)
// Reset server call counts
server.Reset()
// Register handlers
Expand Down Expand Up @@ -156,12 +160,38 @@ var _ = Describe("CF Client tests", Ordered, func() {

Expect(server.ReceivedRequests()).To(HaveLen(4))
})

It("should be able to query two different orgs", func() {
// test org 1
orgClient1, err1 := NewOrganizationClient(OrgName, url, Username, Password)
Expect(err1).To(BeNil())
orgClient1.GetSpace(ctx, Owner)
// Discover UAA endpoint
Expect(server.ReceivedRequests()[0].Method).To(Equal("GET"))
Expect(server.ReceivedRequests()[0].URL.Path).To(Equal("/"))
// Get new oAuth token
Expect(server.ReceivedRequests()[1].Method).To(Equal("POST"))
Expect(server.ReceivedRequests()[1].URL.Path).To(Equal(uaaURI))
// Get instance
Expect(server.ReceivedRequests()[2].Method).To(Equal("GET"))
Expect(server.ReceivedRequests()[2].RequestURI).To(ContainSubstring(Owner))

// test org 2
orgClient2, err2 := NewOrganizationClient(OrgName2, url, Username, Password)
Expect(err2).To(BeNil())
orgClient2.GetSpace(ctx, Owner2)
// no discovery of UAA endpoint or oAuth token here due to caching
// Get instance
Expect(server.ReceivedRequests()[3].Method).To(Equal("GET"))
Expect(server.ReceivedRequests()[3].RequestURI).To(ContainSubstring(Owner2))
})

})

Describe("NewSpaceClient", func() {
BeforeEach(func() {
// Reset the cache so tests can be run independently
spaceClientCache = make(map[clientIdentifier]*spaceClient)
clientCache = make(map[clientIdentifier]*clientCacheEntry)
// Reset server call counts
server.Reset()
// Register handlers
Expand Down Expand Up @@ -230,5 +260,31 @@ var _ = Describe("CF Client tests", Ordered, func() {

Expect(server.ReceivedRequests()).To(HaveLen(4))
})

It("should be able to query two different spaces", func() {
// test space 1
spaceClient1, err1 := NewSpaceClient(SpaceName, url, Username, Password)
Expect(err1).To(BeNil())
spaceClient1.GetInstance(ctx, Owner)
// Discover UAA endpoint
Expect(server.ReceivedRequests()[0].Method).To(Equal("GET"))
Expect(server.ReceivedRequests()[0].URL.Path).To(Equal("/"))
// Get new oAuth token
Expect(server.ReceivedRequests()[1].Method).To(Equal("POST"))
Expect(server.ReceivedRequests()[1].URL.Path).To(Equal(uaaURI))
// Get instance
Expect(server.ReceivedRequests()[2].Method).To(Equal("GET"))
Expect(server.ReceivedRequests()[2].RequestURI).To(ContainSubstring(Owner))

// test space 2
spaceClient2, err2 := NewSpaceClient(SpaceName2, url, Username, Password)
Expect(err2).To(BeNil())
spaceClient2.GetInstance(ctx, Owner2)
// no discovery of UAA endpoint or oAuth token here due to caching
// Get instance
Expect(server.ReceivedRequests()[3].Method).To(Equal("GET"))
Expect(server.ReceivedRequests()[3].RequestURI).To(ContainSubstring(Owner2))
})

})
})

0 comments on commit ccfa83d

Please sign in to comment.