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

Add IPv6 support for local-resolvers substitution script #855

Merged

Conversation

serverwentdown
Copy link
Contributor

@serverwentdown serverwentdown commented Jan 4, 2024

Proposed changes

When running in IPv6 networks, the 15-local-resolvers.envsh script will create invalid nginx configuration because the IPv6 address is not wrapped in brackets as Nginx requires. This change checks for the presence of colons (which all valid IPv6 addresses should have), and wraps the address in square brackets. /etc/resolv.conf syntax should never present a host and port together, so it is a valid assumption that an IPv4 nameserver configuration will never have a colon present.

This is incredibly useful for getting DNS working in IPv6 Kubernetes environments.

Checklist

Before creating a PR, run through this checklist and mark each as complete:

  • I have read the CONTRIBUTING document
  • I have run ./update.sh and ensured all entrypoint/Dockerfile template changes have been applied to the relevant image entrypoint scripts & Dockerfiles
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • I have updated any relevant documentation

@thresheek
Copy link
Collaborator

thresheek commented Jan 9, 2024

Hi @serverwentdown !

Thanks for your contribution - the changes look reasonable to me.

May I ask you to extend config.sh to automatically run the tests?

@thresheek
Copy link
Collaborator

I went ahead and added templates-resolver-ipv6 to the tests run and fixed an actual test to:

  • properly register exit traps
  • use ipv6 address of a test container to connect to
  • check for ipv6 addresses being present in the configuration

The last check depends on the environment behaviour which isnt great, but I don't think we can easily emulate editing /etc/resolv.conf in this case, so we have to rely on GH runners to do the correct thing for us.

@thresheek thresheek merged commit e230e12 into nginxinc:master Feb 7, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants