From 7533f4e5a6e7f8b767c2324266f8785ae09b54e6 Mon Sep 17 00:00:00 2001 From: Ari Palo Date: Fri, 22 Nov 2024 15:43:15 +0200 Subject: [PATCH 1/5] feat: show warnings for deprecations --- src/feature-flags/index.ts | 2 ++ src/feature-flags/legacyTags.test.ts | 22 ++++++++++++++++++ src/feature-flags/legacyTags.ts | 13 +++++++++++ src/feature-flags/v0Tags.test.ts | 22 ++++++++++++++++++ src/feature-flags/v0Tags.ts | 15 ++++++++++++ src/project/deprecation-warnings.ts | 19 ++++++++++++++++ src/project/project.ts | 3 +++ src/smartstack/tags/checks.test.ts | 34 ---------------------------- src/smartstack/tags/checks.ts | 25 -------------------- src/smartstack/tags/taggers.ts | 19 ++++++++-------- 10 files changed, 105 insertions(+), 69 deletions(-) create mode 100644 src/feature-flags/index.ts create mode 100644 src/feature-flags/legacyTags.test.ts create mode 100644 src/feature-flags/legacyTags.ts create mode 100644 src/feature-flags/v0Tags.test.ts create mode 100644 src/feature-flags/v0Tags.ts create mode 100644 src/project/deprecation-warnings.ts delete mode 100644 src/smartstack/tags/checks.test.ts diff --git a/src/feature-flags/index.ts b/src/feature-flags/index.ts new file mode 100644 index 0000000..12d259a --- /dev/null +++ b/src/feature-flags/index.ts @@ -0,0 +1,2 @@ +export * from "./legacyTags"; +export * from "./v0Tags"; diff --git a/src/feature-flags/legacyTags.test.ts b/src/feature-flags/legacyTags.test.ts new file mode 100644 index 0000000..93e9386 --- /dev/null +++ b/src/feature-flags/legacyTags.test.ts @@ -0,0 +1,22 @@ +import { useLegacyTags, LEGACY_TAGS_CONTEXT_KEY } from "./legacyTags"; +import { TestableResource } from "../__test__/TestableResource"; + +describe("useLegacyTags", () => { + test("context key is correct", () => { + expect(LEGACY_TAGS_CONTEXT_KEY).toBe("@alma-cdk/project:legacyTags"); + }); + + 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); + }); +}); diff --git a/src/feature-flags/legacyTags.ts b/src/feature-flags/legacyTags.ts new file mode 100644 index 0000000..c9a9357 --- /dev/null +++ b/src/feature-flags/legacyTags.ts @@ -0,0 +1,13 @@ +import { Construct } from "constructs"; + +export const LEGACY_TAGS_CONTEXT_KEY = "@alma-cdk/project:legacyTags"; + +/** + * 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 { + return scope.node.tryGetContext(LEGACY_TAGS_CONTEXT_KEY) === true; +} diff --git a/src/feature-flags/v0Tags.test.ts b/src/feature-flags/v0Tags.test.ts new file mode 100644 index 0000000..444fb27 --- /dev/null +++ b/src/feature-flags/v0Tags.test.ts @@ -0,0 +1,22 @@ +import { useCompatibilityV0Tags, V0_TAGS_CONTEXT_KEY } from "./v0Tags"; +import { TestableResource } from "../__test__/TestableResource"; + +describe("useCompatibilityV0Tags", () => { + test("context key is correct", () => { + expect(V0_TAGS_CONTEXT_KEY).toBe("@alma-cdk/project:compatibility:v0:tags"); + }); + + 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/feature-flags/v0Tags.ts b/src/feature-flags/v0Tags.ts new file mode 100644 index 0000000..7560669 --- /dev/null +++ b/src/feature-flags/v0Tags.ts @@ -0,0 +1,15 @@ +import { Construct } from "constructs"; + +export const V0_TAGS_CONTEXT_KEY = "@alma-cdk/project:compatibility:v0:tags"; + +/** + * 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 { + return scope.node.tryGetContext(V0_TAGS_CONTEXT_KEY) === true; +} diff --git a/src/project/deprecation-warnings.ts b/src/project/deprecation-warnings.ts new file mode 100644 index 0000000..503fbef --- /dev/null +++ b/src/project/deprecation-warnings.ts @@ -0,0 +1,19 @@ +import { Annotations } from "aws-cdk-lib"; +import { Construct } from "constructs"; +import * as featureFlags from "../feature-flags"; + +export function warnAboutDeprecatedTags(scope: Construct) { + if (featureFlags.useLegacyTags(scope)) { + Annotations.of(scope).addWarningV2( + "@alma-cdk/project@v1:legacy-tags", + `Using @almamedia-cdk/tag-and-name (for AWS CDK v1) construct's legacy tagging behavior via "${featureFlags.LEGACY_TAGS_CONTEXT_KEY}" context key. This is not encouraged and will be removed in v2.`, + ); + } + + if (featureFlags.useCompatibilityV0Tags(scope)) { + Annotations.of(scope).addWarningV2( + "@alma-cdk/project@v1:compatibility-v0-tags", + `Using @alma-cdk/project@v0 construct's tagging behavior via "${featureFlags.V0_TAGS_CONTEXT_KEY}" context key. You should migrate to using the default tagging behavior as this feature flag will be removed in v2.`, + ); + } +} diff --git a/src/project/project.ts b/src/project/project.ts index 2d9ba11..571fb05 100644 --- a/src/project/project.ts +++ b/src/project/project.ts @@ -3,6 +3,7 @@ import { Construct } from "constructs"; import { Account, ProjectConfiguration } from "./interfaces"; import { resolveDefaultRegion } from "./resolve-region"; import { addError } from "../error"; +import { warnAboutDeprecatedTags } from "./deprecation-warnings"; /** Props given to `Project`. * @@ -95,5 +96,7 @@ export class Project extends App { [Project.CONTEXT_SCOPE]: config, // and inject project context }, }); + + warnAboutDeprecatedTags(this); } } diff --git a/src/smartstack/tags/checks.test.ts b/src/smartstack/tags/checks.test.ts deleted file mode 100644 index eea52dd..0000000 --- a/src/smartstack/tags/checks.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { useCompatibilityV0Tags, useLegacyTags } from "./checks"; -import { TestableResource } from "../../__test__/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 2908439..2fe89ba 100644 --- a/src/smartstack/tags/checks.ts +++ b/src/smartstack/tags/checks.ts @@ -1,4 +1,3 @@ -import { Construct } from "constructs"; import { Values } from "./values"; import { isNonEmptyString } from "../../utils/isNonEmptyString"; @@ -9,27 +8,3 @@ export function hasAccount(values: Values): boolean { 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; -} diff --git a/src/smartstack/tags/taggers.ts b/src/smartstack/tags/taggers.ts index 654f467..82dd27b 100644 --- a/src/smartstack/tags/taggers.ts +++ b/src/smartstack/tags/taggers.ts @@ -1,13 +1,9 @@ import { Tags } from "aws-cdk-lib"; import { capitalCase, pascalCase } from "change-case"; import { Construct } from "constructs"; -import { - hasAccount, - hasEnvironment, - useCompatibilityV0Tags, - useLegacyTags, -} from "./checks"; +import { hasAccount, hasEnvironment } from "./checks"; import { tagKey, Values } from "./values"; +import * as featureFlags from "../../feature-flags"; import { isNonEmptyString } from "../../utils/isNonEmptyString"; interface Tagger { @@ -32,7 +28,7 @@ export const tagEnvironment: Tagger = ( if (hasEnvironment(values)) { tags.add(tagKey.ENVIRONMENT, values.environmentType!); - if (useLegacyTags(scope)) { + if (featureFlags.useLegacyTags(scope)) { tags.add( tagKey.LEGACY_PROJECT_ENVIRONMENT, `${pascalCase(values.projectName)}${pascalCase(values.environmentType!)}`, @@ -47,7 +43,7 @@ export const tagProject: Tagger = ( values: Values, ) => { let value = values.projectName; - if (useLegacyTags(scope)) { + if (featureFlags.useLegacyTags(scope)) { value = capitalCase(values.projectName); } tags.add(tagKey.PROJECT, value); @@ -67,7 +63,7 @@ export const tagAuthorOrganization: Tagger = ( values: Values, ) => { if ( - !useCompatibilityV0Tags(scope) && + !featureFlags.useCompatibilityV0Tags(scope) && isNonEmptyString(values.authorOrganization) ) { tags.add(tagKey.AUTHOR_ORGANIZATION, values.authorOrganization); @@ -79,7 +75,10 @@ export const tagAuthorEmail: Tagger = ( tags: Tags, values: Values, ) => { - if (!useCompatibilityV0Tags(scope) && isNonEmptyString(values.authorEmail)) { + if ( + !featureFlags.useCompatibilityV0Tags(scope) && + isNonEmptyString(values.authorEmail) + ) { tags.add(tagKey.AUTHOR_EMAIL, values.authorEmail); } }; From 11a4e89c10b7ef9c2ec6a894f65f453c34042c37 Mon Sep 17 00:00:00 2001 From: Ari Palo Date: Fri, 22 Nov 2024 15:59:22 +0200 Subject: [PATCH 2/5] test: deprecation warnings --- src/project/deprecation-warnings.test.ts | 33 ++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 src/project/deprecation-warnings.test.ts diff --git a/src/project/deprecation-warnings.test.ts b/src/project/deprecation-warnings.test.ts new file mode 100644 index 0000000..9d2d8f1 --- /dev/null +++ b/src/project/deprecation-warnings.test.ts @@ -0,0 +1,33 @@ +import { warnAboutDeprecatedTags } from "./deprecation-warnings"; +import { expectErrorMetadata } from "../__test__/expectErrorMetadata"; +import { TestableResource } from "../__test__/TestableResource"; + +test("@alma-cdk/project:legacyTags", () => { + const testable = new TestableResource({ + context: { + "@alma-cdk/project:legacyTags": true, + }, + }); + + warnAboutDeprecatedTags(testable); + + expectErrorMetadata( + testable, + expect.stringContaining("@alma-cdk/project@v1:legacy-tags"), + ); +}); + +test("@alma-cdk/project:compatibility:v0:tags", () => { + const testable = new TestableResource({ + context: { + "@alma-cdk/project:compatibility:v0:tags": true, + }, + }); + + warnAboutDeprecatedTags(testable); + + expectErrorMetadata( + testable, + expect.stringContaining("@alma-cdk/project@v1:compatibility-v0-tags"), + ); +}); From c7867c2b19b1c57ee3f96dd21b12fd875fe3d1f8 Mon Sep 17 00:00:00 2001 From: Ari Palo Date: Fri, 22 Nov 2024 16:08:59 +0200 Subject: [PATCH 3/5] test: missing feature flags --- src/project/deprecation-warnings.test.ts | 60 ++++++++++++++++-------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/src/project/deprecation-warnings.test.ts b/src/project/deprecation-warnings.test.ts index 9d2d8f1..909f5e8 100644 --- a/src/project/deprecation-warnings.test.ts +++ b/src/project/deprecation-warnings.test.ts @@ -2,32 +2,52 @@ import { warnAboutDeprecatedTags } from "./deprecation-warnings"; import { expectErrorMetadata } from "../__test__/expectErrorMetadata"; import { TestableResource } from "../__test__/TestableResource"; -test("@alma-cdk/project:legacyTags", () => { - const testable = new TestableResource({ - context: { - "@alma-cdk/project:legacyTags": true, - }, +describe("@alma-cdk/project:legacyTags", () => { + test("feature flag present", () => { + const testable = new TestableResource({ + context: { + "@alma-cdk/project:legacyTags": true, + }, + }); + + warnAboutDeprecatedTags(testable); + + expectErrorMetadata( + testable, + expect.stringContaining("@alma-cdk/project@v1:legacy-tags"), + ); }); - warnAboutDeprecatedTags(testable); + test("feature flag missing", () => { + const testable = new TestableResource(); - expectErrorMetadata( - testable, - expect.stringContaining("@alma-cdk/project@v1:legacy-tags"), - ); + warnAboutDeprecatedTags(testable); + + expectErrorMetadata(testable, undefined); + }); }); -test("@alma-cdk/project:compatibility:v0:tags", () => { - const testable = new TestableResource({ - context: { - "@alma-cdk/project:compatibility:v0:tags": true, - }, +describe("@alma-cdk/project:compatibility:v0:tags", () => { + test("feature flag present", () => { + const testable = new TestableResource({ + context: { + "@alma-cdk/project:compatibility:v0:tags": true, + }, + }); + + warnAboutDeprecatedTags(testable); + + expectErrorMetadata( + testable, + expect.stringContaining("@alma-cdk/project@v1:compatibility-v0-tags"), + ); }); - warnAboutDeprecatedTags(testable); + test("feature flag missing", () => { + const testable = new TestableResource(); - expectErrorMetadata( - testable, - expect.stringContaining("@alma-cdk/project@v1:compatibility-v0-tags"), - ); + warnAboutDeprecatedTags(testable); + + expectErrorMetadata(testable, undefined); + }); }); From 23bff4655fac0c5040d59ae81d0d588072551f16 Mon Sep 17 00:00:00 2001 From: Ari Palo Date: Fri, 22 Nov 2024 16:13:48 +0200 Subject: [PATCH 4/5] refactor: import functions directly let's see if sonar is happy now --- src/project/deprecation-warnings.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/project/deprecation-warnings.ts b/src/project/deprecation-warnings.ts index 503fbef..d9d2ce8 100644 --- a/src/project/deprecation-warnings.ts +++ b/src/project/deprecation-warnings.ts @@ -1,19 +1,24 @@ import { Annotations } from "aws-cdk-lib"; import { Construct } from "constructs"; -import * as featureFlags from "../feature-flags"; +import { + useLegacyTags, + useCompatibilityV0Tags, + LEGACY_TAGS_CONTEXT_KEY, + V0_TAGS_CONTEXT_KEY, +} from "../feature-flags"; export function warnAboutDeprecatedTags(scope: Construct) { - if (featureFlags.useLegacyTags(scope)) { + if (useLegacyTags(scope)) { Annotations.of(scope).addWarningV2( "@alma-cdk/project@v1:legacy-tags", - `Using @almamedia-cdk/tag-and-name (for AWS CDK v1) construct's legacy tagging behavior via "${featureFlags.LEGACY_TAGS_CONTEXT_KEY}" context key. This is not encouraged and will be removed in v2.`, + `Using @almamedia-cdk/tag-and-name (for AWS CDK v1) construct's legacy tagging behavior via "${LEGACY_TAGS_CONTEXT_KEY}" context key. This is not encouraged and will be removed in v2.`, ); } - if (featureFlags.useCompatibilityV0Tags(scope)) { + if (useCompatibilityV0Tags(scope)) { Annotations.of(scope).addWarningV2( "@alma-cdk/project@v1:compatibility-v0-tags", - `Using @alma-cdk/project@v0 construct's tagging behavior via "${featureFlags.V0_TAGS_CONTEXT_KEY}" context key. You should migrate to using the default tagging behavior as this feature flag will be removed in v2.`, + `Using @alma-cdk/project@v0 construct's tagging behavior via "${V0_TAGS_CONTEXT_KEY}" context key. You should migrate to using the default tagging behavior as this feature flag will be removed in v2.`, ); } } From 148ceb11458dc617fd2f4d194e4ccc48020287b8 Mon Sep 17 00:00:00 2001 From: Ari Palo Date: Fri, 22 Nov 2024 16:17:27 +0200 Subject: [PATCH 5/5] build: sonar ignore deprecation warnings --- .projenrc.ts | 4 +++- sonar-project.properties | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.projenrc.ts b/.projenrc.ts index efa5ece..48adcb6 100644 --- a/.projenrc.ts +++ b/.projenrc.ts @@ -134,9 +134,11 @@ 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,e2", "sonar.issue.ignore.multicriteria.e1.ruleKey=typescript:S1874", "sonar.issue.ignore.multicriteria.e1.resourceKey=src/smartstack/tags/*.ts", + "sonar.issue.ignore.multicriteria.e2.ruleKey=typescript:S1874", + "sonar.issue.ignore.multicriteria.e2.resourceKey=src/project/deprecation-warnings.ts", ], }); diff --git a/sonar-project.properties b/sonar-project.properties index ee4e9be..7a14aa9 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -5,6 +5,8 @@ sonar.javascript.lcov.reportPaths=./coverage/lcov.info sonar.sources=./src sonar.tests=./test sonar.test.inclusions=**/*.test.* -sonar.issue.ignore.multicriteria=e1 +sonar.issue.ignore.multicriteria=e1,e2 sonar.issue.ignore.multicriteria.e1.ruleKey=typescript:S1874 -sonar.issue.ignore.multicriteria.e1.resourceKey=src/smartstack/tags/*.ts \ No newline at end of file +sonar.issue.ignore.multicriteria.e1.resourceKey=src/smartstack/tags/*.ts +sonar.issue.ignore.multicriteria.e2.ruleKey=typescript:S1874 +sonar.issue.ignore.multicriteria.e2.resourceKey=src/project/deprecation-warnings.ts \ No newline at end of file