-
-
Notifications
You must be signed in to change notification settings - Fork 498
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
feat: Allow vhosts to listen on own hosts #256
base: master
Are you sure you want to change the base?
feat: Allow vhosts to listen on own hosts #256
Conversation
203206f
to
2cc936c
Compare
Current vhosts defenition limit them to listen only on single IP/port. In the meanwhile, there are usecases where vhosts on the same server might wanna listen on different quite independent set of IPs/ports, which is not currently possible. This patch adds such functionality, and an operator might define extra properties for apache_vhosts to make them listening on required ports/interfaces.
2cc936c
to
1b07bbd
Compare
@@ -1,8 +1,25 @@ | |||
{{ apache_global_vhost_settings }} | |||
|
|||
{# Set up VirtualHosts #} | |||
{% set _extra_listen_list = [] %} |
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.
This is a lot of extra logic inside a template to try to build a list of listen IP/ports, it seems like it will add a lot of overhead to maintaining this role if I were to merge it. Is there any other way to achieve this complexity without adding a ton of hard-to-test logic?
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.
So, let me probably explain the usecase better.
Eventually we have series of roles for services that need apache (as one of scenarios when mod_oidc is required for auth). These services by default are running on different ports and listen to specific IPs.
They may (or may not) run on same hosts.
So idea is to include the apache role from these individual service roles when needed and configure series of vhosts on potentially same server.
We already do setup apache in these roles independently, but obviously makes sense to leverage some role instead. Sample of such role vhost:
https://opendev.org/openstack/openstack-ansible-os_keystone/src/commit/d163d9173050a53eb52f3889e8a1cdcb3ce1bfdd/templates/keystone-httpd.conf.j2
So what we currently do there, is just wipe out ports.conf and have all configuration inside vhosts, which makes things flexible and less complex, as you don't need to have Listen defined separately. As the way how ports.conf are configured now makes it kind of very hard to have multiple Listen statements and keep compatibility.
Moving apache_ports_configuration_items
from ports.conf to vhosts is possible and will also simplify logic there, but it feels as potentially breaking change in the logic.
Another way around might be replacing lineinfile
with blockinfile
and placing all options needed inside a block without need to regexp each statement (since you can repeat Listen
multiple times - regexp not great). Or do a copy
with content
at all...
Another complication that comes in picture is Apache 2.2 support. Which I kind of wonder if makes sense at this point of time, given no tested OS does not come with it. And I wonder if any OS that comes with 2.2 has not reached EOL at all this point. And this is second way to reduce complexity I see.
But having listen per vhost and making it configurable makes total sense to ensure the role is reusable, as if it's gonna be included multiple times, you can pass different apache_vhosts_filename
making the role so more flexible...
This pr has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution! Please read this blog post to see the reasons why I mark issues as stale. |
Current vhosts defenition limit them to listen only on single IP/port.
In the meanwhile, there are usecases where vhosts on the same server
might wanna listen on different quite independent set of IPs/ports,
which is not currently possible.
This patch adds such functionality, and an operator might define
extra properties for apache_vhosts to make them listening on required
ports/interfaces.