Fix application id issue (#47)

broker.go

@@ -449,20 +449,23 @@ func (b *Broker) Bind(ctx context.Context, instanceID, bindingID string, details
 		return binding, b.errorf("no instance exists with ID %s", instanceID)
 	}
 
-	// The details.AppGUID isn't _required_ to be provided per the Open Service Broker API spec;
-	// however, in testing PCF's behavior, we found that it's always populated by the platform
-	instance.ApplicationGUID = details.AppGUID
-
-	// Ensure we have application-level mounts
-	mounts := map[string]string{
-		"/cf/" + instance.ApplicationGUID + "/secret":  "generic",
-		"/cf/" + instance.ApplicationGUID + "/transit": "transit",
-	}
+	if details.AppGUID != "" {
+		// The details.AppGUID isn't _required_ to be provided per the Open Service Broker API spec;
+		// however, in testing PCF's behavior, we found that it's always populated by the platform on
+		// later versions, but isn't by earlier versions.
+		instance.ApplicationGUID = details.AppGUID
+
+		// Ensure we have application-level mounts
+		mounts := map[string]string{
+			"/cf/" + instance.ApplicationGUID + "/secret":  "generic",
+			"/cf/" + instance.ApplicationGUID + "/transit": "transit",
+		}
 
-	// Mount the application-level backends
-	b.log.Printf("[DEBUG] creating mounts %s", mapToKV(mounts, ", "))
-	if err := b.idempotentMount(mounts); err != nil {
-		return binding, b.wErrorf(err, "failed to create mounts %s", mapToKV(mounts, ", "))
+		// Mount the application-level backends
+		b.log.Printf("[DEBUG] creating mounts %s", mapToKV(mounts, ", "))
+		if err := b.idempotentMount(mounts); err != nil {
+			return binding, b.wErrorf(err, "failed to create mounts %s", mapToKV(mounts, ", "))
+		}
 	}
 
 	// Generate the new policy
@@ -545,21 +548,21 @@ func (b *Broker) Bind(ctx context.Context, instanceID, bindingID string, details
 	b.binds[bindingID] = info
 
 	// Save the credentials
+	genericBackends := []string{"cf/" + instanceID + "/secret"}
+	transitBackends := []string{"cf/" + instanceID + "/transit"}
+	if details.AppGUID != "" {
+		genericBackends = append(genericBackends, "cf/" + details.AppGUID + "/secret")
+		transitBackends = append(transitBackends, "cf/" + details.AppGUID + "/transit")
+	}
 	binding.Credentials = map[string]interface{}{
 		"address": b.vaultAdvertiseAddr,
 		"auth": map[string]interface{}{
 			"accessor": secret.Auth.Accessor,
 			"token":    secret.Auth.ClientToken,
 		},
 		"backends": map[string]interface{}{
-			"generic": []string{
-				"cf/" + instanceID + "/secret",
-				"cf/" + details.AppGUID + "/secret",
-			},
-			"transit": []string{
-				"cf/" + instanceID + "/transit",
-				"cf/" + details.AppGUID + "/transit",
-			},
+			"generic": genericBackends,
+			"transit": transitBackends,
 		},
 		"backends_shared": map[string]interface{}{
 			"organization": "cf/" + instance.OrganizationGUID + "/secret",

broker_test.go

@@ -102,7 +102,56 @@ func TestBroker_Bind_Unbind(t *testing.T) {
 		t.Fatalf("expected a backends_shared map but received %+v", shared)
 	}
 	if sharedMap["organization"] != "cf/organization-guid/secret" {
-		t.Fatalf("expected cf/space-guid/secret but received %s", sharedMap["organization"])
+		t.Fatalf("expected cf/organization-guid/secret but received %s", sharedMap["organization"])
+	}
+	if sharedMap["space"] != "cf/space-guid/secret" {
+		t.Fatalf("expected cf/space-guid/secret but received %s", sharedMap["space"])
+	}
+
+	if err := env.Broker.Unbind(env.Context, env.InstanceID, env.BindingID, brokerapi.UnbindDetails{}); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestBroker_Bind_Unbind_No_Application_ID(t *testing.T) {
+	env, closer := defaultEnvironment(t)
+	defer closer()
+
+	// Seed the broker with the results of provisioning an instance
+	// so binding can succeed.
+	env.Broker.instances["instance-id"] = &instanceInfo{
+		SpaceGUID:           "space-guid",
+		OrganizationGUID:    "organization-guid",
+		ServiceInstanceGUID: "instance-id",
+	}
+
+	binding, err := env.Broker.Bind(env.Context, env.InstanceID, env.BindingID, brokerapi.BindDetails{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if binding.SyslogDrainURL != "" {
+		t.Fatalf("expected empty SyslogDrainURL but received %s", binding.SyslogDrainURL)
+	}
+	if binding.RouteServiceURL != "" {
+		t.Fatalf("expected empty RouteServiceURL but received %s", binding.RouteServiceURL)
+	}
+	if len(binding.VolumeMounts) != 0 {
+		t.Fatalf("expected no VolumeMounts but received %+v", binding.VolumeMounts)
+	}
+	credMap, ok := binding.Credentials.(map[string]interface{})
+	if !ok {
+		t.Fatalf("expected a credential map but received %+v", binding.Credentials)
+	}
+	shared, ok := credMap["backends_shared"]
+	if !ok {
+		t.Fatalf("expected backends_shared but they're not in %+v", credMap)
+	}
+	sharedMap, ok := shared.(map[string]interface{})
+	if !ok {
+		t.Fatalf("expected a backends_shared map but received %+v", shared)
+	}
+	if sharedMap["organization"] != "cf/organization-guid/secret" {
+		t.Fatalf("expected cf/organization-guid/secret but received %s", sharedMap["organization"])
 	}
 	if sharedMap["space"] != "cf/space-guid/secret" {
 		t.Fatalf("expected cf/space-guid/secret but received %s", sharedMap["space"])

vault.go

@@ -25,30 +25,36 @@ path "cf/{{ .SpaceGUID }}/*" {
   capabilities = ["create", "read", "update", "delete", "list"]
 }
 
-path "cf/{{ .ApplicationGUID }}" {
+path "cf/{{ .OrganizationGUID }}" {
   capabilities = ["list"]
 }
 
-path "cf/{{ .ApplicationGUID }}/*" {
-  capabilities = ["create", "read", "update", "delete", "list"]
+path "cf/{{ .OrganizationGUID }}/*" {
+  capabilities = ["read", "list"]
 }
+`
 
-path "cf/{{ .OrganizationGUID }}" {
+	ApplicationPolicyTemplate = `
+path "cf/{{ .ApplicationGUID }}" {
   capabilities = ["list"]
 }
 
-path "cf/{{ .OrganizationGUID }}/*" {
-  capabilities = ["read", "list"]
+path "cf/{{ .ApplicationGUID }}/*" {
+  capabilities = ["create", "read", "update", "delete", "list"]
 }
 `
 )
 
 // GeneratePolicy takes an io.Writer object and template input and renders the
 // resulting template into the writer.
-func GeneratePolicy(w io.Writer, i *instanceInfo) error {
-	tmpl, err := template.New("service").Parse(ServicePolicyTemplate)
+func GeneratePolicy(w io.Writer, info *instanceInfo) error {
+	policies := ServicePolicyTemplate
+	if info.ApplicationGUID != "" {
+		policies += ApplicationPolicyTemplate
+	}
+	tmpl, err := template.New("service").Parse(policies)
 	if err != nil {
 		return err
 	}
-	return tmpl.Execute(w, i)
+	return tmpl.Execute(w, info)
 }

vault_test.go

@@ -0,0 +1,92 @@
+package main
+
+import (
+	"bytes"
+	"testing"
+)
+
+func TestGeneratePolicy(t *testing.T) {
+	w := new(bytes.Buffer)
+	info := &instanceInfo{
+		OrganizationGUID: "org-id",
+		SpaceGUID: "space-id",
+		ServiceInstanceGUID: "service-instance-id",
+	}
+	if err := GeneratePolicy(w, info); err != nil {
+		t.Fatal(err)
+	}
+	result := w.String()
+	if result != expectedWithoutAppID {
+		t.Fatalf("received unexpected policy of %s", result)
+	}
+
+	w = new(bytes.Buffer)
+	info.ApplicationGUID = "application-id"
+	if err := GeneratePolicy(w, info); err != nil {
+		t.Fatal(err)
+	}
+	result = w.String()
+	if result != expectedWithAppID {
+		t.Fatalf("received unexpected policy of %s", result)
+	}
+}
+
+var expectedWithoutAppID = `
+path "cf/service-instance-id" {
+  capabilities = ["list"]
+}
+
+path "cf/service-instance-id/*" {
+	capabilities = ["create", "read", "update", "delete", "list"]
+}
+
+path "cf/space-id" {
+  capabilities = ["list"]
+}
+
+path "cf/space-id/*" {
+  capabilities = ["create", "read", "update", "delete", "list"]
+}
+
+path "cf/org-id" {
+  capabilities = ["list"]
+}
+
+path "cf/org-id/*" {
+  capabilities = ["read", "list"]
+}
+`
+
+var expectedWithAppID = `
+path "cf/service-instance-id" {
+  capabilities = ["list"]
+}
+
+path "cf/service-instance-id/*" {
+	capabilities = ["create", "read", "update", "delete", "list"]
+}
+
+path "cf/space-id" {
+  capabilities = ["list"]
+}
+
+path "cf/space-id/*" {
+  capabilities = ["create", "read", "update", "delete", "list"]
+}
+
+path "cf/org-id" {
+  capabilities = ["list"]
+}
+
+path "cf/org-id/*" {
+  capabilities = ["read", "list"]
+}
+
+path "cf/application-id" {
+  capabilities = ["list"]
+}
+
+path "cf/application-id/*" {
+  capabilities = ["create", "read", "update", "delete", "list"]
+}
+`