-
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
Allow url map to handle backend service fields that can also be backend buckets. #4269
Allow url map to handle backend service fields that can also be backend buckets. #4269
Conversation
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 70 insertions(+), 137 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=159891" |
f3f5b6a
to
97f42e1
Compare
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 1 file changed, 342 insertions(+), 13 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=159897" |
Tests pass! Do you want to see new tests for this, and if so, what would you like to see? |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceSpannerInstance_basic|TestAccActiveDirectoryDomainTrust_activeDirectoryDomainTrustBasicExample|TestAccBigqueryDataTransferConfig|TestAccComposerEnvironment_withUpdateOnCreate|TestAccComputeGlobalForwardingRule_globalForwardingRuleHttpExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleInternalExample|TestAccComputeManagedSslCertificate_managedSslCertificateBasicExample|TestAccComputeTargetGrpcProxy_targetGrpcProxyBasicExample|TestAccComputeTargetGrpcProxy_update|TestAccComputeTargetHttpProxy_targetHttpProxyBasicExample|TestAccComputeTargetHttpsProxy_targetHttpsProxyBasicExample|TestAccComputeUrlMap_urlMapBasicExample|TestAccComputeUrlMap_urlMapTrafficDirectorRouteExample|TestAccComputeUrlMap_urlMapTrafficDirectorRoutePartialExample|TestAccComputeUrlMap_urlMapHeaderBasedRoutingExample|TestAccComputeUrlMap_urlMapTrafficDirectorPathExample|TestAccComputeUrlMap_urlMapTrafficDirectorPathPartialExample|TestAccComputeUrlMap_urlMapParameterBasedRoutingExample|TestAccComputeUrlMap_defaultRouteActionTrafficDirectorPathUpdate|TestAccComputeUrlMap_defaultRouteActionTrafficDirectorUpdate|TestAccComputeUrlMap_trafficDirectorPathUpdate|TestAccContainerNodePool_basic|TestAccContainerNodePool_nodeLocations|TestAccContainerNodePool_withInvalidUpgradeSettings|TestAccContainerNodePool_maxPodsPerNode|TestAccContainerNodePool_withNodeConfig|TestAccContainerNodePool_withGPU|TestAccContainerNodePool_withLinuxNodeConfig|TestAccContainerNodePool_withUpgradeSettings|TestAccContainerNodePool_withKubeletConfig|TestAccContainerNodePool_withNodeConfigScopeAlias|TestAccContainerNodePool_withSandboxConfig|TestAccContainerNodePool_withWorkloadIdentityConfig|TestAccContainerNodePool_regionalAutoscaling|TestAccContainerNodePool_withManagement|TestAccContainerNodePool_autoscaling|TestAccContainerNodePool_resize|TestAccContainerNodePool_regionalClusters|TestAccContainerNodePool_shieldedInstanceConfig|TestAccContainerNodePool_012_ConfigModeAttr|TestAccContainerNodePool_EmptyGuestAccelerator|TestAccFilestoreInstance_filestoreInstanceBasicExample|TestAccFilestoreInstance_update|TestAccOSLoginSSHPublicKey_osLoginSshKeyExpiry You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=159981" |
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 think we could refer to a backend bucket in a couple of the newly-supported fields in a test, unless there's already an example.
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
-%> | ||
func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) { |
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.
Mind adding a comment here explaining what patterns we match on, and what values we provide if so?
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.
Done!
Lucky for me there was already a test - expanded it to test all the new fields also. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 352 insertions(+), 41 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=160862" |
Fixes hashicorp/terraform-provider-google#7886.
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)