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

feat: Autogenerate admin api key if not passed #11080

Merged
merged 87 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from 81 commits
Commits
Show all changes
87 commits
Select commit Hold shift + click to select a range
33d8f9c
feat: Autogenerate admin api key if not passed
Revolyssup Mar 22, 2024
73efe8b
fix function
Revolyssup Mar 24, 2024
5f3f06f
fix function
Revolyssup Mar 25, 2024
cd4160c
use lyaml dump
Revolyssup Mar 27, 2024
2054e73
Change makefile
Revolyssup Mar 27, 2024
f1fe58b
Change rockspec
Revolyssup Mar 27, 2024
4b64ef7
fix merge conflict
Revolyssup Mar 27, 2024
05ead26
fix CI
Revolyssup Mar 27, 2024
9d18f3a
fix ci
Revolyssup Mar 27, 2024
4449ed1
change install prefix
Revolyssup Mar 27, 2024
d80ba97
add YAML_DIR in luaconfig CI
Revolyssup Mar 27, 2024
4c7d2b7
install libyaml-devel
Revolyssup Mar 27, 2024
336a63f
fix CI
Revolyssup Mar 27, 2024
6fa433d
use flag to install libyaml-devel
Revolyssup Mar 27, 2024
b3ab95c
fix name on ubuntu
Revolyssup Mar 27, 2024
9dd6a1f
fix lint
Revolyssup Mar 27, 2024
ef88059
fix lint
Revolyssup Mar 27, 2024
7d2093f
fix lint
Revolyssup Mar 27, 2024
e6d34a9
add function to extract $admin_key
Revolyssup Mar 27, 2024
e07d3f5
echo for debug
Revolyssup Mar 27, 2024
e0a7d45
add log for debug
Revolyssup Mar 27, 2024
ea4f4f7
add log
Revolyssup Mar 27, 2024
063afb2
add verbose flag
Revolyssup Mar 27, 2024
4cda3f3
fix CI by export
Revolyssup Mar 27, 2024
e5def8e
fix ci
Revolyssup Mar 27, 2024
2cbad3a
use yq
Revolyssup Mar 27, 2024
d2fc7eb
download yq
Revolyssup Mar 27, 2024
465c276
add yq
Revolyssup Mar 27, 2024
7400092
install yq in common
Revolyssup Mar 27, 2024
e86ad6d
remove eval
Revolyssup Mar 27, 2024
42195d5
print file
Revolyssup Mar 27, 2024
b4cfcd2
allow key to be null
Revolyssup Mar 28, 2024
0edef02
add log
Revolyssup Mar 28, 2024
7a69e71
add log
Revolyssup Mar 28, 2024
1b7dd60
change logic to only generate when admin key empty
Revolyssup Mar 28, 2024
7397903
remove sudo
Revolyssup Mar 28, 2024
12c24e6
change logic to only generate when admin key empty
Revolyssup Mar 28, 2024
420d1ab
add log
Revolyssup Mar 28, 2024
5edae90
add log
Revolyssup Mar 28, 2024
d0ffc54
add log
Revolyssup Mar 28, 2024
24a217b
check config.yaml source-install
Revolyssup Mar 28, 2024
18239e9
generate key when passed empty
Revolyssup Mar 28, 2024
b85a0fd
fix lint
Revolyssup Mar 28, 2024
efa0e99
fix lint
Revolyssup Mar 28, 2024
3f75740
do not write file if not changed
Revolyssup Mar 28, 2024
3e37124
fix admin/api.t
Revolyssup Mar 28, 2024
b6d5afd
fix test_admin.sh
Revolyssup Mar 28, 2024
7c9f1f1
cat configyaml
Revolyssup Mar 28, 2024
da823ed
fix lint and cat config for CLI test
Revolyssup Mar 28, 2024
a3add33
add more logs
Revolyssup Mar 28, 2024
11f96bb
fix lint
Revolyssup Mar 28, 2024
311eb41
just ls
Revolyssup Mar 28, 2024
a3f44c7
fix cli tests
Revolyssup Mar 29, 2024
4261bac
merge conflict
Revolyssup Mar 29, 2024
7d9f6a2
remove "" from token
Revolyssup Mar 29, 2024
fd29bf5
fix test_cmd
Revolyssup Mar 29, 2024
d09b3be
fix test_dns.sh
Revolyssup Mar 29, 2024
a32251b
fix test_prometheus
Revolyssup Mar 29, 2024
e6ff3a9
fix fuzzing test
Revolyssup Mar 29, 2024
510f277
fix fuzzing
Revolyssup Mar 29, 2024
709e089
fix lint
Revolyssup Mar 29, 2024
8dc0250
fix lint
Revolyssup Mar 29, 2024
89ad574
fix simple http py
Revolyssup Mar 29, 2024
23d28f0
log key
Revolyssup Mar 29, 2024
adab320
remove "" from key
Revolyssup Mar 29, 2024
ad6cf48
Merge branch 'revolyssup/autogeneratekey' of github.com:Revolyssup/ap…
Revolyssup Mar 29, 2024
190001c
remove "" fuzzing
Revolyssup Mar 30, 2024
4988cb8
print key
Revolyssup Mar 30, 2024
f5aaf05
cat config in fuzzing
Revolyssup Mar 30, 2024
c9f5f5a
fix simple_http
Revolyssup Mar 30, 2024
f09685d
add key in http upstream
Revolyssup Mar 30, 2024
5b0c04e
apply workaround for no_nil
Revolyssup Mar 30, 2024
09a7704
fix lint issue
Revolyssup Mar 30, 2024
26a948a
fix lint
Revolyssup Mar 30, 2024
39cc5c8
Apply suggestions from code review
Revolyssup Mar 31, 2024
fae943b
apply suggestions from code review
Revolyssup Apr 1, 2024
3df4978
Merge branch 'revolyssup/autogeneratekey' of github.com:Revolyssup/ap…
Revolyssup Apr 1, 2024
6357411
fix lint
Revolyssup Apr 1, 2024
c851820
remove unnecessary code from generate_yaml
Revolyssup Apr 1, 2024
df9dc76
update docs
Revolyssup Apr 1, 2024
07afe44
fix doc lint
Revolyssup Apr 1, 2024
ec03ce8
apply doc suggestion
Revolyssup Apr 2, 2024
aacb574
apply suggestions
Revolyssup Apr 2, 2024
258e1cf
fix lint
Revolyssup Apr 2, 2024
277cc74
apply code suggestions from review
Revolyssup Apr 9, 2024
93c0e01
remove space from doc
Revolyssup Apr 9, 2024
612ef88
apply suggestion
Revolyssup Apr 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions .github/workflows/source-install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

get_admin_key() {
local admin_key=$(yq '.deployment.admin.admin_key[0].key' conf/config.yaml)
echo "$admin_key"
}
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": {
Expand All @@ -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`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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: |
Expand Down
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ ENV_DOCKER_COMPOSE ?= docker-compose --project-directory $(CURDIR) -p $(proj
ENV_NGINX ?= $(ENV_NGINX_EXEC) -p $(CURDIR) -c $(CURDIR)/conf/nginx.conf
ENV_NGINX_EXEC := $(shell command -v openresty 2>/dev/null || command -v nginx 2>/dev/null)
ENV_OPENSSL_PREFIX ?= /usr/local/openresty/openssl3
ENV_LIBYAML_INSTALL_PREFIX ?= /usr
ENV_LUAROCKS ?= luarocks
## These variables can be injected by luarocks
ENV_INST_PREFIX ?= /usr
Expand Down Expand Up @@ -131,6 +132,7 @@ deps: install-runtime
mkdir -p ~/.luarocks; \
$(ENV_LUAROCKS) config $(ENV_LUAROCKS_FLAG_LOCAL) variables.OPENSSL_LIBDIR $(addprefix $(ENV_OPENSSL_PREFIX), /lib); \
$(ENV_LUAROCKS) config $(ENV_LUAROCKS_FLAG_LOCAL) variables.OPENSSL_INCDIR $(addprefix $(ENV_OPENSSL_PREFIX), /include); \
$(ENV_LUAROCKS) config $(ENV_LUAROCKS_FLAG_LOCAL) variables.YAML_DIR $(ENV_LIBYAML_INSTALL_PREFIX); \
$(ENV_LUAROCKS) install apisix-master-0.rockspec --tree deps --only-deps $(ENV_LUAROCKS_SERVER_OPT); \
else \
$(call func_echo_warn_status, "WARNING: You're not using LuaRocks 3.x; please remove the luarocks and reinstall it via https://raw.githubusercontent.com/apache/apisix/master/utils/linux-install-luarocks.sh"); \
Expand Down
1 change: 1 addition & 0 deletions apisix-master-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ description = {

dependencies = {
"lua-resty-ctxdump = 0.1-0",
"lyaml = 6.2.8",
"api7-lua-resty-dns-client = 7.0.1",
"lua-resty-template = 2.0",
"lua-resty-etcd = 1.10.5",
Expand Down
2 changes: 1 addition & 1 deletion apisix/admin/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ local function set_ctx_and_check_token()
local ok, err = check_token(api_ctx)
if not ok then
core.log.warn("failed to check token: ", err)
core.response.exit(401, { error_msg = "failed to check token" })
core.response.exit(401, { error_msg = "failed to check token", description = err })
end
end

Expand Down
7 changes: 2 additions & 5 deletions apisix/cli/ops.lua
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,9 @@ Please modify "admin_key" in conf/config.yaml .
end

if admin.key == "" then
util.die(help:format("ERROR: missing valid Admin API token."), "\n")
end

if admin.key == "edd1c9f034335f136f87ad84b625c8f1" then
stderr:write(
help:format([[WARNING: using fixed Admin API token has security risk.]]),
help:format([[WARNING: using empty Admin API.
This will trigger APISIX to automatically generate a random Admin API token.]]),
"\n"
)
end
Expand Down
87 changes: 76 additions & 11 deletions apisix/core/id.lua
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,22 @@
-- @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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please maintain code beautification (equals on top of each other)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 lyaml = require("lyaml")
local smatch = string.match
local open = io.open
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 }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert this change.



local function rtrim(str)
Expand Down Expand Up @@ -62,18 +67,78 @@ local function write_file(path, data)
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)
local yaml = lyaml.dump({ table })
yaml = yaml:gsub("<PLACEHOLDER>", "null"):gsub("%[%s*%]", "null")
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.role is either traditional or control_plane
local deployment_role = default_conf.deployment and default_conf.deployment.role
if deployment_role and (deployment_role == "traditional" or
deployment_role == "control_plane") then
-- Check if deployment.admin.admin_key is not nil and it's an empty string
local admin_keys = default_conf.deployment
and default_conf.deployment.admin
and default_conf.deployment.admin.admin_key
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can try core.table.try_read_attr to improve your code.

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
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)
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)
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand All @@ -89,6 +154,7 @@ function _M.init()
end
end


---
-- Returns the instance id of the running APISIX
--
Expand All @@ -100,5 +166,4 @@ function _M.get()
return apisix_uid
end


return _M
8 changes: 4 additions & 4 deletions benchmark/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ sleep 3

#############################################
echo -e "\n\napisix: $worker_cnt worker + $upstream_cnt upstream + no plugin"

curl http://127.0.0.1:9180/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
admin_key=$(yq '.deployment.admin.admin_key[0].key' conf/config.yaml | sed 's/"//g')
curl http://127.0.0.1:9180/apisix/admin/routes/1 -H "X-API-KEY: $admin_key" -X PUT -d '
{
"uri": "/hello",
"plugins": {
Expand All @@ -100,8 +100,8 @@ sleep 1

#############################################
echo -e "\n\napisix: $worker_cnt worker + $upstream_cnt upstream + 2 plugins (limit-count + prometheus)"

curl http://127.0.0.1:9180/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
admin_key=$(yq '.deployment.admin.admin_key[0].key' conf/config.yaml | sed 's/"//g')
curl http://127.0.0.1:9180/apisix/admin/routes/1 -H "X-API-KEY: $admin_key" -X PUT -d '
{
"uri": "/hello",
"plugins": {
Expand Down
1 change: 1 addition & 0 deletions ci/centos7-ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ install_dependencies() {
cpanminus perl \
openssl-devel

yum install -y libyaml-devel
# install newer curl
yum makecache
yum install -y libnghttp2-devel
Expand Down
3 changes: 3 additions & 0 deletions ci/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export_version_info() {

export_or_prefix() {
export OPENRESTY_PREFIX="/usr/local/openresty"

export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$OPENRESTY_PREFIX/bin:$PATH
export OPENSSL_PREFIX=$OPENRESTY_PREFIX/openssl3
export OPENSSL_BIN=$OPENSSL_PREFIX/bin/openssl
Expand Down Expand Up @@ -178,6 +179,8 @@ GRPC_SERVER_EXAMPLE_VER=20210819
linux_get_dependencies () {
apt update
apt install -y cpanminus build-essential libncurses5-dev libreadline-dev libssl-dev perl libpcre3 libpcre3-dev libldap2-dev
apt-get install -y libyaml-dev
wget https://github.com/mikefarah/yq/releases/download/3.4.1/yq_linux_amd64 -O /usr/bin/yq && sudo chmod +x /usr/bin/yq
}

function start_grpc_server_example() {
Expand Down
2 changes: 1 addition & 1 deletion ci/redhat-ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ install_dependencies() {
wget tar gcc gcc-c++ automake autoconf libtool make unzip git sudo openldap-devel hostname patch \
which ca-certificates pcre pcre-devel xz \
openssl-devel

yum install -y libyaml-devel
yum install -y --disablerepo=* --enablerepo=ubi-8-appstream-rpms --enablerepo=ubi-8-baseos-rpms cpanminus perl

# install newer curl
Expand Down
8 changes: 5 additions & 3 deletions conf/config-default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 | operator avoids this issue by allowing for multiline string

# 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.
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion conf/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,5 @@ deployment:
admin:
admin_key:
- name: admin
key: edd1c9f034335f136f87ad84b625c8f1 # using fixed API token has security risk, please update it when you deploy to production environment
key: '' # using fixed API token has security risk, please update it when you deploy to production environment. If passed empty then will be autogenerated by APISIX and will be written back here. Recommended is to use external mechanism to generate and store the token.
role: admin
Loading
Loading