GroupId should not be required in AWS::EC2::SecurityGroup Egress

[pawelrubin]

Problem

troposphere incorrectly identifies GroupId parameter of SecurityGroupEgress in AWS::EC2::SecurityGroup as required.

Context

GroupId is a required parameter in AWS::EC2::SecurityGroupEgress type, however not in AWS::EC2::SecurityGroup Egress

The template below is a valid Cloudformation template

Parameters:
  DefaultSecurityGroup:
    Type: String
  VpcId:
    Type: String

Resources:
  MySecurityGroup:
    Type: AWS::EC2::SecurityGroup
    Properties:
      GroupDescription: My security group
      VpcId:
        Ref: VpcId
      SecurityGroupIngress:
        - IpProtocol: tcp
          FromPort: 0
          ToPort: 65535
          SourceSecurityGroupId:
            Ref: DefaultSecurityGroup
      SecurityGroupEgress:
        - IpProtocol: "-1"
          CidrIp: 0.0.0.0/0

Steps to reproduce

Consider the code below

from troposphere import Parameter, Ref, Template
from troposphere.ec2 import SecurityGroup, SecurityGroupEgress, SecurityGroupIngress

template = Template()

vpc_id = template.add_parameter(Parameter("VpcId", Type="String"))
default_security_group = template.add_parameter(Parameter("DefaultSecurityGroup", Type="String"))

template.add_resource(
    SecurityGroup(
        "MySecurityGroup",
        GroupDescription="My security group",
        VpcId=Ref(vpc_id),
        SecurityGroupIngress=[
            SecurityGroupIngress(
                title=None,
                IpProtocol="tcp",
                FromPort=0,
                ToPort=65535,
                SourceSecurityGroupId=Ref(default_security_group),
            )
        ],
        SecurityGroupEgress=[
            SecurityGroupEgress(
                title=None,
                IpProtocol="-1",
                CidrIp="0.0.0.0/0",
            )
        ],
    )
)

print(template.to_yaml())

Actual result

ValueError: Resource GroupId required in type AWS::EC2::SecurityGroupEgress

Expected result

no errors

[pawelrubin]

I've realized that one can declare the ingress and egress with dicts.

from troposphere import Parameter, Ref, Template
from troposphere.ec2 import SecurityGroup, SecurityGroupEgress, SecurityGroupIngress

template = Template()

vpc_id = template.add_parameter(Parameter("VpcId", Type="String"))
default_security_group = template.add_parameter(Parameter("DefaultSecurityGroup", Type="String"))

template.add_resource(
    SecurityGroup(
        "MySecurityGroup",
        GroupDescription="My security group",
        VpcId=Ref(vpc_id),
        SecurityGroupIngress=[
            {
                "IpProtocol": "tcp",
                "FromPort": 0,
                "ToPort": 65535,
                "SourceSecurityGroupId": Ref(default_security_group),
            }
        ],
        SecurityGroupEgress=[
            {
                "IpProtocol": "-1",
                "CidrIp": "0.0.0.0/0",
            }
        ],
    )
)

print(template.to_yaml())

IMO this isn't obvious and should be documented.

Both SecurityGroupEgress and SecurityGroupIngress are defined as list, not as [SecurityGroupEngress]/[SecurityGroupEngress]. Perhaps a more specific type - dict[str, Any] or TypedDict would be better.

        "SecurityGroupEgress": (list, False),
        "SecurityGroupIngress": (list, False),

[markpeek]

Apologies for the delay in responding to this issue and the confusion around the Egress/Ingress properties.

You are correct that those lists should have a better defined type. This form of specification pre-dated the more specific typing and was kept that way for backward compatibility purposes. Defining a SecurityGroup should use the Property Types Egress/Ingress in these lists (which don't define a GroupId) rather than the resource types SecurityGroupEgress/SecurityGroupIngress.