-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding initial support for GCF Gen2 terraform integration #5670
Conversation
Oops! It looks like you're using an unknown release-note type in your changelog entries:
Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md. |
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. @rileykarson, please review this PR or find an appropriate assignee. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 8 files changed, 2285 insertions(+), 2 deletions(-)) |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccApigeeEnvironmentIamBindingGenerated|TestAccApigeeEnvironmentIamMemberGenerated|TestAccApigeeEnvironmentIamPolicyGenerated|TestAccComputeBackendServiceIamBindingGenerated|TestAccComputeBackendServiceIamMemberGenerated|TestAccComputeBackendServiceIamPolicyGenerated|TestAccComputeBackendServiceIamBindingGenerated_withCondition|TestAccComputeBackendServiceIamMemberGenerated_withCondition|TestAccComputeBackendServiceIamPolicyGenerated_withCondition|TestAccCloudFunctions2CloudFunction_cloudfunctions2BasicExample|TestAccCloudbuildWorkerPool_basic|TestAccComputeGlobalForwardingRule_externalCndLbWithBackendBucketExample|TestAccComputeSecurityPolicy_withRateLimitOptions|TestAccContainerCluster_withAddons|TestAccContainerCluster_nodeAutoprovisioning|TestAccServiceNetworkingPeeredDNSDomain_basic You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=251031 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made an initial pass- I haven't read the resource proper in depth yet as the test failed, so these are mostly nits and structural.
The test failure was due to the generated name mismatching the resource name in the example:
Copy to clipboard
------- Stdout: -------
=== RUN TestAccCloudFunctions2CloudFunction_cloudfunctions2BasicExample
=== PAUSE TestAccCloudFunctions2CloudFunction_cloudfunctions2BasicExample
=== CONT TestAccCloudFunctions2CloudFunction_cloudfunctions2BasicExample
provider_test.go:286: Step 1/2 error: Error running pre-apply refresh: exit status 1
There are some problems with the CLI configuration:
Error: The specified plugin cache dir /opt/teamcity-agent/work/2e51954ade9939a/.provider-cache cannot be opened: stat /opt/teamcity-agent/work/2e51954ade9939a/.provider-cache: no such file or directory
As a result of the above problems, Terraform may not behave as intended.
Error: Invalid resource type
on terraform_plugin_test.tf line 2, in resource "google_cloudfunctions2_cloud_function" "function":
2: resource "google_cloudfunctions2_cloud_function" "function" {
The provider provider.google does not support resource type
"google_cloudfunctions2_cloud_function".
--- FAIL: TestAccCloudFunctions2CloudFunction_cloudfunctions2BasicExample (6.71s)
FAIL
name: 'CloudFunction' | ||
base_url: projects/{{project}}/locations/{{location}}/functions | ||
create_verb: :POST | ||
description: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible include a references section, similar to
magic-modules/mmv1/products/cloudbuild/api.yaml
Lines 33 to 36 in 4d63482
references: !ruby/object:Api::Resource::ReferenceLinks | |
guides: | |
'Automating builds using build triggers': 'https://cloud.google.com/cloud-build/docs/running-builds/automate-builds' | |
api: 'https://cloud.google.com/cloud-build/docs/api/reference/rest/v1/projects.triggers' |
That'll link the API reference and a user guide from the provider docs for the resource.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a similar note- are those docs published somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have public docs since we're just going to Public Preview, I'll check with my lead on this one though. Let me know if you want to see the internal docs.
name: 'environment' | ||
description: 'The environment the function is hosted on.' | ||
values: | ||
- :ENVIRONMENT_UNSPECIFIED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all enums, X_UNSPECIFIED tends to be invalid to actually send to the API so we can omit it. If that's not the case, please ignore this though.
--- !ruby/object:Provider::Terraform::Config | ||
overrides: !ruby/object:Overrides::ResourceOverrides | ||
CloudFunction: !ruby/object:Overrides::Terraform::ResourceOverride | ||
id_format: 'projects/{{project}}/locations/{{region}}/functions/{{cloud_function}}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id_format: 'projects/{{project}}/locations/{{region}}/functions/{{cloud_function}}' | |
id_format: 'projects/{{project}}/locations/{{region}}/functions/{{name}}' |
Same below- this looks for a field on the resource ultimately, and cloud_function
isn't one. name
should be correct (we may need to fiddle with some overrides to get the right format, depending on the API response)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm running into issues with 404s on the test files, not sure I'm quite understanding this piece to ensure the API request/responses are coming through correctly. I updated it from cloud_function to name. I was taking after the Gen1 GCF configuration, but unsure if those test cases are correct.
Oops! It looks like you're using an unknown release-note type in your changelog entries:
Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 8 files changed, 2285 insertions(+), 2 deletions(-)) |
Oops! It looks like you're using an unknown release-note type in your changelog entries:
Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 8 files changed, 2287 insertions(+), 2 deletions(-)) |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceGoogleActiveFolder_default|TestAccDataSourceGoogleForwardingRule|TestAccDataSourceGoogleFolder_byFullName|TestAccDataSourceGoogleFolder_lookupOrganization|TestAccDataSourceGoogleFolder_byShortName|TestAccDataSourceGoogleFolders_basic|TestAccDataSourceGoogleGlobalForwardingRule|TestAccApigeeEnvironmentIamBindingGenerated|TestAccApigeeEnvironmentIamMemberGenerated|TestAccApigeeEnvironmentIamPolicyGenerated|TestAccComputeBackendServiceIamBindingGenerated|TestAccComputeBackendServiceIamMemberGenerated|TestAccComputeBackendServiceIamPolicyGenerated|TestAccComputeBackendServiceIamBindingGenerated_withCondition|TestAccComputeBackendServiceIamMemberGenerated_withCondition|TestAccComputeBackendServiceIamPolicyGenerated_withCondition|TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccCloudfunctions2function_cloudfunctions2BasicExample|TestAccCloudbuildWorkerPool_basic|TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample|TestAccComputeForwardingRule_forwardingRuleExternallbExample|TestAccComputeForwardingRule_forwardingRuleGlobalInternallbExample|TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExample|TestAccComputeForwardingRule_forwardingRuleBasicExample|TestAccComputeForwardingRule_forwardingRuleInternallbExample|TestAccComputeForwardingRule_forwardingRuleL3DefaultExample|TestAccComputeForwardingRule_update|TestAccComputeForwardingRule_forwardingRuleRegionalHttpXlbExample|TestAccComputeForwardingRule_forwardingRuleHttpLbExample|TestAccComputeGlobalForwardingRule_externalCndLbWithBackendBucketExample|TestAccComputeForwardingRule_ip|TestAccComputeForwardingRule_networkTier|TestAccComputeGlobalForwardingRule_externalTcpProxyLbMigBackendExample|TestAccComputeGlobalForwardingRule_externalHttpLbMigBackendCustomHeaderExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleHttpExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleInternalExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleExternalManagedExample|TestAccComputeGlobalForwardingRule_privateServiceConnectGoogleApisExample|TestAccComputeGlobalForwardingRule_updateTarget|TestAccComputeGlobalForwardingRule_labels|TestAccComputeGlobalForwardingRule_ipv6|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeInstanceFromMachineImage_basic|TestAccComputeInstanceFromMachineImage_overrideMetadataDotStartupScript|TestAccComputeInstanceFromMachineImage_diffProject|TestAccComputeManagedSslCertificate_managedSslCertificateBasicExample|TestAccComputePacketMirroring_computePacketMirroringFullExample|TestAccComputeRoute_routeIlbExample|TestAccComputeRoute_routeIlbVipExample|TestAccComputeRouterPeer_basic|TestAccComputeRouterPeer_advertiseMode|TestAccComputeRouterPeer_enable|TestAccComputeRouterPeer_bfd|TestAccComputeRouterInterface_basic|TestAccComputeRouterInterface_withTunnel|TestAccComputeSecurityPolicy_withRateLimitOptions|TestAccComputeServiceAttachment_serviceAttachmentBasicExample|TestAccComputeServiceAttachment_serviceAttachmentExplicitProjectsExample|TestAccComputeServiceAttachment_serviceAttachmentBasicExampleUpdate|TestAccComputeVpnGateway_targetVpnGatewayBasicExample|TestAccComputeVpnTunnel_vpnTunnelBasicExample|TestAccComputeVpnTunnel_vpnTunnelBetaExample|TestAccComputeVpnTunnel_regionFromGateway|TestAccComputeVpnTunnel_router|TestAccComputeVpnTunnel_defaultTrafficSelectors|TestAccContainerCluster_withAddons|TestAccContainerCluster_nodeAutoprovisioning|TestAccFolderIamAuditConfig_basic|TestAccFolderIamAuditConfig_addFirstExemptMember|TestAccFolderIamAuditConfig_update|TestAccFolderIamAuditConfig_removeLastExemptMember|TestAccFolderIamAuditConfig_updateNoExemptMembers|TestAccFolderIamBinding_basic|TestAccFolderIamBinding_update|TestAccFolderIamMember_basic|TestAccServiceNetworkingPeeredDNSDomain_basic You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=252726 |
Oops! It looks like you're using an unknown release-note type in your changelog entries:
Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 8 files changed, 2284 insertions(+), 2 deletions(-)) |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccApigeeEnvironmentIamMemberGenerated|TestAccCloudfunctions2function_cloudfunctions2BasicExample|TestAccCloudbuildWorkerPool_basic|TestAccContainerCluster_withAuthenticatorGroupsConfig|TestAccServiceNetworkingPeeredDNSDomain_basic You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=258554 |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 8 files changed, 2316 insertions(+), 2 deletions(-)) |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 10 files changed, 2321 insertions(+), 2 deletions(-)) |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic|TestAccApigeeEnvironmentIamBindingGenerated|TestAccApigeeEnvironmentIamMemberGenerated|TestAccCloudfunctions2function_cloudfunctions2BasicExample|TestAccCloudbuildWorkerPool_basic|TestAccComputeInstanceTemplate_reservationAffinities|TestAccContainerCluster_withAuthenticatorGroupsConfig You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=259131 |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 10 files changed, 2316 insertions(+), 2 deletions(-)) |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccApigeeEnvironmentIamBindingGenerated|TestAccApigeeEnvironmentIamMemberGenerated|TestAccApigeeEnvironmentIamPolicyGenerated|TestAccCloudfunctions2function_cloudfunctions2BasicExample|TestAccCloudbuildWorkerPool_basic|TestAccContainerCluster_withAuthenticatorGroupsConfig You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=259138 |
Tests failed during RECORDING mode: TestAccCloudbuildWorkerPool_basic|TestAccContainerCluster_withAuthenticatorGroupsConfig Please fix these to complete your PR |
quick note- handing off review to @c2thorn (my name won't disappear because I've left prior review comments, but you can ignore it) |
versions: | ||
- !ruby/object:Api::Product::Version | ||
name: ga | ||
base_url: https://cloudfunctions.googleapis.com/v2alpha/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a new v2 endpoint now that it's launched? Is alpha the one we want to go with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're still using v2alpha post-launch. Once we reach GA I believe our endpoint will be updated.
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 11 files changed, 2569 insertions(+), 2 deletions(-)) |
/gcbrun |
Re-added commit to fix rebase issue |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 563 insertions(+)) |
/gcbrun |
/gcbrun |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 437 insertions(+)) |
1 similar comment
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 437 insertions(+)) |
mmv1/third_party/terraform/resources/resource_cloudfunction2_function_test.go.erb
Outdated
Show resolved
Hide resolved
…unction_test.go.erb Co-authored-by: Cameron Thornton <[email protected]>
/gcbrun |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 444 insertions(+)) |
1 similar comment
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 444 insertions(+)) |
mmv1/third_party/terraform/resources/resource_cloudfunction2_function_test.go.erb
Outdated
Show resolved
Hide resolved
…unction_test.go.erb
/gcbrun |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 444 insertions(+)) |
1 similar comment
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 444 insertions(+)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests passed from manual run, waiting for VCR tests before merging
Added API and Terraform yamls as well as an example for generating tests.
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)