Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
17 changes: 17 additions & 0 deletions src/main/java/com/uid2/admin/salt/SaltRotation.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public Result rotateSalts(
logSaltAges("refreshable-salts", targetDate, refreshableSalts);
logSaltAges("rotated-salts", targetDate, saltsToRotate);
logSaltAges("total-salts", targetDate, Arrays.asList(postRotationSalts));
logBucketFormatCount(targetDate, preRotationSalts, postRotationSalts);

var nextSnapshot = new SaltSnapshot(
nextEffective,
Expand Down Expand Up @@ -246,6 +247,22 @@ private void logSaltAges(String saltCountType, TargetDate targetDate, Collection
}
}

private void logBucketFormatCount(TargetDate targetDate, SaltEntry[] preRotationSalts, SaltEntry[] postRotationSalts) {
int newKeyBucketCounter = 0, totalKeyBucketCounter = 0, totalSaltBucketCounter = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this deserves a comment - mention the migration, that the salts are the old format and we're transitioning to encryption keys. Mention that this is to monitor the migration.


for (int i = 0; i < preRotationSalts.length && i < postRotationSalts.length; i++) {
var oldSalt = preRotationSalts[i];
var updatedSalt = postRotationSalts[i];

if (updatedSalt.currentKey() != null) totalKeyBucketCounter++;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will we also do the previous salt/keys?

Copy link
Contributor Author

@sophia-chen-ttd sophia-chen-ttd Aug 25, 2025

Choose a reason for hiding this comment

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

I don't think we need to monitor those - I'm not sure how helpful they would be in monitoring the migration?

Copy link
Contributor

@aulme aulme Aug 25, 2025

Choose a reason for hiding this comment

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

We should start seeing the first "previous key" ~1 year after we start migration and we should stop seeing "previous salt" soon after that, at least by day 420. We can only remove the salt-related code after both all current and previous salts are gone.

if (updatedSalt.currentSalt() != null) totalSaltBucketCounter++;
if (updatedSalt.currentKey() != null && oldSalt.currentSalt() != null) newKeyBucketCounter++;
}

LOGGER.info("Salt rotation bucket format: target_date={} new_key_bucket_count={} total_key_bucket_count={} total_salt_bucket_count={}",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be clearer that those are formats of buckets rotated just now, not for all bucket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are for all the buckets in snapshot, including those that haven't rotated, whereas the new_key_bucket_count are for buckets that have rotated just now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated new_key_bucket_count to migrated_key_bucket_count to hopefully make that clearer

targetDate, newKeyBucketCounter, totalKeyBucketCounter, totalSaltBucketCounter);
}

@Getter
public static final class Result {
private final SaltSnapshot snapshot; // can be null if new snapshot is not needed
Expand Down
36 changes: 34 additions & 2 deletions src/test/java/com/uid2/admin/salt/SaltRotationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ void testLogFewSaltAgesOnRotation() throws Exception {
var minAges = new Duration[]{Duration.ofDays(30), Duration.ofDays(60)};
saltRotation.rotateSalts(lastSnapshot, minAges, 0.4, targetDate());

var actual = appender.list.stream().map(Object::toString).collect(Collectors.toSet());
var actual = appender.list.stream().map(Object::toString).filter(s -> s.contains("salt_count_type") || s.contains("Salt rotation complete")).collect(Collectors.toSet());
assertThat(actual).isEqualTo(expected);
}

Expand Down Expand Up @@ -429,7 +429,7 @@ void testLogManySaltAgesOnRotation() throws Exception {
var minAges = new Duration[]{Duration.ofDays(30), Duration.ofDays(60)};
saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate());

var actual = appender.list.stream().map(Object::toString).collect(Collectors.toSet());
var actual = appender.list.stream().map(Object::toString).filter(s -> s.contains("salt_count_type") || s.contains("Salt rotation complete")).collect(Collectors.toSet());
assertThat(actual).isEqualTo(expected);
}

Expand Down Expand Up @@ -726,4 +726,36 @@ void testKeyRotationSaltToKeyRotation() throws Exception {
assertThat(salts[0].currentSalt()).isNotNull();
assertThat(salts[0].previousSalt()).isNull();
}

@ParameterizedTest
@CsvSource({
"true, 1, 3, 1",
"false, 0, 1, 3"
})
void testKeyRotationLogKeyBuckets(boolean v4Enabled, int expectedNewKeyBuckets, int expectedTotalKeyBuckets, int expectedTotalSaltBuckets) throws Exception {
saltRotation = new SaltRotation(keyGenerator, JsonObject.of(AdminConst.ENABLE_V4_RAW_UID, v4Enabled));
when(keyGenerator.generateRandomKeyString(anyInt())).thenReturn("random-key-string");

final Duration[] minAges = {
Duration.ofDays(30)
};

var willRefresh = targetDate();
var willNotRefresh = targetDate().plusDays(30);
var lastSnapshot = SaltSnapshotBuilder.start()
.entries(SaltBuilder.start().lastUpdated(targetDate().minusDays(60)).refreshFrom(willRefresh).currentSalt(),
SaltBuilder.start().lastUpdated(targetDate().minusDays(60)).refreshFrom(willNotRefresh).currentSalt(),
SaltBuilder.start().lastUpdated(targetDate().minusDays(60)).refreshFrom(willRefresh).currentKey(1),
SaltBuilder.start().lastUpdated(targetDate().minusDays(60)).refreshFrom(willNotRefresh).currentKey(2))
.build();

saltRotation.rotateSalts(lastSnapshot, minAges, 1, targetDate());

var expected = Set.of(
String.format("[INFO] Salt rotation bucket format: target_date=2025-01-01 new_key_bucket_count=%d total_key_bucket_count=%d total_salt_bucket_count=%d",
expectedNewKeyBuckets, expectedTotalKeyBuckets, expectedTotalSaltBuckets)
);
var actual = appender.list.stream().map(Object::toString).filter(s -> s.contains("Salt rotation bucket format")).collect(Collectors.toSet());
assertThat(actual).isEqualTo(expected);
}
}