Remove uses of CLRJIT_AZ_KEY/clrjit_key1 from SPMI (dotnet#104164)

vcsjones · Jul 4, 2024 · d198728 · d198728
1 parent 13d2bab
commit d198728
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 53 deletions.
diff --git a/eng/pipelines/common/templates/runtimes/run-test-job.yml b/eng/pipelines/common/templates/runtimes/run-test-job.yml
@@ -615,15 +615,19 @@ jobs:
         condition: always()
 
       # Ensure the Python azure-storage-blob package is installed before doing the upload.
-      - script: $(PipScript) install --upgrade pip && $(PipScript) install azure.storage.blob==12.5.0 --force-reinstall
-        displayName: Upgrade Pip to latest and install azure-storage-blob Python package
+      - script: $(PipScript) install --upgrade pip && $(PipScript) install azure.storage.blob==12.5.0 --force-reinstall && $(PipScript) install azure.identity==1.16.1 --force-reinstall
+        displayName: Upgrade Pip to latest and install azure-storage-blob and azure-identity Python packages
         condition: always()
 
-      - script: $(PythonScript) $(Build.SourcesDirectory)/src/coreclr/scripts/superpmi.py upload -log_level DEBUG -arch $(archType) -build_type $(buildConfig) -mch_files $(MergedMchFileLocation)$(CollectionName).$(CollectionType).$(MchFileTag).mch -core_root $(Build.SourcesDirectory)/artifacts/bin/coreclr/$(osGroup).x64.$(buildConfigUpper)
+      - task: AzureCLI@2
         displayName: 'Upload SuperPMI $(CollectionName)-$(CollectionType) collection to Azure Storage'
+        inputs:
+          azureSubscription: 'superpmi-collect-rw'
+          scriptType: 'pscore'
+          scriptLocation: 'inlineScript'
+          inlineScript: |
+            $(PythonScript) $(Build.SourcesDirectory)/src/coreclr/scripts/superpmi.py upload -log_level DEBUG -arch $(archType) -build_type $(buildConfig) -mch_files $(MergedMchFileLocation)$(CollectionName).$(CollectionType).$(MchFileTag).mch -core_root $(Build.SourcesDirectory)/artifacts/bin/coreclr/$(osGroup).x64.$(buildConfigUpper)
         condition: always()
-        env:
-          CLRJIT_AZ_KEY: $(clrjit_key1) # secret key stored as variable in pipeline
 
       - task: CopyFiles@2
         displayName: Copying superpmi.log of all partitions

diff --git a/eng/pipelines/coreclr/templates/run-superpmi-collect-job.yml b/eng/pipelines/coreclr/templates/run-superpmi-collect-job.yml
@@ -177,13 +177,17 @@ jobs:
         onlyAddExtraIndex: false
 
     # Ensure the Python azure-storage-blob package is installed before doing the upload.
-    - script: $(PipScript) install --upgrade pip && $(PipScript) install azure.storage.blob==12.5.0 --force-reinstall
-      displayName: Upgrade Pip to latest and install azure-storage-blob Python package
+    - script: $(PipScript) install --upgrade pip && $(PipScript) install azure.storage.blob==12.5.0 --force-reinstall && $(PipScript) install azure.identity==1.16.1 --force-reinstall
+      displayName: Upgrade Pip to latest and install azure-storage-blob and azure-identity Python packages
 
-    - script: $(PythonScript) $(Build.SourcesDirectory)/src/coreclr/scripts/superpmi.py upload -log_level DEBUG -arch $(archType) -build_type $(buildConfig) -mch_files $(MergedMchFileLocation)$(CollectionName).$(CollectionType).$(MchFileTag).mch -core_root $(Build.SourcesDirectory)/artifacts/bin/coreclr/$(osGroup).x64.$(buildConfigUpper)
+    - task: AzureCLI@2
       displayName: ${{ format('Upload SuperPMI {0}-{1} collection to Azure Storage', parameters.collectionName, parameters.collectionType) }}
-      env:
-        CLRJIT_AZ_KEY: $(clrjit_key1) # secret key stored as variable in pipeline
+      inputs:
+        azureSubscription: 'superpmi-collect-rw'
+        scriptType: 'pscore'
+        scriptLocation: 'inlineScript'
+        inlineScript: |
+          $(PythonScript) $(Build.SourcesDirectory)/src/coreclr/scripts/superpmi.py upload -log_level DEBUG -arch $(archType) -build_type $(buildConfig) -mch_files $(MergedMchFileLocation)$(CollectionName).$(CollectionType).$(MchFileTag).mch -core_root $(Build.SourcesDirectory)/artifacts/bin/coreclr/$(osGroup).x64.$(buildConfigUpper)
 
     # Always upload the available logs for diagnostics
     - task: CopyFiles@2

diff --git a/eng/pipelines/coreclr/templates/upload-jits-steps.yml b/eng/pipelines/coreclr/templates/upload-jits-steps.yml
@@ -10,10 +10,14 @@ steps:
   displayName: Enable python venv
 
 # Ensure the Python azure-storage-blob package is installed before doing the upload.
-- script: $(PipScript) install --upgrade pip && $(PipScript) install azure.storage.blob==12.5.0 --force-reinstall
-  displayName: Upgrade Pip to latest and install azure-storage-blob Python package
+- script: $(PipScript) install --upgrade pip && $(PipScript) install azure.storage.blob==12.5.0 --force-reinstall && $(PipScript) install azure.identity==1.16.1 --force-reinstall
+  displayName: Upgrade Pip to latest and install azure-storage-blob and azure-identity Python packages
 
-- script: $(PythonScript) $(Build.SourcesDirectory)/src/coreclr/scripts/jitrollingbuild.py upload -build_type $(_BuildConfig) -arch $(archType) -host_os $(osGroup) -git_hash $(Build.SourceVersion) --use_latest_jit_change
-  displayName: Upload JIT to Azure Storage
-  env:
-    CLRJIT_AZ_KEY: $(clrjit_key1) # secret key stored as variable in pipeline
+- task: AzureCLI@2
+  displayName: 'Upload JIT to Azure Storage'
+  inputs:
+    azureSubscription: 'superpmi-collect-rw'
+    scriptType: 'pscore'
+    scriptLocation: 'inlineScript'
+    inlineScript: |
+      $(PythonScript) $(Build.SourcesDirectory)/src/coreclr/scripts/jitrollingbuild.py upload -build_type $(_BuildConfig) -arch $(archType) -host_os $(osGroup) -git_hash $(Build.SourceVersion) --use_latest_jit_change
diff --git a/eng/pipelines/libraries/superpmi-postprocess-step.yml b/eng/pipelines/libraries/superpmi-postprocess-step.yml
@@ -66,15 +66,19 @@ steps:
     condition: always()
 
   # Ensure the Python azure-storage-blob package is installed before doing the upload.
-  - script: ${{ parameters.PipScript }} install --upgrade pip && ${{ parameters.PipScript }} install azure.storage.blob==12.5.0 --force-reinstall
-    displayName: Upgrade Pip to latest and install azure-storage-blob Python package
+  - script: ${{ parameters.PipScript }} install --upgrade pip && ${{ parameters.PipScript }} install azure.storage.blob==12.5.0 --force-reinstall  && ${{ parameters.PipScript }} install azure.identity==1.16.1 --force-reinstall
+    displayName: Upgrade Pip to latest and install azure-storage-blob and azure-identity Python packages
     condition: always()
 
-  - script: ${{ parameters.PythonScript }} $(Build.SourcesDirectory)/src/coreclr/scripts/superpmi.py upload -log_level DEBUG -arch ${{ parameters.archType }} -build_type ${{ parameters.buildConfig }} -mch_files ${{ parameters.MergedMchFileLocation }}${{ parameters.SuperPmiCollectionName }}.${{ parameters.SuperPmiCollectionType }}.${{ parameters.osGroup }}.${{ parameters.archType }}.${{ parameters.buildConfig }}.mch -core_root $(Build.SourcesDirectory)/artifacts/bin/coreclr/${{ parameters.osGroup }}.x64.${{ parameters.buildConfigUpper }}
+  - task: AzureCLI@2
     displayName: 'Upload SuperPMI ${{ parameters.SuperPmiCollectionName }}-${{ parameters.SuperPmiCollectionType }} collection to Azure Storage'
+    inputs:
+      azureSubscription: 'superpmi-collect-rw'
+      scriptType: 'pscore'
+      scriptLocation: 'inlineScript'
+      inlineScript: |
+        ${{ parameters.PythonScript }} $(Build.SourcesDirectory)/src/coreclr/scripts/superpmi.py upload -log_level DEBUG -arch ${{ parameters.archType }} -build_type ${{ parameters.buildConfig }} -mch_files ${{ parameters.MergedMchFileLocation }}${{ parameters.SuperPmiCollectionName }}.${{ parameters.SuperPmiCollectionType }}.${{ parameters.osGroup }}.${{ parameters.archType }}.${{ parameters.buildConfig }}.mch -core_root $(Build.SourcesDirectory)/artifacts/bin/coreclr/${{ parameters.osGroup }}.x64.${{ parameters.buildConfigUpper }}
     condition: always()
-    env:
-      CLRJIT_AZ_KEY: $(clrjit_key1) # secret key stored as variable in pipeline
 
   - task: CopyFiles@2
     displayName: Copying superpmi.log of all partitions

diff --git a/src/coreclr/scripts/jitrollingbuild.py b/src/coreclr/scripts/jitrollingbuild.py
@@ -108,7 +108,6 @@
 
 upload_parser.add_argument("-git_hash", required=True, help=git_hash_help)
 upload_parser.add_argument("--use_latest_jit_change", action="store_true", help=use_latest_jit_change_help)
-upload_parser.add_argument("-az_storage_key", help="Key for the clrjit Azure Storage location. Default: use the value of the CLRJIT_AZ_KEY environment variable.")
 upload_parser.add_argument("--skip_cleanup", action="store_true", help=skip_cleanup_help)
 
 # subparser for download
@@ -458,14 +457,18 @@ def upload_blob(file, blob_name):
 
     try:
         from azure.storage.blob import BlobServiceClient
+        from azure.identity import DefaultAzureCredential
 
     except:
         logging.warning("Please install:")
         logging.warning("  pip install azure-storage-blob")
+        logging.warning("  pip install azure-identiy")
         logging.warning("See also https://learn.microsoft.com/azure/storage/blobs/storage-quickstart-blobs-python")
-        raise RuntimeError("Missing azure storage package.")
+        raise RuntimeError("Missing azure storage or identity packages.")
 
-    blob_service_client = BlobServiceClient(account_url=az_blob_storage_account_uri, credential=coreclr_args.az_storage_key)
+    default_credential = DefaultAzureCredential()
+
+    blob_service_client = BlobServiceClient(account_url=az_blob_storage_account_uri, credential=default_credential)
     blob_folder_name = "{}/{}/{}/{}/{}".format(az_builds_root_folder, jit_git_hash, coreclr_args.host_os, coreclr_args.arch, coreclr_args.build_type)
 
     total_bytes_uploaded = 0
@@ -623,12 +626,6 @@ def setup_spmi_location_arg(spmi_location):
                             lambda unused: True,
                             "Unable to set use_latest_jit_change")
 
-        coreclr_args.verify(args,
-                            "az_storage_key",
-                            lambda item: item is not None,
-                            "Specify az_storage_key or set environment variable CLRJIT_AZ_KEY to the key to use.",
-                            modify_arg=lambda arg: os.environ["CLRJIT_AZ_KEY"] if arg is None and "CLRJIT_AZ_KEY" in os.environ else arg)
-
         coreclr_args.verify(args,
                             "skip_cleanup",
                             lambda unused: True,

diff --git a/src/coreclr/scripts/jitutil.py b/src/coreclr/scripts/jitutil.py
@@ -537,13 +537,13 @@ def require_azure_storage_libraries(need_azure_storage_blob=True, need_azure_ide
         Once we've done it once, we don't do it again.
 
         For this to work for cross-module usage, after you call this function, you need to add a line like:
-            from jitutil import BlobClient, AzureCliCredential
+            from jitutil import BlobClient, DefaultAzureCredential
         naming all the types you want to use.
 
         The full set of types this function loads:
-            BlobServiceClient, BlobClient, ContainerClient, AzureCliCredential
+            BlobServiceClient, BlobClient, ContainerClient, DefaultAzureCredential
     """
-    global azure_storage_libraries_check, BlobServiceClient, BlobClient, ContainerClient, AzureCliCredential
+    global azure_storage_libraries_check, BlobServiceClient, BlobClient, ContainerClient, DefaultAzureCredential
 
     if azure_storage_libraries_check:
         return
@@ -560,7 +560,7 @@ def require_azure_storage_libraries(need_azure_storage_blob=True, need_azure_ide
     azure_identity_import_ok = True
     if need_azure_identity:
         try:
-            from azure.identity import AzureCliCredential
+            from azure.identity import DefaultAzureCredential
         except:
             azure_identity_import_ok = False
 
@@ -608,7 +608,7 @@ def download_with_azure(uri, target_location, fail_if_not_found=True):
     logging.info("Download: %s -> %s", uri, target_location)
 
     ok = True
-    az_credential = AzureCliCredential()
+    az_credential = DefaultAzureCredential()
     blob = BlobClient.from_blob_url(uri, credential=az_credential)
     with open(target_location, "wb") as my_blob:
         try:

diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py
@@ -364,7 +364,6 @@ def add_core_root_arguments(parser, build_type_default, build_type_help):
 upload_parser = subparsers.add_parser("upload", description=upload_description, parents=[core_root_parser, target_parser])
 
 upload_parser.add_argument("-mch_files", metavar="MCH_FILE", required=True, nargs='+', help=upload_mch_files_help)
-upload_parser.add_argument("-az_storage_key", help="Key for the clrjit Azure Storage location. Default: use the value of the CLRJIT_AZ_KEY environment variable.")
 upload_parser.add_argument("-jit_ee_version", help=jit_ee_version_help)
 upload_parser.add_argument("--skip_cleanup", action="store_true", help=skip_cleanup_help)
 
@@ -3563,14 +3562,14 @@ def list_superpmi_collections_container_via_azure_api(path_filter=lambda unused:
     """
 
     require_azure_storage_libraries()
-    from jitutil import ContainerClient, AzureCliCredential
+    from jitutil import ContainerClient, DefaultAzureCredential
 
     superpmi_container_url = az_blob_storage_superpmi_container_uri
 
     paths = []
     ok = True
     try:
-        az_credential = AzureCliCredential()
+        az_credential = DefaultAzureCredential()
         container = ContainerClient.from_container_url(superpmi_container_url, credential=az_credential)
         blob_name_prefix = az_collections_root_folder + "/"
         blob_list = container.list_blobs(name_starts_with=blob_name_prefix, retry_total=0)
@@ -3789,8 +3788,8 @@ def upload_mch(coreclr_args):
         coreclr_args (CoreclrArguments): parsed args
     """
 
-    require_azure_storage_libraries(need_azure_identity=False)
-    from jitutil import BlobServiceClient
+    require_azure_storage_libraries(need_azure_identity=True)
+    from jitutil import BlobServiceClient, DefaultAzureCredential
 
     def upload_blob(file, blob_name):
         blob_client = blob_service_client.get_blob_client(container=az_superpmi_container_name, blob=blob_name)
@@ -3826,7 +3825,9 @@ def upload_blob(file, blob_name):
     for item in files_to_upload:
         logging.info("  %s", item)
 
-    blob_service_client = BlobServiceClient(account_url=az_blob_storage_account_uri, credential=coreclr_args.az_storage_key)
+    default_credential = DefaultAzureCredential()
+
+    blob_service_client = BlobServiceClient(account_url=az_blob_storage_account_uri, credential=default_credential)
     blob_folder_name = "{}/{}/{}/{}".format(az_collections_root_folder, coreclr_args.jit_ee_version, coreclr_args.target_os, coreclr_args.mch_arch)
 
     total_bytes_uploaded = 0
@@ -5005,12 +5006,6 @@ def verify_base_diff_args():
         verify_target_args()
         verify_jit_ee_version_arg()
 
-        coreclr_args.verify(args,
-                            "az_storage_key",
-                            lambda item: item is not None,
-                            "Specify az_storage_key or set environment variable CLRJIT_AZ_KEY to the key to use.",
-                            modify_arg=lambda arg: os.environ["CLRJIT_AZ_KEY"] if arg is None and "CLRJIT_AZ_KEY" in os.environ else arg)
-
         coreclr_args.verify(args,
                             "mch_files",
                             lambda unused: True,
@@ -5035,12 +5030,6 @@ def verify_base_diff_args():
             print("Error: private store directory '" + coreclr_args.private_store + "' not found.")
             sys.exit(1)
 
-        # Safety measure: don't allow CLRJIT_AZ_KEY to be set if we are uploading to a private store.
-        # Note that this should be safe anyway, since we're publishing something private, not public.
-        if "CLRJIT_AZ_KEY" in os.environ:
-            print("Error: environment variable CLRJIT_AZ_KEY is set, but command is `upload-private`, not `upload`. That is not allowed.")
-            sys.exit(1)
-
     elif coreclr_args.mode == "download":
 
         verify_target_args()