From 3406dd4781abc69dc56e515bbb457369fed3b0e2 Mon Sep 17 00:00:00 2001 From: d067564 Date: Thu, 13 Jun 2024 10:44:32 +0200 Subject: [PATCH 1/4] Added authentication token caching --- .local/getcerts.sh | 4 +- internal/cf/client.go | 51 +++++++- internal/cf/client_test.go | 230 +++++++++++++++++++++++++++++++++++++ 3 files changed, 280 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..17eb10e 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,53 @@ 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 + } + } + 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 + } + } + 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 + } + } + return client, err } diff --git a/internal/cf/client_test.go b/internal/cf/client_test.go new file mode 100644 index 0000000..5f0aa0b --- /dev/null +++ b/internal/cf/client_test.go @@ -0,0 +1,230 @@ +/* +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.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.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)) + }) + }) +}) From 658d43bc96918f611d61ed215cc715081d2f104a Mon Sep 17 00:00:00 2001 From: d067564 Date: Thu, 13 Jun 2024 11:30:04 +0200 Subject: [PATCH 2/4] Minor fox --- internal/cf/client_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/cf/client_test.go b/internal/cf/client_test.go index 5f0aa0b..1ab6ee7 100644 --- a/internal/cf/client_test.go +++ b/internal/cf/client_test.go @@ -136,6 +136,8 @@ var _ = Describe("CF Client tests", Ordered, func() { 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 @@ -204,10 +206,12 @@ var _ = Describe("CF Client tests", Ordered, func() { }) It("should be able to query some space twice", func() { - spaceClient, err := newSpaceClient(OrgName, url, Username, Password) + 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 From 16146467db9cec2280d3b14a3077611a7f4bc1d0 Mon Sep 17 00:00:00 2001 From: d067564 Date: Fri, 14 Jun 2024 13:54:14 +0200 Subject: [PATCH 3/4] minor: support password rotation --- internal/cf/client.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/cf/client.go b/internal/cf/client.go index 17eb10e..f538baa 100644 --- a/internal/cf/client.go +++ b/internal/cf/client.go @@ -127,6 +127,11 @@ func NewSpaceClient(spaceGuid string, url string, username string, password stri 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 } From 801038fc0340f89010ffae883438108f35043399 Mon Sep 17 00:00:00 2001 From: bKiralyWdf <65398980+bKiralyWdf@users.noreply.github.com> Date: Fri, 14 Jun 2024 15:39:17 +0200 Subject: [PATCH 4/4] Added password change option for other clients --- internal/cf/client.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/cf/client.go b/internal/cf/client.go index f538baa..a9cbc38 100644 --- a/internal/cf/client.go +++ b/internal/cf/client.go @@ -112,6 +112,11 @@ func NewOrganizationClient(organizationName string, url string, username string, 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 } @@ -147,6 +152,11 @@ func NewSpaceHealthChecker(spaceGuid string, url string, username string, passwo 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 }