Merge lp:~nick-moffitt/charms/trusty/squid-reverseproxy/bikeshedding into lp:charms/trusty/squid-reverseproxy

Proposed by Nick Moffitt
Status: Merged
Merged at revision: 52
Proposed branch: lp:~nick-moffitt/charms/trusty/squid-reverseproxy/bikeshedding
Merge into: lp:charms/trusty/squid-reverseproxy
Prerequisite: lp:~jacekn/charms/trusty/squid-reverseproxy/squid-reverseproxy-nrpe-fix
Diff against target: 261 lines (+56/-84)
2 files modified
hooks/hooks.py (+22/-50)
templates/main_config.template (+34/-34)
To merge this branch: bzr merge lp:~nick-moffitt/charms/trusty/squid-reverseproxy/bikeshedding
Reviewer Review Type Date Requested Status
Adam Israel (community) Approve
Review Queue (community) automated testing Needs Fixing
charmers Pending
Review via email: mp+247038@code.launchpad.net

Description of the change

This is almost entirely cosmetic clean-ups I performed while trying to work out the flow of data in this charm. It shouldn't do too much, but I think it will help maintenance if this codebase is to be preserved.

I started from jacekn's nrpe fix branch, so that's listed as a prereq.

To post a comment you must log in.
Revision history for this message
Review Queue (review-queue) wrote :

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-10961-results

review: Needs Fixing (automated testing)
Revision history for this message
Adam Israel (aisrael) wrote :

Hi Nick,

Thanks for your work on this charm! I've reviewed this MP and everything looks good to me. +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py 2015-01-20 16:13:22 +0000
+++ hooks/hooks.py 2015-01-20 16:13:22 +0000
@@ -146,37 +146,31 @@
146 dpkg.communicate(input=selections)146 dpkg.communicate(input=selections)
147147
148148
149###############################################################################
150# load_squid3_config: Convenience function that loads (as a string) the
151# current squid3 configuration file.
152# Returns a string containing the squid3 config or
153# None
154###############################################################################
155def load_squid3_config(squid3_config_file="/etc/squid3/squid.conf"):149def load_squid3_config(squid3_config_file="/etc/squid3/squid.conf"):
150 """Convenience function that loads (as a string) the current squid3
151 configuration file.
152
153 Returns a string containing the squid3 config or None"""
156 if os.path.isfile(squid3_config_file):154 if os.path.isfile(squid3_config_file):
157 return open(squid3_config_file).read()155 return open(squid3_config_file).read()
158 return156 return
159157
160158
161###############################################################################
162# get_service_ports: Convenience function that scans the existing squid3
163# configuration file and returns a list of the existing
164# ports being used. This is necessary to know which ports
165# to open and close when exposing/unexposing a service
166###############################################################################
167def get_service_ports(squid3_config_file="/etc/squid3/squid.conf"):159def get_service_ports(squid3_config_file="/etc/squid3/squid.conf"):
160 """Convenience function that scans the existing squid3 configuration
161 file and returns a list of the existing ports being used.
162
163 This is necessary to know which ports to open and close when
164 exposing/unexposing a service"""
168 squid3_config = load_squid3_config(squid3_config_file)165 squid3_config = load_squid3_config(squid3_config_file)
169 if squid3_config is None:166 if squid3_config is None:
170 return167 return
171 return re.findall("http_port ([0-9]+)", squid3_config)168 return re.findall("http_port ([0-9]+)", squid3_config)
172169
173170
174###############################################################################
175# get_sitenames: Convenience function that scans the existing squid3
176# configuration file and returns a list of the sitenames
177# being served by the instance.
178###############################################################################
179def get_sitenames(squid3_config_file="/etc/squid3/squid.conf"):171def get_sitenames(squid3_config_file="/etc/squid3/squid.conf"):
172 """Convenience function that scans the existing squid3 configuration
173 file and returns a list of the sitenames being served by the instance."""
180 squid3_config = load_squid3_config(squid3_config_file)174 squid3_config = load_squid3_config(squid3_config_file)
181 if squid3_config is None:175 if squid3_config is None:
182 return176 return
@@ -184,12 +178,9 @@
184 return list(set(sitenames))178 return list(set(sitenames))
185179
186180
187###############################################################################
188# update_service_ports: Convenience function that evaluate the old and new
189# service ports to decide which ports need to be
190# opened and which to close
191###############################################################################
192def update_service_ports(old_service_ports=None, new_service_ports=None):181def update_service_ports(old_service_ports=None, new_service_ports=None):
182 """Convenience function that evaluate the old and new service ports to
183 decide which ports need to be opened and which to close"""
193 if old_service_ports is None or new_service_ports is None:184 if old_service_ports is None or new_service_ports is None:
194 return185 return
195 for port in old_service_ports:186 for port in old_service_ports:
@@ -200,12 +191,10 @@
200 open_port(port)191 open_port(port)
201192
202193
203###############################################################################
204# pwgen: Generates a random password
205# pwd_length: Defines the length of the password to generate
206# default: 20
207###############################################################################
208def pwgen(pwd_length=20):194def pwgen(pwd_length=20):
195 """Generates a random password
196 pwd_length: Defines the length of the password to generate
197 default: 20"""
209 alphanumeric_chars = [l for l in (string.letters + string.digits)198 alphanumeric_chars = [l for l in (string.letters + string.digits)
210 if l not in 'Iil0oO1']199 if l not in 'Iil0oO1']
211 random_chars = [random.choice(alphanumeric_chars)200 random_chars = [random.choice(alphanumeric_chars)
@@ -222,10 +211,8 @@
222 return template.render(vars)211 return template.render(vars)
223212
224213
225###############################################################################
226# construct_squid3_config: Convenience function to write squid.conf
227###############################################################################
228def construct_squid3_config():214def construct_squid3_config():
215 """Convenience function to write squid.conf"""
229 config_data = config_get()216 config_data = config_get()
230 reverse_sites = get_reverse_sites()217 reverse_sites = get_reverse_sites()
231 only_direct = set()218 only_direct = set()
@@ -355,34 +342,19 @@
355 }))342 }))
356343
357344
358###############################################################################
359# service_squid3: Convenience function to start/stop/restart/reload
360# the squid3 service
361###############################################################################
362def service_squid3(action=None, squid3_config=default_squid3_config):345def service_squid3(action=None, squid3_config=default_squid3_config):
346 """Convenience function to start/stop/restart/reload the squid3
347 service"""
363 if action is None or squid3_config is None:348 if action is None or squid3_config is None:
364 return349 return
365 elif action == "check":350 elif action == "check":
366 check_cmd = ['/usr/sbin/squid3', '-f', squid3_config, '-k', 'parse']351 check_cmd = ['/usr/sbin/squid3', '-f', squid3_config, '-k', 'parse']
367 retVal = subprocess.call(check_cmd)352 return not subprocess.call(check_cmd)
368 if retVal == 1:
369 return False
370 elif retVal == 0:
371 return True
372 else:
373 return False
374 elif action == 'status':353 elif action == 'status':
375 status = subprocess.check_output(['status', 'squid3'])354 status = subprocess.check_output(['status', 'squid3'])
376 if re.search('running', status) is not None:355 return re.search('running', status) is not None
377 return True
378 else:
379 return False
380 elif action in ('start', 'stop', 'reload', 'restart'):356 elif action in ('start', 'stop', 'reload', 'restart'):
381 retVal = subprocess.call([action, 'squid3'])357 return not subprocess.call([action, 'squid3'])
382 if retVal == 0:
383 return True
384 else:
385 return False
386358
387359
388def install_nrpe_scripts():360def install_nrpe_scripts():
389361
=== modified file 'templates/main_config.template'
--- templates/main_config.template 2014-05-02 15:23:33 +0000
+++ templates/main_config.template 2015-01-20 16:13:22 +0000
@@ -15,12 +15,12 @@
1515
16{% if config.snmp_community -%}16{% if config.snmp_community -%}
17acl snmp_access snmp_community {{ config.snmp_community }}17acl snmp_access snmp_community {{ config.snmp_community }}
18{% for source in config.snmp_allowed_ips -%}18 {% for source in config.snmp_allowed_ips -%}
19acl snmp_source src {{ source }}19acl snmp_source src {{ source }}
20{% if loop.last -%}20 {% if loop.last -%}
21snmp_access allow snmp_access snmp_source21snmp_access allow snmp_access snmp_source
22{% endif -%}22 {% endif -%}
23{% endfor -%}23 {% endfor -%}
24snmp_access allow localhost24snmp_access allow localhost
25snmp_access deny all25snmp_access deny all
26snmp_port {{ config.snmp_port }}26snmp_port {{ config.snmp_port }}
@@ -60,52 +60,52 @@
6060
61# known services61# known services
62{% if sites -%}62{% if sites -%}
63{% for sitename in sites.keys() -%}63 {% for sitename in sites.keys() -%}
64{% if sitename -%}64 {% if sitename -%}
65{% set site_acl = "s_%s_acl" % loop.index %}65 {% set site_acl = "s_%s_acl" % loop.index %}
66acl {{ site_acl }} dstdomain {{ sitename }}66acl {{ site_acl }} dstdomain {{ sitename }}
67http_access allow accel_ports {{ site_acl }}67http_access allow accel_ports {{ site_acl }}
68http_access allow CONNECT SSL_ports {{ site_acl }}68http_access allow CONNECT SSL_ports {{ site_acl }}
69{% if sitename in only_direct -%}69 {% if sitename in only_direct -%}
70always_direct allow {{ site_acl }}70always_direct allow {{ site_acl }}
71{% else -%}71 {% else -%}
72always_direct allow CONNECT SSL_ports {{ site_acl }}72always_direct allow CONNECT SSL_ports {{ site_acl }}
73always_direct deny {{ site_acl }}73always_direct deny {{ site_acl }}
74{% endif -%}74 {% endif -%}
75{% if config['x_balancer_name_allowed'] -%}75 {% if config['x_balancer_name_allowed'] -%}
76{% set balancer_acl = "s_%s_balancer" % loop.index %}76 {% set balancer_acl = "s_%s_balancer" % loop.index %}
77acl {{ balancer_acl }} req_header X-Balancer-Name {{ sitename.replace('.', '\.') }}77acl {{ balancer_acl }} req_header X-Balancer-Name {{ sitename.replace('.', '\.') }}
78http_access allow accel_ports {{ balancer_acl }}78http_access allow accel_ports {{ balancer_acl }}
79http_access allow CONNECT SSL_ports {{ balancer_acl }}79http_access allow CONNECT SSL_ports {{ balancer_acl }}
80{% if sitename in only_direct -%}80 {% if sitename in only_direct -%}
81always_direct allow {{ balancer_acl }}81always_direct allow {{ balancer_acl }}
82{% else -%}82 {% else -%}
83always_direct allow CONNECT SSL_ports {{ balancer_acl }}83always_direct allow CONNECT SSL_ports {{ balancer_acl }}
84always_direct deny {{ balancer_acl }}84always_direct deny {{ balancer_acl }}
85{% endif -%}85 {% endif -%}
86{% endif -%}86 {% endif -%}
87{% else %} # no sitename87 {% else %} # no sitename
88acl no_sitename_acl myport {{ config.port }}88acl no_sitename_acl myport {{ config.port }}
89{% if config.enable_https -%}89 {% if config.enable_https -%}
90acl no_sitename_acl myport {{ config.https_port }}90acl no_sitename_acl myport {{ config.https_port }}
91{% endif -%}91 {% endif -%}
92http_access allow accel_ports no_sitename_acl92http_access allow accel_ports no_sitename_acl
93never_direct allow no_sitename_acl93never_direct allow no_sitename_acl
94{% endif %}94 {% endif %}
95{% endfor -%}95 {% endfor -%}
96{% endif -%}96{% endif -%}
9797
98{% if config.enable_forward_proxy -%}98{% if config.enable_forward_proxy -%}
99# no access retrictions99# no access retrictions
100{% for relation in forward_relations -%}100 {% for relation in forward_relations -%}
101{# acl names are limited to 31 chars (!), so using short "fwd_" prefix #}101{# acl names are limited to 31 chars (!), so using short "fwd_" prefix #}
102{% set forward_acl = "fwd_%s" % relation['name'] -%}102 {% set forward_acl = "fwd_%s" % relation['name'] -%}
103acl {{ forward_acl }} src {{ relation['private-address'] }}103acl {{ forward_acl }} src {{ relation['private-address'] }}
104http_access allow {{ forward_acl }}104http_access allow {{ forward_acl }}
105http_access allow CONNECT SSL_ports {{ forward_acl }}105http_access allow CONNECT SSL_ports {{ forward_acl }}
106always_direct allow {{ forward_acl }}106always_direct allow {{ forward_acl }}
107always_direct allow CONNECT SSL_ports {{ forward_acl }}107always_direct allow CONNECT SSL_ports {{ forward_acl }}
108{% endfor -%}108 {% endfor -%}
109{% endif -%}109{% endif -%}
110110
111http_access allow manager localhost111http_access allow manager localhost
@@ -120,21 +120,21 @@
120always_direct deny all120always_direct deny all
121121
122{% if sites -%}122{% if sites -%}
123{% for sitename in sites.keys() -%}123 {% for sitename in sites.keys() -%}
124{% set sites_loop = loop -%}124 {% set sites_loop = loop -%}
125{% for peer in sites[sitename] %}125 {% for peer in sites[sitename] %}
126{% if sitename -%}126 {% if sitename -%}
127cache_peer {{ peer.address }} parent {{ peer.port }} 0 name={{ peer.name }} no-query no-digest originserver round-robin login=PASS {{ peer.options }}127cache_peer {{ peer.address }} parent {{ peer.port }} 0 name={{ peer.name }} no-query no-digest originserver round-robin login=PASS {{ peer.options }}
128cache_peer_access {{ peer.name }} allow s_{{ sites_loop.index }}_acl128cache_peer_access {{ peer.name }} allow s_{{ sites_loop.index }}_acl
129{% if config['x_balancer_name_allowed'] -%}129 {% if config['x_balancer_name_allowed'] -%}
130cache_peer_access {{ peer.name }} allow s_{{ sites_loop.index }}_balancer130cache_peer_access {{ peer.name }} allow s_{{ sites_loop.index }}_balancer
131{% endif -%}131 {% endif -%}
132cache_peer_access {{ peer.name }} deny all132cache_peer_access {{ peer.name }} deny all
133{% else %}133 {% else %}
134cache_peer {{ peer.address }} parent {{ peer.port }} 0 name={{ peer.name }} no-query no-digest originserver round-robin login=PASS134cache_peer {{ peer.address }} parent {{ peer.port }} 0 name={{ peer.name }} no-query no-digest originserver round-robin login=PASS
135cache_peer_access {{ peer.name }} allow no_sitename_acl135cache_peer_access {{ peer.name }} allow no_sitename_acl
136cache_peer_access {{ peer.name }} deny all136cache_peer_access {{ peer.name }} deny all
137{% endif -%}137 {% endif -%}
138{% endfor -%}138 {% endfor -%}
139{% endfor -%}139 {% endfor -%}
140{% endif -%}140{% endif -%}

Subscribers

People subscribed via source and target branches

to all changes: