Error when creating service instances for disabled plan

fixes #3574

Co-authored-by: Danail Branekov <danailster@gmail.com>

api/main.go

@@ -201,6 +201,7 @@ func main() {
 		nsPermissions,
 		conditions.NewConditionAwaiter[*korifiv1alpha1.CFServiceInstance, korifiv1alpha1.CFServiceInstance, korifiv1alpha1.CFServiceInstanceList](conditionTimeout),
 		repositories.NewServiceInstanceSorter(),
+		cfg.RootNamespace,
 	)
 	serviceBindingRepo := repositories.NewServiceBindingRepo(
 		namespaceRetriever,

api/repositories/service_instance_repository.go

@@ -46,6 +46,7 @@ type ServiceInstanceRepo struct {
 	namespacePermissions *authorization.NamespacePermissions
 	awaiter              Awaiter[*korifiv1alpha1.CFServiceInstance]
 	sorter               ServiceInstanceSorter
+	rootNamespace        string
 }
 
 //counterfeiter:generate -o fake -fake-name ServiceInstanceSorter . ServiceInstanceSorter
@@ -93,13 +94,15 @@ func NewServiceInstanceRepo(
 	namespacePermissions *authorization.NamespacePermissions,
 	awaiter Awaiter[*korifiv1alpha1.CFServiceInstance],
 	sorter ServiceInstanceSorter,
+	rootNamespace string,
 ) *ServiceInstanceRepo {
 	return &ServiceInstanceRepo{
 		namespaceRetriever:   namespaceRetriever,
 		userClientFactory:    userClientFactory,
 		namespacePermissions: namespacePermissions,
 		awaiter:              awaiter,
 		sorter:               sorter,
+		rootNamespace:        rootNamespace,
 	}
 }
 
@@ -230,6 +233,15 @@ func (r *ServiceInstanceRepo) CreateManagedServiceInstance(ctx context.Context,
 		return ServiceInstanceRecord{}, fmt.Errorf("failed to build user client: %w", err)
 	}
 
+	planVisible, err := r.servicePlanVisible(ctx, userClient, message.PlanGUID, message.SpaceGUID)
+	if err != nil {
+		return ServiceInstanceRecord{}, apierrors.NewUnprocessableEntityError(err, "Invalid service plan. Ensure that the service plan exists, is available, and you have access to it.")
+	}
+
+	if !planVisible {
+		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)
@@ -260,6 +272,34 @@ func (r *ServiceInstanceRepo) CreateManagedServiceInstance(ctx context.Context,
 	return cfServiceInstanceToRecord(*cfServiceInstance), nil
 }
 
+func (r *ServiceInstanceRepo) servicePlanVisible(ctx context.Context, userClient client.Client, planGUID string, spaceGUID string) (bool, error) {
+	servicePlan := &korifiv1alpha1.CFServicePlan{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      planGUID,
+			Namespace: r.rootNamespace,
+		},
+	}
+	err := userClient.Get(ctx, client.ObjectKeyFromObject(servicePlan), servicePlan)
+	if err != nil {
+		return false, err
+	}
+
+	if servicePlan.Spec.Visibility.Type == korifiv1alpha1.PublicServicePlanVisibilityType {
+		return true, nil
+	}
+
+	if servicePlan.Spec.Visibility.Type == korifiv1alpha1.AdminServicePlanVisibilityType {
+		return false, nil
+	}
+
+	orgGUID, err := r.namespaceRetriever.NamespaceFor(ctx, spaceGUID, SpaceResourceType)
+	if err != nil {
+		return false, err
+	}
+
+	return slices.Contains(servicePlan.Spec.Visibility.Organizations, orgGUID), nil
+}
+
 func (r *ServiceInstanceRepo) PatchServiceInstance(ctx context.Context, authInfo authorization.Info, message PatchServiceInstanceMessage) (ServiceInstanceRecord, error) {
 	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {

api/repositories/service_instance_repository_test.go

@@ -58,7 +58,7 @@ var _ = Describe("ServiceInstanceRepository", func() {
 			return records
 		}
 
-		serviceInstanceRepo = repositories.NewServiceInstanceRepo(namespaceRetriever, userClientFactory, nsPerms, conditionAwaiter, sorter)
+		serviceInstanceRepo = repositories.NewServiceInstanceRepo(namespaceRetriever, userClientFactory, nsPerms, conditionAwaiter, sorter, rootNamespace)
 
 		org = createOrgWithCleanup(ctx, uuid.NewString())
 		space = createSpaceWithCleanup(ctx, org.Name, uuid.NewString())
@@ -154,16 +154,30 @@ var _ = Describe("ServiceInstanceRepository", func() {
 
 	Describe("CreateManagedServiceInstance", func() {
 		var (
+			servicePlan                  *korifiv1alpha1.CFServicePlan
 			serviceInstanceCreateMessage repositories.CreateManagedSIMessage
 			record                       repositories.ServiceInstanceRecord
 			createErr                    error
 		)
 
 		BeforeEach(func() {
+			servicePlan = &korifiv1alpha1.CFServicePlan{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: rootNamespace,
+					Name:      uuid.NewString(),
+				},
+				Spec: korifiv1alpha1.CFServicePlanSpec{
+					Visibility: korifiv1alpha1.ServicePlanVisibility{
+						Type: korifiv1alpha1.PublicServicePlanVisibilityType,
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, servicePlan)).To(Succeed())
+
 			serviceInstanceCreateMessage = repositories.CreateManagedSIMessage{
 				Name:      serviceInstanceName,
 				SpaceGUID: space.Name,
-				PlanGUID:  "plan-guid",
+				PlanGUID:  servicePlan.Name,
 				Tags:      []string{"foo", "bar"},
 				Parameters: map[string]any{
 					"p1": map[string]any{
@@ -196,7 +210,7 @@ var _ = Describe("ServiceInstanceRepository", func() {
 				Expect(record.Tags).To(ConsistOf([]string{"foo", "bar"}))
 				Expect(record.SecretName).To(BeEmpty())
 				Expect(record.Relationships()).To(Equal(map[string]string{
-					"service_plan": "plan-guid",
+					"service_plan": servicePlan.Name,
 					"space":        space.Name,
 				}))
 				Expect(record.CreatedAt).To(BeTemporally("~", time.Now(), timeCheckThreshold))
@@ -218,7 +232,7 @@ var _ = Describe("ServiceInstanceRepository", func() {
 				Expect(cfServiceInstance.Spec.SecretName).To(BeEmpty())
 				Expect(cfServiceInstance.Spec.Type).To(BeEquivalentTo(korifiv1alpha1.ManagedType))
 				Expect(cfServiceInstance.Spec.Tags).To(ConsistOf("foo", "bar"))
-				Expect(cfServiceInstance.Spec.PlanGUID).To(Equal("plan-guid"))
+				Expect(cfServiceInstance.Spec.PlanGUID).To(Equal(servicePlan.Name))
 				Expect(cfServiceInstance.Spec.Parameters).NotTo(BeNil())
 
 				actualParams := map[string]any{}
@@ -229,6 +243,66 @@ var _ = Describe("ServiceInstanceRepository", func() {
 					},
 				}))
 			})
+
+			When("the service plan visibility type is admin", func() {
+				BeforeEach(func() {
+					Expect(k8s.PatchResource(ctx, k8sClient, servicePlan, func() {
+						servicePlan.Spec.Visibility.Type = korifiv1alpha1.AdminServicePlanVisibilityType
+					})).To(Succeed())
+				})
+
+				It("returns unprocessable entity error", func() {
+					Expect(createErr).To(BeAssignableToTypeOf(apierrors.UnprocessableEntityError{}))
+				})
+			})
+
+			When("the service plan visibility type is organization", func() {
+				BeforeEach(func() {
+					Expect(k8s.PatchResource(ctx, k8sClient, servicePlan, func() {
+						servicePlan.Spec.Visibility.Type = korifiv1alpha1.OrganizationServicePlanVisibilityType
+					})).To(Succeed())
+				})
+
+				It("returns unprocessable entity error", func() {
+					Expect(createErr).To(BeAssignableToTypeOf(apierrors.UnprocessableEntityError{}))
+				})
+
+				When("the plan is enabled for the current organization", func() {
+					BeforeEach(func() {
+						Expect(k8s.PatchResource(ctx, k8sClient, servicePlan, func() {
+							servicePlan.Spec.Visibility.Organizations = append(servicePlan.Spec.Visibility.Organizations, org.Name)
+						})).To(Succeed())
+					})
+
+					It("succeeds", func() {
+						Expect(createErr).NotTo(HaveOccurred())
+					})
+
+					When("the space does not exist", func() {
+						BeforeEach(func() {
+							serviceInstanceCreateMessage.SpaceGUID = "does-not-exist"
+						})
+
+						It("returns unprocessable entity error", func() {
+							var unprocessableEntityErr apierrors.UnprocessableEntityError
+							Expect(errors.As(createErr, &unprocessableEntityErr)).To(BeTrue())
+							Expect(unprocessableEntityErr).To(MatchError(ContainSubstring("does-not-exist")))
+						})
+					})
+				})
+			})
+
+			When("the service plan does not exist", func() {
+				BeforeEach(func() {
+					serviceInstanceCreateMessage.PlanGUID = "does-not-exist"
+				})
+
+				It("returns unprocessable entity error", func() {
+					var unprocessableEntityErr apierrors.UnprocessableEntityError
+					Expect(errors.As(createErr, &unprocessableEntityErr)).To(BeTrue())
+					Expect(unprocessableEntityErr).To(MatchError(ContainSubstring("does-not-exist")))
+				})
+			})
 		})
 	})
 

tests/e2e/e2e_suite_test.go

@@ -21,6 +21,7 @@ import (
 
 	"code.cloudfoundry.org/go-loggregator/v8/rpc/loggregator_v2"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
+	"github.com/BooleanCat/go-functional/v2/it/itx"
 	"github.com/go-resty/resty/v2"
 	"github.com/google/uuid"
 	. "github.com/onsi/ginkgo/v2"
@@ -1183,6 +1184,25 @@ func createBroker(brokerURL string) string {
 		g.Expect(jobRespBody).To(ContainSubstring("COMPLETE"))
 	}).Should(Succeed())
 
+	plans := resourceList[resource]{}
+	listResp, err := adminClient.R().SetResult(&plans).Get("/v3/service_plans")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(listResp).To(HaveRestyStatusCode(http.StatusOK))
+
+	itx.FromSlice(plans.Resources).Filter(func(r resource) bool {
+		return r.Metadata.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] == brokerGUID
+	}).ForEach(func(plan resource) {
+		resp, err := adminClient.R().
+			SetBody(planVisibilityResource{
+				Type: "public",
+			}).
+			Post(fmt.Sprintf("/v3/service_plans/%s/visibility", plan.GUID))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(resp).To(SatisfyAll(
+			HaveRestyStatusCode(http.StatusOK),
+		))
+	})
+
 	return brokerGUID
 }
 

tests/e2e/service_plans_test.go

@@ -75,7 +75,7 @@ var _ = Describe("Service Plans", func() {
 			It("returns the service plan visibility", func() {
 				Expect(resp).To(HaveRestyStatusCode(http.StatusOK))
 				Expect(result).To(Equal(planVisibilityResource{
-					Type: korifiv1alpha1.AdminServicePlanVisibilityType,
+					Type: "public",
 				}))
 			})
 		})
@@ -86,7 +86,7 @@ var _ = Describe("Service Plans", func() {
 				resp, err = adminClient.R().
 					SetResult(&result).
 					SetBody(planVisibilityResource{
-						Type: "public",
+						Type: "admin",
 					}).
 					Post(fmt.Sprintf("/v3/service_plans/%s/visibility", planGUID))
 				Expect(err).NotTo(HaveOccurred())
@@ -96,7 +96,7 @@ var _ = Describe("Service Plans", func() {
 				Expect(resp).To(SatisfyAll(
 					HaveRestyStatusCode(http.StatusOK),
 					HaveRestyBody(MatchJSON(`{
-						"type": "public"
+						"type": "admin"
 					}`)),
 				))
 			})
@@ -108,7 +108,7 @@ var _ = Describe("Service Plans", func() {
 				resp, err = adminClient.R().
 					SetResult(&result).
 					SetBody(planVisibilityResource{
-						Type: "public",
+						Type: "admin",
 					}).
 					Patch(fmt.Sprintf("/v3/service_plans/%s/visibility", planGUID))
 				Expect(err).NotTo(HaveOccurred())
@@ -118,7 +118,7 @@ var _ = Describe("Service Plans", func() {
 				Expect(resp).To(SatisfyAll(
 					HaveRestyStatusCode(http.StatusOK),
 					HaveRestyBody(MatchJSON(`{
-						"type": "public"
+						"type": "admin"
 					}`)),
 				))
 			})

tests/smoke/bind_service_test.go

@@ -46,6 +46,7 @@ var _ = Describe("cf bind-service", func() {
 				helpers.GetInClusterURL(getAppGUID(sharedData.BrokerAppName)),
 			)).To(Exit(0))
 
+			Expect(helpers.Cf("enable-service-access", "sample-service", "-b", brokerName)).To(Exit(0))
 			session := helpers.Cf("create-service", "sample-service", "sample", serviceName, "-b", brokerName)
 			Expect(session).To(Exit(0))
 		})

tests/smoke/services_test.go

@@ -22,6 +22,7 @@ var _ = Describe("Services", func() {
 			"broker-password",
 			helpers.GetInClusterURL(getAppGUID(sharedData.BrokerAppName)),
 		)).To(Exit(0))
+		Expect(helpers.Cf("enable-service-access", "sample-service", "-b", brokerName)).To(Exit(0))
 	})
 
 	AfterEach(func() {