[RFC0030] Add Support for File based Service Binding Information

[beyhan]

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

Preview Give feedback

1. RFC-0030 - Add support to Diego for file based service bindings diego-release#942
   +0
2. RFC-0030 Add support to Diego for file based service bindings bbs#100
   +0
3. RFC-0030 - Add support to Diego for file based service bindings rep#56
   +0
4. RFC-0030 - Add support to Diego for file based service bindings executor#100
   +0
5. [RFC0030 - 1] Add column file_based_service_bindings_enabled to apps table cloud_controller_ng#3996
   +0
6. [RFC0030 - 2] File-based service bindings app feature cloud_controller_ng#3997
   +0
7. [RFC0030 - 3] File-based service bindings cloud_controller_ng#4026
   +0
8. Technical Specification: Translation of VCAP_SERVICES to Service Binding Files cloud_controller_ng#4036
   +0
9. Acceptance test for file based service bindings cf-acceptance-tests#1374
   +0

[dimitardimitrov13]

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

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

[beyhan]

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

[dimitardimitrov13]

@beyhan, thank you.

[ameowlia]

🎉 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]

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]

Hi @ameowlia,

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

[beyhan]

Should we wait for CAPI then?

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

[ameowlia]

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]

@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]

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]

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]

@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?