Read bindings from VCAP_SERVICES

[c0d1ngm0nk3y]

Buildpacks like https://github.com/paketo-buildpacks/dynatrace need specific bindings (e.g. credentials to download assets). If the bindings are not only read from the file system, but from the env variable VCAP_SERVICES, those buildpacks could be used not only on kubernetes, but also on cloudfoundry.

Backport of #227

[dmikusa]

It looks like the linter is flagging at least one thing here. If you run make lint locally you can see what it's complaining about. Otherwise, when tests are passing we can get this merged.

[c0d1ngm0nk3y]

It looks like the linter is flagging at least one thing here.

My bad. Should be fixed now.

[dmikusa]

Sorry, it's still failing. The DCO check is failing as well. That's a pain, but if you click Details it walks you through how to fix that.

[c0d1ngm0nk3y]

Sorry, it's still failing. The DCO check is failing as well. That's a pain, but if you click Details it walks you through how to fix that.

My bad again. I always forget that one. But also the unit tests are failing, though they worked locally. I'll have a look.

[dmikusa]

But also the unit tests are failing, though they worked locally. I'll have a look.

Maybe it's a race condition? The failing test is reading an env variable, so perhaps two tests are running try to read the same env variable and it's an inconsistent result.

I can't say this is the issue and I don't have a specific reason, other than that's the way I've always seen it done, but when I set env variables in unit tests I always do them in a it.Before block which a specific context for that env variable.

context("VCAP_SERVICES is set with ...", func() {
    it.Before(func() {
       t.Setenv(...)
    })

    it("should parse", func() { ... })
})

In your PR, you're setting the env variables directly in each test.

[c0d1ngm0nk3y]

Maybe it's a race condition?

The problem was that the tests rely on the order of the bindings, gut the order when iterating through a dict is not guaranteed in go. It works most of the time, but not always. The same problem should be in main as well :( I will open another pr.

[dmikusa]

Awesome catch & fix! Thanks for the other PR as well.

[dmikusa]

@c0d1ngm0nk3y - I was thinking about this more, and I think we should adjust the way this maps a little bit.

First, the service binding spec has two entries that you can populate, one is required the type and one is optional the provider. Right now we have the provider value being mapped to the type, I think we should change that to set the provider. Then I think we should take the value of the label and write that as the type. They are sometimes the same thing, but not always.

I think we should also try to flatten out the credentials into key/value pairs on the binding.

Like this:

{
    "<provider>": [
      {
        "name": "<name>",
        "binding_guid": "44ceb72f-100b-4f50-87a2-7809c8b42b8d",
        "binding_name": "elephantsql-binding-c6c60",
        "instance_guid": "391308e8-8586-4c42-b464-c7831aa2ad22",
        "instance_name": "elephantsql-c6c60",
        "label": "<type>",
        "tags": [
          "postgres",
          "postgresql",
          "relational"
        ],
        "plan": "turtle",
        "credentials": {
          "key": "value",
          ...
          "uri": "postgres://exampleuser:examplepass@postgres.example.com:5432/exampleuser"
        },
        "syslog_drain_url": null,
        "volume_mounts": []
      }
    ],
...
}

[c0d1ngm0nk3y]

@c0d1ngm0nk3y - I was thinking about this more, and I think we should adjust the way this maps a little bit.

Like this? https://github.com/buildpacks/libcnb/pull/235