feat(assertions): add stack tagging assertions

[jamestelfer]

Adds a Tag class to the assertions library that permits assertions against tags on synthesized CDK stacks.

Tags on AWS resources can be checked via assertions with the Template class, but since stack tags only appear on the cloud assembly manifest as a sibling of the template, a separate assertion mechanism is required.

Note

Previously submitted as #27633, which was closed automatically while waiting for review.

This PR is complete to the best of my knowledge, needing only an exemption from integration tests. As far as I can tell, this area of the library has no integration tests directly associated.

API

class Tags {
  public static fromStack(stack: Stack) : Tags;
  public hasValues(props: any): void;
  public all(): { [key: string]: string };
}

Usage

This new class permits tests of the form:

import { App, Stack } from 'aws-cdk-lib';
import { Tags } from 'aws-cdk-lib/assertions';

const app = new App();
const stack = new Stack(app, 'MyStack', {
  tags: {
    'tag-name': 'tag-value'
  }
});

const tags = Tags.fromStack(stack);

// using a default 'objectLike' Matcher
tags.hasValues({
  'tag-name': 'tag-value'
});

// or another Matcher
tags.hasValues({
  'tag-name': Match.anyValue()
});

You can also get the set of tags to test them in other ways:

tags.all()

Issues

No tags case

One might expect that the case where no tags are present would match undefined or null, but since the Cloud Assembly API defaults tags to {} when none are present, this isn't possible. It's also not practical to directly test the artifact.manifest.properties.tags value directly, as there is a legacy case that the API handles. This means that the artifact.tags property is the best value to check against.

The tests for this PR show that matching with Match.absent() will fail when there are no tags, but testing against the empty object will succeed.

I think that this behaviour (defaulting to empty) will be OK, but potentially require a callout on the assertion method.

API method naming

The current suggested API went through some evolution, starting with:

class Tags {
  public static fromStack(stack: Stack) : Tags;
  public hasTags(props: any): void;
  public getTags():  { [key: string]: string };
}

But this stuttered, and getTags() wasn't compatible with Java.

I considered:

class Tags {
  public static fromStack(stack: Stack) : Tags;
  public hasValues(props: any): void;
  public values():  { [key: string]: string };
}

and

class Tags {
  public static fromStack(stack: Stack) : Tags;
  public has(props: any): void;
  public all():  { [key: string]: string };
}

... before settling on a mix of the two. I think the current iteration fits with the rest of the assertions API and makes sense by itself, but very open to changes.

Closes #27620.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[jamestelfer]

Exemption Request

As far as I can tell, the assertions library does not have integration tests, but I'm happy to be corrected.

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[aws-cdk-automation]

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[scanlonp]

Hey @jamestelfer, I'm taking a look. This functionality looks terrific!

I think we can write some integ tests, but probably in @aws-cdk-testing/framework-integ. I need to check out how the IntegTest.assertions work, but if they pull from this module, it would be great to use your new functionality! We can also highlight this in our integ testing guide if that's the case.

[scanlonp]

@Mergifyio update

[mergify]

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/repo-metrics-monthly.yml without workflows permission

[scanlonp]

@jamestelfer turns out IntegTest.assertions are completely separate from the assertions module, so we don't need to worry about @aws-cdk-testing at all. I do not think this change needs integ testing.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[scanlonp]

@jamestelfer, thanks so much for this contribution! I really appreciate your detailed thought process in the PR description, and I think you settled on the best API.

Also thanks for bringing up testing for no tags. I agree its a little odd that Match.absent() does not work well. I have a couple ideas on getting around this.

1. Is there a way to see what type of Match statement is passed into hasValues()? If so could you check for Match.absent and substitute Match.objectEquals({})?
2. We could add a Tags.none() or Tags.hasNone() method that gives the result of Tags.hasValues(Match.objectEquals({})).

I would lean towards option 2 with hasNone(), but let me know what you think.

Last comment, I think it would be good to add some unit tests where there are multiple tags on the stack. That way we can see how hasValues() and all() interact with more than just the one key value pair.

Pull request has been modified.

[jamestelfer]

@scanlonp thanks for your thoughtful review and commentary!

I'm going to make the following changes as a result:

1. I've kind of done both of your suggestions. There is now a hasNone() method as you suggested, and hasValue(Match.absent()) now throws an error directing the user to use hasNone(). I felt that an explanatory error instead of magically replacing the matcher would keep the API transparent about what's happening internally. Happy to be wrong on this if you'd like it different.
2. I'll add some further tests, no probs!

[jamestelfer]

😞 Rosetta doesn't like my doco updates. I'll come back around on this after easter.

[jamestelfer]

I've updated the documentation with some better examples, and included some tests with more keys and more complicated matching.

I did add a simple check on the matcher so there was a better error message to API consumers. I figured it was better to explicitly fail with a good message than switch the matcher transparently.

It's ready for another look when you're ready @scanlonp!

[scanlonp]

Looks terrific. Tests look great, and the documentation is fantastic.

I like how you handled the hasNone() and the transparent failing of Match.absent().

Again, appreciate the contribution! Excited for this to get used!

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 4dcbf74
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).