Skip to content

Commit

Permalink
[SERVE][AWS] Don't open ports when all ports and all protocols are sp…
Browse files Browse the repository at this point in the history
…ecified (#3637)

* fix: bug if -1 for all ports

* fix: bug when all traffic doesn't show from and to port

* fix: clean code

* refactor: add clarifying comments and a todo

* Update sky/provision/aws/instance.py

Co-authored-by: Zhanghao Wu <[email protected]>

---------

Co-authored-by: Zhanghao Wu <[email protected]>
  • Loading branch information
JGSweets and Michaelvll authored Jun 21, 2024
1 parent 3436b8c commit feaaa3a
Showing 1 changed file with 15 additions and 8 deletions.
23 changes: 15 additions & 8 deletions sky/provision/aws/instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -717,16 +717,23 @@ def open_ports(

existing_ports: Set[int] = set()
for existing_rule in sg.ip_permissions:
# Skip any non-tcp rules.
if existing_rule['IpProtocol'] != 'tcp':
# Skip any non-tcp rules or if all traffic (-1) is specified.
if existing_rule['IpProtocol'] not in ['tcp', '-1']:
continue
# Skip any rules that don't have a FromPort or ToPort.
if 'FromPort' not in existing_rule or 'ToPort' not in existing_rule:
continue
existing_ports.update(
range(existing_rule['FromPort'], existing_rule['ToPort'] + 1))
ports_to_open = resources_utils.port_set_to_ranges(
resources_utils.port_ranges_to_set(ports) - existing_ports)
if 'FromPort' in existing_rule and 'ToPort' in existing_rule:
existing_ports.update(
range(existing_rule['FromPort'], existing_rule['ToPort'] + 1))
elif existing_rule['IpProtocol'] == '-1':
# For AWS, IpProtocol = -1 means all traffic
existing_ports.add(-1)
break

ports_to_open = []
# Do not need to open any ports when all traffic is already allowed.
if -1 not in existing_ports:
ports_to_open = resources_utils.port_set_to_ranges(
resources_utils.port_ranges_to_set(ports) - existing_ports)

ip_permissions = []
for port in ports_to_open:
Expand Down

0 comments on commit feaaa3a

Please sign in to comment.