Repository: skypilot-org/skypilot
Pull request: Upload all cloud credentials to sky cluster regardless of sky check
<!-- Describe the changes in this PR -->
Resolve https://github.com/skypilot-org/skypilot/issues/3977


<!-- Describe the tests ran -->
<!-- Unit tests (tests/test_*.py) are part of GitHub CI; below are tests that launch on the cloud. -->

Tested (run the relevant ones):

- [x] Code formatting: `bash format.sh`
- [x] Disable all clouds besides 1 non-gcp cloud locally, sky launch on that non-gcp cloud and run a gcloud bucket access script
- [x] Smoke tests: `pytest tests/test_smoke.py:test_minimal` 


Problem statement:
[Core] Upload cloud credentials for bucket access even when cloud is disabled in SkyPilot
<!-- Describe the bug report / feature request here -->

A user reported that their gcp credential may not have the full permission to let `sky check` enable the cloud, but when they launch clusters in kubernetes, they still want the credential to be uploaded for the access to GCS bucket.


<!-- If relevant, fill in versioning info to help us troubleshoot -->
_Version & Commit info:_
* `sky -v`: PLEASE_FILL_IN
* `sky -c`: PLEASE_FILL_IN

Hints from issue discussion:

Commit message: Upload all cloud credentials to sky cluster regardless of sky check

Patch to review:
diff --git a/sky/check.py b/sky/check.py
index 9ac2848733c..1854d6383f5 100644
--- a/sky/check.py
+++ b/sky/check.py
@@ -1,4 +1,5 @@
 """Credential checks: check cloud credentials and enable clouds."""
+import os
 import traceback
 from types import ModuleType
 from typing import Dict, Iterable, List, Optional, Tuple, Union
@@ -194,19 +195,25 @@ def get_cached_enabled_clouds_or_refresh(
 def get_cloud_credential_file_mounts(
         excluded_clouds: Optional[Iterable[sky_clouds.Cloud]]
 ) -> Dict[str, str]:
-    """Returns the files necessary to access all enabled clouds.
+    """Returns the files necessary to access all clouds.
 
     Returns a dictionary that will be added to a task's file mounts
     and a list of patterns that will be excluded (used as rsync_exclude).
     """
-    enabled_clouds = get_cached_enabled_clouds_or_refresh()
+    # Uploading credentials for all clouds instead of only sky check
+    # enabled clouds because users may have partial credentials for some
+    # clouds to access their specific resources (e.g. cloud storage) but
+    # not have the complete credentials to pass sky check.
+    clouds = sky_clouds.CLOUD_REGISTRY.values()
     file_mounts = {}
-    for cloud in enabled_clouds:
+    for cloud in clouds:
         if (excluded_clouds is not None and
                 sky_clouds.cloud_in_iterable(cloud, excluded_clouds)):
             continue
         cloud_file_mounts = cloud.get_credential_file_mounts()
-        file_mounts.update(cloud_file_mounts)
+        for path in cloud_file_mounts.values():
+            if os.path.exists(os.path.expanduser(path)):
+                file_mounts.update(cloud_file_mounts)
     # Currently, get_cached_enabled_clouds_or_refresh() does not support r2 as
     # only clouds with computing instances are marked as enabled by skypilot.
     # This will be removed when cloudflare/r2 is added as a 'cloud'.
diff --git a/sky/clouds/oci.py b/sky/clouds/oci.py
index 810e43fe3b5..c6451a73a1f 100644
--- a/sky/clouds/oci.py
+++ b/sky/clouds/oci.py
@@ -468,7 +468,7 @@ def get_credential_file_mounts(self) -> Dict[str, str]:
             api_key_file = oci_cfg[
                 'key_file'] if 'key_file' in oci_cfg else 'BadConf'
             sky_cfg_file = oci_utils.oci_config.get_sky_user_config_file()
-        except ImportError:
+        except (ImportError, oci_adaptor.oci.exceptions.ConfigFileNotFound):
             return {}
 
         # OCI config and API key file are mandatory


Code review comments:
diff_hunk: @@ -194,19 +195,25 @@ def get_cached_enabled_clouds_or_refresh(
 def get_cloud_credential_file_mounts(
         excluded_clouds: Optional[Iterable[sky_clouds.Cloud]]
 ) -> Dict[str, str]:
-    """Returns the files necessary to access all enabled clouds.
+    """Returns the files necessary to access all clouds.
 
     Returns a dictionary that will be added to a task's file mounts
     and a list of patterns that will be excluded (used as rsync_exclude).
     """
-    enabled_clouds = get_cached_enabled_clouds_or_refresh()
+    # Uploading credentials for all clouds instead of only sky check
+    # enabled clouds because users may have partial credentials for some
+    # clouds to access their specific resources (e.g. cloud storage) but
+    # not have the complete credentials to pass sky check.
+    clouds = sky_clouds.CLOUD_REGISTRY.values()
     file_mounts = {}
-    for cloud in enabled_clouds:
+    for cloud in clouds:
         if (excluded_clouds is not None and
                 sky_clouds.cloud_in_iterable(cloud, excluded_clouds)):
             continue
         cloud_file_mounts = cloud.get_credential_file_mounts()
-        file_mounts.update(cloud_file_mounts)
+        for path in cloud_file_mounts.values():
+            if os.path.exists(os.path.expanduser(path)):
+                file_mounts.update(cloud_file_mounts)
path: sky/check.py
text: @user1:
Is this a typo? Why do we update the entire file when one of the path exists?