Support buildpack registry names

[mars]

Various resources have attributes that allow setting buildpacks:

• heroku_app
• heroku_build

Currently, these resources implicitly support only http URLs, although the Heroku Platform API now supports Buildpack Registry names as well.

Thanks to recent updates to the heroku-go client, registry names can now be implemented in these resources.

I proposed implementing a simple heuristic to detect Registry Names:

• does the string contain a single backslash in between strings of other characters?
• otherwise, consider it a URL.

The heuristic would be shared between the heroku_app & heroku_build resources.

And then the detected value would be set into the underlying name or url property.

[shawncatz]

👍

[talbright]

Seems reasonable. Maybe something along these lines: https://play.golang.org/p/Ndcb-IY5hpP

[shawncatz]

Interesting, I was just thinking of using a regexp:

re := regexp.MustCompile(`^\w+\/\w+$`)
re.MatchString("heroku/go") // => true

Of course they can be a pain

[mars]

Agreed @shawncatz , in the proposal I intentionally avoid parsing URLs in Terraform. Just because a value is a valid URI does not mean it's valid for a buildpack.

By minimizing the Registry Name detection to a very simple regex for the single / surrounded by strings, and otherwise considering it a URL, I think we avoid overfitting the solution. Let the Heroku Platform API return errors for unacceptable URLs, like it already does.

@talbright strings.Split returns 2 for "part1/part2", "part1/", "/part2", & "/", so regex like @shawncatz's seems to be the way. Also, url.ParseRequestURI considers many things valid URIs which are not valid buildpack URLs, including "/part2" & "/". 😬

[shawncatz]

I was going to take a stab at a fix for this, but the complexity in the heroku_app resource and how it uses buildpacks was going to require more time than I can give right now.

[mars]

Understandable @shawncatz , I'm to putting this in my queue to work on in the next few weeks 📥

[talbright]

Yup, that was just a brief example, I didn't take it to the next step. You'd need to tease apart the *URL components to make sure the scheme was correct, etc. And then in the case of split, you would have to make sure both strings aren't empty. I've usually regretted it when not use a languages stdlib to parse a URL, but YMMV.

[mars]

I've been working on implementing this, but I've found it's blocked by the behavior of the Platform API.

Specifically, when setting a buildpack by Name, API accepts it, but then API responds with the resolved buildpack URL instead of the Name. This causes Terraform to force a new resource because the buildpacks read from API do not match the values set in the Terraform configuration.

Definitely open to ideas here, but I don't think we'll be able to cleanly support Buildpack Registry Names because of this state churn.

[mars]

Here's a comparison of my changes thus far.

…and here's the relevant output from the new test TestAccHerokuBuild_BuildpackRegistryName:

buildpacks.0: "https://buildpack-registry.s3.amazonaws.com/buildpacks/heroku/ruby.tgz" 
              => "heroku/ruby" (forces new resource)

[sigmavirus24]

My team has encountered this with heroku-community/l2met-shuttle and heroku/pgbouncer. Even after applying those, changes, it still shows changes to be applied. How can one help get this resolved?

[mars]

@sigmavirus24, per my comment above:

Definitely open to ideas here, but I don't think we'll be able to cleanly support Buildpack Registry Names because of this state churn.

🙅‍♂️ It's an incompatibility with Heroku API, because the API always returns the URL, not the name set in Terraform. This is the expected Terraform behavior, whenever it receives different resource attribute values from a provider's API.

✅ The only clean solution is to use buildpack URLs within Terraform configurations.

[sigmavirus24]

I guess that means there's no way to replicate whatever heroku buildpacks -a sushi provides then? That's disappointing

[mars]

That's a great point @sigmavirus24 👍

If the API call made by heroku buildpacks -a sushi returns the correct translation, then it's possible that the provider could be updated to read the app's buildpacks using that (additional) API call, instead of relying on what comes back in the app info payload.

We're definitely open to contributions ✨

[sigmavirus24]

  http [
  http   {
  http     buildpack: {
  http       url: 'https://github.com/heroku/heroku-buildpack-ssh-key',
  http       name: 'https://github.com/heroku/heroku-buildpack-ssh-key'
  http     },
  http     ordinal: 0
  http   },
  http   {
  http     buildpack: {
  http       url: 'https://buildpack-registry.s3.amazonaws.com/buildpacks/heroku-community/l2met-shuttle.tgz',
  http       name: 'https://buildpack-registry.s3.amazonaws.com/buildpacks/heroku-community/l2met-shuttle.tgz'
  http     },
  http     ordinal: 1
  http   },
  http   {
  http     buildpack: {
  http       url: 'https://buildpack-registry.s3.amazonaws.com/buildpacks/heroku/pgbouncer.tgz',
  http       name: 'https://buildpack-registry.s3.amazonaws.com/buildpacks/heroku/pgbouncer.tgz'
  http     },
  http     ordinal: 2
  http   },
  http   {
  http     buildpack: { url: 'urn:buildpack:heroku/nodejs', name: 'heroku/nodejs' },
  http     ordinal: 3
  http   },
  http   {
  http     buildpack: { url: 'urn:buildpack:heroku/python', name: 'heroku/python' },
  http     ordinal: 4
  http   }
  http ] +277ms
=== sushi Buildpack URLs
1. https://github.com/heroku/heroku-buildpack-ssh-key
2. heroku-community/l2met-shuttle
3. heroku/pgbouncer
4. heroku/nodejs
5. heroku/python

Indicates that the API is returning a URL in the name 😞 and that made me look at the CLI: https://github.com/heroku/cli/blob/01b9030435e457749d7ffa28883d758a5145c5fc/packages/buildpacks/src/buildpacks.ts#L140-L156 which uses this function to translate the URL to a "name" which is why it shows the right stuff above.

It sounds like this kind of translation isn't something this provider wants to do, right?

[mars]

That's enlightening @sigmavirus24, that the API doesn't return a clean translation into the names.

It looks like there's a pattern that could be matched to translate into a buildpack's Registry Name:

• the value of name attribute,
  • when it contains a single slash /
• the last two url path segments, minus the file extension,
  • when prefixed with https://buildpack-registry.s3.amazonaws.com/buildpacks
  • when prefixed with urn:buildpack:

Unsure that this is the complete set of rules 🤷 Implementing this would also require a custom diff to compare the two possible value variations to each of the buildpack list entries.

All that said, I think it's fine to add such translation logic to the provider, even though it's not ideal separation of concerns. We have plenty of crafty workarounds for such challenges with Heroku API, and this would just be another one!