Multi subscription discovery (#31)

* Updated the Create job to use the Subscription ID embedded in the Store Path rather than the default * Improved Exception logging
Keyfactor · Oct 11, 2023 · d3d5854 · d3d5854
1 parent a5abab5
commit d3d5854
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 61 deletions.
diff --git a/AzureKeyVault/AzureClient.cs b/AzureKeyVault/AzureClient.cs
@@ -27,12 +27,10 @@ namespace Keyfactor.Extensions.Orchestrator.AzureKeyVault
 {
     public class AzureClient
     {
-        internal protected virtual AkvProperties VaultProperties { get; set; }
-        private string ActiveTenantId { get; set; } // for searching across multiple tenants.
+        internal protected virtual AkvProperties VaultProperties { get; set; }        
 
         ILogger logger { get; set; }
 
-
         private protected virtual CertificateClient CertClient
         {
             get
@@ -112,45 +110,14 @@ internal protected virtual ArmClient KvManagementClient
                     return _mgmtClient;
                 }
                 return getArmClient(VaultProperties.TenantId);
-
-                // var subId = VaultProperties.SubscriptionId ?? VaultProperties.StorePath.Split("/")[2];
-                // var creds = SdkContext.AzureCredentialsFactory.FromServicePrincipal(VaultProperties.ApplicationId, VaultProperties.ClientSecret, VaultProperties.TenantId, AzureEnvironment.AzureGlobalCloud);
-                //NOTE: creating a certificate store from the platform is currently only supported for Azure GlobalCloud customers.
-
-                //TokenCredential credential;
-
-                //if (this.VaultProperties.UseAzureManagedIdentity)
-                //{
-                //    logger.LogTrace("getting management client for a managed identity");
-                //    var credentialOptions = new DefaultAzureCredentialOptions();
-
-
-                //    if (!string.IsNullOrEmpty(this.VaultProperties.ClientId)) // they have selected a managed identity and provided a client ID, so it is a user assigned identity
-                //    {
-                //        logger.LogTrace("It is a user assigned managed identity");
-                //        credentialOptions.ManagedIdentityClientId = VaultProperties.ClientId;
-                //    }
-                //    credential = new DefaultAzureCredential(credentialOptions);
-                //}
-                //else
-                //{
-                //    logger.LogTrace("getting credentials for a service principal identity");
-                //    credential = new ClientSecretCredential(ActiveTenantId, VaultProperties.ClientId, VaultProperties.ClientSecret);
-                //    logger.LogTrace("got credentials for service principal identity", credential);
-                //}
-
-                //_mgmtClient = new ArmClient(credential);
-                //logger.LogTrace("created management client", _mgmtClient);
-                //return _mgmtClient;
             }
         }
         protected virtual ArmClient _mgmtClient { get; set; }
 
         public AzureClient() { }
         public AzureClient(AkvProperties props)
         {
-            VaultProperties = props;
-            ActiveTenantId = props.TenantId;
+            VaultProperties = props;            
             logger = LogHandler.GetClassLogger<AzureClient>();
         }
 
@@ -166,7 +133,7 @@ public virtual async Task<KeyVaultResource> CreateVault()
             {
                 logger.LogInformation("Begin create vault...");
 
-                SubscriptionResource subscription = await KvManagementClient.GetDefaultSubscriptionAsync();
+                var subscription = KvManagementClient.GetSubscriptions().FirstOrDefault(s => s.Data.SubscriptionId == VaultProperties.SubscriptionId);                
                 ResourceGroupCollection resourceGroups = subscription.GetResourceGroups();
                 ResourceGroupResource resourceGroup = await resourceGroups.GetAsync(this.VaultProperties.ResourceGroupName);
 
@@ -184,7 +151,7 @@ public virtual async Task<KeyVaultResource> CreateVault()
             }
             catch (Exception ex)
             {
-                logger.LogError("Error when trying to create Azure Keyvault", ex);
+                logger.LogError(ex, "Error when trying to create Azure Keyvault");
                 throw;
             }
 
@@ -207,21 +174,22 @@ public virtual async Task<KeyVaultCertificateWithPolicy> ImportCertificateAsync(
                 var bytes = Convert.FromBase64String(contents);
                 var x509 = new X509Certificate2(bytes, pfxPassword, X509KeyStorageFlags.Exportable);
                 var certWithKey = x509.Export(X509ContentType.Pkcs12);
-                logger.LogTrace($"importing created x509 certificate named {1}", certName);
+                logger.LogTrace($"importing created x509 certificate named {certName}");
                 var cert = await CertClient.ImportCertificateAsync(new ImportCertificateOptions(certName, certWithKey));
                 return cert;
             }
             catch (Exception ex)
             {
-                logger.LogError(ex.Message);
+                logger.LogError(ex, "Failed to add certificate to Keyvault.");
                 throw;
             }
         }
 
         public virtual async Task<IEnumerable<CurrentInventoryItem>> GetCertificatesAsync()
         {
             List<CurrentInventoryItem> inventoryItems = new List<CurrentInventoryItem>();
-            Pageable<CertificateProperties> inventory = null;
+            Pageable<CertificateProperties> inventory;
+
             try
             {
                 logger.LogTrace("calling GetPropertiesOfCertificates() on the Certificate Client", CertClient);
@@ -231,7 +199,7 @@ public virtual async Task<IEnumerable<CurrentInventoryItem>> GetCertificatesAsyn
             }
             catch (Exception ex)
             {
-                logger.LogError("Error performing inventory", ex);
+                logger.LogError(ex, "Error performing inventory");
                 throw;
             }
 
@@ -286,7 +254,7 @@ public virtual List<string> GetVaults()
             }
             catch (Exception ex)
             {
-                logger.LogError("Error getting vaults.", ex);
+                logger.LogError(ex, "Error getting vaults.");
                 throw;
             }
         }

diff --git a/AzureKeyVault/Jobs/AzureKeyVaultJob.cs b/AzureKeyVault/Jobs/AzureKeyVaultJob.cs
@@ -31,14 +31,16 @@ public void InitializeStore(dynamic config)
             if (config.GetType().GetProperty("CertificateStoreDetails") != null) // anything except a discovery job
             {
                 VaultProperties.StorePath = config.CertificateStoreDetails?.StorePath;
+
                 dynamic properties = JsonConvert.DeserializeObject(config.CertificateStoreDetails.Properties.ToString());
-                VaultProperties.TenantId = properties.TenantId != null ? properties.TenantId : VaultProperties.TenantId;
-                VaultProperties.TenantId = VaultProperties.TenantId != null ? VaultProperties.TenantId : properties.dirs;
+                VaultProperties.TenantId = config.CertificateStoreDetails?.ClientMachine ?? properties.tenantId;
                 VaultProperties.ResourceGroupName = properties.ResourceGroupName;
                 VaultProperties.VaultName = properties.VaultName;
                 VaultProperties.PremiumSKU = properties.SkuType == "premium";
                 VaultProperties.VaultRegion = properties.VaultRegion ?? "eastus";
                 VaultProperties.VaultRegion = VaultProperties.VaultRegion.ToLower();
+
+                VaultProperties.SubscriptionId = VaultProperties.StorePath.Split('/', System.StringSplitOptions.RemoveEmptyEntries)[1];
             }
             else // discovery job
             {

diff --git a/AzureKeyVault/Jobs/Inventory.cs b/AzureKeyVault/Jobs/Inventory.cs
@@ -6,7 +6,6 @@
 // and limitations under the License.
 
 using System;
-using System.Collections.Generic;
 using System.Linq;
 using Keyfactor.Logging;
 using Keyfactor.Orchestrators.Common.Enums;
@@ -24,18 +23,23 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
             logger.LogDebug($"Begin Inventory...");
 
             InitializeStore(config);
-
-            List<CurrentInventoryItem> inventoryItems = new List<CurrentInventoryItem>();
-
+
             try
             {
-                logger.LogDebug($"Making Request for {0}...", VaultProperties.VaultURL);
+                logger.LogTrace($"Making Request for {0}...", VaultProperties.VaultURL);
+                var inventoryItems = AzClient.GetCertificatesAsync().Result?.ToList();
+                logger.LogTrace($"Found {inventoryItems.Count()} Total Certificates in Azure Key Vault.");
 
-                inventoryItems = AzClient.GetCertificatesAsync().Result?.ToList();
+                // return to orchestrator framework for response to platform
+                callBack.DynamicInvoke(inventoryItems);
 
-                logger.LogDebug($"Found {inventoryItems.Count()} Total Certificates in Azure Key Vault.");
+                return new JobResult
+                {
+                    Result = OrchestratorJobStatusJobResult.Success,
+                    JobHistoryId = config.JobHistoryId,
+                    FailureMessage = string.Empty
+                };
             }
-
             catch (Exception ex)
             {
                 return new JobResult
@@ -46,15 +50,6 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
                 };
             }
 
-            // upload to CMS
-            callBack.DynamicInvoke(inventoryItems);
-
-            return new JobResult
-            {
-                Result = OrchestratorJobStatusJobResult.Success,
-                JobHistoryId = config.JobHistoryId,
-                FailureMessage = string.Empty
-            };
         }
     }
 }

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+- 2.1.1
+  - Updated the Create job to use the Subscription ID embedded in the Store Path rather than the default
+  - Improved Exception logging
+
 - 2.1
   - Updated the Discovery job to support multiple tenants and all accessible subscriptions they contain
   - Added more detailed trace logging during the discovery process