Skip to content

Add support for setting DATABASE_URL from vcap services#123

Merged
pbusko merged 1 commit intocloudfoundry:mainfrom
tomkennedy513:databaseuri
Oct 30, 2025
Merged

Add support for setting DATABASE_URL from vcap services#123
pbusko merged 1 commit intocloudfoundry:mainfrom
tomkennedy513:databaseuri

Conversation

@tomkennedy513
Copy link
Copy Markdown
Contributor

  • This is replicating behavior that exists in buildpack app lifecycle

- This is replicating behavior that exists in buildpack app lifecycle

Signed-off-by: Tom Kennedy <tom.kennedy@broadcom.com>
@tomkennedy513
Copy link
Copy Markdown
Contributor Author

Just bumping this @cloudfoundry/wg-app-runtime-platform-cnbapplifecycle-approvers since its been open over a month

@modulo11
Copy link
Copy Markdown
Contributor

I cannot find any usage of DATABASE_URL in the Paketo Buildpacks. Not sure why we should add it.

@tomkennedy513
Copy link
Copy Markdown
Contributor Author

I cannot find any usage of DATABASE_URL in the Paketo Buildpacks. Not sure why we should add it.

It's for apps to use primarily, but buildpacks can use it to make decisions on things like providing database drivers. This is supported for all other lifecycles on cf

"net/url"
)

func ParseDatabaseURI(services string) (string, error) {
Copy link
Copy Markdown
Contributor

@pbusko pbusko Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomkennedy513 is there an option to have a reusable version of this function? As you mentioned, this code must be the same across all existing lifecycles. It seems that the dockerapplifecycle imports some functions from the buildpackapplifecycle. Should this be extracted to a new repository which contains shared logic?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied it over (though I did tweak it a bit) because I remember when I did the credhub interpolation there was a strong desire to not have any cross lifecycle dependencies. A third repo make senses but obviously requires a lot more coordination to get this change in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we already have quite a lot of common things between the lifecycles, I believe it's time to introduce the library repository. What is your opinion on that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding a library makes sense, but I think that should be separate from getting this fix in to unblock us.

@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Application Runtime Platform Working Group Oct 30, 2025
@pbusko pbusko merged commit da56e21 into cloudfoundry:main Oct 30, 2025
7 checks passed
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Application Runtime Platform Working Group Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants