magic-modules
magic-modules copied to clipboard
GoogleCloudPlatform
Adds support for IAM Policies for Cloud Logging LogView [working in progress]
Adds support for IAM Policies for Cloud Logging LogView.
This pr is working in progress, not adding any tests yet.
Release Note Template for Downstream PRs (will be copied)
Added support for scoped policies in google_logging_log_view.
Will update after office hour if
-
google_access_context_manager_access_policy_iam_policy -
google_access_context_manager_access_policy_iam_binding -
google_access_context_manager_access_policy_iam_memberwill auto generated.
Hello! I am a robot. It looks like you are a: ~Community Contributor~ Googler ~Core Contributor~. Tests will run automatically.
@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.
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 ( 5 files changed, 730 insertions(+), 2 deletions(-)) Terraform Beta: Diff ( 5 files changed, 730 insertions(+), 2 deletions(-)) TF Conversion: Diff ( 3 files changed, 376 insertions(+))
Missing test report
Your PR includes resource fields which are not covered by any test.
Resource: google_logging_log_view_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: google_logging_log_view_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Tests analytics
Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- logging
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR}}$ View the build log
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 ( 5 files changed, 730 insertions(+), 2 deletions(-)) Terraform Beta: Diff ( 5 files changed, 730 insertions(+), 2 deletions(-)) TF Conversion: Diff ( 3 files changed, 376 insertions(+))
Missing test report
Your PR includes resource fields which are not covered by any test.
Resource: google_logging_log_view_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: google_logging_log_view_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Tests analytics
Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- logging
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR}}$ View the build log
Tests analytics
Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- logging
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR}}$ View the build log
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 ( 5 files changed, 730 insertions(+), 2 deletions(-)) Terraform Beta: Diff ( 5 files changed, 730 insertions(+), 2 deletions(-)) TF Conversion: Diff ( 3 files changed, 376 insertions(+))
Missing test report
Your PR includes resource fields which are not covered by any test.
Resource: google_logging_log_view_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: google_logging_log_view_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Tests analytics
Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- logging
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR}}$ View the build log
@pengq-google could you post a comment when this is ready for review? I couldn't quite tell by your PR description. Also, the docs changes do look weird to me in this context. At first glance, I think we would want to make those changes in a separate PR.
@pengq-google could you post a comment when this is ready for review? I couldn't quite tell by your PR description. Also, the docs changes do look weird to me in this context. At first glance, I think we would want to make those changes in a separate PR.
Hi! Sure! Currently I put [working in process] in the title to indicate it's not ready yet. (I've attended office hours twice and there are still issues need to solve.) Once it's done, I'll comment here and let you know.
for the document changes, I am trying to know why they are included and how to separate them >_<
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 ( 5 files changed, 735 insertions(+), 2 deletions(-)) Terraform Beta: Diff ( 5 files changed, 735 insertions(+), 2 deletions(-)) TF Conversion: Diff ( 3 files changed, 376 insertions(+))
Missing test report
Your PR includes resource fields which are not covered by any test.
Resource: google_logging_log_view_iam_binding (0 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_binding" "primary" {
bucket = # value needed
condition {
description = # value needed
expression = # value needed
title = # value needed
}
location = # value needed
log_view = # value needed
members = # value needed
parent = # value needed
role = # value needed
}
Resource: google_logging_log_view_iam_member (0 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_member" "primary" {
bucket = # value needed
condition {
description = # value needed
expression = # value needed
title = # value needed
}
location = # value needed
log_view = # value needed
member = # value needed
parent = # value needed
role = # value needed
}
Resource: google_logging_log_view_iam_policy (0 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_policy" "primary" {
bucket = # value needed
location = # value needed
log_view = # value needed
parent = # value needed
policy_data = # value needed
}
Tests analytics
Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- logging
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR}}$ View the build log
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 ( 5 files changed, 730 insertions(+), 2 deletions(-)) Terraform Beta: Diff ( 5 files changed, 730 insertions(+), 2 deletions(-)) TF Conversion: Diff ( 3 files changed, 376 insertions(+))
Missing test report
Your PR includes resource fields which are not covered by any test.
Resource: google_logging_log_view_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: google_logging_log_view_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Tests analytics
Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- logging
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR}}$ View the build log
It still doesn't work.
Error: google-beta/services/logging/iam_logging_log_view_generated_test.go:48:24: fmt.Sprintf format %s reads arg #3, but call has 2 args
Error: google-beta/services/logging/iam_logging_log_view_generated_test.go:58:24: fmt.Sprintf format %s reads arg #3, but call has 2 args
Error: google-beta/services/logging/iam_logging_log_view_generated_test.go:85:24: fmt.Sprintf format %s reads arg #3, but call has 2 args
Error: google-beta/services/logging/iam_logging_log_view_generated_test.go:112:24: fmt.Sprintf format %s reads arg #3, but call has 2 args
Error: google-beta/services/logging/iam_logging_log_view_generated_test.go:121:24: fmt.Sprintf format %s reads arg #3, but call has 2 args
The error in the generated test code is:
{
ResourceName: "google_logging_log_view_iam_binding.foo",
ImportStateId: fmt.Sprintf("%s/locations/%s/buckets/%s/views/%s roles/logging.admin",
envvar.GetTestRegionFromEnv(),
fmt.Sprintf("projects/%s/locations/%s/buckets/_Default/views/%s",
envvar.GetTestProjectFromEnv(),
envvar.GetTestRegionFromEnv(),
fmt.Sprintf("tf-test-my-view%s", context["random_suffix"]))),
ImportState: true,
ImportStateVerify: true,
}
I start to guess that the failed tests may be caused by {{parent}} in self_link is lacking proper prefix (such as "projects/") to let Terraform know what test_env_var should extract.
Anyone give me advices?
Thanks in advance!
My knowledge of primary_resource_name is limited, but my understanding is that you are passing a test name (or multiple names) to populate the test for the IAM policy, and it doesn't need to be fully qualified like you have here.
You might need something like '"_Default", fmt.Sprintf("tf-test-my-view%s", context["random_suffix"])', to specify a name for the parent bucket and the log view.
Looking at your import format, you might not get some of the automatic behavior that is provided for fields like project and region, because you use parent and location. I think that means you may need more values provided to the test, and parent in particular probably needs to be %parent in your import format so that it can include slashes. See other resources, for example mmv1/products/vertexai/FeaturestoreEntitytype.yaml.
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 ( 5 files changed, 729 insertions(+), 2 deletions(-)) Terraform Beta: Diff ( 5 files changed, 729 insertions(+), 2 deletions(-)) TF Conversion: Diff ( 3 files changed, 376 insertions(+))
Missing test report
Your PR includes resource fields which are not covered by any test.
Resource: google_logging_log_view_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: google_logging_log_view_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Tests analytics
Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- logging
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR}}$ View the build log
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 ( 5 files changed, 729 insertions(+), 2 deletions(-)) Terraform Beta: Diff ( 5 files changed, 729 insertions(+), 2 deletions(-)) TF Conversion: Diff ( 3 files changed, 376 insertions(+))
Missing test report
Your PR includes resource fields which are not covered by any test.
Resource: google_logging_log_view_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: google_logging_log_view_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Tests analytics
Total tests: 69
Passed tests 66
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
TestAccLoggingLogViewIamMemberGenerated|TestAccLoggingLogViewIamBindingGenerated|TestAccLoggingLogViewIamPolicyGenerated
Get to know how VCR tests work
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccLoggingLogViewIamMemberGenerated[Error message] [Debug log]
TestAccLoggingLogViewIamBindingGenerated[Error message] [Debug log]
TestAccLoggingLogViewIamPolicyGenerated[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
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 ( 5 files changed, 729 insertions(+), 2 deletions(-)) Terraform Beta: Diff ( 5 files changed, 729 insertions(+), 2 deletions(-)) TF Conversion: Diff ( 3 files changed, 376 insertions(+))
Missing test report
Your PR includes resource fields which are not covered by any test.
Resource: google_logging_log_view_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: google_logging_log_view_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_logging_log_view_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Tests analytics
Total tests: 69
Passed tests 66
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
TestAccLoggingLogViewIamPolicyGenerated|TestAccLoggingLogViewIamMemberGenerated|TestAccLoggingLogViewIamBindingGenerated
Get to know how VCR tests work
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccLoggingLogViewIamPolicyGenerated[Error message] [Debug log]
TestAccLoggingLogViewIamMemberGenerated[Error message] [Debug log]
TestAccLoggingLogViewIamBindingGenerated[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
I am not sure what it does not pass the VCR-test, but at least it pass the unit-tests now. seems I need to pass in values for:
condition {
description = # value needed
expression = # value needed
title = # value needed
}
keep working on it.
according to https://github.com/GoogleCloudPlatform/magic-modules/pull/9228, seems the Missing test report can be treated as "unrelated test failure!"
---[ REQUEST ]---------------------------------------
POST /v2/projects/ci-test-project-188019/locations/global/buckets/_Default/views/tf-test-my-viewebqqtdkbe6:setIamPolicy?alt=json HTTP/1.1
Host: logging.googleapis.com
User-Agent: Terraform/1.2.5 (+https://www.terraform.io) Terraform-Plugin-SDK/2.10.1 terraform-provider-google-beta/acc
Content-Length: 112
Content-Type: application/json
Accept-Encoding: gzip
{
"policy": {
"bindings": [
{
"members": [
"user:[email protected]"
],
"role": "roles/logging.admin"
}
],
"version": 3
}
}
-----------------------------------------------------
2024/01/13 21:17:44 [DEBUG] Google API Response Details:
---[ RESPONSE ]--------------------------------------
HTTP/2.0 404 Not Found
Cache-Control: private
Content-Type: application/json; charset=UTF-8
Date: Sat, 13 Jan 2024 21:17:44 GMT
Server: ESF
Server-Timing: gfet4t7; dur=20
Vary: Origin
Vary: X-Origin
Vary: Referer
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 0
{
"error": {
"code": 404,
"message": "Method not found.",
"status": "NOT_FOUND"
}
}
-----------------------------------------------------
I'll double check if the meta IAM is in prod. If not, will try to test in staging.
if the meta IAM is in prod, does that means the pr is correct, and all related tests will pass?
also, from https://github.com/GoogleCloudPlatform/magic-modules/pull/9454 it also added files mmv1/templates/terraform/iam/example_config_body/vertex_ai_endpoint.tf.erb is the similar files needed for this pr? why it is needed/not needed? Just a few files in terraform/iam/example_config_body folder
Regarding the example_config_body, this is what we have in our docs. It seems to be required in rare cases where the parent fields are structured in a non-standard way. In the PR you linked, it is unclear to me why it was needed (it's possible it was not required, but was still included in the PR and things worked):
# Some resources (IAP) use fields named differently from the parent resource.
# We need to use the parent's attributes to create an IAM policy, but they may not be
# named as the IAM IAM resource expects.
# This allows us to specify a file (relative to MM root) containing a partial terraform
# config with the test/example attributes of the IAM resource.
Looking at the most recent test failures, I think there are a few different possibilities. One is that sometimes 404 is used to indicate insufficient permissions. Have you been able to test locally with a user you know have the correct permissions to change this?
Other possibilities: I don't see this endpoint in the API documentation, is it definitely available to the public (perhaps a curl could confirm)? Is the url formed properly?
Looking at the most recent test failures, I think there are a few different possibilities. One is that sometimes 404 is used to indicate insufficient permissions. Have you been able to test locally with a user you know have the correct permissions to change this?
Other possibilities: I don't see this endpoint in the API documentation, is it definitely available to the public (perhaps a curl could confirm)? Is the url formed properly?
this api will be public (not ready in prod yet, when in prod it will be public).