refactor: extract applyHostRules and applyNpmrc to functions (renovatebot#41528)

* refactor: extract `applyHostRules` and `applyNpmrc` to functions

* Reuse applyHostRules in inherited.ts

* Restore original log message for npmrc

* Simplify applyNpmrc log context handling

* Remove redundant config word

* Apply Jamie's suggestion

* Restore log messages again

* Revert "Restore log messages again"

This reverts commit 7a67c5d.

* chore: re-add log line

---------

Co-authored-by: Jamie Tanna <jamie.tanna@mend.io>

Author: felipecrs

lib/workers/repository/init/inherited.ts

@@ -16,10 +16,8 @@ import {
 } from '../../../constants/error-messages.ts';
 import { logger } from '../../../logger/index.ts';
 import { platform } from '../../../modules/platform/index.ts';
-import * as hostRules from '../../../util/host-rules.ts';
-import * as queue from '../../../util/http/queue.ts';
-import * as throttle from '../../../util/http/throttle.ts';
 import * as template from '../../../util/template/index.ts';
+import { applyHostRules } from './merge.ts';
 
 export async function mergeInheritedConfig(
   config: RenovateConfig,
@@ -127,7 +125,7 @@ export async function mergeInheritedConfig(
       secrets: config.secrets ?? {},
       variables: config.variables ?? {},
     });
-    setInheritedHostRules(filteredConfig);
+    applyHostRules(filteredConfig);
     filteredConfig = InheritConfig.set(filteredConfig);
     return mergeChildConfig(config, filteredConfig);
   }
@@ -172,28 +170,7 @@ export async function mergeInheritedConfig(
     secrets: config.secrets ?? {},
     variables: config.variables ?? {},
   });
-  setInheritedHostRules(filteredConfig);
+  applyHostRules(filteredConfig);
   filteredConfig = InheritConfig.set(filteredConfig);
   return mergeChildConfig(config, filteredConfig);
 }
-
-function setInheritedHostRules(config: RenovateConfig): void {
-  if (config.hostRules) {
-    logger.debug('Setting hostRules from config');
-    for (const rule of config.hostRules) {
-      try {
-        hostRules.add(rule);
-      } catch (err) {
-        // istanbul ignore next
-        logger.warn(
-          { err, config: rule },
-          'Error setting hostRule from config',
-        );
-      }
-    }
-    // host rules can change concurrency
-    queue.clear();
-    throttle.clear();
-    delete config.hostRules;
-  }
-}

lib/workers/repository/init/merge.spec.ts

@@ -7,14 +7,20 @@ import { getConfig } from '../../../config/defaults.ts';
 import * as _migrateAndValidate from '../../../config/migrate-validate.ts';
 import * as _migrate from '../../../config/migration.ts';
 import type { AllConfig } from '../../../config/types.ts';
+import * as npmApi from '../../../modules/datasource/npm/index.ts';
 import * as memCache from '../../../util/cache/memory/index.ts';
 import * as repoCache from '../../../util/cache/repository/index.ts';
 import { initRepoCache } from '../../../util/cache/repository/init.ts';
 import type { RepoCacheData } from '../../../util/cache/repository/types.ts';
 import { getUserEnv } from '../../../util/env.ts';
+import * as hostRules from '../../../util/host-rules.ts';
+import * as queue from '../../../util/http/queue.ts';
+import * as throttle from '../../../util/http/throttle.ts';
 import * as _onboardingCache from '../onboarding/branch/onboarding-branch-cache.ts';
 import { OnboardingState } from '../onboarding/common.ts';
 import {
+  applyHostRules,
+  applyNpmrc,
   checkForRepoConfigError,
   detectRepoFileConfig,
   mergeRenovateConfig,
@@ -503,6 +509,88 @@ describe('workers/repository/init/merge', () => {
     });
   });
 
+  describe('applyNpmrc', () => {
+    it('does nothing if npmrc is missing after token migration', () => {
+      const setNpmrcSpy = vi.spyOn(npmApi, 'setNpmrc');
+
+      applyNpmrc({});
+
+      expect(setNpmrcSpy).not.toHaveBeenCalled();
+    });
+
+    it('migrates npmToken and sets npmrc', () => {
+      const setNpmrcSpy = vi.spyOn(npmApi, 'setNpmrc');
+      const config = {
+        npmToken: 'token',
+        npmrc: 'something_authToken=${NPM_TOKEN}',
+      };
+
+      applyNpmrc(config);
+
+      expect(config.npmToken).toBeUndefined();
+      expect(config.npmrc).toBe('something_authToken=token');
+      expect(setNpmrcSpy).toHaveBeenCalledExactlyOnceWith(
+        'something_authToken=token',
+      );
+    });
+  });
+
+  describe('applyHostRules', () => {
+    it('does nothing when hostRules is not configured', () => {
+      const addSpy = vi.spyOn(hostRules, 'add');
+      const clearQueueSpy = vi.spyOn(queue, 'clear');
+      const clearThrottleSpy = vi.spyOn(throttle, 'clear');
+
+      applyHostRules({});
+
+      expect(addSpy).not.toHaveBeenCalled();
+      expect(clearQueueSpy).not.toHaveBeenCalled();
+      expect(clearThrottleSpy).not.toHaveBeenCalled();
+    });
+
+    it('adds hostRules and clears queue and throttle', () => {
+      const addSpy = vi
+        .spyOn(hostRules, 'add')
+        .mockImplementation(() => undefined);
+      const clearQueueSpy = vi.spyOn(queue, 'clear');
+      const clearThrottleSpy = vi.spyOn(throttle, 'clear');
+      const config = {
+        hostRules: [{ matchHost: 'registry.npmjs.org' }],
+      };
+
+      applyHostRules(config);
+
+      expect(addSpy).toHaveBeenCalledExactlyOnceWith({
+        matchHost: 'registry.npmjs.org',
+      });
+      expect(clearQueueSpy).toHaveBeenCalledOnce();
+      expect(clearThrottleSpy).toHaveBeenCalledOnce();
+      expect(config.hostRules).toBeUndefined();
+    });
+
+    it('warns on invalid hostRule and continues applying others', () => {
+      const addSpy = vi
+        .spyOn(hostRules, 'add')
+        .mockImplementationOnce(() => {
+          throw new Error('invalid host rule');
+        })
+        .mockImplementation(() => undefined);
+      const clearQueueSpy = vi.spyOn(queue, 'clear');
+      const clearThrottleSpy = vi.spyOn(throttle, 'clear');
+      const config = {
+        hostRules: [{ matchHost: 'one.example' }, { matchHost: 'two.example' }],
+      };
+
+      applyHostRules(config);
+
+      expect(addSpy).toHaveBeenCalledTimes(2);
+      expect(logger.logger.warn).toHaveBeenCalledOnce();
+      expect(clearQueueSpy).toHaveBeenCalledOnce();
+      expect(clearThrottleSpy).toHaveBeenCalledOnce();
+      expect(config.hostRules).toBeUndefined();
+    });
+  });
+
   describe('static repository config', () => {
     const repoStaticConfigFileKey = 'RENOVATE_X_STATIC_REPO_CONFIG_FILE';
 

lib/workers/repository/init/merge.ts

@@ -239,12 +239,7 @@ export async function mergeRenovateConfig(
   const repository = config.repository!;
   // Decrypt before resolving in case we need npm authentication for any presets
   const decryptedConfig = await decryptConfig(migratedConfig, repository);
-  setNpmTokenInNpmrc(decryptedConfig);
-  // istanbul ignore if
-  if (isString(decryptedConfig.npmrc)) {
-    logger.debug('Found npmrc in decrypted config - setting');
-    npmApi.setNpmrc(decryptedConfig.npmrc);
-  }
+  applyNpmrc(decryptedConfig, 'decrypted');
   // Decrypt after resolving in case the preset contains npm authentication instead
   const { config: configToDecrypt } = await presets.resolveConfigPresets(
     decryptedConfig,
@@ -259,14 +254,12 @@ export async function mergeRenovateConfig(
     logger.trace({ config: resolvedConfig }, 'resolved config after migrating');
     resolvedConfig = migrationResult.migratedConfig;
   }
-  setNpmTokenInNpmrc(resolvedConfig);
-  // istanbul ignore if
   if (isString(resolvedConfig.npmrc)) {
     logger.debug(
       'Ignoring any .npmrc files in repository due to configured npmrc',
     );
-    npmApi.setNpmrc(resolvedConfig.npmrc);
   }
+  applyNpmrc(resolvedConfig, 'resolved');
   resolvedConfig = applySecretsAndVariablesToConfig({
     config: resolvedConfig,
     secrets: mergeChildConfig(
@@ -279,24 +272,7 @@ export async function mergeRenovateConfig(
     ),
   });
 
-  // istanbul ignore if
-  if (resolvedConfig.hostRules) {
-    logger.debug('Setting hostRules from config');
-    for (const rule of resolvedConfig.hostRules) {
-      try {
-        hostRules.add(rule);
-      } catch (err) {
-        logger.warn(
-          { err, config: rule },
-          'Error setting hostRule from config',
-        );
-      }
-    }
-    // host rules can change concurrency
-    queue.clear();
-    throttle.clear();
-    delete resolvedConfig.hostRules;
-  }
+  applyHostRules(resolvedConfig);
   returnConfig = mergeChildConfig(returnConfig, resolvedConfig);
   ({ config: returnConfig } = await presets.resolveConfigPresets(
     returnConfig,
@@ -317,6 +293,39 @@ export async function mergeRenovateConfig(
   return returnConfig;
 }
 
+export function applyNpmrc(
+  config: RenovateConfig,
+  configType?: 'resolved' | 'decrypted',
+): void {
+  setNpmTokenInNpmrc(config);
+  if (!isString(config.npmrc)) {
+    return;
+  }
+  logger.debug(
+    `Setting npmrc from ${configType ? `${configType} ` : ''}config`,
+  );
+  npmApi.setNpmrc(config.npmrc);
+}
+
+export function applyHostRules(config: RenovateConfig): void {
+  if (!config.hostRules) {
+    return;
+  }
+
+  logger.debug('Setting hostRules from config');
+  for (const rule of config.hostRules) {
+    try {
+      hostRules.add(rule);
+    } catch (err) {
+      logger.warn({ err, config: rule }, 'Error setting hostRule from config');
+    }
+  }
+  // host rules can change concurrency
+  queue.clear();
+  throttle.clear();
+  delete config.hostRules;
+}
+
 /** needed when using portal secrets for npmToken */
 export function setNpmTokenInNpmrc(config: RenovateConfig): void {
   if (!isString(config.npmToken)) {