From ba1d2d0fa9dbd44f513195ae5b7b3ef5706b8259 Mon Sep 17 00:00:00 2001 From: bKiralyWdf <65398980+bKiralyWdf@users.noreply.github.com> Date: Tue, 18 Jun 2024 10:44:21 +0200 Subject: [PATCH] Added authentication token caching (#51) * Added authentication token caching * Minor fox * minor: support password rotation * Added password change option for other clients --- .local/getcerts.sh | 4 +- internal/cf/client.go | 66 ++++++++++- internal/cf/client_test.go | 234 +++++++++++++++++++++++++++++++++++++ 3 files changed, 299 insertions(+), 5 deletions(-) create mode 100644 internal/cf/client_test.go diff --git a/.local/getcerts.sh b/.local/getcerts.sh index c353402..88ccc6b 100755 --- a/.local/getcerts.sh +++ b/.local/getcerts.sh @@ -6,5 +6,5 @@ cd "$(dirname "$0")" mkdir -p ssl -kubectl get secret cf-service-operator-webhook -o jsonpath='{.data.tls\.key}' | base64 -d > ssl/tls.key -kubectl get secret cf-service-operator-webhook -o jsonpath='{.data.tls\.crt}' | base64 -d > ssl/tls.crt +kubectl get secret cf-service-operator-tls -o jsonpath='{.data.tls\.key}' | base64 -d > ssl/tls.key +kubectl get secret cf-service-operator-tls -o jsonpath='{.data.tls\.crt}' | base64 -d > ssl/tls.crt diff --git a/internal/cf/client.go b/internal/cf/client.go index 72df45a..a9cbc38 100644 --- a/internal/cf/client.go +++ b/internal/cf/client.go @@ -7,6 +7,7 @@ package cf import ( "fmt" + "sync" cfclient "github.com/cloudfoundry-community/go-cfclient/v3/client" cfconfig "github.com/cloudfoundry-community/go-cfclient/v3/config" @@ -33,6 +34,11 @@ type organizationClient struct { client cfclient.Client } +type clientIdentifier struct { + URL string + Username string +} + type spaceClient struct { url string username string @@ -89,14 +95,68 @@ func newSpaceClient(spaceGuid string, url string, username string, password stri return &spaceClient{url: url, username: username, password: password, 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) { - return newOrganizationClient(organizationName, url, username, password) + cacheMutex.Lock() + defer cacheMutex.Unlock() + identifier := clientIdentifier{URL: url, Username: username} + client, cached := orgClientCache[identifier] + var err error + if !cached { + 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 + } + } + return client, err } func NewSpaceClient(spaceGuid string, url string, username string, password string) (facade.SpaceClient, error) { - return newSpaceClient(spaceGuid, url, username, password) + cacheMutex.Lock() + defer cacheMutex.Unlock() + identifier := clientIdentifier{URL: url, Username: username} + client, cached := spaceClientCache[identifier] + var err error + if !cached { + 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 + } + } + return client, err } func NewSpaceHealthChecker(spaceGuid string, url string, username string, password string) (facade.SpaceHealthChecker, error) { - return newSpaceClient(spaceGuid, url, username, password) + cacheMutex.Lock() + defer cacheMutex.Unlock() + identifier := clientIdentifier{URL: url, Username: username} + client, cached := spaceClientCache[identifier] + var err error + if !cached { + 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 + } + } + return client, err } diff --git a/internal/cf/client_test.go b/internal/cf/client_test.go new file mode 100644 index 0000000..1ab6ee7 --- /dev/null +++ b/internal/cf/client_test.go @@ -0,0 +1,234 @@ +/* +SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and cf-service-operator contributors +SPDX-License-Identifier: Apache-2.0 +*/ +package cf + +import ( + "context" + "testing" + "time" + + cfResource "github.com/cloudfoundry-community/go-cfclient/v3/resource" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/onsi/gomega/ghttp" +) + +// constants useful for this file +// Note: +// - if constants are used in multiple controllers, consider moving them to suite_test.go +// - use separate resource names to prevent collisions between tests + +const ( + OrgName = "test-org" + Username = "testUser" + Password = "testPass" + Owner = "testOwner" + + spacesURI = "/v3/spaces" + serviceInstancesURI = "/v3/service_instances" + uaaURI = "/uaa/oauth/token" +) + +type Token struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type,omitempty"` + RefreshToken string `json:"refresh_token,omitempty"` + Expiry time.Time `json:"expiry,omitempty"` +} + +func TestCFClient(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "CF Client Test Suite") +} + +// ----------------------------------------------------------------------------------------------- +// Tests +// ----------------------------------------------------------------------------------------------- + +var _ = Describe("CF Client tests", Ordered, func() { + var server *ghttp.Server + var url string + var rootResult cfResource.Root + var statusCode int + var ctx context.Context + var tokenResult Token + + BeforeAll(func() { + ctx = context.Background() + // Setup fake server + server = ghttp.NewServer() + url = "http://" + server.Addr() + statusCode = 200 + rootResult = cfResource.Root{ + Links: cfResource.RootLinks{ + Uaa: cfResource.Link{ + Href: url + "/uaa", + }, + Login: cfResource.Link{ + Href: url + "/login", + }, + }, + } + tokenResult = Token{ + AccessToken: "Foo", + TokenType: "Bar", + RefreshToken: "Baz", + Expiry: time.Now().Add(time.Minute), + } + By("creating space CR") + }) + AfterAll(func() { + // Shutdown the server after tests + server.Close() + }) + + Describe("NewOrganizationClient", func() { + BeforeEach(func() { + // Reset the cache so tests can be run independently + orgClientCache = make(map[clientIdentifier]*organizationClient) + // Reset server call counts + server.Reset() + // Register handlers + server.RouteToHandler("GET", "/", ghttp.CombineHandlers( + ghttp.RespondWithJSONEncodedPtr(&statusCode, &rootResult), + )) + server.RouteToHandler("GET", spacesURI, ghttp.CombineHandlers( + ghttp.RespondWithJSONEncodedPtr(&statusCode, &rootResult), + )) + server.RouteToHandler("POST", uaaURI, ghttp.CombineHandlers( + ghttp.RespondWithJSONEncodedPtr(&statusCode, &tokenResult), + )) + }) + + It("should create OrgClient", func() { + NewOrganizationClient(OrgName, url, Username, Password) + + // Discover UAA endpoint + Expect(server.ReceivedRequests()[0].Method).To(Equal("GET")) + Expect(server.ReceivedRequests()[0].URL.Path).To(Equal("/")) + + Expect(server.ReceivedRequests()).To(HaveLen(1)) + }) + + It("should be able to query some org", func() { + orgClient, err := NewOrganizationClient(OrgName, url, Username, Password) + Expect(err).To(BeNil()) + + orgClient.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 space + Expect(server.ReceivedRequests()[2].Method).To(Equal("GET")) + Expect(server.ReceivedRequests()[2].URL.Path).To(Equal(spacesURI)) + + Expect(server.ReceivedRequests()).To(HaveLen(3)) + }) + + It("should be able to query some org twice", func() { + orgClient, err := NewOrganizationClient(OrgName, url, Username, Password) + Expect(err).To(BeNil()) + + orgClient.GetSpace(ctx, Owner) + orgClient, err = NewOrganizationClient(OrgName, url, Username, Password) + Expect(err).To(BeNil()) + orgClient.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 space + Expect(server.ReceivedRequests()[2].Method).To(Equal("GET")) + Expect(server.ReceivedRequests()[2].URL.Path).To(Equal(spacesURI)) + + // Get space + Expect(server.ReceivedRequests()[3].Method).To(Equal("GET")) + Expect(server.ReceivedRequests()[3].URL.Path).To(Equal(spacesURI)) + + Expect(server.ReceivedRequests()).To(HaveLen(4)) + }) + }) + + Describe("NewSpaceClient", func() { + BeforeEach(func() { + // Reset the cache so tests can be run independently + spaceClientCache = make(map[clientIdentifier]*spaceClient) + // Reset server call counts + server.Reset() + // Register handlers + server.RouteToHandler("GET", "/", ghttp.CombineHandlers( + ghttp.RespondWithJSONEncodedPtr(&statusCode, &rootResult), + )) + server.RouteToHandler("GET", serviceInstancesURI, ghttp.CombineHandlers( + ghttp.RespondWithJSONEncodedPtr(&statusCode, &rootResult), + )) + server.RouteToHandler("POST", uaaURI, ghttp.CombineHandlers( + ghttp.RespondWithJSONEncodedPtr(&statusCode, &tokenResult), + )) + }) + + It("should create SpaceClient", func() { + NewSpaceClient(OrgName, url, Username, Password) + + // Discover UAA endpoint + Expect(server.ReceivedRequests()[0].Method).To(Equal("GET")) + Expect(server.ReceivedRequests()[0].URL.Path).To(Equal("/")) + + Expect(server.ReceivedRequests()).To(HaveLen(1)) + }) + + It("should be able to query space", func() { + spaceClient, err := NewSpaceClient(OrgName, url, Username, Password) + Expect(err).To(BeNil()) + + spaceClient.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].URL.Path).To(Equal(serviceInstancesURI)) + + Expect(server.ReceivedRequests()).To(HaveLen(3)) + }) + + It("should be able to query some space twice", func() { + spaceClient, err := NewSpaceClient(OrgName, url, Username, Password) + Expect(err).To(BeNil()) + + spaceClient.GetInstance(ctx, Owner) + spaceClient, err = NewSpaceClient(OrgName, url, Username, Password) + Expect(err).To(BeNil()) + spaceClient.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].URL.Path).To(Equal(serviceInstancesURI)) + + // Get instance + Expect(server.ReceivedRequests()[3].Method).To(Equal("GET")) + Expect(server.ReceivedRequests()[3].URL.Path).To(Equal(serviceInstancesURI)) + + Expect(server.ReceivedRequests()).To(HaveLen(4)) + }) + }) +})