Skip to content

Commit

Permalink
fix(cli): the LoadBalancerProvider doesn't match LBs when querying by…
Browse files Browse the repository at this point in the history
… a subset of tags (#32164)

There was a regression in the load balancer lookup, in which we started
requiring that the set of tags in the query is strictly the same as the
set of tags in the load balancer (rather than merely a subset of it).

Remove the length equality constraint and also simplify the code to make
the intent clearer.

Fixes #32161.

### Checklist
- [ ] My code adheres to the [CONTRIBUTING
GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and
[DESIGN
GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

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

Co-authored-by: Momo Kornher <[email protected]>
  • Loading branch information
otaviomacedo and mrgrain authored Nov 17, 2024
1 parent 089e9d8 commit f75dc72
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 9 deletions.
15 changes: 6 additions & 9 deletions packages/aws-cdk/lib/context-providers/load-balancers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,12 @@ class LoadBalancerProvider {
}
return (await this.describeTags(loadBalancers.map((lb) => lb.LoadBalancerArn!)))
.filter((tagDescription) => {
return (
tagDescription.Tags?.length === this.filter.loadBalancerTags?.length &&
tagDescription.Tags?.filter(
(tag) =>
!this.filter.loadBalancerTags!.some((filter) => {
return filter.key === tag.Key && filter.value === tag.Value;
}),
).length === 0
);
// For every tag in the filter, there is some tag in the LB that matches it.
// In other words, the set of tags in the filter is a subset of the set of tags in the LB.
return this.filter.loadBalancerTags!.every((filter) => {
return tagDescription.Tags?.some((tag) =>
filter.key === tag.Key && filter.value === tag.Value);
});
})
.flatMap((tag) => loadBalancers.filter((loadBalancer) => tag.ResourceArn === loadBalancer.LoadBalancerArn));
}
Expand Down
46 changes: 46 additions & 0 deletions packages/aws-cdk/test/context-providers/load-balancers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,52 @@ describe('load balancer context provider plugin', () => {
});
});

test('looks up by tags - query by subset', async () => {
// GIVEN
mockElasticLoadBalancingV2Client
.on(DescribeLoadBalancersCommand)
.resolves({
LoadBalancers: [
{
IpAddressType: 'ipv4',
LoadBalancerArn: 'arn:load-balancer2',
DNSName: 'dns2.example.com',
CanonicalHostedZoneId: 'Z1234',
SecurityGroups: ['sg-1234'],
VpcId: 'vpc-1234',
Type: 'application',
},
],
})
.on(DescribeTagsCommand)
.resolves({
TagDescriptions: [
{
ResourceArn: 'arn:load-balancer2',
Tags: [
// Load balancer has two tags...
{ Key: 'some', Value: 'tag' },
{ Key: 'second', Value: 'tag2' },
],
},
],
});
const provider = new LoadBalancerContextProviderPlugin(mockSDK);

// WHEN
const result = await provider.getValue({
account: '1234',
region: 'us-east-1',
loadBalancerType: LoadBalancerType.APPLICATION,
loadBalancerTags: [
// ...but we are querying for only one of them
{ key: 'second', value: 'tag2' },
],
});

expect(result.loadBalancerArn).toEqual('arn:load-balancer2');
});

test('filters by type', async () => {
// GIVEN
mockElasticLoadBalancingV2Client
Expand Down

0 comments on commit f75dc72

Please sign in to comment.