Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Generic Extensions #2785

Closed
Samze opened this issue Mar 20, 2020 · 6 comments
Closed

Generic Extensions #2785

Samze opened this issue Mar 20, 2020 · 6 comments
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@Samze
Copy link
Contributor

Samze commented Mar 20, 2020

Feature: Generic Extensions

The OSBAPI specification has a pending PR to support Generic Extensions.

I'm creating this issue to start a conversation on how this might be implemented in service-catalog and to get feedback on ideas.

Why

From the PR:

The specification defines endpoints that allow the lifecycle management of
service instances and service bindings. However, a common complaint is that
it does not support some of those important “day 2” operations that
developers might want, e.g backup and restore. Also that it does not allow
service specific operations, e.g. MySQL set leader. To accomplish this
Service Brokers authors have the option to either go off-spec or to misuse
the spec (e.g. cf update-service -p ‘{“trigger-backup”: true}’).

The specification needs an extension mechanism to allow authors to define new
endpoints.

What is it

Service Brokers can provide OpenAPI documents attached to Plans in their Catalog. The extensions can then be triggered by platforms on defined paths on the Service Broker.

For example a Broker may host the following OpenAPI document to perform a backup:

openapi: "3.0.0"
info:
  version: 1.0.0
  title: My Service Extension
paths:
  /backup:
    post:
      summary: Backup of a Service Instance
      operationId: backup
      tags:
        - backup
      responses:
        '202':
          description: Backup accepted

And this can be triggered on the broker on a particular path:

/v2/service_instances/:instance_id/extensions/broker-extension-path/backup.

An example broker implementation can be found here.

The Problem

The problem can be broken down into two parts:

  1. Discoverability

How does a consumer know that the service instance now supports a backup extension? What is the best mechanism for them to consume the information in the OpenAPI document.

  1. Triggering the Extension

How could service-catalog trigger the extension and return a response?

Notes

  • The extensions are behind the same broker auth mechanism and so it must be the platform that makes the extension request and not the client in order to avoid leaking the broker secret.
  • There is no limitation on what kind of OpenAPI document the Broker provides, it could be simple or hugely complicated. Platforms need to support all documents.

In CloudFoundry

For reference this is how we imagine it could be implemented in CF.

  1. Discoverability

In CF when we read the catalog we can make the Broker's OpenAPI document available to the consumer via the CF API.

  1. Triggering the Extension

CF will then give the user an endpoint on the platform on which extensions can be triggered. E.g. /v2/service_instances/:guid/extesions/* In effect CF will act as a proxy to the Service Broker to trigger the extension.

Thoughts for service-catalog

A challenge for service-catalog is that it cannot really act a proxy like CF, since it is based on CRDs and reconciliation.

For discoverability we see a few options:

  1. Put discoverability out-of-scope for now to focus on triggering and just expect consumers to know what extensions are available.
  2. Just add the full OpenAPI document embedded in the ClusterServicePlan resource and let users discover it via k8s resource.
  3. Model the extensions as CRDs and create a new ServiceInstanceExtension resource that belong to Service Instances.
  4. CLI support to display extension OpenAPI.

For Triggering the Extension:

  1. Provide a custom resource to "trigger" extension's HTTP requests. E.g. ServiceInstanceExtensionRequest. See something like this poc
  2. Use Jobs to perform the one-off required HTTP requests. For example, we could develop a job that is able to run curl and execute it with required parameters to trigger a extension
  3. Provide a custom resource that maps to the HTTP resource rather than the HTTP request. E.g. A /backup/{id} resource may have POST / GET / LIST / DELETE. Can we create a CR that you can manipulate via kubernetes that maps to the coresponding HTTP methods? e.g. Create CR -> POST, Delete CR -> DELETE.
  4. Service-catalog ships with a proxy component that triggers extensions.

Questions

  • How much CLI support do we need to provide?
  • Is there any OpenAPI tooling that we can take advantage of?
  • What is the best way to model request/response against APIs in Kubernetes?

Sam & @teddyking

@mszostok
Copy link
Contributor

mszostok commented Jun 2, 2020

IMO CRDs are rather used for the configuration and manifests about the expected cluster state (eventually consistent). CRDs are manifests and the controller should ensure that the described state is present in the cluster.

I'm really not sure if it is a good idea to use CRDs just as a wrap layer for HTTP calls.
My concerns:

  • what about secret values? If a user needs to send or will receive secret value in the body? Such a thing should be stored in K8s Secret but adding that to your proposal will complicate described CRD for HTTP calls

  • what about retries when calling the extension endpoint?. With OSB API we have a nice contract that we can implement in a given platform e.g. in K8s Service Catalog but what about here? what kind of best practice we should implement for extensions? Retries expected or not? orphan mitigation? etc. I see that in the CF proposal, the user is responsible for handling the HTTP calls.

  • The CRD has a max size that can handle. This limitation comes from etcd which has a 1MB limit on object sizes. 1MB is quite big but we have a limitation right at the beginning that e.g. at the same time request body cannot be bigger than 0.5MB and the response body cannot be bigger than 0.5MB

  • How to handle the lifecycle?

    Provide a custom resource that maps to the HTTP resource rather than the HTTP request. E.g. A /backup/{id} resource may have POST / GET / LIST / DELETE. Can we create a CR that you can manipulate via kubernetes that maps to the coresponding HTTP methods? e.g. Create CR -> POST, Delete CR -> DELETE.

    witch such thing we are not able to map the PUT,PATCH,GET,HEAD,OPTIONS HTTP methods

  • How to handle cases when e.g. the response is paginated?

Before doing such thing IMO it will be good to ping the sig API machinery if the CRD can be used in that way if we will not be blocked in the future even more with other restrictions - not only size.

So based on my concerns I think that maybe we can just create a small gateway (separate container) that will only add auth to the request and proxy it to the given broker? In that way, we will give end-user a similar approach both in K8s and CF and also user will be responsible to implement how to handle the HTTP calls (errors, retries, pagination etc)

@Samze
Copy link
Contributor Author

Samze commented Jun 9, 2020

Thanks for the comment @mszostok.

Yeah spending some more time playing around with some PoCs and reading your concerns I agree that CRDs to represent arbitrary Broker actions could lead to a whole world of trouble. I think option 4, Service-catalog ships with a proxy component that triggers extensions is probably the right way forward, which I think aligns with your suggestion.

I know @rsampaio has been thinking about this too, do you have any thoughts?

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 7, 2020
@jhvhs
Copy link
Contributor

jhvhs commented Sep 7, 2020

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 7, 2020
@jhvhs
Copy link
Contributor

jhvhs commented Nov 16, 2020

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Nov 16, 2020
@mrbobbytables
Copy link

This project is being archived, closing open issues and PRs.
Please see this PR for more information: kubernetes/community#6632

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

6 participants