#2544 Allow multiple IP addresses per vhost #1229

Benedikt1992 · 2015-10-20T14:12:49Z

To make a vhost reachable over 2 IP addresses we need to configure 2
similar vhosts which differ in the IP address. This change allows to use an array
of IPs.

Ticket: https://tickets.puppetlabs.com/browse/MODULES-2544

Since Release 0.9.0 we have a dependency on stdlib >= 2.4.x CHANGELOG
With the changes in this pull requests we need stdlib >= 4.0.0 since suffix() is not available in Release 2.4.x stdlib CHANGELOG, Code

igalic · 2015-10-20T16:00:50Z

manifests/vhost.pp

    if $port {
-      $listen_addr_port = "${ip}:${port}"
-      $nvh_addr_port = "${ip}:${port}"
+      $listen_addr_port = parseyaml(inline_template('<% res="" %><% @_ip.each { |a_ip| res = res + "#{a_ip}:#{@port}," } %><%= res.split(",").to_yaml -%>'))


this feels dirty

Let's call this an interesting approach.. @igalic : Would you suggest using a custom function for this? Nevertheless, this could be done a lot easier with future_parser enabled...

we'll have a real hard time porting this module to 4.x because it's pretty much one of the most used ones out there… i don't really have a strategy there…

@igalic Like @timogoebel said: Do you have another approach?

this hole line can be broken up into one call to suffix():

$listen_addr_port = suffix($_ip, $port)

That looks better. But this needs stdlib >= 4.0.0. If the Changelog is right this Requirement will jump from 2.4.x to 4.0.0...

@igalic I've changed that line to suffix(). I've also moved the convertion of nvh_addr_port into an array. It is now in the template. So we nearly don't need any2array() in the vhost.pp

igalic · 2015-10-20T16:04:16Z

anyway, i think what you really want is… something entirely different:

if we're talking about name-based VirtualHosts then:

<fajita> http://httpd.apache.org/docs/current/mod/core.html#namevirtualhost. (Deprecated in 2.4) You must specify only one NameVirtualHost directive, hopefully with an IP address  and port or a *:80. It says on which interfaces should Apache listen to name based vhosts requests, nothing more.

or are you talking about 2.2 / ip-based virtualhosts?

Benedikt1992 · 2015-10-21T09:55:16Z

@igalic as far as i see it does not matter if we have ip based or name-based vhosts. Let's say we have a webpage that should be available over IPv4 and IPv6 with vhosts. With the current apache Module we have to make 2 vhosts which just differ in the IP address. A better solution from my point of view is to make one ip based vhost which listens on two ip addresses. So instead of

<VirtualHost 192.168.1.1>
    DocumentRoot "/www/server"
    ServerName server.example.com
    ServerAlias server

    # Other directives here ...
</VirtualHost>

<VirtualHost 2001:DB8::1>
    DocumentRoot "/www/server"
    ServerName server.example.com
    ServerAlias server

    # Other directives here ...
</VirtualHost>

we have

<VirtualHost 192.168.1.1 2001:DB8::1>
    DocumentRoot "/www/server"
    ServerName server.example.com
    ServerAlias server

    # Other directives here ...
</VirtualHost>

With name-based vhosts and apache 2.2 we would also need

NameVirtualHost 192.168.1.1
NameVirtualHost 2001:DB8::1

Apache 2.4 will automatically configure name-based vhosts if there is more than one VirtualHost definition with the same IP address

igalic · 2015-10-21T10:57:38Z

@Benedikt1992 what i don't understand is why you can't just use *:80

Benedikt1992 · 2015-10-21T11:45:52Z

@igalic That works for name-based vhosts. With ip-based vhosts you have to declare every ip address explicitly. Otherwise it wouldn't be a ip-based vhost.

igalic · 2015-10-21T12:45:24Z

yes, but you don't need NameVirtualHost for each of those.
the point of NameVirtualHost xis to tell httpd (<2.4) that following this point, match all hosts with the pattern x as name-based virtualhosts.

Please read these two https://httpd.apache.org/docs/2.2/vhosts/name-based.html
https://httpd.apache.org/docs/2.2/vhosts/ip-based.html

Benedikt1992 · 2015-10-21T14:42:22Z

NameVirtualHost is only set if ip_based = false and only once per IP address/port combination (because of ensure_resource()) Code

Basically the behaviour has not changed. It is just able to handle an array of IP address/port combinations.

Benedikt1992 · 2015-10-26T13:16:38Z

Is someone still working on this pull request?

igalic · 2015-10-27T10:52:11Z

@Benedikt1992 it was a long weekend.
let's take a fresh look at this.

igalic · 2015-10-28T14:23:08Z

spec/acceptance/vhost_spec.rb

+      it { is_expected.to contain 'Listen 127.0.0.1:80' }
+      it { is_expected.to contain 'Listen ::1:80' }
+      it { is_expected.not_to contain 'NameVirtualHost 127.0.0.1:80' }
+      it { is_expected.not_to contain 'NameVirtualHost ::1:80' }


i'm still confused about this bit here ^

This test case generates an ip-based vhost. Ip-based vhosts don't have a NameVirtualHost directive. Probably we could delete this two tests scince it is not directly related to the changes. But it is neither false.

what i mean, when we want to use ip-based vhosts, then the namevirtualhost stanzas will create just more confusion. they shouldn't be created at all.

To make a vhost reachable over 2 IP addresses we need to configure 2 similar vhosts which differ in the IP address. This change allows to use an array of IPs.

igalic · 2015-10-28T18:29:20Z

@Benedikt1992 aside when the nvh issue, could you please squash…

also, if you want to have extra magick internet points, you can also change the git commit author to something github recognizes, or else add this email into your github settings.

…dresses_per_vhost_are_not_possible #2544 Allow multiple IP addresses per vhost

igalic · 2015-10-28T19:35:44Z

thanks @Benedikt1992!

Benedikt1992 · 2015-10-28T21:56:09Z

@igalic Thank you for merging. It is not possible to change the author afterwards, isn't it?

I will keep squashing in my mind

igalic · 2015-10-29T08:30:30Z

not in git.
not once it's merged.
(well, we too can git push -f, to master, but that's not nice)
it's now there to stay forever.

(i think you can add it to your profile, and maybe github will be… re-scanning it, adding it to your… magick internet point score)

igalic reviewed Oct 20, 2015
View reviewed changes

Benedikt1992 force-pushed the ticket/2544-Multiple_ip_addresses_per_vhost_are_not_possible branch from bf9c2f7 to c662f43 Compare October 20, 2015 18:32

Benedikt1992 force-pushed the ticket/2544-Multiple_ip_addresses_per_vhost_are_not_possible branch from c662f43 to a961bd9 Compare October 28, 2015 10:20

igalic reviewed Oct 28, 2015
View reviewed changes

Allow multiple IP addresses per vhost

c492b2b

To make a vhost reachable over 2 IP addresses we need to configure 2 similar vhosts which differ in the IP address. This change allows to use an array of IPs.

Benedikt1992 force-pushed the ticket/2544-Multiple_ip_addresses_per_vhost_are_not_possible branch from a961bd9 to c492b2b Compare October 28, 2015 15:15

igalic added a commit that referenced this pull request Oct 28, 2015

Merge pull request #1229 from Benedikt1992/ticket/2544-Multiple_ip_ad…

08ead35

…dresses_per_vhost_are_not_possible #2544 Allow multiple IP addresses per vhost

igalic merged commit 08ead35 into puppetlabs:master Oct 28, 2015

GSPatton added the feature label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#2544 Allow multiple IP addresses per vhost #1229

#2544 Allow multiple IP addresses per vhost #1229

Benedikt1992 commented Oct 20, 2015

igalic Oct 20, 2015

timogoebel Oct 20, 2015

igalic Oct 20, 2015

Benedikt1992 Oct 21, 2015

igalic Oct 27, 2015

Benedikt1992 Oct 28, 2015

Benedikt1992 Oct 28, 2015

igalic commented Oct 20, 2015

Benedikt1992 commented Oct 21, 2015

igalic commented Oct 21, 2015

Benedikt1992 commented Oct 21, 2015

igalic commented Oct 21, 2015

Benedikt1992 commented Oct 21, 2015

Benedikt1992 commented Oct 26, 2015

igalic commented Oct 27, 2015

igalic Oct 28, 2015

Benedikt1992 Oct 28, 2015

igalic Oct 28, 2015

igalic commented Oct 28, 2015

igalic commented Oct 28, 2015

Benedikt1992 commented Oct 28, 2015

igalic commented Oct 29, 2015

#2544 Allow multiple IP addresses per vhost #1229

#2544 Allow multiple IP addresses per vhost #1229

Conversation

Benedikt1992 commented Oct 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igalic commented Oct 20, 2015

Benedikt1992 commented Oct 21, 2015

igalic commented Oct 21, 2015

Benedikt1992 commented Oct 21, 2015

igalic commented Oct 21, 2015

Benedikt1992 commented Oct 21, 2015

Benedikt1992 commented Oct 26, 2015

igalic commented Oct 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igalic commented Oct 28, 2015

igalic commented Oct 28, 2015

Benedikt1992 commented Oct 28, 2015

igalic commented Oct 29, 2015