Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning state to rotation #8030

Merged
merged 3 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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}");

Expand All @@ -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"))
{
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ private RotationPlan ResolveRotationPlan(PlanConfiguration planConfiguration, IL
primary!,
secondaries,
planConfiguration.RotationThreshold!.Value,
planConfiguration.WarningThreshold,
planConfiguration.RotationPeriod!.Value,
planConfiguration.RevokeAfterPeriod);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public RotationPlan(ILogger logger,
SecretStore primaryStore,
IList<SecretStore> secondaryStores,
TimeSpan rotationThreshold,
TimeSpan? warningThreshold,
TimeSpan rotationPeriod,
TimeSpan? revokeAfterPeriod)
{
Expand All @@ -25,6 +26,7 @@ public RotationPlan(ILogger logger,
OriginStore = originStore;
PrimaryStore = primaryStore;
RotationThreshold = rotationThreshold;
WarningThreshold = warningThreshold;
RotationPeriod = rotationPeriod;
RevokeAfterPeriod = revokeAfterPeriod;
SecondaryStores = new ReadOnlyCollection<SecretStore>(secondaryStores);
Expand All @@ -40,6 +42,8 @@ public RotationPlan(ILogger logger,

public TimeSpan RotationThreshold { get; }

public TimeSpan? WarningThreshold { get; }

public TimeSpan RotationPeriod { get; }

public TimeSpan? RevokeAfterPeriod { get; }
Expand Down Expand Up @@ -102,21 +106,31 @@ public async Task<RotationPlanStatus> 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have a smart default? Perhaps half the time of the expire?

Copy link
Member Author

@hallipr hallipr Apr 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that will be rotationThreshold / 2.

For the config:

{
  "rotationPeriod": ".180.00:00:00",
  "rotationThreshold": "30.00:00:00"
}

we'd get the implicit:

 "warningThreshold": "15.00:00:00"

meaning, we create a 180 day secret and rotate it 30 days before expiration. If we haven't rotated it 15 days before expiration, we start reporting a warning


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()
Expand All @@ -128,6 +142,7 @@ public async Task<RotationPlanStatus> GetStatusAsync()
{
var status = new RotationPlanStatus
{
State = RotationState.Error,
Exception = ex
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
namespace Azure.Sdk.Tools.SecretRotation.Core;

public enum RotationState
{
Error,
UpToDate,
Rotate,
Warning,
Expired,
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,41 @@ public class RotationPlanTests
/// Then the result's Expired property should return True
/// </summary>
[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<ILogger>(),
Expand All @@ -58,15 +65,15 @@ public async Task GetStatusAsync_ExpectExpirationState(
{
Mock.Of<SecretStore>(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);
}

/// <summary>
Expand Down Expand Up @@ -102,6 +109,7 @@ public async Task GetStatusAsync_RequiresRevocation(int hoursUntilRevocation, bo
Array.Empty<SecretStore>(),
default,
default,
default,
default);

// Act
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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": [
Expand Down
6 changes: 5 additions & 1 deletion tools/secret-management/schema/1.0.0/plan.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down