Add default url redirect to url maps #3379

ctavan · 2020-04-15T15:55:09Z

Fixes hashicorp/terraform-provider-google#6081

A few things that came up while developing this:

RegionTargetHttpProxies are currently in the google-beta terraform provider even though theses resources seem to be GA now: https://www.terraform.io/docs/providers/google/r/compute_region_target_http_proxy.html / https://cloud.google.com/compute/docs/reference/rest/v1/regionTargetHttpProxies -> I did not change the example yet.
urlRedirect parameter seems to be slightly out-of-sync: Update urlRedirect property of UrlMap and RegionUrlMap #3378
Not sure if/how to write tests for this new feature?
Not sure if the google_compute_target_http_proxy resource is the best place for the example?
Also the API docs state: "If defaultUrlRedirect is specified, defaultService or defaultRouteAction must not be set." (How) is such mutual exclusivity modeled in magic-modules?

Release Note Template for Downstream PRs (will be copied)

compute: Added support for default URL redirects to `google_compute_url_map`

compute: Added support for default URL redirects to `google_compute_region_url_map`

googlebot · 2020-04-15T15:55:15Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

modular-magician · 2020-04-15T15:56:26Z

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.

@SirGitsalot, please review this PR or find an appropriate assignee.

modular-magician · 2020-04-15T16:00:19Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 4 files changed, 578 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 8 files changed, 1162 insertions(+), 11 deletions(-))
Ansible: Diff ( 4 files changed, 950 insertions(+))
TF Conversion: Diff ( 1 file changed, 169 insertions(+))
TF OiCS: Diff ( 8 files changed, 230 insertions(+))
Inspec: Diff ( 7 files changed, 136 insertions(+))

modular-magician · 2020-04-15T16:00:25Z

Oops! It looks like you're using an unknown release-note type in your changelog entries:

REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

ctavan · 2020-04-15T17:58:35Z

@googlebot I signed it!

googlebot · 2020-04-15T17:58:39Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

danawillow · 2020-04-21T22:41:10Z

Picking a new reviewer so we get someone who works on TF

chrisst · 2020-04-21T23:29:26Z

I'll be able to review this later this week but I'll start by answering some of your questions:

re RegionTargetHttpProxies being beta when the feature is GA - we currently move features in the provider to GA only when somebody lets us know. We don't currently have a method to find this out proactively. Would you file a request for this? Or you can send a PR if you would like.
writing tests - You can add a hand written test file even though these resources are generated. We frequently will do this for resources so we can test the update method or any special logic. These tests are added in third_party/terraform/tests/*
Mutually exclusive logic can be enforced with the exactly_one_of feature or the conflicts feature. At first glance it looks like conflicts is the better option here.

ctavan · 2020-04-22T05:50:27Z

Re GA, see #3381

I‘ll look into tests and conflicts later today.

Source: https://cloud.google.com/compute/docs/reference/rest/v1/urlMaps Fixes hashicorp/terraform-provider-google#6081

Also update docs according to: * https://cloud.google.com/compute/docs/reference/rest/v1/urlMaps * https://cloud.google.com/compute/docs/reference/rest/v1/regionUrlMaps

modular-magician · 2020-04-22T11:24:28Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 614 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 10 files changed, 1252 insertions(+), 21 deletions(-))
Ansible: Diff ( 4 files changed, 1033 insertions(+), 61 deletions(-))
TF Conversion: Diff ( 1 file changed, 169 insertions(+))
TF OiCS: Diff ( 8 files changed, 230 insertions(+))
Inspec: Diff ( 7 files changed, 138 insertions(+), 2 deletions(-))

modular-magician · 2020-04-22T11:24:34Z

Oops! It looks like you're using an unknown release-note type in your changelog entries:

REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

modular-magician · 2020-04-22T11:31:01Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 615 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 10 files changed, 1258 insertions(+), 21 deletions(-))
Ansible: Diff ( 4 files changed, 1033 insertions(+), 61 deletions(-))
TF Conversion: Diff ( 1 file changed, 169 insertions(+))
TF OiCS: Diff ( 8 files changed, 230 insertions(+))
Inspec: Diff ( 7 files changed, 138 insertions(+), 2 deletions(-))

modular-magician · 2020-04-22T11:31:06Z

Oops! It looks like you're using an unknown release-note type in your changelog entries:

REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

modular-magician · 2020-04-22T11:43:25Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 615 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 10 files changed, 1258 insertions(+), 21 deletions(-))
Ansible: Diff ( 4 files changed, 1033 insertions(+), 61 deletions(-))
TF Conversion: Diff ( 1 file changed, 169 insertions(+))
TF OiCS: Diff ( 8 files changed, 230 insertions(+))
Inspec: Diff ( 7 files changed, 138 insertions(+), 2 deletions(-))

modular-magician · 2020-04-22T11:43:30Z

Oops! It looks like you're using an unknown release-note type in your changelog entries:

REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

modular-magician · 2020-04-22T11:45:59Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 615 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 10 files changed, 1258 insertions(+), 21 deletions(-))
Ansible: Diff ( 4 files changed, 1033 insertions(+), 61 deletions(-))
TF Conversion: Diff ( 1 file changed, 169 insertions(+))
TF OiCS: Diff ( 8 files changed, 230 insertions(+))
Inspec: Diff ( 7 files changed, 138 insertions(+), 2 deletions(-))

modular-magician · 2020-04-22T11:46:04Z

Oops! It looks like you're using an unknown release-note type in your changelog entries:

REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

ctavan · 2020-04-22T11:57:24Z

@chrisst

I've added a basic test that covers the new property. Is it fine to have a separate test case here or should it be added as an additional step in an existing test case?

I've added exactly_one_of rules. I think they are more appropriate than conflicts because "exactly one of" seems to be precisely what the API demands. At least I got the following error message when trying to define none of defaultService or defaultUrlRedirect:

Error: Error updating UrlMap "projects/*/global/urlMaps/url-map": googleapi: Error 400: Invalid value for field 'resource.defaultService': ''. No default backend service specified., invalid

And this error when specifying both:

Error: Error updating UrlMap "projects/*/global/urlMaps/url-map": googleapi: Error 400: Invalid value for field 'resource.defaultUrlRedirect': '{  "httpsRedirect": true}'. A default redirect can only be specified for a url map if it neither specifies a default service nor a default route action, invalid

I think this is ready for review now.

chrisst

Looks great, thanks for adding tests as well!

modular-magician · 2020-04-24T22:19:19Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 629 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 10 files changed, 1292 insertions(+), 27 deletions(-))
Ansible: Diff ( 4 files changed, 1036 insertions(+), 67 deletions(-))
TF Conversion: Diff ( 1 file changed, 169 insertions(+))
TF OiCS: Diff ( 8 files changed, 230 insertions(+))
Inspec: Diff ( 7 files changed, 150 insertions(+), 2 deletions(-))

googlebot added the cla: no label Apr 15, 2020

modular-magician added the community label Apr 15, 2020

googlebot added cla: yes and removed cla: no labels Apr 15, 2020

ctavan mentioned this pull request Apr 15, 2020

Add support for default_url_redirect to google_compute_url_map hashicorp/terraform-provider-google#6081

Closed

danawillow requested a review from chrisst April 21, 2020 22:40

ctavan added 3 commits April 22, 2020 11:39

Add defaultUrlRedirect to UrlMap and RegionUrlMap

fbfc66d

Source: https://cloud.google.com/compute/docs/reference/rest/v1/urlMaps Fixes hashicorp/terraform-provider-google#6081

Add example for http to http redirect proxy

7cab80a

Add exactly_one_of rules

a00cd6c

Also update docs according to: * https://cloud.google.com/compute/docs/reference/rest/v1/urlMaps * https://cloud.google.com/compute/docs/reference/rest/v1/regionUrlMaps

ctavan force-pushed the add-default-url-redirect-to-url-maps branch from 7f1bea3 to 8fd57a0 Compare April 22, 2020 11:19

ctavan force-pushed the add-default-url-redirect-to-url-maps branch from 8fd57a0 to c143b8c Compare April 22, 2020 11:25

ctavan force-pushed the add-default-url-redirect-to-url-maps branch from c143b8c to f7eb76a Compare April 22, 2020 11:38

Add test for defaultUrlRedirect

62d1d71

ctavan force-pushed the add-default-url-redirect-to-url-maps branch from f7eb76a to 62d1d71 Compare April 22, 2020 11:41

chrisst approved these changes Apr 24, 2020

View reviewed changes

Merge branch 'master' into add-default-url-redirect-to-url-maps

743713b

chrisst merged commit 5cb808a into GoogleCloudPlatform:master Apr 24, 2020

danawillow mentioned this pull request Apr 25, 2020

L7-XLB URL Rewrites & Redirects hashicorp/terraform-provider-google#6205

Closed

This was referenced Apr 25, 2020

Promote region* compute services to GA #3381

Merged

Update urlRedirect property of UrlMap and RegionUrlMap #3378

Merged

slevenick mentioned this pull request Apr 29, 2020

Add back requireds on stripquery #3443

Merged

nathkn pushed a commit to nathkn/magic-modules that referenced this pull request May 18, 2020

Add default url redirect to url maps (GoogleCloudPlatform#3379)

89f4d34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add default url redirect to url maps #3379

Add default url redirect to url maps #3379

ctavan commented Apr 15, 2020 •

edited by chrisst

Loading

googlebot commented Apr 15, 2020

modular-magician commented Apr 15, 2020

modular-magician commented Apr 15, 2020

modular-magician commented Apr 15, 2020

ctavan commented Apr 15, 2020

googlebot commented Apr 15, 2020

danawillow commented Apr 21, 2020

chrisst commented Apr 21, 2020

ctavan commented Apr 22, 2020

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

ctavan commented Apr 22, 2020

chrisst left a comment

modular-magician commented Apr 24, 2020

Add default url redirect to url maps #3379

Add default url redirect to url maps #3379

Conversation

ctavan commented Apr 15, 2020 • edited by chrisst Loading

googlebot commented Apr 15, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

modular-magician commented Apr 15, 2020

modular-magician commented Apr 15, 2020

Diff report:

modular-magician commented Apr 15, 2020

ctavan commented Apr 15, 2020

googlebot commented Apr 15, 2020

danawillow commented Apr 21, 2020

chrisst commented Apr 21, 2020

ctavan commented Apr 22, 2020

modular-magician commented Apr 22, 2020

Diff report:

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

Diff report:

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

Diff report:

modular-magician commented Apr 22, 2020

modular-magician commented Apr 22, 2020

Diff report:

modular-magician commented Apr 22, 2020

ctavan commented Apr 22, 2020

chrisst left a comment

Choose a reason for hiding this comment

modular-magician commented Apr 24, 2020

Diff report:

ctavan commented Apr 15, 2020 •

edited by chrisst

Loading