-
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
Promoted (most of) the supported cloud identity resources to GA #4211
Conversation
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 15 files changed, 1792 insertions(+), 13 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=156837" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 15 files changed, 1788 insertions(+), 13 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=156838" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 15 files changed, 1787 insertions(+), 13 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=156839" |
non-beta test run here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=156843 |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 16 files changed, 1790 insertions(+), 15 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157025" |
…hipDestroyProducer
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 16 files changed, 1790 insertions(+), 15 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157032" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 16 files changed, 1767 insertions(+), 15 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157039" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 16 files changed, 1721 insertions(+), 15 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157045" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 16 files changed, 1699 insertions(+), 15 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157180" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 16 files changed, 1696 insertions(+), 15 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157279" |
group = google_cloud_identity_group.group.id | ||
|
||
member_key { | ||
preferred_member_key { |
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.
Why is the name changing here?
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.
member_key is only supported in the beta API. This test is being run against the GA API; member_key usage is instead handled by handwritten tests in third_party/terraform/tests/resource_cloud_identity_group_membership_test.go.erb
@@ -222,6 +143,7 @@ objects: | |||
input: true | |||
description: | | |||
EntityKey of the member. | |||
min_version: beta |
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.
How does exactly_one_of
work when one field is beta and one isn't?
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.
It looks like exactly_one_of is converted directly into terraform, but using get_property_schema_path and array.compact
, which seems to be eliminating the beta field from the resulting golang code.
In other words, in GA, the go code ends up with a "exactly one of" constraint that doesn't include beta fields - in this case meaning it only contains one property, at least one of which much be specified, which I believe is the correct behavior.
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 16 files changed, 1696 insertions(+), 15 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157310" |
Everything still passes after correcting the GA url so I'm gonna go ahead and merge. https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=157312 |
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 think we need to un-guard these lines as well, to make the datasources appear at GA: https://github.com/GoogleCloudPlatform/magic-modules/blob/master/third_party/terraform/utils/provider.go.erb#L183-L186
That should additionally resolve the unused code warnings in tpg eg https://travis-ci.org/github/hashicorp/terraform-provider-google/builds/743049795
Thanks Riley - I've opened #4229 making that change! |
Resolved hashicorp/terraform-provider-google#7757.
Notes:
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)