Korifi API users should be able to provide JSON objects as user provided service parameters

[georgethebeatle]

According to the service credential binding and service instance documentation, the parameters request parameter is A JSON object that is passed to the service broker. This means that users should be able to create a user provided service via the following command:

cf create-user-provided-service s2 -p '{"a":"b", "c":{"d": "e"}}'

On CF for VMs the command above succeeds:

❯ cf create-user-provided-service s2 -p '{"a":"b", "c":{"d": "e"}}' -v
Creating user provided service s2 in org org1 / space space1 as admin...
REQUEST: [2023-09-27T11:41:46Z]
POST /v3/service_instances HTTP/1.1
Host: api.mel-c.korifi.cf-app.com
{
  "credentials": {
    "a": "b",
    "c": {
      "d": "e"
    }
  },
  "name": "s2",
  "relationships": {
    "space": {
      "data": {
        "guid": "5e8590f4-b7db-4529-9f4e-6e4af97abd5a"
      }
    }
  },
  "type": "user-provided"
}


RESPONSE: [2023-09-27T11:41:46Z]
HTTP/1.1 201 Created
{
  "created_at": "2023-09-27T11:41:46Z",
  "guid": "aff1e51d-4a42-406e-abe9-bdb0025cdb7d",
  ....
}


OK

However, on Korifi we get the following error:

❯ cf create-user-provided-service s2 -p '{"a":"b", "c":{"d": "e"}}' -v
Creating user provided service s2 in org org1 / space space1 as cf-admin...
REQUEST: [2023-09-27T11:49:59Z]
POST /v3/service_instances HTTP/1.1
Host: localhost
{
  "credentials": {
    "a": "b",
    "c": {
      "d": "e"
    }
  },
  "name": "s2",
  "relationships": {
    "space": {
      "data": {
        "guid": "cf-space-17c91428-0a29-4a7c-83ba-3740c6276830"
      }
    }
  },
  "type": "user-provided"
}


RESPONSE: [2023-09-27T11:49:59Z]
HTTP/1.1 422 Unprocessable Entity
{
  "errors": [
    {
      "code": 10008,
      "detail": "Credentials must be a string",
      "title": "CF-UnprocessableEntity"
    }
  ]
}


Credentials must be a string
FAILED

The reason for this error is that we expect that the parameters field is map[string]string. In order to support parameters that are json object we should change that to map[string]any. We should ensure that we implement this in a backwards compatible manner, epscially with regards to existing user provided services and the secrets they use to store their parameters.

See this slack discussion for more context

[danail-branekov]

Spike branch
Slack discussion

[danail-branekov]

We got feedback from @vlast3k that some apps would fail when unmarshalling the parameters object if an extra property (such as .metadata) is available. We should therefore ensure we filter out extra fields when building the VCAP_SERVICES content.

In addition to that, Korifi adds a type key to the secret that is eventually getting exposed in VCAP_SERVICES. Maybe we should be filtering it as well.
Also, what would happen if the user provides a type parameters key? Is there going to be a collision?

[georgethebeatle]

We had an alternative spike on the issue, see this commit

• Accept map[string]any instead of map[string]string credentials
  when creating a user-provided service instance

• Backwards compatibility with existing UPSIs and bindings is achieved
  by

  • The CFServiceInstance controller is migrating existing credentials
    secret to a new one that has a single data key that contains the
    marshalled credentials object, the old (legacy) secret is preserved.
  • Introduce CFServiceInstanceStatus.Credentials field to reference
    the service instance secret. For legacy service instances this
    references the migrated secret, while newly created service
    instances reference the newly created secret in the format above.
  • The VCAP_SERVICES builder uses the credentials secret in the
    service instance status (that is guaranteed to be in the new format)
    to build the content of the VCAP_SERVICES env var.
  • Newly created bindings for existing service instances would make use
    of the migrated secret
  • Existing bindings have their CFServiceBindingStatus.Binding
    ducktype set to the old legacy service instance credentials secret.
    The CFServiceBinding controller would notice this and would not
    change them. Thus old bindings would still work, and most
    importantly, would not change and would not cause workloads
    statefulset restart.

• For newly created bindings, the CFServiceBinding controller
  reconciles the service instance secret (the one with the single
  data key) and creates a new secret with multiple keys (that is the
  format of the legacy secret). This secret is used on the
  servicebinding.io ServiceBinding to ensure the expected
  map[string]string format on the filesystem mount

• We have removed the CFServiceInstanceStatus.Binding ducktype as it
  turned out to be unused.

Pros:

• Completely backwards compatible
• No statefulset restart on Korifi update
• VCAP_SERVICES credentials attributes are populated precisely the way the user provided them in the cf create-user-provider-service command, there are no additional typeand provider fields. This addresses picky applications that expect credentials to contain very specific keys.
• The migration logic could be removed once we decide that all users have rebound their apps

Cons:

• None, besides being a bit complex