Skip to content

Commit 8e5616f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2046 from feloy/feat/async-exec-auth
makes exec_auth spawn the command asynchronously
2 parents 2af0a38 + d72dd5e commit 8e5616f

File tree

2 files changed

+204
-64
lines changed

2 files changed

+204
-64
lines changed

‎src/exec_auth.ts‎

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ export interface Credential {
2020
export class ExecAuth implements Authenticator {
2121
private readonly tokenCache: { [key: string]: Credential | null } = {};
2222
private execFn: (
23-
cmd: string,
24-
args: string[],
25-
opts: child_process.SpawnOptions,
26-
) => child_process.SpawnSyncReturns<Buffer> = child_process.spawnSync;
23+
command: string,
24+
args?: readonly string[],
25+
options?: child_process.SpawnOptionsWithoutStdio,
26+
) => child_process.ChildProcessWithoutNullStreams = child_process.spawn;
2727

2828
public isAuthProvider(user: User): boolean {
2929
if (!user) {
@@ -41,7 +41,7 @@ export class ExecAuth implements Authenticator {
4141
}
4242

4343
public async applyAuthentication(user: User, opts: https.RequestOptions): Promise<void> {
44-
const credential = this.getCredential(user);
44+
const credential = await this.getCredential(user);
4545
if (!credential) {
4646
return;
4747
}
@@ -70,7 +70,7 @@ export class ExecAuth implements Authenticator {
7070
return null;
7171
}
7272

73-
private getCredential(user: User): Credential | null {
73+
private async getCredential(user: User): Promise<Credential | null> {
7474
// TODO: Add a unit test for token caching.
7575
const cachedToken = this.tokenCache[user.name];
7676
if (cachedToken) {
@@ -99,15 +99,45 @@ export class ExecAuth implements Authenticator {
9999
exec.env.forEach((elt) => (env[elt.name] = elt.value));
100100
opts = { ...opts, env };
101101
}
102-
const result = this.execFn(exec.command, exec.args, opts);
103-
if (result.error) {
104-
throw result.error;
105-
}
106-
if (result.status === 0) {
107-
const obj = JSON.parse(result.stdout.toString('utf8')) as Credential;
108-
this.tokenCache[user.name] = obj;
109-
return obj;
110-
}
111-
throw new Error(result.stderr.toString('utf8'));
102+
103+
return new Promise((resolve, reject) => {
104+
let stdoutData: string = '';
105+
let stderrData: string = '';
106+
let savedError: Error | undefined = undefined;
107+
108+
const subprocess = this.execFn(exec.command, exec.args, opts);
109+
subprocess.stdout.setEncoding('utf8');
110+
subprocess.stderr.setEncoding('utf8');
111+
112+
subprocess.stdout.on('data', (data: string) => {
113+
stdoutData += data;
114+
});
115+
116+
subprocess.stderr.on('data', (data: string) => {
117+
stderrData += data;
118+
});
119+
120+
subprocess.on('error', (error) => {
121+
savedError = error;
122+
});
123+
124+
subprocess.on('close', (code) => {
125+
if (savedError) {
126+
reject(savedError);
127+
return;
128+
}
129+
if (code !== 0) {
130+
reject(new Error(stderrData));
131+
return;
132+
}
133+
try {
134+
const obj = JSON.parse(stdoutData) as Credential;
135+
this.tokenCache[user.name] = obj;
136+
resolve(obj);
137+
} catch (error) {
138+
reject(error);
139+
}
140+
});
141+
});
112142
}
113143
}

‎src/exec_auth_test.ts‎

Lines changed: 158 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,30 @@ describe('ExecAuth', () => {
6060
const auth = new ExecAuth();
6161
(auth as any).execFn = (
6262
command: string,
63-
args: string[],
64-
opts: child_process.SpawnOptions,
65-
): child_process.SpawnSyncReturns<Buffer> => {
63+
args?: readonly string[],
64+
options?: child_process.SpawnOptionsWithoutStdio,
65+
): child_process.ChildProcessWithoutNullStreams => {
6666
return {
67-
status: 0,
68-
stdout: Buffer.from(JSON.stringify({ status: { token: 'foo' } })),
69-
} as child_process.SpawnSyncReturns<Buffer>;
67+
stdout: {
68+
setEncoding: () => {},
69+
on: (_data: string, f: (data: Buffer | string) => void) => {
70+
f(Buffer.from(JSON.stringify({ status: { token: 'foo' } })));
71+
},
72+
},
73+
stderr: {
74+
setEncoding: () => {},
75+
on: () => {},
76+
},
77+
on: (op: string, f: any) => {
78+
if (op === 'close') {
79+
f(0);
80+
}
81+
},
82+
} as unknown as child_process.ChildProcessWithoutNullStreams;
7083
};
7184
const opts = {} as https.RequestOptions;
7285
opts.headers = {} as OutgoingHttpHeaders;
73-
auth.applyAuthentication(
86+
await auth.applyAuthentication(
7487
{
7588
name: 'user',
7689
authProvider: {
@@ -94,15 +107,32 @@ describe('ExecAuth', () => {
94107
const auth = new ExecAuth();
95108
(auth as any).execFn = (
96109
command: string,
97-
args: string[],
98-
opts: child_process.SpawnOptions,
99-
): child_process.SpawnSyncReturns<Buffer> => {
110+
args?: readonly string[],
111+
options?: child_process.SpawnOptionsWithoutStdio,
112+
): child_process.ChildProcessWithoutNullStreams => {
100113
return {
101-
status: 0,
102-
stdout: Buffer.from(
103-
JSON.stringify({ status: { clientCertificateData: 'foo', clientKeyData: 'bar' } }),
104-
),
105-
} as child_process.SpawnSyncReturns<Buffer>;
114+
stdout: {
115+
setEncoding: () => {},
116+
on: (_data: string, f: (data: Buffer | string) => void) => {
117+
f(
118+
Buffer.from(
119+
JSON.stringify({
120+
status: { clientCertificateData: 'foo', clientKeyData: 'bar' },
121+
}),
122+
),
123+
);
124+
},
125+
},
126+
stderr: {
127+
setEncoding: () => {},
128+
on: () => {},
129+
},
130+
on: (op: string, f: any) => {
131+
if (op === 'close') {
132+
f(0);
133+
}
134+
},
135+
} as unknown as child_process.ChildProcessWithoutNullStreams;
106136
};
107137

108138
const user = {
@@ -119,7 +149,7 @@ describe('ExecAuth', () => {
119149
opts.headers = {} as OutgoingHttpHeaders;
120150
opts.headers = {} as OutgoingHttpHeaders;
121151

122-
auth.applyAuthentication(user, opts);
152+
await auth.applyAuthentication(user, opts);
123153
expect(opts.headers.Authorization).to.be.undefined;
124154
expect(opts.cert).to.equal('foo');
125155
expect(opts.key).to.equal('bar');
@@ -136,18 +166,33 @@ describe('ExecAuth', () => {
136166
var tokenValue = 'foo';
137167
(auth as any).execFn = (
138168
command: string,
139-
args: string[],
140-
opts: child_process.SpawnOptions,
141-
): child_process.SpawnSyncReturns<Buffer> => {
169+
args?: readonly string[],
170+
options?: child_process.SpawnOptionsWithoutStdio,
171+
): child_process.ChildProcessWithoutNullStreams => {
142172
execCount++;
143173
return {
144-
status: 0,
145-
stdout: Buffer.from(
146-
JSON.stringify({
147-
status: { token: tokenValue, expirationTimestamp: expire },
148-
}),
149-
),
150-
} as child_process.SpawnSyncReturns<Buffer>;
174+
stdout: {
175+
setEncoding: () => {},
176+
on: (_data: string, f: (data: Buffer | string) => void) => {
177+
f(
178+
Buffer.from(
179+
JSON.stringify({
180+
status: { token: tokenValue, expirationTimestamp: expire },
181+
}),
182+
),
183+
);
184+
},
185+
},
186+
stderr: {
187+
setEncoding: () => {},
188+
on: () => {},
189+
},
190+
on: (op: string, f: any) => {
191+
if (op === 'close') {
192+
f(0);
193+
}
194+
},
195+
} as unknown as child_process.ChildProcessWithoutNullStreams;
151196
};
152197

153198
const user = {
@@ -207,6 +252,28 @@ describe('ExecAuth', () => {
207252
} as child_process.SpawnSyncReturns<Buffer>;
208253
};
209254

255+
(auth as any).execFn = (
256+
command: string,
257+
args?: readonly string[],
258+
options?: child_process.SpawnOptionsWithoutStdio,
259+
): child_process.ChildProcessWithoutNullStreams => {
260+
return {
261+
stdout: {
262+
setEncoding: () => {},
263+
on: (_data: string, f: (data: Buffer | string) => void) => {},
264+
},
265+
stderr: {
266+
setEncoding: () => {},
267+
on: () => {},
268+
},
269+
on: (op: string, f: any) => {
270+
if (op === 'error') {
271+
throw new Error('Error: spawnSync /path/to/bin ENOENT');
272+
}
273+
},
274+
} as unknown as child_process.ChildProcessWithoutNullStreams;
275+
};
276+
210277
const user = {
211278
name: 'user',
212279
authProvider: {
@@ -230,16 +297,31 @@ describe('ExecAuth', () => {
230297
return;
231298
}
232299
const auth = new ExecAuth();
300+
233301
(auth as any).execFn = (
234302
command: string,
235-
args: string[],
236-
opts: child_process.SpawnOptions,
237-
): child_process.SpawnSyncReturns<Buffer> => {
303+
args?: readonly string[],
304+
options?: child_process.SpawnOptionsWithoutStdio,
305+
): child_process.ChildProcessWithoutNullStreams => {
238306
return {
239-
status: 100,
240-
stdout: Buffer.from(JSON.stringify({ status: { token: 'foo' } })),
241-
stderr: Buffer.from('Some error!'),
242-
} as child_process.SpawnSyncReturns<Buffer>;
307+
stdout: {
308+
setEncoding: () => {},
309+
on: (_data: string, f: (data: Buffer | string) => void) => {
310+
f(Buffer.from(JSON.stringify({ status: { token: 'foo' } })));
311+
},
312+
},
313+
stderr: {
314+
setEncoding: () => {},
315+
on: (_data: string, f: (data: Buffer | string) => void) => {
316+
f(Buffer.from('Some error!'));
317+
},
318+
},
319+
on: (op: string, f: any) => {
320+
if (op === 'close') {
321+
f(100);
322+
}
323+
},
324+
} as unknown as child_process.ChildProcessWithoutNullStreams;
243325
};
244326

245327
const user = {
@@ -265,18 +347,32 @@ describe('ExecAuth', () => {
265347
return;
266348
}
267349
const auth = new ExecAuth();
268-
let optsOut: child_process.SpawnOptions = {};
350+
let optsOut: child_process.SpawnOptions | undefined = {};
269351
(auth as any).execFn = (
270352
command: string,
271-
args: string[],
272-
opts: child_process.SpawnOptions,
273-
): child_process.SpawnSyncReturns<Buffer> => {
274-
optsOut = opts;
353+
args?: readonly string[],
354+
options?: child_process.SpawnOptionsWithoutStdio,
355+
): child_process.ChildProcessWithoutNullStreams => {
356+
optsOut = options;
275357
return {
276-
status: 0,
277-
stdout: Buffer.from(JSON.stringify({ status: { token: 'foo' } })),
278-
} as child_process.SpawnSyncReturns<Buffer>;
358+
stdout: {
359+
setEncoding: () => {},
360+
on: (_data: string, f: (data: Buffer | string) => void) => {
361+
f(Buffer.from(JSON.stringify({ status: { token: 'foo' } })));
362+
},
363+
},
364+
stderr: {
365+
setEncoding: () => {},
366+
on: () => {},
367+
},
368+
on: (op: string, f: any) => {
369+
if (op === 'close') {
370+
f(0);
371+
}
372+
},
373+
} as unknown as child_process.ChildProcessWithoutNullStreams;
279374
};
375+
280376
process.env.BLABBLE = 'flubble';
281377
const opts = {} as https.RequestOptions;
282378
opts.headers = {} as OutgoingHttpHeaders;
@@ -313,16 +409,30 @@ describe('ExecAuth', () => {
313409
const auth = new ExecAuth();
314410
(auth as any).execFn = (
315411
command: string,
316-
args: string[],
317-
opts: child_process.SpawnOptions,
318-
): child_process.SpawnSyncReturns<Buffer> => {
412+
args?: readonly string[],
413+
options?: child_process.SpawnOptionsWithoutStdio,
414+
): child_process.ChildProcessWithoutNullStreams => {
319415
return {
320-
status: 0,
321-
stdout: Buffer.from(JSON.stringify({ status: { token: 'foo' } })),
322-
} as child_process.SpawnSyncReturns<Buffer>;
416+
stdout: {
417+
setEncoding: () => {},
418+
on: (_data: string, f: (data: Buffer | string) => void) => {
419+
f(Buffer.from(JSON.stringify({ status: { token: 'foo' } })));
420+
},
421+
},
422+
stderr: {
423+
setEncoding: () => {},
424+
on: () => {},
425+
},
426+
on: (op: string, f: any) => {
427+
if (op === 'close') {
428+
f(0);
429+
}
430+
},
431+
} as unknown as child_process.ChildProcessWithoutNullStreams;
323432
};
433+
324434
const opts = {} as https.RequestOptions;
325-
auth.applyAuthentication(
435+
await auth.applyAuthentication(
326436
{
327437
name: 'user',
328438
authProvider: {

0 commit comments

Comments
 (0)