diff --git a/.projenrc.ts b/.projenrc.ts index 8a6972e..8a391ae 100644 --- a/.projenrc.ts +++ b/.projenrc.ts @@ -107,6 +107,9 @@ new TextFile(project, 'sonar-project.properties', { 'sonar.sources=./src', 'sonar.tests=./test', 'sonar.test.inclusions=**/*.test.*', + 'sonar.issue.ignore.multicriteria=e1', + 'sonar.issue.ignore.multicriteria.e1.ruleKey=typescript:S1874', + 'sonar.issue.ignore.multicriteria.e1.resourceKey=src/smartstack/tags/*.ts', ], }); diff --git a/README.md b/README.md index cc75c91..6be281c 100644 --- a/README.md +++ b/README.md @@ -210,3 +210,21 @@ Generally speaking you would be most interested in the following: - AccountContext (AC) - EnvironmentContext (EC) - Name / UrlName / PathName + +## Migration + +### v0 to v1 + +#### Tagging behavior + +Due to a bug in `v0`, the `Contact` and `Organization` tags were NOT applied as they should have. This means that by default, upgrading from v0→v1 introduces CloudFormation diff. Basically adding the `Contact` and `Organization` tags to all resources. This should be safe operation, but we allow disabling it via a feature flag (note that `Contact` and `Organization` tags will most likely be enforced in future `v2`). + +```diff +// cdk.json +{ + "context": { + // existing context keys ++ "@alma-cdk/project:compatibility:v0:tags": true + }, +} +``` diff --git a/sonar-project.properties b/sonar-project.properties index 704077b..ee4e9be 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -4,4 +4,7 @@ sonar.organization=alma-cdk sonar.javascript.lcov.reportPaths=./coverage/lcov.info sonar.sources=./src sonar.tests=./test -sonar.test.inclusions=**/*.test.* \ No newline at end of file +sonar.test.inclusions=**/*.test.* +sonar.issue.ignore.multicriteria=e1 +sonar.issue.ignore.multicriteria.e1.ruleKey=typescript:S1874 +sonar.issue.ignore.multicriteria.e1.resourceKey=src/smartstack/tags/*.ts \ No newline at end of file diff --git a/src/smartstack/tags/checks.test.ts b/src/smartstack/tags/checks.test.ts new file mode 100644 index 0000000..dd366ea --- /dev/null +++ b/src/smartstack/tags/checks.test.ts @@ -0,0 +1,34 @@ +import { useCompatibilityV0Tags, useLegacyTags } from './checks'; +import { TestableResource } from './test-helpers/TestableResource'; + +describe('useLegacyTags', () => { + test('returns false if the context key is not set', () => { + const scope = new TestableResource(); + expect(useLegacyTags(scope)).toBe(false); + }); + + test('returns true if the context key is set', () => { + const scope = new TestableResource({ + context: { + '@alma-cdk/project:legacyTags': true, + }, + }); + expect(useLegacyTags(scope)).toBe(true); + }); +}); + +describe('useCompatibilityV0Tags', () => { + test('returns false if the context key is not set', () => { + const scope = new TestableResource(); + expect(useCompatibilityV0Tags(scope)).toBe(false); + }); + + test('returns true if the context key is set', () => { + const scope = new TestableResource({ + context: { + '@alma-cdk/project:compatibility:v0:tags': true, + }, + }); + expect(useCompatibilityV0Tags(scope)).toBe(true); + }); +}); diff --git a/src/smartstack/tags/checks.ts b/src/smartstack/tags/checks.ts index e187b52..31188cc 100644 --- a/src/smartstack/tags/checks.ts +++ b/src/smartstack/tags/checks.ts @@ -10,7 +10,26 @@ export function hasEnvironment(values: Values): boolean { return isNonEmptyString(values.environmentType); } +/** + * Enforces usage of https://github.com/almamedia/alma-cdk-jsii-tag-and-name + * (for AWS CDK v1) compatible tagging behavior. + * + * @deprecated This behavior is not encouraged and will be removed in v2. Additionally according to GitHub search, this is not used anymore. + */ export function useLegacyTags(scope: Construct): boolean { const contextKey = '@alma-cdk/project:legacyTags'; return scope.node.tryGetContext(contextKey) === true; } + +/** + * Compatibility flag for v0 tagging behavior. + * Due to a bug in v0, the `Contact` and `Organization` tags were NOT applied as they should have. + * This flag can be used to enforce behavior that matches v0 implementation: + * I.e. `Contact` and `Organization` tags are NOT applied. + * + * @deprecated This behavior is not encouraged and will be removed in v2. + */ +export function useCompatibilityV0Tags(scope: Construct): boolean { + const contextKey = '@alma-cdk/project:compatibility:v0:tags'; + return scope.node.tryGetContext(contextKey) === true; +} \ No newline at end of file diff --git a/src/smartstack/tags/taggers.test.ts b/src/smartstack/tags/taggers.test.ts new file mode 100644 index 0000000..9455d2b --- /dev/null +++ b/src/smartstack/tags/taggers.test.ts @@ -0,0 +1,311 @@ +import * as cdk from 'aws-cdk-lib'; +import { Match } from 'aws-cdk-lib/assertions'; +import { + tagAccount, + tagEnvironment, + tagProject, + tagAuthorName, + tagAuthorOrganization, + tagAuthorEmail, +} from './taggers'; +import { TestableResource } from './test-helpers/TestableResource'; + +describe('tagAccount', () => { + test('no account', () => { + const testable = new TestableResource(); + + tagAccount(testable, cdk.Tags.of(testable), { + accountType: '', // empty string is considered "no account" + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: Match.absent(), + }); + }); + + test('account specified', () => { + const testable = new TestableResource(); + + tagAccount(testable, cdk.Tags.of(testable), { + accountType: 'test', + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Account', + Value: 'test', + }, + ], + }); + }); +}); + +describe('tagEnvironment', () => { + test('no environment', () => { + const testable = new TestableResource(); + + tagEnvironment(testable, cdk.Tags.of(testable), { + environmentType: '', // empty string is considered "no environment" + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: Match.absent(), + }); + }); + + test('environment specified', () => { + const testable = new TestableResource(); + + tagEnvironment(testable, cdk.Tags.of(testable), { + environmentType: 'test', + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Environment', + Value: 'test', + }, + ], + }); + }); + + test('legacy mode adds also ProjectAndEnvironment tag', () => { + const testable = new TestableResource({ + context: { + '@alma-cdk/project:legacyTags': true, + }, + }); + + tagEnvironment(testable, cdk.Tags.of(testable), { + environmentType: 'test', + projectName: 'my-project', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Environment', + Value: 'test', + }, + { + Key: 'ProjectAndEnvironment', + Value: 'MyProjectTest', + }, + ], + }); + }); +}); + +describe('tagProject', () => { + test('no project', () => { + const testable = new TestableResource(); + + tagProject(testable, cdk.Tags.of(testable), { + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Project', + Value: '', // existing behavior accepts empty string for Project + }, + ], + }); + }); + + test('project specified', () => { + const testable = new TestableResource(); + + tagProject(testable, cdk.Tags.of(testable), { + projectName: 'my-project', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Project', + Value: 'my-project', + }, + ], + }); + }); + + test('legacy mode converts to Capital Case', () => { + const testable = new TestableResource({ + context: { + '@alma-cdk/project:legacyTags': true, + }, + }); + + tagProject(testable, cdk.Tags.of(testable), { + projectName: 'my-project', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Project', + Value: 'My Project', + }, + ], + }); + }); +}); + +test('tagAuthorName', () => { + const testable = new TestableResource(); + + tagAuthorName(testable, cdk.Tags.of(testable), { + authorName: 'test', + projectName: '', + authorEmail: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Author', + Value: 'test', + }, + ], + }); +}); + +describe('tagAuthorOrganization', () => { + test('compatibility mode not set', () => { + const testable = new TestableResource(); + + tagAuthorOrganization(testable, cdk.Tags.of(testable), { + authorOrganization: 'test', + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Organization', + Value: 'test', + }, + ], + }); + }); + + test('compatibility mode is set but disabled', () => { + const testable = new TestableResource({ + context: { + '@alma-cdk/project:compatibility:v0:tags': false, + }, + }); + + tagAuthorOrganization(testable, cdk.Tags.of(testable), { + authorOrganization: 'test', + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Organization', + Value: 'test', + }, + ], + }); + }); + + test('compatibility mode is set', () => { + const testable = new TestableResource({ + context: { + '@alma-cdk/project:compatibility:v0:tags': true, + }, + }); + + tagAuthorOrganization(testable, cdk.Tags.of(testable), { + authorOrganization: 'test', + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: Match.absent(), + }); + }); +}); + +describe('tagAuthorEmail', () => { + test('compatibility mode not set', () => { + const testable = new TestableResource(); + + tagAuthorEmail(testable, cdk.Tags.of(testable), { + authorEmail: 'test@example.com', + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Contact', + Value: 'test@example.com', + }, + ], + }); + }); + + test('compatibility mode is set but disabled', () => { + const testable = new TestableResource({ + context: { + '@alma-cdk/project:compatibility:v0:tags': false, + }, + }); + + tagAuthorEmail(testable, cdk.Tags.of(testable), { + authorEmail: 'test@example.com', + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: [ + { + Key: 'Contact', + Value: 'test@example.com', + }, + ], + }); + }); + + test('compatibility mode is set', () => { + const testable = new TestableResource({ + context: { + '@alma-cdk/project:compatibility:v0:tags': true, + }, + }); + + tagAuthorEmail(testable, cdk.Tags.of(testable), { + authorEmail: 'test@example.com', + projectName: '', + authorName: '', + }); + + testable.hasProperties({ + Tags: Match.absent(), + }); + }); +}); diff --git a/src/smartstack/tags/taggers.ts b/src/smartstack/tags/taggers.ts index b9d2396..6e95fe7 100644 --- a/src/smartstack/tags/taggers.ts +++ b/src/smartstack/tags/taggers.ts @@ -1,7 +1,7 @@ import { Tags } from 'aws-cdk-lib'; import { capitalCase, pascalCase } from 'change-case'; import { Construct } from 'constructs'; -import { hasAccount, hasEnvironment, useLegacyTags } from './checks'; +import { hasAccount, hasEnvironment, useCompatibilityV0Tags, useLegacyTags } from './checks'; import { tagKey, Values } from './values'; import { isNonEmptyString } from '../../utils/isNonEmptyString'; @@ -40,14 +40,14 @@ export const tagAuthorName: Tagger = (_: Construct, tags: Tags, values: Values) tags.add(tagKey.AUTHOR_NAME, values.authorName); }; -export const tagAuthorOrganization: Tagger = (_: Construct, tags: Tags, values: Values) => { - if (isNonEmptyString(values.authorOrganization)) { +export const tagAuthorOrganization: Tagger = (scope: Construct, tags: Tags, values: Values) => { + if (!useCompatibilityV0Tags(scope) && isNonEmptyString(values.authorOrganization)) { tags.add(tagKey.AUTHOR_ORGANIZATION, values.authorOrganization); } }; -export const tagAuthorEmail: Tagger = (_: Construct, tags: Tags, values: Values) => { - if (isNonEmptyString(values.authorEmail)) { +export const tagAuthorEmail: Tagger = (scope: Construct, tags: Tags, values: Values) => { + if (!useCompatibilityV0Tags(scope) && isNonEmptyString(values.authorEmail)) { tags.add(tagKey.AUTHOR_EMAIL, values.authorEmail); } }; diff --git a/src/smartstack/tags/test-helpers/TestableResource.ts b/src/smartstack/tags/test-helpers/TestableResource.ts new file mode 100644 index 0000000..35d8789 --- /dev/null +++ b/src/smartstack/tags/test-helpers/TestableResource.ts @@ -0,0 +1,43 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; +import { Construct } from 'constructs'; + +interface TestableResourceProps { + id?: string; + scope?: Construct; + context?: Record; +} + +/** + * A helper class that allows easier testing. + */ +export class TestableResource extends cdk.Resource implements cdk.ITaggable { + static readonly TYPE = 'For::Testing'; + + public readonly tags = new cdk.TagManager( + cdk.TagType.KEY_VALUE, + TestableResource.TYPE, + ); + + constructor(props: TestableResourceProps = {}) { + const { id = 'TestScope', scope = new cdk.Stack(), context = {} } = props; + + Object.entries(context).forEach(([key, value]) => { + scope.node.setContext(key, value); + }); + + super(scope, id); + + new cdk.CfnResource(this, 'Resource', { + type: TestableResource.TYPE, + properties: { + Tags: this.tags.renderedTags, + }, + }); + } + + public hasProperties(props: Record) { + const template = Template.fromStack(cdk.Stack.of(this)); + template.hasResourceProperties(TestableResource.TYPE, props); + } +}