Treat _Default & _Required Sinks same as buckets #9384

zachberger · 2023-10-31T02:18:32Z

The google_logging_project_bucket_config resource graciously handles the “_Default” bucket by acquiring and updating the resource that already exists at the desired location. This PR updates google_logging_project_sink to behave the same way.

Release Note Template for Downstream PRs (will be copied)

logging: `google_logging_project_sink` now will aqcuire and update the resource that already exists at the desired location. These buckets cannot be removed so deleting this resource will remove the bucket config from your terraform state but will leave the logging bucket unchanged.

modular-magician · 2023-10-31T02:20:03Z

Hello! I am a robot. It looks like you are a: Community Contributor ~~Googler~~ ~~Core Contributor~~. Tests will require approval to run.

@roaks3, 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.

roaks3

I had a few nits on copy, but the code LGTM. Will wait on VCR tests to make sure.

mmv1/third_party/terraform/services/logging/resource_logging_project_sink.go

mmv1/third_party/terraform/website/docs/r/logging_project_sink.html.markdown

roaks3 · 2023-11-02T15:21:04Z

/gcbrun

modular-magician · 2023-11-02T15:30:56Z

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 ( 3 files changed, 57 insertions(+), 15 deletions(-))
Terraform Beta: Diff ( 3 files changed, 57 insertions(+), 15 deletions(-))

modular-magician · 2023-11-02T16:42:38Z

Tests analytics

Total tests: 3198
Passed tests 2860
Skipped tests: 325
Affected tests: 13

Action taken

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

TestAccBigQueryDatasetAccess_iamMember|TestAccLoggingProjectSink_disabled_update|TestAccLoggingProjectSink_updateBigquerySink|TestAccLoggingProjectSink_loggingbucket|TestAccLoggingProjectSink_heredoc|TestAccLoggingProjectSink_updatePreservesCustomWriter|TestAccLoggingProjectSink_default|TestAccLoggingProjectSink_disabled|TestAccLoggingProjectSink_described_update|TestAccLoggingProjectSink_basic|TestAccDataSourceGoogleLoggingSink_basic|TestAccLoggingProjectSink_described|TestAccLoggingProjectSink_updatePreservesUniqueWriter

Get to know how VCR tests work

modular-magician · 2023-11-02T17:05:42Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccBigQueryDatasetAccess_iamMember[Debug log]
TestAccLoggingProjectSink_disabled_update[Debug log]
TestAccLoggingProjectSink_updateBigquerySink[Debug log]
TestAccLoggingProjectSink_loggingbucket[Debug log]
TestAccLoggingProjectSink_heredoc[Debug log]
TestAccLoggingProjectSink_disabled[Debug log]
TestAccLoggingProjectSink_described_update[Debug log]
TestAccLoggingProjectSink_basic[Debug log]
TestAccDataSourceGoogleLoggingSink_basic[Debug log]
TestAccLoggingProjectSink_described[Debug log]
TestAccLoggingProjectSink_updatePreservesUniqueWriter[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccLoggingProjectSink_updatePreservesCustomWriter[Error message] [Debug log]
TestAccLoggingProjectSink_default[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 · 2023-11-05T01:19:27Z

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 ( 4 files changed, 60 insertions(+), 22 deletions(-))
Terraform Beta: Diff ( 4 files changed, 60 insertions(+), 22 deletions(-))

modular-magician · 2023-11-05T02:31:15Z

Tests analytics

Total tests: 3204
Passed tests 2865
Skipped tests: 323
Affected tests: 16

Action taken

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

TestAccContainerAwsCluster_BetaBasicHandWritten|TestAccContainerAwsCluster_BetaBasicEnumHandWritten|TestAccLoggingProjectSink_default|TestAccLoggingOrganizationSink_update|TestAccLoggingFolderSink_update|TestAccLoggingFolderSink_removeOptionals|TestAccLoggingProjectSink_described_update|TestAccLoggingFolderSink_updateBigquerySink|TestAccLoggingProjectSink_disabled_update|TestAccLoggingProjectSink_updatePreservesCustomWriter|TestAccLoggingBillingAccountSink_update|TestAccLoggingOrganizationSink_updateBigquerySink|TestAccLoggingBillingAccountSink_updateBigquerySink|TestAccLoggingProjectSink_updateBigquerySink|TestAccLoggingProjectSink_updatePreservesUniqueWriter|TestAccDataSourceGoogleServiceAccountJwt

Get to know how VCR tests work

modular-magician · 2023-11-05T03:00:16Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccContainerAwsCluster_BetaBasicHandWritten[Debug log]
TestAccContainerAwsCluster_BetaBasicEnumHandWritten[Debug log]
TestAccLoggingProjectSink_default[Debug log]
TestAccLoggingOrganizationSink_update[Debug log]
TestAccLoggingFolderSink_update[Debug log]
TestAccLoggingFolderSink_removeOptionals[Debug log]
TestAccLoggingProjectSink_described_update[Debug log]
TestAccLoggingFolderSink_updateBigquerySink[Debug log]
TestAccLoggingProjectSink_disabled_update[Debug log]
TestAccLoggingBillingAccountSink_update[Debug log]
TestAccLoggingOrganizationSink_updateBigquerySink[Debug log]
TestAccLoggingBillingAccountSink_updateBigquerySink[Debug log]
TestAccLoggingProjectSink_updateBigquerySink[Debug log]
TestAccLoggingProjectSink_updatePreservesUniqueWriter[Debug log]
TestAccDataSourceGoogleServiceAccountJwt[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccLoggingProjectSink_updatePreservesCustomWriter[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

zachberger · 2023-11-05T23:08:44Z

I do not believe the remaining failure is related to my changes.

In TestAccLoggingProjectSink_updatePreservesCustomWriter I believe google_logging_project_sink.custom_writer needs to be dependent on the IAM resource google_service_account_iam_member.loggingsa-customsa-binding. I think there were recent changes in the API that added validation for this at google_logging_project_sink creation time. 693d184 attempts to test this hypothesis.

modular-magician · 2023-11-05T23:16:41Z

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 ( 4 files changed, 68 insertions(+), 24 deletions(-))
Terraform Beta: Diff ( 4 files changed, 68 insertions(+), 24 deletions(-))

modular-magician · 2023-11-06T00:28:29Z

Tests analytics

Total tests: 3204
Passed tests 2880
Skipped tests: 323
Affected tests: 1

Action taken

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

TestAccLoggingProjectSink_updatePreservesCustomWriter

Get to know how VCR tests work

modular-magician · 2023-11-06T00:34:07Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccLoggingProjectSink_updatePreservesCustomWriter[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

zachberger · 2023-11-06T00:35:16Z

Looks like the hypothesis in #9384 (comment) was correct

mmv1/third_party/terraform/services/logging/resource_logging_sink.go

roaks3 · 2023-11-07T22:02:38Z

I will be OOO for the rest of the week, so transferring this review to another team member in case there is progress during that time.

Note that there is some internal discussion at b/304503197

zachberger · 2023-11-10T01:31:29Z

@slevenick I believe all concerns have been resolved. PTAL and let me know if there is anything outstanding before approval.

mmv1/third_party/terraform/services/logging/resource_logging_project_sink.go

zachberger · 2023-11-13T17:49:24Z

Thanks @slevenick

…#9384) * Treat _Default & _Required Sinks same as buckets * Update documentation. * Update exclusions every time. * Do not check if _Default bucket has been deleted. * Clean-up log statements. * Fixup documentation. * Ensure SA IAM set before associating with bucket.

modular-magician requested a review from roaks3 October 31, 2023 02:20

modular-magician added awaiting-approval Pull requests that need reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Oct 31, 2023

roaks3 reviewed Nov 2, 2023

View reviewed changes

modular-magician requested a review from roaks3 November 2, 2023 15:22

zachberger and others added 6 commits November 4, 2023 16:53

Treat _Default & _Required Sinks same as buckets

1ec0bd9

Update documentation.

b37c2a8

Update exclusions every time.

1cea3d4

Do not check if _Default bucket has been deleted.

ecaa711

Clean-up log statements.

9df1e44

Fixup documentation.

1dd6ae9

zachberger force-pushed the main branch from 88426fb to 1dd6ae9 Compare November 5, 2023 01:09

Ensure SA IAM set before associating with bucket.

693d184

roaks3 reviewed Nov 7, 2023

View reviewed changes

mmv1/third_party/terraform/services/logging/resource_logging_sink.go Show resolved Hide resolved

roaks3 requested a review from slevenick November 7, 2023 22:03

slevenick reviewed Nov 13, 2023

View reviewed changes

mmv1/third_party/terraform/services/logging/resource_logging_project_sink.go Show resolved Hide resolved

slevenick approved these changes Nov 13, 2023

View reviewed changes

slevenick merged commit 4076318 into GoogleCloudPlatform:main Nov 13, 2023
6 checks passed

zachberger mentioned this pull request Dec 1, 2023

Add Logging Settings datasources. #9526

Merged

zachberger mentioned this pull request Aug 8, 2024

Add custom logic for handling _Default and _Required for google_logging_project_sink #7226

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Treat _Default & _Required Sinks same as buckets #9384

Treat _Default & _Required Sinks same as buckets #9384

zachberger commented Oct 31, 2023

modular-magician commented Oct 31, 2023

roaks3 left a comment

roaks3 commented Nov 2, 2023

modular-magician commented Nov 2, 2023

modular-magician commented Nov 2, 2023

modular-magician commented Nov 2, 2023

modular-magician commented Nov 5, 2023

modular-magician commented Nov 5, 2023

modular-magician commented Nov 5, 2023

zachberger commented Nov 5, 2023 •

edited

Loading

modular-magician commented Nov 5, 2023

modular-magician commented Nov 6, 2023

modular-magician commented Nov 6, 2023

zachberger commented Nov 6, 2023

roaks3 commented Nov 7, 2023

zachberger commented Nov 10, 2023

zachberger commented Nov 13, 2023

Treat _Default & _Required Sinks same as buckets #9384

Treat _Default & _Required Sinks same as buckets #9384

Conversation

zachberger commented Oct 31, 2023

modular-magician commented Oct 31, 2023

roaks3 left a comment

Choose a reason for hiding this comment

roaks3 commented Nov 2, 2023

modular-magician commented Nov 2, 2023

Diff report

modular-magician commented Nov 2, 2023

Tests analytics

Action taken

modular-magician commented Nov 2, 2023

Rerun these tests in REPLAYING mode to catch issues

modular-magician commented Nov 5, 2023

Diff report

modular-magician commented Nov 5, 2023

Tests analytics

Action taken

modular-magician commented Nov 5, 2023

Rerun these tests in REPLAYING mode to catch issues

zachberger commented Nov 5, 2023 • edited Loading

modular-magician commented Nov 5, 2023

Diff report

modular-magician commented Nov 6, 2023

Tests analytics

Action taken

modular-magician commented Nov 6, 2023

Rerun these tests in REPLAYING mode to catch issues

zachberger commented Nov 6, 2023

roaks3 commented Nov 7, 2023

zachberger commented Nov 10, 2023

zachberger commented Nov 13, 2023

zachberger commented Nov 5, 2023 •

edited

Loading