Skip to content

Commit

Permalink
chore: remove unused "builtin" feature toggle
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
blgm committed May 28, 2024
1 parent 66fde35 commit 323ab42
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 60 deletions.
1 change: 0 additions & 1 deletion docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ Feature flags can be toggled through the following configuration values. See als
| Environment Variable | Config File Value | Type | Description | Default |
|----------------------|------|-------------|------------------|----------|
| <tt>GSB_COMPATIBILITY_ENABLE_BUILTIN_BROKERPAKS</tt> <b>*</b> | compatibility.enable_builtin_brokerpaks | Boolean | <p>Load brokerpaks that are built-in to the software.</p>| "true" |
| <tt>GSB_COMPATIBILITY_ENABLE_BUILTIN_SERVICES</tt> <b>*</b> | compatibility.enable_builtin_services | Boolean | <p>Enable services that are built in to the broker i.e. not brokerpaks.</p>| "true" |
| <tt>GSB_COMPATIBILITY_ENABLE_CATALOG_SCHEMAS</tt> <b>*</b> | compatibility.enable_catalog_schemas | Boolean | <p>Enable generating JSONSchema for the service catalog.</p>| "false" |
| <tt>GSB_COMPATIBILITY_ENABLE_CF_SHARING</tt> <b>*</b> | compatibility.enable_cf_sharing | Boolean | <p>Set all services to have the Sharable flag so they can be shared</p>| "false" |
| <tt>GSB_COMPATIBILITY_ENABLE_EOL_SERVICES</tt> <b>*</b> | compatibility.enable_eol_services | Boolean | <p>Enable broker services that are end of life.</p>| "false" |
Expand Down
6 changes: 0 additions & 6 deletions pkg/broker/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
58 changes: 8 additions & 50 deletions pkg/broker/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ var _ = Describe("Registry", func() {
},
},
},
IsBuiltin: true,
}
})

Expand Down Expand Up @@ -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"`))
})
Expand Down Expand Up @@ -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)
Expand All @@ -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",
Expand All @@ -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)
Expand All @@ -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"),
)
})
})
3 changes: 0 additions & 3 deletions pkg/broker/service_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit 323ab42

Please sign in to comment.