Skip to content

Commit

Permalink
chore: Custom construct ID check (#305)
Browse files Browse the repository at this point in the history
* Implement custom construct ID check to avoid tokens in CDK ID
  • Loading branch information
Dmitry Balabanov authored Dec 20, 2023
1 parent ee444b8 commit d573044
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 6 deletions.
13 changes: 10 additions & 3 deletions .projenrc.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { LernaProject } from 'lerna-projen';
import { awscdk, Task } from 'projen';
import { awscdk, javascript, Task } from 'projen';
import { DependabotScheduleInterval } from 'projen/lib/github';
import { Transform } from "projen/lib/javascript";
import { dirname } from 'path';
Expand Down Expand Up @@ -131,7 +131,9 @@ const fwkProject = new awscdk.AwsCdkConstructLibrary({
'jest-runner-groups',
`@aws-cdk/cli-lib-alpha@${CDK_VERSION}-alpha.0`,
'rosetta',
`@aws-cdk/lambda-layer-kubectl-${KUBECTL_LAYER_VERSION}`
`@aws-cdk/lambda-layer-kubectl-${KUBECTL_LAYER_VERSION}`,
'@types/eslint',
'eslint-plugin-local-rules'
],

bundledDeps: [
Expand Down Expand Up @@ -173,10 +175,14 @@ const fwkProject = new awscdk.AwsCdkConstructLibrary({
'node_modules/',
'*.generated.ts',
'coverage'
]
],
},
});

const eslint = javascript.Eslint.of(fwkProject)!;
eslint.addPlugins('eslint-plugin-local-rules');
eslint.addRules({ 'local-rules/no-tokens-in-construct-id': ["error"] });

fwkProject.addPackageIgnore("!*.lit.ts");

fwkProject.testTask.reset('jest --passWithNoTests --updateSnapshot --group=-e2e', {receiveArgs: true});
Expand All @@ -187,6 +193,7 @@ fwkProject.addTask('test:e2e', {
exec: 'jest --passWithNoTests --updateSnapshot --group=e2e'
});


/**
* Task copy `resources` directories from `src` to `lib`
* This is to package YAML files part of the dist
Expand Down
6 changes: 5 additions & 1 deletion framework/.eslintrc.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions framework/.projen/deps.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions framework/.projen/tasks.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions framework/eslint-local-rules/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
require("ts-node").register({
transpileOnly: true,
compilerOptions: {
module: "commonjs",
},
});

module.exports = require("./rules").default;
59 changes: 59 additions & 0 deletions framework/eslint-local-rules/rules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Put the following comment above a code line you want to exclude from this check, e.g. after review:
// eslint-disable-next-line local-rules/no-tokens-in-construct-id

import type { Rule } from "eslint";
import * as ts from "typescript";

export default {
"no-tokens-in-construct-id": {
meta: {
docs: {
description: 'Checks whether a CDK token might appear in a construct ID on instantiation',
recommended: true
},
schema: [],
},
create: function (context) {
return {
NewExpression: function (node) {
if (node.callee.type != 'Identifier') {
return;
}

const typeChecker = context.parserServices.program.getTypeChecker();
const typescriptNode = <ts.Node>context.parserServices.esTreeNodeToTSNodeMap.get(node);
const instantiatedType = <ts.Type>typeChecker.getTypeAtLocation(typescriptNode);
const baseTypes = instantiatedType.getBaseTypes();

if (instantiatedType.symbol.name == 'Construct' || baseTypes && baseTypes.some(t => t.symbol.name == 'Construct')) {
if (node.arguments.length <= 1) {
// Non-standard schema, expected at least 2 parameters
return;
}

// We expect the second argument to be the ID
const secondArgument = node.arguments[1];

if (secondArgument.type == 'Literal') {
if (secondArgument.value?.toString().startsWith('${TOKEN')) {
context.report({
node,
message: 'The ID argument of a construct contains a token, this is not allowed.'
});
}

// No further check needed for literals
return;
}

context.report({
node,
message: 'An expression is used for the construct ID. it might contain tokens, please review. ' +
'Suppress this finding by putting this comment above the code line: // eslint-disable-next-line local-rules/no-tokens-in-construct-id'
});
}
},
};
},
},
} satisfies Record<string, Rule.RuleModule>;
2 changes: 2 additions & 0 deletions framework/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,8 @@ export class SparkEmrContainersRuntime extends TrackedConstruct {
*/
public createExecutionRole(scope: Construct, id: string, policy: IManagedPolicy, eksNamespace: string, name: string): Role {

// Reviewed
// eslint-disable-next-line local-rules/no-tokens-in-construct-id
let irsaConditionkey: CfnJson = new CfnJson(scope, `${id}IrsaConditionkey'`, {
value: {
[`${this.eksCluster.openIdConnectProvider.openIdConnectProviderIssuer}:sub`]: 'system:serviceaccount:' + eksNamespace + ':emr-containers-sa-*-*-' + Aws.ACCOUNT_ID.toString() + '-' + SimpleBase.base36.encode(name),
Expand Down Expand Up @@ -542,6 +544,8 @@ export class SparkEmrContainersRuntime extends TrackedConstruct {
*/
public uploadPodTemplate(id: string, filePath: string) {

// Reviewed
// eslint-disable-next-line local-rules/no-tokens-in-construct-id
new BucketDeployment(this, `${id}AssetDeployment`, {
destinationBucket: this.assetBucket!,
destinationKeyPrefix: this.podTemplateLocation!.objectKey,
Expand Down
3 changes: 3 additions & 0 deletions framework/test/e2e/test-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ class TestStack implements ICloudAssemblyDirectoryProducer {

public constructor(stackName: string, app?: App, stack?: Stack) {
this.app = app ?? new App();

// Reviewed
// eslint-disable-next-line local-rules/no-tokens-in-construct-id
this.stack = stack ?? new Stack(this.app, stackName + '-' + randomUUID().substring(0, 8).toLowerCase());
this.stack.node.setContext(ContextOptions.DISABLE_CONSTRUCTS_DEPLOYMENT_TRACKING, true);
this.#cli = AwsCdkCli.fromCloudAssemblyDirectoryProducer(this);
Expand Down
23 changes: 23 additions & 0 deletions framework/yarn.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d573044

Please sign in to comment.