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

Feature/configurable port #1504

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

saretter
Copy link

This change allows to set environment variable APACHE_PORT in order to change the Apache port. My motivation is using the image inside Kubernetes running it as non-root user which requires to run Apache on an unprivileged port. I tried to keep the changes as minimal as possible.

Signed-off-by: Sascha <sascha@retter.jetzt>
Signed-off-by: Sascha Retter <sascha.retter@mailbox.org>
@J0WI
Copy link
Contributor

J0WI commented May 26, 2021

We use the PHP base image, so this is kind of a duplicate of docker-library/php#94

@saretter
Copy link
Author

Yes, seems to be a similar discussion. Nevertheless from my point of view the difference is that docker-library/php is a typical base-image while nextcloud is an application used by many users without building their own images and this minimal change would allow to let people run the image directly even if they need/want to follow the best-practice and run the container in unprivileged mode.

@danihodovic
Copy link

I think you need to modify this file https://github.com/nextcloud/docker/blob/master/21.0/apache/entrypoint.sh for it to work.

I applied your patch there and it worked for me.

@saretter
Copy link
Author

This is done by update.sh which is managing the different versions if understood it correctly. If one of the maintainers tells me to do so, I'll run update.sh and commit the changes.

@saretter
Copy link
Author

saretter commented Jun 8, 2021

Any additional comments, remarks ... Is there any chance this PR will be accepted?

@J0WI
Copy link
Contributor

J0WI commented Jun 16, 2021

IMHO this should be addressed in the php/httpd base image. This affects all images based on php and some solutions have already been discussed there.

@saretter
Copy link
Author

IMHO this should be addressed in the php/httpd base image. This affects all images based on php and some solutions have already been discussed there.

In general not each an every feature can make it into a library/framework. It requires a strict management of requirements and precise idea of the purpose of a lib.

I agree, from an applications perspective it would be best to implement it in the library because it would solve the issue not just for the nextcloud-image but for many other applications. Nevertheless I don't know much about and had no time to dig deeper into the design of docker-library and there might be reasons to reject the implementation of such a feature. From my point of view, in this case a simple and minimal change of the application is the second best option and is by far better than leaving it as it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants