Skip to content

Commit

Permalink
Setting addExtension to default to false (#134)
Browse files Browse the repository at this point in the history 
* Setting addExtension to default to false

* Improving addExtension logic

* cleaning up error messages to be less confusing/conflictin

* formatting
  • Loading branch information
@TophrC-dd
TophrC-dd committed Sep 19, 2024
1 parent c38f966 commit f0c3419
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 23 deletions.
8 changes: 5 additions & 3 deletions serverless/src/env.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ const ddApmFlushDeadlineMillisecondsEnvVar = "DD_APM_FLUSH_DEADLINE_MILLISECONDS

export const defaultConfiguration: Configuration = {
addLayers: true,
addExtension: true,
addExtension: false,
flushMetricsToLogs: true,
logLevel: undefined,
site: "datadoghq.com",
Expand Down Expand Up @@ -257,12 +257,14 @@ export function validateParameters(config: Configuration) {
}
if (config.addExtension === true) {
if (config.extensionLayerVersion === undefined) {
errors.push("Please add the 'extensionLayerVersion' parameter for the Datadog serverless macro");
errors.push("Please add the `extensionLayerVersion` parameter when `addExtension` is set.");
}
}
if (config.extensionLayerVersion !== undefined) {
if (config.forwarderArn !== undefined) {
errors.push("`extensionLayerVersion` and `forwarderArn` cannot be set at the same time.");
errors.push(
"setting `forwarderArn` with `addExtension` and/or `extensionLayerVersion` as these parameters cannot be set at the same time.",
);
}
if (config.apiKey === undefined && config.apiKeySecretArn === undefined && config.apiKMSKey === undefined) {
errors.push("When `extensionLayerVersion` is set, `apiKey`, `apiKeySecretArn`, or `apiKmsKey` must also be set.");
Expand Down
3 changes: 1 addition & 2 deletions serverless/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ export const handler = async (event: InputEvent, _: any) => {
};
}

//
const lambdas = findLambdas(resources, event.templateParameterValues);
log.debug(`Lambda resources found: ${JSON.stringify(lambdas)}`);

Expand Down Expand Up @@ -117,7 +116,7 @@ export const handler = async (event: InputEvent, _: any) => {
}
}

if (config.addExtension) {
if (config.addExtension || config.extensionLayerVersion !== undefined) {
errors = applyExtensionLayer(region, lambdas, config.extensionLayerVersion);
if (errors.length > 0) {
return {
Expand Down
18 changes: 10 additions & 8 deletions serverless/test/env.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe("getConfig", () => {
const config = getConfigFromCfnParams(params);
expect(config).toEqual({
addLayers: true,
addExtension: true,
addExtension: false,
flushMetricsToLogs: true,
site: "my-site",
enableXrayTracing: false,
Expand Down Expand Up @@ -63,7 +63,7 @@ describe("getConfig", () => {
const config = getConfigFromEnvVars();
expect(config).toEqual({
addLayers: true,
addExtension: true,
addExtension: false,
flushMetricsToLogs: false,
logLevel: undefined,
site: "datadoghq.com",
Expand Down Expand Up @@ -91,7 +91,7 @@ describe("getConfig", () => {
const config = getConfigFromCfnParams(params);
expect(config).toEqual({
addLayers: true,
addExtension: true,
addExtension: false,
flushMetricsToLogs: false,
site: "my-site",
enableXrayTracing: false,
Expand Down Expand Up @@ -594,7 +594,11 @@ describe("validateParameters", () => {
};

const errors = validateParameters(params);
expect(errors.includes("`extensionLayerVersion` and `forwarderArn` cannot be set at the same time.")).toBe(true);
expect(
errors.includes(
"setting `forwarderArn` with `addExtension` and/or `extensionLayerVersion` as these parameters cannot be set at the same time.",
),
).toBe(true);
});

it("returns an error when extensionLayer is true without setting extensionLayerVersion", () => {
Expand All @@ -613,15 +617,13 @@ describe("validateParameters", () => {
};

const errors = validateParameters(params);
expect(errors.includes("Please add the 'extensionLayerVersion' parameter for the Datadog serverless macro")).toBe(
true,
);
expect(errors.includes("Please add the `extensionLayerVersion` parameter when `addExtension` is set.")).toBe(true);
});

it("returns an error when extensionLayerVersion is set but neither apiKey nor apiKMSKey is set", () => {
const params = {
addLayers: true,
addExtension: true,
addExtension: false,
flushMetricsToLogs: true,
logLevel: "info",
site: "datadoghq.com",
Expand Down
44 changes: 34 additions & 10 deletions serverless/test/index.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ function mockGovCloudInputEvent(params: any, mappings: any, logGroups?: LogGroup
describe("Macro", () => {
describe("parameters and config", () => {
it("uses transform parameters if they are provided", async () => {
const transformParams = { site: "datadoghq.com", addExtension: false };
const transformParams = { site: "datadoghq.com" };
const mappings = {
Datadog: {
Parameters: { site: "mappings-site" },
Expand All @@ -153,7 +153,7 @@ describe("Macro", () => {
it("uses parameters under Mappings section if template parameters are not given", async () => {
const mappings = {
Datadog: {
Parameters: { site: "datadoghq.eu", addExtension: false },
Parameters: { site: "datadoghq.eu" },
},
};
const inputEvent = mockInputEvent({}, mappings);
Expand All @@ -168,7 +168,7 @@ describe("Macro", () => {

describe("lambda layers", () => {
it("adds lambda layers by default", async () => {
const params = { nodeLayerVersion: 25, addExtension: false };
const params = { nodeLayerVersion: 25 };
const inputEvent = mockInputEvent(params, {}); // Use default configuration
const output = await handler(inputEvent, {});
const lambdaProperties: FunctionProperties = output.fragment.Resources[LAMBDA_KEY].Properties;
Expand All @@ -188,8 +188,32 @@ describe("Macro", () => {
expect(output.errorMessage).toEqual(getMissingLayerVersionErrorMsg(LAMBDA_KEY, "Node.js", "node"));
});

it("add only the extension layer", async () => {
const params = { addLayers: false, addExtension: true, extensionLayerVersion: 6, apiKey: "abc123" };
const inputEvent = mockInputEvent(params, {});
const output = await handler(inputEvent, {});
const lambdaProperties: FunctionProperties = output.fragment.Resources[LAMBDA_KEY].Properties;

// Mocked Lambda has the addExtension parameter to true and extensionLayerVersion to 6.
expect(lambdaProperties.Layers).toEqual([
expect.stringMatching(/arn:aws:lambda:us-east-1:.*:layer:Datadog-Extension:6/),
]);
});

it("add only the extension layer only setting the extension layer version", async () => {
const params = { addLayers: false, extensionLayerVersion: 6, apiKey: "abc123" };
const inputEvent = mockInputEvent(params, {});
const output = await handler(inputEvent, {});
const lambdaProperties: FunctionProperties = output.fragment.Resources[LAMBDA_KEY].Properties;

// Mocked Lambda has the addExtension parameter to true and extensionLayerVersion to 6.
expect(lambdaProperties.Layers).toEqual([
expect.stringMatching(/arn:aws:lambda:us-east-1:.*:layer:Datadog-Extension:6/),
]);
});

it("skips adding lambda layers when addLayers is false", async () => {
const params = { addLayers: false, addExtension: false };
const params = { addLayers: false };
const inputEvent = mockInputEvent(params, {});
const output = await handler(inputEvent, {});
const lambdaProperties: FunctionProperties = output.fragment.Resources[LAMBDA_KEY].Properties;
Expand All @@ -198,7 +222,7 @@ describe("Macro", () => {
});

it("uses the GovCloud layer when a GovCloud region is detected", async () => {
const params = { nodeLayerVersion: 25, addExtension: false };
const params = { nodeLayerVersion: 25 };
const inputEvent = mockGovCloudInputEvent(params, {});
const output = await handler(inputEvent, {});
const lambdaProperties: FunctionProperties = output.fragment.Resources[LAMBDA_KEY].Properties;
Expand All @@ -211,7 +235,7 @@ describe("Macro", () => {

describe("tracing", () => {
it("skips adding tracing when enableXrayTracing is false", async () => {
const params = { enableXrayTracing: false, addExtension: false };
const params = { enableXrayTracing: false };
const inputEvent = mockInputEvent(params, {});
const output = await handler(inputEvent, {});
const iamRole: IamRoleProperties = output.fragment.Resources[`${LAMBDA_KEY}Role`].Properties;
Expand Down Expand Up @@ -239,7 +263,7 @@ describe("Macro", () => {
});

it("macro fails when forwarder is provided & at least one lambda has a dynamically generated name, but no stack name is given", async () => {
const params = { forwarderArn: "forwarder-arn", nodeLayerVersion: 25, addExtension: false };
const params = { forwarderArn: "forwarder-arn", nodeLayerVersion: 25 };
const inputEvent = mockInputEvent(params, {});
const output = await handler(inputEvent, {});

Expand All @@ -250,7 +274,7 @@ describe("Macro", () => {

describe("tags", () => {
it("only adds macro version tag when neither 'service' nor 'env' are provided", async () => {
const params = { nodeLayerVersion: 25, addExtension: false };
const params = { nodeLayerVersion: 25 };
const inputEvent = mockInputEvent(params, {});
const output = await handler(inputEvent, {});
const lambdaProperties: FunctionProperties = output.fragment.Resources[LAMBDA_KEY].Properties;
Expand All @@ -259,7 +283,7 @@ describe("Macro", () => {
});

it("only adds cdk created tag when CDKMetadata is present", async () => {
const params = { nodeLayerVersion: 25, addExtension: false };
const params = { nodeLayerVersion: 25 };
const inputEvent = mockInputEvent(params, {}, undefined, true);
const output = await handler(inputEvent, {});
const lambdaProperties: FunctionProperties = output.fragment.Resources[LAMBDA_KEY].Properties;
Expand All @@ -271,7 +295,7 @@ describe("Macro", () => {
});

it("only adds SAM created tag when lambda:createdBy:SAM tag is present", async () => {
const params = { nodeLayerVersion: 25, addExtension: false };
const params = { nodeLayerVersion: 25 };
const inputEvent = mockInputEvent(params, {}, undefined, false, true);
const output = await handler(inputEvent, {});
const lambdaProperties: FunctionProperties = output.fragment.Resources[LAMBDA_KEY].Properties;
Expand Down

0 comments on commit f0c3419

Please sign in to comment.