Pass parameters to instances

- Parameters type in CFServiceInstanceSpec type is changed to LocalObjectReference
- Params secret is created in the service_instance_repository
- Parameters are passed to the provision request with the broker

fixes #2361

api/handlers/service_instance.go

@@ -133,7 +133,7 @@ func (h *ServiceInstance) create(r *http.Request) (*routing.Response, error) {
 		)
 	}
 
-	if payload.Type == "managed" {
+	if payload.Type == korifiv1alpha1.ManagedType {
 		return h.createManagedServiceInstance(r.Context(), logger, authInfo, payload)
 	}
 

api/repositories/service_instance_repository.go

@@ -2,7 +2,6 @@ package repositories
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"slices"
@@ -25,7 +24,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -237,11 +235,6 @@ func (r *ServiceInstanceRepo) CreateManagedServiceInstance(ctx context.Context,
 		return ServiceInstanceRecord{}, apierrors.NewUnprocessableEntityError(nil, "Invalid service plan. Ensure that the service plan exists, is available, and you have access to it.")
 	}
 
-	parameterBytes, err := json.Marshal(message.Parameters)
-	if err != nil {
-		return ServiceInstanceRecord{}, fmt.Errorf("failed to marshal parameters: %w", err)
-	}
-
 	cfServiceInstance := &korifiv1alpha1.CFServiceInstance{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        uuid.NewString(),
@@ -254,19 +247,44 @@ func (r *ServiceInstanceRepo) CreateManagedServiceInstance(ctx context.Context,
 			Type:        korifiv1alpha1.ManagedType,
 			PlanGUID:    message.PlanGUID,
 			Tags:        message.Tags,
-			Parameters: &runtime.RawExtension{
-				Raw: parameterBytes,
+			Parameters: corev1.LocalObjectReference{
+				Name: uuid.NewString(),
 			},
 		},
 	}
+
 	err = userClient.Create(ctx, cfServiceInstance)
 	if err != nil {
 		return ServiceInstanceRecord{}, apierrors.FromK8sError(err, ServiceInstanceResourceType)
 	}
 
+	err = r.createParametersSecret(ctx, userClient, cfServiceInstance, message.Parameters)
+	if err != nil {
+		return ServiceInstanceRecord{}, apierrors.FromK8sError(err, ServiceBindingResourceType)
+	}
+
 	return cfServiceInstanceToRecord(*cfServiceInstance), nil
 }
 
+func (r *ServiceInstanceRepo) createParametersSecret(ctx context.Context, userClient client.Client, cfServiceInstance *korifiv1alpha1.CFServiceInstance, parameters map[string]any) error {
+	parametersData, err := tools.ToParametersSecretData(parameters)
+	if err != nil {
+		return err
+	}
+
+	paramsSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: cfServiceInstance.Namespace,
+			Name:      cfServiceInstance.Spec.Parameters.Name,
+		},
+		Data: parametersData,
+	}
+
+	_ = controllerutil.SetOwnerReference(cfServiceInstance, paramsSecret, scheme.Scheme)
+
+	return userClient.Create(ctx, paramsSecret)
+}
+
 func (r *ServiceInstanceRepo) servicePlanVisible(ctx context.Context, userClient client.Client, planGUID string, spaceGUID string) (bool, error) {
 	servicePlan := &korifiv1alpha1.CFServicePlan{
 		ObjectMeta: metav1.ObjectMeta{

api/repositories/service_instance_repository_test.go

@@ -197,9 +197,7 @@ var _ = Describe("ServiceInstanceRepository", func() {
 				PlanGUID:  servicePlan.Name,
 				Tags:      []string{"foo", "bar"},
 				Parameters: map[string]any{
-					"p1": map[string]any{
-						"p11": "v11",
-					},
+					"p11": "v11",
 				},
 			}
 		})
@@ -249,15 +247,33 @@ var _ = Describe("ServiceInstanceRepository", func() {
 				Expect(cfServiceInstance.Spec.Type).To(BeEquivalentTo(korifiv1alpha1.ManagedType))
 				Expect(cfServiceInstance.Spec.Tags).To(ConsistOf("foo", "bar"))
 				Expect(cfServiceInstance.Spec.PlanGUID).To(Equal(servicePlan.Name))
-				Expect(cfServiceInstance.Spec.Parameters).NotTo(BeNil())
+				Expect(cfServiceInstance.Spec.Parameters.Name).NotTo(BeNil())
+			})
+
+			It("creates the parameters secret", func() {
+				serviceInstance := new(korifiv1alpha1.CFServiceInstance)
+				Expect(
+					k8sClient.Get(ctx, types.NamespacedName{Name: record.GUID, Namespace: space.Name}, serviceInstance),
+				).To(Succeed())
 
-				actualParams := map[string]any{}
-				Expect(json.Unmarshal(cfServiceInstance.Spec.Parameters.Raw, &actualParams)).To(Succeed())
-				Expect(actualParams).To(Equal(map[string]any{
-					"p1": map[string]any{
-						"p11": "v11",
+				paramsSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: serviceInstance.Namespace,
+						Name:      serviceInstance.Spec.Parameters.Name,
 					},
+				}
+				Expect(
+					k8sClient.Get(ctx, client.ObjectKeyFromObject(paramsSecret), paramsSecret),
+				).To(Succeed())
+
+				Expect(paramsSecret.Data).To(Equal(map[string][]byte{
+					tools.ParametersSecretKey: []byte(`{"p11":"v11"}`),
 				}))
+
+				Expect(paramsSecret.OwnerReferences).To(ConsistOf(MatchFields(IgnoreExtras, Fields{
+					"Kind": Equal("CFServiceInstance"),
+					"Name": Equal(serviceInstance.Name),
+				})))
 			})
 
 			When("the service plan visibility type is admin", func() {

controllers/api/v1alpha1/cfserviceinstance_types.go

@@ -22,7 +22,6 @@ import (
 	"code.cloudfoundry.org/korifi/model/services"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	runtime "k8s.io/apimachinery/pkg/runtime"
 )
 
 const (
@@ -57,7 +56,7 @@ type CFServiceInstanceSpec struct {
 
 	PlanGUID string `json:"plan_guid"`
 
-	Parameters *runtime.RawExtension `json:"parameters,omitempty"`
+	Parameters corev1.LocalObjectReference `json:"parameters,omitempty"`
 }
 
 // InstanceType defines the type of the Service Instance

controllers/controllers/services/bindings/controller_test.go

@@ -677,27 +677,6 @@ var _ = Describe("CFServiceBinding", func() {
 					}).Should(Succeed())
 				})
 			})
-
-			When("the parameters are invalid", func() {
-				BeforeEach(func() {
-					Expect(k8s.PatchResource(ctx, adminClient, paramsSecret, func() {
-						paramsSecret.Data = map[string][]byte{
-							tools.ParametersSecretKey: []byte("invalid-json"),
-						}
-					})).To(Succeed())
-				})
-
-				It("sets the ready condition to false", func() {
-					Eventually(func(g Gomega) {
-						g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed())
-						g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll(
-							HasType(Equal(korifiv1alpha1.StatusConditionReady)),
-							HasStatus(Equal(metav1.ConditionFalse)),
-							HasReason(Equal("InvalidParameters")),
-						)))
-					}).Should(Succeed())
-				})
-			})
 		})
 
 		It("does not check for binding last operation", func() {

controllers/controllers/services/instances/managed/controller.go

@@ -18,7 +18,6 @@ package managed
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"slices"
 	"time"
@@ -191,11 +190,10 @@ func (r *Reconciler) provisionServiceInstance(
 ) (osbapi.ProvisionResponse, error) {
 	log := logr.FromContextOrDiscard(ctx).WithName("provision-service-instance")
 
-	parametersMap, err := getServiceInstanceParameters(serviceInstance)
+	parametersMap, err := r.getServiceInstanceParameters(ctx, serviceInstance)
 	if err != nil {
 		log.Error(err, "failed to get service instance parameters")
-		return osbapi.ProvisionResponse{},
-			fmt.Errorf("failed to get service instance parameters: %w", err)
+		return osbapi.ProvisionResponse{}, k8s.NewNotReadyError().WithReason("InvalidParameters")
 	}
 
 	namespace, err := r.getNamespace(ctx, serviceInstance.Namespace)
@@ -386,18 +384,24 @@ func (r *Reconciler) pollLastOperation(
 	return lastOpResponse, nil
 }
 
-func getServiceInstanceParameters(serviceInstance *korifiv1alpha1.CFServiceInstance) (map[string]any, error) {
-	if serviceInstance.Spec.Parameters == nil {
+func (r *Reconciler) getServiceInstanceParameters(ctx context.Context, serviceInstance *korifiv1alpha1.CFServiceInstance) (map[string]any, error) {
+	if serviceInstance.Spec.Parameters.Name == "" {
 		return nil, nil
 	}
 
-	parametersMap := map[string]any{}
-	err := json.Unmarshal(serviceInstance.Spec.Parameters.Raw, &parametersMap)
+	paramsSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: serviceInstance.Namespace,
+			Name:      serviceInstance.Spec.Parameters.Name,
+		},
+	}
+
+	err := r.k8sClient.Get(ctx, client.ObjectKeyFromObject(paramsSecret), paramsSecret)
 	if err != nil {
-		return nil, fmt.Errorf("failed to unmarshal parameters: %w", err)
+		return nil, err
 	}
 
-	return parametersMap, nil
+	return tools.FromParametersSecretData(paramsSecret.Data)
 }
 
 func (r *Reconciler) isServicePlanVisible(

controllers/controllers/services/instances/managed/controller_test.go

@@ -1,7 +1,6 @@
 package managed_test
 
 import (
-	"encoding/json"
 	"errors"
 	"net/http"
 
@@ -23,7 +22,6 @@ import (
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 )
 
 var _ = Describe("CFServiceInstance", func() {
@@ -108,12 +106,6 @@ var _ = Describe("CFServiceInstance", func() {
 		}
 		Expect(adminClient.Create(ctx, servicePlan)).To(Succeed())
 
-		params := map[string]string{
-			"param-key": "param-value",
-		}
-		paramBytes, err := json.Marshal(params)
-		Expect(err).NotTo(HaveOccurred())
-
 		instance = &korifiv1alpha1.CFServiceInstance{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      uuid.NewString(),
@@ -126,9 +118,6 @@ var _ = Describe("CFServiceInstance", func() {
 				DisplayName: "service-instance-name",
 				Type:        korifiv1alpha1.ManagedType,
 				PlanGUID:    servicePlan.Name,
-				Parameters: &runtime.RawExtension{
-					Raw: paramBytes,
-				},
 			},
 		}
 
@@ -191,9 +180,6 @@ var _ = Describe("CFServiceInstance", func() {
 					PlanID:    "service-plan-id",
 					SpaceGUID: "space-guid",
 					OrgGUID:   "org-guid",
-					Parameters: map[string]any{
-						"param-key": "param-value",
-					},
 				},
 			}))
 		}).Should(Succeed())
@@ -213,7 +199,7 @@ var _ = Describe("CFServiceInstance", func() {
 	When("the service instance parameters are not set", func() {
 		BeforeEach(func() {
 			Expect(k8s.PatchResource(ctx, adminClient, instance, func() {
-				instance.Spec.Parameters = nil
+				instance.Spec.Parameters.Name = ""
 			})).To(Succeed())
 		})
 
@@ -283,6 +269,56 @@ var _ = Describe("CFServiceInstance", func() {
 		})
 	})
 
+	When("the instance has parameters", func() {
+		var paramsSecret *corev1.Secret
+
+		BeforeEach(func() {
+			paramsSecret = &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: instance.Namespace,
+					Name:      uuid.NewString(),
+				},
+				Data: map[string][]byte{
+					tools.ParametersSecretKey: []byte(`{"p1":"p1-value"}`),
+				},
+			}
+			Expect(adminClient.Create(ctx, paramsSecret)).To(Succeed())
+
+			Expect(k8s.Patch(ctx, adminClient, instance, func() {
+				instance.Spec.Parameters.Name = paramsSecret.Name
+			})).To(Succeed())
+		})
+
+		It("sends them to the broker on provision", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(brokerClient.ProvisionCallCount()).To(BeNumerically(">", 0))
+				_, payload := brokerClient.ProvisionArgsForCall(0)
+				g.Expect(payload.Parameters).To(Equal(map[string]any{
+					"p1": "p1-value",
+				}))
+			}).Should(Succeed())
+		})
+
+		When("the parameters secret does not exist", func() {
+			BeforeEach(func() {
+				Expect(k8s.PatchResource(ctx, adminClient, instance, func() {
+					instance.Spec.Parameters.Name = "not-valid"
+				})).To(Succeed())
+			})
+
+			It("sets the ready condition to false", func() {
+				Eventually(func(g Gomega) {
+					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed())
+					g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll(
+						HasType(Equal(korifiv1alpha1.StatusConditionReady)),
+						HasStatus(Equal(metav1.ConditionFalse)),
+						HasReason(Equal("InvalidParameters")),
+					)))
+				}).Should(Succeed())
+			})
+		})
+	})
+
 	When("the provisioning is asynchronous", func() {
 		BeforeEach(func() {
 			brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{
@@ -431,9 +467,6 @@ var _ = Describe("CFServiceInstance", func() {
 						PlanID:    "service-plan-id",
 						SpaceGUID: "space-guid",
 						OrgGUID:   "org-guid",
-						Parameters: map[string]any{
-							"param-key": "param-value",
-						},
 					},
 				}))
 			}).Should(Succeed())

helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfserviceinstances.yaml

@@ -51,8 +51,21 @@ spec:
                   Unlike metadata.name, the user can change this field
                 type: string
               parameters:
+                description: |-
+                  LocalObjectReference contains enough information to let you locate the
+                  referenced object inside the same namespace.
+                properties:
+                  name:
+                    default: ""
+                    description: |-
+                      Name of the referent.
+                      This field is effectively required, but due to backwards compatibility is
+                      allowed to be empty. Instances of this type with an empty value here are
+                      almost certainly wrong.
+                      More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                    type: string
                 type: object
-                x-kubernetes-preserve-unknown-fields: true
+                x-kubernetes-map-type: atomic
               plan_guid:
                 type: string
               secretName: