`BigQuery`: support `encryptionConfiguration` in `google_bigquery_data_transfer_config` #11478

BBBmau · 2024-08-17T02:13:23Z

This PR would support the encryptionConfiguration field which is missing.

Related REST API Reference of encryptionConfiguration: https://cloud.google.com/bigquery/docs/reference/datatransfer/rest/v1/projects.locations.transferConfigs#encryptionconfiguration

Release Note Template for Downstream PRs (will be copied)

bigquery: added `encryption_configuration` field to `google_bigquery_data_transfer_config` resource

modular-magician · 2024-08-17T02:29:38Z

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.

google provider: Diff ( 2 files changed, 136 insertions(+))
google-beta provider: Diff ( 2 files changed, 136 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 29 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (13 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  encryption_configuration {
    kms_key_name = # value needed
  }
}

modular-magician · 2024-08-17T02:33:51Z

Tests analytics

Total tests: 1
Passed tests: 1
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

bigquerydatatransfer

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

wj-chen · 2024-08-20T03:24:39Z

mmv1/products/bigquerydatatransfer/Config.yaml

@@ -172,6 +181,18 @@ properties:
      reingests data for [today-10, today-1], rather than ingesting data for
      just [today-1]. Only valid if the data source supports the feature.
      Set the value to 0 to use the default value.
+  - !ruby/object:Api::Type::NestedObject
+    name: 'encryptionConfiguration'
+    description: |


To be future-proof, can we make this description more generic and not specific about encryption key?

updated description to match that found in API Reference.

According to https://cloud.google.com/bigquery/docs/reference/datatransfer/rest/v1/projects.locations.transferConfigs#encryptionconfiguration - "The name of the KMS key used for encrypting BigQuery data." is the API description of kmsKeyName. Description of EncryptionConfiguration is "Represents the encryption configuration for a transfer."

apologies seems like i got the descriptions mixed up, should be correct now.

modular-magician · 2024-08-22T01:00:53Z

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.

google provider: Diff ( 2 files changed, 132 insertions(+))
google-beta provider: Diff ( 2 files changed, 132 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 29 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (12 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  encryption_configuration {
    kms_key_name = # value needed
  }
}

modular-magician · 2024-08-22T01:05:00Z

Tests analytics

Total tests: 1
Passed tests: 1
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

bigquerydatatransfer

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

BBBmau · 2024-08-22T04:12:06Z

marking as ready for review with a reproducible tfconfig for manually testing this:

when applying the tfconfig below:

data "google_project" "project" {
}

resource "google_kms_crypto_key" "mau_crypto_key" {
  name = "mau-crypto-key-test-bq"
  key_ring = "projects/hc-terraform-testing/locations/us-central1/keyRings/mau-latest-test-1"
  purpose = "ENCRYPT_DECRYPT"
}

data "google_iam_policy" "owner" {
  binding {
    role = "roles/bigquery.dataOwner"

    members = [
      "serviceAccount:mauricio-alvarezleon@hc-terraform-testing.iam.gserviceaccount.com",
    ]
  }
}

resource "google_bigquery_dataset_iam_policy" "dataset" {
  dataset_id  = google_bigquery_dataset.my_dataset.dataset_id
  policy_data = data.google_iam_policy.owner.policy_data
}

resource "google_bigquery_data_transfer_config" "query_config" {
  depends_on = [ google_kms_crypto_key.mau_crypto_key ]
  encryption_configuration {
    kms_key_name = "projects/hc-terraform-testing/locations/us-central1/keyRings/mau-latest-test/cryptoKeys/mau-crypto-key-test-bq"
  }
  display_name           = "my-query-mau"
  location               = "us-central1"
  data_source_id         = "scheduled_query"
  schedule               = "first sunday of quarter 00:00"
  destination_dataset_id = google_bigquery_dataset.my_dataset.dataset_id
  params = {
    destination_table_name_template = "my_table"
    write_disposition               = "WRITE_APPEND"
    query                           = "SELECT name FROM table WHERE x = 'y'"
  }
}

resource "google_bigquery_dataset" "my_dataset" {
  dataset_id    = "my_dataset"
  friendly_name = "foo"
  description   = "bar"
  location      = "us-central1"
}

We get a successfull apply with the following showing up for the bigquery_transfer_config, you'll notice that the encryption key has been successfully applied.

Let me know if anything else is needed to make this review easier. @wj-chen

wj-chen · 2024-08-22T22:25:13Z

marking as ready for review with a reproducible tfconfig for manually testing this:

when applying the tfconfig below:

data "google_project" "project" {
}

resource "google_kms_crypto_key" "mau_crypto_key" {
  name = "mau-crypto-key-test-bq"
  key_ring = "projects/hc-terraform-testing/locations/us-central1/keyRings/mau-latest-test-1"
  purpose = "ENCRYPT_DECRYPT"
}

data "google_iam_policy" "owner" {
  binding {
    role = "roles/bigquery.dataOwner"

    members = [
      "serviceAccount:mauricio-alvarezleon@hc-terraform-testing.iam.gserviceaccount.com",
    ]
  }
}

resource "google_bigquery_dataset_iam_policy" "dataset" {
  dataset_id  = google_bigquery_dataset.my_dataset.dataset_id
  policy_data = data.google_iam_policy.owner.policy_data
}

resource "google_bigquery_data_transfer_config" "query_config" {
  depends_on = [ google_kms_crypto_key.mau_crypto_key ]
  encryption_configuration {
    kms_key_name = "projects/hc-terraform-testing/locations/us-central1/keyRings/mau-latest-test/cryptoKeys/mau-crypto-key-test-bq"
  }
  display_name           = "my-query-mau"
  location               = "us-central1"
  data_source_id         = "scheduled_query"
  schedule               = "first sunday of quarter 00:00"
  destination_dataset_id = google_bigquery_dataset.my_dataset.dataset_id
  params = {
    destination_table_name_template = "my_table"
    write_disposition               = "WRITE_APPEND"
    query                           = "SELECT name FROM table WHERE x = 'y'"
  }
}

resource "google_bigquery_dataset" "my_dataset" {
  dataset_id    = "my_dataset"
  friendly_name = "foo"
  description   = "bar"
  location      = "us-central1"
}

We get a successfull apply with the following showing up for the bigquery_transfer_config, you'll notice that the encryption key has been successfully applied.

Let me know if anything else is needed to make this review easier. @wj-chen

Super helpful, thank you for the test notes!

modular-magician · 2024-08-23T00:22:14Z

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.

google provider: Diff ( 2 files changed, 132 insertions(+))
google-beta provider: Diff ( 2 files changed, 132 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 29 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (12 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  encryption_configuration {
    kms_key_name = # value needed
  }
}

modular-magician · 2024-08-23T00:26:10Z

Tests analytics

Total tests: 1
Passed tests: 1
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

bigquerydatatransfer

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

hao-nan-li

Thanks for adding this field.
The Terraform google conversion error is unrelated to this change.

hao-nan-li · 2024-08-23T15:57:21Z

mmv1/products/bigquerydatatransfer/Config.yaml

@@ -46,13 +46,22 @@ examples:
    vars:
      display_name: 'my-query'
      dataset_id: 'my_dataset'
+  - !ruby/object:Provider::Terraform::Examples
+    name: 'bigquerydatatransfer_config_cmek'
+    skip_test: true


Any particular reason to skip the test? If so it looks like the added field is not covered by any test.

This was based off of another example from bigquery_dataset which includes cmek support but doesn't include a test: #2226

I believe we skip it due to the test including a cryptoKeyRing creation, which we are unable to remove: https://cloud.google.com/kms/docs/reference/rest/v1/projects.locations.keyRings

I've included a test that includes the kmsKeyName field.

modular-magician · 2024-08-23T19:50:40Z

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.

google provider: Diff ( 3 files changed, 226 insertions(+))
google-beta provider: Diff ( 3 files changed, 226 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 29 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (12 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  encryption_configuration {
    kms_key_name = # value needed
  }
}

modular-magician · 2024-08-23T19:53:05Z

Tests analytics

Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

bigquerydatatransfer

$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$

View the build log

modular-magician · 2024-08-23T21:20:07Z

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.

google provider: Diff ( 3 files changed, 223 insertions(+))
google-beta provider: Diff ( 3 files changed, 223 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 29 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (12 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  encryption_configuration {
    kms_key_name = # value needed
  }
}

modular-magician · 2024-08-23T21:24:56Z

Tests analytics

Total tests: 1
Passed tests: 1
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

bigquerydatatransfer

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

hao-nan-li · 2024-08-23T22:04:26Z

...party/terraform/services/bigquerydatatransfer/resource_bigquery_data_transfer_config_test.go

@@ -383,6 +383,29 @@ func testAccBigqueryDataTransferConfig_scheduledQuery_update(t *testing.T) {
 	})
 }

+func testAccBigqueryDataTransferConfig_CMEK(t *testing.T) {


Any reason to skip the VCR run? Also I think this function should be named TestAccBigqueryDataTransferConfig_CMEK

same reason from above #11478 (comment)

I also agree with the naming convention but went with what i committed due to the whole test file matching it.

Should we do the renaming of all the tests here or in a separate PR?

Ah thanks for letting me know. If other tests are created in this way then I'm fine with it.

hao-nan-li · 2024-08-26T22:28:34Z

Do you happen to know why this contributor-membership-checker is failing?

BBBmau · 2024-08-26T23:21:19Z

/gcbrun

BBBmau · 2024-08-26T23:23:38Z

Do you happen to know why this contributor-membership-checker is failing?

@hao-nan-li Seems like it's missing the build, i've reran the build to see if it'll get resolved by itself. Edit: looks like after rerunning the build it appears to be passing 😄

Relevant logs:

request url:  https://api.github.com/repos/GoogleCloudPlatform/magic-modules/issues/11478
request body:  null

response status-code:  200

User is a core contributor
Error: Failed to find pending build for PR 11478
Usage:
  magician membership-checker [flags]

modular-magician · 2024-08-26T23:32:53Z

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.

google provider: Diff ( 3 files changed, 223 insertions(+))
google-beta provider: Diff ( 3 files changed, 223 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 29 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_bigquery_data_transfer_config (12 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_bigquery_data_transfer_config" "primary" {
  encryption_configuration {
    kms_key_name = # value needed
  }
}

modular-magician · 2024-08-26T23:36:19Z

Tests analytics

Total tests: 1
Passed tests: 0
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

bigquerydatatransfer

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccBigqueryDataTransferConfig

Get to know how VCR tests work

modular-magician · 2024-08-26T23:41:05Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccBigqueryDataTransferConfig[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

SarahFrench · 2024-08-28T09:56:52Z

Was the change to mmv1/.ruby-version intentional here?

BBBmau and others added 2 commits August 16, 2024 19:11

add CMEK support in bigquery_transferconfig

1fb1281

Merge branch 'main' into add-encryptionConfig-transferConfig-bigquery

44e7607

modular-magician added the service/bigquery label Aug 17, 2024

modular-magician requested a review from wj-chen August 17, 2024 02:29

wj-chen reviewed Aug 20, 2024

View reviewed changes

update description

4bcf27b

modular-magician requested a review from wj-chen August 22, 2024 01:01

BBBmau marked this pull request as ready for review August 22, 2024 04:12

use appropriate description

c993345

wj-chen approved these changes Aug 23, 2024

View reviewed changes

BBBmau requested a review from hao-nan-li August 23, 2024 01:02

hao-nan-li reviewed Aug 23, 2024

View reviewed changes

add cmek test

50ff7c7

modular-magician requested a review from wj-chen August 23, 2024 19:50

fix cmek test

95e0895

BBBmau requested a review from hao-nan-li August 23, 2024 21:45

hao-nan-li reviewed Aug 23, 2024

View reviewed changes

BBBmau requested a review from hao-nan-li August 26, 2024 20:21

hao-nan-li approved these changes Aug 26, 2024

View reviewed changes

hao-nan-li merged commit 8778ef8 into GoogleCloudPlatform:main Aug 26, 2024
13 of 16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`BigQuery`: support `encryptionConfiguration` in `google_bigquery_data_transfer_config` #11478

`BigQuery`: support `encryptionConfiguration` in `google_bigquery_data_transfer_config` #11478

BBBmau commented Aug 17, 2024 •

edited by hao-nan-li

Loading

modular-magician commented Aug 17, 2024

modular-magician commented Aug 17, 2024

wj-chen Aug 20, 2024

BBBmau Aug 22, 2024

wj-chen Aug 22, 2024

BBBmau Aug 23, 2024

modular-magician commented Aug 22, 2024

modular-magician commented Aug 22, 2024

BBBmau commented Aug 22, 2024

wj-chen commented Aug 22, 2024

modular-magician commented Aug 23, 2024

modular-magician commented Aug 23, 2024

hao-nan-li left a comment

hao-nan-li Aug 23, 2024

BBBmau Aug 23, 2024

modular-magician commented Aug 23, 2024

modular-magician commented Aug 23, 2024

modular-magician commented Aug 23, 2024

modular-magician commented Aug 23, 2024

hao-nan-li Aug 23, 2024

BBBmau Aug 23, 2024

hao-nan-li Aug 26, 2024

hao-nan-li commented Aug 26, 2024

BBBmau commented Aug 26, 2024

BBBmau commented Aug 26, 2024 •

edited

Loading

modular-magician commented Aug 26, 2024

modular-magician commented Aug 26, 2024

modular-magician commented Aug 26, 2024

SarahFrench commented Aug 28, 2024

BigQuery: support encryptionConfiguration in google_bigquery_data_transfer_config #11478

BigQuery: support encryptionConfiguration in google_bigquery_data_transfer_config #11478

Conversation

BBBmau commented Aug 17, 2024 • edited by hao-nan-li Loading

modular-magician commented Aug 17, 2024

Diff report

Missing test report

modular-magician commented Aug 17, 2024

Tests analytics

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

modular-magician commented Aug 22, 2024

Diff report

Missing test report

modular-magician commented Aug 22, 2024

Tests analytics

BBBmau commented Aug 22, 2024

wj-chen commented Aug 22, 2024

modular-magician commented Aug 23, 2024

Diff report

Missing test report

modular-magician commented Aug 23, 2024

Tests analytics

hao-nan-li left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Aug 23, 2024

Diff report

Missing test report

modular-magician commented Aug 23, 2024

Tests analytics

modular-magician commented Aug 23, 2024

Diff report

Missing test report

modular-magician commented Aug 23, 2024

Tests analytics

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hao-nan-li commented Aug 26, 2024

BBBmau commented Aug 26, 2024

BBBmau commented Aug 26, 2024 • edited Loading

modular-magician commented Aug 26, 2024

Diff report

Missing test report

modular-magician commented Aug 26, 2024

Tests analytics

Action taken

modular-magician commented Aug 26, 2024

SarahFrench commented Aug 28, 2024

`BigQuery`: support `encryptionConfiguration` in `google_bigquery_data_transfer_config` #11478

`BigQuery`: support `encryptionConfiguration` in `google_bigquery_data_transfer_config` #11478

BBBmau commented Aug 17, 2024 •

edited by hao-nan-li

Loading

BBBmau commented Aug 26, 2024 •

edited

Loading