Skip to content

Conversation

tomkennedy513
Copy link
Contributor

Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:

  • A short explanation of the proposed change:

The ServiceBinding spec has been updated to explicitly mention that underscores are valid in credential keys (https://github.com/servicebinding/spec/blob/main/README.md#workload-projection). Updating the implementation to match the change in teh spec

  • An explanation of the use cases your change solves

Many service bindings contain underscores in the credential keys. Previously these would return an error even though the spec only stated that credential keys should match the previous regex. The spec was updated to better clarify what should be allowed in credential keys

  • Links to any other associated PRs

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

- The ServiceBinding spec has been updated to explicitly mention that underscores are valid in credential keys (https://github.com/servicebinding/spec/blob/main/README.md#workload-projection). Updating the implementation to match the change in the spec

Signed-off-by: Tom Kennedy <[email protected]>
@philippthun philippthun merged commit c4a118b into cloudfoundry:main Aug 21, 2025
24 of 26 checks passed
ari-wg-gitbot added a commit to cloudfoundry/capi-release that referenced this pull request Aug 21, 2025
Changes in cloud_controller_ng:

- Loosen file name validation for kubernetes service bindings
    PR: cloudfoundry/cloud_controller_ng#4525
    Author: Tom Kennedy <[email protected]>
@tomkennedy513 tomkennedy513 deleted the k8s-service-binding-fix branch August 21, 2025 14:11
@philippthun
Copy link
Member

@tomkennedy513 @sethboyles @Gerg @stephanme - As underscores were not valid before, we are transforming several VCAP_SERVICES attributes (e.g. binding_name -> binding-name): https://github.com/cloudfoundry/cloud_controller_ng/blob/a90419c5dcafce9cb209f124e789d255087c5c30/lib/cloud_controller/diego/service_binding_files_builder.rb#L110C11-L112

It would be nice to remove this transformation, although it's an incompatible change. But this feature is marked as experimental, so the question is: shall we dare?

@tomkennedy513
Copy link
Contributor Author

@tomkennedy513 @sethboyles @Gerg @stephanme - As underscores were not valid before, we are transforming several VCAP_SERVICES attributes (e.g. binding_name -> binding-name): https://github.com/cloudfoundry/cloud_controller_ng/blob/a90419c5dcafce9cb209f124e789d255087c5c30/lib/cloud_controller/diego/service_binding_files_builder.rb#L110C11-L112

It would be nice to remove this transformation, although it's an incompatible change. But this feature is marked as experimental, so the question is: shall we dare?

ya, I left that alone because I wasn't sure if it would be ok to break that. If you think its ok, I'm happy to open another pr.

@stephanme
Copy link
Member

@tomkennedy513 : we discussed changing the mapping of the VCAP_SERVICES attributes (currently: underscore to dash) in the ARI WG meeting 26.8.25 and we think it would be good to use the original attribute names. Since the app feature service-binding-k8s is still documented as experimental, we should dare the change (now or never...).

A PR would be very appreciated.

Gerg pushed a commit that referenced this pull request Aug 27, 2025
- The ServiceBinding spec has been updated to explicitly mention that underscores are valid in credential keys (https://github.com/servicebinding/spec/blob/main/README.md#workload-projection). Updating the implementation to match the change in the spec

Signed-off-by: Tom Kennedy <[email protected]>
tomkennedy513 added a commit to tomkennedy513/cloud_controller_ng that referenced this pull request Sep 2, 2025
tomkennedy513 added a commit to tomkennedy513/cloud_controller_ng that referenced this pull request Sep 2, 2025
@tomkennedy513
Copy link
Contributor Author

@tomkennedy513 : we discussed changing the mapping of the VCAP_SERVICES attributes (currently: underscore to dash) in the ARI WG meeting 26.8.25 and we think it would be good to use the original attribute names. Since the app feature service-binding-k8s is still documented as experimental, we should dare the change (now or never...).

A PR would be very appreciated.

@stephanme Here is the pr #4542

stephanme pushed a commit that referenced this pull request Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants