Adds resource BitbucketServerConfig to CloudBuild. #7083

mariomachado94 · 2023-01-05T18:51:14Z

Adds resource BitbucketServerConfig to CloudBuild.

Custom code is added to support connected_connected repositories field via separate API calls

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_cloudbuild_bitbucket_server_config`

modular-magician · 2023-01-05T18:52:15Z

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

I've detected that you're a community contributor. @trodge, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

Ensuring it's backwards compatible, covers common error cases, etc.
Summarizing the change into a user-facing changelog note.
Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.

modular-magician · 2023-01-05T19:12:21Z

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 ( 6 files changed, 1363 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1363 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 211 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 113 insertions(+))

modular-magician · 2023-01-05T20:01:21Z

Tests analytics

Total tests: 2391
Passed tests 2137
Skipped tests: 248
Failed tests: 6

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccDatastreamStream_datastreamStreamFullExample|TestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccDatastreamStream_datastreamStreamBasicExample|TestAccDatastreamStream_update|TestAccCloudBuildBitbucketServerConfig_cloudbuildBitbucketServerConfigExample

modular-magician · 2023-01-05T20:24:53Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccDatastreamStream_datastreamStreamFullExample[Debug log]
TestAccContainerCluster_withInvalidGatewayApiConfigChannel[Debug log]
TestAccDatastreamStream_datastreamStreamBasicExample[Debug log]
TestAccDatastreamStream_update[Debug log]
TestAccCloudBuildBitbucketServerConfig_cloudbuildBitbucketServerConfigExample[Debug log]

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

mmv1/products/cloudbuild/api.yaml

modular-magician · 2023-01-25T21:24:47Z

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 ( 6 files changed, 1416 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1416 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 211 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 113 insertions(+))

modular-magician · 2023-01-25T22:11:29Z

Tests analytics

Total tests: 2430
Passed tests 2175
Skipped tests: 254
Failed tests: 1

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease

modular-magician · 2023-01-25T22:12:00Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

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

modular-magician · 2023-01-26T20:10:47Z

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 ( 6 files changed, 1503 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1503 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 211 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 264 insertions(+))

modular-magician · 2023-01-26T20:52:56Z

Tests analytics

Total tests: 2432
Passed tests 2175
Skipped tests: 254
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccRegionInstanceGroupManager_stateful|TestAccContainerCluster_failedCreation|TestAccCloudBuildBitbucketServerConfig_cloudbuildBitbucketServerConfigPeeredNetworkExample

modular-magician · 2023-01-26T21:08:05Z

Tests passed during RECORDING mode:
TestAccContainerCluster_failedCreation[Debug log]
TestAccCloudBuildBitbucketServerConfig_cloudbuildBitbucketServerConfigPeeredNetworkExample[Debug log]

Tests failed during RECORDING mode:
TestAccRegionInstanceGroupManager_stateful[Error message] [Debug log]

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

trodge

LGTM, adding Shuya for a second review.

modular-magician · 2023-02-06T17:42: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 ( 6 files changed, 1503 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1503 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 211 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 264 insertions(+))

modular-magician · 2023-02-06T18:23:53Z

Tests analytics

Total tests: 2455
Passed tests 2196
Skipped tests: 255
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFrameworkProviderMeta_setModuleName|TestAccRegionInstanceGroupManager_stateful|TestAccDataSourceDnsRecordSet_basic|TestAccApigeeAddonsConfig_apigeeAddonsTestExample

modular-magician · 2023-02-06T18:35:05Z

Tests passed during RECORDING mode:
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]
TestAccApigeeAddonsConfig_apigeeAddonsTestExample[Debug log]

Tests failed during RECORDING mode:
TestAccRegionInstanceGroupManager_stateful[Error message] [Debug log]

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

shuyama1 · 2023-02-06T17:30:23Z

mmv1/products/cloudbuild/api.yaml

+            description: |
+              Immutable. The resource name for the webhook secret's secret version. Once this field has been set, it cannot be changed.
+              If you need to change it, please create another BitbucketServerConfig.
+      - !ruby/object:Api::Type::Time


We can omit the fields that may not be useful for Terraform users

Okay, removing createTime.

shuyama1 · 2023-02-07T15:33:09Z

mmv1/products/cloudbuild/api.yaml

+          name: 'BitbucketServerRepositoryId'
+          description: |
+            BitbucketServerRepositoryId identifies a specific repository hosted on a Bitbucket Server.


I think we can remove these two lines. Don't think they will get generated.

Ah, yes, you are right. I assume you mean name as well? It looks like name and description are not generated. Removing both.

shuyama1 · 2023-02-07T15:38:16Z

mmv1/products/cloudbuild/api.yaml

+          Immutable. API Key that will be attached to webhook. Once this field has been set, it cannot be changed.
+          If you need to change it, please create another BitbucketServerConfig.
+      - !ruby/object:Api::Type::Array
+        name: 'connectedRepositories'


I understand that we probably not able to add tests for this field in our CI. Got a quick question regarding your local test:

Have you tested the update functionality of this field?

I did. I had to be sure to add custom code in the preupdate to remove this field from the field mask, as well as to then update it with custom code. Then made sure to test edge cases, 0 -> 1, many as well as update many, 1 -> 0, as well as a few other permutations.

shuyama1 · 2023-02-07T15:53:27Z

mmv1/products/cloudbuild/api.yaml

+        description: |
+          Time when the config was created.
+      - !ruby/object:Api::Type::String
+        name: 'username'


Just want to confirm the behavior of this field. If we don't specify this field explicitly, will the API return a value for this field?

No, it needs this field from the user, it will return an error if not specified (it is the bitbucket server user name to be used by our api).

Sorry, I missed required: true setting somehow and thought it was an optional field, and that's why I asked the question. Thanks for confirming though!

shuyama1 · 2023-02-07T15:57:55Z

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

+terraform {
+  required_providers {
+    google = {}
+  }
+}
+
+provider "google" {}


any reason we need to set these explicitly?

Nope. Good catch, removing them.

shuyama1 · 2023-02-08T18:19:24Z

/gcbrun

modular-magician · 2023-02-08T18:35:36Z

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 ( 6 files changed, 1472 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1472 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 211 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 256 insertions(+))

modular-magician · 2023-02-08T19:18:04Z

Tests analytics

Total tests: 2465
Passed tests 2204
Skipped tests: 256
Failed tests: 5

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccApigeeAddonsConfig_apigeeAddonsTestExample|TestAccFrameworkProviderMeta_setModuleName|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDnsManagedZone_basic

modular-magician · 2023-02-08T19:29:20Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccApigeeAddonsConfig_apigeeAddonsTestExample[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]

Tests failed during RECORDING mode:
TestAccDataSourceDnsManagedZone_basic[Error message] [Debug log]

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

shuyama1

Only some nitpicks and questions. Sorry for missing some of these in the previous review.

mmv1/products/cloudbuild/api.yaml

mmv1/products/cloudbuild/terraform.yaml

mmv1/templates/terraform/pre_update/cloudbuild_bitbucketserver_config.go.erb

shuyama1 · 2023-02-13T18:23:49Z

/gcbrun

modular-magician · 2023-02-13T18:41:11Z

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 ( 6 files changed, 1472 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1472 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 211 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 256 insertions(+))

modular-magician · 2023-02-13T19:23:18Z

Tests analytics

Total tests: 2470
Passed tests 2210
Skipped tests: 256
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFrameworkProviderMeta_setModuleName|TestAccApigeeAddonsConfig_apigeeAddonsTestExample|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDnsManagedZone_basic

modular-magician · 2023-02-13T19:33:54Z

Tests passed during RECORDING mode:
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccApigeeAddonsConfig_apigeeAddonsTestExample[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]

Tests failed during RECORDING mode:
TestAccDataSourceDnsManagedZone_basic[Error message] [Debug log]

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

…eing used

…ketServerConfig

Co-authored-by: Shuya Ma <[email protected]>

shuyama1 · 2023-02-13T20:45:30Z

/gcbrun

modular-magician · 2023-02-13T21:01:55Z

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 ( 6 files changed, 1472 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1472 insertions(+), 2 deletions(-))
TF Validator: Diff ( 3 files changed, 211 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 256 insertions(+))

modular-magician · 2023-02-13T21:43:41Z

Tests analytics

Total tests: 2472
Passed tests 2211
Skipped tests: 256
Failed tests: 5

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccApigeeAddonsConfig_apigeeAddonsTestExample|TestAccFrameworkProviderMeta_setModuleName|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDnsManagedZone_basic

modular-magician · 2023-02-13T21:54:34Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccApigeeAddonsConfig_apigeeAddonsTestExample[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]

Tests failed during RECORDING mode:
TestAccDataSourceDnsManagedZone_basic[Error message] [Debug log]

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

shuyama1

Thank you for the work!

shuyama1 · 2023-02-13T22:06:35Z

Back to @trodge for final approval!

…rm#7083) * Adds resource BitbucketServerConfig to CloudBuild. * Adds example bitbucket config with peered_network and ssl_ca fields being used * Minor code refactor. Removing unnecessary code for Cloud Build BitbucketServerConfig * Update doc description mmv1/products/cloudbuild/api.yaml Co-authored-by: Shuya Ma <[email protected]> * Update doc description mmv1/products/cloudbuild/api.yaml Co-authored-by: Shuya Ma <[email protected]> --------- Co-authored-by: Mario Machado <[email protected]> Co-authored-by: Shuya Ma <[email protected]>

modular-magician requested a review from trodge January 5, 2023 18:52

mariomachado94 force-pushed the bitbucket branch from 3aca578 to f67f864 Compare January 5, 2023 18:55

trodge requested changes Jan 5, 2023

View reviewed changes

mmv1/products/cloudbuild/api.yaml Show resolved Hide resolved

modular-magician requested a review from trodge January 25, 2023 21:08

mariomachado94 force-pushed the bitbucket branch from cfc608b to 9bd4e1b Compare January 26, 2023 19:54

trodge reviewed Jan 26, 2023

View reviewed changes

trodge requested review from megan07 and shuyama1 and removed request for megan07 January 26, 2023 23:29

mariomachado94 force-pushed the bitbucket branch from 9bd4e1b to a166d74 Compare February 6, 2023 17:26

modular-magician requested a review from trodge February 6, 2023 17:27

shuyama1 reviewed Feb 7, 2023

View reviewed changes

mariomachado94 force-pushed the bitbucket branch from 2828778 to 76ff0c5 Compare February 8, 2023 18:15

modular-magician requested a review from shuyama1 February 8, 2023 18:20

shuyama1 reviewed Feb 13, 2023

View reviewed changes

mariomachado94 requested a review from a team as a code owner February 13, 2023 18:21

modular-magician requested a review from shuyama1 February 13, 2023 18:24

Mario Machado and others added 5 commits February 13, 2023 15:41

Adds resource BitbucketServerConfig to CloudBuild.

8bc9469

Adds example bitbucket config with peered_network and ssl_ca fields b…

d58b7b9

…eing used

Minor code refactor. Removing unnecessary code for Cloud Build Bitbuc…

feda46b

…ketServerConfig

Update doc description mmv1/products/cloudbuild/api.yaml

48e8427

Co-authored-by: Shuya Ma <[email protected]>

Update doc description mmv1/products/cloudbuild/api.yaml

e9c40e1

Co-authored-by: Shuya Ma <[email protected]>

mariomachado94 force-pushed the bitbucket branch from c6fc435 to e9c40e1 Compare February 13, 2023 20:43

shuyama1 approved these changes Feb 13, 2023

View reviewed changes

trodge approved these changes Feb 16, 2023

View reviewed changes

trodge merged commit 9fb5a77 into GoogleCloudPlatform:main Feb 16, 2023

Adds resource BitbucketServerConfig to CloudBuild. #7083

Adds resource BitbucketServerConfig to CloudBuild. #7083

Conversation

mariomachado94 commented Jan 5, 2023

modular-magician commented Jan 5, 2023

modular-magician commented Jan 5, 2023

Diff report

modular-magician commented Jan 5, 2023

Tests analytics

Action taken

modular-magician commented Jan 5, 2023

modular-magician commented Jan 25, 2023

Diff report

modular-magician commented Jan 25, 2023

Tests analytics

Action taken

modular-magician commented Jan 25, 2023

modular-magician commented Jan 26, 2023

Diff report

modular-magician commented Jan 26, 2023

Tests analytics

Action taken

modular-magician commented Jan 26, 2023

trodge left a comment • edited Loading

Choose a reason for hiding this comment

modular-magician commented Feb 6, 2023

Diff report

modular-magician commented Feb 6, 2023

Tests analytics

Action taken

modular-magician commented Feb 6, 2023

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariomachado94 Feb 8, 2023 • edited Loading

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

shuyama1 commented Feb 8, 2023

modular-magician commented Feb 8, 2023

Diff report

modular-magician commented Feb 8, 2023

Tests analytics

Action taken

modular-magician commented Feb 8, 2023

shuyama1 left a comment

Choose a reason for hiding this comment

shuyama1 commented Feb 13, 2023

modular-magician commented Feb 13, 2023

Diff report

modular-magician commented Feb 13, 2023

Tests analytics

Action taken

modular-magician commented Feb 13, 2023

shuyama1 commented Feb 13, 2023

modular-magician commented Feb 13, 2023

Diff report

modular-magician commented Feb 13, 2023

Tests analytics

Action taken

modular-magician commented Feb 13, 2023

shuyama1 left a comment

Choose a reason for hiding this comment

shuyama1 commented Feb 13, 2023

trodge left a comment •

edited

Loading

mariomachado94 Feb 8, 2023 •

edited

Loading