Improve status command output and improve error handling

tools/secret-rotation/Azure.Sdk.Tools.SecretRotation.Cli/Commands/StatusCommand.cs

@@ -1,5 +1,8 @@
-using System.CommandLine.Invocation;
+using System.CommandLine.Invocation;
+using System.Reflection;
+using System.Text;
 using System.Text.Json;
+using System.Text.Json.Serialization;
 using Azure.Sdk.Tools.SecretRotation.Configuration;
 using Azure.Sdk.Tools.SecretRotation.Core;
 
@@ -17,13 +20,130 @@ protected override async Task HandleCommandAsync(ILogger logger, RotationConfigu
         var timeProvider = new TimeProvider();
         IEnumerable<RotationPlan> plans = rotationConfiguration.GetAllRotationPlans(logger, timeProvider);
 
+        List<(RotationPlan Plan, RotationPlanStatus Status)> statuses = new();
+
         foreach (RotationPlan plan in plans)
         {
-            Console.WriteLine();
-            Console.WriteLine($"{plan.Name}:");
-
+            logger.LogInformation($"Getting status for plan '{plan.Name}'");
             RotationPlanStatus status = await plan.GetStatusAsync();
-            Console.WriteLine(JsonSerializer.Serialize(status, new JsonSerializerOptions { WriteIndented = true }));
+
+            if (logger.IsEnabled(LogLevel.Debug))
+            {
+                var builder = new StringBuilder();
+
+                builder.AppendLine($"  Plan:");
+                builder.AppendLine($"    RotationPeriod: {plan.RotationPeriod}");
+                builder.AppendLine($"    RotationThreshold: {plan.RotationThreshold}");
+                builder.AppendLine($"    RevokeAfterPeriod: {plan.RevokeAfterPeriod}");
+
+                builder.AppendLine($"  Status:");
+                builder.AppendLine($"    ExpirationDate: {status.ExpirationDate}");
+                builder.AppendLine($"    Expired: {status.Expired}");
+                builder.AppendLine($"    ThresholdExpired: {status.ThresholdExpired}");
+                builder.AppendLine($"    RequiresRevocation: {status.RequiresRevocation}");
+                builder.AppendLine($"    Exception: {status.Exception?.Message}");
+
+                logger.LogDebug(builder.ToString());
+            }
+
+
+            statuses.Add((plan, status));
         }
+
+        var errored = statuses.Where(x => x.Status.Exception is not null).ToArray();
+        var expired = statuses.Except(errored).Where(x => x.Status is { Expired: true }).ToArray();
+        var expiring = statuses.Except(errored).Where(x => x.Status is { Expired: false, ThresholdExpired: true }).ToArray();
+        var upToDate = statuses.Except(errored).Where(x => x.Status is { Expired: false, ThresholdExpired: false }).ToArray();
+
+        var statusBuilder = new StringBuilder();
+
+        void AppendStatusSection(IList<(RotationPlan Plan, RotationPlanStatus Status)> sectionStatuses, string header)
+        {
+            if (!sectionStatuses.Any())
+            {
+                return;
+            }
+
+            statusBuilder.AppendLine();
+            statusBuilder.AppendLine(header);
+            foreach ((RotationPlan plan, RotationPlanStatus status) in sectionStatuses)
+            {
+                foreach (string line in GetPlanStatusLine(plan, status).Split("\n"))
+                {
+                    statusBuilder.Append("  ");
+                    statusBuilder.AppendLine(line);
+                }
+            }
+        }
+
+        AppendStatusSection(expired, "Expired:");
+        AppendStatusSection(expiring, "Expiring:");
+        AppendStatusSection(upToDate, "Up-to-date:");
+        AppendStatusSection(errored, "Error reading plan status:");
+
+        logger.LogInformation(statusBuilder.ToString());
+
+        if (expired.Any())
+        {
+            invocationContext.ExitCode = 1;
+        }
+    }
+
+    private static string GetPlanStatusLine(RotationPlan plan, RotationPlanStatus status)
+    {
+        if (status.Exception != null)
+        {
+            return $"{plan.Name}:\n  {status.Exception.Message}";
+        }
+
+        DateTimeOffset? expirationDate = status.ExpirationDate;
+
+        DateTimeOffset now = DateTimeOffset.UtcNow;
+
+        string expiration = expirationDate.HasValue
+            ? $"{FormatTimeSpan(expirationDate.Value.Subtract(now))}"
+            : "No expiration date";
+
+        return $"{plan.Name} - {expiration} / ({FormatTimeSpan(plan.RotationPeriod)} @ {FormatTimeSpan(plan.RotationThreshold)})";
+    }
+
+    private static string FormatTimeSpan(TimeSpan timeSpan)
+    {
+        if (timeSpan == TimeSpan.Zero)
+        {
+            return "0d";
+        }
+
+        StringBuilder builder = new StringBuilder();
+
+        if (timeSpan.Days > 0)
+        {
+            builder.Append(timeSpan.Days);
+            builder.Append('d');
+        }
+
+        if (timeSpan.Days < 2 && timeSpan.TotalDays - timeSpan.Days > 0)
+        {
+            if (timeSpan.Hours > 0)
+            {
+                if (builder.Length > 0)
+                {
+                    builder.Append(' ');
+                }
+                builder.Append(timeSpan.Hours);
+                builder.Append('h');
+            }
+            if (timeSpan.Minutes > 0)
+            {
+                if (builder.Length > 0)
+                {
+                    builder.Append(' ');
+                }
+                builder.Append(timeSpan.Minutes);
+                builder.Append('m');
+            }
+        }
+
+        return builder.ToString();
     }
 }

tools/secret-rotation/Azure.Sdk.Tools.SecretRotation.Configuration/PlanConfiguration.cs

@@ -18,7 +18,7 @@ public class PlanConfiguration
         ReadCommentHandling = JsonCommentHandling.Skip,
     };
 
-    private static readonly Regex schemaPattern = new (@"https\://raw\.githubusercontent\.com/azure/azure-sdk-tools/(?<branch>.+?)/schemas/secretrotation/(?<version>.+?)/schema\.json", RegexOptions.IgnoreCase);
+    private static readonly Regex schemaPattern = new (@"/(?<version>.+?)/plan\.json", RegexOptions.IgnoreCase);
 
     public string Name { get; set; } = string.Empty;
 

tools/secret-rotation/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs

@@ -82,42 +82,57 @@ public async Task ExecuteAsync(bool onlyRotateExpiring, bool whatIf)
 
     public async Task<RotationPlanStatus> GetStatusAsync()
     {
-        DateTimeOffset invocationTime = this.timeProvider.GetCurrentDateTimeOffset();
+        try
+        {
+            DateTimeOffset invocationTime = this.timeProvider.GetCurrentDateTimeOffset();
 
-        SecretState primaryStoreState = await PrimaryStore.GetCurrentStateAsync();
+            SecretState primaryStoreState = await PrimaryStore.GetCurrentStateAsync();
 
-        IEnumerable<SecretState> rotationArtifacts = await PrimaryStore.GetRotationArtifactsAsync();
+            IEnumerable<SecretState> rotationArtifacts = await PrimaryStore.GetRotationArtifactsAsync();
 
-        var secondaryStoreStates = new List<SecretState>();
+            var secondaryStoreStates = new List<SecretState>();
 
-        foreach (SecretStore secondaryStore in SecondaryStores)
-        {
-            if (secondaryStore.CanRead)
+            foreach (SecretStore secondaryStore in SecondaryStores)
             {
-                secondaryStoreStates.Add(await secondaryStore.GetCurrentStateAsync());
+                if (secondaryStore.CanRead)
+                {
+                    secondaryStoreStates.Add(await secondaryStore.GetCurrentStateAsync());
+                }
             }
-        }
 
-        SecretState[] allStates = secondaryStoreStates.Prepend(primaryStoreState).ToArray();
+            SecretState[] allStates = secondaryStoreStates.Prepend(primaryStoreState).ToArray();
 
-        bool anyExpired = allStates.Any(state => state.ExpirationDate <= invocationTime);
+            DateTimeOffset thresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(RotationThreshold);
 
-        DateTimeOffset thresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(RotationThreshold);
+            DateTimeOffset? minExpirationDate = allStates.Where(x => x.ExpirationDate.HasValue).Min(x => x.ExpirationDate);
 
-        bool anyThresholdExpired = allStates.Any(state => state.ExpirationDate <= thresholdDate);
+            bool anyExpired = minExpirationDate == null || minExpirationDate <= invocationTime;
 
-        bool anyRequireRevocation = rotationArtifacts.Any(state => state.RevokeAfterDate <= invocationTime);
+            bool anyThresholdExpired = minExpirationDate <= thresholdDate;
 
-        var status = new RotationPlanStatus
+            bool anyRequireRevocation = rotationArtifacts.Any(state => state.RevokeAfterDate <= invocationTime);
+
+            var status = new RotationPlanStatus
+            {
+                ExpirationDate = minExpirationDate,
+                Expired = anyExpired,
+                ThresholdExpired = anyThresholdExpired,
+                RequiresRevocation = anyRequireRevocation,
+                PrimaryStoreState = primaryStoreState,
+                SecondaryStoreStates = secondaryStoreStates.ToArray()
+            };
+
+            return status;
+        }
+        catch (RotationException ex)
         {
-            Expired = anyExpired,
-            ThresholdExpired = anyThresholdExpired,
-            RequiresRevocation = anyRequireRevocation,
-            PrimaryStoreState = primaryStoreState,
-            SecondaryStoreStates = secondaryStoreStates.ToArray()
-        };
-
-        return status;
+            var status = new RotationPlanStatus
+            {
+                Exception = ex
+            };
+
+            return status;
+        }
     }
 
     private async Task RotateAsync(string operationId, SecretState currentState, bool whatIf)

tools/secret-rotation/Azure.Sdk.Tools.SecretRotation.Core/RotationPlanStatus.cs

@@ -1,4 +1,4 @@
-using Azure.Sdk.Tools.SecretRotation.Core;
+using Azure.Sdk.Tools.SecretRotation.Core;
 
 namespace Azure.Sdk.Tools.SecretRotation.Core;
 
@@ -13,4 +13,8 @@ public class RotationPlanStatus
     public SecretState? PrimaryStoreState { get; set; }
 
     public IReadOnlyList<SecretState>? SecondaryStoreStates { get; set; }
+
+    public DateTimeOffset? ExpirationDate { get; set; }
+
+    public RotationException? Exception { get; set; }
 }

tools/secret-rotation/Azure.Sdk.Tools.SecretRotation.Stores.KeyVault/KeyVaultCertificateStore.cs

@@ -12,12 +12,15 @@ namespace Azure.Sdk.Tools.SecretRotation.Stores.KeyVault;
 public class KeyVaultCertificateStore : SecretStore
 {
     public const string MappingKey = "Key Vault Certificate";
+    private const string RevokeAfterTag = "RevokeAfter";
+    private const string OperationIdTag = "OperationId";
+    private const string RevokedTag = "Revoked";
 
     private static readonly Regex uriRegex = new(
         @"^(?<VaultUri>https://.+?)/certificates/(?<CertificateName>[^/]+)$",
         RegexOptions.IgnoreCase | RegexOptions.Compiled,
         TimeSpan.FromSeconds(5));
-
+        
     private readonly CertificateClient certificateClient;
     private readonly string certificateName;
     private readonly ILogger logger;
@@ -41,8 +44,6 @@ public KeyVaultCertificateStore(
 
     public override bool CanAnnotate => true;
 
-    public override bool CanRevoke => true;
-
     public static Func<StoreConfiguration, SecretStore> GetSecretStoreFactory(TokenCredential credential,
         ILogger logger)
     {
@@ -72,7 +73,7 @@ public override async Task<SecretState> GetCurrentStateAsync()
     {
         try
         {
-            this.logger.LogDebug("Getting certificate '{SecretName}' from vault '{VaultUri}'", this.certificateName,
+            this.logger.LogDebug("Getting certificate '{CertificateName}' from vault '{VaultUri}'", this.certificateName,
                 this.vaultUri);
             Response<KeyVaultCertificateWithPolicy>? response =
                 await this.certificateClient.GetCertificateAsync(this.certificateName);
@@ -82,9 +83,13 @@ public override async Task<SecretState> GetCurrentStateAsync()
                 ExpirationDate = response.Value.Properties.ExpiresOn, StatusCode = response.GetRawResponse().Status
             };
         }
+        catch (RequestFailedException ex) when (ex.Status == 404)
+        {
+            return new SecretState { ErrorMessage = GetExceptionMessage(ex) };
+        }
         catch (RequestFailedException ex)
         {
-            return new SecretState { ExpirationDate = null, StatusCode = ex.Status, ErrorMessage = ex.Message };
+            throw new RequestFailedException(GetExceptionMessage(ex), ex);
         }
     }
 
@@ -159,29 +164,63 @@ public override async Task<IEnumerable<SecretState>> GetRotationArtifactsAsync()
     {
         var results = new List<SecretState>();
 
-        await foreach (CertificateProperties? version in
-                       this.certificateClient.GetPropertiesOfCertificateVersionsAsync(this.certificateName))
+        try
         {
-            if (!version.Tags.TryGetValue("revokeAfter", out string? revokeAfterString))
+            await foreach (CertificateProperties version in
+                           this.certificateClient.GetPropertiesOfCertificateVersionsAsync(this.certificateName))
             {
-                continue;
+                if (!version.Tags.TryGetValue("revokeAfter", out string? revokeAfterString))
+                {
+                    continue;
+                }
+
+                if (!DateTimeOffset.TryParse(revokeAfterString, out DateTimeOffset revokeAfterDate))
+                {
+                    // TODO: Warning
+                    continue;
+                }
+
+                version.Tags.TryGetValue(OperationIdTag, out string? operationId);
+
+                results.Add(new SecretState
+                {
+                    Id = version.Id.ToString(),
+                    OperationId = operationId,
+                    Tags = version.Tags, 
+                    RevokeAfterDate = revokeAfterDate,
+                });
             }
 
-            if (!DateTimeOffset.TryParse(revokeAfterString, out DateTimeOffset revokeAfterDate))
+            return results;
+        }
+        catch (RequestFailedException ex) when (ex.Status == 404)
+        {
+            return results;
+        }
+        catch (RequestFailedException ex)
+        {
+            throw new RotationException(GetExceptionMessage(ex), ex);
+        }
+    }
+
+    private string GetExceptionMessage(RequestFailedException exception)
+    {
+        if (exception.Status == 403)
+        {
+            return $"Key vault request not authorized for vault '{this.vaultUri}'";
+        }
+
+        if (exception.Status == 404)
+        {
+            if (exception.Message.StartsWith($"A certificate with (name/id) "))
             {
-                // TODO: Warning
-                continue;
+                return $"Certificate '{this.certificateName}' not found in vault '{this.vaultUri}'";
             }
 
-            results.Add(new SecretState { Tags = version.Tags, RevokeAfterDate = revokeAfterDate });
+            return $"Vault {this.vaultUri} not found.";
         }
 
-        return results;
-    }
-
-    public override Func<Task>? GetRevocationActionAsync(SecretState secretState, bool whatIf)
-    {
-        throw new NotImplementedException();
+        return $"Key vault request failed with code {exception.Status}: {exception.Message}";
     }
 
     private class Parameters