Skip to content

Commit be5d10c

Browse files
m8rmclarenm8rmclaren
committed
chore(client): Aggregate errors that get passed back to Command and return Warning instead of Error if partial error occurs in Inventory
1 parent 1f2c647 commit be5d10c

File tree

7 files changed

+99
-22
lines changed

7 files changed

+99
-22
lines changed

‎AzureAppGatewayOrchestrator.Tests/AzureAppGatewayOrchestrator_AzureAppGW.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
using System.Security.Cryptography.X509Certificates;
1616
using Azure.ResourceManager.Network.Models;
17-
using AzureApplicationGatewayOrchestratorExtension;
1817
using AzureApplicationGatewayOrchestratorExtension.AppGatewayCertificateJobs;
1918
using AzureApplicationGatewayOrchestratorExtension.Client;
2019
using Keyfactor.Logging;

‎AzureAppGatewayOrchestrator.Tests/AzureAppGatewayOrchestrator_Client.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,12 @@ public void AzureClientIntegrationTest()
8787
// Step 3 - Get the certificates that exist on the app gateway
8888

8989
// Act
90-
IEnumerable<Keyfactor.Orchestrators.Extensions.CurrentInventoryItem> certs = client.GetAppGatewaySslCertificates();
90+
OperationResult<IEnumerable<Keyfactor.Orchestrators.Extensions.CurrentInventoryItem>> certs = client.GetAppGatewaySslCertificates();
9191

9292
// Assert
93-
Assert.NotNull(certs);
94-
Assert.NotEmpty(certs);
95-
Assert.Contains(certs, c => c.Alias == certName);
93+
Assert.NotNull(certs.Result);
94+
Assert.NotEmpty(certs.Result);
95+
Assert.Contains(certs.Result, c => c.Alias == certName);
9696

9797
// Step 4 - Try to remove the certificate from the app gateway, which should fail
9898
// since it's bound to an HTTPS listener
@@ -119,7 +119,7 @@ public void AzureClientIntegrationTest()
119119
// Rebind the HTTPS listener with the original certificate, if there was only 1 certificate
120120
// previously, otherwise bind it with the first certificate in the list.
121121

122-
ApplicationGatewaySslCertificate replacement = client.GetAppGatewayCertificateByName(certs.First(c => c.Alias != certName).Alias);
122+
ApplicationGatewaySslCertificate replacement = client.GetAppGatewayCertificateByName(certs.Result.First(c => c.Alias != certName).Alias);
123123

124124
client.UpdateHttpsListenerCertificate(replacement, httpsListenerName);
125125

‎AzureAppGatewayOrchestrator.Tests/AzureAppGatewayOrchestrator_FakeClient.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public IAzureAppGatewayClient Build()
7474
public IEnumerable<string>? AppGatewaysAvailableOnFakeTenant { get; set; }
7575
public Dictionary<string, ApplicationGatewaySslCertificate>? CertificatesAvailableOnFakeAppGateway { get; set; }
7676

77-
public IEnumerable<CurrentInventoryItem> GetAppGatewaySslCertificates()
77+
public OperationResult<IEnumerable<CurrentInventoryItem>> GetAppGatewaySslCertificates()
7878
{
7979
_logger.LogDebug("Getting App Gateway SSL Certificates from fake app gateway");
8080

@@ -84,6 +84,8 @@ public IEnumerable<CurrentInventoryItem> GetAppGatewaySslCertificates()
8484
}
8585

8686
List<CurrentInventoryItem> inventoryItems = new List<CurrentInventoryItem>();
87+
OperationResult<IEnumerable<CurrentInventoryItem>> result = new(inventoryItems);
88+
8789
foreach (ApplicationGatewaySslCertificate cert in CertificatesAvailableOnFakeAppGateway.Values)
8890
{
8991
inventoryItems.Add(new CurrentInventoryItem
@@ -98,7 +100,7 @@ public IEnumerable<CurrentInventoryItem> GetAppGatewaySslCertificates()
98100

99101
_logger.LogDebug($"Fake client has {inventoryItems.Count} certificates in inventory");
100102

101-
return inventoryItems;
103+
return result;
102104
}
103105

104106
public ApplicationGatewaySslCertificate AddCertificate(string certificateName, string certificateData, string certificatePassword)

‎AzureAppGatewayOrchestrator/AppGatewayCertificateJobs/Inventory.cs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,32 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
5151

5252
try
5353
{
54-
inventoryItems = Client.GetAppGatewaySslCertificates().ToList();
54+
OperationResult<IEnumerable<CurrentInventoryItem>> inventoryResult = Client.GetAppGatewaySslCertificates();
55+
if (!inventoryResult.Success)
56+
{
57+
// Aggregate the messages into the failure message. Since an exception wasn't thrown,
58+
// we still have a partial success. We want to return a warning.
59+
result.FailureMessage += inventoryResult.ErrorMessage;
60+
result.Result = OrchestratorJobStatusJobResult.Warning;
61+
_logger.LogWarning(result.FailureMessage);
62+
}
63+
else
64+
{
65+
result.Result = OrchestratorJobStatusJobResult.Success;
66+
}
67+
68+
// At least partial success is guaranteed, so we can continue with the inventory items
69+
// that we were able to pull down.
70+
inventoryItems = inventoryResult.Result.ToList();
71+
5572
} catch (Exception ex)
5673
{
74+
// Exception is triggered if we weren't able to pull down the list of certificates
75+
// from Azure. This could be due to a number of reasons, including network issues,
76+
// or the user not having the correct permissions. An exception won't be triggered
77+
// if there are no certificates in the App Gateway, or if we weren't able to assemble
78+
// the list of certificates into a CurrentInventoryItem.
79+
5780
_logger.LogError(ex, "Error getting App Gateway SSL Certificates:\n" + ex.Message);
5881
result.FailureMessage = "Error getting App Gateway SSL Certificates:\n" + ex.Message;
5982
return result;
@@ -64,7 +87,6 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
6487
//cb.DynamicInvoke(inventoryItems);
6588
cb(inventoryItems);
6689

67-
result.Result = OrchestratorJobStatusJobResult.Success;
6890
return result;
6991
}
7092
}

‎AzureAppGatewayOrchestrator/Client/GatewayClient.cs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -273,15 +273,16 @@ public ApplicationGatewaySslCertificate GetAppGatewayCertificateByName(string ce
273273
return gatewaySslCertificate;
274274
}
275275

276-
public IEnumerable<CurrentInventoryItem> GetAppGatewaySslCertificates()
276+
public OperationResult<IEnumerable<CurrentInventoryItem>> GetAppGatewaySslCertificates()
277277
{
278-
279278
ApplicationGatewayResource appGatewayResource =
280279
_armClient.GetApplicationGatewayResource(AppGatewayResourceId).Get();
281280
_logger.LogDebug($"Getting SSL certificates from App Gateway called \"{appGatewayResource.Data.Name}\"");
282281
_logger.LogDebug($"There are {appGatewayResource.Data.SslCertificates.Count()} certificates in the response.");
283282
List<CurrentInventoryItem> inventoryItems = new List<CurrentInventoryItem>();
284283

284+
OperationResult<IEnumerable<CurrentInventoryItem>> result = new(inventoryItems);
285+
285286
foreach (ApplicationGatewaySslCertificate certObject in appGatewayResource.Data.SslCertificates)
286287
{
287288
List<string> b64EncodedDerCertificateList = new List<string>();
@@ -316,15 +317,19 @@ public IEnumerable<CurrentInventoryItem> GetAppGatewaySslCertificates()
316317
}
317318
catch (Exception e)
318319
{
319-
string error = $"Error retrieving certificate from Azure Key Vault with ID {certObject.KeyVaultSecretId}: {e.Message}";
320-
_logger.LogError(error);
321-
throw new Exception(error);
320+
string error = $"Failed to download certificate from Azure Key Vault with ID {certObject.KeyVaultSecretId}";
321+
_logger.LogError(error + $": {e.Message}");
322+
323+
result.AddRuntimeErrorMessage(error);
324+
continue;
322325
}
323326
}
324327
else
325328
{
326-
_logger.LogError($"Certificate called \"{certObject.Name}\" ({certObject.Id}) does not have any public certificate data or Key Vault secret ID.");
329+
string error = $"Certificate called \"{certObject.Name}\" ({certObject.Id}) does not have any public certificate data or Key Vault secret ID.";
330+
_logger.LogError(error);
327331

332+
result.AddRuntimeErrorMessage(error);
328333
continue;
329334
}
330335

@@ -341,8 +346,13 @@ public IEnumerable<CurrentInventoryItem> GetAppGatewaySslCertificates()
341346
inventoryItems.Add(inventoryItem);
342347
}
343348

349+
if (!result.Success)
350+
{
351+
result.ErrorSummary = $"Application Gateway Certificate inventory may be incomplete. Successfully read {inventoryItems.Count()}/{appGatewayResource.Data.SslCertificates.Count()} certificates present in the Application Gateway called {AppGatewayResourceId.Name} ({AppGatewayResourceId})\nPlease see Orchestrator logs for more details. Error summary:";
352+
}
353+
344354
_logger.LogDebug($"Found {inventoryItems.Count()} certificates in app gateway");
345-
return inventoryItems;
355+
return result;
346356
}
347357

348358
public void UpdateHttpsListenerCertificate(ApplicationGatewaySslCertificate certificate, string listenerName)

‎AzureAppGatewayOrchestrator/Client/IAzureAppGatewayClient.cs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,31 @@ public interface IAzureAppGatewayClientBuilder
2828
public IAzureAppGatewayClient Build();
2929
}
3030

31+
public class OperationResult<T>
32+
{
33+
public T Result { get; set; }
34+
public string ErrorSummary { get; set; }
35+
public List<string> Messages { get; set; } = new List<string>();
36+
public bool Success => Messages.Count == 0;
37+
38+
public OperationResult(T result)
39+
{
40+
Result = result;
41+
}
42+
43+
public void AddRuntimeErrorMessage(string message)
44+
{
45+
Messages.Add(" - " + message);
46+
}
47+
48+
public string ErrorMessage => $"{ErrorSummary}\n{string.Join("\n", Messages)}";
49+
}
50+
3151
public interface IAzureAppGatewayClient
3252
{
3353
public ApplicationGatewaySslCertificate AddCertificate(string certificateName, string certificateData, string certificatePassword);
3454
public void RemoveCertificate(string certificateName);
35-
public IEnumerable<CurrentInventoryItem> GetAppGatewaySslCertificates();
55+
public OperationResult<IEnumerable<CurrentInventoryItem>> GetAppGatewaySslCertificates();
3656
public ApplicationGatewaySslCertificate GetAppGatewayCertificateByName(string certificateName);
3757
public bool CertificateExists(string certificateName);
3858
public IEnumerable<string> DiscoverApplicationGateways();

‎AzureAppGatewayOrchestrator/ListenerBindingJobs/Inventory.cs

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
4444
JobResult result = new JobResult
4545
{
4646
Result = OrchestratorJobStatusJobResult.Failure,
47-
JobHistoryId = config.JobHistoryId
47+
JobHistoryId = config.JobHistoryId,
48+
FailureMessage = ""
4849
};
4950

5051
List<CurrentInventoryItem> appGatewayCertificateInventory;
@@ -62,9 +63,32 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
6263

6364
try
6465
{
65-
appGatewayCertificateInventory = Client.GetAppGatewaySslCertificates().ToList();
66+
OperationResult<IEnumerable<CurrentInventoryItem>> inventoryResult = Client.GetAppGatewaySslCertificates();
67+
if (!inventoryResult.Success)
68+
{
69+
// Aggregate the messages into the failure message. Since an exception wasn't thrown,
70+
// we still have a partial success. We want to return a warning.
71+
result.FailureMessage += inventoryResult.ErrorMessage;
72+
result.Result = OrchestratorJobStatusJobResult.Warning;
73+
_logger.LogWarning(result.FailureMessage);
74+
}
75+
else
76+
{
77+
result.Result = OrchestratorJobStatusJobResult.Success;
78+
}
79+
80+
// At least partial success is guaranteed, so we can continue with the inventory items
81+
// that we were able to pull down.
82+
appGatewayCertificateInventory = inventoryResult.Result.ToList();
83+
6684
} catch (Exception ex)
6785
{
86+
// Exception is triggered if we weren't able to pull down the list of certificates
87+
// from Azure. This could be due to a number of reasons, including network issues,
88+
// or the user not having the correct permissions. An exception won't be triggered
89+
// if there are no certificates in the App Gateway, or if we weren't able to assemble
90+
// the list of certificates into a CurrentInventoryItem.
91+
6892
_logger.LogError(ex, "Error getting App Gateway SSL Certificates:\n" + ex.Message);
6993
result.FailureMessage = "Error getting App Gateway SSL Certificates:\n" + ex.Message;
7094
return result;
@@ -81,7 +105,7 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
81105
} catch (Exception ex)
82106
{
83107
_logger.LogError(ex, "Error getting bound App Gateway HTTPS Listener Certificates:\n" + ex.Message);
84-
result.FailureMessage = "Error getting bound App Gateway HTTPS Listener Certificates:\n" + ex.Message;
108+
result.FailureMessage += "Error getting bound App Gateway HTTPS Listener Certificates:\n" + ex.Message;
85109
return result;
86110
}
87111

@@ -129,7 +153,7 @@ public JobResult ProcessJob(InventoryJobConfiguration config, SubmitInventoryUpd
129153

130154
cb.DynamicInvoke(certificateBindingInventory);
131155

132-
result.Result = OrchestratorJobStatusJobResult.Success;
156+
// Result is already set correctly by this point.
133157
return result;
134158
}
135159
}

0 commit comments

Comments
 (0)