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

fix(config/inherit): resolve presets #31642

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
11 changes: 11 additions & 0 deletions docs/usage/config-overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,17 @@ Inherited config may use all Repository config settings, and any Global config o

For information on how the Mend Renovate App supports Inherited config, see the dedicated "Mend Renovate App Config" section toward the end of this page.

#### Presets handling

If the inherited config has presets, then Renovate:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If the inherited config has presets, then Renovate:
If the inherited config contains `extends` presets, then Renovate:

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If the inherited config has presets, then Renovate:
If the inherited config contains `extends` presets, then Renovate will:


1. Resolves the presets
1. Adds the resolved presets to the inherited config
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. Adds the resolved presets to the inherited config
1. Add resolved preset config at the beginning of the inherited config

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. Adds the resolved presets to the inherited config
1. Add the resolved preset config to the beginning of the inherited config

Maybe this is better. If we go with @rarkins's suggestion, we should keep the the word. 😉

1. Merges the presets on top of the global config
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. Merges the presets on top of the global config
1. Merge the presets on top of the global config


RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
Using `ignorePresets` in your repository config to ignore certain presets will _not_ affect presets present in the inherited config.
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
This is because inherited config is resolved _before_ the repository config.

### Repository config

Repository config is the config loaded from a config file in the repository.
Expand Down
136 changes: 135 additions & 1 deletion lib/workers/repository/init/inherited.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { platform } from '../../../../test/util';
import { mocked, platform } from '../../../../test/util';
import * as presets_ from '../../../config/presets';
import type { RenovateConfig } from '../../../config/types';
import * as validation from '../../../config/validation';
import {
CONFIG_INHERIT_NOT_FOUND,
CONFIG_INHERIT_PARSE_ERROR,
Expand All @@ -8,6 +10,10 @@ import {
import { logger } from '../../../logger';
import { mergeInheritedConfig } from './inherited';

jest.mock('../../../config/presets');

const presets = mocked(presets_);

describe('workers/repository/init/inherited', () => {
let config: RenovateConfig;

Expand Down Expand Up @@ -84,4 +90,132 @@ describe('workers/repository/init/inherited', () => {
expect(res.onboarding).toBeFalse();
expect(logger.warn).not.toHaveBeenCalled();
});

it('should resolve presets found in inherited config', async () => {
platform.getRawFile.mockResolvedValue(
'{"onboarding":false,"labels":["test"],"extends":[":automergeAll"]}',
);
presets.resolveConfigPresets.mockResolvedValue({
onboarding: false,
labels: ['test'],
automerge: true,
});
const res = await mergeInheritedConfig(config);
expect(res.labels).toEqual(['test']);
expect(res.onboarding).toBeFalse();
expect(logger.warn).not.toHaveBeenCalled();
expect(logger.debug).toHaveBeenCalledWith(
'Resolving presets found in inherited config',
);
});

it('should warn if presets fails validation with warnings', async () => {
platform.getRawFile.mockResolvedValue(
'{"onboarding":false,"labels":["test"],"extends":[":automergeAll"]}',
);
jest
.spyOn(validation, 'validateConfig')
.mockResolvedValueOnce({
warnings: [],
errors: [],
})
.mockResolvedValueOnce({
warnings: [
{
message: 'some warning',
topic: 'Configuration Error',
},
],
errors: [],
});
presets.resolveConfigPresets.mockResolvedValue({
onboarding: false,
labels: ['test'],
automerge: true,
});
const res = await mergeInheritedConfig(config);
expect(res.binarySource).toBeUndefined();
expect(logger.warn).toHaveBeenCalledWith(
{
warnings: [
{
message: 'some warning',
topic: 'Configuration Error',
},
],
},
'Found warnings in presets inside the inherited configuration.',
);
});

it('should throw error if presets fails validation with errors', async () => {
platform.getRawFile.mockResolvedValue(
'{"labels":["test"],"extends":[":automergeAll"]}',
);
jest
.spyOn(validation, 'validateConfig')
.mockResolvedValueOnce({
warnings: [],
errors: [],
})
.mockResolvedValueOnce({
warnings: [],
errors: [
{
message: 'some error',
topic: 'Configuration Error',
},
],
});
presets.resolveConfigPresets.mockResolvedValue({
labels: ['test'],
automerge: true,
});
await expect(mergeInheritedConfig(config)).rejects.toThrow(
CONFIG_VALIDATION,
);
expect(logger.warn).toHaveBeenCalledWith(
{
errors: [
{
message: 'some error',
topic: 'Configuration Error',
},
],
},
'Found errors in presets inside the inherited configuration.',
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
);
});

it('should remove global config from presets found in inherited config', async () => {
platform.getRawFile.mockResolvedValue(
'{"labels":["test"],"extends":[":automergeAll"]}',
);
jest.spyOn(validation, 'validateConfig').mockResolvedValue({
warnings: [],
errors: [],
});
presets.resolveConfigPresets.mockResolvedValue({
labels: ['test'],
automerge: true,
binarySource: 'docker', // global config option: should not be here
});
const res = await mergeInheritedConfig(config);
expect(res.labels).toEqual(['test']);
expect(logger.warn).not.toHaveBeenCalled();
expect(logger.debug).toHaveBeenCalledWith(
{
inheritedConfig: {
labels: ['test'],
automerge: true,
binarySource: 'docker',
},
filteredConfig: {
labels: ['test'],
automerge: true,
},
},
'Removed global config from inherited config presets.',
);
});
});
44 changes: 43 additions & 1 deletion lib/workers/repository/init/inherited.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import is from '@sindresorhus/is';
import { dequal } from 'dequal';
import { mergeChildConfig, removeGlobalConfig } from '../../../config';
import { decryptConfig } from '../../../config/decrypt';
import { parseFileConfig } from '../../../config/parse';
import { resolveConfigPresets } from '../../../config/presets';
import type { RenovateConfig } from '../../../config/types';
import { validateConfig } from '../../../config/validation';
import {
Expand Down Expand Up @@ -99,5 +101,45 @@ export async function mergeInheritedConfig(
'Removed global config from inherited config.',
);
}
return mergeChildConfig(config, filteredConfig);

if (is.nullOrUndefined(filteredConfig.extends)) {
return mergeChildConfig(config, filteredConfig);
}

let returnConfig = filteredConfig;

// Decrypt before resloving incase it contains npm authentication for any preset
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
const decryptedConfig = await decryptConfig(returnConfig, config.repository);

// Decrypt after resolving in case the preset contains npm authentication instead
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
logger.debug('Resolving presets found in inherited config');
const resolvedConfig = await decryptConfig(
await resolveConfigPresets(decryptedConfig, config, config.ignorePresets),
config.repository,
);
logger.trace({ config: resolvedConfig }, 'Resolved inherited config');
const validationRes = await validateConfig('inherit', resolvedConfig);
if (validationRes.errors.length) {
logger.warn(
{ errors: validationRes.errors },
'Found errors in presets inside the inherited configuration.',
);
throw new Error(CONFIG_VALIDATION);
}
if (validationRes.warnings.length) {
logger.warn(
{ warnings: validationRes.warnings },
'Found warnings in presets inside the inherited configuration.',
);
}

returnConfig = removeGlobalConfig(resolvedConfig, true);
if (!dequal(resolvedConfig, returnConfig)) {
logger.debug(
{ inheritedConfig: resolvedConfig, filteredConfig: returnConfig },
'Removed global config from inherited config presets.',
);
Comment on lines +133 to +138
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inherited config does allow some fields which were previously considered "global", such as onboarding. Please check that we don't log in this case

}

return mergeChildConfig(config, returnConfig);
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
}