Skip to content
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

Path matcher default_route_action conflicts with default_url_redirect #6967

Closed
wastrachan opened this issue Aug 6, 2020 · 12 comments · Fixed by GoogleCloudPlatform/magic-modules#3885, #7063 or hashicorp/terraform-provider-google-beta#2406
Assignees
Labels

Comments

@wastrachan
Copy link

wastrachan commented Aug 6, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
  • If an issue is assigned to the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

  • Terraform v0.12.29
  • google provider 3.33.0

Affected Resource(s)

  • google_compute_url_map

Terraform Configuration Files

resource "google_compute_url_map" "http" {
  name = "prod-lb-http"

  default_url_redirect {
    https_redirect         = true
    strip_query            = false
    redirect_response_code = "MOVED_PERMANENTLY_DEFAULT"
  }
}

resource "google_compute_url_map" "https" {
  name = "prod-lb-https"

  default_url_redirect {
    host_redirect          = "https://www.${var.domain}"
    strip_query            = true
    redirect_response_code = "FOUND"
    path_redirect          = "/"
  }

  host_rule {
    hosts        = ["${var.domain}"]
    path_matcher = "prod"
  }

  host_rule {
    hosts        = ["quotes.${var.domain}"]
    path_matcher = "quotes"
  }

  path_matcher {
    name            = "prod"
    default_service = module.appserver_backend_service.google_compute_backend_service.id
  }

  path_matcher {
    name            = "quotes"
    default_service = module.appserver_backend_service.google_compute_backend_service.id

    default_route_action {
      url_rewrite {
        path_prefix_rewrite = "/quickquote/"
      }
    }
  }

}

Debug Output

https://u.pcloud.link/publink/show?code=XZkqYGkZiEiVoOr87lYOtrUzQJ9R9BMz7Cf7

Encrypted per https://www.hashicorp.com/security for privacy
695A7F5DEA640DF326CF3BF019458EC7CC6028D6

Expected Behavior

I expect the plan to apply, creating both a default URL redirect for the URL map, and a default route action for my selected path matcher.

Actual Behavior

The terraform run fails with the following output:

Error: "path_matcher.1.default_route_action": conflicts with default_url_redirect

  on lb.tf line 60, in resource "google_compute_url_map" "https":
  60: resource "google_compute_url_map" "https" {

Steps to Reproduce

  1. terraform apply

Important Factoids

References

This issue seems nearly identical to #6695, although the conflict here is with default_url_redirect instead of default_service

@edwardmedia edwardmedia added the bug label Aug 7, 2020
@edwardmedia edwardmedia self-assigned this Aug 7, 2020
@edwardmedia
Copy link
Contributor

edwardmedia commented Aug 7, 2020

@wastrachan can you try moving default_route_action block out of path_math block?

@wastrachan
Copy link
Author

@wastrachan can you try moving default_route_action block out of path_math block?

That I can.

If I remove default_route_action totally, obviously everything works as intended. If I move it directly underneath google_compute_url_map I do get a conflict with default_url_redirect, which is anticipated.

If I move default_route_action to underneath google_compute_url_map and then totally remove default_url_redirect I do get an expected failure:

Error: "default_route_action.0.weighted_backend_services": one of `default_route_action.0.weighted_backend_services,default_service,default_url_redirect` must be specified

  on lb.tf line 60, in resource "google_compute_url_map" "https":
  60: resource "google_compute_url_map" "https" {



Error: "default_url_redirect": one of `default_route_action.0.weighted_backend_services,default_service,default_url_redirect` must be specified

  on lb.tf line 60, in resource "google_compute_url_map" "https":
  60: resource "google_compute_url_map" "https" {



Error: "default_service": one of `default_route_action.0.weighted_backend_services,default_service,default_url_redirect` must be specified

  on lb.tf line 60, in resource "google_compute_url_map" "https":
  60: resource "google_compute_url_map" "https" {

@ghost ghost removed the waiting-response label Aug 7, 2020
@edwardmedia
Copy link
Contributor

@wastrachan here is how default_route_action should be used, along with other attributes. I think default_route_action should be set at the root of the resource, which is your original problem. You might want to double check the following doc and adjust your config. Let me know if this helps address your issue. Thanks

`
default_route_action - (Optional) defaultRouteAction takes effect when none of the hostRules match. The load balancer performs advanced routing actions like URL rewrites, header transformations, etc. prior to forwarding the request to the selected backend. If defaultRouteAction specifies any weightedBackendServices, defaultService must not be set. Conversely if defaultService is set, defaultRouteAction cannot contain any weightedBackendServices. Only one of defaultRouteAction or defaultUrlRedirect must be set. Structure is documented below.

https://www.terraform.io/docs/providers/google/r/compute_url_map.html
`

@wastrachan
Copy link
Author

@wastrachan here is how default_route_action should be used, along with other attributes. I think default_route_action should be set at the root of the resource, which is your original problem. You might want to double check the following doc and adjust your config. Let me know if this helps address your issue. Thanks

`
default_route_action - (Optional) defaultRouteAction takes effect when none of the hostRules match. The load balancer performs advanced routing actions like URL rewrites, header transformations, etc. prior to forwarding the request to the selected backend. If defaultRouteAction specifies any weightedBackendServices, defaultService must not be set. Conversely if defaultService is set, defaultRouteAction cannot contain any weightedBackendServices. Only one of defaultRouteAction or defaultUrlRedirect must be set. Structure is documented below.

https://www.terraform.io/docs/providers/google/r/compute_url_map.html

@edwardmedia I think perhaps there might be some documentation inconsistency then. The docs show that default_route_action is also available under path_matcher

The path_matcher block supports:
[...]
default_route_action - (Optional) defaultRouteAction takes effect when none of the pathRules or routeRules match. The load balancer performs advanced routing actions like URL rewrites, header transformations, etc. prior to forwarding the request to the selected backend. If defaultRouteAction specifies any weightedBackendServices, defaultService must not be set. Conversely if defaultService is set, defaultRouteAction cannot contain any weightedBackendServices. Only one of defaultRouteAction or defaultUrlRedirect must be set. Structure is documented below.

This description is consistent with what appears to be available in GCP. So I suspect that either:

  1. The module documentation is incorrect
  2. The feature to have a default route action is available in GCP but not implemented in the provider
  3. The feature is available and should work, but a sanity check is preventing it from working-- matching a top-level block instead of the path_matcher level block it should be checking for.

Any insight?

@ghost ghost removed waiting-response labels Aug 7, 2020
@edwardmedia
Copy link
Contributor

@wastrachan i agreed the doc is not super clear. There are a lot of fields they are interrelated. Below is the original doc. Hope this might be helpful. I do see you can set default_route_action inside the path_matcher. If you see the conflicts in the doc and behavior between provider and the GCP API, let me know. Please be specific what the difference are. Thank you

https://cloud.google.com/compute/docs/reference/rest/v1/urlMaps

@wastrachan
Copy link
Author

Thanks @edwardmedia, this is helpful to review.

So looking at the official REST docs, this is the section that stands out:

pathMatchers[].defaultRouteAction

defaultRouteAction takes effect when none of the pathRules or routeRules match. The load balancer performs advanced routing actions like URL rewrites, header transformations, etc. prior to forwarding the request to the selected backend. If defaultRouteAction specifies any weightedBackendServices, defaultService must not be set. Conversely if defaultService is set, defaultRouteAction cannot contain any weightedBackendServices.

Only one of defaultRouteAction or defaultUrlRedirect must be set.

UrlMaps for external HTTP(S) load balancers support only the urlRewrite action within a pathMatcher's defaultRouteAction.

Not supported when the backend service is referenced by a URL map that is bound to target gRPC proxy that has validateForProxyless field set to true.

I've definitely not tried to make any calls to the REST API directly, however, I don't see any indication that this would conflict with the root defaultUrlRedirect in the documentation, and the behavior available through the web UI supports the notion that there is no conflict between the elements.

I do see in the documentation that there is a conflict between defaultRouteAction and defaultUrlRedirect at the root level, but no indication that there is a conflict between the a root element and one nuder pathMatcher.

Perhaps there is an exception returned and it's not documented, but without any such documentation it appears that a conflict between the a root defaultRouteAction and a defaultRouteAction under a pathMatcher is inferred in the provider, but not truly relevant.

@ghost ghost removed the waiting-response label Aug 7, 2020
@edwardmedia
Copy link
Contributor

edwardmedia commented Aug 11, 2020

@wastrachan here is what API docs says (search by defaultUrlRedirect)

If defaultUrlRedirect is specified, defaultService or defaultRouteAction must not be set.

https://cloud.google.com/compute/docs/reference/rest/v1/urlMaps

@wastrachan
Copy link
Author

@edward2a I'm still nearly positive this is referring to a conflict withing the current scope.

To prove this, I captured the API request Terraform generated and modified it to include the defaultRouteAction we've been discussion:

curl -X PUT -H "Authorization: Bearer XXXXXX" -H "Content-Type: application/json" \
https://compute.googleapis.com/compute/v1/projects/XXXXXX-prod/global/urlMaps/prod-lb-https?alt=json -d '
 {
  "defaultUrlRedirect": {
   "hostRedirect": "https://www.domain.com",
   "pathRedirect": "/",
   "redirectResponseCode": "FOUND",
   "stripQuery": true
  },
  "fingerprint": "z3wNwBm0jCI=",
  "hostRules": [
   {
    "hosts": [
     "domain.com",
     "testbed.domain.com"
    ],
    "pathMatcher": "prod"
   },
   {
    "hosts": [
     "quotetest.domain.com",
     "quotes.domain.com"
    ],
    "pathMatcher": "quotes"
   }
  ],
  "name": "prod-lb-https",
  "pathMatchers": [
   {
    "defaultService": "https://www.googleapis.com/compute/v1/projects/XXXX-prod/global/backendServices/appserver",
    "name": "prod"
   },
   {
    "defaultService": "https://www.googleapis.com/compute/v1/projects/XXXX-prod/global/backendServices/appserver",
    "name": "quotes",
    "defaultRouteAction": {
      "urlRewrite": {
        "pathPrefixRewrite": "/quickquote/",
      }
    }
   },
  ]
 }
'

Which does work:

{
  "id": "1379902350757441315",
  "name": "operation-1597116876224-5ac91c0127906-616f3936-99cbe243",
  "operationType": "update",
  "targetLink": "https://www.googleapis.com/compute/v1/projects/XXXX-prod/global/urlMaps/prod-lb-https",
  "targetId": "4896982770880303153",
  "status": "RUNNING",
  "user": "XXXXXX",
  "progress": 0,
  "insertTime": "2020-08-10T20:34:36.697-07:00",
  "startTime": "2020-08-10T20:34:36.716-07:00",
  "selfLink": "https://www.googleapis.com/compute/v1/projects/XXXX-prod/global/operations/operation-1597116876224-5ac91c0127906-616f3936-99cbe243",
  "kind": "compute#operation"
}

The result of the operation was then verified with a GET to the selected resource:

curl -X GET -H "Authorization: Bearer XXXXXXXXXXX" -H "Content-Type: application/json" \
https://compute.googleapis.com/compute/v1/projects/XXXXXX-prod/global/urlMaps/prod-lb-https\?alt\=json
{
  "id": "4896982770880303153",
  "creationTimestamp": "2020-08-05T10:39:42.977-07:00",
  "name": "prod-lb-https",
  "selfLink": "https://www.googleapis.com/compute/v1/projects/XXXXX-prod/global/urlMaps/prod-lb-https",
  "hostRules": [
    {
      "hosts": [
        "domain.com",
        "testbed.domain.com"
      ],
      "pathMatcher": "prod"
    },
    {
      "hosts": [
        "quotetest.domain.com",
        "quotes.domain.com"
      ],
      "pathMatcher": "quotes"
    }
  ],
  "pathMatchers": [
    {
      "name": "prod",
      "defaultService": "https://www.googleapis.com/compute/v1/projects/XXXXXXX-prod/global/backendServices/appserver"
    },
    {
      "name": "quotes",
      "defaultService": "https://www.googleapis.com/compute/v1/projects/XXXXXXX-prod/global/backendServices/appserver",
      "defaultRouteAction": {
        "urlRewrite": {
          "pathPrefixRewrite": "/quickquote/"
        }
      }
    }
  ],
  "defaultUrlRedirect": {
    "hostRedirect": "https://www.domain.com",
    "pathRedirect": "/",
    "redirectResponseCode": "FOUND",
    "stripQuery": true
  },
  "fingerprint": "6L_iRebz9kU=",
  "kind": "compute#urlMap"
}

I don't know how I can more conclusively demonstrate that this should work in Terraform, but I'm happy to keep throwing stuff over the fence. Anything else I can provide to back my claim up?

I do appreciate your time on this-- I know it's tempting just to dismiss bug reports like this, and you've been very patient as I make my case-- so thank you!

@ghost ghost removed waiting-response labels Aug 11, 2020
@edwardmedia
Copy link
Contributor

@wastrachan I see. Looks like current Terraform behavior differently. Thank you @wastrachan for reporting this. I am inviting @c2thorn to take another look.

@wastrachan
Copy link
Author

Thanks @edward2a, and thanks in advance @c2thorn

@c2thorn
Copy link
Collaborator

c2thorn commented Aug 19, 2020

Thanks for your patience @wastrachan! Turns out this was definitely unintentional. The fix will take some time to propagate to a release (looks like it will end up in v3.37).

@ghost
Copy link

ghost commented Sep 19, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Sep 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.