Skip to content

Conversation

@membphis
Copy link
Member

@membphis membphis commented Aug 20, 2020

What this PR does / why we need it:

as title

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible?

@moonming
Copy link
Member

@membphis ping

@membphis membphis marked this pull request as ready for review August 26, 2020 16:40
@membphis
Copy link
Member Author

@apache/apisix-committers please take a look at this PR, it is ready to merge

admin.key = tostring(admin.key)
end

if admin.key == "" or admin.key:gsub("*", "") == "" then
Copy link
Member

Choose a reason for hiding this comment

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

After the user installs Apache APISIX, it will definitely fail at start, which is a very bad experience.
As discussed in the mailing list, we need to automatically generate a random key when apisix starts

Copy link
Member Author

Choose a reason for hiding this comment

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

$ ./bin/apisix start
ERROR: missing valid apisix.admin_key

You can call `./bin/apisix gen_admin_key` to generate a new Admin API key or
manually update the `conf/config.yaml` file.

$ ./bin/apisix gen_admin_key
generate admin key successfully

$ ./bin/apisix start

$ ps -ef | grep apisix
resty     551863       1  0 06:45 ?        00:00:00 nginx: master process openresty -p /home/resty/git/membphis/apisix -c /home/resty/git/membphis/apisix/conf/nginx.conf
resty     552021  253872  0 06:46 pts/0    00:00:00 grep --color=auto --exclude-dir=.bzr --exclude-dir=CVS --exclude-dir=.git --exclude-dir=.hg --exclude-dir=.svn apisix

Copy link
Member Author

Choose a reason for hiding this comment

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

the user needs to call apisix gen_admin_key if missing a valid Admin API key.

Copy link
Member

Choose a reason for hiding this comment

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

can we add gen_admin_key to the start function?

Copy link
Member Author

Choose a reason for hiding this comment

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

the gen_admin_key will modify the conf/config.yaml, I think we should only update the conf/config.yaml when the user wants to update it.

When the service is started, APISIX should only be able to read the conf/config.yaml file by default.

@membphis
Copy link
Member Author

I have to close this PR, this PR contains other people's commit logs.

@membphis
Copy link
Member Author

@moonming I created a new PR #2230, with the right git commit log.

welcome to review the new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants