Skip to content

Commit a41741b

Browse files
danieljbrucedanieljbruce
authored
fix: Allow users to set environment variable to connect to emulator running on docker (#1164)
* Check if DATASTORE_EMULATOR_HOST is set Instead of checking to see if the endpoint is local, we should check to see if the datastore emulator variable is set. This is because there is an edge case where the user is using an emulator, but the emulator is not listening from local host and in this case we want to use insecure credentials. * Add tests for combinations of emulator / custom Add unit tests for evaluating value of ssl credentials. Tests evaluate what happens when the user uses remote custom endpoints or local custom endpoints in combination with different values for the DATASTORE_EMULATOR_HOST variable. * Revert "Check if DATASTORE_EMULATOR_HOST is set" This reverts commit dea1706. * Revert "Revert "Check if DATASTORE_EMULATOR_HOST is set"" This reverts commit a3b50df. * Clean up the tests The tests should use a concise apiEndpoint variable and they should set the host for the remote case to be the same as the apiEndpoint variable. * Add more test cases describing all behaviour More test cases are needed to describe what happens when the DATASTORE_EMULATOR_HOST variable is not set both in a remote and in a localhost environment because those behaviors should be different. Setting the apiEndpoint to localhost means the user wants to use the emulator without specifying the environment variable so insecure credentials should be provided. Setting the apiEndpoint to remote means the user likely wishes to use regional endpoints so should not be using insecure credentials to skip authentication. * Allows test cases to pass from previous commit If the user uses localhost as the endpoint or they provide DATASTORE_EMULATOR_HOST then it is assumed that the user is using the emulator and authentication is skipped. Otherwise, authentication is not skipped. * Change old test title back to the way it was The old test title should be what it was before. setHost is now only needed in one place. * Add comments to each test The comments need to be added to explain why the test is written the way that it is written so that we know the motivation behind each test.
1 parent 10ce563 commit a41741b

File tree

2 files changed

+136
-1
lines changed

2 files changed

+136
-1
lines changed

‎src/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,11 +505,13 @@ class Datastore extends DatastoreRequest {
505505
},
506506
options
507507
);
508-
const isUsingEmulator =
508+
const isUsingLocalhost =
509509
this.baseUrl_ &&
510510
(this.baseUrl_.includes('localhost') ||
511511
this.baseUrl_.includes('127.0.0.1') ||
512512
this.baseUrl_.includes('::1'));
513+
const isEmulatorVariableSet = process.env.DATASTORE_EMULATOR_HOST;
514+
const isUsingEmulator = isUsingLocalhost || isEmulatorVariableSet;
513515
if (this.customEndpoint_ && isUsingEmulator) {
514516
this.options.sslCreds ??= grpc.credentials.createInsecure();
515517
}

‎test/index.ts‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,139 @@ async.each(
316316
assert.strictEqual(datastore.options.sslCreds, fakeInsecureCreds);
317317
});
318318

319+
describe('checking ssl credentials are set correctly with custom endpoints', () => {
320+
function setHost(host: string) {
321+
process.env.DATASTORE_EMULATOR_HOST = host;
322+
}
323+
324+
const sslCreds = gax.grpc.ChannelCredentials.createSsl();
325+
const fakeInsecureCreds = {
326+
insecureCredProperty: 'insecureCredPropertyValue',
327+
};
328+
329+
beforeEach(() => {
330+
createInsecureOverride = () => {
331+
return fakeInsecureCreds;
332+
};
333+
});
334+
335+
describe('without DATASTORE_EMULATOR_HOST environment variable set', () => {
336+
beforeEach(() => {
337+
delete process.env.DATASTORE_EMULATOR_HOST;
338+
});
339+
340+
describe('using a localhost endpoint', () => {
341+
const apiEndpoint = 'http://localhost:8080';
342+
it('should use ssl credentials provided', () => {
343+
// SSL credentials provided in the constructor should always be used.
344+
const options = {
345+
apiEndpoint,
346+
sslCreds,
347+
};
348+
const datastore = new Datastore(options);
349+
assert.strictEqual(datastore.options.sslCreds, sslCreds);
350+
});
351+
it('should use insecure ssl credentials when ssl credentials are not provided', () => {
352+
// When using a localhost endpoint it is assumed that the emulator is being used.
353+
// Therefore, sslCreds should be set to insecure credentials to skip authentication.
354+
const datastore = new Datastore({
355+
apiEndpoint,
356+
});
357+
assert.strictEqual(
358+
datastore.options.sslCreds,
359+
fakeInsecureCreds
360+
);
361+
});
362+
});
363+
describe('using a remote endpoint', () => {
364+
const apiEndpoint = 'http://remote:8080';
365+
it('should use ssl credentials provided', () => {
366+
// SSL credentials provided in the constructor should always be used.
367+
const options = {
368+
apiEndpoint,
369+
sslCreds,
370+
};
371+
const datastore = new Datastore(options);
372+
assert.strictEqual(datastore.options.sslCreds, sslCreds);
373+
});
374+
it('should not set ssl credentials when ssl credentials are not provided', () => {
375+
// When using a remote endpoint without DATASTORE_EMULATOR_HOST set,
376+
// it is assumed that the emulator is not being used.
377+
// This test captures the case where users use a regional endpoint.
378+
const datastore = new Datastore({
379+
apiEndpoint,
380+
});
381+
assert.strictEqual(datastore.options.sslCreds, undefined);
382+
});
383+
});
384+
});
385+
describe('with DATASTORE_EMULATOR_HOST environment variable set', () => {
386+
beforeEach(() => {
387+
delete process.env.DATASTORE_EMULATOR_HOST;
388+
});
389+
390+
describe('with DATASTORE_EMULATOR_HOST set to localhost', () => {
391+
const apiEndpoint = 'http://localhost:8080';
392+
beforeEach(() => {
393+
setHost(apiEndpoint);
394+
});
395+
396+
it('should use ssl credentials provided', () => {
397+
// SSL credentials provided in the constructor should always be used.
398+
const datastore = new Datastore({
399+
apiEndpoint,
400+
sslCreds,
401+
});
402+
assert.strictEqual(datastore.options.sslCreds, sslCreds);
403+
});
404+
405+
it('should use insecure ssl credentials when ssl credentials are not provided', () => {
406+
// When DATASTORE_EMULATOR_HOST is set it is assumed that the emulator is being used.
407+
// Therefore, sslCreds should be set to insecure credentials to skip authentication.
408+
const datastore = new Datastore({
409+
apiEndpoint,
410+
});
411+
assert.strictEqual(
412+
datastore.options.sslCreds,
413+
fakeInsecureCreds
414+
);
415+
});
416+
});
417+
418+
describe('with DATASTORE_EMULATOR_HOST set to remote host', () => {
419+
const apiEndpoint = 'http://remote:8080';
420+
beforeEach(() => {
421+
setHost(apiEndpoint);
422+
});
423+
424+
it('should use ssl credentials provided', () => {
425+
// SSL credentials provided in the constructor should always be used.
426+
const datastore = new Datastore({
427+
apiEndpoint,
428+
sslCreds,
429+
});
430+
assert.strictEqual(datastore.options.sslCreds, sslCreds);
431+
});
432+
433+
it('should use insecure ssl credentials when ssl credentials are not provided', () => {
434+
// When DATASTORE_EMULATOR_HOST is set it is assumed that the emulator is being used.
435+
// Therefore, sslCreds should be set to insecure credentials to skip authentication.
436+
const datastore = new Datastore({
437+
apiEndpoint,
438+
});
439+
assert.strictEqual(
440+
datastore.options.sslCreds,
441+
fakeInsecureCreds
442+
);
443+
});
444+
});
445+
446+
after(() => {
447+
delete process.env.DATASTORE_EMULATOR_HOST;
448+
});
449+
});
450+
});
451+
319452
it('should cache a local GoogleAuth instance', () => {
320453
const fakeGoogleAuthInstance = {};
321454

0 commit comments

Comments
 (0)