From ef9502e9b4a509e199a3ff5bc9f8280acc329dd1 Mon Sep 17 00:00:00 2001 From: Patrick Hallisey Date: Fri, 5 Apr 2024 23:32:36 -0700 Subject: [PATCH 1/3] Add warning state to rotation --- .../Commands/StatusCommand.cs | 26 ++++---- .../PlanConfiguration.cs | 2 + .../RotationConfiguration.cs | 1 + .../RotationPlan.cs | 27 ++++++-- .../RotationPlanStatus.cs | 4 +- .../RotationState.cs | 10 +++ .../CoreTests/RotationPlanTests.cs | 65 +++++++++++-------- .../Valid/random-string.json | 1 + .../secret-management/schema/1.0.0/plan.json | 6 +- 9 files changed, 90 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..58fa95619c3 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; 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,31 @@ 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 = WarningThreshold.HasValue + ? this.timeProvider.GetCurrentDateTimeOffset().Add(WarningThreshold.Value) + : default; - 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 (warningThresholdDate.HasValue && 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 +142,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..bfcd0a8f55b 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,41 @@ 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, 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 = TimeSpan.FromHours(warningThresholdHours); - 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 +65,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 +109,7 @@ public async Task GetStatusAsync_RequiresRevocation(int hoursUntilRevocation, bo Array.Empty(), default, default, + default, default); // Act @@ -157,9 +165,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": { From 87800a2a3fe5780562dc7fdcaaabfea23e387289 Mon Sep 17 00:00:00 2001 From: Patrick Hallisey Date: Mon, 15 Apr 2024 10:59:59 -0700 Subject: [PATCH 2/3] Use an implicit warning threshold of `rotationThreshold / 2` --- .../Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 58fa95619c3..0306f7adb03 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs @@ -108,9 +108,7 @@ public async Task GetStatusAsync() DateTimeOffset rotationThresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(RotationThreshold); - DateTimeOffset? warningThresholdDate = WarningThreshold.HasValue - ? this.timeProvider.GetCurrentDateTimeOffset().Add(WarningThreshold.Value) - : default; + DateTimeOffset warningThresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(WarningThreshold ?? RotationThreshold / 2); DateTimeOffset? minExpirationDate = allStates .Where(x => x.ExpirationDate.HasValue) @@ -120,7 +118,7 @@ public async Task GetStatusAsync() if (minExpirationDate == null || minExpirationDate <= invocationTime) state = RotationState.Expired; - else if (warningThresholdDate.HasValue && minExpirationDate <= warningThresholdDate) + else if (minExpirationDate <= warningThresholdDate) state = RotationState.Warning; else if (minExpirationDate <= rotationThresholdDate) state = RotationState.Rotate; From ab92fa0cc2d0d2c1f93db074fcf93b964969a160 Mon Sep 17 00:00:00 2001 From: Patrick Hallisey Date: Mon, 15 Apr 2024 13:29:10 -0700 Subject: [PATCH 3/3] Add unit test for implicit warning window --- .../Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs | 6 +++--- .../CoreTests/RotationPlanTests.cs | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) 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 0306f7adb03..2695525d447 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs @@ -26,7 +26,7 @@ public RotationPlan(ILogger logger, OriginStore = originStore; PrimaryStore = primaryStore; RotationThreshold = rotationThreshold; - WarningThreshold = warningThreshold; + WarningThreshold = warningThreshold ?? rotationThreshold / 2; RotationPeriod = rotationPeriod; RevokeAfterPeriod = revokeAfterPeriod; SecondaryStores = new ReadOnlyCollection(secondaryStores); @@ -42,7 +42,7 @@ public RotationPlan(ILogger logger, public TimeSpan RotationThreshold { get; } - public TimeSpan? WarningThreshold { get; } + public TimeSpan WarningThreshold { get; } public TimeSpan RotationPeriod { get; } @@ -108,7 +108,7 @@ public async Task GetStatusAsync() DateTimeOffset rotationThresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(RotationThreshold); - DateTimeOffset warningThresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(WarningThreshold ?? RotationThreshold / 2); + DateTimeOffset warningThresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(WarningThreshold); DateTimeOffset? minExpirationDate = allStates .Where(x => x.ExpirationDate.HasValue) 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 bfcd0a8f55b..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 @@ -33,17 +33,18 @@ public class RotationPlanTests [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 rotateThresholdHours, - int warningThresholdHours, + int? warningThresholdHours, int hoursUntilPrimaryExpires, int hoursUntilSecondaryExpires, RotationState expectedState) { DateTimeOffset staticTestTime = DateTimeOffset.Parse("2020-06-01T12:00:00Z"); TimeSpan rotateThreshold = TimeSpan.FromHours(rotateThresholdHours); - TimeSpan warningThreshold = TimeSpan.FromHours(warningThresholdHours); + TimeSpan? warningThreshold = warningThresholdHours.HasValue ? TimeSpan.FromHours(warningThresholdHours.Value) : null; var primaryState = new SecretState {