diff --git a/.changeset/perfect-bikes-invite.md b/.changeset/perfect-bikes-invite.md new file mode 100644 index 0000000000..96a4ad5659 --- /dev/null +++ b/.changeset/perfect-bikes-invite.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend-module-gitlab': patch +--- + +Fixed an issue in `GitlabDiscoveryEntityProvider` where the fallback branch was taking precedence over the GitLab default branch. diff --git a/plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts b/plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts index 00d4994e94..80aa0370d3 100644 --- a/plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts +++ b/plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts @@ -165,7 +165,11 @@ const httpProjectCatalogDynamic = all_projects_response.map(project => { `${apiBaseUrl}/projects/${path}/repository/files/catalog-info.yaml`, (req, res, ctx) => { const branch = req.url.searchParams.get('ref'); - if (branch === project.default_branch) { + if ( + branch === project.default_branch || + branch === 'main' || + branch === 'develop' + ) { return res(ctx.status(200)); } return res(ctx.status(404, 'Not Found')); diff --git a/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts b/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts index 5a078ab689..e3154eb1ce 100644 --- a/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts +++ b/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts @@ -162,7 +162,7 @@ export const config_github_host: MockObject = { }, }; -export const config_single_integration_branch: MockObject = { +export const config_single_integration: MockObject = { integrations: { gitlab: [ { @@ -178,7 +178,6 @@ export const config_single_integration_branch: MockObject = { 'test-id': { host: 'example.com', group: 'group1', - branch: 'main', skipForkedRepos: false, schedule: { frequency: 'PT30M', @@ -190,6 +189,33 @@ export const config_single_integration_branch: MockObject = { }, }; +export const config_single_integration_specific_branch: MockObject = { + integrations: { + gitlab: [ + { + host: 'example.com', + apiBaseUrl: 'https://example.com/api/v4', + token: '1234', + }, + ], + }, + catalog: { + providers: { + gitlab: { + 'test-id': { + host: 'example.com', + group: 'group1', + branch: 'develop', + skipForkedRepos: false, + schedule: { + frequency: 'PT30M', + timeout: 'PT3M', + }, + }, + }, + }, + }, +}; export const config_single_integration_group: MockObject = { integrations: { gitlab: [ @@ -234,7 +260,7 @@ export const config_fallbackBranch_branch: MockObject = { 'test-id': { host: 'example.com', group: 'group1', - fallbackBranch: 'staging', + fallbackBranch: 'main', skipForkedRepos: false, schedule: { frequency: 'PT30M', @@ -634,7 +660,7 @@ export const all_projects_response: GitLabProject[] = [ web_url: 'https://example.com/group1/test-repo5-staging', path_with_namespace: 'group1/test-repo5-staging', }, - // diffrent group + // different group { id: 6, description: 'Project Six Description', @@ -646,6 +672,17 @@ export const all_projects_response: GitLabProject[] = [ web_url: 'https://example.com/group1/test-repo6', path_with_namespace: 'awesome-group/test-repo6', }, + // no default branch + { + id: 7, + description: 'Project Seven Description', + name: 'test-repo7', + path: 'test-repo7', + archived: false, + last_activity_at: new Date().toString(), + web_url: 'https://example.com/group1/test-repo7', + path_with_namespace: 'group1/test-repo7', + }, ]; export const all_users_response: GitLabUser[] = [ @@ -686,7 +723,7 @@ export const all_users_response: GitLabUser[] = [ avatar_url: 'https://secure.gravatar.com/', web_url: 'https://gitlab.example/luigi_mario', }, - // malfomed email address + // malformed email address { id: 5, username: 'MarioMario', @@ -1299,9 +1336,43 @@ export const push_modif_event: EventParams = { /** * Expected Backstage entities */ -export const expected_location_entities: MockObject[] = + +// includes only projects that have a default branch (for when the branch and fallback branch were not set in the config) +export const expected_location_entities_default_branch: MockObject[] = + all_projects_response + .filter(project => project.default_branch) + .map(project => { + const targetUrl = `https://example.com/${project.path_with_namespace}/-/blob/${project.default_branch}/catalog-info.yaml`; + + return { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Location', + metadata: { + annotations: { + 'backstage.io/managed-by-location': `url:${targetUrl}`, + 'backstage.io/managed-by-origin-location': `url:${targetUrl}`, + }, + name: locationSpecToMetadataName({ + target: targetUrl, + type: 'url', + }), + }, + spec: { + presence: 'optional', + target: targetUrl, + type: 'url', + }, + }, + locationKey: 'GitlabDiscoveryEntityProvider:test-id', + }; + }); + +// includes every GitLab project that has a default branch and the fallback declared in the config +export const expected_location_entities_fallback_branch: MockObject[] = all_projects_response.map(project => { - const targetUrl = `https://example.com/${project.path_with_namespace}/-/blob/${project.default_branch}/catalog-info.yaml`; + const branch = project.default_branch || 'main'; + const targetUrl = `https://example.com/${project.path_with_namespace}/-/blob/${branch}/catalog-info.yaml`; return { entity: { @@ -1312,7 +1383,40 @@ export const expected_location_entities: MockObject[] = 'backstage.io/managed-by-location': `url:${targetUrl}`, 'backstage.io/managed-by-origin-location': `url:${targetUrl}`, }, - name: locationSpecToMetadataName({ target: targetUrl, type: 'url' }), + name: locationSpecToMetadataName({ + target: targetUrl, + type: 'url', + }), + }, + spec: { + presence: 'optional', + target: targetUrl, + type: 'url', + }, + }, + locationKey: 'GitlabDiscoveryEntityProvider:test-id', + }; + }); + +// includes ONLY the projects with the branch declared in the config +export const expected_location_entities_specific_branch: MockObject[] = + all_projects_response.map(project => { + const branch = 'develop'; + const targetUrl = `https://example.com/${project.path_with_namespace}/-/blob/${branch}/catalog-info.yaml`; + + return { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Location', + metadata: { + annotations: { + 'backstage.io/managed-by-location': `url:${targetUrl}`, + 'backstage.io/managed-by-origin-location': `url:${targetUrl}`, + }, + name: locationSpecToMetadataName({ + target: targetUrl, + type: 'url', + }), }, spec: { presence: 'optional', diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts index febaa00d28..e89b378cfe 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts @@ -63,7 +63,7 @@ describe('GitlabDiscoveryEntityProvider - configuration', () => { }); it('should fail without schedule nor scheduler', () => { - const config = new ConfigReader(mock.config_single_integration_branch); + const config = new ConfigReader(mock.config_single_integration); expect(() => GitlabDiscoveryEntityProvider.fromConfig(config, { @@ -101,7 +101,7 @@ describe('GitlabDiscoveryEntityProvider - configuration', () => { it('should instantiate provider with single simple discovery config', () => { const schedule = new PersistingTaskRunner(); - const config = new ConfigReader(mock.config_single_integration_branch); + const config = new ConfigReader(mock.config_single_integration); const providers = GitlabDiscoveryEntityProvider.fromConfig(config, { logger, schedule, @@ -155,7 +155,7 @@ describe('GitlabDiscoveryEntityProvider - refresh', () => { expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ type: 'full', - entities: mock.expected_location_entities.filter( + entities: mock.expected_location_entities_default_branch.filter( entity => !entity.entity.metadata.annotations[ 'backstage.io/managed-by-location' @@ -189,7 +189,7 @@ describe('GitlabDiscoveryEntityProvider - refresh', () => { expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ type: 'full', - entities: mock.expected_location_entities.filter( + entities: mock.expected_location_entities_default_branch.filter( entity => entity.entity.metadata.annotations[ 'backstage.io/managed-by-location' @@ -219,7 +219,7 @@ describe('GitlabDiscoveryEntityProvider - refresh', () => { expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ type: 'full', - entities: mock.expected_location_entities.filter( + entities: mock.expected_location_entities_default_branch.filter( entity => !entity.entity.metadata.annotations[ 'backstage.io/managed-by-location' @@ -231,8 +231,9 @@ describe('GitlabDiscoveryEntityProvider - refresh', () => { }); }); - it('should filter found projects based on the branch', async () => { - const config = new ConfigReader(mock.config_single_integration_branch); + // branch and fallback branch are undefined in the config + it('should ingest catalog from project default branch only', async () => { + const config = new ConfigReader(mock.config_single_integration); const schedule = new PersistingTaskRunner(); const entityProviderConnection: EntityProviderConnection = { applyMutation: jest.fn(), @@ -243,21 +244,14 @@ describe('GitlabDiscoveryEntityProvider - refresh', () => { schedule, })[0]; - const configured_branch = - mock.config_single_integration_branch.catalog.providers.gitlab['test-id'] - .branch; - await provider.connect(entityProviderConnection); await provider.refresh(logger); expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ type: 'full', - entities: mock.expected_location_entities.filter( + entities: mock.expected_location_entities_default_branch.filter( entity => - entity.entity.metadata.annotations[ - 'backstage.io/managed-by-location' - ].includes(configured_branch) && !entity.entity.metadata.annotations[ 'backstage.io/managed-by-location' ].includes('awesome'), @@ -265,7 +259,38 @@ describe('GitlabDiscoveryEntityProvider - refresh', () => { }); }); - it('should only include projects with fallback branch', async () => { + // branch was set in the config + it('should ingest catalog from specific branch only', async () => { + const config = new ConfigReader( + mock.config_single_integration_specific_branch, + ); + const schedule = new PersistingTaskRunner(); + const entityProviderConnection: EntityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + const provider = GitlabDiscoveryEntityProvider.fromConfig(config, { + logger, + schedule, + })[0]; + + await provider.connect(entityProviderConnection); + + await provider.refresh(logger); + + expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ + type: 'full', + entities: mock.expected_location_entities_specific_branch.filter( + entity => + !entity.entity.metadata.annotations[ + 'backstage.io/managed-by-location' + ].includes('awesome'), + ), + }); + }); + + // fallback branch was set in the config + it('should ingest catalog from default or fallback branch', async () => { const config = new ConfigReader(mock.config_fallbackBranch_branch); const schedule = new PersistingTaskRunner(); const entityProviderConnection: EntityProviderConnection = { @@ -277,21 +302,14 @@ describe('GitlabDiscoveryEntityProvider - refresh', () => { schedule, })[0]; - const configured_branch = - mock.config_fallbackBranch_branch.catalog.providers.gitlab['test-id'] - .fallbackBranch; - await provider.connect(entityProviderConnection); await provider.refresh(logger); expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ type: 'full', - entities: mock.expected_location_entities.filter( + entities: mock.expected_location_entities_fallback_branch.filter( entity => - entity.entity.metadata.annotations[ - 'backstage.io/managed-by-location' - ].includes(configured_branch) && !entity.entity.metadata.annotations[ 'backstage.io/managed-by-location' ].includes('awesome'), @@ -321,7 +339,7 @@ describe('GitlabDiscoveryEntityProvider - refresh', () => { expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ type: 'full', - entities: mock.expected_location_entities.filter(entity => + entities: mock.expected_location_entities_default_branch.filter(entity => entity.entity.metadata.annotations[ 'backstage.io/managed-by-location' ].includes(configured_group), @@ -396,7 +414,7 @@ describe('GitlabDiscoveryEntityProvider - events', () => { expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(0); }); it('should apply delta mutations on added files from push event', async () => { - const config = new ConfigReader(mock.config_single_integration_branch); + const config = new ConfigReader(mock.config_single_integration); const schedule = new PersistingTaskRunner(); const events = DefaultEventsService.create({ logger }); @@ -423,7 +441,7 @@ describe('GitlabDiscoveryEntityProvider - events', () => { }); it('should apply delta mutations on removed files from push event', async () => { - const config = new ConfigReader(mock.config_single_integration_branch); + const config = new ConfigReader(mock.config_single_integration); const schedule = new PersistingTaskRunner(); const events = DefaultEventsService.create({ logger }); const entityProviderConnection: EntityProviderConnection = { @@ -449,7 +467,7 @@ describe('GitlabDiscoveryEntityProvider - events', () => { }); it('should call refresh on added files from push event', async () => { - const config = new ConfigReader(mock.config_single_integration_branch); + const config = new ConfigReader(mock.config_single_integration); const schedule = new PersistingTaskRunner(); const events = DefaultEventsService.create({ logger }); const entityProviderConnection: EntityProviderConnection = { diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts index 73ff5f3a01..b2ef475d9d 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { LoggerService } from '@backstage/backend-plugin-api'; import { PluginTaskScheduler, TaskRunner } from '@backstage/backend-tasks'; import { Config } from '@backstage/config'; import { GitLabIntegration, ScmIntegrations } from '@backstage/integration'; @@ -34,7 +35,6 @@ import { paginated, readGitlabConfigs, } from '../lib'; -import { LoggerService } from '@backstage/backend-plugin-api'; import * as path from 'path'; @@ -470,14 +470,8 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { return false; } - const customFallbackBranch = - this.config.fallbackBranch !== 'master' - ? this.config.fallbackBranch - : undefined; - const project_branch = this.config.branch ?? - customFallbackBranch ?? project.default_branch ?? this.config.fallbackBranch;