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

implement usage of aws_endpoint_url #243

Merged
merged 10 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 1 addition & 1 deletion spec/unit/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ describe("LocalstackPlugin", () => {
expect(request.called).to.be.true;
let templateUrl = request.firstCall.args[2].TemplateURL;
// url should either start with 'http://localhost' or 'http://127.0.0.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that comment seems outdated and misleading

expect(templateUrl).to.satisfy((url) => url === `${config.host}:4566/path/to/template` || url === 'http://127.0.0.1:4566/path/to/template');
expect(templateUrl).to.satisfy((url) => url === `${config.host}:4566/path/to/template` || url === 'https://localhost.localstack.cloud:4566/path/to/template');
});

it('should overwrite the S3 hostname with the value from environment variable', async () => {
Expand Down
163 changes: 93 additions & 70 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ const TYPESCRIPT_PLUGIN_BUILD_DIR_WEBPACK = '.webpack/service'; //TODO detect fr
const TS_PLUGIN_ESBUILD = 'EsbuildServerlessPlugin'
const TYPESCRIPT_PLUGIN_BUILD_DIR_ESBUILD = '.esbuild/.build'; //TODO detect from esbuild.config.js

// Default edge port to use with host
const DEFAULT_EDGE_PORT = '4566';
// Default AWS endpoint URL
const DEFAULT_AWS_ENDPOINT_URL = "https://localhost.localstack.cloud:4566";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ This is a breaking change and should technically be released as a new major version


// Cache hostname to avoid unnecessary connection checks
var resolvedHostname = undefined;

const awsEndpointUrl = process.env.AWS_ENDPOINT_URL || DEFAULT_AWS_ENDPOINT_URL;

class LocalstackPlugin {
constructor(serverless, options) {

Expand Down Expand Up @@ -136,17 +138,17 @@ class LocalstackPlugin {
if (this.detectTypescriptPluginType() === TS_PLUGIN_WEBPACK) {
const p = this.serverless.pluginManager.plugins.find((x) => x.constructor.name === TS_PLUGIN_WEBPACK);
if (
this.shouldMountCode() && (
!p ||
!p.serverless ||
!p.serverless.configurationInput ||
!p.serverless.configurationInput.custom ||
!p.serverless.configurationInput.custom.webpack ||
!p.serverless.configurationInput.custom.webpack.keepOutputDirectory
)
this.shouldMountCode() && (
!p ||
!p.serverless ||
!p.serverless.configurationInput ||
!p.serverless.configurationInput.custom ||
!p.serverless.configurationInput.custom.webpack ||
!p.serverless.configurationInput.custom.webpack.keepOutputDirectory
)
) {
throw new Error('When mounting Lambda code, you must retain webpack output directory. '
+ 'Set custom.webpack.keepOutputDirectory to true.');
+ 'Set custom.webpack.keepOutputDirectory to true.');
}
}
}
Expand All @@ -158,7 +160,7 @@ class LocalstackPlugin {
addHookInFirstPosition(eventName, hookFunction) {
this.serverless.pluginManager.hooks[eventName] = this.serverless.pluginManager.hooks[eventName] || [];
this.serverless.pluginManager.hooks[eventName].unshift(
{ pluginName: 'LocalstackPlugin', hook: hookFunction.bind(this, eventName) });
{ pluginName: 'LocalstackPlugin', hook: hookFunction.bind(this, eventName) });
}

activatePlugin(preHooks) {
Expand Down Expand Up @@ -241,11 +243,11 @@ class LocalstackPlugin {
this.getStageVariable();

return this.startLocalStack().then(
() => {
() => {
this.patchServerlessSecrets();
this.patchS3BucketLocationResponse();
this.patchS3CreateBucketLocationConstraint();
}
}
);
}

Expand Down Expand Up @@ -298,17 +300,17 @@ class LocalstackPlugin {

// overwrite bound functions for specified hook names
(hookNames || []).forEach(
(hookName) => {
plugin.hooks[hookName] = boundOverrideFunction;
const slsHooks = this.serverless.pluginManager.hooks[hookName] || [];
slsHooks.forEach(
(hookItem) => {
if (hookItem.pluginName === pluginName) {
hookItem.hook = boundOverrideFunction;
}
}
);
}
(hookName) => {
plugin.hooks[hookName] = boundOverrideFunction;
const slsHooks = this.serverless.pluginManager.hooks[hookName] || [];
slsHooks.forEach(
(hookItem) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional to mix 4 spaces here vs. 2 spaces elsewhere?

Would it make sense to configure a code formatter for the project using a pre-commit hook + CI enforcement and do this once rather than mixing up formatting changes with functional code changes?

if (hookItem.pluginName === pluginName) {
hookItem.hook = boundOverrideFunction;
}
}
);
}
);
}

Expand Down Expand Up @@ -412,18 +414,18 @@ class LocalstackPlugin {

const getContainer = () => {
return exec('docker ps').then(
(stdout) => {
const exists = stdout.split('\n').filter(
(line) => (
line.indexOf('localstack/localstack') >= 0 ||
line.indexOf('localstack/localstack-pro') >= 0 ||
line.indexOf('localstack_localstack') >= 0
)
);
if (exists.length) {
return exists[0].replace('\t', ' ').split(' ')[0];
(stdout) => {
const exists = stdout.split('\n').filter(
(line) => (
line.indexOf('localstack/localstack') >= 0 ||
line.indexOf('localstack/localstack-pro') >= 0 ||
line.indexOf('localstack_localstack') >= 0
)
);
if (exists.length) {
return exists[0].replace('\t', ' ').split(' ')[0];
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is correct, was it extra before? (just seeing limited context at GH)

)
};

Expand All @@ -438,13 +440,13 @@ class LocalstackPlugin {
return this.sleep(4000).then(() => {
this.log(`Checking state of LocalStack container ${containerID}`)
return exec(`docker logs "${containerID}"`).then(
(logs) => {
const ready = logs.split('\n').filter((line) => line.indexOf('Ready.') >= 0);
if (ready.length) {
return Promise.resolve();
(logs) => {
const ready = logs.split('\n').filter((line) => line.indexOf('Ready.') >= 0);
if (ready.length) {
return Promise.resolve();
}
return checkStatus(containerID, timeout);
}
return checkStatus(containerID, timeout);
}
);
});
}
Expand Down Expand Up @@ -483,17 +485,17 @@ class LocalstackPlugin {
}

return getContainer().then(
(containerID) => {
if(containerID) {
return;
}
(containerID) => {
if(containerID) {
return;
}

if(this.config.docker && this.config.docker.compose_file){
if(this.config.docker && this.config.docker.compose_file){
return startCompose();
}
}

return startContainer();
}
return startContainer();
}
);
}

Expand All @@ -508,12 +510,12 @@ class LocalstackPlugin {
const template = this.serverless.service.provider.compiledCloudFormationTemplate || {};
const resources = template.Resources || {};
Object.keys(resources).forEach(
(resName) => {
const resEntry = resources[resName];
if (resEntry.Type === 'AWS::Lambda::Function') {
resEntry.Properties.Handler = `${this.getTSBuildDir()}/${resEntry.Properties.Handler}`;
(resName) => {
const resEntry = resources[resName];
if (resEntry.Type === 'AWS::Lambda::Function') {
resEntry.Properties.Handler = `${this.getTSBuildDir()}/${resEntry.Properties.Handler}`;
}
}
}
);
}

Expand All @@ -540,9 +542,9 @@ class LocalstackPlugin {
}

/**
* Patch S3 createBucket invocation to not add a LocationContraint if the region is `us-east-1`
* The default SDK check was against endpoint and not the region directly.
*/
* Patch S3 createBucket invocation to not add a LocationContraint if the region is `us-east-1`
* The default SDK check was against endpoint and not the region directly.
*/
patchS3CreateBucketLocationConstraint () {
AWS.util.update(AWS.S3.prototype, {
createBucket: function createBucket (params, callback) {
Expand Down Expand Up @@ -646,7 +648,7 @@ class LocalstackPlugin {
else {
this.endpoints = {}
this.log("Skipping serverless-localstack:\ncustom.localstack.stages: " +
JSON.stringify(this.config.stages) + "\nstage: " + this.config.stage
JSON.stringify(this.config.stages) + "\nstage: " + this.config.stage
)
}
}
Expand Down Expand Up @@ -700,8 +702,23 @@ class LocalstackPlugin {

/* Utility functions below */

getEndpointPort(){
const url = new URL(awsEndpointUrl);
return url.port;
}

getEndpointHostname(){
const url = new URL(awsEndpointUrl);
return url.hostname;
}

getEndpointProtocol(){
const url = new URL(awsEndpointUrl);
return url.protocol.replace(":","");
}

getEdgePort() {
return process.env.EDGE_PORT || this.config.edgePort || DEFAULT_EDGE_PORT;
return process.env.EDGE_PORT || this.config.edgePort || this.getEndpointPort();
}

/**
Expand All @@ -713,7 +730,7 @@ class LocalstackPlugin {
return resolvedHostname;
}

var hostname = process.env.LOCALSTACK_HOSTNAME || 'localhost';
var hostname = process.env.LOCALSTACK_HOSTNAME || this.getEndpointHostname();
if (this.config.host) {
hostname = this.config.host;
if (hostname.indexOf("://") !== -1) {
Expand Down Expand Up @@ -782,13 +799,19 @@ class LocalstackPlugin {
return this.injectHostnameIntoLocalhostURL(process.env.AWS_ENDPOINT_URL, hostname);
}
hostname = hostname || 'localhost';
const proto = TRUE_VALUES.includes(process.env.USE_SSL) ? 'https' : 'http';

let proto = this.getEndpointProtocol();
if (process.env.USE_SSL) {
proto = TRUE_VALUES.includes(process.env.USE_SSL) ? 'https' : 'http';
}else if (this.config.host){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing spaces around conditionals (autoformatter would be helpful here)

proto = this.config.host.split("://")[0];
}
const port = this.getEdgePort();
// little hack here - required to remove the default HTTPS port 443, as otherwise
// routing for some platforms and ephemeral instances (e.g., on namespace.so) fails
const isDefaultPort =
(proto === 'http' && `${port}` === '80') ||
(proto === 'https' && `${port}` === '443');
(proto === 'http' && `${port}` === '80') ||
(proto === 'https' && `${port}` === '443');
if (isDefaultPort) {
return `${proto}://${hostname}`;
}
Expand Down Expand Up @@ -850,14 +873,14 @@ class LocalstackPlugin {
patchCustomResourceLambdaS3ForcePathStyle () {
const awsProvider = this.awsProvider;
const patchMarker = path.join(
awsProvider.serverless.serviceDir,
'.serverless',
'.internal-custom-resources-patched'
awsProvider.serverless.serviceDir,
'.serverless',
'.internal-custom-resources-patched'
);
const zipFilePath = path.join(
awsProvider.serverless.serviceDir,
'.serverless',
awsProvider.naming.getCustomResourcesArtifactName()
awsProvider.serverless.serviceDir,
'.serverless',
awsProvider.naming.getCustomResourcesArtifactName()
);

function fileExists (filePath) {
Expand Down
1 change: 1 addition & 0 deletions volume/cache/machine.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you added all the files in volume/cache by accident? could you please remove them from the PR 🙏

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"machine_id": "dkr_6f0f3e21a3d9"}
Loading
Loading