Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

Reduce creation minimum age to 1w temporarily. #2107

Merged
merged 1 commit into from
Oct 13, 2022
Merged

Conversation

branlwyd
Copy link
Contributor

This is intended to induce a key rotation. Once the key rotates once, we will revert this change.

The current situation: the key versions are (old, bad). Clients are submitting reports with both old & bad key versions. The bad key version cannot be used by Apple due to a technical issue. We cannot immediately delete either key version as we expect to receive reports from both. Our goal is to introduce a new key version without deleting either existing key version.

Overall plan:

  • Submit this PR.
  • Allow the key rotator to run. Since bad is more than 1w old, the key rotator will introduce a new key version new. As old was generated on 1/3 (i.e. 9 months ago), and delete_min_age is 13 months, no key versions will be deleted.
  • Revert this PR within a week of the previous step. This will avoid the key rotator from introducing additional new versions.
  • Continue operating with (old, bad, new) for the time being. On approximately 2/3/2023, the key rotator will remove old & we will return to operating with only two key versions.

This is intended to induce a key rotation. Once the key rotates once, we
will revert this change.
@branlwyd branlwyd requested a review from a team as a code owner October 11, 2022 23:45
@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2022

Codecov Report

Base: 57.56% // Head: 57.56% // No change to project coverage 👍

Coverage data is based on head (1d96a6c) compared to base (b6a3496).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2107   +/-   ##
=======================================
  Coverage   57.56%   57.56%           
=======================================
  Files          37       37           
  Lines       11655    11655           
=======================================
  Hits         6709     6709           
  Misses       4831     4831           
  Partials      115      115           
Flag Coverage Δ
facilitator_tests 59.69% <ø> (ø)
key_rotator_tests 56.02% <ø> (ø)
workflow_manager_tests 39.28% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

branlwyd added a commit that referenced this pull request Oct 11, 2022
This is part of addressing the ENPA key rotation issue. See
#2107 discussion of the
overall plan.
Copy link
Contributor

@tgeoghegan tgeoghegan left a comment

Choose a reason for hiding this comment

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

This plan in conjunction with #2108 seems good. What do you think about using kubectl edit in just one locality to set -create-min-age=168h along with -dry-run so that we can convince ourselves that applying this change in all localities will do what we want?

Copy link
Contributor

@divergentdave divergentdave left a comment

Choose a reason for hiding this comment

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

This plan looks good to me.

If we wanted to test with one locality first, ta-ta would be a good choice, as there's no data going through it. We could even drop the --dry-run and check that it behaves as expected running the second time.

@branlwyd
Copy link
Contributor Author

Indeed, testing is a great idea. ta-ta is a good testing ground for this -- per logs, looks like it rotated keys on 9/12. (I don't remember but I suspect we tested the key-rotator on this locality early.)

My testing steps were (all via GCP web UI):

  • Edit prod-us-key-rotator-ta-ta to specify --packet-encryption-key-create-min-age=168h.
  • Manually trigger a run.
  • Verify that the run created a new key and deleted no keys.
  • Manually trigger a run.
  • Verify that the run did not create nor delete any keys.
  • Edit prod-us-key-rotator-ta-ta to specify --packet-encryption-key-create-min-age=6480h once again.
  • Manually trigger a run.
  • Verify that the run did not create nor delete any keys.

Everything worked as expected. LMK if y'all have any other testing in mind, I'm going to reach out to do one final check that others are ready for the key to rotate.

@branlwyd branlwyd merged commit 0121ca2 into main Oct 13, 2022
@branlwyd branlwyd deleted the bran/rotate-key branch October 13, 2022 16:19
branlwyd added a commit that referenced this pull request Oct 13, 2022
This is part of addressing the ENPA key rotation issue. See
#2107 discussion of the
overall plan.
branlwyd added a commit that referenced this pull request Oct 13, 2022
This is part of addressing the ENPA key rotation issue. See #2107
for discussion of the overall plan.
branlwyd added a commit that referenced this pull request Oct 14, 2022
This is part of addressing the ENPA key rotation issue. See #2107
for discussion of the overall plan.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants