From 6fde3a7ecdb05120555b87db9e2d54d4515f90dc Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 27 Nov 2023 14:55:51 +0545 Subject: [PATCH 1/6] fix(plugin/opa): safely remove upstream when sending route to opa --- apisix/plugins/opa/helper.lua | 3 ++- ci/pod/opa/allow_get.rego | 20 ++++++++++++++++ t/plugin/opa2.t | 43 +++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 ci/pod/opa/allow_get.rego diff --git a/apisix/plugins/opa/helper.lua b/apisix/plugins/opa/helper.lua index abcb44b31c9b..638adcf0ef87 100644 --- a/apisix/plugins/opa/helper.lua +++ b/apisix/plugins/opa/helper.lua @@ -49,9 +49,10 @@ end local function build_http_route(conf, ctx, remove_upstream) - local route = core.table.clone(ctx.matched_route).value + local route = core.table.deepcopy(ctx.matched_route).value if remove_upstream and route and route.upstream then + -- unimportant to send upstream info to OPA route.upstream = nil end diff --git a/ci/pod/opa/allow_get.rego b/ci/pod/opa/allow_get.rego new file mode 100644 index 000000000000..642e1d883005 --- /dev/null +++ b/ci/pod/opa/allow_get.rego @@ -0,0 +1,20 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +package allow_get +default allow = false +allow { input.request.method == "GET"} +status_code = 201 {not allow} diff --git a/t/plugin/opa2.t b/t/plugin/opa2.t index 6bd5d2ba04fa..37033838ca79 100644 --- a/t/plugin/opa2.t +++ b/t/plugin/opa2.t @@ -224,3 +224,46 @@ GET /hello?user=elisa --- error_code: 403 --- response_body chomp {"info":[]} + + + +=== TEST 9: create route for testing with `with_route: true` and opa validation is successful +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "opa": { + "host": "http://127.0.0.1:8181", + "policy": "allow_get", + "with_route": true + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 10: hit route +--- request +GET /hello +--- error_code: 200 From 961da2161ca81aaa4f14c07cdb8d9863a294ea1f Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 27 Nov 2023 17:27:31 +0545 Subject: [PATCH 2/6] add allow_get policy --- ci/pod/docker-compose.plugin.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ci/pod/docker-compose.plugin.yml b/ci/pod/docker-compose.plugin.yml index 748b28b868f6..bd39e17180ef 100644 --- a/ci/pod/docker-compose.plugin.yml +++ b/ci/pod/docker-compose.plugin.yml @@ -182,8 +182,11 @@ services: restart: unless-stopped ports: - 8181:8181 - command: run -s /example.rego /echo.rego /data.json + command: run -s /example.rego /echo.rego /data.json /allow_get.rego volumes: + - type: bind + source: ./ci/pod/opa/allow_get.rego + target: /allow_get.rego - type: bind source: ./ci/pod/opa/example.rego target: /example.rego From 86e1b46509471aecc66fd679ebc81a020c1c3650 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Tue, 28 Nov 2023 22:48:07 +0545 Subject: [PATCH 3/6] replace `allow_get` with `with route` --- ci/pod/docker-compose.plugin.yml | 6 +++--- ci/pod/opa/allow_get.rego | 2 +- t/plugin/opa2.t | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ci/pod/docker-compose.plugin.yml b/ci/pod/docker-compose.plugin.yml index bd39e17180ef..13a34e3908f7 100644 --- a/ci/pod/docker-compose.plugin.yml +++ b/ci/pod/docker-compose.plugin.yml @@ -182,11 +182,11 @@ services: restart: unless-stopped ports: - 8181:8181 - command: run -s /example.rego /echo.rego /data.json /allow_get.rego + command: run -s /example.rego /echo.rego /data.json /with_route.rego volumes: - type: bind - source: ./ci/pod/opa/allow_get.rego - target: /allow_get.rego + source: ./ci/pod/opa/with_route.rego + target: /with_route.rego - type: bind source: ./ci/pod/opa/example.rego target: /example.rego diff --git a/ci/pod/opa/allow_get.rego b/ci/pod/opa/allow_get.rego index 642e1d883005..b32413678b35 100644 --- a/ci/pod/opa/allow_get.rego +++ b/ci/pod/opa/allow_get.rego @@ -14,7 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # -package allow_get +package with_route default allow = false allow { input.request.method == "GET"} status_code = 201 {not allow} diff --git a/t/plugin/opa2.t b/t/plugin/opa2.t index 37033838ca79..5c675d5069e5 100644 --- a/t/plugin/opa2.t +++ b/t/plugin/opa2.t @@ -238,7 +238,7 @@ GET /hello?user=elisa "plugins": { "opa": { "host": "http://127.0.0.1:8181", - "policy": "allow_get", + "policy": "with_route", "with_route": true } }, From c1c15f4e7cc6ab3fcdfa27f1289c43792e57179a Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 29 Nov 2023 16:13:06 +0545 Subject: [PATCH 4/6] rename to `with_route` --- ci/pod/opa/{allow_get.rego => with_route.rego} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename ci/pod/opa/{allow_get.rego => with_route.rego} (100%) diff --git a/ci/pod/opa/allow_get.rego b/ci/pod/opa/with_route.rego similarity index 100% rename from ci/pod/opa/allow_get.rego rename to ci/pod/opa/with_route.rego From 8d9bebebcab60498714b0d518003ea2507066bb2 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 29 Nov 2023 17:29:04 +0545 Subject: [PATCH 5/6] use route information --- ci/pod/opa/with_route.rego | 8 +++++-- t/plugin/opa2.t | 47 +++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/ci/pod/opa/with_route.rego b/ci/pod/opa/with_route.rego index b32413678b35..6ec5620fe2f2 100644 --- a/ci/pod/opa/with_route.rego +++ b/ci/pod/opa/with_route.rego @@ -16,5 +16,9 @@ # package with_route default allow = false -allow { input.request.method == "GET"} -status_code = 201 {not allow} + +allow { + input.route.name == "valid" +} + +status_code = 403 {not allow} diff --git a/t/plugin/opa2.t b/t/plugin/opa2.t index 5c675d5069e5..2dd087789a69 100644 --- a/t/plugin/opa2.t +++ b/t/plugin/opa2.t @@ -227,7 +227,7 @@ GET /hello?user=elisa -=== TEST 9: create route for testing with `with_route: true` and opa validation is successful +=== TEST 9: create route: `with_route = true` and opa validation passes when route name == "valid" --- config location /t { content_by_lua_block { @@ -235,6 +235,7 @@ GET /hello?user=elisa local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, [[{ + "name": "valid", "plugins": { "opa": { "host": "http://127.0.0.1:8181", @@ -267,3 +268,47 @@ passed --- request GET /hello --- error_code: 200 + + + +=== TEST 11: create route: `with_route = true` and opa validation fails when route name != "valid" +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "name": "not_valid", + "plugins": { + "opa": { + "host": "http://127.0.0.1:8181", + "policy": "with_route", + "with_route": true + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 12: hit route +--- request +GET /hello +--- error_code: 403 From 794123c789d0731d19359216ae623344a6b18871 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 29 Nov 2023 17:30:45 +0545 Subject: [PATCH 6/6] remove whitespace --- ci/pod/opa/with_route.rego | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/pod/opa/with_route.rego b/ci/pod/opa/with_route.rego index 6ec5620fe2f2..c6a848e7a16d 100644 --- a/ci/pod/opa/with_route.rego +++ b/ci/pod/opa/with_route.rego @@ -17,7 +17,7 @@ package with_route default allow = false -allow { +allow { input.route.name == "valid" }