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.

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

feat(config-error): raise a warning issue for misconfigured matchConfidence #22296

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/config/options/index.ts
Expand Up @@ -2406,6 +2406,7 @@ const options: RenovateOptions[] = [
'configErrorIssue',
'deprecationWarningIssues',
'lockFileErrors',
'missingCredentialsError',
'onboardingClose',
'prEditedNotification',
'prIgnoreNotification',
Expand Down
3 changes: 3 additions & 0 deletions lib/constants/error-messages.ts
Expand Up @@ -62,3 +62,6 @@ export const INVALID_PATH = 'invalid-path';

// PAGE NOT FOUND
export const PAGE_NOT_FOUND_ERROR = 'page-not-found';

// Missing api required credentials
Gabriel-Ladzaretti marked this conversation as resolved.
Show resolved Hide resolved
export const MISSING_API_CREDENTIALS = 'missing-api-credentials';
12 changes: 6 additions & 6 deletions lib/util/merge-confidence/index.ts
Expand Up @@ -191,6 +191,12 @@ export async function initMergeConfidence(): Promise<void> {
return;
}

export function getApiToken(): string | undefined {
Gabriel-Ladzaretti marked this conversation as resolved.
Show resolved Hide resolved
return hostRules.find({
hostType,
})?.token;
}

function getApiBaseUrl(): string {
const defaultBaseUrl = 'https://badges.renovateapi.com/';
const baseFromEnv = process.env.RENOVATE_X_MERGE_CONFIDENCE_API_BASE_URL;
Expand All @@ -216,12 +222,6 @@ function getApiBaseUrl(): string {
}
}

function getApiToken(): string | undefined {
return hostRules.find({
hostType,
})?.token;
}

/**
* Handles errors returned by the Merge Confidence API.
*
Expand Down
138 changes: 89 additions & 49 deletions lib/util/package-rules/index.spec.ts
@@ -1,6 +1,9 @@
import { hostRules } from '../../../test/util';
import type { PackageRuleInputConfig, UpdateType } from '../../config/types';
import { MISSING_API_CREDENTIALS } from '../../constants/error-messages';
import { DockerDatasource } from '../../modules/datasource/docker';
import { OrbDatasource } from '../../modules/datasource/orb';
import type { HostRule } from '../../types';
import type { MergeConfidence } from '../merge-confidence/types';
import { applyPackageRules } from './index';

Expand Down Expand Up @@ -626,58 +629,95 @@ describe('util/package-rules/index', () => {
expect(res.x).toBeUndefined();
});

it('matches matchConfidence', () => {
const config: TestConfig = {
packageRules: [
{
matchConfidence: ['high'],
x: 1,
},
],
describe('matchConfidence', () => {
const hostRule: HostRule = {
hostType: 'merge-confidence',
token: 'some-token',
};
const dep = {
depType: 'dependencies',
depName: 'a',
mergeConfidenceLevel: 'high' as MergeConfidence,
};
const res = applyPackageRules({ ...config, ...dep });
expect(res.x).toBe(1);
});

it('non-matches matchConfidence', () => {
const config: TestConfig = {
packageRules: [
{
matchConfidence: ['high'],
x: 1,
},
],
};
const dep = {
depType: 'dependencies',
depName: 'a',
mergeConfidenceLevel: 'low' as MergeConfidence,
};
const res = applyPackageRules({ ...config, ...dep });
expect(res.x).toBeUndefined();
});
beforeEach(() => {
hostRules.clear();
hostRules.add(hostRule);
});

it('does not match matchConfidence when there is no mergeConfidenceLevel', () => {
const config: TestConfig = {
packageRules: [
{
matchConfidence: ['high'],
x: 1,
},
],
};
const dep = {
depType: 'dependencies',
depName: 'a',
mergeConfidenceLevel: undefined,
};
const res = applyPackageRules({ ...config, ...dep });
expect(res.x).toBeUndefined();
it('matches matchConfidence', () => {
const config: TestConfig = {
packageRules: [
{
matchConfidence: ['high'],
x: 1,
},
],
};
const dep = {
depType: 'dependencies',
depName: 'a',
mergeConfidenceLevel: 'high' as MergeConfidence,
};
const res = applyPackageRules({ ...config, ...dep });
expect(res.x).toBe(1);
});

it('non-matches matchConfidence', () => {
const config: TestConfig = {
packageRules: [
{
matchConfidence: ['high'],
x: 1,
},
],
};
const dep = {
depType: 'dependencies',
depName: 'a',
mergeConfidenceLevel: 'low' as MergeConfidence,
};
const res = applyPackageRules({ ...config, ...dep });
expect(res.x).toBeUndefined();
});

it('does not match matchConfidence when there is no mergeConfidenceLevel', () => {
const config: TestConfig = {
packageRules: [
{
matchConfidence: ['high'],
x: 1,
},
],
};
const dep = {
depType: 'dependencies',
depName: 'a',
mergeConfidenceLevel: undefined,
};
const res = applyPackageRules({ ...config, ...dep });
expect(res.x).toBeUndefined();
});

it('throws when unauthenticated', () => {
viceice marked this conversation as resolved.
Show resolved Hide resolved
const config: TestConfig = {
packageRules: [
{
matchConfidence: ['high'],
x: 1,
},
],
};
hostRules.clear();

let error = new Error();
try {
applyPackageRules(config);
} catch (err) {
error = err;
}

expect(error).toStrictEqual(new Error(MISSING_API_CREDENTIALS));
expect(error.validationError).toBe('Missing credentials');
expect(error.validationMessage).toBe(
"The 'matchConfidence' matcher in 'packageRules' requires authentication. Please refer to the documentation and add the required host rule."
);
});
});

it('filters naked depType', () => {
Expand Down
14 changes: 14 additions & 0 deletions lib/util/package-rules/merge-confidence.ts
@@ -1,5 +1,7 @@
import is from '@sindresorhus/is';
import type { PackageRule, PackageRuleInputConfig } from '../../config/types';
import { MISSING_API_CREDENTIALS } from '../../constants/error-messages';
import { getApiToken } from '../merge-confidence';
import { Matcher } from './base';

export class MergeConfidenceMatcher extends Matcher {
Expand All @@ -10,6 +12,18 @@ export class MergeConfidenceMatcher extends Matcher {
if (is.nullOrUndefined(matchConfidence)) {
return null;
}

/*
* Throw an error for unauthenticated use of the matchConfidence matcher.
*/
if (is.undefined(getApiToken())) {
const error = new Error(MISSING_API_CREDENTIALS);
error.validationError = 'Missing credentials';
error.validationMessage =
"The 'matchConfidence' matcher in 'packageRules' requires authentication. Please refer to the documentation and add the required host rule.";
rarkins marked this conversation as resolved.
Show resolved Hide resolved
throw error;
}

return (
is.array(matchConfidence) &&
is.nonEmptyString(mergeConfidenceLevel) &&
Expand Down
16 changes: 14 additions & 2 deletions lib/workers/repository/error-config.spec.ts
Expand Up @@ -9,7 +9,10 @@ import { GlobalConfig } from '../../config/global';
import { CONFIG_VALIDATION } from '../../constants/error-messages';
import { logger } from '../../logger';
import type { Pr } from '../../modules/platform';
import { raiseConfigWarningIssue } from './error-config';
import {
raiseConfigWarningIssue,
raiseCredentialsWarningIssue,
} from './error-config';

jest.mock('../../modules/platform');

Expand All @@ -27,19 +30,28 @@ describe('workers/repository/error-config', () => {
});

it('creates issues', async () => {
const expectedBody = `There are missing credentials for the authentication-required feature. As a precaution, Renovate will pause PRs until it is resolved.

Location: \`package.json\`
Error type: some-error
Message: \`some-message\`
`;
const error = new Error(CONFIG_VALIDATION);
error.validationSource = 'package.json';
error.validationMessage = 'some-message';
error.validationError = 'some-error';
platform.ensureIssue.mockResolvedValueOnce('created');

const res = await raiseConfigWarningIssue(config, error);
const res = await raiseCredentialsWarningIssue(config, error);

expect(res).toBeUndefined();
expect(logger.warn).toHaveBeenCalledWith(
{ configError: error, res: 'created' },
'Configuration Warning'
);
expect(platform.ensureIssue).toHaveBeenCalledWith(
expect.objectContaining({ body: expectedBody })
);
});

it('creates issues (dryRun)', async () => {
Expand Down
11 changes: 11 additions & 0 deletions lib/workers/repository/error-config.ts
Expand Up @@ -16,6 +16,17 @@ export function raiseConfigWarningIssue(
return raiseWarningIssue(config, notificationName, title, body, error);
}

export function raiseCredentialsWarningIssue(
config: RenovateConfig,
error: Error
): Promise<void> {
logger.debug('raiseCredentialsWarningIssue()');
const title = `Action Required: Add missing credentials`;
const body = `There are missing credentials for the authentication-required feature. As a precaution, Renovate will pause PRs until it is resolved.\n\n`;
const notificationName = 'missingCredentialsError';
return raiseWarningIssue(config, notificationName, title, body, error);
}

async function raiseWarningIssue(
config: RenovateConfig,
notificationName: string,
Expand Down
2 changes: 2 additions & 0 deletions lib/workers/repository/error.spec.ts
Expand Up @@ -4,6 +4,7 @@ import {
CONFIG_VALIDATION,
EXTERNAL_HOST_ERROR,
MANAGER_LOCKFILE_ERROR,
MISSING_API_CREDENTIALS,
NO_VULNERABILITY_ALERTS,
PLATFORM_AUTHENTICATION_ERROR,
PLATFORM_BAD_CREDENTIALS,
Expand Down Expand Up @@ -59,6 +60,7 @@ describe('workers/repository/error', () => {
PLATFORM_BAD_CREDENTIALS,
PLATFORM_RATE_LIMIT_EXCEEDED,
MANAGER_LOCKFILE_ERROR,
MISSING_API_CREDENTIALS,
SYSTEM_INSUFFICIENT_DISK_SPACE,
SYSTEM_INSUFFICIENT_MEMORY,
NO_VULNERABILITY_ALERTS,
Expand Down
12 changes: 11 additions & 1 deletion lib/workers/repository/error.ts
Expand Up @@ -5,6 +5,7 @@ import {
CONFIG_VALIDATION,
EXTERNAL_HOST_ERROR,
MANAGER_LOCKFILE_ERROR,
MISSING_API_CREDENTIALS,
NO_VULNERABILITY_ALERTS,
PLATFORM_AUTHENTICATION_ERROR,
PLATFORM_BAD_CREDENTIALS,
Expand Down Expand Up @@ -33,7 +34,10 @@ import {
} from '../../constants/error-messages';
import { logger } from '../../logger';
import { ExternalHostError } from '../../types/errors/external-host-error';
import { raiseConfigWarningIssue } from './error-config';
import {
raiseConfigWarningIssue,
raiseCredentialsWarningIssue,
} from './error-config';

export default async function handleError(
config: RenovateConfig,
Expand Down Expand Up @@ -116,6 +120,12 @@ export default async function handleError(
await raiseConfigWarningIssue(config, err);
return err.message;
}
if (err.message === MISSING_API_CREDENTIALS) {
delete config.branchList;
logger.info({ error: err }, MISSING_API_CREDENTIALS);
await raiseCredentialsWarningIssue(config, err);
return err.message;
}
if (err.message === CONFIG_SECRETS_EXPOSED) {
delete config.branchList;
logger.warn(
Expand Down
12 changes: 9 additions & 3 deletions lib/workers/repository/finalize/index.ts
Expand Up @@ -19,9 +19,7 @@ export async function finalizeRepo(
await configMigration(config, branchList);
await repositoryCache.saveCache();
await pruneStaleBranches(config, branchList);
await platform.ensureIssueClosing(
`Action Required: Fix Renovate Configuration`
);
await ensureIssuesClosing();
await clearRenovateRefs();
PackageFiles.clear();
const prList = await platform.getPrList();
Expand All @@ -39,3 +37,11 @@ export async function finalizeRepo(
runBranchSummary(config);
runRenovateRepoStats(config, prList);
}

// istanbul ignore next
viceice marked this conversation as resolved.
Show resolved Hide resolved
function ensureIssuesClosing(): Promise<Awaited<void>[]> {
return Promise.all([
platform.ensureIssueClosing(`Action Required: Fix Renovate Configuration`),
platform.ensureIssueClosing(`Action Required: Add missing credentials`),
]);
}
7 changes: 6 additions & 1 deletion lib/workers/repository/result.ts
Expand Up @@ -3,6 +3,7 @@ import type { RenovateConfig } from '../../config/types';
import {
CONFIG_SECRETS_EXPOSED,
CONFIG_VALIDATION,
MISSING_API_CREDENTIALS,
REPOSITORY_ACCESS_FORBIDDEN,
REPOSITORY_ARCHIVED,
REPOSITORY_BLOCKED,
Expand Down Expand Up @@ -54,7 +55,11 @@ export function processResult(
REPOSITORY_RENAMED,
REPOSITORY_UNINITIATED,
];
const enabledStatuses = [CONFIG_SECRETS_EXPOSED, CONFIG_VALIDATION];
const enabledStatuses = [
CONFIG_SECRETS_EXPOSED,
CONFIG_VALIDATION,
MISSING_API_CREDENTIALS,
];
let status: ProcessStatus;
let enabled: boolean | undefined;
let onboarded: boolean | undefined;
Expand Down