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

Clarify factory resolution process for private Bitbucket repositories #18389

Closed
skabashnyuk opened this issue Nov 17, 2020 · 1 comment
Closed
Labels
area/factory/server Server side of factory implementation kind/task Internal things, technical debt, and to-do tasks to be performed.
Milestone

Comments

@skabashnyuk
Copy link
Contributor

skabashnyuk commented Nov 17, 2020

Is your task related to a problem? Please describe.

Implement Factory resolution process for private Bitbucket repositories

Describe the solution you'd like

In case if che server has enough information (it has Bitbucket personal access token as k8s secret #18386 or it has Bitbucket OAuth1 token)
it should return FactoryDto with devfile inside as a result of /api/factory/resolver method. The goal of this task
is to clarify:

  1. What is the response of /api/factory/resolver if 'OAuth1 token' is not exists
  2. What is the response of /api/factory/resolver if 'Bitbucket personal access token' is expired
  3. What is the response of /api/factory/resolver if the git repository is not exists
  4. What is the response of /api/factory/resolver if the git repository is not available for the current user
  5. Periodical check if Bitbucket personal access token is still valid.
  6. Keep in sync git-credentials-secret and bitbucket-personal-access-token-secret

Describe alternatives you've considered

n/a

Additional context

#18385

@skabashnyuk skabashnyuk added kind/task Internal things, technical debt, and to-do tasks to be performed. area/factory/server Server side of factory implementation labels Nov 17, 2020
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Nov 17, 2020
@vzhukovs vzhukovs removed the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Nov 17, 2020
@mshaposhnik mshaposhnik changed the title Factory resolution process for Bitbucket repositories Factory resolution process for private Bitbucket repositories Nov 18, 2020
@mshaposhnik mshaposhnik changed the title Factory resolution process for private Bitbucket repositories Clarify actory resolution process for private Bitbucket repositories Nov 18, 2020
@mshaposhnik mshaposhnik changed the title Clarify actory resolution process for private Bitbucket repositories Clarify factory resolution process for private Bitbucket repositories Nov 18, 2020
@skabashnyuk skabashnyuk added this to the 7.26 milestone Jan 20, 2021
@mshaposhnik
Copy link
Contributor

mshaposhnik commented Jan 20, 2021

From the theoretical POW, i would propose the following reactions to the situations which are given in descrition:

What is the response of /api/factory/resolver if 'OAuth1 token' is not exists
What is the response of /api/factory/resolver if 'Bitbucket personal access token' is expired

Those two looks pretty similar for me and can be covered with single 303 See other response followed by Location header.
FAQ:

  1. Why not 401 ?
    401 server response usually means that method returning it, expects some kind of authentication data, and this data is invalid or absent, and situation may be fixed by sending correct credentials.
    In our case, we never expect that factory/resolve method accepts some kind of SCM (Bitbucket or other) authentication data, so it is not appropriate response.

  2. Why not 302 or 307.
    Those codes have some restrictions. 307 has strict limitation that original HTTP method should not be changed (/factory/resolve is a POST request) for new URL. 302 have the same limitation but lesst strict and sometimes violated.
    Despite 303 is little exotic, it means that method should be changed to GET which is exacly our case if a m not mistaken. Otherwise 302 can be a good choice as well.

Rest of the situations does not looks very questionable for me:

What is the response of /api/factory/resolver if the git repository is not exists

clear 400 Bad request with error explanation in body

What is the response of /api/factory/resolver if the git repository is not available for the current user

clear 403 Forbidden or 400 with error explanation in body

What if user requested /api/factory/resolver but OAuth integration is not configured on the server side

Since it's an unrecoverable error (i.e. user cannot fix situation by fixing the request) i would prefer 5xx codes. 501 Not Implemented or 500 looks Ok for me in such situations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/factory/server Server side of factory implementation kind/task Internal things, technical debt, and to-do tasks to be performed.
Projects
None yet
Development

No branches or pull requests

4 participants