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

asyncpg does not work with "sslmode" query param when called from SQLAlchemy #737

Open
igortg opened this issue Apr 8, 2021 · 16 comments
Open

Comments

@igortg
Copy link

igortg commented Apr 8, 2021

  • asyncpg version: 0.22
  • PostgreSQL version: 12.3
  • Do you use a PostgreSQL SaaS? If so, which? Can you reproduce
    the issue with a local PostgreSQL install?
    : I use DigitalOcean and was able to also reproduce it locally
  • Python version: 3.8.9
  • Platform: Linux
  • Do you use pgbouncer?: No
  • Did you install asyncpg with pip?: Yes
  • If you built asyncpg locally, which version of Cython did you use?: -
  • Can the issue be reproduced under both asyncio and
    uvloop?
    : N/D

First, congratulation to the people of MagicStack for this great library.

When using DigitalOcean default environment DATABASE URL to set the database string on my app I got the following error:

  File "/usr/local/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 599, in __connect
    connection = pool._invoke_creator(self)
  File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/create.py", line 578, in connect
    return dialect.connect(*cargs, **cparams)
  File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/default.py", line 558, in connect
    return self.dbapi.connect(*cargs, **cparams)
  File "/usr/local/lib/python3.8/site-packages/sqlalchemy/dialects/postgresql/asyncpg.py", line 747, in connect
    await_only(self.asyncpg.connect(*arg, **kw)),
TypeError: connect() got an unexpected keyword argument 'sslmode'

In my app, since I use psycopg2 to do some "management tasks", I also need to do something like that:

if ssl_enabled and driver == "asyncpg":
    base_url += "?ssl=require"
elif ssl_enabled and driver == "psycopg2":
    base_url += "?sslmode=require"

Shouldn't asyncpg use the more standard sslmode= query param instead of the ssl=?

@fantix
Copy link
Member

fantix commented Apr 8, 2021

I think we do use sslmode in the DSN URL?

if 'sslmode' in query:
val = query.pop('sslmode')
if ssl is None:
ssl = val

Is it the ssl parameter in the Python method signature of asyncpg.connect() (not the DSN URL string) you were running into?

@elprans
Copy link
Member

elprans commented Apr 8, 2021

This is a bug in SQLAlchemy's create_connect_args implementation for asyncpg.

@igortg
Copy link
Author

igortg commented Apr 14, 2021

I wouldn't say it's a bug. Seems more an oversight from SQLAlchemy that this param had to be translated for asyncpg since SQLAlchemy parses the URL by itself and calls connect with query params as kwargs. Since asyncpg uses ssl as the param name on connect method, the sslmode query param would not work.

async def connect(dsn=None, *,

I'll open an issue on SQLAlchemy to see if they have something to say about that.

@igortg
Copy link
Author

igortg commented Apr 14, 2021

Just find out that there was a similar issue with pg8000 DBAPI: sqlalchemy/sqlalchemy#4146 (comment)
Seems that SQLAlchemy folks expect DBAPIs to have the standard param names for connect.

Is there any chance asyncpg.connect would change the ssl param to sslmode (or add an extra alias param)?

@igortg igortg changed the title asyncpg does not work with "sslmode" query param asyncpg does not work with "sslmode" query param when called from SQLAlchemy Apr 14, 2021
@elprans
Copy link
Member

elprans commented Apr 14, 2021

Seems that SQLAlchemy folks expect DBAPIs to have the standard param names for connect.

It is weird for SQLAlchemy to require that, given that it's supposed to be an abstraction over varying driver implementations. The create_connect_args hook I linked above seems to be designed exactly for the purpose of massaging SQLAlchemy's connection arguments to the format accepted by the driver. It simply doesn't have code to remap sslmode to ssl.

@igortg
Copy link
Author

igortg commented Apr 15, 2021

Oh... Now I got it. Seems a simple fix on the SQLAlchemy side.

I opened an issue there: sqlalchemy/sqlalchemy#6275

@vashchukmaksim
Copy link

@igortg have you been able to make it work? I read linked issue but I can't make engine creator working with async connect function and also an option with make_url still throws the same error about sslmode parameter.

url = make_url(...)
url = url.update_query_dict({'sslmode': 'require'})

I can't get it

@igortg
Copy link
Author

igortg commented Jun 14, 2021

On asyncpg the query string is ssl instead of sslmode

@vashchukmaksim
Copy link

Hm, with this setup I receive the following error:

url = make_url(...)
url = url.update_query_dict({'sslmode': 'require'})

connect() got an unexpected keyword argument 'sslmode'

If I change sslmode to ssl:

url = make_url(...)
url = url.update_query_dict({'ssl': 'require'})

I receive another one:

Connection reset by peer

So I'm not sure that ssl parameter was passed ok to DigitalOcean eventually (but maybe it is). Also I have pool_pre_ping set to True that helped me in the similar scenario with DO and psycopg2. Probably ssl parameter is passing correctly in the last case and the reason is different. In case you have anything in mind to make it work, since as I understood you also set up asyncpg with DO as well, it will be really appreciated 🙏

@svajipay
Copy link

I am not sure why it still does not work after this #768 ?

@mmarti8895
Copy link

Hello, after running into this problem as well, I think it is a conflict of documentation versus how Postgresql annotates SSL Support (https://www.postgresql.org/docs/9.1/libpq-ssl.html)

The code accepts a ssl parameter:
https://magicstack.github.io/asyncpg/current/api/index.html?highlight=ssl

But the problem I think is the assumption that the keyword sslmode will work from the dsn and that the ssl attribute will be optional or one will overwrite the other. This was not the case for me.

It is still not clear to me when to choose between sslmode in the dsn versus the ssl. But, something that explains these different scenarios might help mitigate people even using sslmode in the query params, since it is obviously not working for other libraries. Maybe even taking reference to sslmode in the dsn out of the documentation or just not supporting it at all.

@karolzlot
Copy link

After using connection_string.replace('sslmode','ssl') I am getting another error:

connect() got an unexpected keyword argument 'sslrootcert'

Do you know any solution so I can still use sslrootcert?

@evindunn
Copy link

evindunn commented Feb 26, 2022

Here's how I finally got it to work (sqlalchemy/sqlalchemy#5975):

from ssl import create_default_context, Purpose as SSLPurpose

ssl_context = create_default_context(
    SSLPurpose.CLIENT_AUTH,
    cafile="docker-compose.d/cockroach-certs/ca.crt"
)
ssl_context.load_cert_chain(
    "docker-compose.d/cockroach-certs/client.root.crt",
    keyfile="docker-compose.d/cockroach-certs/client.root.key"
)
ssl_context.check_hostname = True

_engine = create_async_engine(
    "cockroachdb+asyncpg://root@localhost:26257/mydb",
    echo=True,
    connect_args={"ssl": ssl_context}
)

Equivalent to sslmode=verify-full&sslcert=..&sslkey=..&sslrootcert=.. per https://magicstack.github.io/asyncpg/current/api/index.html

@karolzlot
Copy link

@Abhi011999
Copy link

On asyncpg the query string is ssl instead of sslmode

this worked for me, thanks!

@SnoozeFreddo
Copy link

@evindunn What if my sslrootcert is a string and not a file?

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

No branches or pull requests

10 participants