From 9ae20c3fa206a76ee8350d4508c35209701aee40 Mon Sep 17 00:00:00 2001 From: Patrick Hallisey Date: Mon, 15 Apr 2024 13:39:49 -0700 Subject: [PATCH] Add warning state to rotation (#8030) * Add warning state to rotation * Use an implicit warning threshold of `rotationThreshold / 2` * Add unit test for implicit warning window --- .../Commands/StatusCommand.cs | 26 ++++---- .../PlanConfiguration.cs | 2 + .../RotationConfiguration.cs | 1 + .../RotationPlan.cs | 25 +++++-- .../RotationPlanStatus.cs | 4 +- .../RotationState.cs | 10 +++ .../CoreTests/RotationPlanTests.cs | 66 +++++++++++-------- .../Valid/random-string.json | 1 + .../secret-management/schema/1.0.0/plan.json | 6 +- 9 files changed, 89 insertions(+), 52 deletions(-) create mode 100644 tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationState.cs diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs index 0f84b03744f..bafb33044b5 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs @@ -38,8 +38,7 @@ protected override async Task HandleCommandAsync(ILogger logger, RotationConfigu builder.AppendLine($" Status:"); builder.AppendLine($" ExpirationDate: {status.ExpirationDate}"); - builder.AppendLine($" Expired: {status.Expired}"); - builder.AppendLine($" ThresholdExpired: {status.ThresholdExpired}"); + builder.AppendLine($" State: {status.State}"); builder.AppendLine($" RequiresRevocation: {status.RequiresRevocation}"); builder.AppendLine($" Exception: {status.Exception?.Message}"); @@ -50,23 +49,21 @@ protected override async Task HandleCommandAsync(ILogger logger, RotationConfigu 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 plansBuyState = statuses.GroupBy(x => x.Status.State) + .ToDictionary(x => x.Key, x => x.ToArray()); var statusBuilder = new StringBuilder(); - void AppendStatusSection(IList<(RotationPlan Plan, RotationPlanStatus Status)> sectionStatuses, string header) + void AppendStatusSection(RotationState state, string header) { - if (!sectionStatuses.Any()) + if (!plansBuyState.TryGetValue(RotationState.Expired, out var matchingPlans)) { return; } statusBuilder.AppendLine(); statusBuilder.AppendLine(header); - foreach ((RotationPlan plan, RotationPlanStatus status) in sectionStatuses) + foreach ((RotationPlan plan, RotationPlanStatus status) in matchingPlans) { foreach (string line in GetPlanStatusLine(plan, status).Split("\n")) { @@ -76,14 +73,15 @@ void AppendStatusSection(IList<(RotationPlan Plan, RotationPlanStatus Status)> s } } - AppendStatusSection(expired, "Expired:"); - AppendStatusSection(expiring, "Expiring:"); - AppendStatusSection(upToDate, "Up-to-date:"); - AppendStatusSection(errored, "Error reading plan status:"); + AppendStatusSection(RotationState.Expired, "Expired:"); + AppendStatusSection(RotationState.Warning, "Expiring:"); + AppendStatusSection(RotationState.Rotate, "Should Rotate:"); + AppendStatusSection(RotationState.UpToDate, "Up-to-date:"); + AppendStatusSection(RotationState.Error, "Error reading plan status:"); logger.LogInformation(statusBuilder.ToString()); - if (expired.Any()) + if (statuses.Any(x => x.Status.State is RotationState.Expired or RotationState.Warning)) { invocationContext.ExitCode = 1; } diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/PlanConfiguration.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/PlanConfiguration.cs index 4c355314629..e6027a75495 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/PlanConfiguration.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/PlanConfiguration.cs @@ -24,6 +24,8 @@ public class PlanConfiguration public TimeSpan? RotationThreshold { get; set; } + public TimeSpan? WarningThreshold { get; set; } + public TimeSpan? RotationPeriod { get; set; } public TimeSpan? RevokeAfterPeriod { get; set; } diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/RotationConfiguration.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/RotationConfiguration.cs index f728d78c56f..802d4af47e4 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/RotationConfiguration.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/RotationConfiguration.cs @@ -122,6 +122,7 @@ private RotationPlan ResolveRotationPlan(PlanConfiguration planConfiguration, IL primary!, secondaries, planConfiguration.RotationThreshold!.Value, + planConfiguration.WarningThreshold, planConfiguration.RotationPeriod!.Value, planConfiguration.RevokeAfterPeriod); diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs index 3e7e242ab22..2695525d447 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs @@ -16,6 +16,7 @@ public RotationPlan(ILogger logger, SecretStore primaryStore, IList secondaryStores, TimeSpan rotationThreshold, + TimeSpan? warningThreshold, TimeSpan rotationPeriod, TimeSpan? revokeAfterPeriod) { @@ -25,6 +26,7 @@ public RotationPlan(ILogger logger, OriginStore = originStore; PrimaryStore = primaryStore; RotationThreshold = rotationThreshold; + WarningThreshold = warningThreshold ?? rotationThreshold / 2; RotationPeriod = rotationPeriod; RevokeAfterPeriod = revokeAfterPeriod; SecondaryStores = new ReadOnlyCollection(secondaryStores); @@ -40,6 +42,8 @@ public RotationPlan(ILogger logger, public TimeSpan RotationThreshold { get; } + public TimeSpan WarningThreshold { get; } + public TimeSpan RotationPeriod { get; } public TimeSpan? RevokeAfterPeriod { get; } @@ -102,21 +106,29 @@ public async Task GetStatusAsync() SecretState[] allStates = secondaryStoreStates.Prepend(primaryStoreState).ToArray(); - DateTimeOffset thresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(RotationThreshold); + DateTimeOffset rotationThresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(RotationThreshold); + + DateTimeOffset warningThresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(WarningThreshold); - DateTimeOffset? minExpirationDate = allStates.Where(x => x.ExpirationDate.HasValue).Min(x => x.ExpirationDate); + DateTimeOffset? minExpirationDate = allStates + .Where(x => x.ExpirationDate.HasValue) + .Min(x => x.ExpirationDate); - bool anyExpired = minExpirationDate == null || minExpirationDate <= invocationTime; + var state = RotationState.UpToDate; - bool anyThresholdExpired = minExpirationDate <= thresholdDate; + if (minExpirationDate == null || minExpirationDate <= invocationTime) + state = RotationState.Expired; + else if (minExpirationDate <= warningThresholdDate) + state = RotationState.Warning; + else if (minExpirationDate <= rotationThresholdDate) + state = RotationState.Rotate; bool anyRequireRevocation = rotationArtifacts.Any(state => state.RevokeAfterDate <= invocationTime); var status = new RotationPlanStatus { ExpirationDate = minExpirationDate, - Expired = anyExpired, - ThresholdExpired = anyThresholdExpired, + State = state, RequiresRevocation = anyRequireRevocation, PrimaryStoreState = primaryStoreState, SecondaryStoreStates = secondaryStoreStates.ToArray() @@ -128,6 +140,7 @@ public async Task GetStatusAsync() { var status = new RotationPlanStatus { + State = RotationState.Error, Exception = ex }; diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlanStatus.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlanStatus.cs index cbf8a2eedd1..3f7f13c2e24 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlanStatus.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlanStatus.cs @@ -4,9 +4,7 @@ namespace Azure.Sdk.Tools.SecretRotation.Core; public class RotationPlanStatus { - public bool Expired { get; set; } - - public bool ThresholdExpired { get; set; } + public RotationState State { get; set; } public bool RequiresRevocation { get; set; } diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationState.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationState.cs new file mode 100644 index 00000000000..333395f0c1a --- /dev/null +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationState.cs @@ -0,0 +1,10 @@ +namespace Azure.Sdk.Tools.SecretRotation.Core; + +public enum RotationState +{ + Error, + UpToDate, + Rotate, + Warning, + Expired, +} diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/CoreTests/RotationPlanTests.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/CoreTests/RotationPlanTests.cs index 95f432bce86..2f5eaa4339e 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/CoreTests/RotationPlanTests.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/CoreTests/RotationPlanTests.cs @@ -19,34 +19,42 @@ public class RotationPlanTests /// Then the result's Expired property should return True /// [Theory] - [TestCase(24, 0, 30, true, true)] - [TestCase(24, -1, 30, true, true)] - [TestCase(24, -1, 23, true, true)] - [TestCase(24, 30, 0, true, true)] - [TestCase(24, 30, -1, true, true)] - [TestCase(24, 23, -1, true, true)] - [TestCase(24, 24, 30, false, true)] - [TestCase(24, 23, 30, false, true)] - [TestCase(24, 30, 24, false, true)] - [TestCase(24, 30, 23, false, true)] - [TestCase(24, 25, 25, false, false)] + [TestCase(24, 22, 0, 30, RotationState.Expired)] // primary at expiration + [TestCase(24, 22, -1, 30, RotationState.Expired)] // primary past expiration + [TestCase(24, 22, -1, 23, RotationState.Expired)] // primary past expiration, secondary past rotation + [TestCase(24, 22, 30, 0, RotationState.Expired)] // secondary at expiration + [TestCase(24, 22, 30, -1, RotationState.Expired)] // secondary past expiration + [TestCase(24, 22, 23, -1, RotationState.Expired)] // secondary past expiration, primary past rotation + [TestCase(24, 22, 24, 30, RotationState.Rotate)] // primary at rotation + [TestCase(24, 22, 23, 30, RotationState.Rotate)] // primary between rotate and warning + [TestCase(24, 22, 30, 24, RotationState.Rotate)] // secondary at rotate + [TestCase(24, 22, 30, 23, RotationState.Rotate)] // secondary between rotate and warning + [TestCase(24, 22, 22, 30, RotationState.Warning)] // primary at warning + [TestCase(24, 22, 10, 30, RotationState.Warning)] // primary past warning + [TestCase(24, 22, 30, 22, RotationState.Warning)] // secondary at warning + [TestCase(24, 22, 30, 10, RotationState.Warning)] // secondary past warning + [TestCase(24, null, 30, 10, RotationState.Warning)] // implicit 12h warning window + [TestCase(24, 22, 25, 25, RotationState.UpToDate)] public async Task GetStatusAsync_ExpectExpirationState( - int thresholdHours, + int rotateThresholdHours, + int? warningThresholdHours, int hoursUntilPrimaryExpires, int hoursUntilSecondaryExpires, - bool expectExpired, - bool expectThresholdExpired) + RotationState expectedState) { DateTimeOffset staticTestTime = DateTimeOffset.Parse("2020-06-01T12:00:00Z"); - TimeSpan threshold = TimeSpan.FromHours(thresholdHours); + TimeSpan rotateThreshold = TimeSpan.FromHours(rotateThresholdHours); + TimeSpan? warningThreshold = warningThresholdHours.HasValue ? TimeSpan.FromHours(warningThresholdHours.Value) : null; - var primaryState = - new SecretState { ExpirationDate = staticTestTime.AddHours(hoursUntilPrimaryExpires) }; // after threshold - var secondaryState = - new SecretState - { - ExpirationDate = staticTestTime.AddHours(hoursUntilSecondaryExpires) - }; // before threshold + var primaryState = new SecretState + { + ExpirationDate = staticTestTime.AddHours(hoursUntilPrimaryExpires) + }; + + var secondaryState = new SecretState + { + ExpirationDate = staticTestTime.AddHours(hoursUntilSecondaryExpires) + }; var rotationPlan = new RotationPlan( Mock.Of(), @@ -58,15 +66,15 @@ public async Task GetStatusAsync_ExpectExpirationState( { Mock.Of(x => x.CanRead && x.GetCurrentStateAsync() == Task.FromResult(secondaryState)) }, - threshold, + rotateThreshold, + warningThreshold, default, default); // Act RotationPlanStatus status = await rotationPlan.GetStatusAsync(); - Assert.AreEqual(expectExpired, status.Expired); - Assert.AreEqual(expectThresholdExpired, status.ThresholdExpired); + Assert.AreEqual(expectedState, status.State); } /// @@ -102,6 +110,7 @@ public async Task GetStatusAsync_RequiresRevocation(int hoursUntilRevocation, bo Array.Empty(), default, default, + default, default); // Act @@ -157,9 +166,10 @@ public async Task RotatePlansAsync_RequiresRevocation_DoesRevocation() originStore, primaryStore, new[] { secondaryStore }, - TimeSpan.FromDays(1), - TimeSpan.FromDays(2), - default); + rotationThreshold: TimeSpan.FromDays(3), + warningThreshold: TimeSpan.FromDays(2), + rotationPeriod: TimeSpan.FromDays(2), + revokeAfterPeriod: default); // Act await rotationPlan.ExecuteAsync(true, false); diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/TestConfigurations/Valid/random-string.json b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/TestConfigurations/Valid/random-string.json index f9dcacb4d04..2e8b7535158 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/TestConfigurations/Valid/random-string.json +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/TestConfigurations/Valid/random-string.json @@ -1,6 +1,7 @@ { "$schema": "https://raw.githubusercontent.com/azure/azure-sdk-tools/main/tools/secret-management/schema/1.0.0/plan.json", "rotationThreshold": "9.00:00:00", + "warningThreshold": "6.00:00:00", "rotationPeriod": "12.00:00:00", "tags": [ "random" ], "stores": [ diff --git a/tools/secret-management/schema/1.0.0/plan.json b/tools/secret-management/schema/1.0.0/plan.json index 6538c106126..58a459d93df 100644 --- a/tools/secret-management/schema/1.0.0/plan.json +++ b/tools/secret-management/schema/1.0.0/plan.json @@ -14,7 +14,11 @@ "type": "string" }, "rotationThreshold": { - "description": "Time span indicating when the secret should be rotated before it expires", + "description": "Time span before expiration indicating when the secret should be rotated", + "type": "string" + }, + "warningThreshold": { + "description": "Time span before expiration indicating when warnings should be issued", "type": "string" }, "revokeAfterPeriod": {