Save extracted credentials into a secret (#775)

* initial commit

* Fixing marshal and unmarshal of data.

* fixing binding

* adding documentation for creating a new runtime.

* fixing provision issue when no extracted credentials occurs.

* fix for unbind.go

* move action hooks to base methods for provision and update.

docs/proposals/extracted_credentials_saved_as_secrets.md

@@ -8,9 +8,7 @@ The problem is that we should not manage data that is of a sensitive nature if w
 In the secret, we will save the data in the following format.
 ```yaml
 data:
-  DB_PASSWORD: <GOB_ENCODED_VALUE>
-  DB_USERNAME: <GOB_ENCODED_VALUE>
-  ....
+  credentials: <base64 encoded json>
 apiVersion: v1
 kind: Secret
 metadata:
@@ -20,7 +18,7 @@ metadata:
     <labels provided>
 ```
 
-[Gob encoding](https://godoc.org/encoding/gob) will allow us to save arbitrary data in the secret for a key. The secrets keys will look rational to a user who looks at the created secret. This user would need permissions to see the secret, but if someone is looking at the secret making it obvious what data is in there will be helpful. 
+To encode the credentials in a generic way, we must marshal a `map[string]interface{}` so that we can retrieve the data. This is not how I wanted the secret to look, but will allow anyone to use the credentials and will allow us to retrieve the credentials without writing our own gob parser.
 
 The functions for saving and retrieving will be in the `clients` package. This means the callers will be required to use the underlying extracted credentials type `map[string]interface{}` because we do not want a circular dependency between `apb` package and `clients` package. 
 
@@ -56,9 +54,9 @@ type ExtractedCredentials interface {
 
 
 ### Work Items
-- [ ] Add kubernetes client methods to interact with extracted credentials in the [namespace](https://github.com/openshift/ansible-service-broker/blob/master/docs/config.md#openshift-configuration). 
-- [ ] Add runtime methods for interacting with extracted credentials. These methods should be overridable. 
-- [ ] Remove all dao implementation and interface methods regarding extracted credentials.
-- [ ] Remove all instances of interacting with dao extracted credentials in the `broker` package. Add back call to APB package to get extracted credentials when needed.
-- [ ] Update APB package to create/save/delete extracted credentials for the correct actions. this should call the correct `runtime` package methods.
-- [ ] Add exposed method on APB  that will retrieve the extracted credentials.
+- [x] Add kubernetes client methods to interact with extracted credentials in the [namespace](https://github.com/openshift/ansible-service-broker/blob/master/docs/config.md#openshift-configuration). 
+- [x] Add runtime methods for interacting with extracted credentials. These methods should be overridable. 
+- [x] Remove all dao implementation and interface methods regarding extracted credentials.
+- [x] Remove all instances of interacting with dao extracted credentials in the `broker` package. Add back call to APB package to get extracted credentials when needed.
+- [x] Update APB package to create/save/delete extracted credentials for the correct actions. this should call the correct `runtime` package methods.
+- [x] Add exposed method on APB  that will retrieve the extracted credentials.

pkg/apb/bind.go

@@ -26,7 +26,7 @@ import (
 
 // Bind - Will run the APB with the bind action.
 func (e *executor) Bind(
-	instance *ServiceInstance, parameters *Parameters,
+	instance *ServiceInstance, parameters *Parameters, bindingID string,
 ) <-chan StatusMessage {
 	log.Notice("============================================================")
 	log.Notice("                       BINDING                              ")
@@ -84,6 +84,13 @@ func (e *executor) Bind(
 			return
 		}
 
+		labels := map[string]string{"apbAction": "bind", "apbName": instance.Spec.FQName}
+		err = runtime.Provider.CreateExtractedCredential(bindingID, clusterConfig.Namespace, creds.Credentials, labels)
+		if err != nil {
+			log.Errorf("apb::%v error occurred - %v", executionMethodProvision, err)
+			e.actionFinishedWithError(err)
+			return
+		}
 		e.extractedCredentials = creds
 		e.actionFinishedWithSuccess()
 	}()

pkg/apb/deprovision.go

@@ -74,6 +74,12 @@ func (e *executor) Deprovision(instance *ServiceInstance) <-chan StatusMessage {
 			e.actionFinishedWithError(err)
 			return
 		}
+		err = runtime.Provider.DeleteExtractedCredential(instance.ID.String(), clusterConfig.Namespace)
+		if err != nil {
+			log.Errorf("unable to delete the extracted credentials - %v", err)
+			e.actionFinishedWithError(err)
+			return
+		}
 
 		e.actionFinishedWithSuccess()
 	}()

pkg/apb/executor.go

@@ -44,8 +44,8 @@ type ExecutorAccessors interface {
 type ExecutorAsync interface {
 	Provision(*ServiceInstance) <-chan StatusMessage
 	Deprovision(instance *ServiceInstance) <-chan StatusMessage
-	Bind(instance *ServiceInstance, parameters *Parameters) <-chan StatusMessage
-	Unbind(instance *ServiceInstance, parameters *Parameters) <-chan StatusMessage
+	Bind(instance *ServiceInstance, parameters *Parameters, bindingID string) <-chan StatusMessage
+	Unbind(instance *ServiceInstance, parameters *Parameters, bindingID string) <-chan StatusMessage
 	Update(instance *ServiceInstance) <-chan StatusMessage
 }
 

pkg/apb/ext_creds.go

@@ -25,6 +25,7 @@ import (
 	"time"
 
 	"github.com/openshift/ansible-service-broker/pkg/clients"
+	"github.com/openshift/ansible-service-broker/pkg/runtime"
 	"github.com/openshift/ansible-service-broker/pkg/version"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -41,6 +42,11 @@ const (
 	GatherCredentialsCommand = "broker-bind-creds"
 )
 
+var (
+	// ErrExtractedCredentialsNotFound - Extracted Credentials are not found.
+	ErrExtractedCredentialsNotFound = fmt.Errorf("credentials not found")
+)
+
 type extractCreds func(string, string) (*ExtractedCredentials, error)
 
 // ExtractCredentials - Extract credentials from pod in a certain namespace.
@@ -185,3 +191,31 @@ func decodeOutput(output []byte) ([]byte, error) {
 
 	return decodedjson, nil
 }
+
+// GetExtractedCredentials - Will get the extracted credentials for a caller of the APB package.
+func GetExtractedCredentials(id string) (*ExtractedCredentials, error) {
+	creds, err := runtime.Provider.GetExtractedCredential(id, clusterConfig.Namespace)
+	if err != nil {
+		switch {
+		case err == runtime.ErrCredentialsNotFound:
+			log.Debugf("extracted credential secret not found - %v", id)
+			return nil, ErrExtractedCredentialsNotFound
+		default:
+			log.Errorf("unable to get the extracted credential secret - %v", err)
+			return nil, err
+		}
+	}
+	return &ExtractedCredentials{Credentials: creds}, nil
+}
+
+// DeleteExtractedCredentials - Will delete the extracted credentials for a caller of the APB package.
+// Please use this method with caution.
+func DeleteExtractedCredentials(id string) error {
+	return runtime.Provider.DeleteExtractedCredential(id, clusterConfig.Namespace)
+}
+
+// SetExtractedCredentials - Will set new credentials for an id.
+// Please use this method with caution.
+func SetExtractedCredentials(id string, creds *ExtractedCredentials) error {
+	return runtime.Provider.CreateExtractedCredential(id, clusterConfig.Namespace, creds.Credentials, nil)
+}

pkg/apb/provision.go

@@ -16,6 +16,10 @@
 
 package apb
 
+import (
+	"github.com/openshift/ansible-service-broker/pkg/runtime"
+)
+
 // Provision - will run the abp with the provision action.
 func (e *executor) Provision(instance *ServiceInstance) <-chan StatusMessage {
 	log.Notice("============================================================")
@@ -27,7 +31,25 @@ func (e *executor) Provision(instance *ServiceInstance) <-chan StatusMessage {
 	log.Noticef("Spec.Description: %s", instance.Spec.Description)
 	log.Notice("============================================================")
 
-	go e.provisionOrUpdate(executionMethodProvision, instance)
-
+	go func() {
+		e.actionStarted()
+		err := e.provisionOrUpdate(executionMethodProvision, instance)
+		if err != nil {
+			log.Errorf("Provision APB error: %v", err)
+			e.actionFinishedWithError(err)
+			return
+		}
+		// Provision can not have extracted credentials.
+		if e.extractedCredentials != nil {
+			labels := map[string]string{"apbAction": string(executionMethodProvision), "apbName": instance.Spec.FQName}
+			err := runtime.Provider.CreateExtractedCredential(instance.ID.String(), clusterConfig.Namespace, e.extractedCredentials.Credentials, labels)
+			if err != nil {
+				log.Errorf("apb::%v error occurred - %v", executionMethodProvision, err)
+				e.actionFinishedWithError(err)
+				return
+			}
+		}
+		e.actionFinishedWithSuccess()
+	}()
 	return e.statusChan
 }

pkg/apb/provision_or_update.go

@@ -35,8 +35,7 @@ const (
 )
 
 // returns PodName, ExtractedCredentials, error
-func (e *executor) provisionOrUpdate(method executionMethod, instance *ServiceInstance) {
-	e.actionStarted()
+func (e *executor) provisionOrUpdate(method executionMethod, instance *ServiceInstance) error {
 	// Explicitly error out if image field is missing from instance.Spec
 	// was introduced as a change to the apb instance.Spec to support integration
 	// with the broker and still allow for providing an img path
@@ -46,23 +45,20 @@ func (e *executor) provisionOrUpdate(method executionMethod, instance *ServiceIn
 		log.Error("No image field found on the apb instance.Spec (apb.yaml)")
 		log.Error("apb instance.Spec requires [name] and [image] fields to be separate")
 		log.Error("Are you trying to run a legacy apb without an image field?")
-		e.actionFinishedWithError(errors.New("No image field found on instance.Spec"))
-		return
+		return errors.New("No image field found on instance.Spec")
 	}
 
 	k8scli, err := clients.Kubernetes()
 	if err != nil {
 		log.Error("Something went wrong getting kubernetes client")
-		e.actionFinishedWithError(err)
-		return
+		return err
 	}
 
 	ns := instance.Context.Namespace
 	log.Info("Checking if namespace %s exists.", ns)
 	_, err = k8scli.Client.CoreV1().Namespaces().Get(ns, metav1.GetOptions{})
 	if err != nil {
-		e.actionFinishedWithError(fmt.Errorf("Project %s does not exist", ns))
-		return
+		return fmt.Errorf("Project %s does not exist", ns)
 	}
 
 	metrics.ActionStarted(string(method))
@@ -78,8 +74,7 @@ func (e *executor) provisionOrUpdate(method executionMethod, instance *ServiceIn
 	)
 	if err != nil {
 		log.Errorf("Problem executing apb [%s] provision - err: %v ", executionContext.PodName, err)
-		e.actionFinishedWithError(err)
-		return
+		return err
 	}
 
 	if instance.Spec.Runtime >= 2 || !instance.Spec.Bindable {
@@ -88,14 +83,12 @@ func (e *executor) provisionOrUpdate(method executionMethod, instance *ServiceIn
 			k8scli.Client.CoreV1().Pods(executionContext.Namespace), e.updateDescription)
 		if err != nil {
 			log.Errorf("Provision or Update action failed - %v", err)
-			e.actionFinishedWithError(err)
-			return
+			return err
 		}
 	}
 
 	if !instance.Spec.Bindable {
-		e.actionFinishedWithSuccess()
-		return
+		return nil
 	}
 
 	creds, err := ExtractCredentials(
@@ -106,11 +99,9 @@ func (e *executor) provisionOrUpdate(method executionMethod, instance *ServiceIn
 
 	if err != nil {
 		log.Errorf("apb::%v error occurred - %v", method, err)
-		e.actionFinishedWithError(err)
-		return
+		return err
 	}
 
 	e.extractedCredentials = creds
-	e.actionFinishedWithSuccess()
-	return
+	return nil
 }

pkg/apb/unbind.go

@@ -26,7 +26,7 @@ import (
 
 // Unbind - runs the abp with the unbind action.
 func (e *executor) Unbind(
-	instance *ServiceInstance, parameters *Parameters,
+	instance *ServiceInstance, parameters *Parameters, bindingID string,
 ) <-chan StatusMessage {
 	log.Notice("============================================================")
 	log.Notice("                       UNBINDING                            ")
@@ -70,6 +70,11 @@ func (e *executor) Unbind(
 			e.actionFinishedWithError(err)
 			return
 		}
+		// Delete the binding extracted credential here.
+		err = runtime.Provider.DeleteExtractedCredential(bindingID, clusterConfig.Namespace)
+		if err != nil {
+			log.Infof("Unbind failed to delete extracted credential m- %v", err)
+		}
 
 		e.actionFinishedWithSuccess()
 	}()

pkg/apb/update.go

@@ -16,6 +16,10 @@
 
 package apb
 
+import (
+	"github.com/openshift/ansible-service-broker/pkg/runtime"
+)
+
 // Update - will run the abp with the provision action.
 func (e *executor) Update(instance *ServiceInstance) <-chan StatusMessage {
 	log.Notice("============================================================")
@@ -27,7 +31,25 @@ func (e *executor) Update(instance *ServiceInstance) <-chan StatusMessage {
 	log.Noticef("Spec.Description: %s", instance.Spec.Description)
 	log.Notice("============================================================")
 
-	go e.provisionOrUpdate(executionMethodUpdate, instance)
+	go func() {
+		e.actionStarted()
+		err := e.provisionOrUpdate(executionMethodUpdate, instance)
+		if err != nil {
+			log.Errorf("Update APB error: %v", err)
+			e.actionFinishedWithError(err)
+			return
+		}
+		if e.extractedCredentials != nil {
+			labels := map[string]string{"apbAction": string(executionMethodUpdate), "apbName": instance.Spec.FQName}
+			err := runtime.Provider.UpdateExtractedCredential(instance.ID.String(), clusterConfig.Namespace, e.extractedCredentials.Credentials, labels)
+			if err != nil {
+				log.Errorf("apb::%v error occurred - %v", executionMethodUpdate, err)
+				e.actionFinishedWithError(err)
+				return
+			}
+		}
+		e.actionFinishedWithSuccess()
+	}()
 
 	return e.statusChan
 }

pkg/app/app.go

@@ -185,7 +185,7 @@ func CreateApp() App {
 
 	// Initialize Runtime
 	log.Debug("Connecting to Cluster")
-	agnosticruntime.NewRuntime()
+	agnosticruntime.NewRuntime(nil)
 	agnosticruntime.Provider.ValidateRuntime()
 	if err != nil {
 		log.Error(err.Error())
@@ -257,7 +257,7 @@ func CreateApp() App {
 	// Intiialize the cluster config.
 	apb.InitializeClusterConfig(app.config.GetSubConfig("openshift"))
 	if app.broker, err = broker.NewAnsibleBroker(
-		app.dao, app.registry, *app.engine, app.config.GetSubConfig("broker"),
+		app.dao, app.registry, *app.engine, app.config.GetSubConfig("broker"), app.config.GetString("openshift.namespace"),
 	); err != nil {
 		log.Error("Failed to create AnsibleBroker\n")
 		log.Error(err.Error())

pkg/broker/binding_subscriber.go

@@ -21,7 +21,6 @@
 package broker
 
 import (
-	"github.com/openshift/ansible-service-broker/pkg/apb"
 	"github.com/openshift/ansible-service-broker/pkg/dao"
 )
 
@@ -42,12 +41,6 @@ func (b *BindingWorkSubscriber) Subscribe(msgBuffer <-chan JobMsg) {
 		log.Info("Listening for binding messages")
 		for msg := range msgBuffer {
 			log.Debug("Processed binding message from buffer")
-			if msg.State.State == apb.StateSucceeded {
-				log.Debug("CALL SetExtractedCredentials $v - %v", msg.BindingUUID, msg.ExtractedCredentials)
-				if err := b.dao.SetExtractedCredentials(msg.BindingUUID, &msg.ExtractedCredentials); err != nil {
-					log.Errorf("failed to set extracted credentials after bind %v", err)
-				}
-			}
 			if _, err := b.dao.SetState(msg.InstanceUUID, msg.State); err != nil {
 				log.Errorf("failed to set state after provision %v", err)
 			}