Skip to content

Commit 8434813

Browse files
committed
Fix issues found in second independent review
- run_job(): source reservation/security_group from the `requirements` parameter (already fleet+run-combined) instead of job.job_spec.requirements (run-only). Without this, a run provisioning new capacity into a fleet with a fleet-level security_group/reservation would silently ignore it - the same latent bug reservation already had, inherited by security_group. - Azure create_gateway(): use get_gateway_network_security_group_name (the dedicated, always-created gateway NSG) instead of get_default_network_security_group_name. The default/per-location instance NSG can now be skipped when network_security_group_names covers that location, which would have broken gateway provisioning since it was referencing an NSG that might not exist. - OCI: redesign custom-NSG networking. The previous "separate restricted VCN" approach is fundamentally broken - OCI network security groups are VCN-scoped, so a user's NSG can never be attached to an instance in a different VCN than the one the NSG lives in. Fixed by using a single shared subnet for all instances (default-NSG and custom-NSG alike), with no OCI security list attached. dstack's auto-managed NSG now carries explicit SSH ingress and all-egress rules to compensate for the removed security list; custom NSGs remain fully hands-off, per the feature's contract. Existing subnets are migrated in place (security list detached) since the subnet is dstack-owned infrastructure, not a user-supplied resource. Updated docs to clarify a custom NSG must live in dstack's own default VCN. Regression: 388 core backend tests + base/azure/oci targeted suites + server routers/services (requirements, offers, fleets, runs, backends) all passing. Pre-existing unrelated failures (verda/vastai/nebius modules not installed in this environment) left untouched.
1 parent 5d690f0 commit 8434813

9 files changed

Lines changed: 356 additions & 155 deletions

File tree

mkdocs/docs/concepts/backends.md

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,12 +1162,11 @@ There are two ways to configure OCI: using client credentials or using the defau
11621162
```
11631163

11641164
??? info "Custom network security group"
1165-
By default, `dstack` places instances in a shared subnet whose OCI security list opens SSH
1166-
(TCP port 22) to `0.0.0.0/0` and permits all outbound traffic. On top of that, `dstack` creates
1167-
and manages its own network security group (NSG) per project, which only adds a rule allowing all
1168-
traffic within the group so multi-node clusters work out of the box. In other words, the
1169-
permissive SSH ingress and the outbound access come from the subnet's security list, not from the
1170-
auto-managed NSG.
1165+
By default, `dstack` places instances in a shared subnet that has **no** OCI security list
1166+
attached. On top of that, `dstack` creates and manages its own network security group (NSG) per
1167+
project, which grants everything instances need: SSH ingress (TCP port 22) from `0.0.0.0/0`,
1168+
unrestricted egress, and — so multi-node clusters work out of the box — all traffic between
1169+
instances in the group.
11711170

11721171
OCI network security groups are region-scoped, so a custom NSG is configured per region via
11731172
`network_security_group_ids`:
@@ -1185,14 +1184,16 @@ There are two ways to configure OCI: using client credentials or using the defau
11851184
```
11861185

11871186
Regions not covered by `network_security_group_ids` fall back to dstack's auto-created network
1188-
security group and shared subnet.
1189-
1190-
When a custom NSG is used, `dstack` never adds, removes, or modifies its rules, and it places the
1191-
affected instances in a separate VCN and subnet that has **no** OCI security list (a second subnet
1192-
in the same VCN isn't possible here, since the default subnet already occupies the whole VCN's
1193-
address space). This makes the NSG the sole security boundary — there is no longer any implicit
1194-
SSH-from-anywhere or implicit outbound-all coming from a security list. As a result, your custom
1195-
NSG is fully responsible for:
1187+
security group.
1188+
1189+
OCI requires a network security group to belong to the same VCN as the instances it's attached
1190+
to. Since `dstack` provisions instances into its own default VCN per project
1191+
(`dstack-<project>-default-vcn`), your custom NSG must be created in that VCN.
1192+
1193+
When a custom NSG is used, `dstack` never adds, removes, or modifies its rules. Because the shared
1194+
subnet has no security list, the NSG is the sole security boundary for these instances — there is
1195+
no implicit SSH-from-anywhere or implicit outbound-all falling back from a security list. As a
1196+
result, your custom NSG is fully responsible for:
11961197

11971198
- **Ingress**, including SSH (TCP port 22) from wherever you connect.
11981199
- **Egress**, including outbound internet access. Without an egress rule (e.g. allow all to

src/dstack/_internal/core/backends/azure/compute.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,12 @@ def create_gateway(
262262
location=configuration.region,
263263
allocate_public_ip=True,
264264
)
265-
network_security_group = azure_utils.get_default_network_security_group_name(
265+
# Gateways always use the dedicated gateway NSG (created unconditionally in
266+
# `_create_network_resources`, regardless of `network_security_group_names`),
267+
# never the per-location default/custom instance NSG. This keeps gateway
268+
# provisioning working even for locations where the default instance NSG is
269+
# skipped because a custom one is configured for instances.
270+
network_security_group = azure_utils.get_gateway_network_security_group_name(
266271
resource_group=self.config.resource_group,
267272
location=configuration.region,
268273
)

src/dstack/_internal/core/backends/base/compute.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,8 +320,8 @@ def run_job(
320320
user=run.user,
321321
ssh_keys=[SSHKey(public=project_ssh_public_key.strip())],
322322
volumes=volumes,
323-
reservation=job.job_spec.requirements.reservation,
324-
security_group=job.job_spec.requirements.security_group,
323+
reservation=requirements.reservation,
324+
security_group=requirements.security_group,
325325
tags=run.run_spec.merged_profile.tags,
326326
)
327327
instance_offer = instance_offer.copy()

src/dstack/_internal/core/backends/oci/compute.py

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,13 @@ def create_instance(
135135
listing, self.config.compartment_id, region.marketplace_client
136136
)
137137

138+
# All instances - whether using dstack's auto-managed NSG or a custom
139+
# one - share the same subnet. This is required because OCI NSGs are
140+
# VCN-scoped: an NSG can only be attached to a VNIC whose subnet
141+
# belongs to the same VCN the NSG lives in, and dstack only manages
142+
# one VCN per project. The subnet has no security list attached (see
143+
# `get_or_create_subnet`), so the NSG attached to each instance is its
144+
# sole security boundary.
138145
subnet: oci.core.models.Subnet = region.virtual_network_client.get_subnet(
139146
self.config.subnet_ids_per_region[instance_offer.region]
140147
).data
@@ -143,8 +150,7 @@ def create_instance(
143150
security_group_id = self.config.network_security_group_ids.get(
144151
instance_offer.region
145152
)
146-
using_custom_security_group = security_group_id is not None
147-
if not using_custom_security_group:
153+
if security_group_id is None:
148154
security_group = resources.get_or_create_security_group(
149155
f"dstack-{instance_config.project_name}-default-security-group",
150156
subnet.vcn_id,
@@ -155,25 +161,10 @@ def create_instance(
155161
security_group.id, region.virtual_network_client
156162
)
157163
security_group_id = security_group.id
158-
firewall_allow_from_subnet = resources.VCN_CIDR
159-
else:
160-
# A user-managed (custom) NSG is in use. Place the instance in a
161-
# dedicated VCN/subnet that has no OCI security list, so the NSG is
162-
# the sole security boundary. This is a *separate* VCN, not a second
163-
# subnet in the default one: the default subnet already occupies the
164-
# default VCN's entire CIDR block, so a second subnet there would
165-
# conflict. The default VCN/subnet is left completely untouched,
166-
# so instances using dstack's auto-managed NSG are unaffected.
167-
subnet = resources.set_up_restricted_network_resources_in_region(
168-
compartment_id=self.config.compartment_id,
169-
project_name=instance_config.project_name,
170-
client=region.virtual_network_client,
171-
)
172-
firewall_allow_from_subnet = resources.RESTRICTED_VCN_CIDR
173164

174165
cloud_init_user_data = get_user_data(
175166
authorized_keys=instance_config.get_public_keys(),
176-
firewall_allow_from_subnets=[firewall_allow_from_subnet],
167+
firewall_allow_from_subnets=[resources.VCN_CIDR],
177168
)
178169

179170
display_name = generate_unique_instance_name(instance_config)

src/dstack/_internal/core/backends/oci/models.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,17 @@ class OCIBackendConfig(CoreModel):
7777
" use for instances instead of the one `dstack` creates and manages automatically."
7878
" Regions not present in this mapping fall back to dstack's auto-created network"
7979
" security group."
80+
" Because OCI requires a network security group to belong to the same VCN as the"
81+
" instances it's attached to, and `dstack` provisions instances into its own"
82+
" default VCN for each project, the network security group must be created in that"
83+
" VCN (named `dstack-<project>-default-vcn`)."
84+
" `dstack`'s default subnet in that VCN has no OCI security list attached, so"
85+
" whichever network security group is attached to an instance - yours or dstack's"
86+
" own - is the sole security boundary for it."
8087
" When set, `dstack` does not add, remove, or modify any rules on these network"
81-
" security groups, and it places the affected instances in a separate subnet that"
82-
" has no OCI security list, so the network security group becomes the sole security"
83-
" boundary. You are fully responsible for the network security group's rules,"
84-
" including ingress (e.g. SSH), egress (e.g. outbound access to pull Docker images),"
85-
" and, for multi-node clusters, traffic between instances in the group"
88+
" security groups. You are fully responsible for their rules, including ingress"
89+
" (e.g. SSH), egress (e.g. outbound access to pull Docker images), and, for"
90+
" multi-node clusters, traffic between instances in the group"
8691
)
8792
),
8893
] = None

src/dstack/_internal/core/backends/oci/resources.py

Lines changed: 58 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@
3838
ADD_SECURITY_RULES_MAX_CHUNK_SIZE = 25
3939
LIST_OBJECTS_MAX_LIMIT = 1000
4040
VCN_CIDR = "10.0.0.0/16"
41-
# CIDR for the dedicated VCN used for custom-NSG instances (see `get_or_create_restricted_vcn`).
42-
# Must not overlap `VCN_CIDR` in case the two VCNs are ever peered.
43-
RESTRICTED_VCN_CIDR = "10.1.0.0/16"
4441
WAIT_FOR_COMPARTMENT_ATTEMPS = 36
4542
WAIT_FOR_COMPARTMENT_DELAY = 5
4643

@@ -559,75 +556,52 @@ def get_or_create_vcn(
559556
def get_or_create_subnet(
560557
name: str, vcn_id: str, compartment_id: str, client: oci.core.VirtualNetworkClient
561558
) -> oci.core.models.Subnet:
562-
query_results = chain_paginated_responses(
563-
client.list_subnets, compartment_id=compartment_id, display_name=name
564-
)
565-
if subnet := next(query_results, None):
566-
return subnet
567-
568-
return client.create_subnet(
569-
oci.core.models.CreateSubnetDetails(
570-
cidr_block=VCN_CIDR,
571-
compartment_id=compartment_id,
572-
display_name=name,
573-
vcn_id=vcn_id,
574-
)
575-
).data
576-
577-
578-
def get_or_create_restricted_vcn(
579-
name: str, compartment_id: str, client: oci.core.VirtualNetworkClient
580-
) -> oci.core.models.Vcn:
581559
"""
582-
Like `get_or_create_vcn`, but a separate VCN (own CIDR block) dedicated to
583-
custom-NSG instances. A *separate* VCN is used - rather than a second subnet
584-
in the existing default VCN - because the default subnet already occupies
585-
the default VCN's entire CIDR block, and adding a second CIDR block to an
586-
existing VCN requires an async OCI operation (`add_vcn_cidr`) that takes
587-
the VCN out of service for subnet/route-table updates for its duration.
588-
A brand new VCN avoids that entirely and keeps the default VCN/subnet used
589-
by dstack's auto-managed instances completely untouched.
590-
"""
591-
query_results = chain_paginated_responses(
592-
client.list_vcns, compartment_id=compartment_id, display_name=name
593-
)
594-
if vcn := next(query_results, None):
595-
return vcn
596-
597-
return client.create_vcn(
598-
oci.core.models.CreateVcnDetails(
599-
cidr_blocks=[RESTRICTED_VCN_CIDR],
600-
compartment_id=compartment_id,
601-
display_name=name,
602-
)
603-
).data
604-
605-
606-
def get_or_create_restricted_subnet(
607-
name: str, vcn_id: str, compartment_id: str, client: oci.core.VirtualNetworkClient
608-
) -> oci.core.models.Subnet:
609-
"""
610-
Like `get_or_create_subnet`, but creates the subnet with an empty list of
611-
security lists (`security_list_ids=[]`) instead of letting OCI attach the
612-
VCN's permissive default security list. Must be created in a VCN returned
613-
by `get_or_create_restricted_vcn`, not the default VCN.
614-
615-
This is used for instances that run with a user-managed (custom) network
616-
security group. With no security list contributing rules, the NSG becomes
617-
the sole source of truth for what traffic is allowed to and from these
618-
instances. OCI evaluates security lists and NSGs as a union of allows, so an
619-
empty security list list simply means "the security-list layer grants
620-
nothing"; it does not deny anything on its own.
560+
The subnet is created with an empty list of security lists
561+
(`security_list_ids=[]`) instead of letting OCI attach the VCN's permissive
562+
default security list. All instances - whether they use dstack's
563+
auto-managed network security group (NSG) or a user-supplied custom one -
564+
live in this single shared subnet, so the NSG attached to each instance's
565+
VNIC is the sole security boundary; there is no separate security-list
566+
layer to reason about.
567+
568+
A single shared subnet (rather than a second, NSG-only subnet) is required
569+
because OCI NSGs are scoped to a single VCN: an NSG can only be attached to
570+
a VNIC whose subnet belongs to the *same* VCN the NSG was created in. Since
571+
dstack only manages one VCN, a user's custom NSG must live in - and a
572+
custom-NSG instance must therefore be placed in a subnet within - that same
573+
VCN.
574+
575+
For instances using dstack's auto-managed NSG, the rules normally
576+
contributed by the security list (SSH ingress, unrestricted egress) are
577+
added directly to that NSG instead; see
578+
`update_security_group_rules_for_runner_instances`. Instances using a
579+
user-supplied NSG get no such compensating rules - per dstack's "fully
580+
hands-off" contract, it never adds, removes, or otherwise modifies rules on
581+
a user-supplied security group, so the user is fully responsible for
582+
allowing the traffic their instances need (including SSH).
621583
"""
622584
query_results = chain_paginated_responses(
623585
client.list_subnets, compartment_id=compartment_id, display_name=name
624586
)
625587
if subnet := next(query_results, None):
588+
if subnet.security_list_ids:
589+
# A subnet created before this fix still has the VCN's default
590+
# security list attached. Since dstack owns this subnet
591+
# (it is not user-supplied), it's safe to update it in place -
592+
# detaching the security list so the NSG becomes the sole
593+
# security boundary for every instance in it, matching newly
594+
# created subnets. This is unrelated to dstack's "fully
595+
# hands-off" contract for user-supplied *security groups*, which
596+
# this does not touch.
597+
subnet = client.update_subnet(
598+
subnet.id, oci.core.models.UpdateSubnetDetails(security_list_ids=[])
599+
).data
626600
return subnet
627601

628602
return client.create_subnet(
629603
oci.core.models.CreateSubnetDetails(
630-
cidr_block=RESTRICTED_VCN_CIDR,
604+
cidr_block=VCN_CIDR,
631605
compartment_id=compartment_id,
632606
display_name=name,
633607
vcn_id=vcn_id,
@@ -636,28 +610,6 @@ def get_or_create_restricted_subnet(
636610
).data
637611

638612

639-
def set_up_restricted_network_resources_in_region(
640-
compartment_id: str, project_name: str, client: oci.core.VirtualNetworkClient
641-
) -> oci.core.models.Subnet:
642-
"""
643-
Like `set_up_network_resources_in_region`, but for the dedicated VCN/subnet
644-
used by custom-NSG instances (see `get_or_create_restricted_vcn` and
645-
`get_or_create_restricted_subnet`). Idempotent - safe to call on every
646-
instance launch, mirroring how `get_or_create_security_group` is already
647-
called on every launch for the default (non-custom-NSG) path.
648-
"""
649-
vcn = get_or_create_restricted_vcn(
650-
f"dstack-{project_name}-restricted-vcn", compartment_id, client
651-
)
652-
internet_gateway = get_or_create_internet_gateway(
653-
f"dstack-{project_name}-restricted-internet-gateway", vcn.id, compartment_id, client
654-
)
655-
update_route_table(vcn.default_route_table_id, internet_gateway.id, client)
656-
return get_or_create_restricted_subnet(
657-
f"dstack-{project_name}-restricted-subnet", vcn.id, compartment_id, client
658-
)
659-
660-
661613
def get_or_create_internet_gateway(
662614
name: str, vcn_id: str, compartment_id: str, client: oci.core.VirtualNetworkClient
663615
) -> oci.core.models.InternetGateway:
@@ -714,8 +666,11 @@ def get_or_create_security_group(
714666
def update_security_group_rules_for_runner_instances(
715667
security_group_id: str, client: oci.core.VirtualNetworkClient
716668
) -> None:
717-
# These rules are combined with subnet's default Security List that allows
718-
# ingress TCP on port 22 from anywhere
669+
# The subnet these instances live in has no security list attached (see
670+
# `get_or_create_subnet`), so this NSG must grant everything a runner
671+
# instance needs on its own: SSH ingress from anywhere and unrestricted
672+
# egress, in addition to allowing all traffic between instances that share
673+
# this NSG.
719674
rules = [
720675
SecurityRule(
721676
description="Allow all traffic within this security group",
@@ -724,6 +679,23 @@ def update_security_group_rules_for_runner_instances(
724679
source=security_group_id,
725680
protocol="all",
726681
),
682+
SecurityRule(
683+
description="Allow SSH ingress from anywhere",
684+
direction=oci.core.models.AddSecurityRuleDetails.DIRECTION_INGRESS,
685+
source_type=oci.core.models.AddSecurityRuleDetails.SOURCE_TYPE_CIDR_BLOCK,
686+
source="0.0.0.0/0",
687+
protocol="6", # TCP
688+
tcp_options=oci.core.models.TcpOptions(
689+
destination_port_range=oci.core.models.PortRange(min=22, max=22)
690+
),
691+
),
692+
SecurityRule(
693+
description="Allow all egress traffic",
694+
direction=oci.core.models.AddSecurityRuleDetails.DIRECTION_EGRESS,
695+
destination_type=oci.core.models.AddSecurityRuleDetails.DESTINATION_TYPE_CIDR_BLOCK,
696+
destination="0.0.0.0/0",
697+
protocol="all",
698+
),
727699
]
728700
update_security_group_rules(security_group_id, rules, client)
729701

0 commit comments

Comments
 (0)