feat(x2a): GitLab authentication and token for jobs#2419
Conversation
Changed Packages
|
Review Summary by QodoAdd GitLab authentication and scaffolder integration
WalkthroughsDescription• Add GitLab authentication provider alongside GitHub • Integrate GitLab scaffolder backend module for job support • Update conversion template to support GitLab repositories • Configure GitLab OAuth and environment variables • Add GitLab sign-in option to frontend authentication UI Diagramflowchart LR
A["GitLab OAuth Config"] -->|"Auth Provider"| B["Backend Auth Module"]
B -->|"Scaffolder Support"| C["GitLab Scaffolder Module"]
A -->|"Frontend Integration"| D["GitLab Sign-in UI"]
C -->|"Template Support"| E["Conversion Project Template"]
D -->|"User Authentication"| F["GitLab.com Access"]
File Changes1. workspaces/x2a/packages/backend/src/index.ts
|
Code Review by Qodo
1. Git clone URL malformed
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
mareklibra
force-pushed
the
FLPATH-3353.gitlab
branch
5 times, most recently
from
c5c88c5 to
b833e03
Compare
Review Summary by QodoAdd GitLab authentication and scaffolder backend support
WalkthroughsDescription• Add GitLab authentication and scaffolder backend support alongside GitHub • Implement token augmentation for GitLab OAuth2 git access requirements • Fix typo in parameter name areTargeAndSourceRepoShared to areTargetAndSourceRepoShared • Add userPrompt parameter support for init-phase API requests • Improve error handling and logging in project creation workflow • Update documentation with GitLab OAuth setup and configuration instructions Diagramflowchart LR
A["Backend Setup"] -->|Add GitLab modules| B["Auth & Scaffolder"]
C["Token Management"] -->|Augment for GitLab| D["Git Access"]
E["Project Creation"] -->|Support userPrompt| F["Init-phase API"]
G["Documentation"] -->|GitLab OAuth scopes| H["Setup Guide"]
B --> I["Multi-provider Support"]
D --> I
F --> I
File Changes1. workspaces/x2a/packages/backend/src/index.ts
|
Code Review by Qodo
1. Double JSON read
|
| ctx.logger.error( | ||
| `Project creation response status: ${response.status}, error: ${JSON.stringify(error)}`, | ||
| ); | ||
| throw new Error(error); | ||
|
|
||
| const errorObj = (await response.json()) as any; | ||
| const msg = errorObj?.message ?? JSON.stringify(errorObj); | ||
| throw new Error(msg); |
There was a problem hiding this comment.
1. Double json read 🐞 Bug ⛯ Reliability
On project creation failure, the code calls response.json() twice, which will throw on the second call because the body stream is already consumed. This masks the actual backend error and can cause confusing failures whenever project creation returns a non-2xx response.
Agent Prompt
### Issue description
`createProjectAction` consumes the HTTP response body twice on the non-OK path (`response.json()` is called twice). The second read will throw and hide the real failure.
### Issue Context
This affects the scaffolder action `x2a:project:create` when the x2a backend returns any non-2xx response.
### Fix Focus Areas
- workspaces/x2a/plugins/scaffolder-backend-module-x2a/src/actions/createProject.ts[162-172]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
mareklibra
force-pushed
the
FLPATH-3353.gitlab
branch
from
b833e03 to
25261e2
Compare
Signed-off-by: Marek Libra <marek.libra@gmail.com>
mareklibra
force-pushed
the
FLPATH-3353.gitlab
branch
from
25261e2 to
8955e7d
Compare
elai-shalev
left a comment
elai-shalev
left a comment
There was a problem hiding this comment.
some comments, two actual bugs IMO
Question - this PR only supports "gitlab.com"? not all enterprise gitlab instances?
| @@ -147,13 +165,17 @@ export function createProjectAction( | |||
| ctx.logger.error( | |||
| `Project creation response status: ${response.status}, error: ${JSON.stringify(error)}`, | |||
| ); | |||
| throw new Error(error); | |||
|
|
|||
| const errorObj = (await response.json()) as any; | |||
| const msg = errorObj?.message ?? JSON.stringify(errorObj); | |||
| throw new Error(msg); | |||
There was a problem hiding this comment.
When the project creation response is not OK, the code calls response.json() on line 164 (assigned to error), logs it, then calls response.json() again on line 169 (assigned to errorObj).
A fetch Response body can only be consumed once — the second call will throw TypeError: body used already. This means the error message will never be extracted properly; instead, the catch block will get the "body used already" error. We should the first response.json() call or reuse the result.
| targetRepoToken = augmentRepoToken( | ||
| targetRepoToken, | ||
| getAuthTokenDescriptor({ | ||
| repoUrl: targetRepoUrl, | ||
| readOnly: false, |
There was a problem hiding this comment.
When areTargetAndSourceRepoShared is true, Then line 134-135 sets targetRepoToken = sourceRepoToken (already augmented).
Then lines 140-146 call augmentRepoToken again on targetRepoToken, producing oauth2:oauth2:. I think this is a bug
There was a problem hiding this comment.
right, later change :-( Thanks for catching it
| "@eloycoto" | ||
| ], | ||
| "dependencies": { | ||
| "@backstage/core-plugin-api": "^1.10.9", |
There was a problem hiding this comment.
should we couple the common package to the frontend-only package?
We could the import to import type { OAuthScope } and move @backstage/core-plugin-api to devDependencies to avoid pulling frontend code into the backend.
There was a problem hiding this comment.
The common pckg is for bth the BE and frontend. Removing this dependency completely, does not bring much benefits.
| }): AuthTokenDescriptor => { | ||
| // Based on https://docs.github.com/en/enterprise-cloud[@latest](https://github.com/latest)/admin/managing-your-enterprise-account/changing-the-url-for-your-enterprise | ||
| // the GH URL should always contain github.com. | ||
| const provider = repoUrl.includes('github.com') ? 'github' : 'gitlab'; |
There was a problem hiding this comment.
gitlab.com is a very improbable provider
we should support all gitlab* enterprise providers, it's odd to "default" to gitlab.com
There was a problem hiding this comment.
This call is about choosing the provider, either github or gitlab, no matter if upstream or enterprise-deployed.
A custom-deployed gitlab can have a different URL, not containing the gitlab.com which this particular function is aligned with.
The standard and commonly used component for SCM URL selection (the RepoUrlPicker) does not provide the provider, just the URL (or I can not find otherwise). All suggestions I could find lead to parsing the URL :-(
To our luck (so far), we support gitlab-* and github only.
The GitHub Enterprise URL should always contain github.com (based on the documentation).
So the rest of the URLs lead to the the gitlab auth provider.
If we need to add an additional provider (like Azure), we will probably need to avoid this Backstage-standard component and reimplement it on our own.
In a follow-up, I will focus on the RH gitlab and defining steps needed towards integrating with it. The rcent understanding is that this function should not be affected.
|
|
||
| 3. Start the development environment with just the plugin loaded: | ||
|
|
||
| **GitHub OAuth**: Create a GitHub OAuth application](https://github.com/settings/developers). |
mareklibra
force-pushed
the
FLPATH-3353.gitlab
branch
from
435f8b8 to
b28abea
Compare
|
So far tested on gitlab.com only. I believe just the authentication part will be affected by that, not the plugin code itself. |
…#2419) * feat(x2a): GitLab authentication and token for jobs Signed-off-by: Marek Libra <marek.libra@gmail.com> * documentation * review --------- Signed-off-by: Marek Libra <marek.libra@gmail.com>
2 participants
Fixes: FLPATH-3353
The user can be authenticated against gitlab.com.
In addition. the Create Conversion Project template can handle gitlab.com for source and target repos and passing down correct tokens.
No matter what provider is selected to authenticate the user for the app (guest, GitHub, something else), the template collects (pops-up relevant dialog) GitLab credentials to issue the correct token.
TODO: