allow specifying a VPC from another GCP project (#5143)

cg505 · Michaelvll · web-flow · commit f520059ae2e1 · 2025-04-08T15:46:16.000-07:00
* allow specicifying a VPC from another GCP project

* Update docs/source/reference/config.rst

Co-authored-by: Zhanghao Wu &lt;zhanghao.wu@outlook.com&gt;

* add to jsonschema

* add test

---------

Co-authored-by: Zhanghao Wu &lt;zhanghao.wu@outlook.com&gt;
diff --git a/docs/source/reference/config.rst b/docs/source/reference/config.rst
@@ -621,15 +621,30 @@ rules for SkyPilot to function. If any VPC satisfies these rules, it is
 used. Otherwise, a new VPC named ``skypilot-vpc`` is automatically created
 with the minimal recommended firewall rules and will be used.
 
-If this field is set, SkyPilot will use the VPC with this name. Useful for
-when users want to manually set up a VPC and precisely control its
+If this field is set, SkyPilot will use the VPC with this name. The VPC must
+have the :ref:`necessary firewall rules <gcp-minimum-firewall-rules>`. Useful
+for when users want to manually set up a VPC and precisely control its
 firewall rules. If no region restrictions are given, SkyPilot only
 provisions in regions for which a subnet of this VPC exists. Errors are
 thrown if VPC with this name is not found. The VPC does not get modified
 in any way, except when opening ports (e.g., via ``resources.ports``) in
 which case new firewall rules permitting public traffic to those ports
 will be added.
 
+By default, only VPCs from the current project are used.
+
+.. code-block:: yaml
+
+  gcp:
+    vpc-name: my-vpc
+
+To use a shared VPC from another GCP project, specify the name as ``<project ID>/<vpc name>``. For example:
+
+.. code-block:: yaml
+
+  gcp:
+    vpc-name: my-project-123456/default
+
 .. _config-yaml-gcp-use-internal-ips:
 
 ``gcp.use_internal_ips``
diff --git a/sky/provision/gcp/config.py b/sky/provision/gcp/config.py
@@ -571,35 +571,45 @@ def get_usable_vpc_and_subnet(
 
     specific_vpc_to_use = config.provider_config.get('vpc_name', None)
     if specific_vpc_to_use is not None:
+        if '/' in specific_vpc_to_use:
+            # VPC can also be specified in the format PROJECT_ID/VPC_NAME.
+            # This enables use of shared VPCs.
+            split_vpc_value = specific_vpc_to_use.split('/')
+            if len(split_vpc_value) != 2:
+                raise ValueError(f'Invalid VPC name: {specific_vpc_to_use}. '
+                                 'Please specify the VPC name in the format '
+                                 'PROJECT_ID/VPC_NAME.')
+            project_id = split_vpc_value[0]
+            specific_vpc_to_use = split_vpc_value[1]
+
         vpcnets_all = _list_vpcnets(project_id,
                                     compute,
                                     filter=f'name={specific_vpc_to_use}')
-        # On GCP, VPC names are unique, so it'd be 0 or 1 VPC found.
-        assert (len(vpcnets_all) <=
-                1), (f'{len(vpcnets_all)} VPCs found with the same name '
-                     f'{specific_vpc_to_use}')
-        if len(vpcnets_all) == 1:
-            # Skip checking any firewall rules if the user has specified a VPC.
-            logger.info(f'Using user-specified VPC {specific_vpc_to_use!r}.')
-            subnets = _list_subnets(project_id,
-                                    region,
-                                    compute,
-                                    network=specific_vpc_to_use)
-            if not subnets:
-                _skypilot_log_error_and_exit_for_failover(
-                    'SUBNET_NOT_FOUND_FOR_VPC',
-                    f'No subnet for region {region} found for specified VPC '
-                    f'{specific_vpc_to_use!r}. '
-                    f'Check the subnets of VPC {specific_vpc_to_use!r} at '
-                    'https://console.cloud.google.com/networking/networks')
-            return specific_vpc_to_use, subnets[0]
-        else:
+        if not vpcnets_all:
             # VPC with this name not found. Error out and let SkyPilot failover.
             _skypilot_log_error_and_exit_for_failover(
                 'VPC_NOT_FOUND',
                 f'No VPC with name {specific_vpc_to_use!r} is found. '
                 'To fix: specify a correct VPC name.')
             # Should not reach here.
+            assert False
+
+        # On GCP, VPC names are unique within a project.
+        assert len(vpcnets_all) == 1, (vpcnets_all, specific_vpc_to_use)
+        # Skip checking any firewall rules if the user has specified a VPC.
+        logger.info(f'Using user-specified VPC {specific_vpc_to_use!r}.')
+        subnets = _list_subnets(project_id,
+                                region,
+                                compute,
+                                network=specific_vpc_to_use)
+        if not subnets:
+            _skypilot_log_error_and_exit_for_failover(
+                'SUBNET_NOT_FOUND_FOR_VPC',
+                f'No subnet for region {region} found for specified VPC '
+                f'{specific_vpc_to_use!r}. '
+                f'Check the subnets of VPC {specific_vpc_to_use!r} at '
+                'https://console.cloud.google.com/networking/networks')
+        return specific_vpc_to_use, subnets[0]
 
     subnets_all = _list_subnets(project_id, region, compute)
 
diff --git a/sky/utils/schemas.py b/sky/utils/schemas.py
@@ -604,13 +604,6 @@ def get_cluster_schema():
 
 
 _NETWORK_CONFIG_SCHEMA = {
-    'vpc_name': {
-        'oneOf': [{
-            'type': 'string',
-        }, {
-            'type': 'null',
-        }],
-    },
     'use_internal_ips': {
         'type': 'boolean',
     },
@@ -767,6 +760,13 @@ def get_config_schema():
                 },
                 'security_group_name':
                     (_PRORPERTY_NAME_OR_CLUSTER_NAME_TO_PROPERTY),
+                'vpc_name': {
+                    'oneOf': [{
+                        'type': 'string',
+                    }, {
+                        'type': 'null',
+                    }],
+                },
                 **_LABELS_SCHEMA,
                 **_NETWORK_CONFIG_SCHEMA,
             },
@@ -805,6 +805,19 @@ def get_config_schema():
                 'enable_gvnic': {
                     'type': 'boolean'
                 },
+                'vpc_name': {
+                    'oneOf': [
+                        {
+                            'type': 'string',
+                            # vpc-name or project-id/vpc-name
+                            # VPC name and Project ID have -, a-z, and 0-9.
+                            'pattern': '^(?:[-a-z0-9]+/)?[-a-z0-9]+$'
+                        },
+                        {
+                            'type': 'null',
+                        }
+                    ],
+                },
                 **_LABELS_SCHEMA,
                 **_NETWORK_CONFIG_SCHEMA,
             },
diff --git a/tests/test_config.py b/tests/test_config.py
@@ -237,6 +237,43 @@ def test_invalid_enum_config(monkeypatch, tmp_path) -> None:
     assert 'Invalid config YAML' in e.value.args[0]
 
 
+def test_gcp_vpc_name_validation(monkeypatch, tmp_path) -> None:
+    """Test GCP vpc_name validation with valid and invalid pattern.
+
+    This tests the schema changes where vpc_name was moved from _NETWORK_CONFIG_SCHEMA
+    to provider-specific schemas and pattern validation was added for GCP vpc_name.
+    """
+    # Test valid vpc_name format
+    for valid_vpc in ['my-vpc', 'project-id/my-vpc', 'project-123/vpc-456']:
+        config_path = tmp_path / f'valid_{valid_vpc.replace("/", "_")}.yaml'
+        config_path.open('w', encoding='utf-8').write(
+            textwrap.dedent(f"""\
+            gcp:
+                vpc_name: {valid_vpc}
+            """))
+        monkeypatch.setattr(skypilot_config, 'CONFIG_PATH', config_path)
+        # Should not raise an exception
+        skypilot_config._reload_config()
+        assert skypilot_config.get_nested(('gcp', 'vpc_name'),
+                                          None) == valid_vpc
+
+    # Test invalid vpc_name format with multiple slashes
+    for invalid_vpc in [
+            'UPPERCASE-VPC', 'project_id/my-vpc', 'invalid/path/format',
+            '/missing-project', 'project-id/', 'project/vpc/extra'
+    ]:
+        config_path = tmp_path / f'invalid_{invalid_vpc.replace("/", "_")}.yaml'
+        config_path.open('w', encoding='utf-8').write(
+            textwrap.dedent(f"""\
+            gcp:
+                vpc_name: {invalid_vpc}
+            """))
+        monkeypatch.setattr(skypilot_config, 'CONFIG_PATH', config_path)
+        with pytest.raises(ValueError) as e:
+            skypilot_config._reload_config()
+        assert 'Invalid config YAML' in e.value.args[0]
+
+
 def test_valid_num_items_config(monkeypatch, tmp_path) -> None:
     """Test that the config is not loaded if the config file contains an invalid number of array items."""
     config_path = tmp_path / 'valid.yaml'
