Credentials in VCAP_SERVICES are objects

rather than map[string]string

Issue: #2900

Co-authored-by: Danail Branekov <[email protected]>

api/Dockerfile

@@ -25,6 +25,7 @@ COPY controllers/api controllers/api
 COPY controllers/config controllers/config
 COPY controllers/controllers/shared controllers/controllers/shared
 COPY controllers/controllers/workloads controllers/controllers/workloads
+COPY controllers/controllers/services/credentials controllers/controllers/services/credentials
 COPY controllers/webhooks controllers/webhooks
 COPY tools tools
 COPY version version

api/repositories/app_repository_test.go

@@ -1597,7 +1597,7 @@ func generateVcapServiceSecretDataByte() (map[string][]byte, error) {
 		InstanceName: "myupsi",
 		BindingGUID:  "73f68d28-4602-47a3-8110-74ca991d5032",
 		BindingName:  nil,
-		Credentials: map[string]string{
+		Credentials: map[string]any{
 			"foo": "bar",
 		},
 		SyslogDrainURL: nil,

controllers/api/v1alpha1/cfservicebinding_types.go

@@ -37,11 +37,19 @@ type CFServiceBindingSpec struct {
 
 // CFServiceBindingStatus defines the observed state of CFServiceBinding
 type CFServiceBindingStatus struct {
-	// A reference to the Secret containing the credentials.
-	// This is required to conform to the Kubernetes Service Bindings spec
+	// A reference to the Secret containing the binding Credentials in
+	// servicebinding.io format. In order to conform to that spec the resource
+	// should have a "duck type" field called `binding`. From more info see the
+	// servicebinding.io [spec](https://servicebinding.io/spec/core/1.0.0-rc3/#Duck%20Type)
 	// +optional
 	Binding v1.LocalObjectReference `json:"binding"`
 
+	// A reference to the Secret containing the binding Credentials object. For
+	// bindings to user-provided services this refers to the credentials secret
+	// from the service instance
+	// +optional
+	Credentials v1.LocalObjectReference `json:"credentials"`
+
 	//+kubebuilder:validation:Optional
 	Conditions []metav1.Condition `json:"conditions,omitempty"`
 

controllers/controllers/services/cfservicebinding_controller.go

@@ -191,6 +191,8 @@ func (r *CFServiceBindingReconciler) ReconcileResource(ctx context.Context, cfSe
 }
 
 func (r *CFServiceBindingReconciler) reconcileCredentials(ctx context.Context, cfServiceInstance *korifiv1alpha1.CFServiceInstance, cfServiceBinding *korifiv1alpha1.CFServiceBinding) (*corev1.Secret, error) {
+	cfServiceBinding.Status.Credentials.Name = cfServiceInstance.Status.Credentials.Name
+
 	if isLegacyServiceBinding(cfServiceBinding, cfServiceInstance) {
 		bindingSecret := &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
@@ -232,7 +234,7 @@ func (r *CFServiceBindingReconciler) reconcileCredentials(ctx context.Context, c
 		if err != nil {
 			return err
 		}
-		bindingSecret.Data, err = credentials.GetBindingSecretData(credentialsSecret)
+		bindingSecret.Data, err = credentials.GetServiceBindingIOSecretData(credentialsSecret)
 		if err != nil {
 			return err
 		}

controllers/controllers/services/cfservicebinding_controller_test.go

@@ -153,8 +153,13 @@ var _ = Describe("CFServiceBinding", func() {
 			Eventually(func(g Gomega) {
 				g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfServiceBinding), cfServiceBinding)).To(Succeed())
 				g.Expect(meta.IsStatusConditionTrue(cfServiceBinding.Status.Conditions, services.BindingSecretAvailableCondition)).To(BeTrue())
+
+				g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(cfServiceInstance), cfServiceInstance)).To(Succeed())
+				g.Expect(cfServiceInstance.Status.Credentials.Name).NotTo(BeEmpty())
 			}).Should(Succeed())
 
+			Expect(cfServiceBinding.Status.Credentials.Name).To(Equal(cfServiceInstance.Status.Credentials.Name))
+
 			Expect(cfServiceBinding.Status.Binding.Name).NotTo(BeEmpty())
 
 			bindingSecret := &corev1.Secret{

controllers/controllers/services/credentials/credentials.go

@@ -11,7 +11,7 @@ import (
 const ServiceBindingSecretTypePrefix = "servicebinding.io/"
 
 func GetBindingSecretType(credentialsSecret *corev1.Secret) (corev1.SecretType, error) {
-	credentials, err := getCredentials(credentialsSecret)
+	credentials, err := GetCredentials(credentialsSecret)
 	if err != nil {
 		return "", err
 	}
@@ -24,8 +24,8 @@ func GetBindingSecretType(credentialsSecret *corev1.Secret) (corev1.SecretType,
 	return corev1.SecretType(ServiceBindingSecretTypePrefix + korifiv1alpha1.UserProvidedType), nil
 }
 
-func GetBindingSecretData(credentialsSecret *corev1.Secret) (map[string][]byte, error) {
-	credentials, err := getCredentials(credentialsSecret)
+func GetServiceBindingIOSecretData(credentialsSecret *corev1.Secret) (map[string][]byte, error) {
+	credentials, err := GetCredentials(credentialsSecret)
 	if err != nil {
 		return nil, err
 	}
@@ -49,7 +49,7 @@ func toBytes(value any) ([]byte, error) {
 	return json.Marshal(value)
 }
 
-func getCredentials(credentialsSecret *corev1.Secret) (map[string]any, error) {
+func GetCredentials(credentialsSecret *corev1.Secret) (map[string]any, error) {
 	credentials, ok := credentialsSecret.Data[korifiv1alpha1.CredentialsSecretKey]
 	if !ok {
 		return nil, fmt.Errorf(

controllers/controllers/services/credentials/credentials_test.go

@@ -25,38 +25,20 @@ var _ = Describe("Credentials", func() {
 		}
 	})
 
-	Describe("GetBindingSecretType", func() {
-		var secretType corev1.SecretType
+	Describe("GetCredentials", func() {
+		var creds map[string]any
 
 		JustBeforeEach(func() {
-			secretType, err = credentials.GetBindingSecretType(credentialsSecret)
+			creds, err = credentials.GetCredentials(credentialsSecret)
 		})
 
-		It("returns user-provided type by default", func() {
+		It("returns the credentials object", func() {
 			Expect(err).NotTo(HaveOccurred())
-			Expect(secretType).To(BeEquivalentTo(credentials.ServiceBindingSecretTypePrefix + "user-provided"))
-		})
-
-		When("the type is specified in the credentials", func() {
-			BeforeEach(func() {
-				credentialsSecret.Data[korifiv1alpha1.CredentialsSecretKey] = []byte(`{"type":"my-type"}`)
-			})
-
-			It("returns the speicified type", func() {
-				Expect(err).NotTo(HaveOccurred())
-				Expect(secretType).To(BeEquivalentTo(credentials.ServiceBindingSecretTypePrefix + "my-type"))
-			})
-		})
-
-		When("the type is not a plain string", func() {
-			BeforeEach(func() {
-				credentialsSecret.Data[korifiv1alpha1.CredentialsSecretKey] = []byte(`{"type":{"my-type":"is-not-a-string"}}`)
-			})
-
-			It("returns user-provided type", func() {
-				Expect(err).NotTo(HaveOccurred())
-				Expect(secretType).To(BeEquivalentTo(credentials.ServiceBindingSecretTypePrefix + "user-provided"))
-			})
+			Expect(creds).To(MatchAllKeys(Keys{
+				"foo": MatchAllKeys(Keys{
+					"bar": Equal("baz"),
+				}),
+			}))
 		})
 
 		When("the credentials cannot be unmarshalled", func() {
@@ -84,42 +66,53 @@ var _ = Describe("Credentials", func() {
 		})
 	})
 
-	Describe("GetBindingSecretData", func() {
-		var bindingSecretData map[string][]byte
+	Describe("GetBindingSecretType", func() {
+		var secretType corev1.SecretType
 
 		JustBeforeEach(func() {
-			bindingSecretData, err = credentials.GetBindingSecretData(credentialsSecret)
+			secretType, err = credentials.GetBindingSecretType(credentialsSecret)
 		})
 
-		It("converts the credentials into a flat strings map", func() {
+		It("returns user-provided type by default", func() {
 			Expect(err).NotTo(HaveOccurred())
-			Expect(bindingSecretData).To(MatchAllKeys(Keys{
-				"foo": Equal([]byte(`{"bar":"baz"}`)),
-			}))
+			Expect(secretType).To(BeEquivalentTo(credentials.ServiceBindingSecretTypePrefix + "user-provided"))
 		})
 
-		When("the credentials key is missing from the secret data", func() {
+		When("the type is specified in the credentials", func() {
 			BeforeEach(func() {
-				credentialsSecret.Data = map[string][]byte{
-					"foo": {},
-				}
+				credentialsSecret.Data[korifiv1alpha1.CredentialsSecretKey] = []byte(`{"type":"my-type"}`)
 			})
 
-			It("returns an error", func() {
-				Expect(err).To(MatchError(ContainSubstring(
-					fmt.Sprintf("does not contain the %q key", korifiv1alpha1.CredentialsSecretKey),
-				)))
+			It("returns the speicified type", func() {
+				Expect(err).NotTo(HaveOccurred())
+				Expect(secretType).To(BeEquivalentTo(credentials.ServiceBindingSecretTypePrefix + "my-type"))
 			})
 		})
 
-		When("the credentials cannot be unmarshalled", func() {
+		When("the type is not a plain string", func() {
 			BeforeEach(func() {
-				credentialsSecret.Data[korifiv1alpha1.CredentialsSecretKey] = []byte("invalid")
+				credentialsSecret.Data[korifiv1alpha1.CredentialsSecretKey] = []byte(`{"type":{"my-type":"is-not-a-string"}}`)
 			})
 
-			It("returns an error", func() {
-				Expect(err).To(MatchError(ContainSubstring("failed to unmarshal secret data")))
+			It("returns user-provided type", func() {
+				Expect(err).NotTo(HaveOccurred())
+				Expect(secretType).To(BeEquivalentTo(credentials.ServiceBindingSecretTypePrefix + "user-provided"))
 			})
 		})
 	})
+
+	Describe("GetServiceBindingIOSecretData", func() {
+		var bindingSecretData map[string][]byte
+
+		JustBeforeEach(func() {
+			bindingSecretData, err = credentials.GetServiceBindingIOSecretData(credentialsSecret)
+		})
+
+		It("converts the credentials into a flat strings map", func() {
+			Expect(err).NotTo(HaveOccurred())
+			Expect(bindingSecretData).To(MatchAllKeys(Keys{
+				"foo": Equal([]byte(`{"bar":"baz"}`)),
+			}))
+		})
+	})
 })

controllers/controllers/workloads/env/builder.go

@@ -14,16 +14,16 @@ import (
 type VCAPServices map[string][]ServiceDetails
 
 type ServiceDetails struct {
-	Label          string            `json:"label"`
-	Name           string            `json:"name"`
-	Tags           []string          `json:"tags"`
-	InstanceGUID   string            `json:"instance_guid"`
-	InstanceName   string            `json:"instance_name"`
-	BindingGUID    string            `json:"binding_guid"`
-	BindingName    *string           `json:"binding_name"`
-	Credentials    map[string]string `json:"credentials"`
-	SyslogDrainURL *string           `json:"syslog_drain_url"`
-	VolumeMounts   []string          `json:"volume_mounts"`
+	Label          string         `json:"label"`
+	Name           string         `json:"name"`
+	Tags           []string       `json:"tags"`
+	InstanceGUID   string         `json:"instance_guid"`
+	InstanceName   string         `json:"instance_name"`
+	BindingGUID    string         `json:"binding_guid"`
+	BindingName    *string        `json:"binding_name"`
+	Credentials    map[string]any `json:"credentials"`
+	SyslogDrainURL *string        `json:"syslog_drain_url"`
+	VolumeMounts   []string       `json:"volume_mounts"`
 }
 
 type WorkloadEnvBuilder struct {

controllers/controllers/workloads/env/vcap_services_builder.go

@@ -6,9 +6,11 @@ import (
 	"fmt"
 
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
+	"code.cloudfoundry.org/korifi/controllers/controllers/services/credentials"
 	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -65,8 +67,8 @@ func (b *VCAPServicesEnvValueBuilder) BuildEnvValue(ctx context.Context, cfApp *
 }
 
 func buildSingleServiceEnv(ctx context.Context, k8sClient client.Client, serviceBinding korifiv1alpha1.CFServiceBinding) (ServiceDetails, string, error) {
-	if serviceBinding.Status.Binding.Name == "" {
-		return ServiceDetails{}, "", fmt.Errorf("secret name not set for service binding %q", serviceBinding.Name)
+	if serviceBinding.Status.Credentials.Name == "" {
+		return ServiceDetails{}, "", fmt.Errorf("credentials secret name not set for service binding %q", serviceBinding.Name)
 	}
 
 	serviceLabel := UserProvided
@@ -77,8 +79,13 @@ func buildSingleServiceEnv(ctx context.Context, k8sClient client.Client, service
 		return ServiceDetails{}, "", fmt.Errorf("error fetching CFServiceInstance: %w", err)
 	}
 
-	secret := corev1.Secret{}
-	err = k8sClient.Get(ctx, types.NamespacedName{Namespace: serviceBinding.Namespace, Name: serviceBinding.Status.Binding.Name}, &secret)
+	credentialsSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: serviceBinding.Namespace,
+			Name:      serviceBinding.Status.Credentials.Name,
+		},
+	}
+	err = k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)
 	if err != nil {
 		return ServiceDetails{}, "", fmt.Errorf("error fetching CFServiceBinding Secret: %w", err)
 	}
@@ -87,15 +94,20 @@ func buildSingleServiceEnv(ctx context.Context, k8sClient client.Client, service
 		serviceLabel = *serviceInstance.Spec.ServiceLabel
 	}
 
-	return fromServiceBinding(serviceBinding, serviceInstance, secret, serviceLabel), serviceLabel, nil
+	serviceDetails, err := fromServiceBinding(serviceBinding, serviceInstance, credentialsSecret, serviceLabel)
+	if err != nil {
+		return ServiceDetails{}, "", fmt.Errorf("error fetching CFServiceBinding details: %w", err)
+	}
+
+	return serviceDetails, serviceLabel, nil
 }
 
 func fromServiceBinding(
 	serviceBinding korifiv1alpha1.CFServiceBinding,
 	serviceInstance korifiv1alpha1.CFServiceInstance,
-	serviceBindingSecret corev1.Secret,
+	credentialsSecret *corev1.Secret,
 	serviceLabel string,
-) ServiceDetails {
+) (ServiceDetails, error) {
 	var serviceName string
 	var bindingName *string
 
@@ -112,6 +124,11 @@ func fromServiceBinding(
 		tags = []string{}
 	}
 
+	credentials, err := credentials.GetCredentials(credentialsSecret)
+	if err != nil {
+		return ServiceDetails{}, fmt.Errorf("failed to get credentials for service binding %q: %w", serviceBinding.Name, err)
+	}
+
 	return ServiceDetails{
 		Label:          serviceLabel,
 		Name:           serviceName,
@@ -120,16 +137,8 @@ func fromServiceBinding(
 		InstanceName:   serviceInstance.Spec.DisplayName,
 		BindingGUID:    serviceBinding.Name,
 		BindingName:    bindingName,
-		Credentials:    mapFromSecret(serviceBindingSecret),
+		Credentials:    credentials,
 		SyslogDrainURL: nil,
 		VolumeMounts:   []string{},
-	}
-}
-
-func mapFromSecret(secret corev1.Secret) map[string]string {
-	convertedMap := make(map[string]string)
-	for k, v := range secret.Data {
-		convertedMap[k] = string(v)
-	}
-	return convertedMap
+	}, nil
 }