[RFC0030] Add Support for File based Service Binding Information #901

beyhan · 2024-06-18T12:17:49Z

This is a tracking issue for the implementation of https://github.com/cloudfoundry/community/blob/main/toc/rfc/rfc-0030-add-support-for-file-based-service-binding.md

Tasks

Give feedback

dimitardimitrov13 · 2024-07-25T11:44:17Z

Hello there, here are the 4 PRs within diego-release, bbs, rep, and executor. They are needed for RFC-0030.

Diego Release: RFC-0030 - Add support to Diego for file based service bindings diego-release#942
BBS: RFC-0030 Add support to Diego for file based service bindings bbs#100
REP: RFC-0030 - Add support to Diego for file based service bindings rep#56
Executor: RFC-0030 - Add support to Diego for file based service bindings executor#100

beyhan · 2024-07-25T12:02:15Z

@dimitardimitrov13 thanks for the update! I updated the tasks list.

dimitardimitrov13 · 2024-07-25T12:33:27Z

@beyhan, thank you.

ameowlia · 2024-08-06T15:06:17Z

🎉 Hi @dimitardimitrov13 , Excellent work so far!

Please ping me on slack (too easy to miss github notifications with 100+ repos 🫠 ) when the CAPI PRs are done and we can test this end-to-end. At that point I will assign a Diego reviewer.

dimitardimitrov13 · 2024-08-09T07:48:01Z

Hello Amelia,

Thanks for your message. Unfortunately, we do not have a straightforward way to test this end-to-end. During the implementation stage, I used a custom build of cfdot and a mocked code inside the BBS to simulate this. The code was cleaned up from the BBS before the PR. It seems we will need to wait for the CAPI team.

Best regards,
Dimitar

PlamenDoychev · 2024-08-13T07:28:50Z

Hi @ameowlia,

Just in case you haven't seen the input from @dimitardimitrov13 above.
Should we wait for CAPI then?

beyhan · 2024-09-03T15:37:18Z

Should we wait for CAPI then?

This is what I read in the comment above #901 (comment)

ameowlia · 2024-09-05T14:28:05Z

Yes, I want to wait to review and merge until we can test it end-to-end. That way we can make sure everything works all the way through and we don't have to worry about breaking changes if part of it is released, but then it ends up things need to change to work with CAPI.

philippthun · 2024-10-16T12:19:30Z

@beyhan, @dimitardimitrov13: As I've started to implement the CAPI part of this, I came across the changed BBS interface. I suggest that we adapt Files to File as this entity represents a single file. This would be inline with e.g. EnvironmentVariable.

When already changing the BBS interface, I would also like to discuss, if we can adapt the attributes. Currently they are named name and value. I would find path and content more intuitive. path due to the fact that we always specify files within a parent folder named after the binding, so the value would be something like my-binding/instance_name. Maybe relative_path would be even better...

philippthun · 2024-10-16T14:25:02Z

And another topic to discuss is the usage of non-Ascii characters. Today you can do the following:

cf create-user-provided-service 💥 -p '{"💣":"🔥"}'

The env var looks then as follows:

VCAP_SERVICES: {
  "user-provided": [
    {
      "binding_guid": "1389756a-016a-4dac-80c6-e0a71b80c8e3",
      "binding_name": null,
      "credentials": {
        "💣": "🔥"
      },
      "instance_guid": "ed10c46a-3d75-4832-b53b-294ac7cfae09",
      "instance_name": "💥",
      "label": "user-provided",
      "name": "💥",
      "syslog_drain_url": null,
      "tags": [],
      "volume_mounts": []
    }
  ]
}

With file-based service bindings this would result in a file $SERVICE_BINDING_ROOT/💥/💣. Besides contradicting the Service Binding for Kubernetes spec (Binding names MUST match [a-z0-9-.]{1,253} / The name of a binding entry file name SHOULD match [a-z0-9-.]{1,253}), I'm wondering if this would work. What are the specs for directory and file names in the tmpfs file system?

philippthun · 2024-10-16T15:04:39Z

There's also a possible name conflict. The RFC states that the directory should be named after the service binding. I guess that this refers to the name attribute, which is either binding_name or (if the first one is not set) instance_name. Whereas it is not possible to create two service instances with the same name or two service bindings with the same name, it is possible to create a first service binding with the name foobar from an arbitrary service instance and another service binding without a name from a service instance called foobar. This would mean that both binding entries end up with name=foobar. When mapping this to file-based service bindings, there would be a conflict...

beyhan · 2024-10-17T08:02:40Z

@beyhan, @dimitardimitrov13: As I've started to implement the CAPI part of this, I came across the changed BBS interface. I suggest that we adapt Files to File as this entity represents a single file. This would be inline with e.g. EnvironmentVariable.

When already changing the BBS interface, I would also like to discuss, if we can adapt the attributes. Currently they are named name and value. I would find path and content more intuitive. path due to the fact that we always specify files within a parent folder named after the binding, so the value would be something like my-binding/instance_name. Maybe relative_path would be even better...

Referring to a discussion (#994 (comment)) in an RFC willing to build on top of this I would say that we should go for the suggested name there change ServiceBindingFiles to VolumeMountedFiles and Files to File. The suggestion to change name to path and value to content sounds reasonable to me. If everyone is OK with this I will update the RFC. @ameowlia , @ChrisMcGowan , @rkoster , @stephanme, @Gerg, @PlamenDoychev, @dimitardimitrov13 any thoughts on this?

dimitardimitrov13 · 2024-10-17T08:16:30Z

Hello,

I agree that good naming conventions should always be chosen. Since we have already changed it once (as requested by Stefan Merker), it could be done again. Regarding the other comments, I'll take a look next week and get back to you with answers.

Best regards,
Dimitar

beyhan · 2024-10-17T08:26:24Z

And another topic to discuss is the usage of non-Ascii characters. Today you can do the following:
cf create-user-provided-service 💥 -p '{"💣":"🔥"}'
The env var looks then as follows:
VCAP_SERVICES: {
  "user-provided": [
    {
      "binding_guid": "1389756a-016a-4dac-80c6-e0a71b80c8e3",
      "binding_name": null,
      "credentials": {
        "💣": "🔥"
      },
      "instance_guid": "ed10c46a-3d75-4832-b53b-294ac7cfae09",
      "instance_name": "💥",
      "label": "user-provided",
      "name": "💥",
      "syslog_drain_url": null,
      "tags": [],
      "volume_mounts": []
    }
  ]
}
With file-based service bindings this would result in a file $SERVICE_BINDING_ROOT/💥/💣. Besides contradicting the Service Binding for Kubernetes spec (Binding names MUST match [a-z0-9-.]{1,253} / The name of a binding entry file name SHOULD match [a-z0-9-.]{1,253}), I'm wondering if this would work. What are the specs for directory and file names in the tmpfs file system?

The RFC defines that we should support the K8s service binding spec naming conventions in

community/toc/rfc/rfc-0030-add-support-for-file-based-service-binding.md

Line 36 in 411475b

    
           2. Implement the K8s service binding specification. The environment variable `SERVICE_BINDING_ROOT` defines the location for the service bindings. The name of the file and the format follow the [K8s specification](https://servicebinding.io/):

. I would suggest that the CC validates this and returns an error if it is not the case. This will be a breaking change for users' with naming which is not supported by the K8s spec but the users must anyway opt-in here and in this case it should be fine. I couldn't find any specific rules for tmpfs file system but most probably it supports the same as Linux OS and according to the rules listed in https://en.wikipedia.org/wiki/Filename#Comparison_of_filename_limitations K8s naming specification should be fine. Any thoughts on this @ameowlia , @ChrisMcGowan , @rkoster , @stephanme, @Gerg?

beyhan · 2024-10-17T08:36:24Z

There's also a possible name conflict. The RFC states that the directory should be named after the service binding. I guess that this refers to the name attribute, which is either binding_name or (if the first one is not set) instance_name. Whereas it is not possible to create two service instances with the same name or two service bindings with the same name, it is possible to create a first service binding with the name foobar from an arbitrary service instance and another service binding without a name from a service instance called foobar. This would mean that both binding entries end up with name=foobar. When mapping this to file-based service bindings, there would be a conflict...

In the K8s spec is stated that:

Users SHOULD ensure each binding has a unique name. The behavior for name collisions is undefined. Implementations MAY attempt a good faith check for collisions to provide a meaningful error message.

I think we should make sure that this conflict results in an error like with the name in the previous comment. It also sounds as a missing validation in the current behaviour of CC. @philippthun Do we have any other alternative?

philippthun · 2024-10-18T12:18:48Z

And another topic to discuss is the usage of non-Ascii characters. [...]

I would suggest that the CC validates this and returns an error if it is not the case. [...]

Okay, I've implemented the MUST requirement for service binding names. But what about the SHOULD requirement for binding entry file names?

We would already violate this by using the defined VCAP_SERVICES attributes as file names, e.g. binding_guid, as the underscore is not allowed by the k8s spec. Should we:

translate underscores to dashes (e.g. binding_guid to binding-guid) and then strictly enforce the k8s spec?
add the underscore to the list of allowed characters and enforce this slightly modified spec?
simply ignore this SHOULD requirement...

I would prefer one of the first two options.

philippthun · 2024-10-18T12:21:59Z

There's also a possible name conflict. [...]

I think we should make sure that this conflict results in an error [...]

Agree - I've implemented this check.

beyhan · 2024-10-22T15:23:02Z

And another topic to discuss is the usage of non-Ascii characters. [...]

I would suggest that the CC validates this and returns an error if it is not the case. [...]

Okay, I've implemented the MUST requirement for service binding names. But what about the SHOULD requirement for binding entry file names?

We would already violate this by using the defined VCAP_SERVICES attributes as file names, e.g. binding_guid, as the underscore is not allowed by the k8s spec. Should we:

translate underscores to dashes (e.g. binding_guid to binding-guid) and then strictly enforce the k8s spec?

add the underscore to the list of allowed characters and enforce this slightly modified spec?

simply ignore this SHOULD requirement...

I would prefer one of the first two options.

Hi @philippthun,

I would say that we should go with option 1. because we aim to reuse existing tooling from the CNCF community here and it could be that they fail if we aren't conform with the K8s spe.

@ameowlia , @ChrisMcGowan , @rkoster , @stephanme, @Gerg any opinion on this?

beyhan · 2024-10-24T07:57:33Z

I opened #1008 to address all this discussions.

ameowlia · 2024-12-11T15:42:13Z

👋 Hi @philippthun, I am in the middle of doing acceptance for the diego PRs for file-based service binding info.

❓ Are there docs on how to enable file-based service binding info for apps?

I looked through...

The description in the PRs for diego and capi
The RFC
This technical specification doc

...and I can't seem to find it. Is there a new CF CLI or a CAPI API command I should be using?

beyhan · 2024-12-11T18:13:20Z

Hi @ameowlia,

in the RFC it is specified in

community/toc/rfc/rfc-0030-add-support-for-file-based-service-binding.md

Lines 50 to 51 in 55a5ff9

    
           Cloud Controller should introduce a new app feature for activation of the file-based approach. This means that the [App Features API](https://v3-apidocs.cloudfoundry.org/version/3.159.0/index.html#app-features) could be used here and a new feature flag called “file-based-service-bindings" should be introduced. 
        
           The [contract](https://github.com/cloudfoundry/bbs/blob/main/doc/actions.md) between Cloud Controller and Diego should be extended so that file name and file content for the application container can be specified. E.g. the run action could look like this when file approach is selected:

and possible manifest implementation in

community/toc/rfc/rfc-0030-add-support-for-file-based-service-binding.md

Lines 107 to 113 in 55a5ff9

    
           ``` 
        
           --- 
        
           applications: 
        
           - name: test-app 
        
             features: 
        
             - file-based-service-bindings: true 
        
           ```

.

Looking into the implementation in cloudfoundry/cloud_controller_ng#3997 the feature flag has the name as defined in the RFC file-based-service-bindings and it should be possible to activate it via https://v3-apidocs.cloudfoundry.org/version/3.182.0/index.html#update-an-app-feature for a given applications.

philippthun · 2024-12-12T11:09:05Z

@ameowlia - The release-candidate version of the docs has some info about this flag: https://v3-apidocs.cloudfoundry.org/version/release-candidate/#supported-app-features - actually only the name is interesting... As Beyhan already wrote, you have to enable it via the API.

Maybe you find this repository helpful - it contains test apps (and test descriptions) that I used for end-to-end validation of this feature. Our plan is to integrate this into CATS.

ameowlia · 2024-12-12T22:05:16Z

Thanks for those links @philippthun they really help! I was able to black box test the basic functionality today and everything worked. I will continue reviewing tomorrow.

beyhan added the rfc CFF community RFC label Jun 18, 2024

cf-foundation-community-automation bot added this to CF Community Jun 18, 2024

cf-foundation-community-automation bot moved this to Inbox in CF Community Jun 18, 2024

beyhan mentioned this issue Oct 31, 2024

[RFC0030] Updates #1008

Merged

jochenehret mentioned this issue Dec 12, 2024

Acceptance test for file based service bindings cloudfoundry/cf-acceptance-tests#1374

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC0030] Add Support for File based Service Binding Information #901

[RFC0030] Add Support for File based Service Binding Information #901

beyhan commented Jun 18, 2024 •

edited by jochenehret

Loading

Tasks

dimitardimitrov13 commented Jul 25, 2024

beyhan commented Jul 25, 2024 •

edited

Loading

dimitardimitrov13 commented Jul 25, 2024

ameowlia commented Aug 6, 2024

dimitardimitrov13 commented Aug 9, 2024

PlamenDoychev commented Aug 13, 2024

beyhan commented Sep 3, 2024

ameowlia commented Sep 5, 2024 •

edited

Loading

philippthun commented Oct 16, 2024

philippthun commented Oct 16, 2024

philippthun commented Oct 16, 2024

beyhan commented Oct 17, 2024 •

edited

Loading

dimitardimitrov13 commented Oct 17, 2024

beyhan commented Oct 17, 2024

beyhan commented Oct 17, 2024 •

edited

Loading

philippthun commented Oct 18, 2024

philippthun commented Oct 18, 2024

beyhan commented Oct 22, 2024

beyhan commented Oct 24, 2024

ameowlia commented Dec 11, 2024 •

edited

Loading

beyhan commented Dec 11, 2024

philippthun commented Dec 12, 2024

ameowlia commented Dec 12, 2024 •

edited

Loading

[RFC0030] Add Support for File based Service Binding Information #901

[RFC0030] Add Support for File based Service Binding Information #901

Comments

beyhan commented Jun 18, 2024 • edited by jochenehret Loading

Tasks

dimitardimitrov13 commented Jul 25, 2024

beyhan commented Jul 25, 2024 • edited Loading

dimitardimitrov13 commented Jul 25, 2024

ameowlia commented Aug 6, 2024

dimitardimitrov13 commented Aug 9, 2024

PlamenDoychev commented Aug 13, 2024

beyhan commented Sep 3, 2024

ameowlia commented Sep 5, 2024 • edited Loading

philippthun commented Oct 16, 2024

philippthun commented Oct 16, 2024

philippthun commented Oct 16, 2024

beyhan commented Oct 17, 2024 • edited Loading

dimitardimitrov13 commented Oct 17, 2024

beyhan commented Oct 17, 2024

beyhan commented Oct 17, 2024 • edited Loading

philippthun commented Oct 18, 2024

philippthun commented Oct 18, 2024

beyhan commented Oct 22, 2024

beyhan commented Oct 24, 2024

ameowlia commented Dec 11, 2024 • edited Loading

beyhan commented Dec 11, 2024

philippthun commented Dec 12, 2024

ameowlia commented Dec 12, 2024 • edited Loading

beyhan commented Jun 18, 2024 •

edited by jochenehret

Loading

beyhan commented Jul 25, 2024 •

edited

Loading

ameowlia commented Sep 5, 2024 •

edited

Loading

beyhan commented Oct 17, 2024 •

edited

Loading

beyhan commented Oct 17, 2024 •

edited

Loading

ameowlia commented Dec 11, 2024 •

edited

Loading

ameowlia commented Dec 12, 2024 •

edited

Loading