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

Routing policy issues with URL rewrite policy. #1084

Closed
eloycoto opened this issue Jul 4, 2019 · 8 comments
Closed

Routing policy issues with URL rewrite policy. #1084

eloycoto opened this issue Jul 4, 2019 · 8 comments

Comments

@eloycoto
Copy link
Contributor

eloycoto commented Jul 4, 2019

Hi,

When URL rewrite changes the URI, the routing policy will no longer work with the original URI, so if the user needs to do the rewrite will never happen, due to the rewrite in routing policies happens on content phase.

Invalid config:

{
  "services": [
    {
      "id": 200,
      "backend_version":  1,
      "backend_authentication_type": "service_token",
      "backend_authentication_value": "token-value",
      "proxy": {
        "hosts": [
          "one", 
          "http://172.18.0.3:8080"
        ],
        "api_backend": "http://192.168.15.103:10001/",
        "proxy_rules": [
          {
            "pattern": "/",
            "http_method": "GET",
            "metric_system_name": "hits",
            "delta": 1
          }
        ],
        "policy_chain": [
          {
            "name": "routing",
            "configuration": {
              "rules": [
                {
                "url": "http://192.168.15.103:10000",
                  "condition": {
                    "combine_op": "and",
                    "operations": [
                      {
                        "op": "matches",
                        "value": "^foo",
                        "value_type": "plain",
                        "match": "path"
                      }
                    ]
                  }
                } 
              ]
            }
          },
          {
            "name": "url_rewriting",
            "configuration": {
              "commands": [
                {
                  "op": "sub",
                  "regex": "^/foo/",
                  "replace": "/"
                }
              ]
            }
          },
          {
            "name": "apicast.policy.apicast"
          }
        ]
      }
    }
  ]
}

The only way to fix this is with a new header, but the routing policy decision should happen on the rewrite phase.

"policy_chain": [
  {
    "name": "routing",
    "configuration": {
      "rules": [
        {
        "url": "http://192.168.15.103:10000",
          "condition": {
            "combine_op": "and",
            "operations": [
              {
                "match": "header",
                "header_name": "R-URI",
                "op": "matches",
                "value": "foo"
              }
            ]
          }
        } 
      ]
    }
  },
  {
    "name": "headers",
    "version": "builtin",
    "configuration": {
      "response": [],
      "request": [
        {
          "op": "set",
          "header": "R-URI",
          "value_type": "liquid",
          "value": "{{uri}}"
        }
      ]
    }
  },
  {
    "name": "url_rewriting",
    "configuration": {
      "commands": [
        {
          "op": "sub",
          "regex": "^/foo/",
          "replace": "/"
        }
      ]
    }
  },
  {
    "name": "apicast.policy.apicast"
  }
]
@mikz
Copy link
Contributor

mikz commented Jul 4, 2019

Shouldn't it be just matter of documenting that the rewrite happens first and the routing happens on the rewritten URI?

@ppatierno
Copy link

The use case I am having here is about using 3scale with the Strimzi Kafka bridge having more bridge instances backed by one 3scale apicast.
I am using the routing policy based on path for routing request from clients to specific bridge instance through the gateway.
For example, a request to http://my-gateway:8080/my-bridge-1/consumers/groupid should be forwarded to bridge1 as http://bridge1:8080/consumers/groupid and so on for my-bridge-2, etc etc
Using just the routing policy fails the mapping rule because the endpoint on the bridge is /consumers/groupid and not /my-bridge-1/consumers/groupid so I would like to apply a URL rewriting stripping the bridge name from the path.
But adding the URL rewriting policy is breaking the routing policy.

@mikz
Copy link
Contributor

mikz commented Jul 4, 2019

Ok, that is a very valid use case I think.

If the rewrite policy would store the original URL before the rewrite, then the routing policy could just use that one instead. That would allow people to chose if they want to use the URL before or after rewrite.

@davidor
Copy link
Contributor

davidor commented Jul 4, 2019

I agree that this is something that we need to address. We'll try to provide a fix for the next release (3.7.0).

@eloycoto
Copy link
Contributor Author

Hi,

I was checking this, but it's not as easy as looks like, at the moment we have the following phases in Openresty:


+---------------+----------------------------+
|     Phase     |         LUA BLOCK          |
+---------------+----------------------------+
| rewrite       | rewrite_by_lua_block       |
| access        | access_by_lua_block        |
| content       | content_by_lua_block       |
| body_filter   | body_filter_by_lua_block   |
| header_filter | header_filter_by_lua_block |
| post_action   | content_by_lua_block       |
| log           | log_by_lua_block           |
+---------------+----------------------------+

The url_rewrite policy happens on the rewrite phase, and that's is the natural place to do that. The routing policy occurs in the content phase, and it's not a logical phase at all. There is a good reason for that; body information in the rewrite phase is not available.

Due to routing policy needs access to JWT, can route based on JWT claims, headers and information it's almost impossible to integrate this on the rewrite phase.

I propose to move rewrite_by_lua_block to content_by_lua_block. I'm not sure if any specific use case that needs to happen in that block, but I'm also sure that there is a good reason why that block is used, any input here?

Regards

@mikz
Copy link
Contributor

mikz commented Jul 30, 2019

The policy phases map to nginx phases. We should keep the 1:1 mapping between policy phases and ngninx phases. Policies can totally be configured to operate on different phases by own configuration. But on which phase it can operate should be left to the policy author, we should not try to merge two nginx phases into one of ours.

JWT is available in a header, not in a body. So it can totally operated on rewrite phase.

I don't see why we can't have both. Just offer the original request URI in some other variable, so the routing policy can access both.

@eloycoto
Copy link
Contributor Author

Ok, about to pass the original URI, the thing is that it's used on ngx_variable.lua, so we need to overwrite that context variables, or add more information to the liquid context.

so, users can use original_uri or orirignal_path maybe we can set that before entering the policies, but each variable needs memory, not sure what it's the balance here.

@mikz
Copy link
Contributor

mikz commented Jul 30, 2019

Strings are immutable in Lua, so as long as they are the same strings it should be using the same memory. And then after a rewrite there would be some copy of it anyway until the request finishes.

Yep, adding it to the liquid context is the way IMO. So it is availabe for other policies too. It can be done when the context is first allocated.

eloycoto added a commit to eloycoto/APIcast that referenced this issue Aug 1, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
eloycoto added a commit to eloycoto/APIcast that referenced this issue Aug 1, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
eloycoto added a commit to eloycoto/APIcast that referenced this issue Aug 1, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
eloycoto added a commit to eloycoto/APIcast that referenced this issue Aug 1, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
eloycoto added a commit to eloycoto/APIcast that referenced this issue Aug 1, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
eloycoto added a commit to eloycoto/APIcast that referenced this issue Aug 1, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
eloycoto added a commit to eloycoto/APIcast that referenced this issue Aug 16, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
eloycoto added a commit to eloycoto/APIcast that referenced this issue Aug 23, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
eloycoto added a commit to eloycoto/APIcast that referenced this issue Sep 4, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
@davidor davidor closed this as completed in 7906bd0 Sep 9, 2019
eloycoto added a commit to eloycoto/APIcast that referenced this issue Sep 16, 2019
To be able to retrieve original request information on the policies
without adding/deleting headers.

This change allows users to handle routing policy with the original
information, full disclosure on issue 3scale#1084

Fix 3scale#1084

Signed-off-by: Eloy Coto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants