-
Notifications
You must be signed in to change notification settings - Fork 2
configure IPv6 via ENV #3
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
base: master
Are you sure you want to change the base?
Conversation
gmta
left a comment
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.
Thank you for working on this!
README.md
Outdated
| 3. Edit `.env` and set: | ||
| * `VELOCITA_URL`: the URL (e.g. `https://mydomain.tld`) on which Velocita is hosted | ||
| * `VELOCITA_IPV6_ENABLED`: set to `true` to enable IPv6 |
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.
You don't need to add this here; IPv6 being enabled is already the default. This section is meant to show some configuration options a lot of people might want to change when initially setting up the proxy.
proxy/templates/proxy.conf.erb
Outdated
| velocita_url = velocita_url.sub(/\/+$/, '') | ||
| end | ||
| tls_enabled = ENV['VELOCITA_TLS_ENABLED'] == 'true' | ||
| ipv6_enabled = ENV['VELOCITA_IPV6_ENABLED'] == 'true' |
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.
- Should be called
VELOCITA_IPV6_LISTEN - The default value should be
true, so maybe make this(ENV['VELOCITA_IPV6_ENABLED'] || 'true') == 'true'? - Could you document this new setting in
proxy/README.md? Specifically this section: https://github.com/gmta/velocita-proxy/tree/master/proxy#configuring-velocita
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.
yeah, will be
ipv6_listen = (ENV['VELOCITA_IPV6_LISTEN'] || 'true') == 'true'
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.
I completely overlooked the second readme
.env.dist
Outdated
| VELOCITA_URL=http://localhost:8080 | ||
| VELOCITA_HTTP_PORT=8080 | ||
| VELOCITA_HTTPS_PORT=8443 | ||
| VELOCITA_IPV6_ENABLED=true |
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.
| VELOCITA_IPV6_ENABLED=true | |
| #VELOCITA_IPV6_LISTEN=true |
- Let's name this "listen" instead of "enabled" since it does not affect IPv6 connectivity as a whole, but just on which interfaces nginx is listening.
- The default value should be
true, so this is an optional setting.
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.
check
|
Did I miss something? |
regarding #2