Skip to content

Commit 5d690f0

Browse files
committed
Fix issues found in independent review
An independent review of the custom-security-group feature (previous two commits) found several real gaps. Fixed all of them: - security_group was dropped when a run provisioned a new instance into an existing fleet (only the fleet-apply path honored it). Fixed by threading security_group through the Requirements pipeline exactly like reservation already is (Requirements.security_group, combine_fleet_and_run_profiles/ combine_fleet_and_run_requirements, and sourcing run_job's InstanceConfiguration from job.job_spec.requirements.security_group instead of the run's raw profile). - security_group was silently ignored when an offer resolved to a backend that doesn't support it (e.g. GCP). offers.py now narrows backend_types to BACKENDS_WITH_SECURITY_GROUP_SUPPORT when security_group is set, mirroring the existing reservation filtering. - AWS: the configurator forbade combining security_group_name with security_group_ids, but compute.py implements (and docs/tests described) a fallback from ids to name - the combination is now allowed. Also added validation catching region-key typos in security_group_ids, and a clearer ComputeError instead of a confusing NoCapacityError retry loop when a configured security group doesn't exist in the target VPC. - Azure: renamed network_security_group_ids to network_security_group_names since the values are NSG names (not IDs) scoped to the backend's resource_group, and added region-key typo validation. - GCP: create_firewall_rules no longer disables the gateway firewall rule, matching AWS/Azure/OCI where gateway security resources are always auto-managed regardless of the custom-security-group settings. - OCI: the shared subnet has no security_list_ids, so it inherits the VCN's permissive default security list (SSH open to 0.0.0.0/0, allow-all egress). Since OCI evaluates security lists and NSGs as a union of allows, a custom NSG could not actually restrict anything. Fixed by routing custom-NSG instances into a separate, dedicated VCN/subnet with no security list, so the NSG becomes the sole security boundary. The default VCN/subnet used by auto-managed-NSG instances is completely untouched. Also added region-key typo validation and corrected the docs to accurately attribute default SSH exposure to the security list, not the NSG.
1 parent 3be52da commit 5d690f0

30 files changed

Lines changed: 646 additions & 47 deletions

File tree

mkdocs/docs/concepts/backends.md

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,9 @@ There are two ways to configure Azure: using a client secret or using the defaul
489489
By default, `dstack` creates and manages its own network security group (opening SSH to the internet
490490
and allowing all traffic within the group so multi-node clusters work out of the box).
491491
Azure NSG names must be unique within a resource group regardless of region, so a custom NSG is
492-
configured per location via `network_security_group_ids`:
492+
configured per location via `network_security_group_names`. The values are plain NSG names
493+
within the configured `resource_group` (not full Azure resource IDs), so NSGs in a different
494+
resource group cannot be referenced:
493495

494496
```yaml
495497
projects:
@@ -499,12 +501,12 @@ There are two ways to configure Azure: using a client secret or using the defaul
499501
creds:
500502
type: default
501503
regions: [westeurope, eastus]
502-
network_security_group_ids:
504+
network_security_group_names:
503505
westeurope: my-network-security-group-we
504506
eastus: my-network-security-group-eus
505507
```
506508

507-
Locations not covered by `network_security_group_ids` fall back to dstack's auto-created network
509+
Locations not covered by `network_security_group_names` fall back to dstack's auto-created network
508510
security group. Either way, `dstack` attaches the network security group to instances as-is and
509511
never adds, removes, or modifies its rules. You're responsible for SSH reachability and, for
510512
multi-node clusters, for allowing traffic between instances in the group.
@@ -730,11 +732,11 @@ gcloud projects list --format="json(projectId)"
730732
Additionally, [Cloud NAT](https://cloud.google.com/nat/docs/overview) must be configured to provide access to external resources for provisioned instances.
731733

732734
??? info "Custom firewall rules"
733-
By default, `dstack` creates VPC firewall rules allowing inbound SSH (and, for gateways, HTTP/HTTPS) from
734-
the internet, scoped to the `dstack-runner-instance` and `dstack-gateway-instance` target tags.
735+
By default, `dstack` creates a VPC firewall rule allowing inbound SSH from the internet to instances,
736+
scoped to the `dstack-runner-instance` target tag.
735737
Unlike AWS/Azure/OCI, GCP firewall rules apply to the whole VPC rather than to a single attachable resource,
736738
so there's no per-fleet override — if you manage your own firewall rules and don't want `dstack` creating
737-
rules that open ports to `0.0.0.0/0`, disable this at the project level with `create_firewall_rules: false`:
739+
a rule that opens port 22 to `0.0.0.0/0`, disable this at the project level with `create_firewall_rules: false`:
738740

739741
```yaml
740742
projects:
@@ -749,7 +751,9 @@ gcloud projects list --format="json(projectId)"
749751
```
750752

751753
You're then responsible for ensuring your VPC's own firewall rules allow whatever SSH and cluster traffic
752-
`dstack` needs.
754+
`dstack` needs. This setting only affects the instance SSH rule — the separate firewall rule `dstack`
755+
creates for gateways (allowing HTTP/HTTPS from the internet, scoped to the `dstack-gateway-instance`
756+
target tag) is always auto-managed, since gateways are meant to be internet-reachable.
753757

754758
### Lambda
755759

@@ -1158,8 +1162,13 @@ There are two ways to configure OCI: using client credentials or using the defau
11581162
```
11591163

11601164
??? info "Custom network security group"
1161-
By default, `dstack` creates and manages its own network security group per project (opening SSH to
1162-
`0.0.0.0/0` and allowing all traffic within the VCN so multi-node clusters work out of the box).
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.
1171+
11631172
OCI network security groups are region-scoped, so a custom NSG is configured per region via
11641173
`network_security_group_ids`:
11651174

@@ -1176,9 +1185,20 @@ There are two ways to configure OCI: using client credentials or using the defau
11761185
```
11771186

11781187
Regions not covered by `network_security_group_ids` fall back to dstack's auto-created network
1179-
security group. Either way, `dstack` attaches the network security group to instances as-is and
1180-
never adds, removes, or modifies its rules. You're responsible for SSH reachability and, for
1181-
multi-node clusters, for allowing traffic between instances in the group.
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:
1196+
1197+
- **Ingress**, including SSH (TCP port 22) from wherever you connect.
1198+
- **Egress**, including outbound internet access. Without an egress rule (e.g. allow all to
1199+
`0.0.0.0/0`), instances will have no outbound connectivity and will fail to pull Docker images
1200+
and start runs.
1201+
- For multi-node clusters, **traffic between instances** in the group.
11821202

11831203
You can also override this per fleet or run using the `security_group` profile property.
11841204

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,9 +380,18 @@ def create_instance(
380380
)
381381
except botocore.exceptions.ClientError as e:
382382
logger.warning("Got botocore.exceptions.ClientError: %s", e)
383-
if e.response["Error"]["Code"] == "InvalidParameterValue":
383+
error_code = e.response["Error"]["Code"]
384+
if error_code == "InvalidParameterValue":
384385
msg = e.response["Error"].get("Message", "")
385386
raise ComputeError(f"Invalid AWS request: {msg}")
387+
if error_code == "InvalidGroup.NotFound":
388+
# A misconfigured security group (e.g. wrong VPC/region) is not a
389+
# capacity issue, so surface it clearly instead of retrying other AZs.
390+
msg = e.response["Error"].get("Message", "")
391+
raise ComputeError(
392+
f"Security group not found for instance in region"
393+
f" {instance_offer.region}: {msg}"
394+
)
386395
continue
387396
instance = response[0]
388397
# wait_until_running() is only needed so that instance is immediately ready for volume attach.

src/dstack/_internal/core/backends/aws/configurator.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,16 @@ def _check_config_iam_instance_profile(
148148
)
149149

150150
def _check_config_security_group(self, config: AWSBackendConfigWithCreds):
151-
if config.security_group_name is not None and config.security_group_ids is not None:
151+
if config.security_group_ids is None:
152+
return
153+
regions = config.regions if config.regions is not None else DEFAULT_REGIONS
154+
unknown_regions = [r for r in config.security_group_ids if r not in regions]
155+
if unknown_regions:
152156
raise ServerClientError(
153-
msg="Only one of `security_group_name` and `security_group_ids` can be specified"
157+
msg=(
158+
f"`security_group_ids` specifies regions not in `regions`: {unknown_regions}."
159+
" This is likely a typo — remove the extra keys or add them to `regions`"
160+
)
154161
)
155162

156163
def _check_config_vpc(self, session: Session, config: AWSBackendConfigWithCreds):

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ def create_instance(
149149
allocate_public_ip=allocate_public_ip,
150150
)
151151
network_security_group = instance_config.security_group
152-
if network_security_group is None and self.config.network_security_group_ids is not None:
153-
network_security_group = self.config.network_security_group_ids.get(location)
152+
if network_security_group is None and self.config.network_security_group_names is not None:
153+
network_security_group = self.config.network_security_group_names.get(location)
154154
if network_security_group is None:
155155
network_security_group = azure_utils.get_default_network_security_group_name(
156156
resource_group=self.config.resource_group,

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

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ def validate_config(self, config: AzureBackendConfigWithCreds, default_creds_ena
104104
self._check_config_resource_group(config=config, credential=credential)
105105
self._check_config_vm_managed_identity(config=config, credential=credential)
106106
self._check_config_vpc(config=config, credential=credential)
107+
self._check_config_network_security_groups(config)
107108

108109
def create_backend(
109110
self, project_name: str, config: AzureBackendConfigWithCreds
@@ -126,7 +127,7 @@ def create_backend(
126127
resource_group=config.resource_group,
127128
locations=config.regions,
128129
create_default_network=config.vpc_ids is None and config.subnet_ids is None,
129-
network_security_group_ids=config.network_security_group_ids,
130+
network_security_group_names=config.network_security_group_names,
130131
)
131132
return BackendRecord(
132133
config=AzureStoredConfig(
@@ -283,6 +284,27 @@ def _check_config_vpc(
283284
except BackendError as e:
284285
raise ServerClientError(e.args[0])
285286

287+
def _check_config_network_security_groups(self, config: AzureBackendConfigWithCreds):
288+
if not config.network_security_group_names:
289+
return
290+
# When `regions` is None, all regions are used and there is no feasible way to validate
291+
# location keys against the full list of Azure regions here, so the check is skipped.
292+
if config.regions is None:
293+
return
294+
configured_regions = set(config.regions)
295+
unknown_locations = sorted(
296+
location
297+
for location in config.network_security_group_names
298+
if location not in configured_regions
299+
)
300+
if unknown_locations:
301+
raise ServerClientError(
302+
f"Locations {unknown_locations} in `network_security_group_names` are not in"
303+
" `regions`. Otherwise, these network security groups would be silently ignored"
304+
" and instances in those locations would fall back to dstack's auto-created"
305+
" network security group."
306+
)
307+
286308
def _check_config_vm_managed_identity(
287309
self, config: AzureBackendConfigWithCreds, credential: auth.AzureCredential
288310
):
@@ -341,7 +363,7 @@ def _create_network_resources(
341363
resource_group: str,
342364
locations: List[str],
343365
create_default_network: bool,
344-
network_security_group_ids: Optional[Dict[str, str]] = None,
366+
network_security_group_names: Optional[Dict[str, str]] = None,
345367
):
346368
def func(location: str):
347369
network_manager = NetworkManager(
@@ -354,9 +376,9 @@ def func(location: str):
354376
name=azure_utils.get_default_network_name(resource_group, location),
355377
subnet_name=azure_utils.get_default_subnet_name(resource_group, location),
356378
)
357-
if location not in (network_security_group_ids or {}):
379+
if location not in (network_security_group_names or {}):
358380
# Skipped when the user supplies their own network security group for this
359-
# location via `network_security_group_ids` - dstack does not create or manage
381+
# location via `network_security_group_names` - dstack does not create or manage
360382
# it in that case.
361383
network_manager.create_network_security_group(
362384
resource_group=resource_group,

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,15 @@ class AzureBackendConfig(CoreModel):
8181
)
8282
),
8383
] = None
84-
network_security_group_ids: Annotated[
84+
network_security_group_names: Annotated[
8585
Optional[Dict[str, str]],
8686
Field(
8787
description=(
8888
"The mapping from Azure locations to the names of existing network security groups"
89-
" (in the configured resource group) to use for instances instead of the one `dstack`"
90-
" creates and manages automatically."
89+
" to use for instances instead of the one `dstack` creates and manages automatically."
90+
" The values are plain NSG names (not full Azure resource IDs) and must refer to"
91+
" network security groups within the configured `resource_group` — NSGs in a different"
92+
" resource group cannot be referenced."
9193
" Locations not present in this mapping fall back to dstack's auto-created"
9294
" network security group."
9395
" When set, `dstack` does not add, remove, or modify any rules on these network"

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ def run_job(
321321
ssh_keys=[SSHKey(public=project_ssh_public_key.strip())],
322322
volumes=volumes,
323323
reservation=job.job_spec.requirements.reservation,
324-
security_group=run.run_spec.merged_profile.security_group,
324+
security_group=job.job_spec.requirements.security_group,
325325
tags=run.run_spec.merged_profile.tags,
326326
)
327327
instance_offer = instance_offer.copy()

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,10 @@ def create_gateway(
561561
self,
562562
configuration: GatewayComputeConfiguration,
563563
) -> GatewayProvisioningData:
564-
if self.config.vpc_project_id is None and self.config.create_firewall_rules is not False:
564+
if self.config.vpc_project_id is None:
565+
# Gateway firewall rules are intentionally not gated by `create_firewall_rules`:
566+
# gateways are meant to be internet-reachable, and this keeps behavior consistent
567+
# with AWS/Azure/OCI, where gateway security groups/NSGs are always auto-managed.
565568
gcp_resources.create_gateway_firewall_rules(
566569
firewalls_client=self.firewalls_client,
567570
project_id=self.config.project_id,

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,12 @@ class GCPBackendConfig(CoreModel):
8484
Optional[bool],
8585
Field(
8686
description=(
87-
"A flag to enable/disable `dstack` creating VPC firewall rules that allow SSH"
88-
" (and, for gateways, HTTP/HTTPS) traffic from the internet."
87+
"A flag to enable/disable `dstack` creating a VPC firewall rule that allows SSH"
88+
" traffic from the internet to instances."
8989
" Set to `false` if you manage your own firewall rules and don't want `dstack`"
90-
" creating rules that open ports to `0.0.0.0/0`. Defaults to `true`"
90+
" creating a rule that opens port 22 to `0.0.0.0/0`. Defaults to `true`."
91+
" This does not affect the separate firewall rule `dstack` creates for gateways,"
92+
" which are meant to be internet-reachable and are always auto-managed"
9193
)
9294
),
9395
] = None

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ def create_instance(
143143
security_group_id = self.config.network_security_group_ids.get(
144144
instance_offer.region
145145
)
146-
if security_group_id is None:
146+
using_custom_security_group = security_group_id is not None
147+
if not using_custom_security_group:
147148
security_group = resources.get_or_create_security_group(
148149
f"dstack-{instance_config.project_name}-default-security-group",
149150
subnet.vcn_id,
@@ -154,10 +155,25 @@ def create_instance(
154155
security_group.id, region.virtual_network_client
155156
)
156157
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
157173

158174
cloud_init_user_data = get_user_data(
159175
authorized_keys=instance_config.get_public_keys(),
160-
firewall_allow_from_subnets=[resources.VCN_CIDR],
176+
firewall_allow_from_subnets=[firewall_allow_from_subnet],
161177
)
162178

163179
display_name = generate_unique_instance_name(instance_config)

0 commit comments

Comments
 (0)