Locate Registry with Resources (#74)

Resources rely on the registry for garbage collection, and that cannot
span namespaces.  This adds in support in your bindings to define where
the registry should go, defaulting to the service broker namespace, but
allowing a hard coded value (e.g. cluster wide singletons), or per
service instance.  Sadly, this does expose all the registry secrets now,
but it has to happen this way unless we want to track literally
everything.

Fixes #73

Author: spjmurray

crds/servicebroker.couchbase.com_servicebrokerconfigs.yaml

@@ -59,6 +59,30 @@ spec:
                       description: Plan is the name of the service plan to bind to.
                       minLength: 1
                       type: string
+                    registryNamespace:
+                      description: RegistryNamespace is only relevant when used with
+                        RegistryScope in the "Explicit" mode, and specifies the exact
+                        namespace a service instance registry will be generated in.
+                      type: string
+                    registryScope:
+                      default: BrokerLocal
+                      description: RegistryScope controls where the registry for a
+                        service instance or binding is located.  The service broker
+                        makes all generated resources owned by the relevant registry,
+                        so deleting a service instance means deleting the registry
+                        and letting garbage collection do the rest.  What is particularly
+                        important is that resources must be located in the same namespace
+                        as their owners, or they will be garbage collected.  "BrokerLocal",
+                        the default provisions service registries in the same namespace
+                        as the service broker.  "Explicit" allows service registries
+                        to be hard coded to a specific namespace. "InstanceLocal"
+                        will provision service registries in the same namespace as
+                        the service instance was provisioned in.
+                      enum:
+                      - Explicit
+                      - BrokerLocal
+                      - InstanceLocal
+                      type: string
                     service:
                       description: Service is the name of the service offering to
                         bind to.

documentation/modules/ROOT/pages/concepts/registry.adoc

@@ -126,6 +126,31 @@ Therefore, where singleton templates have been used to generate Kubernetes resou
 .Registry Ownership of Resources and Resource Sharing with Singletons
 image::reg-gc.png[align="center"]
 
+[IMPORTANT]
+====
+Kubernetes garbage collection only works when the registry and its dependent resources reside in the same namespace.
+Failure to observe this constraint will result in your service instances and bindings being deleted erroneously.
+
+See the https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/[offical documentation^] for additional garbage collection rules.
+====
+
+=== Registry Scoping
+
+The namespace in which a registry is generated is fully under your control, see the documentation for the configuration bindings for more details (`kubectl explain servicebrokerconfig.spec.bindings.registryScope`)
+The default option -- `BrokerLocal` -- maintains backward compatibility, and registries will be generated in the same namespace as the service broker.
+This, therefore, leads to the requirement that all service instance resources must be provisioned in that namespace.
+
+Other options include an explicit namespace -- `Explicit` -- where the registry and resources are hard coded to a specific namespace, and service instance local -- `InstanceLocal` -- where the registry will be provisioned in the same namespace as the service instance, as dictated by the namespace provided in the service instance creation context.
+
+== Registry Directory
+
+Garbage collection only works when the registry is located in the same namespace as the resources that it is associated with.
+The Service Broker only knows the exact namespace a service will be provisioned in on service instance creation, as provided by the request context.
+In order to keep track of what namespace contains the service instance and binding registries, the Service Broker maintains a directory.
+
+The directory is a simple map from service instance ID to a JSON document that records the service instance namespace.
+Thus, when a request to get or poll a service instance is made, where the namespace is unknown, the Service Broker can interrogate its directory and determine the correct namespace to use to look for the relevant registries.
+
 == Next Steps
 
 We have now covered all basic configuration topics.

documentation/modules/ROOT/pages/concepts/security.adoc

@@ -128,4 +128,5 @@ image::sec-static.png[align="center"]
 
 == Hybrid Namespace Selection
 
-Due to the flexibility of the Service Broker, you may choose to combine dynamically and statically namespaced configuration templates as you choose.
+Mixing the two forms of namespace selection is not supported.
+Read xref:concepts/registry.adoc#registry-based-garbage-collection[the registry based garbage collection] documentation for further details on why, and how to correctly configure your service instances and bindings.

pkg/apis/servicebroker/v1alpha1/errors.go

@@ -0,0 +1,23 @@
+// Copyright 2021 Couchbase, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file  except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the  License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1alpha1
+
+import (
+	"errors"
+)
+
+// ErrResourceReferenceMissing is raised when a resource reference to another resource/attribute
+// is missing.
+var ErrResourceReferenceMissing = errors.New("resource reference missing")

pkg/apis/servicebroker/v1alpha1/helpers.go

@@ -0,0 +1,53 @@
+// Copyright 2021 Couchbase, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file  except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the  License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1alpha1
+
+import (
+	"fmt"
+)
+
+// GetServiceAndPlanNames translates from GUIDs to human readable names used in configuration.
+func (config *ServiceBrokerConfig) GetServiceAndPlanNames(serviceID, planID string) (string, string, error) {
+	for _, service := range config.Spec.Catalog.Services {
+		if service.ID == serviceID {
+			for _, plan := range service.Plans {
+				if plan.ID == planID {
+					return service.Name, plan.Name, nil
+				}
+			}
+
+			return "", "", fmt.Errorf("%w: unable to locate plan for ID %s", ErrResourceReferenceMissing, planID)
+		}
+	}
+
+	return "", "", fmt.Errorf("%w: unable to locate service for ID %s", ErrResourceReferenceMissing, serviceID)
+}
+
+// GetTemplateBindings returns the template bindings associated with a creation request's
+// service and plan IDs.
+func (config *ServiceBrokerConfig) GetTemplateBindings(serviceID, planID string) (*ConfigurationBinding, error) {
+	service, plan, err := config.GetServiceAndPlanNames(serviceID, planID)
+	if err != nil {
+		return nil, err
+	}
+
+	for index, binding := range config.Spec.Bindings {
+		if binding.Service == service && binding.Plan == plan {
+			return &config.Spec.Bindings[index], nil
+		}
+	}
+
+	return nil, fmt.Errorf("%w: unable to locate template bindings for service plan %s/%s", ErrResourceReferenceMissing, service, plan)
+}

pkg/apis/servicebroker/v1alpha1/types.go

@@ -266,13 +266,49 @@ type RegistryValue struct {
 	Value string `json:"value"`
 }
 
+// RegistryScope allows the user to configure where the registry will be provisioned.
+// +kubebuilder:validation:Enum=Explicit;BrokerLocal;InstanceLocal
+type RegistryScope string
+
+const (
+	// RegistryScopeExplicit provisions the registry where you tell it to.
+	RegistryScopeExplicit RegistryScope = "Explicit"
+
+	// RegistryScopeBrokerLocal provisions the registry in the same namespace
+	// as the broker is running in.
+	RegistryScopeBrokerLocal RegistryScope = "BrokerLocal"
+
+	// RegistryScopeInstanceLocal provisions the registry in the same namespace
+	// as the service instance.
+	RegistryScopeInstanceLocal RegistryScope = "InstanceLocal"
+)
+
 // ConfigurationBinding binds a service plan to a set of templates
 // required to realize that plan.
 type ConfigurationBinding struct {
 	// Name is a unique identifier for the binding.
 	// +kubebuilder:validation:MinLength=1
 	Name string `json:"name"`
 
+	// RegistryScope controls where the registry for a service instance
+	// or binding is located.  The service broker makes all generated
+	// resources owned by the relevant registry, so deleting a service
+	// instance means deleting the registry and letting garbage collection
+	// do the rest.  What is particularly important is that resources
+	// must be located in the same namespace as their owners, or they will
+	// be garbage collected.  "BrokerLocal", the default provisions service
+	// registries in the same namespace as the service broker.  "Explicit"
+	// allows service registries to be hard coded to a specific namespace.
+	// "InstanceLocal" will provision service registries in the same
+	// namespace as the service instance was provisioned in.
+	// +kubebuilder:default="BrokerLocal"
+	RegistryScope RegistryScope `json:"registryScope,omitempty"`
+
+	// RegistryNamespace is only relevant when used with RegistryScope in the
+	// "Explicit" mode, and specifies the exact namespace a service instance
+	// registry will be generated in.
+	RegistryNamespace string `json:"registryNamespace,omitempty"`
+
 	// Service is the name of the service offering to bind to.
 	// +kubebuilder:validation:MinLength=1
 	Service string `json:"service"`

pkg/broker/handlers.go

@@ -81,8 +81,14 @@ func handleCreateServiceInstance(configuration *ServerConfiguration) func(http.R
 			return
 		}
 
+		dirent, err := registerDirectoryInstance(config.Config(), request.Context, configuration.Namespace, instanceID, request.ServiceID, request.PlanID)
+		if err != nil {
+			jsonError(w, err)
+			return
+		}
+
 		// Check if the instance already exists.
-		entry, err := registry.New(registry.ServiceInstance, configuration.Namespace, instanceID, false)
+		entry, err := registry.New(registry.ServiceInstance, dirent.Namespace, instanceID, false)
 		if err != nil {
 			jsonError(w, err)
 			return
@@ -333,8 +339,10 @@ func handleReadServiceInstance(configuration *ServerConfiguration) func(http.Res
 			return
 		}
 
+		dirent := getDirectoryInstance(configuration.Namespace, instanceID)
+
 		// Check if the instance exists.
-		entry, err := registry.New(registry.ServiceInstance, configuration.Namespace, instanceID, true)
+		entry, err := registry.New(registry.ServiceInstance, dirent.Namespace, instanceID, true)
 		if err != nil {
 			jsonError(w, err)
 			return
@@ -449,9 +457,11 @@ func handleUpdateServiceInstance(configuration *ServerConfiguration) func(http.R
 			return
 		}
 
+		dirent := getDirectoryInstance(configuration.Namespace, instanceID)
+
 		// Check if the instance already exists.
 		// Check if the instance exists.
-		entry, err := registry.New(registry.ServiceInstance, configuration.Namespace, instanceID, false)
+		entry, err := registry.New(registry.ServiceInstance, dirent.Namespace, instanceID, false)
 		if err != nil {
 			jsonError(w, err)
 			return
@@ -562,7 +572,12 @@ func handleDeleteServiceInstance(configuration *ServerConfiguration) func(http.R
 			return
 		}
 
-		entry, err := registry.New(registry.ServiceInstance, configuration.Namespace, instanceID, false)
+		dirent := getDirectoryInstance(configuration.Namespace, instanceID)
+
+		// Probably the wrong place for this...
+		deleteDirectoryInstance(configuration.Namespace, instanceID)
+
+		entry, err := registry.New(registry.ServiceInstance, dirent.Namespace, instanceID, false)
 		if err != nil {
 			jsonError(w, err)
 			return
@@ -653,7 +668,9 @@ func handlePollServiceInstance(configuration *ServerConfiguration) func(http.Res
 			return
 		}
 
-		entry, err := registry.New(registry.ServiceInstance, configuration.Namespace, instanceID, false)
+		dirent := getDirectoryInstance(configuration.Namespace, instanceID)
+
+		entry, err := registry.New(registry.ServiceInstance, dirent.Namespace, instanceID, false)
 		if err != nil {
 			jsonError(w, err)
 			return
@@ -838,7 +855,9 @@ func handleCreateServiceBinding(configuration *ServerConfiguration) func(http.Re
 		}
 
 		// Check if the service instance exists.
-		instanceEntry, err := registry.New(registry.ServiceInstance, configuration.Namespace, instanceID, true)
+		dirent := getDirectoryInstance(configuration.Namespace, instanceID)
+
+		instanceEntry, err := registry.New(registry.ServiceInstance, dirent.Namespace, instanceID, true)
 		if err != nil {
 			jsonError(w, err)
 			return
@@ -850,7 +869,7 @@ func handleCreateServiceBinding(configuration *ServerConfiguration) func(http.Re
 		}
 
 		// Check if the binding already exists.
-		entry, err := registry.New(registry.ServiceBinding, configuration.Namespace, bindingID, false)
+		entry, err := registry.New(registry.ServiceBinding, dirent.Namespace, bindingID, false)
 		if err != nil {
 			jsonError(w, err)
 			return
@@ -1081,7 +1100,9 @@ func handleDeleteServiceBinding(configuration *ServerConfiguration) func(http.Re
 			return
 		}
 
-		instanceEntry, err := registry.New(registry.ServiceInstance, configuration.Namespace, instanceID, true)
+		dirent := getDirectoryInstance(configuration.Namespace, instanceID)
+
+		instanceEntry, err := registry.New(registry.ServiceInstance, dirent.Namespace, instanceID, true)
 		if err != nil {
 			jsonError(w, err)
 			return
@@ -1099,7 +1120,7 @@ func handleDeleteServiceBinding(configuration *ServerConfiguration) func(http.Re
 			return
 		}
 
-		entry, err := registry.New(registry.ServiceBinding, configuration.Namespace, bindingID, false)
+		entry, err := registry.New(registry.ServiceBinding, dirent.Namespace, bindingID, false)
 		if err != nil {
 			jsonError(w, err)
 			return

pkg/broker/util.go

@@ -25,6 +25,7 @@ import (
 	v1 "github.com/couchbase/service-broker/pkg/apis/servicebroker/v1alpha1"
 	"github.com/couchbase/service-broker/pkg/errors"
 	"github.com/couchbase/service-broker/pkg/log"
+	"github.com/couchbase/service-broker/pkg/registry"
 
 	"github.com/go-openapi/jsonpointer"
 	"github.com/go-openapi/spec"
@@ -377,3 +378,75 @@ func getNamespace(context *runtime.RawExtension, namespace string) (string, erro
 
 	return namespace, nil
 }
+
+// registerDirectoryInstance allows the namespace of the registry to be chosen so garbage
+// collection works as intended.  All service instances get a directory entry for simplicity.
+func registerDirectoryInstance(config *v1.ServiceBrokerConfig, context *runtime.RawExtension, namespace, instanceID, serviceID, planID string) (*registry.DirectoryEntry, error) {
+	binding, err := config.GetTemplateBindings(serviceID, planID)
+	if err != nil {
+		return nil, err
+	}
+
+	dirent := &registry.DirectoryEntry{}
+
+	switch binding.RegistryScope {
+	case "", v1.RegistryScopeBrokerLocal:
+		// This is the default for backwards compatibility.
+		dirent.Namespace = namespace
+	case v1.RegistryScopeExplicit:
+		dirent.Namespace = binding.RegistryNamespace
+	case v1.RegistryScopeInstanceLocal:
+		rn, err := getNamespace(context, namespace)
+		if err != nil {
+			return nil, err
+		}
+
+		dirent.Namespace = rn
+	default:
+		return nil, errors.NewConfigurationError("unable to resolve registry namespace type %s", binding.RegistryScope)
+	}
+
+	directory, err := registry.NewDirectory(namespace)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := directory.Add(instanceID, dirent); err != nil {
+		return nil, err
+	}
+
+	return dirent, nil
+}
+
+// getDirectoryInstance returns the corresponding registry namespace for a service instance.
+// As this is required functionality, and existing users will not have a directory, we cannot
+// raise any errors here, instead return the service broker namespace to maintain backward
+// compaibility.
+func getDirectoryInstance(namespace, instanceID string) *registry.DirectoryEntry {
+	fake := &registry.DirectoryEntry{
+		Namespace: namespace,
+	}
+
+	directory, err := registry.NewDirectory(namespace)
+	if err != nil {
+		return fake
+	}
+
+	dirent, err := directory.Lookup(instanceID)
+	if err != nil {
+		return fake
+	}
+
+	return dirent
+}
+
+// deleteDirectoryInstance like it's counterpart above needs to ignore errors here
+// as existing service instances from earlier version won't have a directory entry.
+func deleteDirectoryInstance(namespace, instanceID string) {
+	directory, err := registry.NewDirectory(namespace)
+	if err != nil {
+		return
+	}
+
+	_ = directory.Remove(instanceID)
+}