magic-modules icon indicating copy to clipboard operation
magic-modules copied to clipboard

Make kmsServiceAccountId editable for Logging Settings

Open zachberger opened this issue 1 year ago • 11 comments

Google Cloud Logging now supports setting the kmsServiceAccountId setting at the Project, Folder, and Organization level. This PR updates the google_logging_folder_settings and google_logging_organization_settings resource to allow setting this field. A future PR will be opened to create a new google_logging_project_settings resource, there is only a datasource right now as until recently there were no modifiable fields on google_logging_project_settings

Release Note Template for Downstream PRs (will be copied)

`google_logging_folder_settings` and  `google_logging_organization_settings` resources to allow setting `kms_service_account_id`

zachberger avatar Feb 26 '24 22:02 zachberger

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@ScottSuarez, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

github-actions[bot] avatar Feb 26 '24 23:02 github-actions[bot]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 7 files changed, 84 insertions(+), 43 deletions(-)) Terraform Beta: Diff ( 7 files changed, 84 insertions(+), 43 deletions(-)) TF Conversion: Diff ( 2 files changed, 20 insertions(+))

modular-magician avatar Feb 27 '24 03:02 modular-magician

Tests analytics

Total tests: 66 Passed tests: 63 Skipped tests: 0 Affected tests: 3

Click here to see the affected service packages
  • logging

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccLoggingFolderSettings_loggingFolderSettingsAllExample|TestAccLoggingFolderSettings_update|TestAccLoggingOrganizationSettings_update

Get to know how VCR tests work

modular-magician avatar Feb 27 '24 03:02 modular-magician

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccLoggingFolderSettings_loggingFolderSettingsAllExample[Error message] [Debug log] TestAccLoggingFolderSettings_update[Error message] [Debug log] TestAccLoggingOrganizationSettings_update[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test

modular-magician avatar Feb 27 '24 03:02 modular-magician

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 7 files changed, 84 insertions(+), 43 deletions(-)) Terraform Beta: Diff ( 7 files changed, 84 insertions(+), 43 deletions(-)) TF Conversion: Diff ( 2 files changed, 20 insertions(+))

modular-magician avatar Feb 27 '24 04:02 modular-magician

Tests analytics

Total tests: 66 Passed tests: 63 Skipped tests: 0 Affected tests: 3

Click here to see the affected service packages
  • logging

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccLoggingFolderSettings_loggingFolderSettingsAllExample|TestAccLoggingFolderSettings_update|TestAccLoggingOrganizationSettings_update

Get to know how VCR tests work

modular-magician avatar Feb 27 '24 04:02 modular-magician

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccLoggingFolderSettings_loggingFolderSettingsAllExample[Error message] [Debug log] TestAccLoggingFolderSettings_update[Error message] [Debug log] TestAccLoggingOrganizationSettings_update[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test

modular-magician avatar Feb 27 '24 04:02 modular-magician

@ScottSuarez your advice would be appreciated here. The property kms_service_account_id can be updated once and only once, and only when its a migration from a legacy Service Account (prefixed with cmek-) to a different Service Account (prefixed with service-). It is not possible to create organizations, folders, or projects configured with that legacy service account, so I'm not sure how to reliably and repeatedly test that within terraform. More details on why this migration is required can be found here Migrate CMEK service accounts

zachberger avatar Feb 27 '24 04:02 zachberger

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 7 files changed, 84 insertions(+), 43 deletions(-)) Terraform Beta: Diff ( 7 files changed, 84 insertions(+), 43 deletions(-)) TF Conversion: Diff ( 2 files changed, 20 insertions(+))

modular-magician avatar Feb 27 '24 04:02 modular-magician

Tests analytics

Total tests: 66 Passed tests: 63 Skipped tests: 0 Affected tests: 3

Click here to see the affected service packages
  • logging

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccLoggingFolderSettings_loggingFolderSettingsAllExample|TestAccLoggingFolderSettings_update|TestAccLoggingOrganizationSettings_update

Get to know how VCR tests work

modular-magician avatar Feb 27 '24 04:02 modular-magician

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccLoggingFolderSettings_loggingFolderSettingsAllExample[Error message] [Debug log] TestAccLoggingFolderSettings_update[Error message] [Debug log] TestAccLoggingOrganizationSettings_update[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test

modular-magician avatar Feb 27 '24 04:02 modular-magician

@ScottSuarez your advice would be appreciated here. The property kms_service_account_id can be updated once and only once, and only when its a migration from a legacy Service Account (prefixed with cmek-) to a different Service Account (prefixed with service-). It is not possible to create organizations, folders, or projects configured with that legacy service account, so I'm not sure how to reliably and repeatedly test that within terraform. More details on why this migration is required can be found here Migrate CMEK service accounts

I don't believe we should support this at the terraform level if it can done only once and only applies to legacy service accounts. I think that users should set this out of band of terraform in that case.

ScottSuarez avatar Mar 04 '24 18:03 ScottSuarez