From 323ab42238d5803b818d5f76321db024032b2de1 Mon Sep 17 00:00:00 2001 From: George Blue Date: Tue, 28 May 2024 17:37:07 +0100 Subject: [PATCH] chore: remove unused "builtin" feature toggle The "enable-builtin-services" feature toggle enabled the GCP API services in the original Google Service Broker. These were removed long ago in the Cloud Service Broker, and this config flag does nothing. --- docs/configuration.md | 1 - pkg/broker/registry.go | 6 ---- pkg/broker/registry_test.go | 58 +++++--------------------------- pkg/broker/service_definition.go | 3 -- 4 files changed, 8 insertions(+), 60 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index e2f706f97..e5f164873 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -90,7 +90,6 @@ Feature flags can be toggled through the following configuration values. See als | Environment Variable | Config File Value | Type | Description | Default | |----------------------|------|-------------|------------------|----------| | GSB_COMPATIBILITY_ENABLE_BUILTIN_BROKERPAKS * | compatibility.enable_builtin_brokerpaks | Boolean |

Load brokerpaks that are built-in to the software.

| "true" | -| GSB_COMPATIBILITY_ENABLE_BUILTIN_SERVICES * | compatibility.enable_builtin_services | Boolean |

Enable services that are built in to the broker i.e. not brokerpaks.

| "true" | | GSB_COMPATIBILITY_ENABLE_CATALOG_SCHEMAS * | compatibility.enable_catalog_schemas | Boolean |

Enable generating JSONSchema for the service catalog.

| "false" | | GSB_COMPATIBILITY_ENABLE_CF_SHARING * | compatibility.enable_cf_sharing | Boolean |

Set all services to have the Sharable flag so they can be shared

| "false" | | GSB_COMPATIBILITY_ENABLE_EOL_SERVICES * | compatibility.enable_eol_services | Boolean |

Enable broker services that are end of life.

| "false" | diff --git a/pkg/broker/registry.go b/pkg/broker/registry.go index 16c3e6101..5de50aab8 100644 --- a/pkg/broker/registry.go +++ b/pkg/broker/registry.go @@ -37,8 +37,6 @@ var ( "deprecated": toggles.Features.Toggle("enable-gcp-deprecated-services", false, "Enable services that use deprecated GCP components."), "terraform": toggles.Features.Toggle("enable-terraform-services", false, "Enable services that use the experimental, unstable, Terraform back-end."), } - - enableBuiltinServices = toggles.Features.Toggle("enable-builtin-services", true, `Enable services that are built in to the broker i.e. not brokerpaks.`) ) // BrokerRegistry holds the list of ServiceDefinitions that can be provisioned @@ -100,10 +98,6 @@ func (brokerRegistry *BrokerRegistry) GetEnabledServices() ([]*ServiceDefinition for _, svc := range brokerRegistry.GetAllServices() { isEnabled := true - if svc.IsBuiltin { - isEnabled = enableBuiltinServices.IsActive() - } - entry := svc.CatalogEntry() tags := utils.NewStringSet(entry.Tags...) for tag, toggle := range lifecycleTagToggles { diff --git a/pkg/broker/registry_test.go b/pkg/broker/registry_test.go index 2cc565f8c..b55cfbcd3 100644 --- a/pkg/broker/registry_test.go +++ b/pkg/broker/registry_test.go @@ -30,7 +30,6 @@ var _ = Describe("Registry", func() { }, }, }, - IsBuiltin: true, } }) @@ -85,10 +84,9 @@ var _ = Describe("Registry", func() { registry := make(BrokerRegistry) err := registry.Register(&ServiceDefinition{ - ID: "b9e4332e-b42b-4680-bda5-ea1506797474", - Name: "test-service", - Plans: []ServicePlan{}, - IsBuiltin: true, + ID: "b9e4332e-b42b-4680-bda5-ea1506797474", + Name: "test-service", + Plans: []ServicePlan{}, }, nil) Expect(err).To(MatchError(`service "test-service" has no plans defined; at least one plan must be specified in the service definition or via the environment variable "GSB_SERVICE_TEST_SERVICE_PLANS" or "TEST_SERVICE_CUSTOM_PLANS"`)) }) @@ -185,11 +183,9 @@ var _ = Describe("Registry", func() { }, }, }, - IsBuiltin: true, } viper.Set(property, false) - viper.Set("compatibility.enable-builtin-services", true) registry := BrokerRegistry{} err := registry.Register(&serviceDef, nil) @@ -199,11 +195,11 @@ var _ = Describe("Registry", func() { Expect(err).ToNot(HaveOccurred()) Expect(result).To(BeEmpty()) }, - Entry("when preview are disabled and build-ins are enabled", "preview", "compatibility.enable-preview-services"), - Entry("when unmaintained are disabled and build-ins are enabled", "unmaintained", "compatibility.enable-unmaintained-services"), - Entry("when eol are disabled and build-ins are enabled", "eol", "compatibility.enable-eol-services"), - Entry("when beta are disabled and build-ins are enabled", "beta", "compatibility.enable-gcp-beta-services"), - Entry("when deprecated are disabled and build-ins are enabled", "deprecated", "compatibility.enable-gcp-deprecated-services"), + Entry("when preview are disabled", "preview", "compatibility.enable-preview-services"), + Entry("when unmaintained are disabled", "unmaintained", "compatibility.enable-unmaintained-services"), + Entry("when eol are disabled", "eol", "compatibility.enable-eol-services"), + Entry("when beta are disabled", "beta", "compatibility.enable-gcp-beta-services"), + Entry("when deprecated are disabled", "deprecated", "compatibility.enable-gcp-deprecated-services"), ) DescribeTable("should show offering", @@ -221,11 +217,9 @@ var _ = Describe("Registry", func() { }, }, }, - IsBuiltin: true, } viper.Set(property, true) - viper.Set("compatibility.enable-builtin-services", true) registry := BrokerRegistry{} err := registry.Register(&serviceDef, nil) @@ -241,41 +235,5 @@ var _ = Describe("Registry", func() { Entry("when beta are enabled and build-ins are enabled", "beta", "compatibility.enable-beta-services"), Entry("when deprecated are enabled and build-ins are enabled", "deprecated", "compatibility.enable-gcp-deprecated-services"), ) - - DescribeTable("should not show offering", - func(tag, property string) { - serviceDef := ServiceDefinition{ - ID: "b9e4332e-b42b-4680-bda5-ea1506797474", - Name: "test-service", - Tags: []string{"gcp", tag}, - Plans: []ServicePlan{ - { - ServicePlan: domain.ServicePlan{ - ID: "e1d11f65-da66-46ad-977c-6d56513baf43", - Name: "Builtin!", - Description: "Standard storage class", - }, - }, - }, - IsBuiltin: true, - } - - viper.Set(property, true) - viper.Set("compatibility.enable-builtin-services", false) - - registry := BrokerRegistry{} - err := registry.Register(&serviceDef, nil) - Expect(err).ToNot(HaveOccurred()) - - result, err := registry.GetEnabledServices() - Expect(err).ToNot(HaveOccurred()) - Expect(result).To(BeEmpty()) - }, - Entry("when preview are enabled and build-ins are disabled", "preview", "compatibility.enable-preview-services"), - Entry("when unmaintained are enabled and build-ins are disabled", "unmaintained", "compatibility.enable-unmaintained-services"), - Entry("when eol are enabled and build-ins are disabled", "eol", "compatibility.enable-eol-services"), - Entry("when beta are enabled and build-ins are disabled", "beta", "compatibility.enable-gcp-beta-services"), - Entry("when deprecated are enabled and build-ins are disabled", "deprecated", "compatibility.enable-gcp-deprecated-services"), - ) }) }) diff --git a/pkg/broker/service_definition.go b/pkg/broker/service_definition.go index 4a2a29ff5..710141944 100644 --- a/pkg/broker/service_definition.go +++ b/pkg/broker/service_definition.go @@ -66,9 +66,6 @@ type ServiceDefinition struct { // ProviderBuilder creates a new provider given the project, auth, and logger. ProviderBuilder func(plogger lager.Logger, store ServiceProviderStorage) ServiceProvider - // IsBuiltin is true if the service is built-in to the platform. - IsBuiltin bool - GlobalLabels map[string]string }