Skip to content

Commit

Permalink
feat(api): identify gitlab instances by hostname
Browse files Browse the repository at this point in the history
gitlab-backend router and processor no longer use a generated index number to identify the instance but the hostname of the gitlab instance.
closes #118

BREAKING CHANGE: Annotations `gitlab.com/instance` require the hostname of the gitlab instance, replacing index numbers. The processor now creates annotations with hostnames. GitLabCIApiRef calls using /api/gitlab/{number}/ are not resolved and return 404 not found.

Signed-off-by: Manuel Stein <[email protected]>
  • Loading branch information
manuelstein committed Mar 8, 2023
1 parent 58ea43f commit dff875d
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 64 deletions.
22 changes: 16 additions & 6 deletions packages/gitlab-backend/src/processor/processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ describe('Processor', () => {
expect(entity.metadata?.annotations?.[GITLAB_PROJECT_SLUG]).toEqual(
'backstage/backstage'
);
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual('0');
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual(
'my.custom-gitlab.com'
);
});

it('Processor creates the right annotation for second instance', async () => {
Expand All @@ -74,7 +76,9 @@ describe('Processor', () => {
expect(entity.metadata?.annotations?.[GITLAB_PROJECT_SLUG]).toEqual(
'backstage/backstage'
);
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual('1');
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual(
'my.second-custom-gitlab.com'
);
});

it('Processor creates the right annotation for old gitlab instance', async () => {
Expand All @@ -98,7 +102,9 @@ describe('Processor', () => {
expect(entity.metadata?.annotations?.[GITLAB_PROJECT_SLUG]).toEqual(
'backstage/backstage'
);
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual('0');
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual(
'my.custom-gitlab.com'
);
});

it('The processor does not update GITLAB_PROJECT_SLUG if the annotations GITLAB_PROJECT_ID or GITLAB_PROJECT_SLUG exist', async () => {
Expand Down Expand Up @@ -126,7 +132,9 @@ describe('Processor', () => {
expect(
entity.metadata?.annotations?.[GITLAB_PROJECT_SLUG]
).toBeUndefined();
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual('0');
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual(
'my.custom-gitlab.com'
);

expect(entity.metadata?.annotations?.[GITLAB_PROJECT_ID]).toEqual(
projectId
Expand All @@ -141,7 +149,7 @@ describe('Processor', () => {
metadata: {
name: 'backstage',
annotations: {
[GITLAB_INSTANCE]: '1',
[GITLAB_INSTANCE]: 'my.custom-gitlab.com',
},
},
};
Expand All @@ -157,7 +165,9 @@ describe('Processor', () => {
expect(entity.metadata?.annotations?.[GITLAB_PROJECT_SLUG]).toEqual(
'backstage/backstage'
);
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual('1');
expect(entity.metadata?.annotations?.[GITLAB_INSTANCE]).toEqual(
'my.custom-gitlab.com'
);

expect(
entity.metadata?.annotations?.[GITLAB_PROJECT_ID]
Expand Down
82 changes: 40 additions & 42 deletions packages/gitlab-backend/src/processor/processor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { Entity } from '@backstage/catalog-model';
import { Config } from '@backstage/config';
import { readGitLabIntegrationConfigs } from '@backstage/integration';
import {
readGitLabIntegrationConfigs,
GitLabIntegrationConfig,
} from '@backstage/integration';
import { getProjectPath } from './urls';
import type {
CatalogProcessor,
Expand All @@ -12,7 +15,6 @@ import {
GITLAB_PROJECT_SLUG,
} from './../annotations';
import type { LocationSpec } from '@backstage/plugin-catalog-common';
import type { GitLabIntegrationConfig } from '@backstage/integration';

/** @public */
export class GitlabFillerProcessor implements CatalogProcessor {
Expand Down Expand Up @@ -41,57 +43,53 @@ export class GitlabFillerProcessor implements CatalogProcessor {
location: LocationSpec,
_emit: CatalogProcessorEmit
): Promise<Entity> {
// Check if it is a GitLab Host
if (this.isValidLocation(location) && this.isAllowedEntity(entity)) {
if (!entity.metadata.annotations) entity.metadata.annotations = {};
// Generate Project Slug if not specified
if (
!entity.metadata.annotations?.[GITLAB_PROJECT_ID] &&
!entity.metadata.annotations?.[GITLAB_PROJECT_SLUG]
) {
entity.metadata.annotations[GITLAB_PROJECT_SLUG] =
getProjectPath(location.target);
}
// Discover gitlab instance
if (!entity.metadata.annotations?.[GITLAB_INSTANCE]) {
entity.metadata.annotations[GITLAB_INSTANCE] =
this.getGitlabInstance(location.target);
// Check if we should process its kind first
if (this.isAllowedEntity(entity)) {
// Check if it has a GitLab integration
const gitlabInstance = this.getGitlabInstance(location.target);
if (gitlabInstance) {
if (!entity.metadata.annotations)
entity.metadata.annotations = {};

// Set GitLab Instance
if (!entity.metadata.annotations?.[GITLAB_INSTANCE]) {
entity.metadata.annotations![GITLAB_INSTANCE] =
gitlabInstance;
}

// Generate Project Slug from location URL if neither Project ID nor Project Slug are specified
if (
!entity.metadata.annotations?.[GITLAB_PROJECT_ID] &&
!entity.metadata.annotations?.[GITLAB_PROJECT_SLUG]
) {
entity.metadata.annotations![GITLAB_PROJECT_SLUG] =
getProjectPath(location.target);
}
}
}

return entity;
}

private getGitlabInstance(target: string): string {
const url = new URL(target);

// TODO: handle the possibility to have a baseUrl with a path
const index = this.gitLabIntegrationsConfig.findIndex(
({ baseUrl }) => baseUrl === url.origin
);

if (index < 0) return '0';

return index.toString(10);
}

private isAllowedEntity(entity: Entity): boolean {
return this.allowedKinds.has(entity.kind.toLowerCase());
}

private isValidLocation({ target, type }: LocationSpec): boolean {
if (type !== 'url') return false;

private getGitlabInstance(target: string): string | undefined {
let url: URL;
try {
url = new URL(target);
} catch (e) {
return false;
return undefined;
}
// Check if it is valid gitlab Host
return this.gitLabIntegrationsConfig.some((config) => {
const baseUrl = new URL(config.baseUrl);
return url.origin === baseUrl.origin;

const gitlabConfig = this.gitLabIntegrationsConfig.find((config) => {
const baseUrl = config.baseUrl
? new URL(config.baseUrl)
: new URL(`https://${config.host}`);
return baseUrl.origin === url.origin;
});

return gitlabConfig?.host;
}

private isAllowedEntity(entity: Entity): boolean {
return this.allowedKinds.has(entity.kind.toLowerCase());
}
}
14 changes: 10 additions & 4 deletions packages/gitlab-backend/src/service/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ describe('createRouter', () => {
const agent = request.agent(app);
// this is set to let msw pass test requests through the mock server
agent.set('User-Agent', 'supertest');
const response = await agent.get('/api/gitlab/0/projects/434');
const response = await agent.get(
'/api/gitlab/non-existing-example.com/projects/434'
);
expect(response.status).toEqual(200);
expect(response.body).toEqual({
headers: {
Expand All @@ -96,7 +98,9 @@ describe('createRouter', () => {
const agent = request.agent(app);
// this is set to let msw pass test requests through the mock server
agent.set('User-Agent', 'supertest');
const response = await agent.get('/api/gitlab/1/projects/434');
const response = await agent.get(
'/api/gitlab/non-existing-example-2.com/projects/434'
);
expect(response.status).toEqual(200);
expect(response.body).toEqual({
headers: {
Expand Down Expand Up @@ -125,7 +129,7 @@ describe('createRouter', () => {
]) {
// @ts-ignore
const response = await agent?.[method](
'/api/gitlab/1/projects/434'
'/api/gitlab/non-existing-example-2.com/projects/434'
);
expect(response.status).toEqual(404);
expect(response.body).toEqual({});
Expand All @@ -136,7 +140,9 @@ describe('createRouter', () => {
const agent = request.agent(app);
// this is set to let msw pass test requests through the mock server
agent.set('User-Agent', 'supertest');
const response = await agent.get('/api/gitlab/3/projects/434');
const response = await agent.get(
'/api/gitlab/does.not.exist/projects/434'
);
expect(response.status).toEqual(404);
expect(response.body).toEqual({});
});
Expand Down
18 changes: 11 additions & 7 deletions packages/gitlab-backend/src/service/router.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { errorHandler } from '@backstage/backend-common';
import { Config } from '@backstage/config';
import { readGitLabIntegrationConfigs } from '@backstage/integration';
import {
readGitLabIntegrationConfigs,
GitLabIntegrationConfig,
} from '@backstage/integration';
import express from 'express';
import Router from 'express-promise-router';
import { Logger } from 'winston';
Expand All @@ -16,16 +19,17 @@ export async function createRouter(
): Promise<express.Router> {
const { logger, config } = options;

const gitlabIntegrations = readGitLabIntegrationConfigs(
config.getConfigArray('integrations.gitlab')
);
const gitlabIntegrations: GitLabIntegrationConfig[] =
readGitLabIntegrationConfigs(
config.getConfigArray('integrations.gitlab')
);

const router = Router();

for (const [i, { apiBaseUrl, token }] of gitlabIntegrations.entries()) {
for (const { host, apiBaseUrl, token } of gitlabIntegrations) {
const apiUrl = new URL(apiBaseUrl);
router.use(
`/${i}`,
`/${host}`,
createProxyMiddleware((_pathname, req) => req.method === 'GET', {
target: apiUrl.origin,
changeOrigin: true,
Expand All @@ -34,7 +38,7 @@ export async function createRouter(
},
logProvider: () => logger,
pathRewrite: {
[`^/api/gitlab/${i}`]: apiUrl.pathname,
[`^/api/gitlab/${host}`]: apiUrl.pathname,
},
onProxyReq: (proxyReq) => {
proxyReq.removeHeader('Authorization');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const LanguagesCard = ({}) => {
const project_slug = gitlabProjectSlug();
const gitlab_instance = gitlabInstance();

const GitlabCIAPI = useApi(GitlabCIApiRef).build(gitlab_instance || '0');
const GitlabCIAPI = useApi(GitlabCIApiRef).build(gitlab_instance || 'gitlab.com');

const { value, loading, error } = useAsync(async (): Promise<Language> => {
const projectDetails: any = await GitlabCIAPI.getProjectDetails(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ const MergeRequestStats = (props: Props) => {
const project_slug = gitlabProjectSlug();
const gitlab_instance = gitlabInstance();

const GitlabCIAPI = useApi(GitlabCIApiRef).build(gitlab_instance || '0');
const GitlabCIAPI = useApi(GitlabCIApiRef).build(gitlab_instance || 'gitlab.com');
const mergeStat: MergeRequestStatsCount = {
avgTimeUntilMerge: 0,
closedCount: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const PeopleCard = ({}) => {
const gitlab_instance = gitlabInstance();
const codeowners_path = gitlabCodeOwnerPath();

const GitlabCIAPI = useApi(GitlabCIApiRef).build(gitlab_instance || '0');
const GitlabCIAPI = useApi(GitlabCIApiRef).build(gitlab_instance || 'gitlab.com');
/* TODO: to change the below logic to get contributors data*/
const { value, loading, error } = useAsync(async (): Promise<{
contributors: PeopleCardEntityData[] | undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const PipelinesTable = ({}) => {
const project_slug = gitlabProjectSlug();
const gitlab_instance = gitlabInstance();

const GitlabCIAPI = useApi(GitlabCIApiRef).build(gitlab_instance || '0');
const GitlabCIAPI = useApi(GitlabCIApiRef).build(gitlab_instance || 'gitlab.com');

const { value, loading, error } = useAsync(async (): Promise<
PipelineObject[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export const ReleasesCard = (props: ReleasesCardProps) => {
const gitlab_instance = gitlabInstance();

const GitlabCIAPI = useApi(GitlabCIApiRef).build(
gitlab_instance || '0'
gitlab_instance || 'gitlab.com'
);
/* TODO: to change the below logic to get contributors data*/
const { value, loading, error } = useAsync(async (): Promise<{
Expand Down

0 comments on commit dff875d

Please sign in to comment.