-
Notifications
You must be signed in to change notification settings - Fork 685
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
Creates a symlink to the .so file in postint for mod_wsgi #5443
Conversation
Failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The proposed approach for resolving the symlink path conflicts with the related postinst changes introduced in #4622. The wsgi module path is read from /etc/apache2/mods-available/wsgi.load
, which we write to in postinst on L162: https://github.com/freedomofpress/securedrop/pull/4622/files#diff-4dd8e904f4b653b022dd495265bd22bdR156
Rather than add a symlink as a second indication of where the module should be imported from, can we instead revise the existing postinst logic to address the problem under Focal?
wsgi_so_link="/opt/venvs/securedrop-app-code/lib/python3.8/site-packages/mod_wsgi/server/mod_wsgi-py38.so" | ||
wsgi_so="/opt/venvs/securedrop-app-code/lib/python3.8/site-packages/mod_wsgi/server/mod_wsgi-py38.cpython-38-x86_64-linux-gnu.so" | ||
if test -f "/usr/bin/python3.8"; then | ||
if test ! -f $wsgi_so_link; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test -f
conditional on L100 will always fail, since the target is a symlink, not a file. Rather than -f
, -L
would be a better test.
On Focal, mod_wsgi expects a shared library for the python module at: $VENV/lib/python3.8/site-packages/mod_wsgi/server/mod_wsgi-py38.so But, the real shared library path is $VENV/lib/python3.8/site-packages/mod_wsgi/server/mod_wsgi-py38.cpython-38-x86_64-linux-gnu.so" So we are creating a symlink with the proper path if we are using Python 3.8
cd17797
to
b9e0d5b
Compare
@conorsch updated the PR based on feedback. |
@kushaldas independently of the way this symlink is applied, would it be appropriate here to add a testinfra test to ensure the symlink is properly applied (or not) depending on the targeted platform? |
I tried the
The site's working. 🤔 I'll go see if the way I installed dh-virtualenv in the builder image (building its .deb in a clone of its repo, because apt complained about the signature on the |
Investigation complete, and the mod_wsgi config is still getting generated correctly, so I think the symlink is unnecessary. I'm closing this; we can of course reopen if someone else sees the problem. |
It seems this path modification is happening only in the virtual environment of the
|
Reopening based on comments by @kushaldas & @rmol in standup today. Once the dust settles a bit on the build system choice for dh-virtualenv on Focal (#5484), looks like we will indeed need to munge the so path. |
GitHub wouldn't let us reopen the PR because the underlying branch had been modified, so this same PR was resubmitted as #5489. |
Status
Ready for review
Description of Changes
Fixes in part of #4768
On Focal, mod_wsgi expects a shared library for the python module at:
But, the real shared library path is
So we are creating a symlink with the proper path if we are using Python 3.8 (that is on Focal)
Testing
make build-deps
on this branchmake staging
to create a fresh staging environment from the debian packages. There should not be any error in the postinstall sectionmolecule -s libvirt-staging-xenial -h app-staging
(on mac point to virtualbox) and see that the following file does not exist.sudo touch /usr/bin/python3.8
to emulate a python3.8 env in focalsecuredrop-app-code
package or reconfigure it, it should create a symlink as mentioned above.Deployment
Any special considerations for deployment? Consider both:
Checklist
If you made changes to the server application code:
make lint
) and tests (make test
) pass in the development containerIf you made changes to
securedrop-admin
:make -C admin test
) pass in the admin development containerIf you made changes to the system configuration:
If you made non-trivial code changes:
If you made changes to documentation:
make docs-lint
) passed locallyIf you added or updated a code dependency:
Choose one of the following: