Snowflake-Labs / django-snowflake

MIT License
59 stars 15 forks source link

Option private_key does not work when launching SnowSQL #67

Closed sfc-gh-hachouraria closed 6 months ago

sfc-gh-hachouraria commented 1 year ago

The Python Connector accepts an option private_key to perform auth. For a server backend such as a Django app this is often the most preferable option.

The value for private_key is accepted only as a bytes object that represents a decrypted key's bytes.

The OPTIONS clause accepts passing a value for private_key, but it attempts to also reuse the same value as a file path when launching snowsql. Since the valid value can only be a raw bytes object, launching SnowSQL with its value for --private-key-path will fail (or conversely the regular connections in the server will fail if its a path):

Traceback (most recent call last):
  File "/django-tests/mysite/manage.py", line 22, in <module>
    main()
  File "/django-tests/mysite/manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "/django-tests/denv/lib/python3.10/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/django-tests/denv/lib/python3.10/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/django-tests/denv/lib/python3.10/site-packages/django/core/management/base.py", line 412, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/django-tests/denv/lib/python3.10/site-packages/django/core/management/base.py", line 458, in execute
    output = self.handle(*args, **options)
  File "/django-tests/denv/lib/python3.10/site-packages/django/core/management/commands/dbshell.py", line 30, in handle
    connection.client.runshell(options["parameters"])
  File "/django-tests/denv/lib/python3.10/site-packages/django_snowflake/client.py", line 68, in runshell
    super().runshell(parameters)
  File "/django-tests/denv/lib/python3.10/site-packages/django/db/backends/base/client.py", line 28, in runshell
    subprocess.run(args, env=env, check=True)
  File "/3.10/lib/python3.10/subprocess.py", line 503, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/3.10/lib/python3.10/subprocess.py", line 971, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/3.10/lib/python3.10/subprocess.py", line 1796, in _execute_child
    self.pid = _posixsubprocess.fork_exec(
ValueError: embedded null byte

I propose introducing special options private_key_file and private_key_passphrase that are specific to this project. Use of this internally auto-decrypts the file to the bytes format required by the connector (injecting private_key automatically into connector calls), and uses the same file path for its SnowSQL launch args.

Perhaps it makes sense to implement these directly into the connector itself (rather than in this plugin).

Happy to send a PR implementing this. Thoughts?

timgraham commented 1 year ago

I think I would first raise the issue with Snowflake Connector for Python and ask if they would be willing to add private_key_file and private_key_passphrase options. It's not clear to me whether such boilerplate is best placed in this project as seems likely useful for other connector users.

It's possible decrypting the key as documented may not be generalizable, e.g. could a key have a different format or encoding? But it seems some form of general decryption was implemented in snowsql since it only accepts a path?

sfc-gh-hachouraria commented 1 year ago

I've filed a feature request at https://github.com/snowflakedb/snowflake-connector-python/issues/1565 referencing this issue.

Should we formally document the current limitation around using the specific combo of private_key + manage.py dbshell (the Python interpreter via manage.py shell would work, and this isn't a blocker)?

If not, we can close this issue and revisit in the future depending on the feature being accepted.

timgraham commented 1 year ago

Thanks. I would keep this open because it's a valid bug. I hope we can solve it one way or another in the short-term (maybe in the next month-ish), so I don't think we need to document it in the README, which is more for longer term limitations.

One way to document it would be to change the code to raise NotSupportError when trying to run dbshell if private_key is specified. That's probably not worthwhile in the short term though.

Let's see what response https://github.com/snowflakedb/snowflake-connector-python/issues/1565 gets. If it's not accepted, we could consider adding private_key_file and private_key_passphrase options specific to this project.