Page MenuHomePhabricator

[terraform] configure personal ip address through script and not allowed_ips var
ClosedPublic

Authored by will on Aug 4 2024, 8:45 PM.
Tags
None
Referenced Files
F3197734: D12960.diff
Sat, Nov 9, 9:32 AM
F3193567: D12960.id43300.diff
Fri, Nov 8, 11:05 PM
Unknown Object (File)
Fri, Nov 8, 12:24 PM
Unknown Object (File)
Fri, Nov 8, 12:24 PM
Unknown Object (File)
Fri, Nov 8, 12:24 PM
Unknown Object (File)
Sat, Nov 2, 5:25 AM
Unknown Object (File)
Sat, Nov 2, 5:25 AM
Unknown Object (File)
Sat, Nov 2, 5:24 AM
Subscribers

Details

Summary

We rethought the approach in allowing script access to receive a health check from the keyserver load balancer.

Instead of allowing allowed_ips constant access to the load balancer, we decided to simply have the script enable access to the load balancer from the ip address of the
script caller by adding an ingress rule through aws cli during the duration of the script's run.

Test Plan

Ran the aws-deploy script and ensured that a separate ip machine was unable to access the load balancer while the script was able to perform a health check

Diff Detail

Repository
rCOMM Comm
Branch
script_changes
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

will requested review of this revision.Aug 4 2024, 9:03 PM
services/terraform/self-host/aws-deploy.sh
76 ↗(On Diff #43058)

What will the value of this string be if curl fails due to e.g. no network connection?

80–91 ↗(On Diff #43058)

Can we avoid calling aws ec2 describe-security-groups twice?

(Not sure if my suggestion actually works)

services/terraform/self-host/aws-deploy.sh
76 ↗(On Diff #43058)
# Get the current public IP address
ip_address=$(curl -s ipv4.wtfismyip.com/text)

# Check if the IP address was retrieved successfully
if [ -z "$ip_address" ]; then
  echo "Failed to retrieve IP address. Exiting."
  return 1
fi

wonder if we should do something like this before we call authorize-security-group-ingress

80–91 ↗(On Diff #43058)

bartek's suggestion makes sense to me and i think it should work

varun requested changes to this revision.Aug 7 2024, 4:16 PM
This revision now requires changes to proceed.Aug 7 2024, 4:16 PM
services/terraform/self-host/aws-deploy.sh
76 ↗(On Diff #43058)

In that case it would be (6) Could not resolve host: ipv4.wtfismyip.com and the command would fail with a faulty cidr.

services/terraform/self-host/aws-deploy.sh
76 ↗(On Diff #43058)

Just tested this and it works. Will include in the next rebase

80–91 ↗(On Diff #43058)

This suggestion works. Including in next rebase

This revision is now accepted and ready to land.Aug 9 2024, 2:04 PM
services/terraform/self-host/aws-deploy.sh
39 ↗(On Diff #43278)

Didn't we want to be consistent about [ vs [[?

bartek added inline comments.
services/terraform/self-host/aws-deploy.sh
45 ↗(On Diff #43278)

nit: I guess sg stands for "security group" so you don't need to have "sg group"

This revision was landed with ongoing or failed builds.Aug 11 2024, 8:54 PM
This revision was automatically updated to reflect the committed changes.