From 90646b4c028e9a998a992cd399fe11f7300bd3e5 Mon Sep 17 00:00:00 2001 From: Jasper Boeijenga Date: Wed, 11 Oct 2023 11:48:45 +0200 Subject: [PATCH 1/5] Added wrapper around fetching users from gitlab group to make it fail gracefully Signed-off-by: Jasper Boeijenga --- .../src/lib/client.ts | 12 +++----- .../src/lib/types.ts | 6 +++- .../GitlabOrgDiscoveryEntityProvider.ts | 29 ++++++++++--------- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.ts index 722f079a21..c4cdd444cb 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.ts @@ -13,18 +13,19 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -import fetch from 'node-fetch'; import { getGitLabRequestOptions, GitLabIntegrationConfig, } from '@backstage/integration'; +import fetch from 'node-fetch'; import { Logger } from 'winston'; + import { - GitLabGroup, GitLabDescendantGroupsResponse, + GitLabGroup, GitLabGroupMembersResponse, GitLabUser, + PagedResponse, } from './types'; export type CommonListOptions = { @@ -44,11 +45,6 @@ interface UserListOptions extends CommonListOptions { exclude_internal?: boolean | undefined; } -export type PagedResponse = { - items: T[]; - nextPage?: number; -}; - export class GitLabClient { private readonly config: GitLabIntegrationConfig; private readonly logger: Logger; diff --git a/plugins/catalog-backend-module-gitlab/src/lib/types.ts b/plugins/catalog-backend-module-gitlab/src/lib/types.ts index 946317a11a..fa2eb73da6 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/types.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/types.ts @@ -13,9 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import { TaskScheduleDefinition } from '@backstage/backend-tasks'; +export type PagedResponse = { + items: T[]; + nextPage?: number; +}; + export type GitlabGroupDescription = { id: number; web_url: string; diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts index 732f9f9fac..54373791ff 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts @@ -13,31 +13,31 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import { PluginTaskScheduler, TaskRunner } from '@backstage/backend-tasks'; +import { + ANNOTATION_LOCATION, + ANNOTATION_ORIGIN_LOCATION, + Entity, + GroupEntity, + UserEntity, +} from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { GitLabIntegration, ScmIntegrations } from '@backstage/integration'; import { EntityProvider, EntityProviderConnection, } from '@backstage/plugin-catalog-node'; +import { merge } from 'lodash'; import * as uuid from 'uuid'; import { Logger } from 'winston'; + import { GitLabClient, GitlabProviderConfig, paginated, readGitlabConfigs, } from '../lib'; -import { GitLabGroup, GitLabUser } from '../lib/types'; -import { - ANNOTATION_LOCATION, - ANNOTATION_ORIGIN_LOCATION, - Entity, - UserEntity, - GroupEntity, -} from '@backstage/catalog-model'; -import { merge } from 'lodash'; +import { GitLabGroup, GitLabUser, PagedResponse } from '../lib/types'; type Result = { scanned: number; @@ -245,9 +245,12 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { groupRes.scanned++; groupRes.matches.push(group); - const groupUsers = await client.getGroupMembers(group.full_path, [ - 'DIRECT', - ]); + let groupUsers: PagedResponse = { items: [] }; + try { + groupUsers = await client.getGroupMembers(group.full_path, ['DIRECT']); + } catch { + logger.error(`Failed fetching user for group: ${group.full_path}`); + } for (const groupUser of groupUsers.items) { const user = idMappedUser[groupUser.id]; From d732f176101f3f097bca4ef2fe05342e7cef7091 Mon Sep 17 00:00:00 2001 From: Jasper Boeijenga Date: Wed, 11 Oct 2023 11:57:07 +0200 Subject: [PATCH 2/5] Added changeset Signed-off-by: Jasper Boeijenga --- .changeset/fresh-schools-thank.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fresh-schools-thank.md diff --git a/.changeset/fresh-schools-thank.md b/.changeset/fresh-schools-thank.md new file mode 100644 index 0000000000..a5ab99587b --- /dev/null +++ b/.changeset/fresh-schools-thank.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend-module-gitlab': patch +--- + +Added try catch arround fetching gitlab group users to prevent refresh from failing completely while only a select number of groups might not be able to load correctly. From ee87bef45634b3d2daa08edc5a762fc373d86e4a Mon Sep 17 00:00:00 2001 From: Jasper Boeijenga Date: Wed, 11 Oct 2023 12:43:41 +0200 Subject: [PATCH 3/5] Correct spelling in changeset Signed-off-by: Jasper Boeijenga --- .changeset/fresh-schools-thank.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/fresh-schools-thank.md b/.changeset/fresh-schools-thank.md index a5ab99587b..1e32f82051 100644 --- a/.changeset/fresh-schools-thank.md +++ b/.changeset/fresh-schools-thank.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-backend-module-gitlab': patch --- -Added try catch arround fetching gitlab group users to prevent refresh from failing completely while only a select number of groups might not be able to load correctly. +Added try catch around fetching gitlab group users to prevent refresh from failing completely while only a select number of groups might not be able to load correctly. From 1150a8cdeca6d79335f72e13f24b39f50a1872ae Mon Sep 17 00:00:00 2001 From: Jasper Boeijenga Date: Wed, 11 Oct 2023 13:00:35 +0200 Subject: [PATCH 4/5] Add group in the error message from the graphQL query Signed-off-by: Jasper Boeijenga --- plugins/catalog-backend-module-gitlab/src/lib/client.ts | 6 +++++- .../src/providers/GitlabOrgDiscoveryEntityProvider.ts | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.ts index c4cdd444cb..5447dec9d4 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.ts @@ -236,7 +236,11 @@ export class GitLabClient { }, ).then(r => r.json()); if (response.errors) { - throw new Error(`GraphQL errors: ${JSON.stringify(response.errors)}`); + throw new Error( + `GraphQL errors: ${JSON.stringify( + response.errors, + )}, for group: ${groupPath}`, + ); } if (!response.data.group?.groupMembers?.nodes) { diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts index 54373791ff..6dbb0d162b 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts @@ -249,7 +249,7 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { try { groupUsers = await client.getGroupMembers(group.full_path, ['DIRECT']); } catch { - logger.error(`Failed fetching user for group: ${group.full_path}`); + logger.error(`Failed fetching users for group: ${group.full_path}`); } for (const groupUser of groupUsers.items) { From a40a23776804eb448b89ac57bfe44571563a8ff8 Mon Sep 17 00:00:00 2001 From: Jasper Boeijenga Date: Wed, 11 Oct 2023 14:12:02 +0200 Subject: [PATCH 5/5] Applied PR suggestions Signed-off-by: Jasper Boeijenga --- plugins/catalog-backend-module-gitlab/src/lib/client.ts | 6 +----- .../src/providers/GitlabOrgDiscoveryEntityProvider.ts | 6 ++++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.ts index 5447dec9d4..c4cdd444cb 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.ts @@ -236,11 +236,7 @@ export class GitLabClient { }, ).then(r => r.json()); if (response.errors) { - throw new Error( - `GraphQL errors: ${JSON.stringify( - response.errors, - )}, for group: ${groupPath}`, - ); + throw new Error(`GraphQL errors: ${JSON.stringify(response.errors)}`); } if (!response.data.group?.groupMembers?.nodes) { diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts index 6dbb0d162b..affa8a41cf 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts @@ -248,8 +248,10 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { let groupUsers: PagedResponse = { items: [] }; try { groupUsers = await client.getGroupMembers(group.full_path, ['DIRECT']); - } catch { - logger.error(`Failed fetching users for group: ${group.full_path}`); + } catch (e) { + logger.error( + `Failed fetching users for group '${group.full_path}': ${e}`, + ); } for (const groupUser of groupUsers.items) {