[THREESCALE-9320] Conditional policy evaluating incorrectly: second policy in policy chain that implement export() always triggers #1485

tkan145 · 2024-07-11T06:30:40Z

What

Fix https://issues.redhat.com/browse/THREESCALE-9320

An interesting side effect of this PR is that it also fixes the following problems
https://issues.redhat.com/browse/THREESCALE-8146
https://issues.redhat.com/browse/THREESCALE-8149

Why

When included in the policy chain, conditional policy will construct an internal policy chain with a list of policies.

When APIcast build a shared context, it will then call export() on the chain, including the chain constructed by the conditional policy. This means that any policy that implements export function will be triggered regardless of what the user sets the conditions for.

We can avoid this by moving the context export to a different phase. But first let check their current phases

Policy	Executed by	executed in phase	Phases in official docs	Order in the chain	Proposed phase change	Order in the chain after change
upstream_connection	apicast/upstream.lua	content -> balancer	not documented	the position of this policy does not matter.	rewrite	the position of this policy does not matter.
retry	apicast/upstream.lua	content -> balancer	not documented	the position of this policy does not matter.	rewrite	the position of this policy does not matter.
camel	apicast/upstream.lua	access -> content -> balancer	access	access	rewrite	the position of this policy does not matter.
on_failed	apicast/policy_chain.lua	every phase	not documented	not documented	I prefer to keep the current behavior
caching	policy/3scale_batcher/3scale_batcher.lua apicast/apicast.lua apicast/proxy.lua	rewrite-> access	not documented	the position of this policy does not matter.	I prefer to keep the current behavior
http_proxy	apicast/upstream.lua	content -> balancer	not documented	the position of this policy does not matter.	rewrite	the position of this policy does not matter.
request_unbuffered	apicast/upstream.lua	content -> balancer	not documented	the position of this policy does not matter.	rewrite	the position of this policy does not matter.

Verification steps

Upstream connection policy

Checkout this branch
Start development environment

make development
make dependencies

Create apicast-config.json

{
  "services": [
    {
      "backend_version": "1",
      "id": "1",
      "proxy": {
        "hosts": [
          "one"
        ],
        "api_backend": "https://httpbingo.org/delay/",
        "backend": {
          "endpoint": "http://127.0.0.1:8081",
          "host": "backend"
        },
        "policy_chain": [
          {
            "name": "apicast.policy.conditional",
            "configuration": {
              "condition": {
                "operations": [
                  {
                    "left": "{{ uri }}",
                    "left_type": "liquid",
                    "op": "==",
                    "right": "/2",
                    "right_type": "plain"
                  }
                ]
              },
              "policy_chain": [
                {
                  "name": "apicast.policy.upstream_connection",
                  "configuration": {
                    "connect_timeout": 1,
                    "send_timeout": 1,
                    "read_timeout": 1
                  }
                }
              ]
            }
          },
          {
            "name": "apicast.policy.apicast"
          }
        ],
        "proxy_rules": [
          {
            "http_method": "GET",
            "pattern": "/",
            "metric_system_name": "hits",
            "delta": 1,
            "parameters": [],
            "querystring_parameters": {}
          }
        ]
      }
    }
  ]
}

Start APIcast

THREESCALE_DEPLOYMENT_ENV=staging APICAST_LOG_LEVEL=warn APICAST_WORKER=1 APICAST_CONFIGURATION_LOADER=lazy APICAST_CONFIGURATION_CACHE=0 THREESCALE_CONFIG_FILE=apicast-config.json ./bin/apicast

Capture APIcast IP

APICAST_IP=$(docker inspect apicast_build_0-development-1 | yq e -P '.[0].NetworkSettings.Networks.apicast_build_0_default.IPAddress' -)

Send first request

curl -i -k -H "Host: one" "http://${APICAST_IP}:8080/?user_key="

You should receive HTTP/1.1 200 OK

Send second request, this will trigger the conditional policy

curl -i -k -H "Host: one" "http://${APICAST_IP}:8080/2?user_key="

You should receive HTTP/1.1 504 Gateway Time-out

HTTP/1.1 504 Gateway Time-out
Server: openresty
Date: Wed, 24 Jul 2024 05:31:47 GMT
Content-Type: text/plain
Transfer-Encoding: chunked
Connection: keep-alive

Stop APIcast

CTRL-C

Retry policy

Modify apicast-config.json as follow

diff --git a/apicast-config.json b/apicast-config.json
index 28744dd0..eda557b7 100644
--- a/apicast-config.json
+++ b/apicast-config.json
@@ -7,7 +7,7 @@
         "hosts": [
           "one"
         ],
-        "api_backend": "https://httpbingo.org/delay/",
+        "api_backend": "https://httpbingo.org/status/",
         "backend": {
           "endpoint": "http://127.0.0.1:8081",
           "host": "backend"
@@ -22,7 +22,7 @@
                     "left": "{{ uri }}",
                     "left_type": "liquid",
                     "op": "==",
-                    "right": "/2",
+                    "right": "/503",
                     "right_type": "plain"
                   }
                 ]
@@ -31,9 +31,7 @@
                 {
                   "name": "apicast.policy.retry",
                   "configuration": {
-                    "connect_timeout": 1,
-                    "send_timeout": 1,
-                    "read_timeout": 1
+                    "retries": 3
                   }
                 }
               ]

Start APIcast with APICAST_UPSTREAM_RETRY_CASES="error http_503 http_504"

THREESCALE_DEPLOYMENT_ENV=staging APICAST_LOG_LEVEL=warn APICAST_WORKER=1 APICAST_CONFIGURATION_LOADER=lazy APICAST_CONFIGURATION_CACHE=0 APICAST_UPSTREAM_RETRY_CASES="error http_503 http_504" THREESCALE_CONFIG_FILE=apicast-config.json ./bin/apicast

Send first request

curl -i -k -H "Host: one" "http://${APICAST_IP}:8080/504?user_key="

You should receive HTTP/1.1 504 Gateway Time-out immediately

Send second request, this will trigger the conditional policy

curl -i -k -H "Host: one" "http://${APICAST_IP}:8080/503?user_key="

Check the log and confirm that APIcast retry the request 3 times

Stop APIcast

CTRL-C

Camel proxy

Build a new run-time image

make runtime-image IMAGE_NAME=apicast-test

Move into camel-proxy dev-environment

cd dev-environments/camel-proxy

Edit apicast-config.json as follow

diff --git a/dev-environments/camel-proxy/apicast-config.json b/dev-environments/camel-proxy/apicast-config.json
index 91201afa..6cee023a 100644
--- a/dev-environments/camel-proxy/apicast-config.json
+++ b/dev-environments/camel-proxy/apicast-config.json
@@ -5,16 +5,34 @@
       "backend_version": "1",
       "proxy": {
         "hosts": ["http-proxy.example.com"],
-        "api_backend": "http://example.com:80/get",
+        "api_backend": "http://example.com:80/",
         "backend": {
           "endpoint": "http://127.0.0.1:8081",
           "host": "backend"
         },
         "policy_chain": [
           {
-            "name": "apicast.policy.camel",
+            "name": "apicast.policy.conditional",
             "configuration": {
-              "http_proxy": "http://proxy.socat:8080/"
+              "condition": {
+                "operations": [
+                  {
+                    "left": "{{ uri }}",
+                    "left_type": "liquid",
+                    "op": "==",
+                    "right": "/headers",
+                    "right_type": "plain"
+                  }
+                ]
+              },
+              "policy_chain": [
+                  {
+                    "name": "apicast.policy.camel",
+                    "configuration": {
+                      "http_proxy": "http://proxy.socat:8080/"
+                    }
+                  }
+              ]
             }
           },
           {

Generate certs

make certs

Start APIcast

make gateway IMAGE_NAME=apicast-test

Send first request

curl --resolve http-proxy.example.com:8080:127.0.0.1 -v "http://http-proxy.example.com:8080/get?user_key=123"

Expected response

< HTTP/1.1 200 OK
< Server: openresty
< Date: Wed, 24 Jul 2024 06:19:48 GMT
< Content-Type: application/json
< Content-Length: 221
< Connection: keep-alive
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Credentials: true
<
{
  "args": {
    "user_key": "123"
  },
  "headers": {
    "Accept": "*/*",
    "Host": "example.com",
    "User-Agent": "curl/7.61.1"
  },
  "origin": "172.20.0.4",
  "url": "http://example.com/get?user_key=123"
}
* Connection #0 to host http-proxy.example.com left intact

Send second request that will trigger camel proxy

curl --resolve http-proxy.example.com:8080:127.0.0.1 -v "http://http-proxy.example.com:8080/headers?user_key=123"

Expected response

< HTTP/1.1 200 OK
< Server: openresty
< Date: Wed, 24 Jul 2024 06:19:54 GMT
< Content-Type: application/json
< Content-Length: 138
< Connection: keep-alive
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Origin: *
<
{
  "HEADERS": {
    "ACCEPT": "*/*",
    "CONNECTION": "KEEP-ALIVE",
    "HOST": "EXAMPLE.COM",
    "USER-AGENT": "CURL/7.61.1"
  }
}
* Connection #0 to host http-proxy.example.com left intact

Stop APIcast

CTRL-C

HTTP Proxy policy

Move into http proxy dev-environment

cd http-proxy-plain-http-upstream

Edit apicast-config.json as follow

diff --git a/dev-environments/http-proxy-plain-http-upstream/apicast-config.json b/dev-environments/http-proxy-plain-http-upstream/apicast-config.json
index daa6967c..60231ba4 100644
--- a/dev-environments/http-proxy-plain-http-upstream/apicast-config.json
+++ b/dev-environments/http-proxy-plain-http-upstream/apicast-config.json
@@ -5,16 +5,34 @@
       "backend_version": "1",
       "proxy": {
         "hosts": ["get.example.com"],
-        "api_backend": "http://example.com/get",
+        "api_backend": "http://example.com/",
         "backend": {
           "endpoint": "http://127.0.0.1:8081",
           "host": "backend"
         },
         "policy_chain": [
           {
-            "name": "apicast.policy.http_proxy",
+            "name": "apicast.policy.conditional",
             "configuration": {
-              "http_proxy": "http://proxy:8080/"
+              "condition": {
+                "operations": [
+                  {
+                    "left": "{{ uri }}",
+                    "left_type": "liquid",
+                    "op": "==",
+                    "right": "/headers",
+                    "right_type": "plain"
+                  }
+                ]
+              },
+              "policy_chain": [
+                {
+                  "name": "apicast.policy.http_proxy",
+                  "configuration": {
+                    "http_proxy": "http://proxy:8080/"
+                  }
+                }
+              ]
             }
           },
           {

Start APIcast

make gateway IMAGE_NAME=apicast-test

Send first request

curl --resolve get.example.com:8080:127.0.0.1 -v "http://get.example.com:8080/get?user_key=123"

Check proxy logs and make sure that no request goes through the proxy

docker compose -p http-proxy-plain-http-upstream logs -f proxy

Send second request that will trigger camel proxy

curl --resolve get.example.com:8080:127.0.0.1 -v "http://get.example.com:8080/headers?user_key=123"

Check proxy logs again and this time we should see a new request in the log

 ▲ APIcast/dev-environments/http-proxy-plain-http-upstream docker compose -p http-proxy-plain-http-upstream logs -f proxy
proxy  | 2024/07/24 06:57:01 socat[1] N listening on AF=2 0.0.0.0:8080
proxy  | 2024/07/24 06:59:07 socat[1] N accepting connection from AF=2 172.22.0.6:56794 on AF=2 172.22.0.2:8080
proxy  | 2024/07/24 06:59:07 socat[1] N forked off child process 7
proxy  | 2024/07/24 06:59:07 socat[1] N listening on AF=2 0.0.0.0:8080
proxy  | 2024/07/24 06:59:07 socat[7] N opening connection to AF=2 172.22.0.4:443
proxy  | 2024/07/24 06:59:07 socat[7] N successfully connected from local address AF=2 172.22.0.2:46212
proxy  | 2024/07/24 06:59:07 socat[7] N starting data transfer loop with FDs [6,6] and [5,5]
proxy  | > 2024/07/24 06:59:07.000640505  length=136 from=0 to=135
proxy  | GET http://example.com/headers?user_key=123 HTTP/1.1\r
proxy  | X-Real-IP: 172.22.0.1\r
proxy  | Host: example.com\r
proxy  | User-Agent: curl/7.61.1\r
proxy  | Accept: */*\r
proxy  | \r
proxy  | < 2024/07/24 06:59:07.000643512  length=17 from=0 to=16
proxy  | HTTP/1.1 200 OK\r
proxy  | < 2024/07/24 06:59:07.000643558  length=64 from=17 to=80
proxy  | Via: 1.1 tinyproxy (tinyproxy/1.11.2)\r
proxy  | Server: gunicorn/19.9.0\r
proxy  | < 2024/07/24 06:59:07.000643618  length=69 from=81 to=149
proxy  | Date: Wed, 24 Jul 2024 06:59:07 GMT\r
proxy  | Content-Type: application/json\r
proxy  | < 2024/07/24 06:59:07.000643686  length=95 from=150 to=244
proxy  | Content-Length: 133\r
proxy  | Access-Control-Allow-Origin: *\r
proxy  | Access-Control-Allow-Credentials: true\r
proxy  | \r
proxy  | < 2024/07/24 06:59:07.000643763  length=133 from=245 to=377
proxy  | {
proxy  |   "headers": {
proxy  |     "Accept": "*/*",
proxy  |     "Connection": "close",
proxy  |     "Host": "example.com",
proxy  |     "User-Agent": "curl/7.61.1"
proxy  |   }
proxy  | }
proxy  | 2024/07/24 06:59:07 socat[7] N socket 2 (fd 5) is at EOF
proxy  | 2024/07/24 06:59:07 socat[7] N socket 1 (fd 6) is at EOF
proxy  | 2024/07/24 06:59:07 socat[7] N exiting with status 0
proxy  | 2024/07/24 06:59:07 socat[1] N childdied(): handling signal 17

Stop APIcast

CTRL-C

Request unbuffered policy

Move to plain-http-upstream dev-env

cd dev-environments/plain-http-upstream

Edit apicast-config.json as follow

diff --git a/dev-environments/plain-http-upstream/apicast-config.json b/dev-environments/plain-http-upstream/apicast-config.json
index 30c2db39..f832a2ca 100644
--- a/dev-environments/plain-http-upstream/apicast-config.json
+++ b/dev-environments/plain-http-upstream/apicast-config.json
@@ -38,6 +38,27 @@
           "host": "backend"
         },
         "policy_chain": [
+          {
+            "name": "apicast.policy.conditional",
+            "configuration": {
+              "condition": {
+                "operations": [
+                  {
+                    "left": "{{ headers['Foo'] }}",
+                    "left_type": "liquid",
+                    "op": "==",
+                    "right": "test",
+                    "right_type": "plain"
+                  }
+                ]
+              },
+              "policy_chain": [
+                {
+                   "name": "apicast.policy.request_unbuffered"
+                 }
+              ]
+            }
+          },
           {
             "name": "apicast.policy.apicast"
           }

Start APIcast

make gateway IMAGE_NAME=apicast-test

Generate a big enough file

fallocate -l 1M bigfile

Open another terminal to inspect traffic between APIcast and upstream

docker compose -p plain-http-upstream logs -f example.com

Send a request to APIcast

curl --resolve post.example.com:8080:127.0.0.1 -v -X POST -H "Transfer-Encoding: chunked" -F file=@bigfile "http://post.example.com:8080/?user_key=123"

The upstream returns 200, and checking example service log, you should see that APIcast send request with Content-Lenght header

example.com  | POST /post?user_key=123 HTTP/1.1\r
example.com  | X-Real-IP: 192.168.32.1\r
example.com  | Host: example.com\r
example.com  | Content-Length: 4295\r
example.com  | User-Agent: curl/7.61.1\r
example.com  | Accept: */*\r
example.com  | Content-Type: multipart/form-data; boundary=------------------------b5ca507060dd316c\r

Send a second request with extra header

curl --resolve post.example.com:8080:127.0.0.1 -v -X POST -H "Transfer-Encoding: chunked" -F file=@bigfile -H "Foo: test" "http://post.example.com:8080/?user_key=123"

This time we can see that upstream return 200 and APIcast send request to upstream with Transfer-encoding: chunked header

example.com  | POST /post?user_key=123 HTTP/1.1\r
example.com  | X-Real-IP: 192.168.32.1\r
example.com  | Host: example.com\r
example.com  | Transfer-Encoding: chunked\r
example.com  | User-Agent: curl/7.61.1\r
example.com  | Accept: */*\r
example.com  | Foo: test\r
example.com  | Content-Type: multipart/form-data; boundary=------------------------b0d2db867787e304\r

eguzki · 2024-07-26T07:54:25Z

This commit from this PR 32d110e Add support to set proxy buffer size looks like out of context.

eguzki · 2024-07-26T08:03:49Z

https://httpbingo.org/delay returns 404 and APIcast returns 400

[26/Jul/2024:07:59:14 +0000] one:8080 172.19.0.1:41290 "GET /?user_key= HTTP/1.1" 400 133 (0.289) 0

I have changed the first request to curl -i -k -H "Host: one" "http://${APICAST_IP}:8080/1?user_key=" and conditional Upstream connection policy works.

dfennessy

lgtm

eguzki · 2024-07-26T08:49:19Z

the apicast-config.json for the Request unbuffered policy is invalid json. FYI.

eguzki · 2024-07-26T08:55:30Z

Sorry to say that the verification steps for the Request unbuffered policy do not work for me. (Even after fixing the json issue with the apicast-config.json)

When the conditional policy activating header Foo: test, apicast says that condition met in conditional policy, but the upstream does not return the Transfer-encoding: chunked header

With this change, the policy will stil works regardless of it position in the chain.

tkan145 · 2024-07-26T10:33:01Z

the apicast-config.json for the Request unbuffered policy is invalid json. FYI.

Fixed

Sorry to say that the verification steps for the Request unbuffered policy do not work for me. (Even after fixing the json issue with the apicast-config.json)

Updated verification steps for request_unbuffered policy

eguzki · 2024-07-26T13:16:23Z

Still not working. Sorry to say. But I found the issue 🎉

The file is too small that apicast can read it from one shot so does not need to do transfer encoding chunked. When I tried with 1M file, it worked as expected.

The request with the header activating the conditional policy is not correct in the verifucation steps. I used this one:

curl --resolve post.example.com:8080:127.0.0.1 -v -X POST -H "Transfer-Encoding: chunked" -F file=@bigfile -H "Foo: test" "http://post.example.com:8080/?user_key=123"

tkan145 · 2024-08-07T01:16:38Z

My bad, I just realized I was using a 4k file this whole time.

Update verification steps to reflect comments

tkan145 requested a review from a team as a code owner July 11, 2024 06:30

tkan145 changed the title ~~[WIP] THREESCALE-9320 conditional policy~~ [WIP] [THREESCALE-9320]Conditional policy evaluating incorrectly: second policy in policy chain that implement export() always triggers Jul 12, 2024

tkan145 force-pushed the THREESCALE-9320-conditional-policy branch 2 times, most recently from 8946c81 to 7d51694 Compare July 25, 2024 02:36

tkan145 requested a review from a team as a code owner July 25, 2024 02:36

dfennessy approved these changes Jul 26, 2024

View reviewed changes

tkan145 added 5 commits July 26, 2024 20:29

[http_proxy] Move get_http_proxy function to rewrite phase

9193474

With this change, the policy will stil works regardless of it position in the chain.

[camel] Move get_http_proxy function to rewrite phase

14088b0

[upstream_connection] Move upstream_timeout options to rewrite phase

e5691c8

[proxy_unbuffered] Export context in rewrite phase

c2742df

[retry] Move context export to rewrite phase

967d0c8

tkan145 force-pushed the THREESCALE-9320-conditional-policy branch from 7d51694 to 967d0c8 Compare July 26, 2024 10:31

eguzki approved these changes Jul 26, 2024

View reviewed changes

[upstream_connection] use number type instead of integer

97a98fc

tkan145 merged commit 8607e7c into 3scale:master Aug 9, 2024
14 checks passed

tkan145 deleted the THREESCALE-9320-conditional-policy branch August 9, 2024 08:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[THREESCALE-9320] Conditional policy evaluating incorrectly: second policy in policy chain that implement export() always triggers #1485

[THREESCALE-9320] Conditional policy evaluating incorrectly: second policy in policy chain that implement export() always triggers #1485

tkan145 commented Jul 11, 2024 •

edited

Loading

eguzki commented Jul 26, 2024

eguzki commented Jul 26, 2024

dfennessy left a comment

eguzki commented Jul 26, 2024

eguzki commented Jul 26, 2024

tkan145 commented Jul 26, 2024

eguzki commented Jul 26, 2024 •

edited

Loading

tkan145 commented Aug 7, 2024

[THREESCALE-9320] Conditional policy evaluating incorrectly: second policy in policy chain that implement export() always triggers #1485

[THREESCALE-9320] Conditional policy evaluating incorrectly: second policy in policy chain that implement export() always triggers #1485

Conversation

tkan145 commented Jul 11, 2024 • edited Loading

What

Why

Verification steps

Upstream connection policy

Retry policy

Camel proxy

HTTP Proxy policy

Request unbuffered policy

eguzki commented Jul 26, 2024

eguzki commented Jul 26, 2024

dfennessy left a comment

Choose a reason for hiding this comment

eguzki commented Jul 26, 2024

eguzki commented Jul 26, 2024

tkan145 commented Jul 26, 2024

eguzki commented Jul 26, 2024 • edited Loading

tkan145 commented Aug 7, 2024

tkan145 commented Jul 11, 2024 •

edited

Loading

eguzki commented Jul 26, 2024 •

edited

Loading