paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

feat(gerrit): make the gitilesBaseUrl config mandatory

Browse Source 
Signed-off-by: Niklas Aronsson <niklasar@axis.com>
This commit is contained in:
Niklas Aronsson
2024-06-11 09:04:49 +02:00
parent 9caeec3ef1
commit 662dce80fe
16 changed files with 71 additions and 186 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/polite-emus-invent.md
+8
View File
@@ -0,0 +1,8 @@
---
'@backstage/backend-defaults': minor
---
**BREAKING**: The `workdir` argument have been removed from The `GerritUrlReader` constructor.
**BREAKING**: The Gerrit `readTree` implementation will now only use the Gitiles api. Support
for using git to clone the repo has been removed.
.changeset/wicked-dingos-heal.md
+7
View File
@@ -0,0 +1,7 @@
---
'@backstage/integration': minor
---
**BREAKING**: `gitilesBaseUrl` is now mandatory for the Gerrit integration. The
ability to override this requirement using the `DISABLE_GERRIT_GITILES_REQUIREMENT`
environment variable has been removed.
docs/integrations/gerrit/locations.md
+2 -6
View File
@@ -19,9 +19,9 @@ To use this integration, add at least one Gerrit configuration to your root `app
integrations:
gerrit:
- host: gerrit.company.com
gitilesBaseUrl: https://gerrit.company.com/gitiles
baseUrl: https://gerrit.company.com/gerrit
cloneUrl: https://gerrit.company.com/clone
gitilesBaseUrl: https://gerrit.company.com/gitiles
username: ${GERRIT_USERNAME}
password: ${GERRIT_PASSWORD}
```
@@ -31,16 +31,12 @@ you can list the Gerrit instances you want to fetch data from. Each entry is
a structure with up to six elements:
- `host`: The host of the Gerrit instance, e.g. `gerrit.company.com`.
- `gitilesBaseUrl`: The base url of the Gitiles instance.
- `baseUrl` (optional): Needed if the Gerrit instance is not reachable at
the base of the `host` option (e.g. `https://gerrit.company.com`) set the
address here. This is the address that you would open in a browser.
- `cloneUrl` (optional): The base URL for HTTP clones. Will default to `baseUrl` if
not set. The address used to clone a repo is the `cloneUrl` plus the repo name.
- `gitilesBaseUrl` (optional): This is needed for creating a valid user-friendly URL
that can be used for browsing the content of the provider. If not set a default
value will be created in the same way as the `baseUrl` option. There is no
requirement to have Gitiles for the Backstage Gerrit integration but without it
some links in the Backstage UI will be broken.
- `username` (optional): The Gerrit username to use in API requests. If
neither a username nor password are supplied, anonymous access will be used.
- `password` (optional): The password or http token for the Gerrit user.
packages/backend-defaults/api-report-urlReader.md
-1
View File
@@ -217,7 +217,6 @@ export class GerritUrlReader implements UrlReaderService {
deps: {
treeResponseFactory: ReadTreeResponseFactory;
},
workDir: string,
);
// (undocumented)
static factory: ReaderFactory;
packages/backend-defaults/src/entrypoints/urlReader/lib/GerritUrlReader.test.ts
+11 -74
View File
@@ -32,10 +32,7 @@ import fs from 'fs-extra';
import path from 'path';
import { UrlReaderPredicateTuple } from './types';
import { DefaultReadTreeResponseFactory } from './tree';
import {
GITILES_BASE_URL_DEPRECATION_MESSSAGE,
GerritUrlReader,
} from './GerritUrlReader';
import { GerritUrlReader } from './GerritUrlReader';
import getRawBody from 'raw-body';
const mockDir = createMockDirectory({ mockOsTmpDir: true });
@@ -61,11 +58,11 @@ const gerritProcessor = new GerritUrlReader(
readGerritIntegrationConfig(
new ConfigReader({
host: 'gerrit.com',
gitilesBaseUrl: 'https://gerrit.com/gitiles',
}),
),
),
{ treeResponseFactory },
'/tmp',
);
// Gerrit processor with a gitilesBaseUrl configured.
@@ -80,7 +77,6 @@ const gerritProcessorWithGitiles = new GerritUrlReader(
),
),
{ treeResponseFactory },
'/tmp',
);
const createReader = (config: JsonObject): UrlReaderPredicateTuple[] => {
@@ -112,7 +108,12 @@ describe.skip('GerritUrlReader', () => {
it('creates a reader.', () => {
const readers = createReader({
integrations: {
gerrit: [{ host: 'gerrit.com' }],
gerrit: [
{
host: 'gerrit.com',
gitilesBaseUrl: 'https://gerrit.com/gitiles',
},
],
},
});
expect(readers).toHaveLength(1);
@@ -126,33 +127,12 @@ describe.skip('GerritUrlReader', () => {
});
});
describe('handle optional gitilesBaseUrl deprecation', () => {
it('should throw if gitilesBaseUrl is not set.', () => {
process.env = env;
expect(() =>
createReader({
integrations: {
gerrit: [{ host: 'gerrit.com' }],
},
}),
).toThrow(GITILES_BASE_URL_DEPRECATION_MESSSAGE);
});
it('should not throw if gitilesBaseUrl requirement is overridden.', () => {
process.env = { ...env, DISABLE_GERRIT_GITILES_REQUIREMENT: '1' };
expect(() =>
createReader({
integrations: {
gerrit: [{ host: 'gerrit.com' }],
},
}),
).not.toThrow();
});
});
describe('predicates without Gitiles', () => {
const readers = createReader({
integrations: {
gerrit: [{ host: 'gerrit.com' }],
gerrit: [
{ host: 'gerrit.com', gitilesBaseUrl: 'https://gerrit.com/gitiles' },
],
},
});
const predicate = readers[0].predicate;
@@ -354,29 +334,6 @@ describe.skip('GerritUrlReader', () => {
expect(cloneMock).not.toHaveBeenCalled();
});
it('reads the wanted files correctly using git clone.', async () => {
worker.use(
rest.get(branchAPIUrl, (_, res, ctx) => {
return res(ctx.status(200), ctx.body(branchAPIresponse));
}),
);
const response = await gerritProcessor.readTree(treeUrl);
expect(response.etag).toBe(etag);
const files = await response.files();
expect(files.length).toBe(2);
const docsYaml = await files[0].content();
expect(docsYaml.toString()).toBe(mkdocsContent);
const mdFile = await files[1].content();
expect(mdFile.toString()).toBe(mdContent);
expect(cloneMock).toHaveBeenCalled();
});
it('throws NotModifiedError for matching etags.', async () => {
worker.use(
rest.get(branchAPIUrl, (_, res, ctx) => {
@@ -432,25 +389,5 @@ describe.skip('GerritUrlReader', () => {
expect(cloneMock).not.toHaveBeenCalled();
});
it('should returns wanted files with a subpath using git clone', async () => {
worker.use(
rest.get(branchAPIUrl, (_, res, ctx) => {
return res(ctx.status(200), ctx.body(branchAPIresponse));
}),
);
const response = await gerritProcessor.readTree(`${treeUrl}/docs`);
expect(response.etag).toBe(etag);
const files = await response.files();
expect(files.length).toBe(1);
const mdFile = await files[0].content();
expect(mdFile.toString()).toBe(mdContent);
expect(cloneMock).toHaveBeenCalled();
});
});
});
packages/backend-defaults/src/entrypoints/urlReader/lib/GerritUrlReader.ts
+3 -83
View File
@@ -23,20 +23,13 @@ import {
UrlReaderSearchResponse,
} from '@backstage/backend-plugin-api';
import { Base64Decode } from 'base64-stream';
import concatStream from 'concat-stream';
import fs from 'fs-extra';
import fetch, { Response } from 'node-fetch';
import os from 'os';
import { join as joinPath } from 'path';
import { Readable, pipeline as pipelineCb } from 'stream';
import tar from 'tar';
import { promisify } from 'util';
import { Readable } from 'stream';
import {
GerritIntegration,
ScmIntegrations,
buildGerritGitilesArchiveUrl,
getGerritBranchApiUrl,
getGerritCloneRepoUrl,
getGerritFileContentsApiUrl,
getGerritRequestOptions,
parseGerritGitilesUrl,
@@ -44,18 +37,6 @@ import {
} from '@backstage/integration';
import { NotFoundError, NotModifiedError } from '@backstage/errors';
import { ReadTreeResponseFactory, ReaderFactory } from './types';
import { Git } from './git';
const pipeline = promisify(pipelineCb);
export const GITILES_BASE_URL_DEPRECATION_MESSSAGE = `A gitilesBaseUrl must be provided \
for the gerrit integration to work. You can disable this check by setting \
DISABLE_GERRIT_GITILES_REQUIREMENT=1 but this will be removed in a future release. If you \
are not able to use the gitiles gerrit plugin, please open an issue towards \
https://github.com/backstage/backstage`;
const createTemporaryDirectory = async (workDir: string): Promise<string> =>
await fs.mkdtemp(joinPath(workDir, '/gerrit-clone-'));
/**
* Implements a {@link @backstage/backend-plugin-api#UrlReaderService} for files in Gerrit.
@@ -83,20 +64,8 @@ export class GerritUrlReader implements UrlReaderService {
if (!integrations.gerrit) {
return [];
}
const workDir =
config.getOptionalString('backend.workingDirectory') ?? os.tmpdir();
return integrations.gerrit.list().map(integration => {
if (
integration.config.gitilesBaseUrl === integration.config.baseUrl &&
process.env.DISABLE_GERRIT_GITILES_REQUIREMENT === undefined
) {
throw new Error(GITILES_BASE_URL_DEPRECATION_MESSSAGE);
}
const reader = new GerritUrlReader(
integration,
{ treeResponseFactory },
workDir,
);
const reader = new GerritUrlReader(integration, { treeResponseFactory });
const predicate = (url: URL) => {
const gitilesUrl = new URL(integration.config.gitilesBaseUrl!);
// If gitilesUrl is not specified it will default to
@@ -110,7 +79,6 @@ export class GerritUrlReader implements UrlReaderService {
constructor(
private readonly integration: GerritIntegration,
private readonly deps: { treeResponseFactory: ReadTreeResponseFactory },
private readonly workDir: string,
) {}
async read(url: string): Promise<Buffer> {
@@ -194,12 +162,7 @@ export class GerritUrlReader implements UrlReaderService {
throw new NotModifiedError();
}
if (
this.integration.config.gitilesBaseUrl !== this.integration.config.baseUrl
) {
return this.readTreeFromGitiles(url, branchInfo.revision, options);
}
return this.readTreeFromGitClone(url, branchInfo.revision, options);
return this.readTreeFromGitiles(url, branchInfo.revision, options);
}
async search(): Promise<UrlReaderSearchResponse> {
@@ -211,49 +174,6 @@ export class GerritUrlReader implements UrlReaderService {
return `gerrit{host=${host},authed=${Boolean(password)}}`;
}
private async readTreeFromGitClone(
url: string,
revision: string,
options?: UrlReaderReadTreeOptions,
) {
const { filePath } = parseGerritGitilesUrl(this.integration.config, url);
const git = Git.fromAuth({
username: this.integration.config.username,
password: this.integration.config.password,
});
const tempDir = await createTemporaryDirectory(this.workDir);
const cloneUrl = getGerritCloneRepoUrl(this.integration.config, url);
try {
// The "fromTarArchive" function will strip the top level directory so
// an additional directory level is created when we clone.
await git.clone({
url: cloneUrl,
dir: joinPath(tempDir, 'repo'),
ref: revision,
depth: 1,
});
const data = await new Promise<Buffer>(async resolve => {
await pipeline(
tar.create({ cwd: tempDir }, ['']),
concatStream(resolve),
);
});
const tarArchive = Readable.from(data);
return await this.deps.treeResponseFactory.fromTarArchive({
stream: tarArchive,
subpath: filePath === '/' ? undefined : filePath,
etag: revision,
filter: options?.filter,
});
} catch (error) {
throw new Error(`Could not clone ${cloneUrl}: ${error}`);
} finally {
await fs.rm(tempDir, { recursive: true, force: true });
}
}
private async readTreeFromGitiles(
url: string,
revision: string,
packages/integration/api-report.md
+1 -1
View File
@@ -326,7 +326,7 @@ export type GerritIntegrationConfig = {
host: string;
baseUrl?: string;
cloneUrl?: string;
gitilesBaseUrl?: string;
gitilesBaseUrl: string;
username?: string;
password?: string;
};
packages/integration/config.d.ts
Vendored
+5
View File
@@ -151,6 +151,11 @@ export interface Config {
* @visibility frontend
*/
baseUrl?: string;
/**
* The gitiles base url.
* @visibility frontend
*/
gitilesBaseUrl: string;
/**
* The base url for cloning repos.
* @visibility frontend
packages/integration/src/gerrit/GerritIntegration.test.ts
+2
View File
@@ -27,6 +27,8 @@ describe('GerritIntegration', () => {
host: 'gerrit-review.example.com',
username: 'gerrituser',
baseUrl: 'https://gerrit-review.example.com/gerrit',
gitilesBaseUrl:
'https://gerrit-review.example.com/gerrit/plugins/gitiles',
password: '1234',
},
],
packages/integration/src/gerrit/config.test.ts
+8 -4
View File
@@ -76,13 +76,14 @@ describe('readGerritIntegrationConfig', () => {
const output = readGerritIntegrationConfig(
buildConfig({
host: 'a.com',
gitilesBaseUrl: 'https://a.com/gerrit/plugins/gitiles',
}),
);
expect(output).toEqual({
host: 'a.com',
baseUrl: 'https://a.com',
cloneUrl: 'https://a.com',
gitilesBaseUrl: 'https://a.com',
gitilesBaseUrl: 'https://a.com/gerrit/plugins/gitiles',
username: undefined,
password: undefined,
});
@@ -106,6 +107,7 @@ describe('readGerritIntegrationConfig', () => {
await buildFrontendConfig({
host: 'a.com',
baseUrl: 'https://a.com/gerrit',
gitilesBaseUrl: 'https://a.com/gerrit/plugins/gitiles',
username: 'u',
password: 'p',
}),
@@ -114,7 +116,7 @@ describe('readGerritIntegrationConfig', () => {
host: 'a.com',
baseUrl: 'https://a.com/gerrit',
cloneUrl: 'https://a.com/gerrit',
gitilesBaseUrl: 'https://a.com',
gitilesBaseUrl: 'https://a.com/gerrit/plugins/gitiles',
});
});
});
@@ -130,12 +132,14 @@ describe('readGerritIntegrationConfigs', () => {
{
host: 'a.com',
baseUrl: 'https://a.com/api',
gitilesBaseUrl: 'https://a.com/gerrit/plugins/gitiles',
username: 'u',
password: 'p',
},
{
host: 'b.com',
baseUrl: 'https://b.com/api',
gitilesBaseUrl: 'https://b.com/gerrit/plugins/gitiles',
},
]),
);
@@ -144,7 +148,7 @@ describe('readGerritIntegrationConfigs', () => {
host: 'a.com',
baseUrl: 'https://a.com/api',
cloneUrl: 'https://a.com/api',
gitilesBaseUrl: 'https://a.com',
gitilesBaseUrl: 'https://a.com/gerrit/plugins/gitiles',
username: 'u',
password: 'p',
},
@@ -152,7 +156,7 @@ describe('readGerritIntegrationConfigs', () => {
host: 'b.com',
baseUrl: 'https://b.com/api',
cloneUrl: 'https://b.com/api',
gitilesBaseUrl: 'https://b.com',
gitilesBaseUrl: 'https://b.com/gerrit/plugins/gitiles',
username: undefined,
password: undefined,
},
packages/integration/src/gerrit/config.ts
+5 -6
View File
@@ -45,12 +45,11 @@ export type GerritIntegrationConfig = {
cloneUrl?: string;
/**
* Optional base url for Gitiles. This is needed for creating a valid
* Base url for Gitiles. This is needed for creating a valid
* user-friendly url that can be used for browsing the content of the
* provider. If not set a default value will be created in the same way
* as the "baseUrl" option.
* provider.
*/
gitilesBaseUrl?: string;
gitilesBaseUrl: string;
/**
* The username to use for requests to gerrit.
@@ -76,7 +75,7 @@ export function readGerritIntegrationConfig(
const host = config.getString('host');
let baseUrl = config.getOptionalString('baseUrl');
let cloneUrl = config.getOptionalString('cloneUrl');
let gitilesBaseUrl = config.getOptionalString('gitilesBaseUrl');
let gitilesBaseUrl = config.getString('gitilesBaseUrl');
const username = config.getOptionalString('username');
const password = config.getOptionalString('password')?.trim();
@@ -92,7 +91,7 @@ export function readGerritIntegrationConfig(
throw new Error(
`Invalid Gerrit integration config, '${cloneUrl}' is not a valid cloneUrl`,
);
} else if (gitilesBaseUrl && !isValidUrl(gitilesBaseUrl)) {
} else if (!isValidUrl(gitilesBaseUrl)) {
throw new Error(
`Invalid Gerrit integration config, '${gitilesBaseUrl}' is not a valid gitilesBaseUrl`,
);
packages/integration/src/gerrit/core.test.ts
+2
View File
@@ -134,9 +134,11 @@ describe('gerrit core', () => {
host: 'gerrit.com',
username: 'U',
password: 'P',
gitilesBaseUrl: 'https://gerrit.com/gerrit/plugins/gitiles',
};
const anonymousRequest: GerritIntegrationConfig = {
host: 'gerrit.com',
gitilesBaseUrl: 'https://gerrit.com/gerrit/plugins/gitiles',
};
expect(
(getGerritRequestOptions(authRequest).headers as any).Authorization,
plugins/catalog-backend-module-gerrit/src/providers/GerritEntityProvider.test.ts
+3
View File
@@ -166,9 +166,11 @@ describe('GerritEntityProvider', () => {
gerrit: [
{
host: 'gerrit1.com',
gitilesBaseUrl: 'https://gerrit1.com/gitiles',
},
{
host: 'gerrit2.com',
gitilesBaseUrl: 'https://gerrit2.com/gitiles',
},
],
},
@@ -201,6 +203,7 @@ describe('GerritEntityProvider', () => {
gerrit: [
{
host: 'gerrit1.com',
gitilesBaseUrl: 'https://gerrit1.com/gitiles',
},
],
},
plugins/scaffolder-backend-module-gerrit/src/actions/gerrit.test.ts
+5 -4
View File
@@ -42,6 +42,7 @@ describe('publish:gerrit', () => {
gerrit: [
{
host: 'gerrithost.org',
gitilesBaseUrl: 'https://gerrithost.org/gitiles',
username: 'gerrituser',
password: 'usertoken',
},
@@ -133,7 +134,7 @@ describe('publish:gerrit', () => {
);
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/master',
'https://gerrithost.org/gitiles/repo/+/refs/heads/master',
);
});
@@ -184,7 +185,7 @@ describe('publish:gerrit', () => {
);
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/master',
'https://gerrithost.org/gitiles/repo/+/refs/heads/master',
);
});
@@ -235,7 +236,7 @@ describe('publish:gerrit', () => {
);
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/master',
'https://gerrithost.org/gitiles/repo/+/refs/heads/master',
);
});
@@ -286,7 +287,7 @@ describe('publish:gerrit', () => {
);
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/main',
'https://gerrithost.org/gitiles/repo/+/refs/heads/main',
);
});
plugins/scaffolder-backend-module-gerrit/src/actions/gerritReview.examples.test.ts
+7 -6
View File
@@ -35,6 +35,7 @@ describe('publish:gerrit:review', () => {
gerrit: [
{
host: 'gerrithost.org',
gitilesBaseUrl: 'https://gerrithost.org/gitiles',
username: 'gerrituser',
password: 'usertoken',
},
@@ -77,7 +78,7 @@ describe('publish:gerrit:review', () => {
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/master',
'https://gerrithost.org/gitiles/repo/+/refs/heads/master',
);
expect(mockContext.output).toHaveBeenCalledWith(
'reviewUrl',
@@ -109,7 +110,7 @@ describe('publish:gerrit:review', () => {
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/master',
'https://gerrithost.org/gitiles/repo/+/refs/heads/master',
);
expect(mockContext.output).toHaveBeenCalledWith(
'reviewUrl',
@@ -141,7 +142,7 @@ describe('publish:gerrit:review', () => {
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/master',
'https://gerrithost.org/gitiles/repo/+/refs/heads/master',
);
expect(mockContext.output).toHaveBeenCalledWith(
'reviewUrl',
@@ -170,7 +171,7 @@ describe('publish:gerrit:review', () => {
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/develop',
'https://gerrithost.org/gitiles/repo/+/refs/heads/develop',
);
expect(mockContext.output).toHaveBeenCalledWith(
'reviewUrl',
@@ -199,7 +200,7 @@ describe('publish:gerrit:review', () => {
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/master',
'https://gerrithost.org/gitiles/repo/+/refs/heads/master',
);
expect(mockContext.output).toHaveBeenCalledWith(
'reviewUrl',
@@ -231,7 +232,7 @@ describe('publish:gerrit:review', () => {
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/develop',
'https://gerrithost.org/gitiles/repo/+/refs/heads/develop',
);
expect(mockContext.output).toHaveBeenCalledWith(
'reviewUrl',
plugins/scaffolder-backend-module-gerrit/src/actions/gerritReview.test.ts
+2 -1
View File
@@ -33,6 +33,7 @@ describe('publish:gerrit:review', () => {
gerrit: [
{
host: 'gerrithost.org',
gitilesBaseUrl: 'https://gerrithost.org/gitiles',
username: 'gerrituser',
password: 'usertoken',
},
@@ -104,7 +105,7 @@ describe('publish:gerrit:review', () => {
expect(mockContext.output).toHaveBeenCalledWith(
'repoContentsUrl',
'https://gerrithost.org/repo/+/refs/heads/master',
'https://gerrithost.org/gitiles/repo/+/refs/heads/master',
);
expect(mockContext.output).toHaveBeenCalledWith(
'reviewUrl',