Add Service Credential Bindings of type Key #3635
Conversation
* Added Service Credential Bindings of type Key * Added type label to CFServiceBinding CRD * Tweaked list for bindings so you can filter by type --------- Co-authored-by: Dimitar Draganov <[email protected]>
|
api/handlers/service_binding.go
Outdated
| serviceInstance, err := h.serviceInstanceRepo.GetServiceInstance(r.Context(), authInfo, payload.Relationships.ServiceInstance.Data.GUID) | ||
| if err != nil { | ||
| return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "failed to get "+repositories.ServiceInstanceResourceType) | ||
| } | ||
|
|
||
| if app.SpaceGUID != serviceInstance.SpaceGUID { | ||
| if payload.Type == korifiv1alpha1.CFServiceBindingTypeKey && serviceInstance.Type != korifiv1alpha1.ManagedType { |
There was a problem hiding this comment.
This check should be removed as we do it in the createUserProvided method.
| BeforeEach(func() { | ||
| createRoleBinding(ctx, userName, spaceDeveloperRole.Name, space.Name) | ||
| createMsg = repositories.CreateServiceBindingMessage{ | ||
| Type: korifiv1alpha1.CFServiceBindingTypeApp, |
There was a problem hiding this comment.
This type should be "key"?
| }, | ||
| AppRef: corev1.LocalObjectReference{ | ||
| Name: appGUID, | ||
| It("fails the validation", func() { |
There was a problem hiding this comment.
Missing error assertion?
| Describe("type app", func() { | ||
|
|
||
| When("the user is not allowed to create CFServiceBindings", func() { | ||
| BeforeEach(func() { |
There was a problem hiding this comment.
This BeforeEach should be moved up to the Describe("type app")
| ).To(Succeed()) | ||
|
|
||
| Expect(serviceBinding.Labels).To(HaveKeyWithValue("servicebinding.io/provisioned-service", "true")) | ||
| Expect(serviceBinding.Spec.Type).To(Equal(korifiv1alpha1.CFServiceBindingTypeKey)) |
There was a problem hiding this comment.
This line should assert that type is "app"
| "Name": Equal(serviceBinding4.Spec.DisplayName), | ||
| "ServiceInstanceGUID": Equal(serviceBinding4.Spec.Service.Name), | ||
| "SpaceGUID": Equal(serviceBinding4.Namespace), | ||
| }), | ||
| )) | ||
| }) | ||
| }) | ||
|
|
||
| When("filtered by service instance GUID", func() { |
There was a problem hiding this comment.
Filtering by instance GUID is orhogonal to filtering by type, so this test should not be changed.
| @@ -840,6 +954,9 @@ var _ = Describe("ServiceBindingRepo", func() { | |||
| Expect(k8s.PatchResource(ctx, k8sClient, serviceBinding3, func() { | |||
| serviceBinding3.Labels = map[string]string{"not_foo": "NOT_FOO"} | |||
| })).To(Succeed()) | |||
| Expect(k8s.PatchResource(ctx, k8sClient, serviceBinding4, func() { | |||
There was a problem hiding this comment.
Listing service bindings by labels is orthogonal to type "key" so this test should not be modified
| @@ -196,11 +197,11 @@ var _ = Describe("CFServiceBinding", func() { | |||
| g.Expect(sbServiceBinding.Spec.Name).To(Equal(binding.Name)) | |||
| g.Expect(sbServiceBinding.Spec.Type).To(Equal("user-provided")) | |||
| g.Expect(sbServiceBinding.Spec.Provider).To(BeEmpty()) | |||
| g.Expect(sbServiceBinding.Spec.Type).To(Equal(korifiv1alpha1.CFServiceBindingTypeApp)) | |||
There was a problem hiding this comment.
This line should go. The sbServiceBinding does not have types "app" and "key" as it is a different spec. It can be of type "user-provided" or "managed" and this is already checked on line 198
|
|
||
| g.Expect(sbServiceBinding.Labels).To(SatisfyAll( | ||
| HaveKeyWithValue(bindings.ServiceBindingGUIDLabel, binding.Name), | ||
| HaveKeyWithValue(korifiv1alpha1.CFAppGUIDLabelKey, cfAppGUID), | ||
| HaveKeyWithValue(bindings.ServiceCredentialBindingTypeLabel, "app"), |
There was a problem hiding this comment.
This line shoul stay as it checks the labels of the service binding io bindings
Is there a related GitHub Issue?
#2320
What is this change about?
With this PR we are introducing a new type of service binding - 'key'.
Does this PR introduce a breaking change?
No
Acceptance Steps
Tag your pair, your PM, and/or team