Skip to content

Commit

Permalink
Merge pull request #42 from alma-cdk/beta-add-warnings
Browse files Browse the repository at this point in the history
chore: add warnings about use of to-be-deprecated feature flags
  • Loading branch information
aripalo authored Nov 22, 2024
2 parents 4f80847 + 148ceb1 commit ab4ba94
Show file tree
Hide file tree
Showing 13 changed files with 170 additions and 72 deletions.
4 changes: 3 additions & 1 deletion .projenrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
});

Expand Down
6 changes: 4 additions & 2 deletions sonar-project.properties

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

2 changes: 2 additions & 0 deletions src/feature-flags/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from "./legacyTags";
export * from "./v0Tags";
22 changes: 22 additions & 0 deletions src/feature-flags/legacyTags.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
13 changes: 13 additions & 0 deletions src/feature-flags/legacyTags.ts
Original file line number Diff line number Diff line change
@@ -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;
}
22 changes: 22 additions & 0 deletions src/feature-flags/v0Tags.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
15 changes: 15 additions & 0 deletions src/feature-flags/v0Tags.ts
Original file line number Diff line number Diff line change
@@ -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;
}
53 changes: 53 additions & 0 deletions src/project/deprecation-warnings.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { warnAboutDeprecatedTags } from "./deprecation-warnings";
import { expectErrorMetadata } from "../__test__/expectErrorMetadata";
import { TestableResource } from "../__test__/TestableResource";

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"),
);
});

test("feature flag missing", () => {
const testable = new TestableResource();

warnAboutDeprecatedTags(testable);

expectErrorMetadata(testable, undefined);
});
});

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"),
);
});

test("feature flag missing", () => {
const testable = new TestableResource();

warnAboutDeprecatedTags(testable);

expectErrorMetadata(testable, undefined);
});
});
24 changes: 24 additions & 0 deletions src/project/deprecation-warnings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Annotations } from "aws-cdk-lib";
import { Construct } from "constructs";
import {
useLegacyTags,
useCompatibilityV0Tags,
LEGACY_TAGS_CONTEXT_KEY,
V0_TAGS_CONTEXT_KEY,
} from "../feature-flags";

export function warnAboutDeprecatedTags(scope: Construct) {
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 "${LEGACY_TAGS_CONTEXT_KEY}" context key. This is not encouraged and will be removed in v2.`,
);
}

if (useCompatibilityV0Tags(scope)) {
Annotations.of(scope).addWarningV2(
"@alma-cdk/project@v1:compatibility-v0-tags",
`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.`,
);
}
}
3 changes: 3 additions & 0 deletions src/project/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
*
Expand Down Expand Up @@ -95,5 +96,7 @@ export class Project extends App {
[Project.CONTEXT_SCOPE]: config, // and inject project context
},
});

warnAboutDeprecatedTags(this);
}
}
34 changes: 0 additions & 34 deletions src/smartstack/tags/checks.test.ts

This file was deleted.

25 changes: 0 additions & 25 deletions src/smartstack/tags/checks.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Construct } from "constructs";
import { Values } from "./values";
import { isNonEmptyString } from "../../utils/isNonEmptyString";

Expand All @@ -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;
}
19 changes: 9 additions & 10 deletions src/smartstack/tags/taggers.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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!)}`,
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}
};

0 comments on commit ab4ba94

Please sign in to comment.