OpusVL / docker-odoo-custom

An Odoo docker image with extended capabilities and fixes over the official one
Other
1 stars 1 forks source link

Entrypoint assumes user is odoo #5

Closed PeterAlabaster closed 5 years ago

PeterAlabaster commented 6 years ago

https://github.com/OpusVL/docker-odoo-custom/blob/562b5b0281deb474f44b0f9cf55a11b9f40dac86/opusvl-entrypoint.py#L52

A few lines should be at the start (to be used to chown stuff) such as

odoo_uid = pwd.getpwnam('odoo').pw_uid
odoo_gid = pwd.getpwnam('odoo').pw_gid

A line should precede the os.execl with os.setuid(odoo_uid)

Note that this would require the Dockerfile for any dependant images to be changed on the last line, removing USER odoo

@Nick-OpusVL - I want to run this past you before making the change, as you may have an opinion as to why we should not do this.

Altreus commented 6 years ago

Per that email what I sent, it seems fair for the entrypoint to assume the user is odoo when the Dockerfile says the user is odoo.

If you're execing in, then you should use -u odoo to mimic what the Dockerfile does and what the entrypoint expects

IMO

PeterAlabaster commented 6 years ago

In conjunction with this change, the last line of the project specific Dockerfile which says USER odoo would be removed.

There is also some stuff which Craig said he'd like, such as having a line:

    os.system("chown -R odoo:odoo /var/lib/odoo/")

in the entrypoint

Nick-OpusVL commented 5 years ago

@PeterAlabaster it looks like this is already done (though there is at least one project not currently taking advantage of it, which I won't name because this is a public forum)

Am I correct in this?

PeterAlabaster commented 5 years ago

It most likely already is done, i think this issue just forgot to get closed. My last comment was almost a year ago

PeterAlabaster commented 5 years ago

Yes, - https://github.com/OpusVL/docker-odoo-custom/commit/a4f8717b52cb760bf73459dc03df16e288a8bd84 fixed this issue, but there was no message saying Closes #5. So this can be closed.

Nick-OpusVL commented 5 years ago

Yes, - a4f8717 fixed this issue, but there was no message saying Closes #5. So this can be closed.