Merge lp:~cbjchen/charms/trusty/openstack-dashboard/enforce-ssl into lp:~openstack-charmers-archive/charms/trusty/openstack-dashboard/next

[Liang Chen]

Adding a enforce-ssl config option to enforce a redirection of port 80 to 443.

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #3079 openstack-dashboard-next for cbjchen mp255156
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/3079/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #2867 openstack-dashboard-next for cbjchen mp255156
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/2867/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #2897 openstack-dashboard-next for cbjchen mp255156
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/10757814/
Build: http://10.245.162.77:8080/job/charm_amulet_test/2897/

[Jonathan Davies (jpds)]

HTTPS is on 443 (and the number might not even be necessary with https://).

review: Needs Fixing

[Liang Chen (cbjchen)]

Jonathan,

Thanks a lot! 433 would skip haproxy and directly hit apache. I will re-submit the patch.

Thanks,
Liang

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #3110 openstack-dashboard-next for cbjchen mp255156
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/3110/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #2898 openstack-dashboard-next for cbjchen mp255156
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/2898/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #2943 openstack-dashboard-next for cbjchen mp255156
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/10767172/
Build: http://10.245.162.77:8080/job/charm_amulet_test/2943/

[James Page (james-page)]

Hi Liang

This generally looks OK - a couple of minor comments on the change below.

I had a merge conflict when I reviewed - please can you rebase again.

I'd also like to see some unit tests to exercise this configuration option before we land it.

review: Needs Fixing

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #3602 openstack-dashboard-next for cbjchen mp255156
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/3602/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #3815 openstack-dashboard-next for cbjchen mp255156
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/3815/

[Liang Chen (cbjchen)]

Hi James,

Thanks for the review. I rebased the patch and added unit test for the new config option. Please take a look. Thank you!

Thanks,
Liang

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #4236 openstack-dashboard-next for cbjchen mp255156
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/10965099/
Build: http://10.245.162.77:8080/job/charm_lint_check/4236/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #4252 openstack-dashboard-next for cbjchen mp255156
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/4252/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #3978 openstack-dashboard-next for cbjchen mp255156
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/3978/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #11086 openstack-dashboard-next for cbjchen mp255156
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11086/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #10293 openstack-dashboard-next for cbjchen mp255156
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10293/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6893 openstack-dashboard-next for cbjchen mp255156
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12625367/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6893/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6931 openstack-dashboard-next for cbjchen mp255156
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6931/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #7029 openstack-dashboard-next for cbjchen mp255156
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/7029/

[James Page (james-page)]

Hi

OpenStack Charm development is no longer hosted on launchpad; please read

  https://github.com/openstack-charmers/openstack-community/blob/master/README.dev-charms.md

and re-target your changes to the appropriate locations!

Thanks!

review: Disapprove

Unmerged revisions

90. By lchen <<email address hidden>@canonical.com> on 2015-09-30

  Add enfore-ssl config option

  Add an enforce-ssl config option to redirect 80
  port traffic to 433

  [cbjchen,r=]

=== modified file 'config.yaml'
--- config.yaml	2016-02-18 13:52:53 +0000
+++ config.yaml	2016-04-20 07:12:26 +0000
@@ -193,6 +193,7 @@
       wait for you to execute the openstack-upgrade action for this charm on
       each unit. If False it will revert to existing behavior of upgrading
       all units on config change.
+<<<<<<< TREE
   haproxy-server-timeout:
     type: int
     default:
@@ -217,3 +218,11 @@
     description: |
        Connect timeout configuration in ms for haproxy, used in HA
        configurations. If not provided, default value of 5000ms is used.
+=======
+  enforce-ssl:
+    type: boolean 
+    default: False
+    description: |
+      If True redirects plain http request to https port
+
+>>>>>>> MERGE-SOURCE

=== modified file 'hooks/horizon_contexts.py'
--- hooks/horizon_contexts.py	2016-02-18 15:13:57 +0000
+++ hooks/horizon_contexts.py	2016-04-20 07:12:26 +0000
@@ -14,6 +14,7 @@
     HAProxyContext,
     context_complete
 )
+from charmhelpers.contrib.openstack.utils import get_host_ip
 from charmhelpers.contrib.hahelpers.apache import (
     get_ca_cert,
     get_cert,
@@ -174,6 +175,16 @@
             'http_port': 70,
             'https_port': 433
         }
+        if config('enforce-ssl'):
+            if config('vip'):
+                addr = config('vip')
+            elif config('prefer-ipv6'):
+                addr = get_ipv6_addr()[0]
+            else:
+                addr = get_host_ip(unit_get('private-address'))
+            ctxt['addr'] = addr
+            ctxt['enforce_ssl'] = True
+
         return ctxt
 
 

=== modified file 'templates/default'
--- templates/default	2013-07-15 16:25:46 +0000
+++ templates/default	2016-04-20 07:12:26 +0000
@@ -1,4 +1,9 @@
 <VirtualHost *:{{ http_port }}>
+
+        {% if enforce_ssl %}
+                RedirectPermanent / https://{{ addr }}:443/
+        {% endif %}
+
         ServerAdmin webmaster@localhost
 
         DocumentRoot /var/www

=== modified file 'unit_tests/test_horizon_contexts.py'
--- unit_tests/test_horizon_contexts.py	2015-10-10 23:38:28 +0000
+++ unit_tests/test_horizon_contexts.py	2016-04-20 07:12:26 +0000
@@ -17,7 +17,8 @@
     'context_complete',
     'local_unit',
     'unit_get',
-    'pwgen'
+    'pwgen',
+    'get_host_ip'
 ]
 
 
@@ -50,6 +51,14 @@
         self.assertEquals(horizon_contexts.ApacheContext()(),
                           {'http_port': 70, 'https_port': 433})
 
+    def test_Apachecontext_enforce_ssl(self):
+        self.test_config.set('enforce-ssl', True)
+        self.get_host_ip.return_value = '10.0.0.1'
+        self.assertEqual(horizon_contexts.ApacheContext()(),
+                         {'http_port': 70, 'https_port': 433,
+                          'enforce_ssl': True,
+                          'addr': '10.0.0.1'})
+
     @patch.object(horizon_contexts, 'get_ca_cert', lambda: None)
     @patch('os.chmod')
     def test_ApacheSSLContext_enabled(self, _chmod):