Error when creating service instances for disabled plan

fixes cloudfoundry#3574 Co-authored-by: Danail Branekov <danailster@gmail.com>
zabanov-lab · Oct 30, 2024 · 9c7a3d5 · 9c7a3d5
1 parent d0483c8
commit 9c7a3d5
Show file tree

Hide file tree

Showing 3 changed files with 119 additions and 4 deletions.
diff --git a/api/main.go b/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,

diff --git a/api/repositories/service_instance_repository.go b/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 == "public" {
+		return true, nil
+	}
+
+	if servicePlan.Spec.Visibility.Type == "admin" {
+		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 {

diff --git a/api/repositories/service_instance_repository_test.go b/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: "public",
+					},
+				},
+			}
+			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 = "admin"
+					})).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 = "organization"
+					})).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")))
+				})
+			})
 		})
 	})