Keystores aliases key cert file by 5fff · Pull Request #7518 · GoogleCloudPlatform/magic-modules

5fff · 2023-03-23T01:11:41Z

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).
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).
Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
Ran relevant acceptance tests using my own Google Cloud project and credentials (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)

Apigee: added resource `apigeeKeystoresAliasesKeyCertFile`

hao-nan-li · 2023-03-23T16:06:08Z

/gcbrun

modular-magician · 2023-03-23T16:22:04Z

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 ( 8 files changed, 1348 insertions(+), 75 deletions(-))
Terraform Beta: Diff ( 8 files changed, 1353 insertions(+), 80 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-03-23T17:08:58Z

Tests analytics

Total tests: 2522
Passed tests 2253
Skipped tests: 264
Affected tests: 5

Action taken

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

TestAccFirebaserulesRelease_BasicRelease|TestAccDataSourceGoogleCloudAssetResourcesSearchAll_basic|TestAccDataSourceDnsManagedZone_basic|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccNetworkServicesGateway_update

Get to know how VCR tests work

modular-magician · 2023-03-23T17:22:42Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccDataSourceGoogleCloudAssetResourcesSearchAll_basic[Debug log]
TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample[Debug log]
TestAccNetworkServicesGateway_update[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

hao-nan-li

Quick comments before starting to review:

New resources are recommended to be created through mmv1 generated resources, is there any particular reason you made this handwritten?
Looks like there are some lint problems, could you run gofmt on the created files? Sometimes there are lint problems due to missing addtional empty line at the end of the file.

5fff · 2023-03-23T18:56:35Z

Quick comments before starting to review:

New resources are recommended to be created through mmv1 generated resources, is there any particular reason you made this handwritten?

Looks like there are some lint problems, could you run gofmt on the created files? Sometimes there are lint problems due to missing addtional empty line at the end of the file.

Hi @hao-nan-li The API for Keystore Aliases takes multipart-form data request, which is not supported by MMV1, Apigee being a acquired product has a different API pattern than in house developed GCP product.

Regarding the lint problem, can you pls let me know which file is having issue? I have indeed ran go fmt before I add the file. Thanks!

hao-nan-li · 2023-03-23T21:57:00Z

Quick comments before starting to review:

New resources are recommended to be created through mmv1 generated resources, is there any particular reason you made this handwritten?

Looks like there are some lint problems, could you run gofmt on the created files? Sometimes there are lint problems due to missing addtional empty line at the end of the file.

Hi @hao-nan-li The API for Keystore Aliases takes multipart-form data request, which is not supported by MMV1, Apigee being a acquired product has a different API pattern than in house developed GCP product.

Regarding the lint problem, can you pls let me know which file is having issue? I have indeed ran go fmt before I add the file. Thanks!

Quick note: By running make lint, I got:

gofmt needs running on the following files:
./google/resource_apigee_keystores_aliases_key_cert_file_generated_test.go
./google/resource_apigee_keystores_aliases_key_cert_file.go

hao-nan-li · 2023-03-23T22:20:56Z

/gcbrun

modular-magician · 2023-03-23T22:36: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.

Terraform GA: Diff ( 8 files changed, 1348 insertions(+), 75 deletions(-))
Terraform Beta: Diff ( 8 files changed, 1353 insertions(+), 80 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-03-23T23:23:09Z

Tests analytics

Total tests: 2524
Passed tests 2255
Skipped tests: 264
Affected tests: 5

Action taken

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

TestAccNetworkServicesGateway_update|TestAccHealthcareFhirStore_healthcareFhirStoreStreamingConfigExample|TestAccFirebaserulesRelease_BasicRelease|TestAccDataSourceDnsManagedZone_basic|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample

Get to know how VCR tests work

modular-magician · 2023-03-23T23:36:28Z

Tests passed during RECORDING mode:
TestAccNetworkServicesGateway_update[Debug log]
TestAccHealthcareFhirStore_healthcareFhirStoreStreamingConfigExample[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample[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

hao-nan-li

Could you do a rebase so that I can run the tests locally?

…rt-file

hao-nan-li

LGTM in general. Just some small concern about the password field and the documentation.

hao-nan-li · 2023-03-27T22:27:24Z

+
+* `certs_info` -
+  Chain of certificates under this alias.
+  Structure is [documented below](#nested_certs_info).


Not a big issue, but this hyperlink is not working as intended.

hao-nan-li · 2023-03-27T22:27:36Z

+* `cert_info` -
+  (Output)
+  List of all properties in the object.
+  Structure is [documented below](#nested_cert_info).


Same problem here.

hao-nan-li · 2023-03-28T16:46:20Z

+				Optional:    true,
+				Description: `Private Key content, omit if uploading to truststore`,
+			},
+			"password": {


If there's a password field, I think it's necessary to make this field sensitive. Sensitive: true

hao-nan-li · 2023-03-28T16:48:34Z

/gcbrun

modular-magician · 2023-03-28T17:05:26Z

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 ( 8 files changed, 1349 insertions(+), 76 deletions(-))
Terraform Beta: Diff ( 8 files changed, 1354 insertions(+), 81 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-03-28T17:58:55Z

Tests analytics

Total tests: 2570
Passed tests 2291
Skipped tests: 267
Affected tests: 12

Action taken

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

TestAccNetworkServicesGateway_update|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstanceFromRegionTemplate_basic|TestAccComposerEnvironment_withEncryptionConfigComposer2|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccDataSourceGoogleFirebaseAppleAppConfig|TestAccFrameworkProviderBasePath_setBasePath|TestAccFrameworkProviderMeta_setModuleName|TestAccDataSourceDNSKeys_basic|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDNSKeys_noDnsSec

Get to know how VCR tests work

modular-magician · 2023-03-28T18:14:36Z

Tests passed during RECORDING mode:
TestAccNetworkServicesGateway_update[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccComputeInstanceFromRegionTemplate_basic[Debug log]
TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample[Debug log]
TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log]
TestAccFrameworkProviderBasePath_setBasePath[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccDataSourceDNSKeys_basic[Debug log]
TestAccDataSourceDnsManagedZone_basic[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]
TestAccDataSourceDNSKeys_noDnsSec[Debug log]

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

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

make password and key sensitive field

hao-nan-li · 2023-03-28T20:04:17Z

/gcbrun

modular-magician · 2023-03-28T20:19: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.

Terraform GA: Diff ( 8 files changed, 1351 insertions(+), 76 deletions(-))
Terraform Beta: Diff ( 8 files changed, 1356 insertions(+), 81 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-03-28T21:11:50Z

Tests analytics

Total tests: 2570
Passed tests 2293
Skipped tests: 267
Affected tests: 10

Action taken

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

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstanceFromRegionTemplate_basic|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccFrameworkProviderMeta_setModuleName|TestAccFrameworkProviderBasePath_setBasePath|TestAccDataSourceDNSKeys_noDnsSec|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDNSKeys_basic|TestAccDataSourceGoogleFirebaseAppleAppConfig

Get to know how VCR tests work

modular-magician · 2023-03-28T21:25:36Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccComputeInstanceFromRegionTemplate_basic[Debug log]
TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccFrameworkProviderBasePath_setBasePath[Debug log]
TestAccDataSourceDNSKeys_noDnsSec[Debug log]
TestAccDataSourceDnsManagedZone_basic[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]
TestAccDataSourceDNSKeys_basic[Debug log]
TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log]

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

hao-nan-li

LGTM, could you change the release note to follow this format?
https://github.com/GoogleCloudPlatform/magic-modules/blob/main/.ci/RELEASE_NOTES_GUIDE.md

* add apigee keystores aliases keystorefile provider * add doc * update doc * update linted files * Update resource_apigee_keystores_aliases_key_cert_file.go make password and key sensitive field --------- Co-authored-by: Ray Xu <xrc@google.com>

Ray Xu added 3 commits March 23, 2023 00:49

add apigee keystores aliases keystorefile provider

fd5b6ed

add doc

c1eebe3

update doc

6f0755a

5fff requested a review from a team as a code owner March 23, 2023 01:11

5fff requested review from hao-nan-li and removed request for a team March 23, 2023 01:11

hao-nan-li reviewed Mar 23, 2023

View reviewed changes

update linted files

080b5c8

modular-magician requested a review from hao-nan-li March 23, 2023 22:22

hao-nan-li reviewed Mar 24, 2023

View reviewed changes

Comment thread mmv1/third_party/terraform/resources/resource_apigee_keystores_aliases_key_cert_file.go

Comment thread mmv1/third_party/terraform/resources/resource_apigee_keystores_aliases_key_cert_file.go

Merge branch 'GoogleCloudPlatform:main' into keystores-aliases-key-ce…

12bd97b

…rt-file

hao-nan-li reviewed Mar 28, 2023

View reviewed changes

modular-magician requested a review from hao-nan-li March 28, 2023 16:49

Update resource_apigee_keystores_aliases_key_cert_file.go

efc4494

make password and key sensitive field

hao-nan-li approved these changes Mar 28, 2023

View reviewed changes

hao-nan-li merged commit 1ab0354 into GoogleCloudPlatform:main Mar 28, 2023

This was referenced Mar 28, 2023

Keystores aliases key cert file hashicorp/terraform-provider-google-beta#5386

Merged

Keystores aliases key cert file hashicorp/terraform-provider-google#14130

Merged

Keystores aliases key cert file GoogleCloudPlatform/terraform-validator#1502

Merged

rileykarson mentioned this pull request Apr 7, 2023

Update apigee keystore resource names to be consistent #7665

Merged

5 tasks

Uh oh!

Conversation

5fff commented Mar 23, 2023 • edited by hao-nan-li Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hao-nan-li commented Mar 23, 2023

Uh oh!

modular-magician commented Mar 23, 2023

Diff report

Uh oh!

modular-magician commented Mar 23, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Mar 23, 2023

Uh oh!

hao-nan-li left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

5fff commented Mar 23, 2023

Uh oh!

hao-nan-li commented Mar 23, 2023

Uh oh!

hao-nan-li commented Mar 23, 2023

Uh oh!

modular-magician commented Mar 23, 2023

Diff report

Uh oh!

modular-magician commented Mar 23, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Mar 23, 2023

Uh oh!

hao-nan-li left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hao-nan-li left a comment

Choose a reason for hiding this comment

Uh oh!

hao-nan-li Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

hao-nan-li Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

hao-nan-li Mar 28, 2023

Choose a reason for hiding this comment

Uh oh!

hao-nan-li commented Mar 28, 2023

Uh oh!

modular-magician commented Mar 28, 2023

Diff report

Uh oh!

modular-magician commented Mar 28, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Mar 28, 2023

Uh oh!

hao-nan-li commented Mar 28, 2023

Uh oh!

modular-magician commented Mar 28, 2023

Diff report

Uh oh!

modular-magician commented Mar 28, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Mar 28, 2023

Uh oh!

hao-nan-li left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

5fff commented Mar 23, 2023 •

edited by hao-nan-li

Loading

hao-nan-li left a comment •

edited

Loading