Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further align job definition config with observed GitHub defaults #1536

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
+82 −62
Draft

Further align job definition config with observed GitHub defaults #1536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d5fb4d3
Update job config to more closely match GitHub
rhyskoedijk Jan 13, 2025
d9e23f3
Fix merge issue
rhyskoedijk Jan 17, 2025
3c3c818
More config updates
rhyskoedijk Jan 18, 2025
a437879
Update unit tests
rhyskoedijk Jan 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 4 additions & 18 deletions extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ describe('mapGroupsFromDependabotConfigToJobConfig', () => {
expect(result).toBeUndefined();
});

it('should return an empty array if dependencyGroups is an empty object', () => {
it('should return undefined if dependencyGroups is an empty object', () => {
const result = mapGroupsFromDependabotConfigToJobConfig({});
expect(result).toEqual([]);
expect(result).toBeUndefined();
});

it('should filter out undefined groups', () => {
Expand All @@ -21,14 +21,7 @@ describe('mapGroupsFromDependabotConfigToJobConfig', () => {
};

const result = mapGroupsFromDependabotConfigToJobConfig(dependencyGroups);
expect(result).toEqual([
{
name: 'group2',
rules: {
patterns: ['pattern2'],
},
},
]);
expect(result).toHaveLength(1);
});

it('should filter out null groups', () => {
Expand All @@ -40,14 +33,7 @@ describe('mapGroupsFromDependabotConfigToJobConfig', () => {
};

const result = mapGroupsFromDependabotConfigToJobConfig(dependencyGroups);
expect(result).toEqual([
{
name: 'group2',
rules: {
patterns: ['pattern2'],
},
},
]);
expect(result).toHaveLength(1);
});

it('should map dependency group properties correctly', () => {
Expand Down
82 changes: 51 additions & 31 deletions extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class DependabotJobBuilder {
}
}

function buildUpdateJobConfig(
export function buildUpdateJobConfig(
id: string,
taskInputs: ISharedVariables,
update: IDependabotUpdate,
Expand All @@ -122,20 +122,20 @@ function buildUpdateJobConfig(
updateDependencyNames?: string[] | undefined,
existingPullRequests?: any[],
securityVulnerabilities?: ISecurityVulnerability[],
) {
): IDependabotUpdateOperation {
const securityOnlyUpdate = update['open-pull-requests-limit'] == 0;
return {
config: update,
job: {
'id': id,
'package-manager': update['package-ecosystem'],
'update-subdependencies': true, // TODO: add config for this?
'updating-a-pull-request': updatingPullRequest || false,
'dependency-group-to-refresh': updateDependencyGroupName,
'dependency-groups': mapGroupsFromDependabotConfigToJobConfig(update.groups),
'dependencies': updateDependencyNames,
'allowed-updates': mapAllowedUpdatesFromDependabotConfigToJobConfig(update.allow),
'dependencies': updateDependencyNames.length ? updateDependencyNames : undefined,
'allowed-updates': mapAllowedUpdatesFromDependabotConfigToJobConfig(update.allow, securityOnlyUpdate),
'ignore-conditions': mapIgnoreConditionsFromDependabotConfigToJobConfig(update.ignore),
'security-updates-only': update['open-pull-requests-limit'] == 0,
'security-updates-only': securityOnlyUpdate,
'security-advisories': mapSecurityAdvisories(securityVulnerabilities),
'source': mapSourceFromDependabotConfigToJobConfig(taskInputs, update),
'existing-pull-requests': existingPullRequests?.filter((pr) => !pr['dependency-group-name']),
Expand All @@ -146,30 +146,31 @@ function buildUpdateJobConfig(
: {
'prefix': update['commit-message']?.['prefix'],
'prefix-development': update['commit-message']?.['prefix-development'],
'include-scope': update['commit-message']?.['include'],
'include-scope':
update['commit-message']?.['include']?.toLocaleLowerCase()?.trim() == 'scope' ? true : undefined,
},
'experiments': Object.keys(taskInputs.experiments || {}).reduce(
(acc, key) => {
// Replace '-' with '_' in the experiment keys to match the dependabot-core models
acc[key.replace(/-/g, '_')] = taskInputs.experiments[key];
return acc;
},
{} as Record<string, string | boolean>,
),
'max-updater-run-time': undefined, // TODO: add config for this?
'reject-external-code': update['insecure-external-code-execution']?.toLocaleLowerCase() == 'allow',
'experiments': taskInputs.experiments,
'reject-external-code': update['insecure-external-code-execution']?.toLocaleLowerCase()?.trim() == 'allow',
'repo-private': undefined, // TODO: add config for this?
'repo-contents-path': undefined, // TODO: add config for this?
'requirements-update-strategy': mapVersionStrategyToRequirementsUpdateStrategy(update['versioning-strategy']),
'lockfile-only': update['versioning-strategy'] === 'lockfile-only',
'vendor-dependencies': update.vendor,
'debug': taskInputs.debug,

// TODO: Investigate if these options are needed or are obsolete
// The following options don't appear to be used by dependabot-core yet/anymore, but do appear in GitHub dependabot job logs.
// They are added here for completeness and so that we mimic the GitHub dependabot config as closely as possible.
// It is possible that these options might become live in a future dependabot-core release.
//'max-updater-run-time': 2700,
//'proxy-log-response-body-on-auth-failure': true,
//'update-subdependencies': false,
},
credentials: mapRegistryCredentialsFromDependabotConfigToJobConfig(taskInputs, registries),
};
}

function mapSourceFromDependabotConfigToJobConfig(taskInputs: ISharedVariables, update: IDependabotUpdate): any {
export function mapSourceFromDependabotConfigToJobConfig(taskInputs: ISharedVariables, update: IDependabotUpdate): any {
return {
'provider': 'azure',
'api-endpoint': taskInputs.apiEndpointUrl,
Expand All @@ -182,8 +183,10 @@ function mapSourceFromDependabotConfigToJobConfig(taskInputs: ISharedVariables,
};
}

export function mapGroupsFromDependabotConfigToJobConfig(dependencyGroups: Record<string, IDependabotGroup>): any[] {
if (!dependencyGroups) {
export function mapGroupsFromDependabotConfigToJobConfig(
dependencyGroups: Record<string, IDependabotGroup>,
): any[] | undefined {
if (!dependencyGroups || !Object.keys(dependencyGroups).length) {
return undefined;
}
return Object.keys(dependencyGroups)
Expand All @@ -206,36 +209,53 @@ export function mapGroupsFromDependabotConfigToJobConfig(dependencyGroups: Recor
.filter((g) => g);
}

function mapAllowedUpdatesFromDependabotConfigToJobConfig(allowedUpdates: IDependabotAllowCondition[]): any[] {
export function mapAllowedUpdatesFromDependabotConfigToJobConfig(
allowedUpdates: IDependabotAllowCondition[],
securityOnlyUpdate?: boolean,
): any[] {
// If no allow conditions are specified, update direct dependencies by default
// NOTE: 'update-type' appears to be a deprecated config, but still appears in the dependabot-core model and GitHub dependabot job logs.
// See: https://github.com/dependabot/dependabot-core/blob/b3a0c1f86c20729494097ebc695067099f5b4ada/updater/lib/dependabot/job.rb#L253C1-L257C78
if (!allowedUpdates) {
// If no allow conditions are specified, update all dependencies by default
return [{ 'dependency-type': 'all' }];
return [
{
'dependency-type': 'direct',
'update-type': securityOnlyUpdate ? 'security' : 'all',
},
];
}
return allowedUpdates.map((allow) => {
return {
'dependency-name': allow['dependency-name'],
'dependency-type': allow['dependency-type'],
//'update-type': allow["update-type"] // TODO: This is missing from dependabot.ymal docs, but is used in the dependabot-core job model!?
'update-type': allow['update-type'],
};
});
}

function mapIgnoreConditionsFromDependabotConfigToJobConfig(ignoreConditions: IDependabotAllowCondition[]): any[] {
export function mapIgnoreConditionsFromDependabotConfigToJobConfig(
ignoreConditions: IDependabotAllowCondition[],
): any[] {
if (!ignoreConditions) {
return undefined;
}
return ignoreConditions.map((ignore) => {
return {
'source': ignore['source'],
'updated-at': ignore['updated-at'],
'dependency-name': ignore['dependency-name'],
//'source': ignore["source"], // TODO: This is missing from dependabot.ymal docs, but is used in the dependabot-core job model!?
'update-types': ignore['update-types'],
//'updated-at': ignore["updated-at"], // TODO: This is missing from dependabot.ymal docs, but is used in the dependabot-core job model!?
'version-requirement': (<string[]>ignore['versions'])?.join(', '), // TODO: Test this, not sure how this should be parsed...

// The dependabot.yml config docs are not very clear about acceptable values; after scanning dependabot-core and dependabot-cli,
// this could either be a single version string (e.g. '>1.0.0'), or multiple version strings separated by commas (e.g. '>1.0.0, <2.0.0')
'version-requirement': Array.isArray(ignore['versions'])
? (<string[]>ignore['versions'])?.join(', ')
: <string>ignore['versions'],
};
});
}

function mapSecurityAdvisories(securityVulnerabilities: ISecurityVulnerability[]): any[] {
export function mapSecurityAdvisories(securityVulnerabilities: ISecurityVulnerability[]): any[] {
if (!securityVulnerabilities) {
return undefined;
}
Expand All @@ -260,7 +280,7 @@ function mapSecurityAdvisories(securityVulnerabilities: ISecurityVulnerability[]
});
}

function mapVersionStrategyToRequirementsUpdateStrategy(versioningStrategy: string): string | undefined {
export function mapVersionStrategyToRequirementsUpdateStrategy(versioningStrategy: string): string | undefined {
if (!versioningStrategy) {
return undefined;
}
Expand All @@ -280,7 +300,7 @@ function mapVersionStrategyToRequirementsUpdateStrategy(versioningStrategy: stri
}
}

function mapRegistryCredentialsFromDependabotConfigToJobConfig(
export function mapRegistryCredentialsFromDependabotConfigToJobConfig(
taskInputs: ISharedVariables,
registries: Record<string, IDependabotRegistry>,
): any[] {
Expand Down
7 changes: 1 addition & 6 deletions extension/tasks/dependabotV2/utils/dependabot-cli/interfaces/IDependabotUpdateJobConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ export interface IDependabotUpdateJobConfig {
'affected-versions': string[];
'patched-versions': string[];
'unaffected-versions': string[];
// TODO: The below configs are not in the dependabot-cli model, but are in the dependabot-core model
'title'?: string;
'description'?: string;
'source-name'?: string;
'source-url'?: string;
}[];
'source': {
'provider': string;
Expand Down Expand Up @@ -72,7 +67,7 @@ export interface IDependabotUpdateJobConfig {
'commit-message-options'?: {
'prefix'?: string;
'prefix-development'?: string;
'include-scope'?: string;
'include-scope'?: boolean;
};
'experiments'?: Record<string, string | boolean>;
'max-updater-run-time'?: number;
Expand Down
33 changes: 26 additions & 7 deletions extension/tasks/dependabotV2/utils/dependabot/parseConfigFile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ export default async function parseConfigFile(taskInputs: ISharedVariables): Pro
'/.github/dependabot.yml',
];

let contents: null | string;
let configPath: null | string;
let configContents: null | string;

/*
* The configuration file can be available locally if the repository is cloned.
Expand All @@ -54,7 +55,8 @@ export default async function parseConfigFile(taskInputs: ISharedVariables): Pro
});
if (response.status === 200) {
tl.debug(`Found configuration file at '${url}'`);
contents = response.data;
configContents = response.data;
configPath = fp;
break;
}
} catch (error) {
Expand All @@ -78,7 +80,8 @@ export default async function parseConfigFile(taskInputs: ISharedVariables): Pro
var filePath = path.join(rootDir, fp);
if (fs.existsSync(filePath)) {
tl.debug(`Found configuration file cloned at ${filePath}`);
contents = fs.readFileSync(filePath, 'utf-8');
configContents = fs.readFileSync(filePath, 'utf-8');
configPath = filePath;
break;
} else {
tl.debug(`No configuration file cloned at ${filePath}`);
Expand All @@ -87,13 +90,13 @@ export default async function parseConfigFile(taskInputs: ISharedVariables): Pro
}

// Ensure we have file contents. Otherwise throw a well readable error.
if (!contents || typeof contents !== 'string') {
if (!configContents || typeof configContents !== 'string') {
throw new Error(`Configuration file not found at possible locations: ${possibleFilePaths.join(', ')}`);
} else {
tl.debug('Configuration file contents read.');
}

let config: any = load(contents);
let config: any = load(configContents);

// Ensure the config object parsed is an object
if (config === null || typeof config !== 'object') {
Expand All @@ -120,7 +123,7 @@ export default async function parseConfigFile(taskInputs: ISharedVariables): Pro
throw new Error('Only version 2 of dependabot is supported. Version specified: ' + version);
}

const updates = parseUpdates(config);
const updates = parseUpdates(config, configPath);
const registries = parseRegistries(config);
validateConfiguration(updates, registries);

Expand All @@ -131,7 +134,7 @@ export default async function parseConfigFile(taskInputs: ISharedVariables): Pro
};
}

function parseUpdates(config: any): IDependabotUpdate[] {
function parseUpdates(config: any, configPath: string): IDependabotUpdate[] {
var updates: IDependabotUpdate[] = [];

// Check the updates parsed
Expand Down Expand Up @@ -191,12 +194,28 @@ function parseUpdates(config: any): IDependabotUpdate[] {
dependabotUpdate['open-pull-requests-limit'] = 5;
}

// either 'directory' or 'directories' must be specified
if (!dependabotUpdate.directory && dependabotUpdate.directories.length === 0) {
throw new Error(
"The values 'directory' and 'directories' in dependency update config is missing, you must specify at least one",
);
}

// populate the 'ignore' conditions 'source' and 'updated-at' properties, if missing
// NOTE: 'source' and 'updated-at' are not documented in the dependabot.yml config docs, but are defined in the dependabot-core and dependabot-cli models.
// Currently they don't appear to add much value to the update process, but are populated here for completeness.
if (dependabotUpdate.ignore) {
dependabotUpdate.ignore.forEach((ignoreCondition) => {
if (!ignoreCondition['source']) {
ignoreCondition['source'] = configPath;
}
if (!ignoreCondition['updated-at']) {
// we don't know the last updated time, so we use the current time
ignoreCondition['updated-at'] = new Date().toISOString();
}
});
}

updates.push(dependabotUpdate);
});
return updates;
Expand Down
Loading