feat: Add public IP association to github runner

[imishchuk-carbon]

Description

• Add option to associate public IP with runner (disabled by default)

Fixes 3528

Suggested changes have been used in our env for over a month and it works as expected.

Checklists

Development and testing:

• All tests related to the changed code pass in development
• Pull request is ready for review

[imishchuk-carbon]

Pre-commit tests

[npalm]

@imishchuk-carbon thx looks good. Tested with ip4 did you also check the change with ip6 / dualstack?

[imishchuk-carbon]

Hey @npalm
No, did not test with IPv6/dualstack. Can do that, if it's required.

[npalm]

Hey @npalm No, did not test with IPv6/dualstack. Can do that, if it's required.

In case you have time would be nice. I think the current module is not taking care of ipv6. So assume it is not working. In case you have time to run a quick check that would be nice. If not please can you update the variable description with only tested with ipv4?

[imishchuk-carbon]

Hey @npalm
Tested with dualstack and it did not work for me, in the end. Issues encountered:

1. Using IPv6/Dualstack makes calls to dualstack endpoint of S3, which requires additional configuration, such as

ipv6=$(TOKEN=`curl -X PUT "http://169.254.169.254/latest/api/token" -H "X-aws-ec2-metadata-token-ttl-seconds: 21600"` \
&& curl -IH "X-aws-ec2-metadata-token: $TOKEN" -v http://169.254.169.254/latest/meta-data/ipv6 2>/dev/null | head -n 1 | cut -d$' ' -f2)

if [[ $ipv6 == '200' ]]; then
  aws configure set default.s3.use_dualstack_endpoint true
  aws configure set default.s3.addressing_style virtual
fi

2. Connection to https://pipelinesghubeus22.actions.githubusercontent.com/.. time outs. As far as I see, Github does not support IPv6 for some endpoints. This probably can be worked around by using Egress-Only gateway in AWS, but requires further testing and that is getting out-of-scope for this case.
3. Runner startup is much slower for some reason. While on IPv4-only, it takes less than 2 minutes from instance start to job being picked up (We are using ephemeral runners and execute config/install scripts from scratch). On IPv6/Dualstack it took ~6 minutes to start the runner. Now sure where this slowdown is coming from...

---

That being said, I've renamed variable to explicitly state that it is for IPv4 and added Only tested with IPv4 to variable description.

[imishchuk-carbon]

Thanks @npalm

[ay0o]

@npalm @imishchuk-carbon I know this is already pushed to main, but I'm wondering about a couple of things.

Shouldn't this code be as simple as:

network_interfaces {
  associate_public_ipv4_address = var.associate_public_ip4_address
}

where associate_public_ip4_address is a bool variable configured to true by default?

This is important because some tools rely on the existence of this property to work properly. For example, if you have a SCP to forbid EC2 instances with an associated public IP, the runners will not be created because you might have three scenarios:

• If the runner has an associated public IP: ❌
• If the runner doesn't have an associated public IP: ✅
• And here comes the problem. If the runner doesn't specify the associate public IP address, it means the status of the property is not-specified, which will be blocked by the SCP because only EC2 with associate public IP address to false are allowed.

For this reason, this property should be always specified, whether it's true or false. Right now, you only define it to true if the value of this variable is true, or don't specify it at all.

Additionally, why do you need to assign the security groups within network_interfaces if they are already assigned using vpc_security_group_ids?

[imishchuk-carbon]

Hello @ay0o

Shouldn't this code be as simple as:

network_interfaces {
  associate_public_ipv4_address = var.associate_public_ip4_address
}

Additionally, why do you need to assign the security groups within network_interfaces if they are already assigned using vpc_security_group_ids

No, see commit which tried this approach. Once you define network_interfaces block you need to specify security_groups and that conflicts with vpc_security_group_ids.

where associate_public_ip4_address is a bool variable configured to true by default?

Setting it to true would unnecessarily change module's default behavior.

And here comes the problem. If the runner doesn't specify the associate public IP address, it means the status of the property is not-specified, which will be blocked by the SCP because only EC2 with associate public IP address to false are allowed.

Do you have an example SCP in mind? For what I see, in SCP you define which actions are denied, e.g. from here

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Deny",
      "Action": [
        "ec2:RunInstances"
      ],
      "Condition": {
        "BoolIfExists": {
          "ec2:AssociatePublicIpAddress": "true"
        }
      },
      "Resource": "arn:aws:ec2:*:*:network-interface/*"
    },
    {
      "Action": [
        "ec2:AssociateAddress"
      ],
      "Resource": "*",
      "Effect": "Deny"
    }
  ]
}

And that means that if PublicIP association is explicitly requested - action will be denied, otherwise instance will be deployed without public IP. Unless, I am missing something?

---

I guess this functionality could be implemented by fully removing vpc_security_group_ids and just using network_interfaces block but that needs testing.

[ay0o]

@imishchuk-carbon you can forget about my message. There was some misunderstanding with the AWS documentation, because it led to think that AssociatePublicIpAddress could be true, false or not specified at all. However, that's not true. The property is only added to the request when it's true, if it's false, it is not, so my point of always forcing the network_interfaces block to either true or false doesn't apply.