-
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: Autogenerate admin api key if not passed #11080
Changes from 74 commits
33d8f9c
73efe8b
5f3f06f
cd4160c
2054e73
f1fe58b
4b64ef7
05ead26
9d18f3a
4449ed1
d80ba97
4c7d2b7
336a63f
6fa433d
b3ab95c
9dd6a1f
ef88059
7d2093f
e6d34a9
e07d3f5
e0a7d45
ea4f4f7
063afb2
4cda3f3
e5def8e
2cbad3a
d2fc7eb
465c276
7400092
e86ad6d
42195d5
b4cfcd2
0edef02
7a69e71
1b7dd60
7397903
12c24e6
420d1ab
5edae90
d0ffc54
24a217b
18239e9
b85a0fd
efa0e99
3f75740
3e37124
b6d5afd
7c9f1f1
da823ed
a3add33
11f96bb
311eb41
a3f44c7
4261bac
7d9f6a2
fd29bf5
d09b3be
a32251b
e6ff3a9
510f277
709e089
8dc0250
89ad574
23d28f0
adab320
ad6cf48
190001c
4988cb8
f5aaf05
c9f5f5a
f09685d
5b0c04e
09a7704
26a948a
39cc5c8
fae943b
3df4978
6357411
c851820
df9dc76
07afe44
ec03ce8
aacb574
258e1cf
277cc74
93c0e01
612ef88
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 |
---|---|---|
|
@@ -88,8 +88,15 @@ jobs: | |
|
||
- name: Test apisix | ||
run: | | ||
curl http://127.0.0.1:9180/apisix/admin/routes/1 \ | ||
-H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d ' | ||
wget https://github.com/mikefarah/yq/releases/download/3.4.1/yq_linux_amd64 -O /usr/bin/yq && sudo chmod +x /usr/bin/yq | ||
get_admin_key() { | ||
local admin_key=$(yq '.deployment.admin.admin_key[0].key' conf/config.yaml) | ||
echo "$admin_key" | ||
Revolyssup marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
export admin_key=$(get_admin_key); echo $admin_key | ||
cat conf/config.yaml | ||
curl -v http://127.0.0.1:9180/apisix/admin/routes/1 \ | ||
-H "X-API-KEY: $admin_key" -X PUT -d ' | ||
{ | ||
"uri": "/get", | ||
"upstream": { | ||
|
@@ -99,15 +106,15 @@ jobs: | |
} | ||
} | ||
}' | ||
result_code=`curl -I -m 10 -o /dev/null -s -w %{http_code} http://127.0.0.1:9080/get` | ||
if [[ $result_code -ne 200 ]]; then | ||
printf "result_code: %s\n" "$result_code" | ||
echo "===============access.log===============" | ||
cat logs/access.log | ||
echo "===============error.log===============" | ||
cat logs/error.log | ||
exit 125 | ||
fi | ||
result_code=`curl -I -m 10 -o /dev/null -s -w %{http_code} http://127.0.0.1:9080/get` | ||
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 change is not needed. 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. fixed |
||
if [[ $result_code -ne 200 ]]; then | ||
printf "result_code: %s\n" "$result_code" | ||
echo "===============access.log===============" | ||
cat logs/access.log | ||
echo "===============error.log===============" | ||
cat logs/error.log | ||
exit 125 | ||
fi | ||
|
||
- name: Check error log | ||
run: | | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,36 +20,38 @@ | |
-- @module core.id | ||
|
||
local fetch_local_conf = require("apisix.core.config_local").local_conf | ||
local try_read_attr = require("apisix.core.table").try_read_attr | ||
local log = require("apisix.core.log") | ||
local uuid = require('resty.jit-uuid') | ||
local smatch = string.match | ||
local open = io.open | ||
|
||
|
||
local try_read_attr = require("apisix.core.table").try_read_attr | ||
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. please maintain code beautification (equals on top of each other) 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. done |
||
local profile = require("apisix.core.profile") | ||
local log = require("apisix.core.log") | ||
local uuid = require("resty.jit-uuid") | ||
local smatch = string.match | ||
local open = io.open | ||
local lyaml = require("lyaml") | ||
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. Please put this line after 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. and beautify the code placing |
||
local type = type | ||
local ipairs = ipairs | ||
local string = string | ||
local math = math | ||
local prefix = ngx.config.prefix() | ||
local apisix_uid | ||
local pairs = pairs | ||
|
||
local _M = {version = 0.1} | ||
|
||
local _M = { version = 0.1 } | ||
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. please revert this change. |
||
|
||
local function rtrim(str) | ||
return smatch(str, "^(.-)%s*$") | ||
end | ||
|
||
|
||
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. two blank lines are needed between functions, please check in other places as well. |
||
local function read_file(path) | ||
local file = open(path, "rb") -- r read mode and b binary mode | ||
if not file then | ||
return nil | ||
end | ||
|
||
local content = file:read("*a") -- *a or *all reads the whole file | ||
local content = file:read("*a") -- *a or *all reads the whole file | ||
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. better no not introduce changes like this, as this is already a big PR. Plus, a PR should only contain changes meaningful to the PR title. |
||
file:close() | ||
return rtrim(content) | ||
end | ||
|
||
|
||
local function write_file(path, data) | ||
local file = open(path, "w+") | ||
if not file then | ||
|
@@ -61,19 +63,88 @@ local function write_file(path, data) | |
return true | ||
end | ||
|
||
local function generate_yaml(table) | ||
-- By default lyaml will parse null values as [] | ||
-- The following logic is a workaround so that null values are parsed as null | ||
local function replace_null(tbl) | ||
for k, v in pairs(tbl) do | ||
if type(v) == "table" then | ||
replace_null(v) | ||
elseif v == nil then | ||
tbl[k] = "<PLACEHOLDER>" | ||
end | ||
end | ||
end | ||
|
||
-- Replace null values with "<PLACEHOLDER>" | ||
replace_null(table) | ||
|
||
-- Convert Lua table to YAML string without parsing null values | ||
local yaml = lyaml.dump({ table }, { no_nil = true }) | ||
|
||
-- Replace "<PLACEHOLDER>" with null except for empty arrays | ||
yaml = yaml:gsub("<PLACEHOLDER>", "null"):gsub("%[%s*%]", "null") | ||
|
||
-- Ensure boolean values remain intact | ||
yaml = yaml:gsub(":%s*true%s*true", ": true"):gsub(":%s*false%s*true", ": false") | ||
|
||
-- Replace *no_nil with true | ||
yaml = yaml:gsub("&no_nil", "true") | ||
|
||
-- Remove any occurrences of *no_nil | ||
yaml = yaml:gsub(":%s*%*no_nil", ": true") | ||
|
||
-- Remove duplicates for boolean values | ||
yaml = yaml:gsub("true%s*true", "true"):gsub("false%s*false", "false") | ||
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 do we need those line ? 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 removed the unnecessary lines. |
||
|
||
return yaml | ||
end | ||
|
||
|
||
_M.gen_uuid_v4 = uuid.generate_v4 | ||
|
||
--- This will autogenerate the admin key if it's passed as an empty string in the configuration. | ||
local function autogenerate_admin_key(default_conf) | ||
local changed = false | ||
-- Check if deployment.admin.admin_key is not nil and it's an array | ||
local admin_keys = default_conf.deployment | ||
and default_conf.deployment.admin | ||
and default_conf.deployment.admin.admin_key | ||
if admin_keys and type(admin_keys) == "table" then | ||
for i, admin_key in ipairs(admin_keys) do | ||
if admin_key.role == "admin" and admin_key.key == "" then | ||
changed = true | ||
admin_keys[i].key = "" | ||
for _ = 1, 32 do | ||
admin_keys[i].key = admin_keys[i].key .. | ||
string.char(math.random(65, 90) + math.random(0, 1) * 32) | ||
end | ||
shreemaan-abhishek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
end | ||
end | ||
end | ||
return default_conf,changed | ||
end | ||
|
||
function _M.init() | ||
local local_conf = fetch_local_conf() | ||
|
||
local local_conf,changed = autogenerate_admin_key(local_conf) | ||
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. Only generate the admin key if the role is traditional and control plane. 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. okay
Revolyssup marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if changed then | ||
local yaml_conf = generate_yaml(local_conf) | ||
local local_conf_path = profile:yaml_path("config") | ||
local ok, err = write_file(local_conf_path, yaml_conf) | ||
if not ok then | ||
log.error(err) | ||
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 think you should panic if we can't write file. |
||
end | ||
end | ||
|
||
--allow user to specify a meaningful id as apisix instance id | ||
local uid_file_path = prefix .. "/conf/apisix.uid" | ||
apisix_uid = read_file(uid_file_path) | ||
if apisix_uid then | ||
return | ||
end | ||
|
||
--allow user to specify a meaningful id as apisix instance id | ||
local local_conf = fetch_local_conf() | ||
local id = try_read_attr(local_conf, "apisix", "id") | ||
if id then | ||
apisix_uid = local_conf.apisix.id | ||
|
@@ -100,5 +171,4 @@ function _M.get() | |
return apisix_uid | ||
end | ||
|
||
|
||
return _M |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,11 +24,12 @@ install_dependencies() { | |
|
||
# install build & runtime deps | ||
yum install -y wget tar gcc gcc-c++ automake autoconf libtool make unzip patch \ | ||
git sudo openldap-devel which ca-certificates \ | ||
git sudo openldap-devel which ca-certificates\ | ||
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. please remove such unnecessary changes. |
||
epel-release \ | ||
cpanminus perl \ | ||
openssl-devel | ||
|
||
yum install -y libyaml-devel | ||
# install newer curl | ||
yum makecache | ||
yum install -y libnghttp2-devel | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -170,7 +170,8 @@ nginx_config: # Config for render the template to generate n | |
stream: | ||
enable_access_log: false # Enable stream proxy access logging. | ||
access_log: logs/access_stream.log # Location of the stream access log. | ||
access_log_format: "$remote_addr [$time_local] $protocol $status $bytes_sent $bytes_received $session_time" # Customize log format: http://nginx.org/en/docs/varindex.html | ||
access_log_format: | | ||
"$remote_addr [$time_local] $protocol $status $bytes_sent $bytes_received $session_time" # Customize log format: http://nginx.org/en/docs/varindex.html | ||
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. due to this change, are you sure the comment won't be parsed as a part of the string? 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 change has nothing to do with this modification. If you think it is necessary, it is recommended to open a new PR to deal with it. 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. @Revolyssup please address this comment. |
||
access_log_format_escape: default # Escape default or json characters in variables. | ||
lua_shared_dict: # Nginx Lua shared memory zone. Size units are m or k. | ||
etcd-cluster-health-check-stream: 10m | ||
|
@@ -208,7 +209,8 @@ nginx_config: # Config for render the template to generate n | |
enable_access_log: true # Enable HTTP proxy access logging. | ||
access_log: logs/access.log # Location of the access log. | ||
access_log_buffer: 16384 # buffer size of access log. | ||
access_log_format: "$remote_addr - $remote_user [$time_local] $http_host \"$request\" $status $body_bytes_sent $request_time \"$http_referer\" \"$http_user_agent\" $upstream_addr $upstream_status $upstream_response_time \"$upstream_scheme://$upstream_host$upstream_uri\"" | ||
access_log_format: | | ||
"$remote_addr - $remote_user [$time_local] $http_host \"$request\" $status $body_bytes_sent $request_time \"$http_referer\" \"$http_user_agent\" $upstream_addr $upstream_status $upstream_response_time \"$upstream_scheme://$upstream_host$upstream_uri\"" | ||
Comment on lines
+212
to
+213
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 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. The lyaml.dump function which is being used here to convert lua table into yaml, messes up the formatting and adds a new line automatically. Using |
||
# Customize log format: http://nginx.org/en/docs/varindex.html | ||
access_log_format_escape: default # Escape default or json characters in variables. | ||
keepalive_timeout: 60s # Set the maximum time for which TCP connection keeps alive. | ||
|
@@ -643,7 +645,7 @@ deployment: # Deployment configurations | |
admin_key: | ||
- | ||
name: admin # admin: write access to configurations. | ||
key: edd1c9f034335f136f87ad84b625c8f1 # Set API key for the admin of Admin API. | ||
key: '' # Set API key for the admin of Admin API. | ||
role: admin | ||
- | ||
name: viewer # viewer: read-only to configurations. | ||
|
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.
this block has 4 indents, where as everywhere in this file there are 2 indents.
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.
fixed