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(migration): throw error for invalid hostRules #20540

Merged
merged 27 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8b7ef1d
throw error during migrtaion
RahulGautamSingh Feb 21, 2023
b345f18
use CONFIG_VALIDATION error
RahulGautamSingh Feb 21, 2023
3c5b7ad
fix tests
RahulGautamSingh Feb 21, 2023
2974816
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh Feb 21, 2023
1c0e606
spply suggestions
RahulGautamSingh Feb 21, 2023
fab15cc
Merge branch 'fix/host-rules-migration' of https://github.com/RahulGa…
RahulGautamSingh Feb 21, 2023
7bc58c7
fix test
RahulGautamSingh Feb 21, 2023
e725000
fix test
RahulGautamSingh Feb 23, 2023
92f8800
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh Feb 23, 2023
0bc8cb3
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh Mar 22, 2023
f45e152
clone params-obj before modifying
RahulGautamSingh Mar 22, 2023
85aef37
fix test
RahulGautamSingh Mar 22, 2023
34b86ee
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh Apr 12, 2023
588fda1
- revert changes to add() and migrateRule()
RahulGautamSingh Apr 26, 2023
056a77d
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh Apr 26, 2023
8ea2539
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh Apr 27, 2023
ec7fde9
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh Apr 27, 2023
9a9f9a7
fix lint issue
RahulGautamSingh Apr 27, 2023
08e0ff4
allow duplicate hosts with warning
RahulGautamSingh May 16, 2023
d5bb429
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh May 16, 2023
02c9952
log key/value pairs
RahulGautamSingh May 19, 2023
ac41339
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh May 19, 2023
bcf59c1
refactor
RahulGautamSingh May 19, 2023
d49258c
Merge branch 'fix/host-rules-migration' of https://github.com/RahulGa…
RahulGautamSingh May 19, 2023
9fe92b1
use for..of
RahulGautamSingh May 19, 2023
2b9bbd5
remove nested fns
RahulGautamSingh May 19, 2023
86486a6
Merge branch 'main' into fix/host-rules-migration
RahulGautamSingh May 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion lib/config/migration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ describe('config/migration', () => {
{
platform: 'docker',
endpoint: 'https://docker.io',
host: 'docker.io',
rarkins marked this conversation as resolved.
Show resolved Hide resolved
username: 'some-username',
password: 'some-password',
},
Expand Down
39 changes: 39 additions & 0 deletions lib/config/migrations/custom/host-rules-migration.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CONFIG_VALIDATION } from '../../../constants/error-messages';
import { HostRulesMigration } from './host-rules-migration';

describe('config/migrations/custom/host-rules-migration', () => {
Expand All @@ -10,6 +11,12 @@ describe('config/migrations/custom/host-rules-migration', () => {
baseUrl: 'https://some.domain.com',
token: '123test',
},
{
hostType: 'dotnet',
baseUrl: 'https://some.domain.com',
matchHost: 'https://some.domain.com',
token: '123test',
},
{
hostType: 'adoptium-java',
domainName: 'domain.com',
Expand All @@ -18,10 +25,17 @@ describe('config/migrations/custom/host-rules-migration', () => {
{ domainName: 'domain.com/', token: '123test' },
{ hostType: 'docker', matchHost: 'domain.com/', token: '123test' },
{ hostName: 'some.domain.com', token: '123test' },
{ endpoint: 'domain.com/', token: '123test' },
{ host: 'some.domain.com', token: '123test' },
],
} as any,
{
hostRules: [
{
hostType: 'dotnet-version',
matchHost: 'https://some.domain.com',
token: '123test',
},
{
hostType: 'dotnet-version',
matchHost: 'https://some.domain.com',
Expand All @@ -42,8 +56,33 @@ describe('config/migrations/custom/host-rules-migration', () => {
token: '123test',
},
{ matchHost: 'some.domain.com', token: '123test' },
{ matchHost: 'https://domain.com/', token: '123test' },
{ matchHost: 'some.domain.com', token: '123test' },
],
}
);
});

it('throws when multiple hosts are present', () => {
expect(() =>
new HostRulesMigration(
{
hostRules: [
{
matchHost: 'https://some-diff.domain.com',
baseUrl: 'https://some.domain.com',
token: '123test',
},
],
} as any,
{}
).run([
{
matchHost: 'https://some-diff.domain.com',
baseUrl: 'https://some.domain.com',
token: '123test',
},
])
).toThrow(CONFIG_VALIDATION);
});
});
48 changes: 47 additions & 1 deletion lib/config/migrations/custom/host-rules-migration.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import is from '@sindresorhus/is';
import { CONFIG_VALIDATION } from '../../../constants/error-messages';
import { logger } from '../../../logger';
import type { HostRule } from '../../../types';
import type { LegacyHostRule } from '../../../util/host-rules';
import { AbstractMigration } from '../base/abstract-migration';
import { migrateDatasource } from './datasource-migration';

export class HostRulesMigration extends AbstractMigration {
override readonly propertyName = 'hostRules';

override run(value: Record<string, unknown>[]): void {
override run(value: (LegacyHostRule & HostRule)[]): void {
const newHostRules: HostRule[] = [];
for (const hostRule of value) {
validateHostRule(hostRule);
const newRule: any = {};

for (const [key, value] of Object.entries(hostRule)) {
Expand Down Expand Up @@ -56,6 +60,48 @@ export class HostRulesMigration extends AbstractMigration {
}
}

function validateHostRule(rule: LegacyHostRule & HostRule): void {
const { matchHost, hostName, domainName, baseUrl, endpoint, host } = rule;
const hosts: Record<string, string> = removeUndefinedFields({
matchHost,
hostName,
domainName,
baseUrl,
endpoint,
host,
});

function removeUndefinedFields(
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
obj: Record<string, any>
): Record<string, string> {
const keys = Object.keys(obj);
for (const key of keys) {
if (obj[key] === undefined) {
delete obj[key];
}
}
return obj;
}

if (Object.keys(hosts).length > 1) {
const distinctHostValues = new Set(Object.values(hosts));
// check if the host values are duplicated
if (distinctHostValues.size > 1) {
const error = new Error(CONFIG_VALIDATION);
error.validationSource = 'config';
error.validationMessage = `hostRules cannot contain more than one host-matching field - use "matchHost" only.`;
error.validationError =
'The renovate configuration file contains some invalid settings';
throw error;
} else {
logger.warn(
{ hosts },
'Duplicate host values found, please only use `matchHost` to specify the host'
);
}
}
}

function massageUrl(url: string): string {
if (!url.includes('://') && url.includes('/')) {
return 'https://' + url;
Expand Down
6 changes: 4 additions & 2 deletions lib/util/host-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import { parseUrl, validateUrl } from './url';

let hostRules: HostRule[] = [];

interface LegacyHostRule {
export interface LegacyHostRule {
hostName?: string;
domainName?: string;
baseUrl?: string;
host?: string;
endpoint?: string;
}

function migrateRule(rule: LegacyHostRule & HostRule): HostRule {
export function migrateRule(rule: LegacyHostRule & HostRule): HostRule {
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
const cloned: LegacyHostRule & HostRule = structuredClone(rule);
delete cloned.hostName;
delete cloned.domainName;
Expand Down