(ec2): addIngressRule and addEgressRule detect unresolved tokens as duplicates

[peterwoodworth]

What is the problem?

When passing in a token to the peer property of these functions, the renderPeer() function is called and will return a constant value '{IndirectPeer}'.

aws-cdk/packages/@aws-cdk/aws-ec2/lib/security-group.ts

Lines 174 to 193 in 5831456

	function determineRuleScope(
	group: SecurityGroupBase,
	peer: IPeer,
	connection: Port,
	fromTo: 'from' | 'to',
	remoteRule?: boolean): [SecurityGroupBase, string] {
	if (remoteRule && SecurityGroupBase.isSecurityGroup(peer) && differentStacks(group, peer)) {
	// Reversed
	const reversedFromTo = fromTo === 'from' ? 'to' : 'from';
	return [peer, `${group.uniqueId}:${connection} ${reversedFromTo}`];
	} else {
	// Regular (do old ID escaping to in order to not disturb existing deployments)
	return [group, `${fromTo} ${renderPeer(peer)}:${connection}`.replace('/', '_')];
	}
	}
	function renderPeer(peer: IPeer) {
	return Token.isUnresolved(peer.uniqueId) ? '{IndirectPeer}' : peer.uniqueId;
	}

If the other properties remain constant, calling this multiple times will cause only one rule to be added to the security group due to the duplicate checker seen here

aws-cdk/packages/@aws-cdk/aws-ec2/lib/security-group.ts

Lines 86 to 96 in 5831456

	const [scope, id] = determineRuleScope(this, peer, connection, 'from', remoteRule);
	// Skip duplicates
	if (scope.node.tryFindChild(id) === undefined) {
	new CfnSecurityGroupIngress(scope, id, {
	groupId: this.securityGroupId,
	...peer.toIngressRuleConfig(),
	...connection.toRuleJson(),
	description,
	});
	}

Reproduction Steps

Call addIngressRule() or addEgressRule() multiple times on a security group, while only changing the peer prop from one token to another token. Only one rule will be added

What did you expect to happen?

I was trying to add multiple ingress rules to a security group

What actually happened?

I only added one ingress rule to a security group

CDK CLI Version

latest

Framework Version

No response

Node.js Version

16

OS

mac

Language

Typescript

Language Version

No response

Other information

No response

[njlynch]

The first solution that comes to mind would be to maintain a lookup of the peer.uniqueId to renderPeer result. If it's the first unresolved Token we've seen, we return {IndirectPeer}. For each new unique Id, we add a counter (e.g., {IndirectPeer2}. This would maintain backwards compatibility for existing apps, but allow new scenarios with multiple unresolved tokens.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.