feat(config-error): raise a warning issue for misconfigured `matchCon…

…fidence` (#22296) Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com>
renovatebot · May 18, 2023 · d88d63b · d88d63b
1 parent 8cdd1a2
commit d88d63b
Show file tree

Hide file tree

Showing 11 changed files with 161 additions and 57 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;