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

Update 100-setup-error-pages.sh #12

Merged
merged 2 commits into from
May 2, 2021
Merged

Update 100-setup-error-pages.sh #12

merged 2 commits into from
May 2, 2021

Conversation

xpliz
Copy link
Contributor

@xpliz xpliz commented Apr 27, 2021

Description

Random template generator, also picked up nginx-error-pages template, which we don't want. Proposing small patch to exclude from allowed_templates

Random template generator, also picked up nginx-error-pages template, which we don't want. Proposing small patch to exclude from allowed_templates
@tarampampam
Copy link
Owner

👍 Great thanks for your contribution!

@xpliz xpliz closed this Apr 27, 2021
@xpliz xpliz deleted the patch-2 branch April 27, 2021 17:35
@xpliz xpliz restored the patch-2 branch April 27, 2021 18:37
@xpliz
Copy link
Contributor Author

xpliz commented Apr 29, 2021

Did I mess up this patch? Can't find it in code.

@tarampampam
Copy link
Owner

@xpliz You closed this PR before I merge it. Also, can you say how to reproduce the fixed error?

@xpliz
Copy link
Contributor Author

xpliz commented Apr 30, 2021

Well I just exec-ed into the container and checked what your piece of code returns and I saw find /opt/html/* -maxdepth 1 -type d also listed nginx-error-pages template/directory beside all available templates. I noticed that sometimes I got reply when getting an error with standard nginx message which bothered me and I looked into the code why.

errorpages    | /docker-entrypoint.sh: /docker-entrypoint.d/ is not empty, will attempt to perform configuration
errorpages    | /docker-entrypoint.sh: Looking for shell scripts in /docker-entrypoint.d/
errorpages    | /docker-entrypoint.sh: Launching /docker-entrypoint.d/10-listen-on-ipv6-by-default.sh
errorpages    | 10-listen-on-ipv6-by-default.sh: info: Getting the checksum of /etc/nginx/conf.d/default.conf
errorpages    | 10-listen-on-ipv6-by-default.sh: info: /etc/nginx/conf.d/default.conf differs from the packaged version
errorpages    | /docker-entrypoint.sh: Launching /docker-entrypoint.d/20-envsubst-on-templates.sh
errorpages    | /docker-entrypoint.sh: Launching /docker-entrypoint.d/30-tune-worker-processes.sh
errorpages    | /docker-entrypoint.sh: Launching /docker-entrypoint.d/100-setup-error-pages.sh
errorpages    | /docker-entrypoint.d/100-setup-error-pages.sh: Use 'nginx-error-pages' as randomly selected template
errorpages    | /docker-entrypoint.d/100-setup-error-pages.sh: Set pages for template 'nginx-error-pages' as default (make accessible in root directory)
errorpages    | /docker-entrypoint.sh: Configuration complete; ready for start up

This error message from container logs bothered me and I looked into it.

Will reopen the patch. Sorry I'm not that handy with git. I thought I pushed 2 patches by mistake ...

Also that feature request with random pages was more of a can I get a random template on every request not just container restart :) But of course anything is better than nothing :) Once I looked more into the code how things are spawn I saw template is generated on start and your random page solution was a nice workaround/hack :) Not sure why standard nginx-error-pages is there tho to be honest.

@xpliz xpliz reopened this Apr 30, 2021
@tarampampam
Copy link
Owner

Thanks for the detailed response! I have made other changes, could you approve them?

@tarampampam tarampampam added the type:bug Something isn't working label May 1, 2021
@xpliz
Copy link
Contributor Author

xpliz commented May 2, 2021

This github baffles me :)

@xpliz xpliz closed this May 2, 2021
@xpliz xpliz reopened this May 2, 2021
@xpliz
Copy link
Contributor Author

xpliz commented May 2, 2021

I can't seem to find approve or merge lol ¯_(ツ)_/¯

@tarampampam tarampampam merged commit 914d657 into tarampampam:master May 2, 2021
@tarampampam
Copy link
Owner

Released in https://github.com/tarampampam/error-pages/releases/tag/v1.7.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants