Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add X-Gu-CDK-Version tag to GuStack #223

Merged
merged 3 commits into from
Feb 4, 2021
Merged

Conversation

akash1810
Copy link
Member

@akash1810 akash1810 commented Feb 3, 2021

What does this change?

Adds X-Gu-CDK-Version tag to GuStack. We can use this to track which stacks have been created using the library, and with which version. Adding the tag to GuStack means it'll get applied to all resources.

The absence of this tag suggests the stack hasn't been created with the cdk library. Using the version number allows us to track how up to date stacks are too.

We mock the library version in tests to remove having to update snapshots whenever we publish a new version of the library, which, among other things, would make automated publishing difficult! The mock is applied globally for simplicity.

Does this change require changes to existing projects or CDK CLI?

Yes! The change is small though, it should be a matter of bumping the version of this library being used.

How to test

Observe existing tests.

How can we measure success?

We know what version of this library was used to create a stack, if any.

Have we considered potential risks?

The version number is retrieved from the package.json. Does this resolve the the correct value once published? That is, if we use the library in a project, is the version number that of guardian/cdk or of the consuming project? Update: this is fine. The import path is relative so will always resolve to this package's package.json file.

@@ -0,0 +1,8 @@
export const LibraryInfo = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -3,4 +3,5 @@ module.exports = {
transform: {
"^.+\\.tsx?$": "ts-jest",
},
setupFilesAfterEnv: ["./jest.setup.js"],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akash1810 akash1810 marked this pull request as ready for review February 3, 2021 00:51
@akash1810 akash1810 requested a review from a team February 3, 2021 00:51
@akash1810 akash1810 added the needs-new-release Identifying significant changes to the library label Feb 3, 2021
@jacobwinch
Copy link
Contributor

We know what version of this library was used to create a stack, if any.

Great idea 💡 I think this will be really useful in the future, especially if we can easily find stacks using specific versions via Prism!

Copy link
Contributor

@jacobwinch jacobwinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work 💯

As far as I can tell, the only downside of this approach is that it will attempt to modify a large number of resources whenever users upgrade the library (because we'll need to perform tag updates, even if the underlying resources are unchanged)?

I think it'll be really useful to have this info, but perhaps we should document this behaviour so that users are not concerned by these frequent snapshot updates or change sets?

@akash1810
Copy link
Member Author

As far as I can tell, the only downside of this approach is that it will attempt to modify a large number of resources whenever users upgrade the library (because we'll need to perform tag updates, even if the underlying resources are unchanged)?

Ah great point! Will have a look to see if a tag can be applied only to the stack and not spread across all its resources.

We can use this to track which stacks have been created using the library, and with which version.

The absence of this tag suggests the stack hasn't been created with the cdk library.
Mocking this means we can make the value static and it doesn't need updating when new versions of the library are published.

Configuring Jest's `setupFilesAfterEnv` means we apply the mock globally. See https://jestjs.io/docs/en/configuration.html#setupfilesafterenv-array.
@akash1810 akash1810 changed the title Add X-Gu-CDK-Version tag to GuStack feat: Add X-Gu-CDK-Version tag to GuStack Feb 3, 2021
@akash1810
Copy link
Member Author

akash1810 commented Feb 3, 2021

Will have a look to see if a tag can be applied only to the stack and not spread across all its resources.

From the docs it is possible to choose which resources to add a tag to, however it looks like tags for the stack can only be set at create/update time*, that is, they cannot be part of the template.

There is this hack(?), but I'm not particularly keen on it as it is pretty obscure and unintuitive.

There's also this which, from testing, this doesn't change the resulting template but does change the cdk artefacts output from cdk synth (namely manifest.json and tree.json). Alas, we're not currently deploying stacks via cdk deploy but through applying the json/yaml template. That is, we'd still rely on the stacks' tags being manually applied.

I see three options to progress:

  1. Document this behaviour and merge
  2. Make the tag more static, something like X-Gu-Stack-Author => guardian/cdk
  3. Make it easier to apply updates through CD, which can hide the verbose change sets away

I think I favour 1 (3245a2c).

* as we can't automate the application of tags against the stack, our tracking strategy might need to change.

Copy link
Contributor

@jacobwinch jacobwinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I favour 1 (3245a2c).

Yep, I agree - thanks for looking into alternatives though!

👍 🚢

@akash1810 akash1810 merged commit b1dfc9a into main Feb 4, 2021
@akash1810 akash1810 deleted the aa-tracking-tag branch February 4, 2021 12:29
This was referenced Feb 4, 2021
akash1810 added a commit that referenced this pull request Apr 14, 2021
In #223 (and #233) we started adding a tag (`gu:cdk:version`) to track which version of `@guardian/cdk` is being used.

This tag helps us measure adoption:
  - the presence of the tag tells us the library is being used
  - the value of the tag tells us the version of the library being used

Whilst this is really useful, it does somewhat block users from using tools such as dependabot.
This is because the snapshot tests will _always_ fail due to the value of the tag changing between version numbers.

In this repository, we have configured a global mock for this tag to ensure its value is static and thus simplifing the tests.

This change updates the TypeScript configuration to include (more specifically not exclude) the `__mocks__` directories.

This results in all mocks making their way into the published library which can then be used within client stacks,
to also have a static value.

We currently have a single mock in the repository. If we add more, we may want to be more selective about which of them get published.
akash1810 added a commit that referenced this pull request Apr 14, 2021
In #223 (and #233) we started adding a tag (`gu:cdk:version`) to track which version of `@guardian/cdk` is being used.

This tag helps us measure adoption:
  - the presence of the tag tells us the library is being used
  - the value of the tag tells us the version of the library being used

Whilst this is really useful, it does somewhat block users from using tools such as dependabot.
This is because the snapshot tests will _always_ fail due to the value of the tag changing between version numbers.

In this repository, we have configured a global mock for this tag to ensure its value is static and thus simplifing the tests.

This change updates the TypeScript configuration to include (more specifically not exclude) the `__mocks__` directories.

This results in all mocks making their way into the published library which can then be used within client stacks,
to also have a static value.

We currently have a single mock in the repository. If we add more, we may want to be more selective about which of them get published.
akash1810 added a commit that referenced this pull request Apr 14, 2021
In #223 (and #233) we started adding a tag (`gu:cdk:version`) to track which version of `@guardian/cdk` is being used.

This tag helps us measure adoption:
  - the presence of the tag tells us the library is being used
  - the value of the tag tells us the version of the library being used

Whilst this is really useful, it does somewhat block users from using tools such as dependabot.
This is because the snapshot tests will _always_ fail due to the value of the tag changing between version numbers.

In this repository, we have configured a global mock for this tag to ensure its value is static, thus simplifing the tests.

This change updates the TypeScript configuration to include (more specifically stop excluding) the `__mocks__` directories.

This results in all mocks making their way into the published library which can then be used within client stacks,
to also have a static value.

We currently have a single mock in the repository. If we add more, we may want to be more selective about which of them get published.
akash1810 added a commit that referenced this pull request Apr 14, 2021
In #223 (and #233) we started adding a tag (`gu:cdk:version`) to track which version of `@guardian/cdk` is being used.

This tag helps us measure adoption:
  - the presence of the tag tells us the library is being used
  - the value of the tag tells us the version of the library being used

Whilst this is really useful, it does somewhat block users from using tools such as dependabot.
This is because the snapshot tests will _always_ fail due to the value of the tag changing between version numbers.

In this repository, we have configured a global mock for this tag to ensure its value is static, thus simplifying the tests.

This change updates the TypeScript configuration to include (more specifically stop excluding) the `__mocks__` directories.

This results in all mocks making their way into the published library which can then be used within client stacks,
to also have a static value.

We currently have a single mock in the repository. If we add more, we may want to be more selective about which of them get published.
akash1810 added a commit that referenced this pull request Apr 15, 2021
In #223 (and #233) we started adding a tag (`gu:cdk:version`) to track which version of `@guardian/cdk` is being used.

This tag helps us measure adoption:
  - the presence of the tag tells us the library is being used
  - the value of the tag tells us the version of the library being used

Whilst this is really useful, it does somewhat block users from using tools such as dependabot.
This is because the snapshot tests will _always_ fail due to the value of the tag changing between version numbers.

In this repository, we have configured a global mock for this tag to ensure its value is static, thus simplifying the tests.

This change updates the TypeScript configuration to include (more specifically stop excluding) the `__mocks__` directories.

This results in all mocks making their way into the published library which can then be used within client stacks,
to also have a static value.

We currently have a single mock in the repository. If we add more, we may want to be more selective about which of them get published.
akash1810 added a commit that referenced this pull request Apr 15, 2021
In #223 (and #233) we started adding a tag (`gu:cdk:version`) to track which version of `@guardian/cdk` is being used.

This tag helps us measure adoption:
  - the presence of the tag tells us the library is being used
  - the value of the tag tells us the version of the library being used

Whilst this is really useful, it does somewhat block users from using tools such as dependabot.
This is because the snapshot tests will _always_ fail due to the value of the tag changing between version numbers.

In this repository, we have configured a global mock for this tag to ensure its value is static, thus simplifying the tests.

This change updates the TypeScript configuration to include (more specifically stop excluding) the `__mocks__` directories.

This results in all mocks making their way into the published library which can then be used within client stacks,
to also have a static value.

We currently have a single mock in the repository. If we add more, we may want to be more selective about which of them get published.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-new-release Identifying significant changes to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants