fix: gracefully handle comments in resolv.conf. Fixes #171 #176
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
dekobon
requested changes
Oct 2, 2023
common/docker-entrypoint.sh
Outdated
| # /etc/resolv.conf taken from the entrypoint script in the | ||
| # official docker image. | ||
| # https://github.com/nginxinc/docker-nginx/blob/master/entrypoint/15-local-resolvers.envsh | ||
| export DNS_RESOLVERS="$(awk 'BEGIN{ORS=" "} $1=="nameserver" {print $2}' /etc/resolv.conf)" |
There was a problem hiding this comment.
Thank you for catching this! I had overlooked this case and missed the enclosure in square brackets from the original code. I've made changes in this commit that combines your fix with the changes from the official dockerfile entrypoint.
Given a resolv.conf that looks like this:
nameserver 127.0.0.11
nameserver 8.8.4.4
nameserver 94.198.184.14
nameserver 94.198.184.34
# NOTE: the libc resolver may not support more than 3 nameservers.
# The nameservers listed below may not be recognized.
nameserver 8.8.8.8
# This is another comment
nameserver 8.8.8.9
nameserver fe80::f03c:93ff:fe25:6762
nameserver 8.9.9.9
nameserver 2a03:a800:2:1::1
nameserver 8.9.9.7
options ndots:0
We see:
DNS Resolvers: 127.0.0.11 8.8.4.4 94.198.184.14 94.198.184.34 8.8.8.8 8.8.8.9 [fe80::f03c:93ff:fe25:6762] 8.9.9.9 [2a03:a800:2:1::1] 8.9.9.7
Which looks right to me. Based on my read of the resolver syntax each address should be separately enclosed in square brackets.
dekobon
approved these changes
Oct 3, 2023
elJosho
pushed a commit
to elJosho/nginx-s3-gateway
that referenced
this pull request
Oct 25, 2023
…ginxinc#176) # What Fixes an issue where comments in the `/etc/resolv.conf` would get into the dns resolvers list and cause an error when starting the s3 gateway. ## How Following a suggestion on the issue, the logic from [this file](https://github.com/nginxinc/docker-nginx/blob/master/entrypoint/15-local-resolvers.envsh) in the official NGINX docker container was ported over to replace the existing logic. Given a `/etc/resolv.conf` that looks like this: ``` nameserver 127.0.0.11 nameserver 8.8.4.4 nameserver 94.198.184.14 nameserver 94.198.184.34 # NOTE: the libc resolver may not support more than 3 nameservers. # The nameservers listed below may not be recognized. nameserver 8.8.8.8 options ndots:0 ``` The existing code would produce this: ``` 127.0.0.11 8.8.4.4 94.198.184.14 94.198.184.34 [NOTE:] The 8.8.8.8 ``` With this change applied it looks like this: ``` 127.0.0.11 8.8.4.4 94.198.184.14 94.198.184.34 8.8.8.8 ``` The startup printout looks like this with the change applied: ``` Origin: http://bucket-1.minio:9000 Region: us-east-1 Addressing Style: virtual AWS Signatures Version: v2 DNS Resolvers: 127.0.0.11 8.8.4.4 94.198.184.14 94.198.184.34 8.8.8.8 Directory Listing Enabled: false Directory Listing Path Prefix: Provide Index Pages Enabled: Append slash for directory enabled: Stripping the following headers from responses: x-amz-; CORS Enabled: 0 /docker-entrypoint.sh: Launching /docker-entrypoint.d/10-listen-on-ipv6-by-default.sh 10-listen-on-ipv6-by-default.sh: info: Getting the checksum of /etc/nginx/conf.d/default.conf ``` ## Notes Ideally we should be incorporating the entrypoint scripts from the official nginx docker image. For now we're just porting key logic to get the issue solved quickly. The integration of these scripts will have some other concerns. An issue has been filed for this work: nginxinc#175
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Fixes an issue where comments in the
/etc/resolv.confwould get into the dns resolvers list and cause an error when starting the s3 gateway.How
Following a suggestion on the issue, the logic from this file in the official NGINX docker container was ported over to replace the existing logic.
Given a
/etc/resolv.confthat looks like this:The existing code would produce this:
With this change applied it looks like this:
The startup printout looks like this with the change applied:
Notes
Ideally we should be incorporating the entrypoint scripts from the official nginx docker image. For now we're just porting key logic to get the issue solved quickly. The integration of these scripts will have some other concerns. An issue has been filed for this work: #175