From 97f0e7fa85052dd14bfea98fb5efd619de1b75d6 Mon Sep 17 00:00:00 2001 From: Tuan Anh Tran Date: Tue, 25 Apr 2023 00:47:40 +0700 Subject: [PATCH 1/4] scaffolder: Fix GitLab repo picker in case workspace is nested subgroup Signed-off-by: Tuan Anh Tran --- .../fields/RepoUrlPicker/RepoUrlPicker.tsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index 9bd904de13..ba047baaed 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -128,17 +128,19 @@ export const RepoUrlPicker = (props: RepoUrlPickerProps) => { return; } - const [encodedHost, encodedWorkspace, encodedRepoName] = [ - state.host, - workspace, - state.repoName, - ].map(encodeURIComponent); + // previously, we were encodeURI for state.host, workspace and state.repoName separately. + // That created an issue where GitLab workspace can be nested like groupA/subgroupB + // when we encodeURi separately and then join, the URL will be malformed and + // resulting in 400 request error from GitLab API + const url = encodeURIComponent( + [state.host, workspace, state.repoName].join(), + ); // user has requested that we use the users credentials // so lets grab them using the scmAuthApi and pass through // any additional scopes from the ui:options const { token } = await scmAuthApi.getCredentials({ - url: `https://${encodedHost}/${encodedWorkspace}/${encodedRepoName}`, + url, additionalScope: { repoWrite: true, customScopes: requestUserCredentials.additionalScopes, From f93cde3444fd37405bba815344c0dd3dcec0affd Mon Sep 17 00:00:00 2001 From: Tuan Anh Tran Date: Tue, 25 Apr 2023 01:28:46 +0700 Subject: [PATCH 2/4] Add tests for case Gitlab workspace is nested subgroup Signed-off-by: Tuan Anh Tran --- .../RepoUrlPicker/RepoUrlPicker.test.tsx | 58 +++++++++++++++++++ .../fields/RepoUrlPicker/RepoUrlPicker.tsx | 6 +- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx index c36374f939..82803436a5 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx @@ -43,6 +43,11 @@ describe('RepoUrlPicker', () => { type: 'bitbucketServer', title: 'server.bitbucket.org', }, + { + host: 'gitlab.example.com', + type: 'gitlab', + title: 'gitlab.example.com', + }, ], }), }; @@ -184,6 +189,59 @@ describe('RepoUrlPicker', () => { secrets: { testKey: 'abc123' }, }); }); + it('should call the scmAuthApi with the correct params if workspace is nested', async () => { + const SecretsComponent = () => { + const { secrets } = useTemplateSecrets(); + return ( +
{JSON.stringify({ secrets })}
+ ); + }; + const { getAllByRole, getByTestId } = await renderInTestApp( + + +
+ + + , + ); + + const [projectInput, repoInput] = getAllByRole('textbox'); + + await act(async () => { + fireEvent.change(projectInput, { + target: { value: 'backstage/mysubgroup' }, + }); + fireEvent.change(repoInput, { target: { value: 'repo123' } }); + + // need to wait for the debounce to finish + await new Promise(resolve => setTimeout(resolve, 600)); + }); + + expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({ + url: 'https://gitlab.example.com/backstage/mysubgroup/repo123', + additionalScope: { + repoWrite: true, + }, + }); + }); it('should call the scmAuthApi with the correct params if only a project is set', async () => { const SecretsComponent = () => { const { secrets } = useTemplateSecrets(); diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index ba047baaed..57cebb3d96 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -132,15 +132,15 @@ export const RepoUrlPicker = (props: RepoUrlPickerProps) => { // That created an issue where GitLab workspace can be nested like groupA/subgroupB // when we encodeURi separately and then join, the URL will be malformed and // resulting in 400 request error from GitLab API - const url = encodeURIComponent( - [state.host, workspace, state.repoName].join(), + const [encodedHost, encodedRepoName] = [state.host, state.repoName].map( + encodeURIComponent, ); // user has requested that we use the users credentials // so lets grab them using the scmAuthApi and pass through // any additional scopes from the ui:options const { token } = await scmAuthApi.getCredentials({ - url, + url: `https://${encodedHost}/${workspace}/${encodedRepoName}`, additionalScope: { repoWrite: true, customScopes: requestUserCredentials.additionalScopes, From fd1f4f67063a63a07ab05976a608aebd7e28b4a2 Mon Sep 17 00:00:00 2001 From: Tuan Anh Tran Date: Tue, 25 Apr 2023 01:32:43 +0700 Subject: [PATCH 3/4] Remove unused getByTestId in repo picker test Signed-off-by: Tuan Anh Tran --- .../src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx index 82803436a5..4b44a4fa3e 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx @@ -196,7 +196,7 @@ describe('RepoUrlPicker', () => {
{JSON.stringify({ secrets })}
); }; - const { getAllByRole, getByTestId } = await renderInTestApp( + const { getAllByRole } = await renderInTestApp( Date: Tue, 25 Apr 2023 21:54:23 +0700 Subject: [PATCH 4/4] Add changeset Signed-off-by: Tuan Anh Tran --- .changeset/beige-tigers-matter.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/beige-tigers-matter.md diff --git a/.changeset/beige-tigers-matter.md b/.changeset/beige-tigers-matter.md new file mode 100644 index 0000000000..248ce1fbf9 --- /dev/null +++ b/.changeset/beige-tigers-matter.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': patch +--- + +Fix case GitLab workspace is a nested subgroup