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: unable to setup ssls with env reference #11141

Open
Sebastian-Pietrzak opened this issue Apr 10, 2024 · 6 comments · May be fixed by #11172
Open

bug: unable to setup ssls with env reference #11141

Sebastian-Pietrzak opened this issue Apr 10, 2024 · 6 comments · May be fixed by #11172
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@Sebastian-Pietrzak
Copy link

Current Behavior

I'm struggling with adding certificate with /ssls admin endpoint using env reference.

curl --location --request PUT 'http://127.0.0.1:9180/apisix/admin/ssls' \
--header 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' \
--header 'Content-Type: application/json' \
--data '{
    "id":  "some_id",
    "type": "server",
    "snis": ["localhost"],
    "cert": "$ENV://APISIX_ENV_CERT",
    "key": "$ENV://APISIX_ENV_KEY",
    "ssl_protocols": ["TLSv1.2", "TLSv1.3"]
  }'

I'm always getting this, no matter what I put inside cert/key fields unless they are proper certs.

{
    "error_msg": "invalid configuration: property \"key\" validation failed: value should match only one schema, but matches none"
}

Accordingly to admin api documentation those fields support those env references, but it seems it's not the case. Note: I'm using such env reference in key-auth plugin and it works just fine, but here it seems like it doesn't expect any other pattern than cert, and message/logs are not helpful.

Expected Behavior

It's possible to setup ssls and provide cert location as env reference.

Error Logs

No response

Steps to Reproduce

Send following request to admin-api:

curl --location --request PUT 'http://127.0.0.1:9180/apisix/admin/ssls' \
--header 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' \
--header 'Content-Type: application/json' \
--data '{
    "id":  "some_id",
    "type": "server",
    "snis": ["localhost"],
    "cert": "$ENV://APISIX_ENV_CERT",
    "key": "$ENV://APISIX_ENV_KEY",
    "ssl_protocols": ["TLSv1.2", "TLSv1.3"]
  }'

Environment

  • APISIX version (run apisix version): 3.7.0
  • Operating system (run uname -a): Linux 4091912aa453 6.6.16-linuxkit change: added doc of how to load plugin. #1 SMP Fri Feb 16 11:54:02 UTC 2024 x86_64 GNU/Linux
  • OpenResty / Nginx version (run openresty -V or nginx -V): n/a
  • etcd version, if relevant (run curl http://127.0.0.1:9090/v1/server_info): 3.5.9
  • APISIX Dashboard version, if relevant: n/a
  • Plugin runner version, for issues related to plugin runners: n/a
  • LuaRocks version, for installation issues (run luarocks --version): n/a
@Sebastian-Pietrzak Sebastian-Pietrzak changed the title bug: unable to setup ssls bug: unable to setup ssls with env reference Apr 10, 2024
@kayx23
Copy link
Member

kayx23 commented Apr 12, 2024

From TEST18 in https://github.com/apache/apisix/blob/master/t/router/radixtree-sni2.t, it looks like it should be supported.

@shreemaan-abhishek
Copy link
Contributor

@Sebastian-Pietrzak this is a limitation (or a bug) with APISIX, using env reference will work as expected if you use the declaration in lower case. i.e $env:// instead of $ENV:// 😅

Would you like to fix this?

@shreemaan-abhishek shreemaan-abhishek added good first issue Good for newcomers bug Something isn't working labels Apr 12, 2024
@Sebastian-Pietrzak
Copy link
Author

Thanks for checking! Knowing there's workaround is helpful. I think it would make sense to fix it at some point for consistency with other places, so that nobody will have issues with this anymore.

@shreemaan-abhishek
Copy link
Contributor

anyone interested in taking this up can refer this piece of code:

https://github.com/shreemaan-abhishek/apisix/blob/1f775c8ace851a8b8862801ad35e7cf4cd00851f/apisix/schema_def.lua#L739

This is exactly where the bug is.

@LinkinStars
Copy link
Member

anyone interested in taking this up can refer this piece of code:

https://github.com/shreemaan-abhishek/apisix/blob/1f775c8ace851a8b8862801ad35e7cf4cd00851f/apisix/schema_def.lua#L739

This is exactly where the bug is.

@shreemaan-abhishek This looks easier to fix. I want to try it out. Do you think we need to support matching both uppercase and lowercase at the same time? I would think it's necessary to consider the users who are already using it. So, the ^\\$(secret|env|SECRET|ENV):// would be better?

@shreemaan-abhishek
Copy link
Contributor

yep, LGTM

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
Status: 📋 Backlog
Development

Successfully merging a pull request may close this issue.

4 participants