-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
feat: use env var instead of plain text for vault token #8866
Changes from 8 commits
aa8b1bd
7f938e2
c13c6ec
6bc6d61
111ac11
2d988b7
62eb6f9
26627b7
0868b43
c562216
206dd90
3aeed0d
6c8cf91
756f3b4
37a3406
d4d5fb3
30a296b
1626dce
46b992e
f2c1c27
9bfaede
234346a
8b72f12
74b3e66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ local norm_path = require("pl.path").normpath | |
local sub = core.string.sub | ||
local rfind_char = core.string.rfind_char | ||
|
||
local env = require("apisix.core.env") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we import it like |
||
|
||
local schema = { | ||
type = "object", | ||
|
@@ -53,10 +54,15 @@ local function make_request_to_vault(conf, method, key, data) | |
local req_addr = conf.uri .. norm_path("/v1/" | ||
.. conf.prefix .. "/" .. key) | ||
|
||
local token, _ = env.fetch_by_uri(conf.token) | ||
soulbird marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if not token then | ||
token = conf.token | ||
end | ||
|
||
local res, err = httpc:request_uri(req_addr, { | ||
method = method, | ||
headers = { | ||
["X-Vault-Token"] = conf.token | ||
["X-Vault-Token"] = token | ||
}, | ||
body = core.json.encode(data or {}, true) | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,10 @@ | |
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
BEGIN { | ||
$ENV{VAULT_TOKEN} = "root"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this added? |
||
} | ||
|
||
use t::APISIX 'no_plan'; | ||
|
||
repeat_each(1); | ||
|
@@ -388,3 +392,131 @@ env secret=apisix; | |
GET /t | ||
--- response_body | ||
nil | ||
|
||
|
||
|
||
=== TEST 17: validate secret/vault with the token in an env var: wrong schema | ||
--- apisix_yaml | ||
secrets: | ||
- id: vault/1 | ||
prefix: kv/apisix | ||
token: "$ENV://VAULT_TOKEN" | ||
uri: 127.0.0.1:8200 | ||
#END | ||
--- config | ||
location /t { | ||
content_by_lua_block { | ||
local secret = require("apisix.secret") | ||
local values = secret.secrets() | ||
ngx.say(#values) | ||
} | ||
} | ||
--- request | ||
GET /t | ||
--- response_body | ||
0 | ||
--- error_log | ||
property "uri" validation failed: failed to match pattern "^[^\\/]+:\\/\\/([\\da-zA-Z.-]+|\\[[\\da-fA-F:]+\\])(:\\d+)?" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why add this test case? It doesn't look like it has anything to do with your modification? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just duplicated all the test cases containing the token with just one change: replace the token with the environment variable. Let me do something better. 😂 |
||
|
||
|
||
|
||
=== TEST 18: validate secrets with the token in an env var: manager not exits | ||
--- apisix_yaml | ||
secrets: | ||
- id: hhh/1 | ||
prefix: kv/apisix | ||
token: "$ENV://VAULT_TOKEN" | ||
uri: 127.0.0.1:8200 | ||
#END | ||
--- config | ||
location /t { | ||
content_by_lua_block { | ||
local secret = require("apisix.secret") | ||
local values = secret.secrets() | ||
ngx.say(#values) | ||
} | ||
} | ||
--- request | ||
GET /t | ||
--- response_body | ||
0 | ||
--- error_log | ||
secret manager not exits | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
|
||
|
||
|
||
=== TEST 19: load config normal with the token in an env var | ||
--- apisix_yaml | ||
secrets: | ||
- id: vault/1 | ||
prefix: kv/apisix | ||
token: "$ENV://VAULT_TOKEN" | ||
uri: http://127.0.0.1:8200 | ||
#END | ||
--- config | ||
location /t { | ||
content_by_lua_block { | ||
local secret = require("apisix.secret") | ||
local values = secret.secrets() | ||
ngx.say("len: ", #values) | ||
|
||
ngx.say("id: ", values[1].value.id) | ||
ngx.say("prefix: ", values[1].value.prefix) | ||
ngx.say("token: ", values[1].value.token) | ||
ngx.say("uri: ", values[1].value.uri) | ||
} | ||
} | ||
--- request | ||
GET /t | ||
--- response_body | ||
len: 1 | ||
id: vault/1 | ||
prefix: kv/apisix | ||
token: $ENV://VAULT_TOKEN | ||
uri: http://127.0.0.1:8200 | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to add this test |
||
|
||
|
||
=== TEST 20: secret.fetch_by_uri with the token in an env var: start with $secret:// | ||
--- apisix_yaml | ||
secrets: | ||
- id: vault/1 | ||
prefix: kv/apisix | ||
token: "$ENV://VAULT_TOKEN" | ||
uri: http://127.0.0.1:8200 | ||
#END | ||
--- config | ||
location /t { | ||
content_by_lua_block { | ||
local secret = require("apisix.secret") | ||
local value = secret.fetch_by_uri("$secret://vault/1/apisix-key/key") | ||
ngx.say(value) | ||
} | ||
} | ||
--- request | ||
GET /t | ||
--- response_body | ||
value | ||
|
||
|
||
|
||
=== TEST 21: secret.fetch_by_uri, no sub key value with the token in an env var | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test case already exists |
||
--- apisix_yaml | ||
secrets: | ||
- id: vault/1 | ||
prefix: kv/apisix | ||
token: "$ENV://VAULT_TOKEN" | ||
uri: http://127.0.0.1:8200 | ||
#END | ||
--- config | ||
location /t { | ||
content_by_lua_block { | ||
local secret = require("apisix.secret") | ||
local value = secret.fetch_by_uri("$secret://vault/1/apisix-key/bar") | ||
ngx.say(value) | ||
} | ||
} | ||
--- request | ||
GET /t | ||
--- response_body | ||
nil |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,10 @@ | |
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
BEGIN { | ||
$ENV{VAULT_TOKEN} = "root"; | ||
} | ||
|
||
use t::APISIX 'no_plan'; | ||
|
||
repeat_each(2); | ||
|
@@ -540,3 +544,73 @@ GET /echo | |
Authorization: Basic Zm9vOmJhcg== | ||
--- response_headers | ||
Authorization: Basic Zm9vOmJhcg== | ||
|
||
|
||
|
||
=== TEST 25: set basic-auth conf with the token in an env var: password uses secret ref | ||
--- request | ||
GET /t | ||
--- config | ||
location /t { | ||
content_by_lua_block { | ||
local t = require("lib.test_admin").test | ||
-- put secret vault config | ||
local code, body = t('/apisix/admin/secrets/vault/test1', | ||
ngx.HTTP_PUT, | ||
[[{ | ||
"uri": "http://127.0.0.1:8200", | ||
"prefix" : "kv/apisix", | ||
"token" : "$ENV://VAULT_TOKEN" | ||
}]] | ||
) | ||
|
||
if code >= 300 then | ||
ngx.status = code | ||
return ngx.say(body) | ||
end | ||
|
||
-- change consumer with secrets ref: vault | ||
code, body = t('/apisix/admin/consumers', | ||
ngx.HTTP_PUT, | ||
[[{ | ||
"username": "foo", | ||
"plugins": { | ||
"basic-auth": { | ||
"username": "foo", | ||
"password": "$secret://vault/test1/foo/passwd" | ||
} | ||
} | ||
}]] | ||
) | ||
if code >= 300 then | ||
ngx.status = code | ||
return ngx.say(body) | ||
end | ||
|
||
-- set route | ||
code, body = t('/apisix/admin/routes/1', | ||
ngx.HTTP_PUT, | ||
[[{ | ||
"plugins": { | ||
"basic-auth": { | ||
"hide_credentials": false | ||
} | ||
}, | ||
"upstream": { | ||
"nodes": { | ||
"127.0.0.1:1980": 1 | ||
}, | ||
"type": "roundrobin" | ||
}, | ||
"uri": "/echo" | ||
}]] | ||
) | ||
|
||
if code >= 300 then | ||
ngx.status = code | ||
end | ||
ngx.say(body) | ||
} | ||
} | ||
--- response_body | ||
passed | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You only configured the route, you also need to verify whether the route is effective |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can avoid the blank line here? Other files don't add a blank line among localized variables.