Обсуждение: Proposal for changes in official Docker image

Hi!

I accidentially sent this email to pgsql-hackers yesterday, sorry!

First of all, thanks for the great app :)

I started using PgAdmin with docker image (dpage/pgadmin4) a few weeks
ago, however I thought that it had some issues, so I decided to make
my own image. Some of the advantages:

- Use alpine linux instead of centos to greatly reduce image size
(170MB vs 560MB)
- Use lightweight pure-python HTTP server waitress instead of heavy
apache/mod_wsgi
- Use python 3.6

You can test the image at https://hub.docker.com/r/maksbotan/pgadmin4/
Readme contains more detailed explanation and usage instructions.

The Dockerfile is hosted at github: https://github.com/maksbotan/pgadmin4_docker

If you find my work useful, I'd love to make a contribution with these
scripts, after some discussion with pgadmin developers and further
improvements.

Looking forward for an answer,
Maxim.

Hi

2018-02-19 12:13 GMT+03:00 Dave Page <dpage@pgadmin.org>:
> Hi
>
> On Sun, Feb 18, 2018 at 5:41 PM, Максим Кольцов <kolmax94@gmail.com> wrote:
>>
>> Hi!
>>
>> I accidentially sent this email to pgsql-hackers yesterday, sorry!
>>
>> First of all, thanks for the great app :)
>>
>> I started using PgAdmin with docker image (dpage/pgadmin4) a few weeks
>> ago, however I thought that it had some issues, so I decided to make
>> my own image. Some of the advantages:
>>
>> - Use alpine linux instead of centos to greatly reduce image size
>> (170MB vs 560MB)
>> - Use lightweight pure-python HTTP server waitress instead of heavy
>> apache/mod_wsgi
>> - Use python 3.6
>>
>> You can test the image at https://hub.docker.com/r/maksbotan/pgadmin4/
>> Readme contains more detailed explanation and usage instructions.
>>
>> The Dockerfile is hosted at github:
>> https://github.com/maksbotan/pgadmin4_docker
>>
>> If you find my work useful, I'd love to make a contribution with these
>> scripts, after some discussion with pgadmin developers and further
>> improvements.
>
>
> Please feel free to submit patches to the existing code. I have no objection
> to the any of the alternate design decisions you've made (in principal),
> except for the intentional lack of SSL support.
>
> Thanks, Dave.

I updated my image to simplify installing of Python packages. I
decided I do not need a separate build step after all.
Can you point me at documentation on submitting patches to pgadmin?

What are your points in including SSL support into container? This can
be done by using, for example, gunicorn instead of waitress,
but I believe that this should be handled by reverse-proxy, like
nginx, in production environment. In non-production environment, i.e.
on developer's localhost, you do not need SSL at all.

By the way, in my opinion, on production there is one more task to be
handled by reverse-proxy - static files. By that I mean that all
static, not-changing files accessible at '/static/' URL should be
extracted from the container and served by nginx from a local folder.
This does not mean we shouldn't keep them in the image -- it's very
convenient for localhost usage. I haven't found a way to extract
all Flask's static files yet.

> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

2018-02-25 20:59 GMT+03:00 Dave Page <dpage@pgadmin.org>:
> Hi
>
> On Sat, Feb 24, 2018 at 9:04 PM, Максим Кольцов <kolmax94@gmail.com> wrote:
>>
>> Hi
>>
>> 2018-02-19 12:13 GMT+03:00 Dave Page <dpage@pgadmin.org>:
>> > Hi
>> >
>> > On Sun, Feb 18, 2018 at 5:41 PM, Максим Кольцов <kolmax94@gmail.com>
>> > wrote:
>> >>
>> >> Hi!
>> >>
>> >> I accidentially sent this email to pgsql-hackers yesterday, sorry!
>> >>
>> >> First of all, thanks for the great app :)
>> >>
>> >> I started using PgAdmin with docker image (dpage/pgadmin4) a few weeks
>> >> ago, however I thought that it had some issues, so I decided to make
>> >> my own image. Some of the advantages:
>> >>
>> >> - Use alpine linux instead of centos to greatly reduce image size
>> >> (170MB vs 560MB)
>> >> - Use lightweight pure-python HTTP server waitress instead of heavy
>> >> apache/mod_wsgi
>> >> - Use python 3.6
>> >>
>> >> You can test the image at https://hub.docker.com/r/maksbotan/pgadmin4/
>> >> Readme contains more detailed explanation and usage instructions.
>> >>
>> >> The Dockerfile is hosted at github:
>> >> https://github.com/maksbotan/pgadmin4_docker
>> >>
>> >> If you find my work useful, I'd love to make a contribution with these
>> >> scripts, after some discussion with pgadmin developers and further
>> >> improvements.
>> >
>> >
>> > Please feel free to submit patches to the existing code. I have no
>> > objection
>> > to the any of the alternate design decisions you've made (in principal),
>> > except for the intentional lack of SSL support.
>> >
>> > Thanks, Dave.
>>
>> I updated my image to simplify installing of Python packages. I
>> decided I do not need a separate build step after all.
>> Can you point me at documentation on submitting patches to pgadmin?
>
>
> There are some docs on the git repo and mailing list at
> https://www.pgadmin.org/development/resources/. To submit a patch, send an
> email to the hackers list describing the patch and attaching the "git diff"
> formatted patch file.
>
>>
>>
>> What are your points in including SSL support into container? This can
>> be done by using, for example, gunicorn instead of waitress,
>> but I believe that this should be handled by reverse-proxy, like
>> nginx, in production environment. In non-production environment, i.e.
>> on developer's localhost, you do not need SSL at all.
>>
>> By the way, in my opinion, on production there is one more task to be
>> handled by reverse-proxy - static files. By that I mean that all
>> static, not-changing files accessible at '/static/' URL should be
>> extracted from the container and served by nginx from a local folder.
>> This does not mean we shouldn't keep them in the image -- it's very
>> convenient for localhost usage. I haven't found a way to extract
>> all Flask's static files yet.
>
>
> Well that additional complexity is a very good reason why using two
> containers for this is overkill. Having two containers to run pgAdmin makes
> things unnecessarily complex in my opinion, especially given that it can
> (and is in the current container) achieved with the simple addition of a
> config snippet for Apache and mod_ssl. The current trend for micro services
> can easily be taken too far - we should keep the KISS principle in mind.

I did not mean to run two containers. I mean that pgadmin image, as I
picture it, may serve two purposes:

- localhost deployment on developer's machine to ease interaction with
postgres DB, local or remote.
  In this mode container serves it's own static files and is
accessible via plain HTTP
- Deployment in enterprise production environment, for many users,
possibly accessible from the Internet.
  In this mode container should only serve the API, possibly running
in several replicas. static files and SSL
  termination should be done by _existing_ nginx or something else
present in that organisation. For that I'd wish
  to have a way to extract static files from the container for
deployment, but not changing anything in the image.

> Another reason for including SSL support, is that users have asked for it.

In my humble opinion, if users want SSL support in application
container, they are doing something wrong and are
asking for troubles. But I respect this choice and I'm ready to allow
for it. I'll integrate gunicorn server in the image, which
supports SSL.

> Regards, Dave.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

2018-02-26 13:46 GMT+03:00 Dave Page <dpage@pgadmin.org>:
> Hi
>
> On Mon, Feb 26, 2018 at 10:09 AM, Максим Кольцов <kolmax94@gmail.com> wrote:
>>
>> 2018-02-25 20:59 GMT+03:00 Dave Page <dpage@pgadmin.org>:
>> > Hi
>> >
>> > On Sat, Feb 24, 2018 at 9:04 PM, Максим Кольцов <kolmax94@gmail.com>
>> > wrote:
>> >>
>> >> Hi
>> >>
>> >> 2018-02-19 12:13 GMT+03:00 Dave Page <dpage@pgadmin.org>:
>> >> > Hi
>> >> >
>> >> > On Sun, Feb 18, 2018 at 5:41 PM, Максим Кольцов <kolmax94@gmail.com>
>> >> > wrote:
>> >> >>
>> >> >> Hi!
>> >> >>
>> >> >> I accidentially sent this email to pgsql-hackers yesterday, sorry!
>> >> >>
>> >> >> First of all, thanks for the great app :)
>> >> >>
>> >> >> I started using PgAdmin with docker image (dpage/pgadmin4) a few
>> >> >> weeks
>> >> >> ago, however I thought that it had some issues, so I decided to make
>> >> >> my own image. Some of the advantages:
>> >> >>
>> >> >> - Use alpine linux instead of centos to greatly reduce image size
>> >> >> (170MB vs 560MB)
>> >> >> - Use lightweight pure-python HTTP server waitress instead of heavy
>> >> >> apache/mod_wsgi
>> >> >> - Use python 3.6
>> >> >>
>> >> >> You can test the image at
>> >> >> https://hub.docker.com/r/maksbotan/pgadmin4/
>> >> >> Readme contains more detailed explanation and usage instructions.
>> >> >>
>> >> >> The Dockerfile is hosted at github:
>> >> >> https://github.com/maksbotan/pgadmin4_docker
>> >> >>
>> >> >> If you find my work useful, I'd love to make a contribution with
>> >> >> these
>> >> >> scripts, after some discussion with pgadmin developers and further
>> >> >> improvements.
>> >> >
>> >> >
>> >> > Please feel free to submit patches to the existing code. I have no
>> >> > objection
>> >> > to the any of the alternate design decisions you've made (in
>> >> > principal),
>> >> > except for the intentional lack of SSL support.
>> >> >
>> >> > Thanks, Dave.
>> >>
>> >> I updated my image to simplify installing of Python packages. I
>> >> decided I do not need a separate build step after all.
>> >> Can you point me at documentation on submitting patches to pgadmin?
>> >
>> >
>> > There are some docs on the git repo and mailing list at
>> > https://www.pgadmin.org/development/resources/. To submit a patch, send
>> > an
>> > email to the hackers list describing the patch and attaching the "git
>> > diff"
>> > formatted patch file.
>> >
>> >>
>> >>
>> >> What are your points in including SSL support into container? This can
>> >> be done by using, for example, gunicorn instead of waitress,
>> >> but I believe that this should be handled by reverse-proxy, like
>> >> nginx, in production environment. In non-production environment, i.e.
>> >> on developer's localhost, you do not need SSL at all.
>> >>
>> >> By the way, in my opinion, on production there is one more task to be
>> >> handled by reverse-proxy - static files. By that I mean that all
>> >> static, not-changing files accessible at '/static/' URL should be
>> >> extracted from the container and served by nginx from a local folder.
>> >> This does not mean we shouldn't keep them in the image -- it's very
>> >> convenient for localhost usage. I haven't found a way to extract
>> >> all Flask's static files yet.
>> >
>> >
>> > Well that additional complexity is a very good reason why using two
>> > containers for this is overkill. Having two containers to run pgAdmin
>> > makes
>> > things unnecessarily complex in my opinion, especially given that it can
>> > (and is in the current container) achieved with the simple addition of a
>> > config snippet for Apache and mod_ssl. The current trend for micro
>> > services
>> > can easily be taken too far - we should keep the KISS principle in mind.
>>
>> I did not mean to run two containers. I mean that pgadmin image, as I
>> picture it, may serve two purposes:
>>
>> - localhost deployment on developer's machine to ease interaction with
>> postgres DB, local or remote.
>>   In this mode container serves it's own static files and is
>> accessible via plain HTTP
>> - Deployment in enterprise production environment, for many users,
>> possibly accessible from the Internet.
>>   In this mode container should only serve the API, possibly running
>> in several replicas. static files and SSL
>>   termination should be done by _existing_ nginx or something else
>> present in that organisation. For that I'd wish
>>   to have a way to extract static files from the container for
>> deployment, but not changing anything in the image.
>
>
> As I see it, that does essentially mean two containers (or 1 container and a
> VM or whatever). Either way, it adds a lot of complexity for the user.
>
>>
>>
>> > Another reason for including SSL support, is that users have asked for
>> > it.
>>
>> In my humble opinion, if users want SSL support in application
>> container, they are doing something wrong and are
>> asking for troubles. But I respect this choice and I'm ready to allow
>> for it. I'll integrate gunicorn server in the image, which
>> supports SSL.
>
>
> Doing it that way gives them both options (well, we'd still need to figure
> out the static file extraction). Those that want a quick and easy SSL
> solution can do it with one container, those running on localhost can use
> plain HTTP, and those who want an external reverse proxy to add SSL would
> also have that option. I think this would be the most flexible and
> convenient for users.

I've switched to Gunicorn, adding SSL support. It has the same
interface as the original container: PGADMIN_ENABLE_TLS,
/certs/server.key and /certs/server.cert.
I also incorporated building of sphinx manual in Dockerfile, so now
the image should be complete.

I noticed that I can't use gunicorn forking worker with pgadmin4, this
is probably caused by session implementation, but I'm not sure. You
can investigate this by using e.g. `-w 4` in entrypoint.sh, otherwise
it's working fine with single-process threaded worker.

I will make my work into a patch and send it to the mail list soon.
Meanwhile, it'd be great if you tested the updated image at
https://hub.docker.com/r/maksbotan/pgadmin4/

> Thanks, Dave.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

2018-03-19 17:55 GMT+03:00 Dave Page <dpage@pgadmin.org>:
> Hi
>
> On Mon, Mar 19, 2018 at 11:19 AM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Fri, Mar 16, 2018 at 8:52 PM, Максим Кольцов <kolmax94@gmail.com>
>> wrote:
>>>
>>>
>>> I've switched to Gunicorn, adding SSL support. It has the same
>>> interface as the original container: PGADMIN_ENABLE_TLS,
>>> /certs/server.key and /certs/server.cert.
>>> I also incorporated building of sphinx manual in Dockerfile, so now
>>> the image should be complete.
>>
>>
>> Nice!
>>
>>>
>>>
>>> I noticed that I can't use gunicorn forking worker with pgadmin4, this
>>> is probably caused by session implementation, but I'm not sure. You
>>> can investigate this by using e.g. `-w 4` in entrypoint.sh, otherwise
>>> it's working fine with single-process threaded worker.
>>
>>
>> This is expected, and the reason why is explained here:
>> https://www.pgadmin.org/docs/pgadmin4/2.x/server_deployment.html#requirements
>>
>>>
>>>
>>> I will make my work into a patch and send it to the mail list soon.
>>> Meanwhile, it'd be great if you tested the updated image at
>>> https://hub.docker.com/r/maksbotan/pgadmin4/
>>
>>
>> I'll try to take a look later in the week (busy prepping for a release at
>> the moment).
>
>
> I actually found some time for this today. It's looking pretty good - TLS
> vs. non-TLS seems to work well. The couple of things I did notice were:
>
> - PGADMIN_SERVER_NAME doesn't appear to be supported. This was added at user
> request, for security reasons (to help ensure the connection is going where
> expected). I'm not entirely convinced of the value of that, but if it's
> fairly painless to add, it may well be worth it.

Can you explain the meaning of this option, or maybe give me a link to
original feature request?
As far as I understand, Apache uses this to identify virtual hosts
based on HTTP Host header,
but there are no virtual hosts in Gunicorn, so no need for this.

> - If you don't map /var/lib/pgadmin (in my case, to a directory already
> containing a config), then I was seeing the following error. I would expect
> it to init within the container if the config directory isn't mapped:
>
> piranha:web dpage$ docker logs affectionate_spence
> sh: -z: unknown operand
> NOTE: Configuring authentication for SERVER mode.
>
> Enter the email address and password to use for the initial pgAdmin user
> account:
>
> Traceback (most recent call last):
>   File "run_pgadmin.py", line 4, in <module>
>     from pgAdmin4 import app
>   File "/pgadmin4/pgAdmin4.py", line 67, in <module>
>     app = create_app()
>   File "/pgadmin4/pgadmin/__init__.py", line 306, in create_app
>     db_upgrade(app)
>   File "/pgadmin4/pgadmin/setup/db_upgrade.py", line 25, in db_upgrade
>     flask_migrate.upgrade(migration_folder)
>   File "/usr/local/lib/python3.6/site-packages/flask_migrate/__init__.py",
> line 244, in upgrade
>     command.upgrade(config, revision, sql=sql, tag=tag)
>   File "/usr/local/lib/python3.6/site-packages/alembic/command.py", line
> 254, in upgrade
>     script.run_env()
>   File "/usr/local/lib/python3.6/site-packages/alembic/script/base.py", line
> 427, in run_env
>     util.load_python_file(self.dir, 'env.py')
>   File "/usr/local/lib/python3.6/site-packages/alembic/util/pyfiles.py",
> line 81, in load_python_file
>     module = load_module_py(module_id, path)
>   File "/usr/local/lib/python3.6/site-packages/alembic/util/compat.py", line
> 83, in load_module_py
>     spec.loader.exec_module(module)
>   File "/pgadmin4/pgadmin/setup/../../migrations/env.py", line 94, in
> <module>
>     run_migrations_online()
>   File "/pgadmin4/pgadmin/setup/../../migrations/env.py", line 87, in
> run_migrations_online
>     context.run_migrations()
>   File "<string>", line 8, in run_migrations
>   File
> "/usr/local/lib/python3.6/site-packages/alembic/runtime/environment.py",
> line 836, in run_migrations
>     self.get_context().run_migrations(**kw)
>   File
> "/usr/local/lib/python3.6/site-packages/alembic/runtime/migration.py", line
> 330, in run_migrations
>     step.migration_fn(**kw)
>   File "/pgadmin4/migrations/versions/fdc58d9bd449_.py", line 112, in
> upgrade
>     email, password = user_info()
>   File "/pgadmin4/pgadmin/setup/user_info.py", line 55, in user_info
>     email = input("Email address: ")
> EOFError: EOF when reading a line
> [2018-03-19 14:50:59 +0000] [1] [INFO] Starting gunicorn 19.7.1
> [2018-03-19 14:50:59 +0000] [1] [INFO] Listening at: http://0.0.0.0:8080 (1)
> [2018-03-19 14:50:59 +0000] [1] [INFO] Using worker: threads
> [2018-03-19 14:50:59 +0000] [14] [INFO] Booting worker with pid: 14
> [2018-03-19 14:50:59 +0000] [14] [ERROR] Exception in worker process
> Traceback (most recent call last):
>   File "/usr/local/lib/python3.6/site-packages/gunicorn/arbiter.py", line
> 578, in spawn_worker
>     worker.init_process()
>   File "/usr/local/lib/python3.6/site-packages/gunicorn/workers/gthread.py",
> line 109, in init_process
>     super(ThreadWorker, self).init_process()
>   File "/usr/local/lib/python3.6/site-packages/gunicorn/workers/base.py",
> line 126, in init_process
>     self.load_wsgi()
>   File "/usr/local/lib/python3.6/site-packages/gunicorn/workers/base.py",
> line 135, in load_wsgi
>     self.wsgi = self.app.wsgi()
>   File "/usr/local/lib/python3.6/site-packages/gunicorn/app/base.py", line
> 67, in wsgi
>     self.callable = self.load()
>   File "/usr/local/lib/python3.6/site-packages/gunicorn/app/wsgiapp.py",
> line 65, in load
>     return self.load_wsgiapp()
>   File "/usr/local/lib/python3.6/site-packages/gunicorn/app/wsgiapp.py",
> line 52, in load_wsgiapp
>     return util.import_app(self.app_uri)
>   File "/usr/local/lib/python3.6/site-packages/gunicorn/util.py", line 352,
> in import_app
>     __import__(module)
>   File "/pgadmin4/run_pgadmin.py", line 4, in <module>
>     from pgAdmin4 import app
>   File "/pgadmin4/pgAdmin4.py", line 67, in <module>
>     app = create_app()
>   File "/pgadmin4/pgadmin/__init__.py", line 309, in create_app
>     schema_version = version.value
> AttributeError: 'NoneType' object has no attribute 'value'
> [2018-03-19 14:50:59 +0000] [14] [INFO] Worker exiting (pid: 14)
> [2018-03-19 14:50:59 +0000] [1] [INFO] Shutting down: Master
> [2018-03-19 14:50:59 +0000] [1] [INFO] Reason: Worker failed to boot.

Yeah, that was shell quoting issue, should be fixed.

I will send a patch for review later this evening.

>
> Thanks!
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company