Add support for Cloud Deploy resources #5710

upodroid · 2022-02-14T17:19:51Z

Fixes: hashicorp/terraform-provider-google#11063

If this PR is for Terraform, I acknowledge that I have:

Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

`google_cloud_deploy_delivery_pipeline`

`google_cloud_deploy_target`

modular-magician · 2022-02-14T17:21:12Z

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review.

Thanks for your contribution! A human will be with you soon.

@slevenick, please review this PR or find an appropriate assignee.

modular-magician · 2022-02-14T17:35:34Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 12 files changed, 2100 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 12 files changed, 2100 insertions(+), 2 deletions(-))
TF Validator: Diff ( 5 files changed, 407 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 241 insertions(+))

upodroid · 2022-03-23T23:13:27Z

bump

mmv1/products/clouddeploy/api.yaml

slevenick · 2022-04-06T23:53:19Z

mmv1/products/clouddeploy/api.yaml

+        description: |
+          The timestamp of when the Target was last updated in RFC3339 UTC "Zulu" format, with nanosecond resolution and up to nine fractional digits.
+      - !ruby/object:Api::Type::String
+        name: 'etag'


Is this needed for any particular reason? We tend to avoid adding etag and some of these other output-only fields if they aren't needed for other resources that depend on this. Similarly, updateTime & createTime can be removed

Were these needed?

mmv1/products/clouddeploy/api.yaml

slevenick · 2022-04-06T23:54:42Z

mmv1/products/clouddeploy/terraform.yaml

+        default_from_api: true
+  Target: !ruby/object:Overrides::Terraform::ResourceOverride
+    autogen_async: true
+    id_format: '{{name}}'


Is name the right format? This is the short-form, so won't be able to identify the full resource during import?

Yes

https://github.com/modular-magician/terraform-provider-google/compare/auto-pr-5710-old..auto-pr-5710#diff-e36b98e511d7d496a126e50204560eea400fe43213204672a22acece89f3f4ebR409

On a side note, I'm not a fan of something_Id as name is the consistent resource name in most resources. The API uses name as selflink but an identical value can be read with the id attribute.

Precedent: #4951

So, having this id_format ends up with the following code to set the ID of the resource:

// Store the ID now id, err := replaceVars(d, config, "{{name}}") if err != nil { return fmt.Errorf("Error constructing id: %s", err) } d.SetId(id)

I don't believe this is right, because it doesn't represent the full resource. The ID format should be projects/{{project}}/locations/{{region}}/deliveryPipelines/{{name}} so that we can reference this value from other resources within a config (if needed)

mmv1/products/clouddeploy/terraform.yaml

mmv1/templates/terraform/examples/deploy_target.tf.erb

mmv1/products/clouddeploy/terraform.yaml

Co-authored-by: upodroid <cy@borg.dev>

upodroid · 2022-04-13T23:16:29Z

Ready for review. I'll take another pass at DeliveryPipeline in a separate PR. My assumptions around Target weren't true but docs are non-existant and the UI can't be used to patch the resource. The API docs also doesn't make it clear which fields are required,immutable, etc

modular-magician · 2022-04-13T23:36:06Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 13 files changed, 2232 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 13 files changed, 2232 insertions(+), 2 deletions(-))
TF Validator: Diff ( 5 files changed, 407 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 117 insertions(+))

modular-magician · 2022-04-14T00:18:14Z

Tests analytics

Total tests: 1985
Passed tests 1741
Skipped tests: 240
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccCloudDeployTarget_update|TestAccCloudDeployTarget_cloudDeployTargetExample|TestAccCloudDeployDeliveryPipeline_cloudDeployDeliveryPipelineExample|TestAccServiceNetworkingPeeredDNSDomain_basic

modular-magician · 2022-04-14T00:24:12Z

Tests passed during RECORDING mode:
TestAccServiceNetworkingPeeredDNSDomain_basic[view]

Tests failed during RECORDING mode:
TestAccCloudDeployDeliveryPipeline_cloudDeployDeliveryPipelineExample[view]
TestAccCloudDeployTarget_update[view]
TestAccCloudDeployTarget_cloudDeployTargetExample[view]

Please fix these to complete your PR
View the build log or the debug log for each test

slevenick · 2022-04-14T00:15:55Z

mmv1/products/clouddeploy/api.yaml

+                - !ruby/object:Api::Type::String
+                  name: 'targetId'
+                  required: true
+                  input: true


These are unnecessary as the whole resource is input: true

slevenick · 2022-04-14T00:27:34Z

mmv1/products/clouddeploy/terraform.yaml

+        default_from_api: true
+  Target: !ruby/object:Overrides::Terraform::ResourceOverride
+    autogen_async: true
+    id_format: '{{name}}'


So, having this id_format ends up with the following code to set the ID of the resource:

// Store the ID now id, err := replaceVars(d, config, "{{name}}") if err != nil { return fmt.Errorf("Error constructing id: %s", err) } d.SetId(id)

I don't believe this is right, because it doesn't represent the full resource. The ID format should be projects/{{project}}/locations/{{region}}/deliveryPipelines/{{name}} so that we can reference this value from other resources within a config (if needed)

mmv1/products/clouddeploy/api.yaml

slevenick · 2022-04-14T00:32:14Z

mmv1/products/clouddeploy/api.yaml

+        description: |
+          The timestamp of when the Target was last updated in RFC3339 UTC "Zulu" format, with nanosecond resolution and up to nine fractional digits.
+      - !ruby/object:Api::Type::String
+        name: 'etag'


Were these needed?

slevenick · 2022-04-14T00:33:08Z

/gcbrun

I enabled the API so the tests should actually run

modular-magician · 2022-04-14T00:47:11Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 13 files changed, 2232 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 13 files changed, 2232 insertions(+), 2 deletions(-))
TF Validator: Diff ( 5 files changed, 407 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 117 insertions(+))

modular-magician · 2022-04-14T01:31:19Z

Tests analytics

Total tests: 1985
Passed tests 1740
Skipped tests: 240
Failed tests: 5

Action taken

Triggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccServiceNetworkingPeeredDNSDomain_basic|TestAccCloudDeployTarget_update|TestAccCloudDeployTarget_cloudDeployTargetExample|TestAccCloudDeployDeliveryPipeline_cloudDeployDeliveryPipelineExample|TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic

modular-magician · 2022-04-14T01:42:45Z

Tests passed during RECORDING mode:
TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic[view]
TestAccServiceNetworkingPeeredDNSDomain_basic[view]

Tests failed during RECORDING mode:
TestAccCloudDeployTarget_update[view]
TestAccCloudDeployTarget_cloudDeployTargetExample[view]
TestAccCloudDeployDeliveryPipeline_cloudDeployDeliveryPipelineExample[view]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-04-14T10:01:16Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 13 files changed, 2237 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 13 files changed, 2237 insertions(+), 2 deletions(-))
TF Validator: Diff ( 5 files changed, 407 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 117 insertions(+))

modular-magician · 2022-04-14T10:44:12Z

Tests analytics

Total tests: 1986
Passed tests 1740
Skipped tests: 240
Failed tests: 6

Action taken

Triggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccFirebaserulesRelease_BasicRelease|TestAccComputeForwardingRule_update|TestAccCloudDeployTarget_update|TestAccCloudDeployTarget_cloudDeployTargetFullExample|TestAccCloudDeployTarget_cloudDeployTargetExample|TestAccCloudDeployDeliveryPipeline_cloudDeployDeliveryPipelineExample

modular-magician · 2022-04-14T10:45:52Z

Tests passed during RECORDING mode:
TestAccComputeForwardingRule_update[view]
TestAccFirebaserulesRelease_BasicRelease[view]

Tests failed during RECORDING mode:
TestAccCloudDeployDeliveryPipeline_cloudDeployDeliveryPipelineExample[view]
TestAccCloudDeployTarget_cloudDeployTargetFullExample[view]
TestAccCloudDeployTarget_update[view]
TestAccCloudDeployTarget_cloudDeployTargetExample[view]

Please fix these to complete your PR
View the build log or the debug log for each test

slevenick · 2022-04-14T17:42:36Z

/gcbrun

API should be enabled this time!

modular-magician · 2022-04-14T17:56:48Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 13 files changed, 2237 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 13 files changed, 2237 insertions(+), 2 deletions(-))
TF Validator: Diff ( 5 files changed, 407 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 117 insertions(+))

modular-magician · 2022-04-14T18:41:52Z

Tests analytics

Total tests: 1986
Passed tests 1740
Skipped tests: 240
Failed tests: 6

Action taken

Triggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccServiceNetworkingPeeredDNSDomain_basic|TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic|TestAccCloudDeployDeliveryPipeline_cloudDeployDeliveryPipelineExample|TestAccCloudDeployTarget_update|TestAccCloudDeployTarget_cloudDeployTargetFullExample|TestAccCloudDeployTarget_cloudDeployTargetExample

modular-magician · 2022-04-14T18:54:54Z

Tests passed during RECORDING mode:
TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic[view]
TestAccCloudDeployTarget_update[view]
TestAccServiceNetworkingPeeredDNSDomain_basic[view]
TestAccCloudDeployTarget_cloudDeployTargetExample[view]
TestAccCloudDeployTarget_cloudDeployTargetFullExample[view]
TestAccCloudDeployDeliveryPipeline_cloudDeployDeliveryPipelineExample[view]

All tests passed
View the build log or the debug log for each test

slevenick

Thanks!

upodroid · 2022-04-14T21:24:21Z

Thanks!

No problem, I like the new CI process. Did the team work out how to give "trusted" community contributors permissions to view that bucket?

slevenick · 2022-04-14T21:28:20Z

Thanks!

No problem, I like the new CI process. Did the team work out how to give "trusted" community contributors permissions to view that bucket?

Unfortunately not yet, but the feedback process should be much faster now!

This reverts commit e379f06.

)" (GoogleCloudPlatform#5968) This reverts commit e379f06.

* add cloud deploy resources Co-authored-by: upodroid <cy@borg.dev> * add new test * apply suggestions from sam * remove custom flattener * fix import id, add new tests and fix bad rebase

)" (GoogleCloudPlatform#5968) This reverts commit e379f06.

modular-magician requested a review from slevenick February 14, 2022 17:21

slevenick reviewed Apr 6, 2022

View reviewed changes

upodroid added 3 commits April 13, 2022 20:58

add cloud deploy resources

2b34526

Co-authored-by: upodroid <cy@borg.dev>

add new test

617daea

apply suggestions from sam

c67424c

upodroid force-pushed the cloud-deploy-resources branch from 9968530 to c67424c Compare April 13, 2022 23:13

remove custom flattener

f53e541

slevenick reviewed Apr 14, 2022

View reviewed changes

fix import id, add new tests and fix bad rebase

d8e431f

slevenick approved these changes Apr 14, 2022

View reviewed changes

slevenick merged commit e379f06 into GoogleCloudPlatform:main Apr 14, 2022

modular-magician mentioned this pull request Apr 14, 2022

Add support for Cloud Deploy resources terraform-google-modules/docs-examples#212

Merged

5 tasks

This was referenced Apr 14, 2022

Add support for Cloud Deploy resources hashicorp/terraform-provider-google#11513

Merged

Add support for Cloud Deploy resources hashicorp/terraform-provider-google-beta#4225

Merged

slevenick added a commit that referenced this pull request Apr 22, 2022

Revert "Add support for Cloud Deploy resources (#5710)"

7575c69

This reverts commit e379f06.

slevenick mentioned this pull request Apr 22, 2022

Revert "Add support for Cloud Deploy resources" #5968

Merged

slevenick added a commit that referenced this pull request Apr 22, 2022

Revert "Add support for Cloud Deploy resources (#5710)" (#5968)

4850507

This reverts commit e379f06.

This was referenced Apr 22, 2022

Revert "Add support for Cloud Deploy resources" hashicorp/terraform-provider-google-beta#4248

Merged

Revert "Add support for Cloud Deploy resources" hashicorp/terraform-provider-google#11568

Merged

saqibkhanspeaks12 pushed a commit to saqibkhanspeaks12/magic-modules that referenced this pull request Apr 25, 2022

Revert "Add support for Cloud Deploy resources (GoogleCloudPlatform#5710

5fb1acd

)" (GoogleCloudPlatform#5968) This reverts commit e379f06.

betsy-lichtenberg pushed a commit to betsy-lichtenberg/magic-modules that referenced this pull request Apr 25, 2022

Revert "Add support for Cloud Deploy resources (GoogleCloudPlatform#5710

fe276a7

)" (GoogleCloudPlatform#5968) This reverts commit e379f06.

modular-magician mentioned this pull request Apr 26, 2022

Revert "Add support for Cloud Deploy resources" GoogleCloudPlatform/terraform-validator#662

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Cloud Deploy resources #5710

Add support for Cloud Deploy resources #5710

upodroid commented Feb 14, 2022

modular-magician commented Feb 14, 2022

modular-magician commented Feb 14, 2022

upodroid commented Mar 23, 2022

slevenick Apr 6, 2022

slevenick Apr 14, 2022

slevenick Apr 6, 2022

upodroid Apr 12, 2022

upodroid Apr 12, 2022 •

edited

Loading

slevenick Apr 14, 2022

upodroid commented Apr 13, 2022

modular-magician commented Apr 13, 2022

modular-magician commented Apr 14, 2022

modular-magician commented Apr 14, 2022

slevenick Apr 14, 2022

slevenick Apr 14, 2022

slevenick Apr 14, 2022

slevenick commented Apr 14, 2022

modular-magician commented Apr 14, 2022

modular-magician commented Apr 14, 2022

modular-magician commented Apr 14, 2022

modular-magician commented Apr 14, 2022

modular-magician commented Apr 14, 2022

modular-magician commented Apr 14, 2022

slevenick commented Apr 14, 2022

modular-magician commented Apr 14, 2022

modular-magician commented Apr 14, 2022

modular-magician commented Apr 14, 2022

slevenick left a comment

upodroid commented Apr 14, 2022

slevenick commented Apr 14, 2022

Add support for Cloud Deploy resources #5710

Add support for Cloud Deploy resources #5710

Conversation

upodroid commented Feb 14, 2022

modular-magician commented Feb 14, 2022

modular-magician commented Feb 14, 2022

Diff report:

upodroid commented Mar 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

upodroid Apr 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

upodroid commented Apr 13, 2022

modular-magician commented Apr 13, 2022

Diff report:

modular-magician commented Apr 14, 2022

Tests analytics

Action taken

modular-magician commented Apr 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slevenick commented Apr 14, 2022

modular-magician commented Apr 14, 2022

Diff report:

modular-magician commented Apr 14, 2022

Tests analytics

Action taken

modular-magician commented Apr 14, 2022

modular-magician commented Apr 14, 2022

Diff report:

modular-magician commented Apr 14, 2022

Tests analytics

Action taken

modular-magician commented Apr 14, 2022

slevenick commented Apr 14, 2022

modular-magician commented Apr 14, 2022

Diff report:

modular-magician commented Apr 14, 2022

Tests analytics

Action taken

modular-magician commented Apr 14, 2022

slevenick left a comment

Choose a reason for hiding this comment

upodroid commented Apr 14, 2022

slevenick commented Apr 14, 2022

upodroid Apr 12, 2022 •

edited

Loading