Refactor reconciler internals to streamline annotations and multiple …

…services in custom environment variables templatesSome existing components have been simplified and several others havebeen removed in order to reduce the number of indirections in thesystem.The `annotations` package has been created to group all annotationrelated symbols. `annotations.Handler` is an interface specifying theannotation handler contract, which is responsible for processing asingle annotation key and value pairs as for example`.../status.dbConnectionIP` and `binding:env:attribute`; an instance canbe obtained through `annotations.BuildHandler()`.There are currently two annotation handlers:`annotations.AttributeHandler` and `annotations.ResourceHandler`. Thehandlers return a `Result` containing the data produced by theannotation in `Data`, which is stored in a deep structure with the rootpath found in the `Path` field. Additionally the binding type (whethervolume mounts or environment variables should be used) is present in the`Type` field.The `AttributeHandler` is used to collect a value from the object wherethe annotation is declared; for example, in the example`.../status.dbConnectionIP` and `binding:env:attribute` annotation pairthe data found in the simple JSONPath `status.dbConnectionIP` will beextracted from the object. On the other hand, the `ResourceHandler` isused to collect information present in another external resource, forexample a config map or a secret; in the `.../status.dbCredentials-user`and `binding:env:object:configmap` annotation pairs example, the`data.user` field from the config map resource which name is present in`status.dbCredentials` will be extracted.The `envvars` package contains tooling to construct `map[string][]byte`data-structures from an arbitrarily nested `map[string]interface{}`value.The `nested` package contains utilities to extract the arbitrarily deepdata-structures in the annotation handlers.Since the whole data extraction process has been modified to provide thetesting baseline, this change also includes support for customenvironment variable templates such as `{{ index "v1alpha1""postgresql.baiju.dev" "Database" "db-testing" "metadata" "name" }}` and`{{ .v1alpha1.postgresql_baiju_dev.Database.db_testing.metadata.name}}`.Test coverage should be improved, and internals should keep to besimplified in subsequent work units, such as:- Review whether `ServiceBinder` and `Binder` can be further simplified.- Review whether the data being propagated through function parameters in the reconciliation can be re-grouped now that some indirections have been removed.

Makefile

@@ -195,17 +195,18 @@ out/test-namespace:
 get-test-namespace: out/test-namespace
 	$(eval TEST_NAMESPACE := $(shell cat $(OUTPUT_DIR)/test-namespace))
 
-.PHONY: deploy-e2e-crds
-deploy-e2e-crds:
-	$(Q)kubectl --namespace $(TEST_NAMESPACE) apply -f ./test/third-party-crds/postgresql_v1alpha1_database_crd.yaml
-
 # E2E test
-.PHONY: e2e-setup
-e2e-setup: e2e-cleanup
+.PHONY: e2e-deploy-3rd-party-crds
+e2e-deploy-3rd-party-crds: get-test-namespace
+	$(Q)kubectl --namespace $(TEST_NAMESPACE) apply -f ./test/third-party-crds/
+
+.PHONY: e2e-create-namespace
+e2e-create-namespace:
 	$(Q)kubectl create namespace $(TEST_NAMESPACE)
-	$(Q)kubectl --namespace $(TEST_NAMESPACE) apply -f ./test/third-party-crds/postgresql_v1alpha1_database_crd.yaml
-	$(Q)kubectl --namespace $(TEST_NAMESPACE) apply -f ./test/third-party-crds/etcd_v1beta2_etcdcluster_crd.yaml
-	$(Q)mkdir -p $(LOGS_DIR)/e2e
+
+.PHONY: e2e-setup
+e2e-setup: e2e-cleanup e2e-create-namespace e2e-deploy-3rd-party-crds
+	$(Q)mkdir -p ${LOGS_DIR}/e2e
 
 .PHONY: e2e-cleanup
 e2e-cleanup: get-test-namespace

go.mod

@@ -8,15 +8,14 @@ require (
 	github.com/go-logr/logr v0.1.0
 	github.com/go-openapi/spec v0.19.4
 	github.com/google/go-containerregistry v0.0.0-20191218175032-34fb8ff33bed // indirect
+	github.com/imdario/mergo v0.3.8
 	github.com/openshift/api v3.9.1-0.20190924102528-32369d4db2ad+incompatible
 	github.com/openshift/custom-resource-status v0.0.0-20190822192428-e62f2f3b79f3
 	github.com/operator-backing-service-samples/postgresql-operator v0.0.0-20191023140509-5c3697ed3069
 	github.com/operator-framework/operator-lifecycle-manager v0.0.0-20191115003340-16619cd27fa5
 	github.com/operator-framework/operator-sdk v0.15.2
-	github.com/pmezard/go-difflib v1.0.0
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.4.0
-	gopkg.in/yaml.v2 v2.2.4
 	gotest.tools v2.2.0+incompatible
 	k8s.io/api v0.0.0
 	k8s.io/apiextensions-apiserver v0.0.0

pkg/controller/servicebindingrequest/annotations/annotations.go

@@ -0,0 +1,90 @@
+package annotations
+
+import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/client-go/dynamic"
+)
+
+// bindingType encodes the medium the binding should deliver the configuration value.
+type bindingType string
+
+const (
+	// BindingTypeVolumeMount indicates the binding should happen through a volume mount.
+	BindingTypeVolumeMount bindingType = "volumemount"
+	// BindingTypeEnvVar indicates the binding should happen through environment variables.
+	BindingTypeEnvVar bindingType = "env"
+)
+
+// supportedBindingTypes contains all currently supported binding types.
+var supportedBindingTypes = map[bindingType]bool{
+	BindingTypeVolumeMount: true,
+	BindingTypeEnvVar:      true,
+}
+
+// dataPath is the path ConfigMap and Secret resources use to scope their data.
+//
+// note: it is currently used to provide a pointer to the "data" string, which is the location
+// ConfigMap and Secret resources keep user data.
+var dataPath = "data"
+
+// Result contains data that has been collected by an annotation handler.
+type Result struct {
+	// Data contains the annotation data collected by an annotation handler inside a deep structure
+	// with its root being the value specified in the Path field.
+	Data map[string]interface{}
+	// Type indicates where the Object field should be injected in the application; can be either
+	// "env" or "volumemount".
+	Type bindingType
+	// Path is the nested location the collected data can be found in the Data field.
+	Path string
+}
+
+// Handler should be implemented by types that want to offer a mechanism to provide binding data to
+// the system.
+type Handler interface {
+	// Handle returns binding data.
+	Handle() (Result, error)
+}
+
+type ErrHandlerNotFound string
+
+func (e ErrHandlerNotFound) Error() string {
+	return fmt.Sprintf("could not find handler for annotation value %q", string(e))
+}
+
+func IsErrHandlerNotFound(err error) bool {
+	_, ok := err.(ErrHandlerNotFound)
+	return ok
+}
+
+// BuildHandler attempts to create an annotation handler for the given annotationKey and
+// annotationValue. kubeClient is required by some annotation handlers, and an error is returned in
+// the case it is required by an annotation handler but is not defined.
+func BuildHandler(
+	kubeClient dynamic.Interface,
+	obj *unstructured.Unstructured,
+	annotationKey string,
+	annotationValue string,
+	restMapper meta.RESTMapper,
+) (Handler, error) {
+	bindingInfo, err := NewBindingInfo(annotationKey, annotationValue)
+	if err != nil {
+		return nil, err
+	}
+
+	val := bindingInfo.Value
+
+	switch {
+	case IsAttribute(val):
+		return NewAttributeHandler(bindingInfo, *obj), nil
+	case IsSecret(val):
+		return NewSecretHandler(kubeClient, bindingInfo, *obj, restMapper)
+	case IsConfigMap(val):
+		return NewConfigMapHandler(kubeClient, bindingInfo, *obj, restMapper)
+	default:
+		return nil, ErrHandlerNotFound(val)
+	}
+}

pkg/controller/servicebindingrequest/annotations/attribute.go

@@ -0,0 +1,64 @@
+package annotations
+
+import (
+	"strings"
+
+	"github.com/redhat-developer/service-binding-operator/pkg/controller/servicebindingrequest/nested"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+const AttributeValue = "binding:env:attribute"
+
+// AttributeHandler handles "binding:env:attribute" annotations.
+type AttributeHandler struct {
+	// inputPath is the path that should be extracted from the resource. Required.
+	inputPath string
+	// outputPath is the path the extracted data should be placed under in the
+	// resulting unstructured object in Handler. Required.
+	outputPath string
+	// resource is the unstructured object to extract data using inputPath. Required.
+	resource unstructured.Unstructured
+}
+
+// Handle returns a unstructured object according to the "binding:env:attribute"
+// annotation strategy.
+func (h *AttributeHandler) Handle() (Result, error) {
+	val, _, err := nested.GetValue(h.resource.Object, h.inputPath, h.outputPath)
+	if err != nil {
+		return Result{}, err
+	}
+	return Result{
+		Data: val,
+	}, nil
+}
+
+// IsAttribute returns true if the annotation value should trigger the attribute
+// handler.
+func IsAttribute(s string) bool {
+	return AttributeValue == s
+}
+
+// NewAttributeHandler constructs an AttributeHandler.
+func NewAttributeHandler(
+	bindingInfo *BindingInfo,
+	resource unstructured.Unstructured,
+) *AttributeHandler {
+	outputPath := bindingInfo.SourcePath
+	if len(bindingInfo.ResourceReferencePath) > 0 {
+		outputPath = bindingInfo.ResourceReferencePath
+	}
+
+	// the current implementation removes "status." and "spec." from fields exported through
+	// annotations.
+	for _, prefix := range []string{"status.", "spec."} {
+		if strings.HasPrefix(outputPath, prefix) {
+			outputPath = strings.Replace(outputPath, prefix, "", 1)
+		}
+	}
+
+	return &AttributeHandler{
+		inputPath:  bindingInfo.SourcePath,
+		outputPath: outputPath,
+		resource:   resource,
+	}
+}

pkg/controller/servicebindingrequest/annotations/attribute_test.go

@@ -0,0 +1,132 @@
+package annotations
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+// TestAttributeHandler exercises the AttributeHandler's ability to extract values according to the
+// given annotation name and value.
+func TestAttributeHandler(t *testing.T) {
+	type args struct {
+		obj      *unstructured.Unstructured
+		key      string
+		value    string
+		expected map[string]interface{}
+	}
+
+	assertHandler := func(args args) func(t *testing.T) {
+		return func(t *testing.T) {
+			bindingInfo, err := NewBindingInfo(args.key, args.value)
+			require.NoError(t, err)
+			require.NotNil(t, bindingInfo)
+			handler := NewAttributeHandler(bindingInfo, *args.obj)
+			got, err := handler.Handle()
+			require.NoError(t, err)
+			require.NotNil(t, got)
+			require.Equal(t, args.expected, got.Data)
+		}
+	}
+
+	// "scalar" tests whether a single deep scalar value can be extracted from the given object.
+	t.Run("should extract a single value from .status.dbConnectionIP into .dbConnectionIP",
+		assertHandler(args{
+			expected: map[string]interface{}{
+				"dbConnectionIP": "127.0.0.1",
+			},
+			key:   "servicebindingoperator.redhat.io/status.dbConnectionIP",
+			value: "binding:env:attribute",
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"status": map[string]interface{}{
+						"dbConnectionIP": "127.0.0.1",
+					},
+				},
+			},
+		}),
+	)
+
+	// "scalar#alias" tests whether a single deep scalar value can be extracted from the given object
+	// returning a different name than the original given path.
+	t.Run("should extract a single value from .status.dbConnectionIP into .alias",
+		assertHandler(args{
+			expected: map[string]interface{}{
+				"alias": "127.0.0.1",
+			},
+			key:   "servicebindingoperator.redhat.io/alias-status.dbConnectionIP",
+			value: "binding:env:attribute",
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"status": map[string]interface{}{
+						"dbConnectionIP": "127.0.0.1",
+					},
+				},
+			},
+		}),
+	)
+
+	// tests whether a deep slice value can be extracted from the given object.
+	t.Run("should extract a slice from .status.dbConnectionIPs into .dbConnectionIPs",
+		assertHandler(args{
+			expected: map[string]interface{}{
+				"dbConnectionIPs": []string{"127.0.0.1", "1.1.1.1"},
+			},
+			key:   "servicebindingoperator.redhat.io/status.dbConnectionIPs",
+			value: "binding:env:attribute",
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"status": map[string]interface{}{
+						"dbConnectionIPs": []string{"127.0.0.1", "1.1.1.1"},
+					},
+				},
+			},
+		}),
+	)
+
+	// tests whether a deep map value can be extracted from the given object.
+	t.Run("should extract a map from .status.connection into .connection", assertHandler(args{
+		expected: map[string]interface{}{
+			"connection": map[string]interface{}{
+				"host": "127.0.0.1",
+				"port": "1234",
+			},
+		},
+		key:   "servicebindingoperator.redhat.io/status.connection",
+		value: "binding:env:attribute",
+		obj: &unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"status": map[string]interface{}{
+					"connection": map[string]interface{}{
+						"host": "127.0.0.1",
+						"port": "1234",
+					},
+				},
+			},
+		},
+	}))
+
+	// "map.key" tests whether a deep map key can be extracted from the given object.
+	t.Run("should extract a single map key from .status.connection into .connection",
+		assertHandler(args{
+			expected: map[string]interface{}{
+				"connection": map[string]interface{}{
+					"host": "127.0.0.1",
+				},
+			},
+			key:   "servicebindingoperator.redhat.io/status.connection.host",
+			value: "binding:env:attribute",
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"status": map[string]interface{}{
+						"connection": map[string]interface{}{
+							"host": "127.0.0.1",
+							"port": "1234",
+						},
+					},
+				},
+			},
+		}),
+	)
+}