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.

Sign up for GitHub

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

Jump to bottom

Fix bugs & add feature #19

Merged
merged 13 commits into from Jun 13, 2016
Merged

Fix bugs & add feature #19

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b3be108
Fix General Settings/Custom options not applied
manusfreedom Jun 7, 2016
d5bc5f6
Fix frontend without SSL&certificate after one with SSL&certificate
manusfreedom Jun 7, 2016
991b856
Put server at end of backend
manusfreedom Jun 7, 2016
a8d7589
Add default-server parameter in backend
manusfreedom Jun 7, 2016
9478304
Add port range support in frontend
manusfreedom Jun 7, 2016
6b9cada
Set backend.tuning_defaultserver as advanced
manusfreedom Jun 7, 2016
6ed7455
Add custom options in General Settings/Default Parameters
manusfreedom Jun 7, 2016
661646c
Add custom options in General Settings/Statistics Configuration
manusfreedom Jun 7, 2016
90e7224
Fix service unable to run with uid in global ("Some configuration opt…
manusfreedom Jun 7, 2016
75bcb37
Fix connectionBehaviour in tcp mode
manusfreedom Jun 7, 2016
4ef2cb7
Shorter label
manusfreedom Jun 12, 2016
2e641cf
Add run as root option
manusfreedom Jun 12, 2016
e64babd
Fix ssl_certs not always reset
manusfreedom Jun 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions net/haproxy/src/opnsense/mvc/app/controllers/OPNsense/HAProxy/forms/dialogBackend.xml
View file
Open in desktop
Expand Up @@ -130,6 +130,20 @@
<help><![CDATA[These lines will be added to the HAProxy backend configuration.<br/><div class="text-info"><b>NOTE:</b> The syntax will not be checked, use at your own risk!</div>]]></help>
<advanced>true</advanced>
</field>
<field>
<id>backend.tuning_defaultserver</id>
<label>Default for server</label>
<type>text</type>
<help><![CDATA[Default option for all server entries.]]></help>
<advanced>true</advanced>
</field>
<field>
<id>backend.tuning_noport</id>
<label>Don't use port on server</label>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a shorter "label". What about "Use Frontend port" instead?
The help text does provide a good explanation, so we should keep labels short.

<type>checkbox</type>
<help><![CDATA[Don't use port on server, use the same port as frontend receive. If check enable, require port check in server.]]></help>
<advanced>true</advanced>
</field>
<field>
<label>Actions (ACLs)</label>
<type>header</type>
Expand Down
7 changes: 7 additions & 0 deletions net/haproxy/src/opnsense/mvc/app/controllers/OPNsense/HAProxy/forms/dialogServer.xml
View file
Open in desktop
Expand Up @@ -51,4 +51,11 @@
<help><![CDATA[Sets the interval (in milliseconds) for running health checks on the server.]]></help>
<advanced>true</advanced>
</field>
<field>
<id>server.checkport</id>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition.

<label>Port to check</label>
<type>text</type>
<help><![CDATA[Provide the TCP communication port to use during check, i.e. 80 or 443.]]></help>
<advanced>true</advanced>
</field>
</form>
14 changes: 14 additions & 0 deletions net/haproxy/src/opnsense/mvc/app/controllers/OPNsense/HAProxy/forms/main.xml
View file
Open in desktop
Expand Up @@ -118,6 +118,13 @@
<type>dropdown</type>
<help><![CDATA[Enable or disable session redistribution in case of connection failure.]]></help>
</field>
<field>
<id>haproxy.general.defaults.customOptions</id>
<label>Custom options</label>
<type>textbox</type>
<help><![CDATA[These lines will be added to the defaults settings of to the HAProxy configuration file.<br/><div class="text-info"><b>NOTE:</b> The syntax will not be checked, use at your own risk!</div>]]></help>
<advanced>true</advanced>
</field>
</subtab>
<subtab id="haproxy-general-logging" description="Logging Configuration">
<field>
Expand Down Expand Up @@ -189,6 +196,13 @@
<help><![CDATA[Grant access to HAProxy statistics page. Please provide both user and password in clear text separated by a ':', i.e. john:secret123 or jdoe:anonymous. Use TAB key to complete adding a user.]]></help>
<hint>Enter user:password here. Finish with TAB.</hint>
</field>
<field>
<id>haproxy.general.stats.customOptions</id>
<label>Custom options</label>
<type>textbox</type>
<help><![CDATA[These lines will be added to the statistics settings of to the HAProxy configuration file.<br/><div class="text-info"><b>NOTE:</b> The syntax will not be checked, use at your own risk!</div>]]></help>
<advanced>true</advanced>
</field>
</subtab>
</tab>

Expand Down
24 changes: 22 additions & 2 deletions net/haproxy/src/opnsense/mvc/app/models/OPNsense/HAProxy/HAProxy.xml
View file
Open in desktop
Expand Up @@ -114,6 +114,9 @@
<x-3>redispatch on the 3rd retry prior to the last retry</x-3>
</OptionValues>
</redispatch>
<customOptions type="TextField">
<Required>N</Required>
</customOptions>
</defaults>
<logging>
<host type="TextField">
Expand Down Expand Up @@ -208,6 +211,9 @@
<mask>/^((([0-9a-zA-Z._\-]+:[0-9a-zA-Z._\-]+)([,]){0,1}))*/u</mask>
<ValidationMessage>Please provide a valid user and password, i.e. user:secret123.</ValidationMessage>
</users>
<customOptions type="TextField">
<Required>N</Required>
</customOptions>
</stats>
</general>
<frontends>
Expand All @@ -233,9 +239,9 @@
<Required>Y</Required>
<multiple>Y</multiple>
<!-- <default>localhost:8080</default> -->
<mask>/^((([0-9a-zA-Z._\-\*]+:[0-9]+)([,]){0,1}))*/u</mask>
<mask>/^((([0-9a-zA-Z._\-\*]+:[0-9]+(-[0-9]+)?)([,]){0,1}))*/u</mask>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very useful! Of course port ranges should be supported.

<ChangeCase>lower</ChangeCase>
<ValidationMessage>Please provide a valid listen address, i.e. 127.0.0.1:8080 or www.example.com:443.</ValidationMessage>
<ValidationMessage>Please provide a valid listen address, i.e. 127.0.0.1:8080 or www.example.com:443. Port range as start-end, i.e. 127.0.0.1:1220-1240.</ValidationMessage>
</bind>
<mode type="OptionField">
<Required>Y</Required>
Expand Down Expand Up @@ -476,6 +482,13 @@
<customOptions type="TextField">
<Required>N</Required>
</customOptions>
<tuning_defaultserver type="TextField">
<Required>N</Required>
</tuning_defaultserver>
<tuning_noport type="BooleanField">
<default>0</default>
<Required>Y</Required>
</tuning_noport>
<linkedActions type="ModelRelationField">
<Model>
<template>
Expand Down Expand Up @@ -526,6 +539,13 @@
<ValidationMessage>Please specify a value between 1 and 65535.</ValidationMessage>
<Required>Y</Required>
</port>
<checkport type="IntegerField">
<default>80</default>
<MinimumValue>1</MinimumValue>
<MaximumValue>65535</MaximumValue>
<ValidationMessage>Please specify a value between 1 and 65535.</ValidationMessage>
<Required>N</Required>
</checkport>
<mode type="OptionField">
<Required>Y</Required>
<default>active</default>
Expand Down
99 changes: 65 additions & 34 deletions net/haproxy/src/opnsense/service/templates/OPNsense/HAProxy/haproxy.conf
View file
Open in desktop
Expand Up @@ -441,7 +441,7 @@
{# ############################### #}

global
uid 80
#uid 80
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't do this. HAProxy should NOT run as root in the default configuration.
Please add a new Option "Run as root" to "General Settings", it should be an "advanced" option and the default value set to disabled.

gid 80
{% if OPNsense.HAProxy.general.tuning.chroot == "1" %}
# NOTE: chroot prevents (most) local logging, you need to enable remote
Expand Down Expand Up @@ -491,7 +491,12 @@ global
{% endif %}
{% endfor %}
{% endif %}

{% if OPNsense.HAProxy.general.tuning.customOptions|default("") != "" %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! :)

# WARNING: pass through options below this line
{% for customOpt in OPNsense.HAProxy.general.tuning.customOptions.split("\n") %}
{{customOpt}}
{% endfor %}
{% endif %}

{# ############################### #}
{# DEFAULTS #}
Expand All @@ -518,6 +523,12 @@ defaults
{% if OPNsense.HAProxy.general.defaults.retries|default("") != "" %}
retries {{OPNsense.HAProxy.general.defaults.retries}}
{% endif %}
{% if OPNsense.HAProxy.general.defaults.customOptions|default("") != "" %}
# WARNING: pass through options below this line
{% for customOpt in OPNsense.HAProxy.general.defaults.customOptions.split("\n") %}
{{customOpt}}
{% endfor %}
{% endif %}
{% endif %}

{# ############################### #}
Expand All @@ -526,30 +537,34 @@ defaults

{% if helpers.exists('OPNsense.HAProxy.frontends') %}
{% for frontend in helpers.toList('OPNsense.HAProxy.frontends.frontend') %}
{% if frontend.enabled=='1' %}
{% if frontend.enabled == '1' %}
# Frontend: {{frontend.name}} ({{frontend.description}})
frontend {{frontend.name}}
{# # collect ssl certs (if configured) #}
{% if frontend.ssl_certificates|default("") != "" %}
{% set ssl_certs = [] %}
{% for cert in frontend.ssl_certificates.split(",") %}
{% do ssl_certs.append('crt /var/etc/haproxy/ssl/' ~ cert ~ '.pem') %}
{% endfor %}
{% endif %}
{# # advanced ssl options #}
{% if frontend.ssl_customOptions|default("") != "" %}
{# # add a space to separate it from other ssl params #}
{% set ssl_options = frontend.ssl_customOptions ~ ' ' %}
{% if frontend.ssl_enabled == '1' %}
Copy link
Member

@fraenki fraenki Jun 9, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better fix would be to...

{# # collect ssl certs (if configured) #}
{% if frontend.ssl_certificates|default("") != "" %}
{% set ssl_certs = [] %}
Copy link
Member

@fraenki fraenki Jun 9, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...move this "set ssl_certs = []" outside of the "if frontend.ssl_enabled" clause to always reset it's value, even if SSL is not configured.
My fault, I should've noted this before. :(

{% for cert in frontend.ssl_certificates.split(",") %}
{% do ssl_certs.append('crt /var/etc/haproxy/ssl/' ~ cert ~ '.pem') %}
{% endfor %}
{% endif %}
{# # advanced ssl options #}
{% if frontend.ssl_customOptions|default("") != "" %}
{# # add a space to separate it from other ssl params #}
{% set ssl_options = frontend.ssl_customOptions ~ ' ' %}
{% endif %}
{% endif %}
{# # bind/listen configuration #}
{% if frontend.bind|default("") != "" %}
{% for bind in frontend.bind.split(",") %}
bind {{bind}} name {{bind}} {% if ssl_certs|default("") != "" %}ssl {{ ssl_options }}{{ssl_certs|join(' ')}}{% endif %}
bind {{bind}} name {{bind}} {% if frontend.ssl_enabled == '1' and ssl_certs|default("") != "" %}ssl {{ ssl_options }}{{ssl_certs|join(' ')}} {% endif %}
Copy link
Member

@fraenki fraenki Jun 9, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new check looks good, we should keep it.


{% endfor %}
{% endif %}
mode {{frontend.mode}}
{% if frontend.mode != "tcp" %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! This will prevent many configuration errors.

option {{frontend.connectionBehaviour}}
{% endif %}
{# # select backend #}
{% if frontend.defaultBackend|default("") != "" %}
{% set backend_data = helpers.getUUID(frontend.defaultBackend) %}
Expand Down Expand Up @@ -688,23 +703,6 @@ backend {{backend.name}}
# health checking is DISABLED
{% set healthcheck_enabled = '0' %}
{% endif %}
{% for server in backend.linkedServers.split(",") %}
{% set server_data = helpers.getUUID(server) %}
{# # collect optional server parameters #}
{% set server_options = [] %}
{# # check if health check is enabled #}
{% if healthcheck_enabled == '1' %}
{% do server_options.append('check') %}
{% do server_options.append('inter ' ~ server_data.checkInterval) %}
{# # add all additions from healthchecks here #}
{% do server_options.append(healthcheck_additions|join(' ')) if healthcheck_additions.length != '0' %}
{% endif %}
{# # server weight #}
{% do server_options.append('weight ' ~ server_data.weight) if server_data.weight|default("") != "" %}
{# # server role/mode #}
{% do server_options.append(server_data.mode) if server_data.mode|default("") != "active" %}
server {{server_data.name}} {{server_data.address}}:{{server_data.port}} {{server_options|join(' ')}}
{% endfor %}
{# # XXX: Usually the frontend and the backend are in the same mode, #}
{# # but we have no way to know what frontend uses this backend. #}
{# # Hence we can't automatically set the mode and thus need a #}
Expand Down Expand Up @@ -759,6 +757,27 @@ backend {{backend.name}}
{{customOpt}}
{% endfor %}
{% endif %}
{% if backend.tuning_defaultserver|default("") != "" %}
default-server {{backend.tuning_defaultserver}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useful addition.

{% endif %}

{% for server in backend.linkedServers.split(",") %}
{% set server_data = helpers.getUUID(server) %}
{# # collect optional server parameters #}
{% set server_options = [] %}
{# if# check if health check is enabled #}("") != "" %}
{% if healthcheck_enabled == '1' %}
{% do server_options.append('check') %}
{% do server_options.append('inter ' ~ server_data.checkInterval) %}
{# # add all additions from healthchecks here #}
{% do server_options.append(healthcheck_additions|join(' ')) if healthcheck_additions.length != '0' %}
{% endif %}
{# # server weight #}
{% do server_options.append('weight ' ~ server_data.weight) if server_data.weight|default("") != "" %}
{# # server role/mode #}
{% do server_options.append(server_data.mode) if server_data.mode|default("") != "active" %}
server {{server_data.name}} {{server_data.address}}:{% if backend.tuning_noport != '1' %}{{server_data.port}}{% endif %}{% if server_data.checkport|default("") != "" %} port {{server_data.checkport}}{% endif %} {{server_options|join(' ')}}
{% endfor %}

{% else %}
# Backend (DISABLED): {{backend.description}}
Expand All @@ -779,6 +798,12 @@ listen local_statistics
stats uri /haproxy?stats
stats realm HAProxy\ statistics
stats admin if TRUE
{% if OPNsense.HAProxy.general.stats.customOptions|default("") != "" %}
# WARNING: pass through options below this line
{% for customOpt in OPNsense.HAProxy.general.stats.customOptions.split("\n") %}
{{customOpt}}
{% endfor %}
{% endif %}

{# # remote stats are optional #}
{% if OPNsense.HAProxy.general.stats.remoteEnabled|default("") == "1" %}
Expand All @@ -799,9 +824,15 @@ listen remote_statistics
{% endfor %}
{% endif %}
{% endif %}
{% else %}
{% if OPNsense.HAProxy.general.stats.customOptions|default("") != "" %}
# WARNING: pass through options below this line
{% for customOpt in OPNsense.HAProxy.general.stats.customOptions.split("\n") %}
{{customOpt}}
{% endfor %}
{% endif %}
{% else %}
# ERROR: remote statistics disabled, because no listen address was specified
{% endif %}
{% endif %}
{% endif %}
{% else %}
# statistics are DISABLED
Expand Down