Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SB: Service Binding for Kyma Service Instance - Epic #284

Closed
13 tasks done
pbochynski opened this issue Dec 19, 2023 · 22 comments
Closed
13 tasks done

SB: Service Binding for Kyma Service Instance - Epic #284

pbochynski opened this issue Dec 19, 2023 · 22 comments
Assignees
Labels
2024-Q4 Planned for Q4 2024 kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Comments

@pbochynski
Copy link

pbochynski commented Dec 19, 2023

Description
Provide a service binding for Kyma Runtime instance. The binding should contain all the information to connect to the runtime (KUBECONFIG or all the information required to build a KUBECONFIG). The binding request can create TokenRequest and return the token in the binding.

Reasons
Users can create Kyma Runtime instances with API or BTP CLI. But the instance cannot be accessed without user being authenticated (OIDC flow). Therefore any automation flow that involves provisioning Kyma and deploying any workload there cannot be easily implemented. The only way to do that is to use custom OIDC provider, but that option is not easy to set up and use.

Attachments
Other way to achieve the automation would be kyma-project/kyma#18305

AC Added by PK

Links

  1. how to make given plan bindable on given env: https://github.tools.sap/kyma/management-plane-config/blob/main/argoenv/keb/dev/values.yaml#L101, and code: https://github.com/kyma-project/kyma-environment-broker/blob/main/internal/broker/bind_create.go#L18
  2. Gardener: https://github.com/gardener/gardener/blob/master/docs/usage/shoot_access.md#shootsadminkubeconfig-subresource
@pbochynski pbochynski changed the title Service binding for Kyma sercice instance Service binding for Kyma service instance Dec 19, 2023
@kwiatekus
Copy link
Contributor

the static bootstrap token created at the provisioning is extremely short living. (~5min)

@pbochynski
Copy link
Author

As we discussed we should not use static token. I think TokenRequest api would be the best. KEB can call it directly (without provisioner / infrastructure manager in the middle) using own Kubeconfig. That would be quick and easy solution.

@PK85
Copy link

PK85 commented Dec 19, 2023

POCs:

  1. create a technical user in BTP and with BTP CLI create Kyma instance and Kyma binding
  2. create a technical user in BTP and create btpOperator instance and then with clientCredential create Kyma instance and Kyma binding

@danail-branekov
Copy link

FWIW, in order to address kyma-project/kyma#18092 in our project we have come up with a script that

  • Uses the kyma human user kubeconfig to create an admin service account
  • Generates a kubeconfig containing the service account secret

The human user has to be a cluster admin in order to be able to create the service account though.

@szwedm
Copy link
Contributor

szwedm commented Mar 7, 2024

Next steps should be discussed with @pbochynski @PK85 @piotrmiskiewicz.

@kwiatekus
Copy link
Contributor

kwiatekus commented Mar 15, 2024

@PK85, @tobiscr, @pbochynski wouldnt' this gardener's feature offer an alternative, shortcut solution?
https://github.com/gardener/gardener/blob/master/docs/usage/shoot_access.md#shootsadminkubeconfig-subresource

@PK85
Copy link

PK85 commented May 6, 2024

Another meeting with CIS team needed

@PK85
Copy link

PK85 commented May 21, 2024

We had a meeting again, and we agreed to implement MVP with Service Bindings. The MVP proposal is already to CIS team, awaiting timelines

@PK85
Copy link

PK85 commented Jun 3, 2024

@pbochynski pbochynski added 2024-Q4 Planned for Q4 2024 and removed 2024-Q1 labels Jun 10, 2024
@PK85
Copy link

PK85 commented Aug 5, 2024

Decisions after a meeting (5.08.2024) with CIS team:

  • binding ID - will be generated by ERS and the user will not have to send it to the create binding API
  • context - will be filled with the user id and origin (same as in create instance)
  • parameters - user will provide that as part for the ERS payload and no schema validation will be done on the parameters
  • bindable - ERS will validate the service and plan bindable property, if it is false, the binding request will be rejected.
  • rotate - will not be supported, the user will have to delete and create a new binding.
  • service and plan IDs - will be calculated by ERS (fetched form the service instance), the user will not have to send it as part of the payload.

@PK85
Copy link

PK85 commented Aug 12, 2024

Communication from CIS (11.08.2024):

A few updates from ERS side:

  1. I saw that the user is sent as part of the headers in Service Instance creation, so we will align to it
    ERS is acting as a proxy - meaning - we will not save anything on our side (from bindings perspective). this leads to the following:
  • a. The API user (call to ERS->createBinding) will need to pass the binding UUID (he is the one that manages the binding, and needs to know the ID)
  • b. We will support async creation, but polling will be on the API user's side (since we cannot poll on our side) - the user that creates the binding (and knows the ID), will request polling on that binding, from us
  • c. We will expose also API for polling last operation from the service broker

So, in terms of payload for service broker from ERS:

  • binding ID - will be sent by the user, we will not generate since we can't save it on our side
  • context - no need to send it, we will fill the user id and origin (same as in create instance) as part of the headers
    parameters - user will provide that as part of the ERS payload and no schema validation will be done on the parameters (see bellow)
  • bindable - ERS will validate the service and plan bindable property, if it is false, the binding request will be rejected.
  • rotate - will not be supported, the user will have to delete and create a new binding.
  • service and plan IDs - will be calculated by ERS (fetched form the service instance), the user will not have to send it as part of the payload.
  • Headers - we will add user origin and email, as an encoded string in "X-Broker-API-Originating-Identity" header (same as in service instance creation)

And my answer:

  1. about user id we are getting that as a part of the context for SI operations:
    "context": {
        ...
        "user_id": "piotr.kopczynski@sap.com"
    }
  1. About UUID, when user creates Service Instance, the ID is generated by ERS. I think it should be still generated for user. There is as well a way to list all Environments list,

That means you should gerenate UUID, or we will generate that, see SM APIs https://api.sap.com/api/APIServiceManager/resource/ for service bindings

and then if sync mode 201 with ID and credentials in the response, or if async 202 and location header where the path serves as the relative path for the base request URI of the API: 'Get operation status'.

I think we need to meet and clarify that. Cause ERS is a platform and features like GET bindings belongs to it. Same like we have GET environments.

@PK85 PK85 added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 28, 2024
@ralikio ralikio self-assigned this Sep 2, 2024
@ukff ukff self-assigned this Sep 3, 2024
@ralikio ralikio unassigned ukff Sep 5, 2024
@PK85
Copy link

PK85 commented Sep 11, 2024

Areas:

@gowrisankar22
Copy link

@pbochynski Is it a right approach, for the runtime implement a service binding to provide the kubeconfig ? Usually, service binding is for the services.

@PK85
Copy link

PK85 commented Sep 17, 2024

@gowrisankar22 Service Bindings APIs are right now under implementation for Provisioning Service by CIS team. The Kyma Environment Broker is the OSB API implementation. Why do you think Service Bindings are only for services? Cheers, PK

@pbochynski
Copy link
Author

@pbochynski Is it a right approach, for the runtime implement a service binding to provide the kubeconfig ? Usually, service binding is for the services.

Exactly - Kyma runtime is a service and this is the long-term strategy. You can already provision Kyma from Service Marketplace in BTP cockpit.

@PK85
Copy link

PK85 commented Oct 7, 2024

Final meetings with CIS team (Zahi)

Hi, we should talk about three things.

  1. Add user_id (clientid from Cloud Management Service binding) to the context in create binding request (PUT).
  • Zahi: asked about checking again with full body log + headers (Ksawery please check again)
  1. Set accepts_incomplete to false, because we've decided to use synchronous responses.
  • Zahi: it is removed from API call completely
  1. When binding expires we are removing entry form KEB database. Let's say user created 20 bindings and they all expired. In this case there will be no entries in KEB database, but get all bindings endpoint will return 20 binding with status OK. When user will try to get Credentials for one of them he will receive an error. Possible solution:
    KEB will return expires_at field in Binding Metadata during binding creation. You can save this field in the database, and return only valid bindings to user.

@KsaweryZietara
Copy link
Contributor

KsaweryZietara commented Oct 10, 2024

There are two ways of getting an access token for SAP Provisioning Service API:

  • Using Password grant type
curl -L -X POST '<uaa_url>/oauth/token' \ 
-H 'Content-Type: application/x-www-form-urlencoded' \ 
-u '<clientid>:<clientsecret>' \ 
-d 'grant_type=password' \ 
-d 'username=<user email>' \
-d 'password=<password>'

During binding creation KEB receives body with context:

context: {
   "email":"user email",
   "origin":"origin"
}

Context should be saved to database as created_by = email + origin.

  • Using Client Credentials grant type
curl -L -X POST '<uaa_url>/oauth/token' \ 
-H 'Content-Type: application/x-www-form-urlencoded' \ 
-u '<clientid>:<clientsecret>' \ 
-d 'grant_type=client_credentials' \ 

During binding creation KEB receives body with context:

context: {
   "origin":"clientid"
}

Context should be saved to database as created_by = origin.

@szwedm
Copy link
Contributor

szwedm commented Oct 16, 2024

Meeting notes 16.10.24

  1. Service Binding states are not required, we can delete objects by determining expires_at and kubeconfig fields. If expires_at is older than the current time, SB should be deleted, - SB: Binding Doc Update #1357
  2. kubeconfig value determines SB state. If it's empty, SB is in an unknown state, it may be in progress (max 800ms) or error, SB: Binding Doc Update #1357
  3. The first step in SB provisioning is creating a record in DB with empty kubeconfig which is populated in the last step,
  4. The first step in SB deprovisioning is the cleanup of resources (ServiceAccount, ClusterRole, ClusterRoleBinding) in SKR, the last step is SB removal from DB, ✅
  5. Return info about kubeconfig existence (not value!) on call to /runtimes endpoint from KCP CLI, SB: KCP CLI - Kubeconfig Existence #1358
  6. During SB creation, we check wether the binding exists in DB ✅ ❓ . If kubeconfig is missing, return 500 ❓Should we really return 500 in this case ❓ , if it exists, return 200 (do nothing).
    6.1. During SB Creation first insert a record in DB, if conflict, then GET. Service Binding Insert Order #1364
  7. Create a ticket for monitoring and metrics for KEB and SB cleanup job. SB: Metrics #1359

@ralikio
Copy link
Member

ralikio commented Oct 17, 2024

10.10.2024 - Summary from the meeting about using only token requests from SA from SKR and idempotent behaviour:

Legend:
BindingID: means: BindingId - InstanceID pair on KEB side

Contract between Provisioning Service and KEB bindings:
Section moved to #1352 (comment)

KEB bindings responses details: @MarekMichali
Section moved to #1352

KEB implementation about handling binding related resources, like ServiceAccount, ClusterRole, ClusterRoleBidning

  • 1) we need to have internal binding state representing ok, in_progress and error states - The state is represented by kubeconfig field - non-empty kubeconfig: binding created ok; empty kubeconfig: otherwise
  • 2) probably we have two calls to db, at the begining setting in_progress state and later at the end ok or error at the beginning with an empty kubeconfig, and setting the kubeconfig when all is done/created without error Corrected Binding Insert Order #1366
  • 3) to eliminate temporary issues when calling SKR to create SA, CR, CRB we need to add retries (2,3 attempts) every 1,2 seconds - [JP - it seems that during Wednesday meeting we decided otherwise]
  • 4) for whole binding creation we have internal overall timeout set to 15 seconds - Timeout of creation Service Binding #1388
  • 5) If we have expired bindings we do not return them when GetBinding is comming from Provisioner Service. We just keep that binding internally to be cleaned up by binding-cleanup-job later. Return 404 from GET Bindings for Expired Bindings #1355
  • 6) we have limit up to 10 valid bindings (expired ones do not count)
  • 7) delete Bindings enpoint deletes all sub resources, if some of them already not exist go to next one and send success code Missing Bindings' Resources Removal #1322

KEB binding-cleanup-job details
Section moved to #1347 (comment)

@kyma-project kyma-project deleted a comment from PK85 Oct 17, 2024
@ralikio ralikio changed the title Service binding for Kyma service instance SB: Service Binding for Kyma Service Instance - Epic Oct 17, 2024
@kyma-project kyma-project deleted a comment from PK85 Oct 17, 2024
@PK85 PK85 mentioned this issue Oct 23, 2024
2 tasks
@kwiatekus
Copy link
Contributor

kwiatekus commented Nov 18, 2024

@PK85
Is it planned to support creation of kyma environment bindings via

It would be necessary for good devex in automation scenario

@PK85
Copy link

PK85 commented Nov 18, 2024

@kwiatekus
BTP CLI and Cockpit was removed from the scope by CIS team. Maybe in the future we can back to them asking about that.

@IwonaLanger
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2024-Q4 Planned for Q4 2024 kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

No branches or pull requests