Skip to content

Commit

Permalink
fix: Allow users to set environment variable to connect to emulator r…
Browse files Browse the repository at this point in the history
…unning 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.
  • Loading branch information
danieljbruce committed Sep 29, 2023
1 parent 10ce563 commit a41741b
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/index.ts
Expand Up @@ -505,11 +505,13 @@ class Datastore extends DatastoreRequest {
},
options
);
const isUsingEmulator =
const isUsingLocalhost =
this.baseUrl_ &&
(this.baseUrl_.includes('localhost') ||
this.baseUrl_.includes('127.0.0.1') ||
this.baseUrl_.includes('::1'));
const isEmulatorVariableSet = process.env.DATASTORE_EMULATOR_HOST;
const isUsingEmulator = isUsingLocalhost || isEmulatorVariableSet;
if (this.customEndpoint_ && isUsingEmulator) {
this.options.sslCreds ??= grpc.credentials.createInsecure();
}
Expand Down
133 changes: 133 additions & 0 deletions test/index.ts
Expand Up @@ -316,6 +316,139 @@ async.each(
assert.strictEqual(datastore.options.sslCreds, fakeInsecureCreds);
});

describe('checking ssl credentials are set correctly with custom endpoints', () => {
function setHost(host: string) {
process.env.DATASTORE_EMULATOR_HOST = host;
}

const sslCreds = gax.grpc.ChannelCredentials.createSsl();
const fakeInsecureCreds = {
insecureCredProperty: 'insecureCredPropertyValue',
};

beforeEach(() => {
createInsecureOverride = () => {
return fakeInsecureCreds;
};
});

describe('without DATASTORE_EMULATOR_HOST environment variable set', () => {
beforeEach(() => {
delete process.env.DATASTORE_EMULATOR_HOST;
});

describe('using a localhost endpoint', () => {
const apiEndpoint = 'http://localhost:8080';
it('should use ssl credentials provided', () => {
// SSL credentials provided in the constructor should always be used.
const options = {
apiEndpoint,
sslCreds,
};
const datastore = new Datastore(options);
assert.strictEqual(datastore.options.sslCreds, sslCreds);
});
it('should use insecure ssl credentials when ssl credentials are not provided', () => {
// When using a localhost endpoint it is assumed that the emulator is being used.
// Therefore, sslCreds should be set to insecure credentials to skip authentication.
const datastore = new Datastore({
apiEndpoint,
});
assert.strictEqual(
datastore.options.sslCreds,
fakeInsecureCreds
);
});
});
describe('using a remote endpoint', () => {
const apiEndpoint = 'http://remote:8080';
it('should use ssl credentials provided', () => {
// SSL credentials provided in the constructor should always be used.
const options = {
apiEndpoint,
sslCreds,
};
const datastore = new Datastore(options);
assert.strictEqual(datastore.options.sslCreds, sslCreds);
});
it('should not set ssl credentials when ssl credentials are not provided', () => {
// When using a remote endpoint without DATASTORE_EMULATOR_HOST set,
// it is assumed that the emulator is not being used.
// This test captures the case where users use a regional endpoint.
const datastore = new Datastore({
apiEndpoint,
});
assert.strictEqual(datastore.options.sslCreds, undefined);
});
});
});
describe('with DATASTORE_EMULATOR_HOST environment variable set', () => {
beforeEach(() => {
delete process.env.DATASTORE_EMULATOR_HOST;
});

describe('with DATASTORE_EMULATOR_HOST set to localhost', () => {
const apiEndpoint = 'http://localhost:8080';
beforeEach(() => {
setHost(apiEndpoint);
});

it('should use ssl credentials provided', () => {
// SSL credentials provided in the constructor should always be used.
const datastore = new Datastore({
apiEndpoint,
sslCreds,
});
assert.strictEqual(datastore.options.sslCreds, sslCreds);
});

it('should use insecure ssl credentials when ssl credentials are not provided', () => {
// When DATASTORE_EMULATOR_HOST is set it is assumed that the emulator is being used.
// Therefore, sslCreds should be set to insecure credentials to skip authentication.
const datastore = new Datastore({
apiEndpoint,
});
assert.strictEqual(
datastore.options.sslCreds,
fakeInsecureCreds
);
});
});

describe('with DATASTORE_EMULATOR_HOST set to remote host', () => {
const apiEndpoint = 'http://remote:8080';
beforeEach(() => {
setHost(apiEndpoint);
});

it('should use ssl credentials provided', () => {
// SSL credentials provided in the constructor should always be used.
const datastore = new Datastore({
apiEndpoint,
sslCreds,
});
assert.strictEqual(datastore.options.sslCreds, sslCreds);
});

it('should use insecure ssl credentials when ssl credentials are not provided', () => {
// When DATASTORE_EMULATOR_HOST is set it is assumed that the emulator is being used.
// Therefore, sslCreds should be set to insecure credentials to skip authentication.
const datastore = new Datastore({
apiEndpoint,
});
assert.strictEqual(
datastore.options.sslCreds,
fakeInsecureCreds
);
});
});

after(() => {
delete process.env.DATASTORE_EMULATOR_HOST;
});
});
});

it('should cache a local GoogleAuth instance', () => {
const fakeGoogleAuthInstance = {};

Expand Down

0 comments on commit a41741b

Please sign in to comment.