magic-modules icon indicating copy to clipboard operation
magic-modules copied to clipboard

Use data google_kms(_key_ring | _crypto_key) instead of resource

Open damondouglas opened this issue 2 months ago • 13 comments

This PR fixes https://github.com/hashicorp/terraform-provider-google/issues/17773 refactoring mmv1/third_party/terraform/services/dataflow/resource_dataflow_flex_template_job_test.go.erb to reference pre-existing google_kms_key_ring and google_kms_crypto_key, instead of creating/destroying new ones for each test.

Release Note Template for Downstream PRs (will be copied)


damondouglas avatar Apr 22 '24 22:04 damondouglas

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@ScottSuarez, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

github-actions[bot] avatar Apr 22 '24 22:04 github-actions[bot]

@melinath I was confused by https://github.com/GoogleCloudPlatform/magic-modules/actions/runs/8791696170/job/24126457007?pr=10510. Thank you for your help.

damondouglas avatar Apr 22 '24 22:04 damondouglas

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-beta provider: Diff ( 1 file changed, 4 insertions(+), 5 deletions(-))

modular-magician avatar Apr 22 '24 22:04 modular-magician

Tests analytics

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

Click here to see the affected service packages
  • dataflow

$\textcolor{green}{\textsf{All tests passed!}}$ View the build log

modular-magician avatar Apr 22 '24 22:04 modular-magician

@melinath I was confused by https://github.com/GoogleCloudPlatform/magic-modules/actions/runs/8791696170/job/24126457007?pr=10510. Thank you for your help.

@damondouglas you're welcome - could you remind me how / where I helped?

melinath avatar Apr 22 '24 22:04 melinath

oh it looks like it's still failing, maybe you were asking for help? check out the link at https://github.com/GoogleCloudPlatform/magic-modules/actions/runs/8791696170/job/24126457007?pr=10510#step:5:20 for information on release note formatting

melinath avatar Apr 22 '24 22:04 melinath

oh it looks like it's still failing, maybe you were asking for help? check out the link at https://github.com/GoogleCloudPlatform/magic-modules/actions/runs/8791696170/job/24126457007?pr=10510#step:5:20 for information on release note formatting

The note is what actually confused me.

damondouglas avatar Apr 22 '24 23:04 damondouglas

The note is what actually confused me.

The issue is that you need to update the first comment on this PR to have a well-formatted release note. (Currently it just has the default release-note:REPLACEME.) https://googlecloudplatform.github.io/magic-modules/contribute/release-notes/ is the documentation describing how to format release notes. Does that clarify the next steps?

melinath avatar Apr 23 '24 15:04 melinath

This fails as the datasource key the config is referencing doesn't exist since the keyring is random each time. https://github.com/damondouglas/magic-modules/blob/6af9a0e0a38dc5671211a1b8784b12c708da0e31/mmv1/third_party/terraform/services/dataflow/resource_dataflow_flex_template_job_test.go.erb#L302

I would recommend using a bootstrap function for kms key. This will use a preexisting key. See the example below. https://github.com/modular-magician/terraform-provider-google-beta/blob/2bc478fc9f9befd4bbe3ccc9ece18d7ca430199c/google-beta/services/workflows/resource_workflows_workflow_test.go#L170

--- FAIL: TestAccDataflowFlexTemplateJob_withKmsKey (7.82s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-google-beta/google-beta/services/dataflow       10.535s
FAIL
make: *** [testacc] Error 1

ScottSuarez avatar Apr 23 '24 18:04 ScottSuarez

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-beta provider: Diff ( 1 file changed, 7 insertions(+), 7 deletions(-))

modular-magician avatar Apr 26 '24 20:04 modular-magician

Tests analytics

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

Click here to see the affected service packages
  • dataflow

$\textcolor{green}{\textsf{All tests passed!}}$ View the build log

modular-magician avatar Apr 26 '24 21:04 modular-magician

I found it challenging to work with the go code embedded in ruby code framework. I tried to look up what I could in pkg.go.dev to solve this.

damondouglas avatar Apr 26 '24 21:04 damondouglas

I found it challenging to work with the go code embedded in ruby code framework. I tried to look up what I could in pkg.go.dev to solve this.

You could work in the downstream repo first (ie https://github.com/hashicorp/terraform-provider-google-beta) then upstream the change to magic-modules once you've validated that it works.

Running the test right now. https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleCloud_GOOGLE_BETA_MMUPSTREAMTESTS_GOOGLEBETA_PACKAGE_DATAFLOW?branch=refs%2Fheads%2Fauto-pr-10510&mode=builds#all-projects

ScottSuarez avatar Apr 26 '24 22:04 ScottSuarez

I'm going to be taking over this review from Scott.

melinath avatar May 09 '24 22:05 melinath