-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add dynamicGroupMetadata in cloud_identity_group #9452
Conversation
Hello! I am a robot. It looks like you are a: @slevenick, 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 reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 436 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup|TestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample |
|
Failed with Error
test account need to be upgrade to cloud identity premium according to @rileykarson |
a0eed5e
to
636683e
Compare
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 436 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup|TestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample|TestAccDataSourceGoogleServiceAccountAccessToken_basic |
Rerun these tests in REPLAYING mode to catch issues
|
id = "tf-test-my-identity-dynamic-group%{random_suffix}@%{org_domain}" | ||
} | ||
|
||
labels = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the automatically added dynamic group label is causing problems here. We may need to either add a DiffSuppressFunc for that label, or add it explicitly to the tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, added it to the tests.
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 437 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup|TestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample|TestAccDataprocJobIamPolicy |
Rerun these tests in REPLAYING mode to catch issues
|
04dc53b
to
e38f6ae
Compare
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 442 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup|TestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 429 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still seeing this show up in the tests:
~ labels = {
- "cloudidentity.googleapis.com/groups.dynamic" = "" -> null
# (1 unchanged element hidden)
}
Are you able to successfully run the tests?
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 440 insertions(+), 14 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_cloud_identity_group" "primary" {
dynamic_group_metadata {
queries {
query = # value needed
resource_type = # value needed
}
}
}
|
The test failed at group creation, added a diffsuppressfunc instead, workaround to use diffsupressfunc for map. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 423 insertions(+), 5 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup|TestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample |
Rerun these tests in REPLAYING mode to catch issues
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continuing to fail due to server-set label
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 423 insertions(+), 5 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample|TestAccComputeRegionPerInstanceConfig_removeInstanceOnDestroy |
Rerun these tests in REPLAYING mode to catch issues
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 423 insertions(+), 5 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample|TestAccDataprocClusterIamPolicy |
Rerun these tests in REPLAYING mode to catch issues
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 423 insertions(+), 5 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample |
Rerun these tests in REPLAYING mode to catch issues
|
This reverts commit 90ca9d4.
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 456 insertions(+), 5 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccCloudIdentityGroup_cloudIdentityGroupsDynamicExample |
|
@GoogleCloudPlatform/terraform-team This PR has been waiting for review for 3 weeks. Please take a look! Use the label |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test failing, please fix
@sqin2019, this PR is waiting for action from you. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. If no action is taken, this PR will be closed in 28 days. This notification can be disabled with the |
@sqin2019, this PR is waiting for action from you. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. If no action is taken, this PR will be closed in 14 days. This notification can be disabled with the |
Add dynamicGroupMetadata in cloud_identity_group
cann't test the change as I don't have a test cloud identity account.