horizon haproxy firewall rules shadows haproxy stats iptables rule

Bug #1829338 reported by Michele Baldessari
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
tripleo
Fix Released
High
Michele Baldessari

Bug Description

Things broke with https://review.opendev.org/#/c/625600/.
The reason that was pushed is that in composable roles, when splitting off horizon off from where haproxy runs, we would not have the proper iptables rules on the haproxy role. This was due to the fact that we had the following code:
      service_config_settings:
        haproxy:
          tripleo.horizon.firewall_rules:
            '127 horizon':
              dport:
                - 80
                - 443

The code never worked as explained in 3f8ce6fd96bc4f28a052b4c87a19b4b152734091 and so we fixed it by setting the proper tripleo.haproxy.firewall_rules key. The issue is fundamentally that rules for haproxy should just never have been set at all via service_config_settings keys in the first place. As demonstrated with this bug, the merging of hiera dictionaries will mess us up and we'll end up overwriting other keys. Haproxy stats access has this:
outputs:
  role_data:
    description: Role data for the HAproxy role.
    value:
      service_name: haproxy
      monitoring_subscription: {get_param: MonitoringSubscriptionHaproxy}
      config_settings:
        map_merge:
          - tripleo.haproxy.firewall_rules:
              '107 haproxy stats':
                dport: 1993

And since hiera will return the horizon settings for tripleo.haproxy.firewall_rules which won't be deep merged with the firewall rules from haproxy stats

Rules for haproxy need to happen in puppet-tripleo/manifests/haproxy*. Normally they do, the exception is horizon which uses a specialized horizon_endpoint.pp manifest which does not trigger these rules.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to puppet-tripleo (master)

Related fix proposed to branch: master
Review: https://review.opendev.org/659466

Changed in tripleo:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tripleo-heat-templates (master)

Fix proposed to branch: master
Review: https://review.opendev.org/659467

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to puppet-tripleo (master)

Reviewed: https://review.opendev.org/659466
Committed: https://git.openstack.org/cgit/openstack/puppet-tripleo/commit/?id=6c2e164adaa0bf0bbc38a3d69e16db105a5519aa
Submitter: Zuul
Branch: master

commit 6c2e164adaa0bf0bbc38a3d69e16db105a5519aa
Author: Michele Baldessari <email address hidden>
Date: Thu May 16 09:12:50 2019 +0200

    Fix horizon firewall rules in composable roles

    Atm horizon haproxy firewall rules obfuscate any other rule defined via
    the tripleo.haproxy.firewall_rules key.

    Things broke with https://review.opendev.org/#/c/625600/. The reason
    that was pushed is that in composable roles, when splitting off horizon
    away from where haproxy runs, we would not have the proper iptables rules
    on the haproxy role. This was due to the fact that we had
    the following code:
          service_config_settings:
            haproxy:
              tripleo.horizon.firewall_rules:
                '127 horizon':
                  dport:
                    - 80
                    - 443

    The above code never worked as explained in
    3f8ce6fd96bc4f28a052b4c87a19b4b152734091 and so we fixed it by setting
    the proper tripleo.haproxy.firewall_rules key. The issue is that rules
    for haproxy should just never have been set at all via
    service_config_settings keys in the first place. As demonstrated with
    this bug, the merging of hiera dictionaries will mess us up and we'll
    end up overwriting other keys. Haproxy stats access has this:
    outputs:
      role_data:
        description: Role data for the HAproxy role.
        value:
          service_name: haproxy
          monitoring_subscription: {get_param: MonitoringSubscriptionHaproxy}
          config_settings:
            map_merge:
              - tripleo.haproxy.firewall_rules:
                  '107 haproxy stats':
                    dport: 1993

    And since hiera will return the horizon settings for
    tripleo.haproxy.firewall_rules which won't be deep merged with the
    firewall rules from haproxy stats and so rule '107 haproxy stats' will
    never be present.

    Rules for haproxy need to happen in puppet-tripleo/manifests/haproxy*.
    Normally they do, the exception is horizon which uses a specialized
    horizon_endpoint.pp manifest which does not trigger these rules.

    Let's create the firewall rules in haproxy/horizon_endpoint.pp like we
    do for all other endpoints.

    Tested and correctly got:
    [root@controller-0 ~]# iptables -nvL |grep hor
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 80 state NEW /* 100 horizon_haproxy ipv4 */
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 443 state NEW /* 100 horizon_haproxy_ssl ipv4 */
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 80,443 state NEW /* 126 horizon ipv4 */

    Change-Id: I1325171ef60d7a7e3b57373082fcdb5487be939b
    Related-Bug: #1829338

Changed in tripleo:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tripleo-heat-templates (master)

Reviewed: https://review.opendev.org/659467
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=114e5778f95d51e518c0ef73b05c54fca160dbe0
Submitter: Zuul
Branch: master

commit 114e5778f95d51e518c0ef73b05c54fca160dbe0
Author: Michele Baldessari <email address hidden>
Date: Thu May 16 09:16:51 2019 +0200

    Remove the iptables rules set via service_config_settings

    This breaks the rules for the haproxy stats access because it
    shadows them. Let's remove these rules and move the iptables
    rules for haproxy in puppet-tripleo where they should have
    been in the first place, like for all other services.

    Depends-On: I1325171ef60d7a7e3b57373082fcdb5487be939b
    Change-Id: I2f177c930567b3a45f0d95cec4140f478f14a074
    Closes-Bug: #1829338

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to puppet-tripleo (stable/stein)

Related fix proposed to branch: stable/stein
Review: https://review.opendev.org/661339

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tripleo-heat-templates (stable/stein)

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/661340

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to puppet-tripleo (stable/stein)

Reviewed: https://review.opendev.org/661339
Committed: https://git.openstack.org/cgit/openstack/puppet-tripleo/commit/?id=f58d8af343c24a709fdfb735534e2dc6b4209338
Submitter: Zuul
Branch: stable/stein

commit f58d8af343c24a709fdfb735534e2dc6b4209338
Author: Michele Baldessari <email address hidden>
Date: Thu May 16 09:12:50 2019 +0200

    Fix horizon firewall rules in composable roles

    Atm horizon haproxy firewall rules obfuscate any other rule defined via
    the tripleo.haproxy.firewall_rules key.

    Things broke with https://review.opendev.org/#/c/625600/. The reason
    that was pushed is that in composable roles, when splitting off horizon
    away from where haproxy runs, we would not have the proper iptables rules
    on the haproxy role. This was due to the fact that we had
    the following code:
          service_config_settings:
            haproxy:
              tripleo.horizon.firewall_rules:
                '127 horizon':
                  dport:
                    - 80
                    - 443

    The above code never worked as explained in
    3f8ce6fd96bc4f28a052b4c87a19b4b152734091 and so we fixed it by setting
    the proper tripleo.haproxy.firewall_rules key. The issue is that rules
    for haproxy should just never have been set at all via
    service_config_settings keys in the first place. As demonstrated with
    this bug, the merging of hiera dictionaries will mess us up and we'll
    end up overwriting other keys. Haproxy stats access has this:
    outputs:
      role_data:
        description: Role data for the HAproxy role.
        value:
          service_name: haproxy
          monitoring_subscription: {get_param: MonitoringSubscriptionHaproxy}
          config_settings:
            map_merge:
              - tripleo.haproxy.firewall_rules:
                  '107 haproxy stats':
                    dport: 1993

    And since hiera will return the horizon settings for
    tripleo.haproxy.firewall_rules which won't be deep merged with the
    firewall rules from haproxy stats and so rule '107 haproxy stats' will
    never be present.

    Rules for haproxy need to happen in puppet-tripleo/manifests/haproxy*.
    Normally they do, the exception is horizon which uses a specialized
    horizon_endpoint.pp manifest which does not trigger these rules.

    Let's create the firewall rules in haproxy/horizon_endpoint.pp like we
    do for all other endpoints.

    Tested and correctly got:
    [root@controller-0 ~]# iptables -nvL |grep hor
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 80 state NEW /* 100 horizon_haproxy ipv4 */
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 443 state NEW /* 100 horizon_haproxy_ssl ipv4 */
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 80,443 state NEW /* 126 horizon ipv4 */

    Change-Id: I1325171ef60d7a7e3b57373082fcdb5487be939b
    Related-Bug: #1829338
    (cherry picked from commit 6c2e164adaa0bf0bbc38a3d69e16db105a5519aa)

tags: added: in-stable-stein
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tripleo-heat-templates (stable/stein)

Reviewed: https://review.opendev.org/661340
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=6b4f03f9ea0e958e75f82040d674f67a6cdcfc87
Submitter: Zuul
Branch: stable/stein

commit 6b4f03f9ea0e958e75f82040d674f67a6cdcfc87
Author: Michele Baldessari <email address hidden>
Date: Thu May 16 09:16:51 2019 +0200

    Remove the iptables rules set via service_config_settings

    This breaks the rules for the haproxy stats access because it
    shadows them. Let's remove these rules and move the iptables
    rules for haproxy in puppet-tripleo where they should have
    been in the first place, like for all other services.

    Depends-On: I1325171ef60d7a7e3b57373082fcdb5487be939b
    Change-Id: I2f177c930567b3a45f0d95cec4140f478f14a074
    Closes-Bug: #1829338
    (cherry picked from commit 114e5778f95d51e518c0ef73b05c54fca160dbe0)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to puppet-tripleo (stable/rocky)

Related fix proposed to branch: stable/rocky
Review: https://review.opendev.org/661745

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tripleo-heat-templates (stable/rocky)

Fix proposed to branch: stable/rocky
Review: https://review.opendev.org/661746

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to puppet-tripleo (stable/rocky)
Download full text (3.2 KiB)

Reviewed: https://review.opendev.org/661745
Committed: https://git.openstack.org/cgit/openstack/puppet-tripleo/commit/?id=b34f02c7fe3acf3fcd33b0284a765268f403a142
Submitter: Zuul
Branch: stable/rocky

commit b34f02c7fe3acf3fcd33b0284a765268f403a142
Author: Michele Baldessari <email address hidden>
Date: Thu May 16 09:12:50 2019 +0200

    Fix horizon firewall rules in composable roles

    Atm horizon haproxy firewall rules obfuscate any other rule defined via
    the tripleo.haproxy.firewall_rules key.

    Things broke with https://review.opendev.org/#/c/625600/. The reason
    that was pushed is that in composable roles, when splitting off horizon
    away from where haproxy runs, we would not have the proper iptables rules
    on the haproxy role. This was due to the fact that we had
    the following code:
          service_config_settings:
            haproxy:
              tripleo.horizon.firewall_rules:
                '127 horizon':
                  dport:
                    - 80
                    - 443

    The above code never worked as explained in
    3f8ce6fd96bc4f28a052b4c87a19b4b152734091 and so we fixed it by setting
    the proper tripleo.haproxy.firewall_rules key. The issue is that rules
    for haproxy should just never have been set at all via
    service_config_settings keys in the first place. As demonstrated with
    this bug, the merging of hiera dictionaries will mess us up and we'll
    end up overwriting other keys. Haproxy stats access has this:
    outputs:
      role_data:
        description: Role data for the HAproxy role.
        value:
          service_name: haproxy
          monitoring_subscription: {get_param: MonitoringSubscriptionHaproxy}
          config_settings:
            map_merge:
              - tripleo.haproxy.firewall_rules:
                  '107 haproxy stats':
                    dport: 1993

    And since hiera will return the horizon settings for
    tripleo.haproxy.firewall_rules which won't be deep merged with the
    firewall rules from haproxy stats and so rule '107 haproxy stats' will
    never be present.

    Rules for haproxy need to happen in puppet-tripleo/manifests/haproxy*.
    Normally they do, the exception is horizon which uses a specialized
    horizon_endpoint.pp manifest which does not trigger these rules.

    Let's create the firewall rules in haproxy/horizon_endpoint.pp like we
    do for all other endpoints.

    Tested and correctly got:
    [root@controller-0 ~]# iptables -nvL |grep hor
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 80 state NEW /* 100 horizon_haproxy ipv4 */
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 443 state NEW /* 100 horizon_haproxy_ssl ipv4 */
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 80,443 state NEW /* 126 horizon ipv4 */

    Change-Id: I1325171ef60d7a7e3b57373082fcdb5487be939b
    Related-Bug: #1829338
    (cherry picked from commit 6c2e164adaa0bf0bbc38a3d69e16db105a5519aa)
    (cherry picked from commi...

Read more...

tags: added: in-stable-rocky
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tripleo-heat-templates (stable/rocky)

Reviewed: https://review.opendev.org/661746
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=ff10e2f6b9968d51f43c95eaf2082d85fe215a34
Submitter: Zuul
Branch: stable/rocky

commit ff10e2f6b9968d51f43c95eaf2082d85fe215a34
Author: Michele Baldessari <email address hidden>
Date: Thu May 16 09:16:51 2019 +0200

    Remove the iptables rules set via service_config_settings

    This breaks the rules for the haproxy stats access because it
    shadows them. Let's remove these rules and move the iptables
    rules for haproxy in puppet-tripleo where they should have
    been in the first place, like for all other services.

    NB: Cherry pick is clean but had to be moved to
    puppet/services/horizon.yaml instead of the docker/services/horizon.yaml
    Depends-On: I1325171ef60d7a7e3b57373082fcdb5487be939b
    Change-Id: I2f177c930567b3a45f0d95cec4140f478f14a074
    Closes-Bug: #1829338
    (cherry picked from commit 114e5778f95d51e518c0ef73b05c54fca160dbe0)
    (cherry picked from commit 6b4f03f9ea0e958e75f82040d674f67a6cdcfc87)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to puppet-tripleo (stable/queens)

Related fix proposed to branch: stable/queens
Review: https://review.opendev.org/662137

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tripleo-heat-templates (stable/queens)

Fix proposed to branch: stable/queens
Review: https://review.opendev.org/662138

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to puppet-tripleo (stable/queens)
Download full text (3.2 KiB)

Reviewed: https://review.opendev.org/662137
Committed: https://git.openstack.org/cgit/openstack/puppet-tripleo/commit/?id=ec4b1b6fc4c950958ca976f8c0cadb07cd83439c
Submitter: Zuul
Branch: stable/queens

commit ec4b1b6fc4c950958ca976f8c0cadb07cd83439c
Author: Michele Baldessari <email address hidden>
Date: Thu May 16 09:12:50 2019 +0200

    Fix horizon firewall rules in composable roles

    Atm horizon haproxy firewall rules obfuscate any other rule defined via
    the tripleo.haproxy.firewall_rules key.

    Things broke with https://review.opendev.org/#/c/625600/. The reason
    that was pushed is that in composable roles, when splitting off horizon
    away from where haproxy runs, we would not have the proper iptables rules
    on the haproxy role. This was due to the fact that we had
    the following code:
          service_config_settings:
            haproxy:
              tripleo.horizon.firewall_rules:
                '127 horizon':
                  dport:
                    - 80
                    - 443

    The above code never worked as explained in
    3f8ce6fd96bc4f28a052b4c87a19b4b152734091 and so we fixed it by setting
    the proper tripleo.haproxy.firewall_rules key. The issue is that rules
    for haproxy should just never have been set at all via
    service_config_settings keys in the first place. As demonstrated with
    this bug, the merging of hiera dictionaries will mess us up and we'll
    end up overwriting other keys. Haproxy stats access has this:
    outputs:
      role_data:
        description: Role data for the HAproxy role.
        value:
          service_name: haproxy
          monitoring_subscription: {get_param: MonitoringSubscriptionHaproxy}
          config_settings:
            map_merge:
              - tripleo.haproxy.firewall_rules:
                  '107 haproxy stats':
                    dport: 1993

    And since hiera will return the horizon settings for
    tripleo.haproxy.firewall_rules which won't be deep merged with the
    firewall rules from haproxy stats and so rule '107 haproxy stats' will
    never be present.

    Rules for haproxy need to happen in puppet-tripleo/manifests/haproxy*.
    Normally they do, the exception is horizon which uses a specialized
    horizon_endpoint.pp manifest which does not trigger these rules.

    Let's create the firewall rules in haproxy/horizon_endpoint.pp like we
    do for all other endpoints.

    Tested and correctly got:
    [root@controller-0 ~]# iptables -nvL |grep hor
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 80 state NEW /* 100 horizon_haproxy ipv4 */
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 443 state NEW /* 100 horizon_haproxy_ssl ipv4 */
        0 0 ACCEPT tcp -- * * 0.0.0.0/0 0.0.0.0/0 multiport dports 80,443 state NEW /* 126 horizon ipv4 */

    Change-Id: I1325171ef60d7a7e3b57373082fcdb5487be939b
    Related-Bug: #1829338
    (cherry picked from commit 6c2e164adaa0bf0bbc38a3d69e16db105a5519aa)
    (cherry picked from comm...

Read more...

tags: added: in-stable-queens
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tripleo-heat-templates (stable/queens)

Reviewed: https://review.opendev.org/662138
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=525def10109b04d05ff833e6ea1207b0fa7d2e0b
Submitter: Zuul
Branch: stable/queens

commit 525def10109b04d05ff833e6ea1207b0fa7d2e0b
Author: Michele Baldessari <email address hidden>
Date: Thu May 16 09:16:51 2019 +0200

    Remove the iptables rules set via service_config_settings

    This breaks the rules for the haproxy stats access because it
    shadows them. Let's remove these rules and move the iptables
    rules for haproxy in puppet-tripleo where they should have
    been in the first place, like for all other services.

    NB: Cherry pick is clean but had to be moved to
    puppet/services/horizon.yaml instead of the docker/services/horizon.yaml
    Depends-On: I1325171ef60d7a7e3b57373082fcdb5487be939b
    Change-Id: I2f177c930567b3a45f0d95cec4140f478f14a074
    Closes-Bug: #1829338
    (cherry picked from commit 114e5778f95d51e518c0ef73b05c54fca160dbe0)
    (cherry picked from commit 6b4f03f9ea0e958e75f82040d674f67a6cdcfc87)
    (cherry picked from commit ff10e2f6b9968d51f43c95eaf2082d85fe215a34)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/tripleo-heat-templates 11.0.0

This issue was fixed in the openstack/tripleo-heat-templates 11.0.0 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/tripleo-heat-templates 9.4.0

This issue was fixed in the openstack/tripleo-heat-templates 9.4.0 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/tripleo-heat-templates 8.4.0

This issue was fixed in the openstack/tripleo-heat-templates 8.4.0 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/tripleo-heat-templates 10.6.0

This issue was fixed in the openstack/tripleo-heat-templates 10.6.0 release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.