Skip to content

Commit

Permalink
Fix workflow check in repocop dep graph integration (#1315)
Browse files Browse the repository at this point in the history 
* fix: bug in evaluation of workflows for repo

* fix: correctly detect repos without workflows and refactor

* refactor: update tests

* lint: fix linting errors

* refactor: only send slice of repos to createSnsEventsForDependencyGraphIntegration

* refactor: give dep graph language distinct field name

* refactor: change suitableRepos var to reposRequiringDepGraphIntegration to make clearer

* refactor: make getReposWithoutWorkflows more functional
  • Loading branch information
@tjsilver
tjsilver authored Nov 4, 2024
1 parent c1aea5b commit 1dcc8c5
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 72 deletions.
4 changes: 4 additions & 0 deletions packages/common/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ export interface Repository extends RepositoryFields {
id: NonNullable<RepositoryFields['id']>;
}

export interface RepositoryWithDepGraphLanguage extends Repository {
dependency_graph_language: DepGraphLanguage;
}

// The number of days teams have to fix vulnerabilities of a given severity
export const SLAs: Record<Severity, number | undefined> = {
critical: 2,
Expand Down
2 changes: 1 addition & 1 deletion packages/repocop/src/evaluation/repository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ export function evaluateRepositories(
const teamsForRepo = owners.filter((o) => o.full_repo_name === r.full_name);
const branchesForRepo = branches.filter((b) => b.repository_id === r.id);
const workflowsForRepo = productionWorkflowUsages.filter(
(repo) => (repo.full_name = r.full_name),
(repo) => repo.full_name === r.full_name,
);

return evaluateOneRepo(
Expand Down
60 changes: 35 additions & 25 deletions packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@ import type {
guardian_github_actions_usage,
view_repo_ownership,
} from '@prisma/client';
import type { Repository } from 'common/src/types';
import type {
DepGraphLanguage,
Repository,
RepositoryWithDepGraphLanguage,
} from 'common/src/types';
import { removeRepoOwner } from '../shared-utilities';
import {
checkRepoForLanguage,
createSnsEventsForDependencyGraphIntegration,
doesRepoHaveDepSubmissionWorkflowForLanguage,
getReposWithoutWorkflows,
} from './send-to-sns';

const fullName = 'guardian/repo-name';
Expand Down Expand Up @@ -56,6 +61,17 @@ function repository(fullName: string): Repository {
};
}

function repositoryWithDepGraphLanguage(
fullName: string,
language: DepGraphLanguage,
): RepositoryWithDepGraphLanguage {
const repo = repository(fullName);
return {
...repo,
dependency_graph_language: language,
};
}

function repoWithTargetLanguage(fullName: string): github_languages {
return repoWithLanguages(fullName, ['Scala', 'TypeScript']);
}
Expand Down Expand Up @@ -121,46 +137,44 @@ describe('When checking a repo for an existing dependency submission workflow',
});

describe('When getting suitable events to send to SNS', () => {
test('return an event when a Scala repo is found without an existing workflow', () => {
const result = createSnsEventsForDependencyGraphIntegration(
test('return the repo when a Scala repo is found without an existing workflow', () => {
const result = getReposWithoutWorkflows(
[repoWithTargetLanguage(fullName)],
[repository(fullName)],
[repoWithoutWorkflow(fullName)],
[],
);
expect(result).toEqual([
{ name: removeRepoOwner(fullName), language: 'Scala', admins: [] },
]);
const expected = [repositoryWithDepGraphLanguage(fullName, 'Scala')];

expect(result).toEqual(expected);
});
test('return empty event array when a Scala repo is found with an existing workflow', () => {
const result = createSnsEventsForDependencyGraphIntegration(
test('return empty repo array when a Scala repo is found with an existing workflow', () => {
const result = getReposWithoutWorkflows(
[repoWithTargetLanguage(fullName)],
[repository(fullName)],
[repoWithDepSubmissionWorkflow(fullName)],
[],
);
expect(result).toEqual([]);
});
test('return empty array when non-Scala repo is found with without an existing workflow', () => {
const result = createSnsEventsForDependencyGraphIntegration(
const result = getReposWithoutWorkflows(
[repoWithoutTargetLanguage(fullName)],
[repository(fullName)],
[repoWithoutWorkflow(fullName)],
[],
);
expect(result).toEqual([]);
});
test('return 2 events when 2 Scala repos are found without an existing workflow', () => {
const result = createSnsEventsForDependencyGraphIntegration(
const result = getReposWithoutWorkflows(
[repoWithTargetLanguage(fullName), repoWithTargetLanguage(fullName2)],
[repository(fullName), repository(fullName2)],
[repoWithoutWorkflow(fullName), repoWithoutWorkflow(fullName2)],
[],
);
expect(result).toEqual([
{ name: removeRepoOwner(fullName), language: 'Scala', admins: [] },
{ name: removeRepoOwner(fullName2), language: 'Scala', admins: [] },
]);
const expected = [
repositoryWithDepGraphLanguage(fullName, 'Scala'),
repositoryWithDepGraphLanguage(fullName2, 'Scala'),
];

expect(result).toEqual(expected);
});

const ownershipRecord1: view_repo_ownership = {
Expand All @@ -184,9 +198,7 @@ describe('When getting suitable events to send to SNS', () => {
};

const result = createSnsEventsForDependencyGraphIntegration(
[repoWithTargetLanguage(fullName)],
[repository(fullName)],
[repoWithoutWorkflow(fullName)],
[repositoryWithDepGraphLanguage(fullName, 'Scala')],
[ownershipRecord1, ownershipRecord2],
);
expect(result).toEqual([
Expand All @@ -197,17 +209,15 @@ describe('When getting suitable events to send to SNS', () => {
},
]);
});
test('return not event with an admin if none are correct', () => {
test('do not return event with an admin if none are correct', () => {
const ownershipRecord: view_repo_ownership = {
...ownershipRecord1,
full_repo_name: 'guardian/other-repo',
short_repo_name: 'other-repo',
};

const result = createSnsEventsForDependencyGraphIntegration(
[repoWithTargetLanguage(fullName)],
[repository(fullName)],
[repoWithoutWorkflow(fullName)],
[repositoryWithDepGraphLanguage(fullName, 'Scala')],
[ownershipRecord],
);
expect(result).toEqual([
Expand Down
117 changes: 71 additions & 46 deletions packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
DependencyGraphIntegratorEvent,
DepGraphLanguage,
Repository,
RepositoryWithDepGraphLanguage,
} from 'common/src/types';
import type { Config } from '../../config';
import { findContactableOwners, removeRepoOwner } from '../shared-utilities';
Expand All @@ -27,12 +28,12 @@ export function checkRepoForLanguage(

export function doesRepoHaveDepSubmissionWorkflowForLanguage(
repo: Repository,
workflow_usages: guardian_github_actions_usage[],
workflowUsagesForRepo: guardian_github_actions_usage[],
language: DepGraphLanguage,
): boolean {
const actionsForRepo = workflow_usages
.filter((usages) => repo.full_name === usages.full_name)
.flatMap((workflow) => workflow.workflow_uses);
const actionsForRepo = workflowUsagesForRepo.flatMap(
(workflow) => workflow.workflow_uses,
);

const workflows: Record<DepGraphLanguage, string> = {
Scala: 'scalacenter/sbt-dependency-submission',
Expand All @@ -49,44 +50,18 @@ export function doesRepoHaveDepSubmissionWorkflowForLanguage(
}

export function createSnsEventsForDependencyGraphIntegration(
languages: github_languages[],
productionRepos: Repository[],
workflow_usages: guardian_github_actions_usage[],
view_repo_ownership: view_repo_ownership[],
reposWithoutWorkflows: RepositoryWithDepGraphLanguage[],
repoOwnership: view_repo_ownership[],
): DependencyGraphIntegratorEvent[] {
const depGraphLanguages: DepGraphLanguage[] = ['Scala', 'Kotlin'];
const eventsForAllLanguages: DependencyGraphIntegratorEvent[] = [];

depGraphLanguages.forEach((language) => {
let reposWithDepGraphLanguages: Repository[] = [];
const repos = productionRepos.filter((repo) =>
checkRepoForLanguage(repo, languages, language),
);
console.log(`Found ${repos.length} ${language} repos in production`);

reposWithDepGraphLanguages = reposWithDepGraphLanguages.concat(repos);

const reposWithoutWorkflows = reposWithDepGraphLanguages.filter(
(repo) =>
!doesRepoHaveDepSubmissionWorkflowForLanguage(
repo,
workflow_usages,
language,
),
);
console.log(
`Found ${reposWithoutWorkflows.length} production repos without ${language} dependency submission workflows`,
);
reposWithoutWorkflows.map((repo) =>
eventsForAllLanguages.push({
name: removeRepoOwner(repo.full_name),
language,
admins: findContactableOwners(repo.full_name, view_repo_ownership),
}),
);
});
const eventsForAllLanguages: DependencyGraphIntegratorEvent[] =
reposWithoutWorkflows.map((repo) => ({
name: removeRepoOwner(repo.full_name),
language: repo.dependency_graph_language,
admins: findContactableOwners(repo.full_name, repoOwnership),
}));

console.log(`Found ${eventsForAllLanguages.length} events to send to SNS`);

return eventsForAllLanguages;
}

Expand All @@ -110,6 +85,42 @@ async function sendOneRepoToDepGraphIntegrator(
}
}

export function getReposWithoutWorkflows(
languages: github_languages[],
productionRepos: Repository[],
productionWorkflowUsages: guardian_github_actions_usage[],
): RepositoryWithDepGraphLanguage[] {
const depGraphLanguages: DepGraphLanguage[] = ['Scala', 'Kotlin'];

const allReposWithoutWorkflows: RepositoryWithDepGraphLanguage[] =
depGraphLanguages.flatMap((language) => {
const reposWithDepGraphLanguages: Repository[] = productionRepos.filter(
(repo) => checkRepoForLanguage(repo, languages, language),
);
console.log(
`Found ${reposWithDepGraphLanguages.length} ${language} repos in production`,
);

return reposWithDepGraphLanguages
.filter((repo) => {
const workflowUsagesForRepo = productionWorkflowUsages.filter(
(workflow) => workflow.full_name === repo.full_name,
);
return !doesRepoHaveDepSubmissionWorkflowForLanguage(
repo,
workflowUsagesForRepo,
language,
);
})
.map((repo) => ({ ...repo, dependency_graph_language: language }));
});

console.log(
`Found ${allReposWithoutWorkflows.length} production repos without dependency submission workflows`,
);
return allReposWithoutWorkflows;
}

export async function sendReposToDependencyGraphIntegrator(
config: Config,
repoLanguages: github_languages[],
Expand All @@ -118,16 +129,30 @@ export async function sendReposToDependencyGraphIntegrator(
repoOwners: view_repo_ownership[],
repoCount: number,
): Promise<void> {
const eventsToSend: DependencyGraphIntegratorEvent[] = shuffle(
createSnsEventsForDependencyGraphIntegration(
const reposRequiringDepGraphIntegration: RepositoryWithDepGraphLanguage[] =
getReposWithoutWorkflows(
repoLanguages,
productionRepos,
productionWorkflowUsages,
repoOwners,
),
).slice(0, repoCount);
);

if (reposRequiringDepGraphIntegration.length !== 0) {
console.log(
`Found ${reposRequiringDepGraphIntegration.length} repos requiring dependency graph integration`,
);

for (const event of eventsToSend) {
await sendOneRepoToDepGraphIntegrator(config, event);
const selectedRepos = shuffle(reposRequiringDepGraphIntegration).slice(
0,
repoCount,
);

const eventsToSend: DependencyGraphIntegratorEvent[] =
createSnsEventsForDependencyGraphIntegration(selectedRepos, repoOwners);

for (const event of eventsToSend) {
await sendOneRepoToDepGraphIntegrator(config, event);
}
} else {
console.log('No suitable repos found to create events for.');
}
}

0 comments on commit 1dcc8c5

Please sign in to comment.