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.

Sign up for GitHub

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

implement usage of aws_endpoint_url #243

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
32f232f
implement usage of aws_endpoint_url
pinzon Jan 29, 2024
ebf56b0
remove unused var
pinzon Jan 29, 2024
2d1c737
retrieve clean protocol
pinzon Jan 29, 2024
13495c3
test should assert the new default endpoint
pinzon Jan 29, 2024
8b3c4f3
ci test
pinzon Jan 30, 2024
dbf84d6
restore test and fix for test
pinzon Jan 30, 2024
1b66b21
clean volume folder
pinzon Feb 1, 2024
933695b
use localhost as aws endpoint
pinzon Feb 2, 2024
69a1ad3
rewrite test
pinzon Feb 2, 2024
b7b642c
rewrite test
pinzon Feb 6, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading