From 354952e2caf35d5d7ec99fd695d3d999fc71b830 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Fri, 6 Dec 2019 13:02:21 +0200 Subject: [PATCH] Extract CC Page and Chunk sizes as configurations (#83) --- Gopkg.lock | 2 +- Gopkg.toml | 2 +- cf/client.go | 20 ++++++++--------- cf/client_test.go | 18 ++++++++++----- cf/config.go | 45 +++++++++++++++++++++++++------------- cf/config_test.go | 24 +++++++++++--------- cf/env_test.go | 2 +- cf/service_access_test.go | 2 +- cf/service_visibilities.go | 41 ++++++++++++++++++++-------------- 9 files changed, 95 insertions(+), 61 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 1bd4ed2..72e0423 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -40,7 +40,6 @@ version = "v1.5.0" [[projects]] - branch = "master" digest = "1:b3a24d07d84fb45352e3b8f53a0a4850f9a052245a9cc0cb57d2705649141cb5" name = "github.com/Peripli/service-broker-proxy" packages = [ @@ -55,6 +54,7 @@ ] pruneopts = "UT" revision = "7942aea817dd6b404db17bdc0474c43a10baaba2" + version = "v0.7.1" [[projects]] digest = "1:127aa936d6291ec7ebb8678fef00779fc3574a616cb2b938dbaa407af7221471" diff --git a/Gopkg.toml b/Gopkg.toml index 544489b..c7751ef 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -35,7 +35,7 @@ [[constraint]] name = "github.com/Peripli/service-broker-proxy" - branch = "master" + version = "=0.7.1" [[constraint]] name = "github.com/cloudfoundry-community/go-cfenv" diff --git a/cf/client.go b/cf/client.go index 61a3218..aea9976 100644 --- a/cf/client.go +++ b/cf/client.go @@ -3,8 +3,6 @@ package cf import ( "fmt" - "github.com/Peripli/service-broker-proxy/pkg/sbproxy" - "github.com/Peripli/service-broker-proxy/pkg/platform" "github.com/cloudfoundry-community/go-cfclient" @@ -18,22 +16,22 @@ type CloudFoundryErr cfclient.CloudFoundryError // It is used to call into the cf that the proxy deployed at. type PlatformClient struct { client cfclient.CloudFoundryClient - settings *sbproxy.Settings + settings *Settings } // Broker returns platform client which can perform platform broker operations -func (c *PlatformClient) Broker() platform.BrokerClient { - return c +func (pc *PlatformClient) Broker() platform.BrokerClient { + return pc } // Visibility returns platform client which can perform visibility operations -func (c *PlatformClient) Visibility() platform.VisibilityClient { - return c +func (pc *PlatformClient) Visibility() platform.VisibilityClient { + return pc } // CatalogFetcher returns platform client which can perform refetching of service broker catalogs -func (c *PlatformClient) CatalogFetcher() platform.CatalogFetcher { - return c +func (pc *PlatformClient) CatalogFetcher() platform.CatalogFetcher { + return pc } // NewClient creates a new CF cf client from the specified configuration. @@ -41,14 +39,14 @@ func NewClient(config *Settings) (*PlatformClient, error) { if err := config.Validate(); err != nil { return nil, err } - cfClient, err := config.CF.CFClientProvider(config.CF.Config) + cfClient, err := config.CF.CFClientProvider(&config.CF.Config) if err != nil { return nil, err } return &PlatformClient{ client: cfClient, - settings: &config.Settings, + settings: config, }, nil } diff --git a/cf/client_test.go b/cf/client_test.go index b206322..3b3b39a 100644 --- a/cf/client_test.go +++ b/cf/client_test.go @@ -131,11 +131,15 @@ func ccClient(URL string) (*cf.Settings, *cf.PlatformClient) { } func ccClientWithThrottling(URL string, maxAllowedParallelRequests int) (*cf.Settings, *cf.PlatformClient) { - cfConfig := &cfclient.Config{ + cfConfig := cfclient.Config{ ApiAddress: URL, } - config := &cf.ClientConfiguration{ - Config: cfConfig, + config := &cf.Config{ + ClientConfiguration: &cf.ClientConfiguration{ + Config: cfConfig, + PageSize: 500, + ChunkSize: 10, + }, CFClientProvider: cfclient.NewClient, } settings := &cf.Settings{ @@ -192,8 +196,12 @@ var _ = Describe("Client", func() { ) BeforeEach(func() { - config := &cf.ClientConfiguration{ - Config: cfclient.DefaultConfig(), + config := &cf.Config{ + ClientConfiguration: &cf.ClientConfiguration{ + Config: *cfclient.DefaultConfig(), + PageSize: 500, + ChunkSize: 10, + }, CFClientProvider: cfclient.NewClient, } settings = &cf.Settings{ diff --git a/cf/config.go b/cf/config.go index 925567d..e6e40c2 100644 --- a/cf/config.go +++ b/cf/config.go @@ -14,27 +14,35 @@ import ( "github.com/spf13/pflag" ) -// ClientConfiguration type holds config info for building the cf client -type ClientConfiguration struct { - *cfclient.Config `mapstructure:"client"` +// Config type holds config info for building the cf client +type Config struct { + *ClientConfiguration `mapstructure:"client"` // CFClientProvider delays the creation of the creation of the CF client as it does remote calls during its creation which should be delayed // until the application is ran. CFClientProvider func(*cfclient.Config) (*cfclient.Client, error) `mapstructure:"-"` } +// ClientConfiguration holds cf client configurations +type ClientConfiguration struct { + cfclient.Config `mapstructure:",squash"` + + PageSize int `mapstructure:"page_size"` + ChunkSize int `mapstructure:"chunk_size"` +} + // Settings type wraps the CF client configuration type Settings struct { sbproxy.Settings `mapstructure:",squash"` - CF *ClientConfiguration `mapstructure:"cf"` + CF *Config `mapstructure:"cf"` } // DefaultSettings returns the default application settings func DefaultSettings() *Settings { return &Settings{ Settings: *sbproxy.DefaultSettings(), - CF: DefaultClientConfiguration(), + CF: DefaultCFConfiguration(), } } @@ -47,14 +55,18 @@ func (s *Settings) Validate() error { return s.Settings.Validate() } -// DefaultClientConfiguration creates a default config for the CF client -func DefaultClientConfiguration() *ClientConfiguration { +// DefaultCFConfiguration creates a default config for the CF client +func DefaultCFConfiguration() *Config { cfClientConfig := cfclient.DefaultConfig() cfClientConfig.HttpClient.Timeout = 10 * time.Second cfClientConfig.ApiAddress = "" - return &ClientConfiguration{ - Config: cfClientConfig, + return &Config{ + ClientConfiguration: &ClientConfiguration{ + Config: *cfClientConfig, + PageSize: 500, + ChunkSize: 10, + }, CFClientProvider: cfclient.NewClient, } } @@ -65,16 +77,19 @@ func CreatePFlagsForCFClient(set *pflag.FlagSet) { } // Validate validates the configuration and returns appropriate errors in case it is invalid -func (c *ClientConfiguration) Validate() error { +func (c *Config) Validate() error { if c == nil { return fmt.Errorf("CF Client configuration missing") } + if c.ChunkSize <= 0 { + return errors.New("CF ChunkSize must be positive") + } + if c.PageSize <= 0 || c.PageSize > 500 { + return errors.New("CF PageSize must be between 1 and 500 inclusive") + } if c.CFClientProvider == nil { return errors.New("CF ClientCreateFunc missing") } - if c.Config == nil { - return errors.New("CF client configuration missing") - } if len(c.ApiAddress) == 0 { return errors.New("CF client configuration ApiAddress missing") } @@ -84,11 +99,11 @@ func (c *ClientConfiguration) Validate() error { return nil } -// NewConfig creates ClientConfiguration from the provided environment +// NewConfig creates Config from the provided environment func NewConfig(env env.Environment) (*Settings, error) { cfSettings := &Settings{ Settings: *sbproxy.DefaultSettings(), - CF: DefaultClientConfiguration(), + CF: DefaultCFConfiguration(), } if err := env.Unmarshal(cfSettings); err != nil { diff --git a/cf/config_test.go b/cf/config_test.go index 6dd1552..877dff7 100644 --- a/cf/config_test.go +++ b/cf/config_test.go @@ -48,14 +48,14 @@ var _ = Describe("Config", func() { Context("when address is missing", func() { It("returns an error", func() { - settings.CF.Config = nil + settings.CF.ApiAddress = "" assertErrorDuringValidate() }) }) Context("when request timeout is missing", func() { It("returns an error", func() { - settings.CF.ApiAddress = "" + settings.CF.HttpClient.Timeout = 0 assertErrorDuringValidate() }) }) @@ -96,13 +96,17 @@ var _ = Describe("Config", func() { settings cf.Settings envSettings = cf.Settings{ - CF: &cf.ClientConfiguration{ - Config: &cfclient.Config{ - ApiAddress: "https://example.com", - Username: "user", - Password: "password", - ClientID: "clientid", - ClientSecret: "clientsecret", + CF: &cf.Config{ + ClientConfiguration: &cf.ClientConfiguration{ + Config: cfclient.Config{ + ApiAddress: "https://example.com", + Username: "user", + Password: "password", + ClientID: "clientid", + ClientSecret: "clientsecret", + }, + PageSize: 500, + ChunkSize: 10, }, CFClientProvider: cfclient.NewClient, }, @@ -110,7 +114,7 @@ var _ = Describe("Config", func() { } emptySettings = cf.Settings{ - CF: &cf.ClientConfiguration{}, + CF: &cf.Config{}, Settings: *sbproxy.DefaultSettings(), } ) diff --git a/cf/env_test.go b/cf/env_test.go index 920f26a..d45622d 100644 --- a/cf/env_test.go +++ b/cf/env_test.go @@ -83,7 +83,7 @@ var _ = Describe("CF Env", func() { It("CF overrides are not applied", func() { Expect(environment.Get("app.legacy_url")).Should(Equal(server.DefaultSettings().Host)) Expect(environment.Get("server.port")).Should(Equal(server.DefaultSettings().Port)) - Expect(environment.Get("cf.client.apiAddress")).Should(Equal(cf.DefaultClientConfiguration().ApiAddress)) + Expect(environment.Get("cf.client.apiAddress")).Should(Equal(cf.DefaultCFConfiguration().ApiAddress)) }) }) diff --git a/cf/service_access_test.go b/cf/service_access_test.go index eadec22..cff1879 100644 --- a/cf/service_access_test.go +++ b/cf/service_access_test.go @@ -331,7 +331,7 @@ var _ = Describe("Client Service Plan Access", func() { requestChecks: expectedRequest{ Method: http.MethodGet, Path: "/v2/service_brokers", - RawQuery: encodeQuery(query), + RawQuery: encodeQuery(query) + "&results-per-page=500", }, reaction: reactionResponse{ Code: http.StatusOK, diff --git a/cf/service_visibilities.go b/cf/service_visibilities.go index 0426c9f..0bae6b1 100644 --- a/cf/service_visibilities.go +++ b/cf/service_visibilities.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/url" + "strconv" "strings" "sync" @@ -18,8 +19,6 @@ import ( "github.com/cloudfoundry-community/go-cfclient" ) -const maxChunkLength = 50 - // OrgLabelKey label key for CF organization visibilities const OrgLabelKey = "organization_guid" @@ -121,7 +120,7 @@ func (pc *PlatformClient) getBrokersByName(ctx context.Context, names []string) wgLimitChannel := make(chan struct{}, pc.settings.Reconcile.MaxParallelRequests) result := make([]cfclient.ServiceBroker, 0, len(names)) - chunks := splitStringsIntoChunks(names) + chunks := splitStringsIntoChunks(names, pc.settings.CF.ChunkSize) for _, chunk := range chunks { select { @@ -137,7 +136,9 @@ func (pc *PlatformClient) getBrokersByName(ctx context.Context, names []string) }() brokerNames := make([]string, 0, len(chunk)) brokerNames = append(brokerNames, chunk...) - query := queryBuilder{} + query := queryBuilder{ + pageSize: pc.settings.CF.PageSize, + } query.set("name", brokerNames) brokers, err := pc.client.ListServiceBrokersByQuery(query.build(ctx)) mutex.Lock() @@ -163,7 +164,7 @@ func (pc *PlatformClient) getServicesByBrokers(ctx context.Context, brokers []cf wgLimitChannel := make(chan struct{}, pc.settings.Reconcile.MaxParallelRequests) result := make([]cfclient.Service, 0, len(brokers)) - chunks := splitBrokersIntoChunks(brokers) + chunks := splitBrokersIntoChunks(brokers, pc.settings.CF.ChunkSize) for _, chunk := range chunks { select { @@ -199,7 +200,9 @@ func (pc *PlatformClient) getServicesByBrokers(ctx context.Context, brokers []cf } func (pc *PlatformClient) getServicesByBrokerGUIDs(ctx context.Context, brokerGUIDs []string) ([]cfclient.Service, error) { - query := queryBuilder{} + query := queryBuilder{ + pageSize: pc.settings.CF.PageSize, + } query.set("service_broker_guid", brokerGUIDs) return pc.client.ListServicesByQuery(query.build(ctx)) } @@ -211,7 +214,7 @@ func (pc *PlatformClient) getPlansByServices(ctx context.Context, services []cfc wgLimitChannel := make(chan struct{}, pc.settings.Reconcile.MaxParallelRequests) result := make([]cfclient.ServicePlan, 0, len(services)) - chunks := splitServicesIntoChunks(services) + chunks := splitServicesIntoChunks(services, pc.settings.CF.ChunkSize) for _, chunk := range chunks { select { @@ -247,7 +250,9 @@ func (pc *PlatformClient) getPlansByServices(ctx context.Context, services []cfc } func (pc *PlatformClient) getPlansByServiceGUIDs(ctx context.Context, serviceGUIDs []string) ([]cfclient.ServicePlan, error) { - query := queryBuilder{} + query := queryBuilder{ + pageSize: pc.settings.CF.PageSize, + } query.set("service_guid", serviceGUIDs) return pc.client.ListServicePlansByQuery(query.build(ctx)) } @@ -259,7 +264,7 @@ func (pc *PlatformClient) getPlansVisibilities(ctx context.Context, plans []cfcl var mutex sync.Mutex wgLimitChannel := make(chan struct{}, pc.settings.Reconcile.MaxParallelRequests) - chunks := splitCFPlansIntoChunks(plans) + chunks := splitCFPlansIntoChunks(plans, pc.settings.CF.ChunkSize) for _, chunk := range chunks { select { @@ -296,13 +301,16 @@ func (pc *PlatformClient) getPlansVisibilities(ctx context.Context, plans []cfcl } func (pc *PlatformClient) getPlanVisibilitiesByPlanGUID(ctx context.Context, plansGUID []string) ([]cfclient.ServicePlanVisibility, error) { - query := queryBuilder{} + query := queryBuilder{ + pageSize: pc.settings.CF.PageSize, + } query.set("service_plan_guid", plansGUID) return pc.client.ListServicePlanVisibilitiesByQuery(query.build(ctx)) } type queryBuilder struct { - filters map[string]string + filters map[string]string + pageSize int } func (q *queryBuilder) set(key string, elements []string) *queryBuilder { @@ -323,11 +331,12 @@ func (q *queryBuilder) build(ctx context.Context) map[string][]string { query := strings.Join(queryComponents, ";") log.C(ctx).Debugf("CF filter query built: %s", query) return url.Values{ - "q": []string{query}, + "q": []string{query}, + "results-per-page": []string{strconv.Itoa(q.pageSize)}, } } -func splitCFPlansIntoChunks(plans []cfclient.ServicePlan) [][]cfclient.ServicePlan { +func splitCFPlansIntoChunks(plans []cfclient.ServicePlan, maxChunkLength int) [][]cfclient.ServicePlan { resultChunks := make([][]cfclient.ServicePlan, 0) for count := len(plans); count > 0; count = len(plans) { @@ -338,7 +347,7 @@ func splitCFPlansIntoChunks(plans []cfclient.ServicePlan) [][]cfclient.ServicePl return resultChunks } -func splitStringsIntoChunks(names []string) [][]string { +func splitStringsIntoChunks(names []string, maxChunkLength int) [][]string { resultChunks := make([][]string, 0) for count := len(names); count > 0; count = len(names) { @@ -349,7 +358,7 @@ func splitStringsIntoChunks(names []string) [][]string { return resultChunks } -func splitBrokersIntoChunks(brokers []cfclient.ServiceBroker) [][]cfclient.ServiceBroker { +func splitBrokersIntoChunks(brokers []cfclient.ServiceBroker, maxChunkLength int) [][]cfclient.ServiceBroker { resultChunks := make([][]cfclient.ServiceBroker, 0) for count := len(brokers); count > 0; count = len(brokers) { @@ -360,7 +369,7 @@ func splitBrokersIntoChunks(brokers []cfclient.ServiceBroker) [][]cfclient.Servi return resultChunks } -func splitServicesIntoChunks(services []cfclient.Service) [][]cfclient.Service { +func splitServicesIntoChunks(services []cfclient.Service, maxChunkLength int) [][]cfclient.Service { resultChunks := make([][]cfclient.Service, 0) for count := len(services); count > 0; count = len(services) {