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

bug: auto-generated apisix id is likely to be duplicated in containerized environments #5417

Closed
tokers opened this issue Nov 4, 2021 · 18 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@tokers
Copy link
Contributor

tokers commented Nov 4, 2021

Issue description

Currently, Apache APISIX will generate an id while initializing if the user doesn't specify one, and it relies on the lua-resty-jit-uuid library but without an explicit seed.

uuid.seed()
apisix_uid = uuid.generate_v4()
log.notice("not found apisix uid, generate a new one: ", apisix_uid)

While the jit-uuid library creates the seed by the process id and the time in ngx_lua context.

https://github.com/thibaultcha/lua-resty-jit-uuid/blob/82538049040ae85ff880b79886f21d8593140c7d/lib/resty/jit-uuid.lua#L53-L54

However, in a containerized environment, the process id (the master process) might be the same, i.e. the No. 1 process, also, if users try to deploy Apache APISIX clusters on Kubernetes through the Deployment resource, the time might be the same for several Pods, since ngx.time doesn't have enough precisions (only at milliseconds level). so the generated apisix id might be duplicated, and if the id is critical, this may cause some fatal problems in the business scenarios.

Environment

  • apisix version (cmd: apisix version):
  • OS (cmd: uname -a):
  • OpenResty / Nginx version (cmd: nginx -V or openresty -V):
  • etcd version, if have (cmd: run curl http://127.0.0.1:9090/v1/server_info to get the info from server-info API):
  • apisix-dashboard version, if have:
  • the plugin runner version, if the issue is about a plugin runner (cmd: depended on the kind of runner):
  • luarocks version, if the issue is about installation (cmd: luarocks --version):

Steps to reproduce

N/A

Actual result

N/A

Error log

N/A

Expected result

No response

@spacewander
Copy link
Member

Maybe we can suffix some rand bytes from thibaultcha/lua-resty-jit-uuid#19 (comment)

@tokers tokers added bug Something isn't working good first issue Good for newcomers labels Nov 4, 2021
@tokers
Copy link
Contributor Author

tokers commented Nov 4, 2021

Maybe we can suffix some rand bytes from thibaultcha/lua-resty-jit-uuid#19 (comment)

Good idea.

@spacewander
Copy link
Member

@leslie-tsang
Would you like to have a try? Thanks!

@leslie-tsang
Copy link
Member

@leslie-tsang Would you like to have a try? Thanks!

Sure.

@leslie-tsang
Copy link
Member

After reading the lua-resty-jit-uuid's issue, there are three solution for this issue:

  1. use resty.random to generate UUID, it depend on OpenSSL RAND_bytes
  2. use tostring(table or str) to get a random seed, Ref to lua-nginx-module issue #1457 and apisix PR #5414
  3. patch _G.math.randomseed like kong does

@spacewander @tokers What do you guys think ?

@spacewander
Copy link
Member

I vote for the kong way.

@tzssangglass
Copy link
Member

vote the kong way too

@tokers
Copy link
Contributor Author

tokers commented Nov 15, 2021

Vote for Kong, it's on a higher level.

@aseaday
Copy link

aseaday commented Nov 24, 2021

Considering a more stable production purpose use, how about we did our best in the k8s environment or something else?
Project Hipsor

@aseaday
Copy link

aseaday commented Nov 24, 2021

@membphis
Copy link
Member

After a careful look at the two implementations, the core is the same.

Let's do it.

@aseaday
Copy link

aseaday commented Nov 24, 2021

After a careful look at the two implementations, the core is the same.

Let's do it.

I want to fix this bug as an first step to contribute to apisix. As an newbie, I had one question:

  1. Need we to patch the math.randomseed or just fix the id problem?

@membphis
Copy link
Member

Which one is the best, then use which one.

@membphis
Copy link
Member

I prefer patch math.randomseed way

@aseaday
Copy link

aseaday commented Nov 25, 2021

I prefer patch math.randomseed way

If we patch the math.randomsee globally as the kong way, we need to add a module as the global patch. Is it a good choice to put the module in the core?

@aseaday
Copy link

aseaday commented Nov 25, 2021

Or add randomsed in apisix/patch.lua

@membphis
Copy link
Member

membphis commented Dec 2, 2021

Or add randomsed in apisix/patch.lua

I prefer this way. what is your opinion? @spacewander

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants