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

fix: Change default rotation period from 28 hours to 90 days #111

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

kjwinters
Copy link
Contributor

Based on Google public docs (https://cloud.google.com/kms/docs/key-rotation#how_often_to_rotate_keys), changing the default key rotation period from ~28 hours to a suggested 90 days.

@kjwinters kjwinters requested a review from a team as a code owner November 2, 2023 20:57
@apeabody
Copy link
Contributor

apeabody commented Nov 2, 2023

/gcbrun

@apeabody apeabody self-assigned this Nov 2, 2023
Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @kjwinters - Will changing the default value force a replace for existing resources using the default? If so, we should probably mark as a breaking change.

@apeabody apeabody changed the title Changing default rotation period from 28 hours to 90 days fix: Change default rotation period from 28 hours to 90 days Nov 2, 2023
@chrisw-tracer
Copy link
Contributor

chrisw-tracer commented Nov 3, 2023

Thanks for the contribution @kjwinters - Will changing the default value force a replace for existing resources using the default? If so, we should probably mark as a breaking change.

@apeabody I just tested this and it won't replace keys, just update them:

# module.kms.google_kms_crypto_key.key[0] will be updated in-place
  ~ resource "google_kms_crypto_key" "key" {
        id                            = "projects/XXXX/locations/us/keyRings/X-keyring/cryptoKeys/X-kms"
        name                          = "XXXX"
      ~ rotation_period               = "7776000s" -> "777000s"
        # (6 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

@apeabody
Copy link
Contributor

apeabody commented Nov 3, 2023

Thanks for the contribution @kjwinters - Will changing the default value force a replace for existing resources using the default? If so, we should probably mark as a breaking change.

@apeabody I just tested this and it won't replace keys, just update them:

# module.kms.google_kms_crypto_key.key[0] will be updated in-place
  ~ resource "google_kms_crypto_key" "key" {
        id                            = "projects/XXXX/locations/us/keyRings/X-keyring/cryptoKeys/X-kms"
        name                          = "XXXX"
      ~ rotation_period               = "7776000s" -> "777000s"
        # (6 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Great - Thanks!

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @kjwinters! - LGTM

@apeabody apeabody merged commit 44d01da into terraform-google-modules:master Nov 3, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants