remove template.openshift.io/namespace parameter from template servicebroker and use context object instead

pkg/openservicebroker/api/types.go

@@ -1,7 +1,8 @@
 package api
 
-// from https://github.com/openservicebrokerapi/servicebroker/blob/1d301105c66187b5aa2e061a1264ecf3cbc3d2a0/_spec.md
-// and https://github.com/avade/servicebroker/blob/8c340c610c6085c1aa32144faef0d1600c21576c/_spec.md
+// from https://github.com/openservicebrokerapi/servicebroker/blob/b33070e07cea53b9540b1d5ab8d1f62ddc556eb5/spec.md
+// and https://github.com/openservicebrokerapi/servicebroker/blob/b33070e07cea53b9540b1d5ab8d1f62ddc556eb5/profile.md
+// and https://github.com/avade/servicebroker/blob/9165f14ce6c54c3f81fba760cca53bd781febd6f/spec.md
 
 import (
 	jsschema "github.com/lestrrat/go-jsschema"
@@ -73,14 +74,21 @@ const (
 )
 
 type ProvisionRequest struct {
-	ServiceID         string            `json:"service_id"`
-	PlanID            string            `json:"plan_id"`
-	Parameters        map[string]string `json:"parameters,omitempty"`
-	AcceptsIncomplete bool              `json:"accepts_incomplete,omitempty"`
-	OrganizationID    string            `json:"organization_guid"`
-	SpaceID           string            `json:"space_guid"`
+	ServiceID      string            `json:"service_id"`
+	PlanID         string            `json:"plan_id"`
+	Context        KubernetesContext `json:"context,omitempty"`
+	OrganizationID string            `json:"organization_guid"`
+	SpaceID        string            `json:"space_guid"`
+	Parameters     map[string]string `json:"parameters,omitempty"`
 }
 
+type KubernetesContext struct {
+	Platform  string `json:"platform"`
+	Namespace string `json:"namespace"`
+}
+
+const ContextPlatformKubernetes = "kubernetes"
+
 type ProvisionResponse struct {
 	DashboardURL string    `json:"dashboard_url,omitempty"`
 	Operation    Operation `json:"operation,omitempty"`
@@ -89,11 +97,11 @@ type ProvisionResponse struct {
 type Operation string
 
 type UpdateRequest struct {
-	ServiceID         string            `json:"service_id"`
-	PlanID            string            `json:"plan_id,omitempty"`
-	Parameters        map[string]string `json:"parameters,omitempty"`
-	AcceptsIncomplete bool              `json:"accepts_incomplete,omitempty"`
-	PreviousValues    struct {
+	Context        KubernetesContext `json:"context,omitempty"`
+	ServiceID      string            `json:"service_id"`
+	PlanID         string            `json:"plan_id,omitempty"`
+	Parameters     map[string]string `json:"parameters,omitempty"`
+	PreviousValues struct {
 		ServiceID      string `json:"service_id,omitempty"`
 		PlanID         string `json:"plan_id,omitempty"`
 		OrganizationID string `json:"organization_id,omitempty"`

pkg/openservicebroker/api/validation.go

@@ -3,13 +3,30 @@ package api
 import (
 	"regexp"
 
+	"k8s.io/apimachinery/pkg/api/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+
+	oapi "github.com/openshift/origin/pkg/api"
 )
 
 func ValidateProvisionRequest(preq *ProvisionRequest) field.ErrorList {
 	errors := ValidateUUID(field.NewPath("service_id"), preq.ServiceID)
 	errors = append(errors, ValidateUUID(field.NewPath("plan_id"), preq.PlanID)...)
 
+	if preq.Context.Platform == "" {
+		errors = append(errors, field.Required(field.NewPath("context.platform"), ""))
+	} else if preq.Context.Platform != ContextPlatformKubernetes {
+		errors = append(errors, field.Invalid(field.NewPath("context.platform"), preq.Context.Platform, "must equal "+ContextPlatformKubernetes))
+	}
+
+	if preq.Context.Namespace == "" {
+		errors = append(errors, field.Required(field.NewPath("context.namespace"), ""))
+	} else {
+		for _, msg := range oapi.GetNameValidationFunc(validation.ValidateNamespaceName)(preq.Context.Namespace, false) {
+			errors = append(errors, field.Invalid(field.NewPath("context.namespace"), preq.Context.Namespace, msg))
+		}
+	}
+
 	return errors
 }
 

pkg/openservicebroker/api/validation_test.go

@@ -1,6 +1,7 @@
 package api
 
 import (
+	"strings"
 	"testing"
 
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -10,66 +11,132 @@ const validUUID = "fe6e44ea-377a-457c-9fa1-ba06ad356839"
 
 func TestValidateProvisionRequest(t *testing.T) {
 	tests := []struct {
-		name        string
-		preq        ProvisionRequest
-		expectError string
+		name              string
+		preq              ProvisionRequest
+		expectErrorPrefix string
 	}{
+		{
+			name: "missing context platform",
+			preq: ProvisionRequest{
+				ServiceID: validUUID,
+				PlanID:    validUUID,
+				Context: KubernetesContext{
+					Namespace: "test",
+				},
+			},
+			expectErrorPrefix: `context.platform: Required value`,
+		},
+		{
+			name: "bad context platform",
+			preq: ProvisionRequest{
+				ServiceID: validUUID,
+				PlanID:    validUUID,
+				Context: KubernetesContext{
+					Platform:  "b@d",
+					Namespace: "test",
+				},
+			},
+			expectErrorPrefix: `context.platform: Invalid value: "b@d": must equal kubernetes`,
+		},
+		{
+			name: "missing context namespace",
+			preq: ProvisionRequest{
+				ServiceID: validUUID,
+				PlanID:    validUUID,
+				Context: KubernetesContext{
+					Platform: ContextPlatformKubernetes,
+				},
+			},
+			expectErrorPrefix: `context.namespace: Required value`,
+		},
+		{
+			name: "bad context namespace",
+			preq: ProvisionRequest{
+				ServiceID: validUUID,
+				PlanID:    validUUID,
+				Context: KubernetesContext{
+					Platform:  ContextPlatformKubernetes,
+					Namespace: "b@d",
+				},
+			},
+			expectErrorPrefix: `context.namespace: Invalid value: "b@d": `, // a DNS-1123 label must consist of ...
+		},
 		{
 			name: "empty ServiceID",
 			preq: ProvisionRequest{
 				PlanID: validUUID,
+				Context: KubernetesContext{
+					Platform:  ContextPlatformKubernetes,
+					Namespace: "test",
+				},
 			},
-			expectError: `service_id: Invalid value: "": must be a valid UUID`,
+			expectErrorPrefix: `service_id: Invalid value: "": must be a valid UUID`,
 		},
 		{
 			name: "bad ServiceID",
 			preq: ProvisionRequest{
 				ServiceID: "bad",
 				PlanID:    validUUID,
+				Context: KubernetesContext{
+					Platform:  ContextPlatformKubernetes,
+					Namespace: "test",
+				},
 			},
-			expectError: `service_id: Invalid value: "bad": must be a valid UUID`,
+			expectErrorPrefix: `service_id: Invalid value: "bad": must be a valid UUID`,
 		},
 		{
 			name: "empty PlanID",
 			preq: ProvisionRequest{
 				ServiceID: validUUID,
+				Context: KubernetesContext{
+					Platform:  ContextPlatformKubernetes,
+					Namespace: "test",
+				},
 			},
-			expectError: `plan_id: Invalid value: "": must be a valid UUID`,
+			expectErrorPrefix: `plan_id: Invalid value: "": must be a valid UUID`,
 		},
 		{
 			name: "bad PlanID",
 			preq: ProvisionRequest{
 				ServiceID: validUUID,
 				PlanID:    "bad",
+				Context: KubernetesContext{
+					Platform:  ContextPlatformKubernetes,
+					Namespace: "test",
+				},
 			},
-			expectError: `plan_id: Invalid value: "bad": must be a valid UUID`,
+			expectErrorPrefix: `plan_id: Invalid value: "bad": must be a valid UUID`,
 		},
 		{
 			name: "good",
 			preq: ProvisionRequest{
 				ServiceID: validUUID,
 				PlanID:    validUUID,
+				Context: KubernetesContext{
+					Platform:  ContextPlatformKubernetes,
+					Namespace: "test",
+				},
 			},
-			expectError: ``,
+			expectErrorPrefix: ``,
 		},
 	}
 
 	for _, test := range tests {
 		errors := ValidateProvisionRequest(&test.preq)
-		if test.expectError == "" {
+		if test.expectErrorPrefix == "" {
 			if len(errors) > 0 {
-				t.Errorf("%q: expectError was %q but errors was %q", test.name, test.expectError, errors)
+				t.Errorf("%q: expectErrorPrefix was %q but errors was %q", test.name, test.expectErrorPrefix, errors)
 			}
 		} else {
 			found := false
 			for _, err := range errors {
-				if err.Error() == test.expectError {
+				if strings.HasPrefix(err.Error(), test.expectErrorPrefix) {
 					found = true
 					break
 				}
 			}
 			if !found {
-				t.Errorf("%q: expectError was %q but errors was %q", test.name, test.expectError, errors)
+				t.Errorf("%q: expectErrorPrefix was %q but errors was %q", test.name, test.expectErrorPrefix, errors)
 			}
 		}
 	}

pkg/openservicebroker/client/client.go

@@ -85,15 +85,13 @@ func (c *client) Provision(ctx context.Context, instanceID string, preq *api.Pro
 		return nil, errs.ToAggregate()
 	}
 
-	preq.AcceptsIncomplete = true
-
 	pr, pw := io.Pipe()
 	go func() {
 		e := json.NewEncoder(pw)
 		pw.CloseWithError(e.Encode(preq))
 	}()
 
-	req, err := http.NewRequest(http.MethodPut, c.root+"/v2/service_instances/"+instanceID, pr)
+	req, err := http.NewRequest(http.MethodPut, c.root+"/v2/service_instances/"+instanceID+"?accepts_incomplete=true", pr)
 	if err != nil {
 		return nil, err
 	}

pkg/openservicebroker/server/route.go

@@ -109,8 +109,8 @@ func provision(b api.Broker, req *restful.Request) *api.Response {
 		return api.BadRequest(errors.ToAggregate())
 	}
 
-	if !preq.AcceptsIncomplete {
-		return api.NewResponse(http.StatusUnprocessableEntity, api.AsyncRequired, nil)
+	if req.QueryParameter("accepts_incomplete") != "true" {
+		return api.NewResponse(http.StatusUnprocessableEntity, &api.AsyncRequired, nil)
 	}
 
 	return b.Provision(instanceID, &preq)

pkg/openservicebroker/server/route_test.go

@@ -244,19 +244,26 @@ func TestProvision(t *testing.T) {
 			body: &api.ProvisionRequest{
 				ServiceID: validUUID,
 				PlanID:    validUUID,
+				Context: api.KubernetesContext{
+					Platform:  api.ContextPlatformKubernetes,
+					Namespace: "test",
+				},
 			},
 			expectCode:  http.StatusUnprocessableEntity,
 			expectError: `This service plan requires client support for asynchronous service operations.`,
 		},
 		{
 			name: "good",
 			req: http.Request{
-				URL: parseUrl(t, "/v2/service_instances/"+validUUID),
+				URL: parseUrl(t, "/v2/service_instances/"+validUUID+"?accepts_incomplete=true"),
 			},
 			body: &api.ProvisionRequest{
-				ServiceID:         validUUID,
-				PlanID:            validUUID,
-				AcceptsIncomplete: true,
+				ServiceID: validUUID,
+				PlanID:    validUUID,
+				Context: api.KubernetesContext{
+					Platform:  api.ContextPlatformKubernetes,
+					Namespace: "test",
+				},
 			},
 			expectCode: http.StatusOK,
 		},

pkg/template/api/constants.go

@@ -23,18 +23,12 @@ const (
 	// TemplateInstance API.
 	TemplateInstanceLabel = "template.openshift.io/template-instance"
 
-	// NamespaceParameterKey is the name of the key in the Open Service Broker API
-	// ProvisionRequest Parameters object where we receive the name of the
-	// namespace into which a template should be provisioned.  The '/' and '.'
-	// characters in the name happen to make this an invalid template parameter
-	// name so there is no immediate overlap with passed template parameters in
-	// the same object.
-	NamespaceParameterKey = "template.openshift.io/namespace"
-
 	// RequesterUsernameParameterKey is the name of the key in the Open Service
 	// Broker API ProvisionRequest Parameters object where we receive the user
-	// name which will be impersonated during template provisioning.  See above
-	// note.
+	// name which will be impersonated during template provisioning.  The '/'
+	// and '.' characters in the name happen to make this an invalid template
+	// parameter name so there is no immediate overlap with passed template
+	// parameters in the same object.
 	RequesterUsernameParameterKey = "template.openshift.io/requester-username"
 
 	// ServiceBrokerRoot is the API root of the template service broker.

pkg/template/servicebroker/catalog.go

@@ -46,18 +46,13 @@ func serviceFromTemplate(template *templateapi.Template) *api.Service {
 	}
 
 	properties := map[string]*jsschema.Schema{
-		templateapi.NamespaceParameterKey: {
-			Title:       namespaceTitle,
-			Description: namespaceDescription,
-			Type:        []jsschema.PrimitiveType{jsschema.StringType},
-		},
 		templateapi.RequesterUsernameParameterKey: {
 			Title:       requesterUsernameTitle,
 			Description: requesterUsernameDescription,
 			Type:        []jsschema.PrimitiveType{jsschema.StringType},
 		},
 	}
-	required := []string{templateapi.NamespaceParameterKey, templateapi.RequesterUsernameParameterKey}
+	required := []string{templateapi.RequesterUsernameParameterKey}
 	for _, param := range template.Parameters {
 		properties[param.Name] = &jsschema.Schema{
 			Title:       param.DisplayName,

pkg/template/servicebroker/catalog_test.go

@@ -76,16 +76,10 @@ func TestServiceFromTemplate(t *testing.T) {
 								Type:      schema.PrimitiveTypes{schema.ObjectType},
 								SchemaRef: "http://json-schema.org/draft-04/schema",
 								Required: []string{
-									"template.openshift.io/namespace",
 									"template.openshift.io/requester-username",
 									"param1",
 								},
 								Properties: map[string]*schema.Schema{
-									"template.openshift.io/namespace": {
-										Title:       "Template service broker: namespace",
-										Description: "OpenShift namespace in which to provision service",
-										Type:        schema.PrimitiveTypes{schema.StringType},
-									},
 									"template.openshift.io/requester-username": {
 										Title:       "Template service broker: requester username",
 										Description: "OpenShift user requesting provision/bind",

pkg/template/servicebroker/provision.go

@@ -28,7 +28,7 @@ func (b *Broker) ensureSecret(u user.Info, namespace string, instanceID string,
 	}
 
 	for k, v := range preq.Parameters {
-		if k != templateapi.NamespaceParameterKey && k != templateapi.RequesterUsernameParameterKey {
+		if k != templateapi.RequesterUsernameParameterKey {
 			secret.Data[k] = []byte(v)
 		}
 	}
@@ -201,7 +201,7 @@ func (b *Broker) Provision(instanceID string, preq *api.ProvisionRequest) *api.R
 		return api.BadRequest(errs.ToAggregate())
 	}
 
-	namespace := preq.Parameters[templateapi.NamespaceParameterKey]
+	namespace := preq.Context.Namespace
 	impersonate := preq.Parameters[templateapi.RequesterUsernameParameterKey]
 	u := &user.DefaultInfo{Name: impersonate}
 

pkg/template/servicebroker/test-scripts/provision.sh

@@ -7,9 +7,12 @@ serviceUUID=${serviceUUID-$(oc get template cakephp-mysql-example -n openshift -
 req="{
   \"plan_id\": \"$planUUID\",
   \"service_id\": \"$serviceUUID\",
+  \"context\": {
+    \"platform\": \"kubernetes\",
+    \"namespace\": \"$namespace\"
+  },
   \"parameters\": {
     \"MYSQL_USER\": \"username\",
-    \"template.openshift.io/namespace\": \"$namespace\",
     \"template.openshift.io/requester-username\": \"$requesterUsername\"
   },
   \"accepts_incomplete\": true