Skip to content

Commit 2ac7e3c

Browse files
janbucharvladfrangu
janbuchar
and
vladfrangu
authored
fix: Throw an error on missing secrets (#1039)
- closes #921 --------- Co-authored-by: Vlad Frangu <me@vladfrangu.dev>
1 parent ef6844d commit 2ac7e3c

4 files changed

Lines changed: 176 additions & 16 deletions

File tree

‎src/commands/actors/push.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ export class ActorsPushCommand extends ApifyCommand<typeof ActorsPushCommand> {
8383
description: 'Directory where the Actor is located',
8484
required: false,
8585
}),
86+
'allow-missing-secrets': Flags.boolean({
87+
description:
88+
'Allow the command to continue even when secret values are not found in the local secrets storage.',
89+
required: false,
90+
default: false,
91+
}),
8692
};
8793

8894
static override args = {
@@ -293,7 +299,9 @@ Skipping push. Use --force to override.`,
293299
// Update Actor version
294300
const actorCurrentVersion = await actorClient.version(version).get();
295301
const envVars = actorConfig!.environmentVariables
296-
? transformEnvToEnvVars(actorConfig!.environmentVariables as Record<string, string>)
302+
? transformEnvToEnvVars(actorConfig!.environmentVariables as Record<string, string>, undefined, {
303+
allowMissing: this.flags.allowMissingSecrets,
304+
})
297305
: undefined;
298306

299307
if (actorCurrentVersion) {

‎src/commands/run.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ export class RunCommand extends ApifyCommand<typeof RunCommand> {
9898
stdin: StdinMode.Stringified,
9999
exclusive: ['input'],
100100
}),
101+
'allow-missing-secrets': Flags.boolean({
102+
description:
103+
'Allow the command to continue even when secret values are not found in the local secrets storage.',
104+
required: false,
105+
default: false,
106+
}),
101107
};
102108

103109
async run() {
@@ -265,7 +271,11 @@ export class RunCommand extends ApifyCommand<typeof RunCommand> {
265271
if (userId) localEnvVars[APIFY_ENV_VARS.USER_ID] = userId;
266272
if (token) localEnvVars[APIFY_ENV_VARS.TOKEN] = token;
267273
if (localConfig!.environmentVariables) {
268-
const updatedEnv = replaceSecretsValue(localConfig!.environmentVariables as Record<string, string>);
274+
const updatedEnv = replaceSecretsValue(
275+
localConfig!.environmentVariables as Record<string, string>,
276+
undefined,
277+
{ allowMissing: this.flags.allowMissingSecrets },
278+
);
269279
Object.assign(localEnvVars, updatedEnv);
270280
}
271281

‎src/lib/secrets.ts‎

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,25 +55,47 @@ const isSecretKey = (envValue: string) => {
5555
* @param env
5656
* @param secrets - Object with secrets, if not set, will be load from secrets file.
5757
*/
58-
export const replaceSecretsValue = (env: Record<string, string>, secrets?: Record<string, string>) => {
58+
export const replaceSecretsValue = (
59+
env: Record<string, string>,
60+
secrets?: Record<string, string>,
61+
{ allowMissing = false }: { allowMissing?: boolean } = {},
62+
) => {
5963
secrets = secrets || getSecretsFile();
6064
const updatedEnv = {};
65+
const missingSecrets: string[] = [];
66+
6167
Object.keys(env).forEach((key) => {
6268
if (isSecretKey(env[key])) {
6369
const secretKey = env[key].replace(new RegExp(`^${SECRET_KEY_PREFIX}`), '');
6470
if (secrets![secretKey]) {
6571
// @ts-expect-error - we are replacing the value
6672
updatedEnv[key] = secrets[secretKey];
6773
} else {
68-
warning({
69-
message: `Value for ${secretKey} not found in local secrets. Set it by calling "apify secrets add ${secretKey} [SECRET_VALUE]"`,
70-
});
74+
missingSecrets.push(secretKey);
7175
}
7276
} else {
7377
// @ts-expect-error - we are replacing the value
7478
updatedEnv[key] = env[key];
7579
}
7680
});
81+
82+
if (missingSecrets.length > 0) {
83+
const secretsList = missingSecrets.map((s) => ` - ${s}`).join('\n');
84+
if (allowMissing) {
85+
for (const secretKey of missingSecrets) {
86+
warning({
87+
message: `Value for ${secretKey} not found in local secrets. Set it by calling "apify secrets add ${secretKey} [SECRET_VALUE]"`,
88+
});
89+
}
90+
} else {
91+
throw new Error(
92+
`The following secrets are missing:\n${secretsList}\n\n` +
93+
`Set them by calling "apify secrets add <SECRET_NAME> <SECRET_VALUE>" for each missing secret.\n` +
94+
`If you want to skip missing secrets, run the command with the --allow-missing-secrets flag.`,
95+
);
96+
}
97+
}
98+
7799
return updatedEnv;
78100
};
79101

@@ -88,9 +110,15 @@ interface EnvVar {
88110
* It replaces secrets to values from secrets file.
89111
* @param secrets - Object with secrets, if not set, will be load from secrets file.
90112
*/
91-
export const transformEnvToEnvVars = (env: Record<string, string>, secrets?: Record<string, string>) => {
113+
export const transformEnvToEnvVars = (
114+
env: Record<string, string>,
115+
secrets?: Record<string, string>,
116+
{ allowMissing = false }: { allowMissing?: boolean } = {},
117+
) => {
92118
secrets = secrets || getSecretsFile();
93119
const envVars: EnvVar[] = [];
120+
const missingSecrets: string[] = [];
121+
94122
Object.keys(env).forEach((key) => {
95123
if (isSecretKey(env[key])) {
96124
const secretKey = env[key].replace(new RegExp(`^${SECRET_KEY_PREFIX}`), '');
@@ -101,9 +129,7 @@ export const transformEnvToEnvVars = (env: Record<string, string>, secrets?: Rec
101129
isSecret: true,
102130
});
103131
} else {
104-
warning({
105-
message: `Value for ${secretKey} not found in local secrets. Set it by calling "apify secrets add ${secretKey} [SECRET_VALUE]"`,
106-
});
132+
missingSecrets.push(secretKey);
107133
}
108134
} else {
109135
envVars.push({
@@ -112,5 +138,23 @@ export const transformEnvToEnvVars = (env: Record<string, string>, secrets?: Rec
112138
});
113139
}
114140
});
141+
142+
if (missingSecrets.length > 0) {
143+
const secretsList = missingSecrets.map((s) => ` - ${s}`).join('\n');
144+
if (allowMissing) {
145+
for (const secretKey of missingSecrets) {
146+
warning({
147+
message: `Value for ${secretKey} not found in local secrets. Set it by calling "apify secrets add ${secretKey} [SECRET_VALUE]"`,
148+
});
149+
}
150+
} else {
151+
throw new Error(
152+
`The following secrets are missing:\n${secretsList}\n\n` +
153+
`Set them by calling "apify secrets add <SECRET_NAME> <SECRET_VALUE>" for each missing secret.\n` +
154+
`If you want to skip missing secrets, run the command with the --allow-missing-secrets flag.`,
155+
);
156+
}
157+
}
158+
115159
return envVars;
116160
};

‎test/local/lib/secrets.test.ts‎

Lines changed: 104 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
import { replaceSecretsValue } from '../../../src/lib/secrets.js';
1+
import { replaceSecretsValue, transformEnvToEnvVars } from '../../../src/lib/secrets.js';
22

33
describe('Secrets', () => {
44
describe('replaceSecretsValue()', () => {
5-
it('should work', () => {
6-
const spy = vitest.spyOn(console, 'error');
7-
5+
it('should replace secret references with their values', () => {
86
const secrets = {
97
myProdToken: 'mySecretToken',
108
mongoUrl: 'mongo://bla@bla:supermongo.com:27017',
@@ -13,7 +11,6 @@ describe('Secrets', () => {
1311
TOKEN: '@myProdToken',
1412
USER: 'jakub.drobnik@apify.com',
1513
MONGO_URL: '@mongoUrl',
16-
WARNING: '@doesNotExist',
1714
};
1815
const updatedEnv = replaceSecretsValue(env, secrets);
1916

@@ -22,9 +19,110 @@ describe('Secrets', () => {
2219
USER: 'jakub.drobnik@apify.com',
2320
MONGO_URL: secrets.mongoUrl,
2421
});
22+
});
23+
24+
it('should throw an error when secrets are missing', () => {
25+
const secrets = {
26+
myProdToken: 'mySecretToken',
27+
};
28+
const env = {
29+
TOKEN: '@myProdToken',
30+
MISSING_ONE: '@doesNotExist',
31+
MISSING_TWO: '@alsoMissing',
32+
};
33+
34+
expect(() => replaceSecretsValue(env, secrets)).toThrow(
35+
/The following secrets are missing:\n\s+- doesNotExist\n\s+- alsoMissing/,
36+
);
37+
});
38+
39+
it('should mention --allow-missing-secrets in the error message', () => {
40+
const env = { TOKEN: '@doesNotExist' };
41+
42+
expect(() => replaceSecretsValue(env, {})).toThrow(/--allow-missing-secrets/);
43+
});
44+
45+
it('should warn instead of throwing when allowMissing is true', () => {
46+
const spy = vitest.spyOn(console, 'error');
47+
48+
const secrets = {
49+
myProdToken: 'mySecretToken',
50+
};
51+
const env = {
52+
TOKEN: '@myProdToken',
53+
MISSING_ONE: '@doesNotExist',
54+
MISSING_TWO: '@alsoMissing',
55+
};
56+
57+
const updatedEnv = replaceSecretsValue(env, secrets, {
58+
allowMissing: true,
59+
});
60+
61+
expect(updatedEnv).toStrictEqual({
62+
TOKEN: secrets.myProdToken,
63+
});
64+
65+
expect(spy).toHaveBeenCalled();
66+
expect(spy.mock.calls.flat().join(' ')).to.include('doesNotExist');
67+
68+
spy.mockRestore();
69+
});
70+
});
71+
72+
describe('transformEnvToEnvVars()', () => {
73+
it('should transform env to envVars format with secret resolution', () => {
74+
const secrets = {
75+
myProdToken: 'mySecretToken',
76+
};
77+
const env = {
78+
TOKEN: '@myProdToken',
79+
USER: 'jakub.drobnik@apify.com',
80+
};
81+
const envVars = transformEnvToEnvVars(env, secrets);
82+
83+
expect(envVars).toStrictEqual([
84+
{ name: 'TOKEN', value: 'mySecretToken', isSecret: true },
85+
{ name: 'USER', value: 'jakub.drobnik@apify.com' },
86+
]);
87+
});
88+
89+
it('should throw an error when secrets are missing', () => {
90+
const secrets = {};
91+
const env = {
92+
TOKEN: '@doesNotExist',
93+
USER: 'plain-value',
94+
};
95+
96+
expect(() => transformEnvToEnvVars(env, secrets)).toThrow(
97+
/The following secrets are missing:\n\s+- doesNotExist/,
98+
);
99+
});
100+
101+
it('should mention --allow-missing-secrets in the error message', () => {
102+
const env = { TOKEN: '@doesNotExist' };
103+
104+
expect(() => transformEnvToEnvVars(env, {})).toThrow(/--allow-missing-secrets/);
105+
});
106+
107+
it('should warn instead of throwing when allowMissing is true', () => {
108+
const spy = vitest.spyOn(console, 'error');
109+
110+
const secrets = {};
111+
const env = {
112+
TOKEN: '@doesNotExist',
113+
USER: 'plain-value',
114+
};
115+
116+
const envVars = transformEnvToEnvVars(env, secrets, {
117+
allowMissing: true,
118+
});
119+
120+
expect(envVars).toStrictEqual([{ name: 'USER', value: 'plain-value' }]);
25121

26122
expect(spy).toHaveBeenCalled();
27-
expect(spy.mock.calls[0][0]).to.include('Warning:');
123+
expect(spy.mock.calls.flat().join(' ')).to.include('doesNotExist');
124+
125+
spy.mockRestore();
28126
});
29127
});
30128
});

0 commit comments

Comments
 (0)