From d88d63b4f9de0cfce10ccb95a570d015bcab4386 Mon Sep 17 00:00:00 2001
From: Gabriel-Ladzaretti
<97394622+Gabriel-Ladzaretti@users.noreply.github.com>
Date: Thu, 18 May 2023 20:02:13 +0300
Subject: [PATCH] feat(config-error): raise a warning issue for misconfigured
`matchConfidence` (#22296)
Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com>
---
lib/config/options/index.ts | 1 +
lib/constants/error-messages.ts | 3 +
lib/util/merge-confidence/index.ts | 2 +-
lib/util/package-rules/index.spec.ts | 138 +++++++++++++-------
lib/util/package-rules/merge-confidence.ts | 14 ++
lib/workers/repository/error-config.spec.ts | 16 ++-
lib/workers/repository/error-config.ts | 11 ++
lib/workers/repository/error.spec.ts | 2 +
lib/workers/repository/error.ts | 12 +-
lib/workers/repository/finalize/index.ts | 12 +-
lib/workers/repository/result.ts | 7 +-
11 files changed, 161 insertions(+), 57 deletions(-)
diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts
index a41fb0fdd7..149a9ecc82 100644
--- a/lib/config/options/index.ts
+++ b/lib/config/options/index.ts
@@ -2406,6 +2406,7 @@ const options: RenovateOptions[] = [
'configErrorIssue',
'deprecationWarningIssues',
'lockFileErrors',
+ 'missingCredentialsError',
'onboardingClose',
'prEditedNotification',
'prIgnoreNotification',
diff --git a/lib/constants/error-messages.ts b/lib/constants/error-messages.ts
index 21cc4e88cb..490a325f0e 100644
--- a/lib/constants/error-messages.ts
+++ b/lib/constants/error-messages.ts
@@ -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
+export const MISSING_API_CREDENTIALS = 'missing-api-credentials';
diff --git a/lib/util/merge-confidence/index.ts b/lib/util/merge-confidence/index.ts
index d8be2bd742..1ce49eb86c 100644
--- a/lib/util/merge-confidence/index.ts
+++ b/lib/util/merge-confidence/index.ts
@@ -216,7 +216,7 @@ function getApiBaseUrl(): string {
}
}
-function getApiToken(): string | undefined {
+export function getApiToken(): string | undefined {
return hostRules.find({
hostType,
})?.token;
diff --git a/lib/util/package-rules/index.spec.ts b/lib/util/package-rules/index.spec.ts
index dc3d30bc62..9e6f48f4b3 100644
--- a/lib/util/package-rules/index.spec.ts
+++ b/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';
@@ -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', () => {
+ 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.validationMessage).toBe('Missing credentials');
+ expect(error.validationError).toBe(
+ 'The `matchConfidence` matcher in `packageRules` requires authentication. Please refer to the [documentation](https://docs.renovatebot.com/configuration-options/#matchconfidence) and add the required host rule.'
+ );
+ });
});
it('filters naked depType', () => {
diff --git a/lib/util/package-rules/merge-confidence.ts b/lib/util/package-rules/merge-confidence.ts
index a29a6cfb55..9836892f49 100644
--- a/lib/util/package-rules/merge-confidence.ts
+++ b/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 {
@@ -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.validationMessage = 'Missing credentials';
+ error.validationError =
+ 'The `matchConfidence` matcher in `packageRules` requires authentication. Please refer to the [documentation](https://docs.renovatebot.com/configuration-options/#matchconfidence) and add the required host rule.';
+ throw error;
+ }
+
return (
is.array(matchConfidence) &&
is.nonEmptyString(mergeConfidenceLevel) &&
diff --git a/lib/workers/repository/error-config.spec.ts b/lib/workers/repository/error-config.spec.ts
index 109bc7aa7d..8afce57c48 100644
--- a/lib/workers/repository/error-config.spec.ts
+++ b/lib/workers/repository/error-config.spec.ts
@@ -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');
@@ -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 () => {
diff --git a/lib/workers/repository/error-config.ts b/lib/workers/repository/error-config.ts
index 54a6d659e0..b580d909f4 100644
--- a/lib/workers/repository/error-config.ts
+++ b/lib/workers/repository/error-config.ts
@@ -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,
diff --git a/lib/workers/repository/error.spec.ts b/lib/workers/repository/error.spec.ts
index 891dd33e2b..3271f7bfbb 100644
--- a/lib/workers/repository/error.spec.ts
+++ b/lib/workers/repository/error.spec.ts
@@ -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,
@@ -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,
diff --git a/lib/workers/repository/error.ts b/lib/workers/repository/error.ts
index 7517891314..9effb449b6 100644
--- a/lib/workers/repository/error.ts
+++ b/lib/workers/repository/error.ts
@@ -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,
@@ -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,
@@ -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(
diff --git a/lib/workers/repository/finalize/index.ts b/lib/workers/repository/finalize/index.ts
index 0a5064e445..e0afb3893f 100644
--- a/lib/workers/repository/finalize/index.ts
+++ b/lib/workers/repository/finalize/index.ts
@@ -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();
@@ -39,3 +37,11 @@ export async function finalizeRepo(
runBranchSummary(config);
runRenovateRepoStats(config, prList);
}
+
+// istanbul ignore next
+function ensureIssuesClosing(): Promise<Awaited<void>[]> {
+ return Promise.all([
+ platform.ensureIssueClosing(`Action Required: Fix Renovate Configuration`),
+ platform.ensureIssueClosing(`Action Required: Add missing credentials`),
+ ]);
+}
diff --git a/lib/workers/repository/result.ts b/lib/workers/repository/result.ts
index 9868f3edec..c228db0ed8 100644
--- a/lib/workers/repository/result.ts
+++ b/lib/workers/repository/result.ts
@@ -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,
@@ -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;
--
GitLab