Merge lp:~danilo/landscape-charm/openid-configuration into lp:~landscape/landscape-charm/trunk

[Данило Шеган]

Allow OpenID to be configured with the Landscape charm.

We introduce two options in the config.yaml that set appropriate openid-provider-url and openid-logout-url in service.conf [landscape] section (needed for our hosted deployments).
When they are not provided, OpenID is not configured.

Drive-by fix to make secret-token a required key as well in the landscape config.

Allow OpenID to be configured with the Landscape charm.

We introduce two options in the config.yaml that set appropriate openid-provider-url and openid-logout-url in service.conf [landscape] section (needed for our hosted deployments).
When they are not provided, OpenID is not configured.

Drive-by fix to make secret-token a required key as well in the landscape config.

To test, you can use juju-deployer with the following config:

  https://pastebin.canonical.com/128312/

Save it as landscape.yaml and run with

  juju-deployer -vdW -w180 -c landscape.yaml landscape

[Free Ekanayaka (free.ekanayaka)]

Why do we need to use the leader context? The config is the same for all units.

review: Needs Information

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Fail
Revno: 239
Branch: lp:~danilo/landscape-charm/openid-configuration
Jenkins: https://ci.lscape.net/job/latch-test/328/
-------------------------------------------
./dev/ubuntu-deps
Reading package lists...
Building dependency tree...
Reading state information...
software-properties-common is already the newest version.
The following package was automatically installed and is no longer required:
  os-prober
Use 'apt-get autoremove' to remove it.
0 upgraded, 0 newly installed, 0 to remove and 7 not upgraded.
gpg: keyring `/tmp/tmpvspjif3w/secring.gpg' created
gpg: keyring `/tmp/tmpvspjif3w/pubring.gpg' created
gpg: requesting key C8068B11 from hkp server keyserver.ubuntu.com
gpg: /tmp/tmpvspjif3w/trustdb.gpg: trustdb created
gpg: key C8068B11: public key "Launchpad Ensemble PPA" imported
gpg: Total number processed: 1
gpg: imported: 1 (RSA: 1)
OK
Ign http://security.ubuntu.com trusty-security InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Ign http://ppa.launchpad.net trusty InRelease
Ign http://archive.ubuntu.com trusty InRelease
Hit http://security.ubuntu.com trusty-security Release
Ign http://archive.ubuntu.com trusty-updates InRelease
Ign http://ppa.launchpad.net trusty InRelease
Hit http://archive.ubuntu.com trusty Release.gpg
Hit http://archive.ubuntu.com trusty-updates Release.gpg
Hit http://archive.ubuntu.com trusty Release
Hit http://ppa.launchpad.net trusty Release.gpg
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://archive.ubuntu.com trusty-updates Release
Hit http://ppa.launchpad.net trusty Release.gpg
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://archive.ubuntu.com trusty/main Sources
Get:1 http://security.ubuntu.com trusty-security/main Translation-en [126 kB]
Hit http://archive.ubuntu.com trusty/universe Sources
Hit http://archive.ubuntu.com trusty/main amd64 Packages
Hit http://archive.ubuntu.com trusty/universe amd64 Packages
Hit http://archive.ubuntu.com trusty/main Translation-en
Hit http://archive.ubuntu.com trusty/universe Translation-en
Hit http://archive.ubuntu.com trusty-updates/main Sources
Hit http://archive.ubuntu.com trusty-updates/universe Sources
Hit http://archive.ubuntu.com trusty-updates/main amd64 Packages
Hit http://archive.ubuntu.com trusty-updates/universe amd64 Packages
Hit http://archive.ubuntu.com trusty-updates/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://ppa.launchpad.net trusty Release
Hit http://ppa.launchpad.net trusty Release
Hit http://ppa.launchpad.net trusty/main amd64 Packages
Hit http://archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://archive.ubuntu.com trusty/main Translation-en_US
Ign http://archive.ubuntu.com trusty/universe Translation-en_US
Hit http://ppa.launchpad.net trusty/main Translation-en
Hit http://ppa.launchpad.net trusty/main amd64 Packages
Hit http://ppa.launchpad.net trusty/main Translation-en
Ign https://private-ppa.launchpad.net trusty InRelease...

review: Needs Fixing (test results)

[Данило Шеган (danilo)]

Ok, I've moved to passing in the entire config as part of required_data. Please re-review.

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 241
Branch: lp:~danilo/landscape-charm/openid-configuration
Jenkins: https://ci.lscape.net/job/latch-test/333/

review: Approve (test results)

[Free Ekanayaka (free.ekanayaka)]

Thanks Danilo, looks good to me. +1

review: Approve

[Benji York (benji)]

This branch looks good.

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2015-03-19 15:40:25 +0000
+++ config.yaml	2015-03-26 09:04:07 +0000
@@ -14,6 +14,16 @@
         description: License file for LDS.
         type: string
         default:
+    openid-provider-url:
+        default:
+        type: string
+        description: |
+            OpenID provider URL to use for authentication to Landscape.
+    openid-logout-url:
+        default:
+        type: string
+        description: |
+            OpenID provider URL to use to log out of Landscape.
     admin-name:
         description: |
             Name of the account administrator. If this, admin-email and

=== modified file 'hooks/lib/relations/landscape.py'
--- hooks/lib/relations/landscape.py	2015-03-11 13:33:16 +0000
+++ hooks/lib/relations/landscape.py	2015-03-26 09:04:07 +0000
@@ -18,6 +18,7 @@
     interface = "landscape-ha"
     required_keys = [
         "database-password",  # Password for the 'landscape' database user.
+        "secret-token",       # Landscape-wide secret token.
     ]
 
     def __init__(self, leader_context):
@@ -39,6 +40,7 @@
     interface = "landscape-ha"
     required_keys = [
         "database-password",  # Password for the 'landscape' database user.
+        "secret-token",       # Landscape-wide secret token.
     ]
 
     def __init__(self, leader_context):

=== modified file 'hooks/lib/relations/tests/test_landscape.py'
--- hooks/lib/relations/tests/test_landscape.py	2015-03-11 10:00:30 +0000
+++ hooks/lib/relations/tests/test_landscape.py	2015-03-26 09:04:07 +0000
@@ -19,8 +19,9 @@
         be set on the cluster relation in order for the relation to be
         considered ready.
         """
-        self.assertEqual(
-            ["database-password"], LandscapeRequirer.required_keys)
+        self.assertItemsEqual(
+            ["database-password", "secret-token"],
+            LandscapeRequirer.required_keys)
 
     def test_is_leader(self):
         """
@@ -85,8 +86,9 @@
         The L{LandscapeProvider} class defines all keys that are required to
         be set before we actually modify the relation.
         """
-        self.assertEqual(
-            ["database-password"], LandscapeRequirer.required_keys)
+        self.assertItemsEqual(
+            ["database-password", "secret-token"],
+            LandscapeRequirer.required_keys)
 
     def test_provide_data(self):
         """

=== modified file 'hooks/lib/services.py'
--- hooks/lib/services.py	2015-03-02 12:39:25 +0000
+++ hooks/lib/services.py	2015-03-26 09:04:07 +0000
@@ -49,6 +49,7 @@
                 LandscapeRequirer(leader_context),
                 PostgreSQLRequirer(),
                 RabbitMQRequirer(),
+                {"config": hookenv.config()},
             ],
             "data_ready": [
                 render_template(

=== modified file 'hooks/lib/tests/sample.py'
--- hooks/lib/tests/sample.py	2015-03-10 12:04:11 +0000
+++ hooks/lib/tests/sample.py	2015-03-26 09:04:07 +0000
@@ -17,3 +17,8 @@
     "hostname": "10.0.3.170",
     "password": "guessme",
 }
+
+SAMPLE_CONFIG_OPENID_DATA = {
+    "openid-provider-url": "http://openid-host/",
+    "openid-logout-url": "http://openid-host/logout",
+}

=== modified file 'hooks/lib/tests/test_services.py'
--- hooks/lib/tests/test_services.py	2015-03-11 13:33:16 +0000
+++ hooks/lib/tests/test_services.py	2015-03-26 09:04:07 +0000
@@ -3,7 +3,8 @@
 from lib.tests.helpers import HookenvTest
 from lib.tests.stubs import ClusterStub, HostStub, SubprocessStub
 from lib.tests.sample import (
-    SAMPLE_DB_UNIT_DATA, SAMPLE_LEADER_CONTEXT_DATA, SAMPLE_AMQP_UNIT_DATA)
+    SAMPLE_DB_UNIT_DATA, SAMPLE_LEADER_CONTEXT_DATA, SAMPLE_AMQP_UNIT_DATA,
+    SAMPLE_CONFIG_OPENID_DATA)
 from lib.services import ServicesHook, SERVICE_CONF, DEFAULT_FILE
 
 
@@ -91,6 +92,7 @@
             "db": [SAMPLE_DB_UNIT_DATA],
             "leader": SAMPLE_LEADER_CONTEXT_DATA,
             "amqp": [SAMPLE_AMQP_UNIT_DATA],
+            "config": {},
         }
 
         self.assertEqual(
@@ -105,6 +107,36 @@
         self.assertEqual(
             ["/usr/bin/lsctl", "restart"], call2[0])
 
+    def test_ready_with_openid_configuration(self):
+        """
+        OpenID configuration is passed in to service.conf generation if
+        it is set in the hook configuration.
+        """
+        self.hookenv.relations = {
+            "db": {
+                "db:1": {
+                    "postgresql/0": SAMPLE_DB_UNIT_DATA,
+                },
+            },
+            "amqp": {
+                "amqp:1": {
+                    "rabbitmq-server/0": SAMPLE_AMQP_UNIT_DATA,
+                },
+            },
+        }
+        self.hookenv.config().update(SAMPLE_CONFIG_OPENID_DATA)
+        self.hook()
+        context = {
+            "db": [SAMPLE_DB_UNIT_DATA],
+            "leader": SAMPLE_LEADER_CONTEXT_DATA,
+            "amqp": [SAMPLE_AMQP_UNIT_DATA],
+            "config": SAMPLE_CONFIG_OPENID_DATA,
+        }
+
+        self.assertEqual(
+            ("service.conf", SERVICE_CONF, context, "landscape", "root", 416),
+            self.renders[0])
+
     def test_remote_leader_not_ready(self):
         """
         If we're not the leader unit and we didn't yet get relation data from

=== modified file 'hooks/lib/tests/test_templates.py'
--- hooks/lib/tests/test_templates.py	2015-03-12 09:44:58 +0000
+++ hooks/lib/tests/test_templates.py	2015-03-26 09:04:07 +0000
@@ -11,14 +11,17 @@
 
     template_filename = "service.conf"
 
-    def test_render_stores(self):
+    def test_render(self):
         """
-        The service.conf template renders data about PostgreSQL configuration.
+        The service.conf template renders data about generic Landscape
+        configuration which includes PostgreSQL configuration, AMQP
+        configuration, secret token and no OpenID settings by default.
         """
         context = {
             "db": [SAMPLE_DB_UNIT_DATA],
             "amqp": [SAMPLE_AMQP_UNIT_DATA],
             "leader": SAMPLE_LEADER_CONTEXT_DATA,
+            "config": {},
         }
         buffer = StringIO(self.template.render(context))
         config = ConfigParser()
@@ -31,3 +34,29 @@
         self.assertEqual("guessme", config.get("broker", "password"))
         self.assertEqual(
             "landscape-token", config.get("landscape", "secret-token"))
+        self.assertFalse(config.has_option("landscape", "openid-provider-url"))
+        self.assertFalse(config.has_option("landscape", "openid-logout-url"))
+
+    def test_render_with_openid(self):
+        """
+        When OpenID configuration is present in the leader context,
+        openid-related options are set.
+        """
+        context = {
+            "db": [SAMPLE_DB_UNIT_DATA],
+            "amqp": [SAMPLE_AMQP_UNIT_DATA],
+            "leader": SAMPLE_LEADER_CONTEXT_DATA,
+            "config": {
+                "openid-provider-url": "http://openid-host/",
+                "openid-logout-url": "http://openid-host/logout",
+            },
+        }
+        buffer = StringIO(self.template.render(context))
+        config = ConfigParser()
+        config.readfp(buffer)
+        self.assertEqual(
+            "http://openid-host/",
+            config.get("landscape", "openid-provider-url"))
+        self.assertEqual(
+            "http://openid-host/logout",
+            config.get("landscape", "openid-logout-url"))

=== modified file 'templates/service.conf'
--- templates/service.conf	2015-03-12 09:44:58 +0000
+++ templates/service.conf	2015-03-26 09:04:07 +0000
@@ -38,6 +38,12 @@
 reprepro-binary = /opt/canonical/landscape/scripts/reprepro-wrapper.sh
 repository-path = /var/lib/landscape/landscape-repository
 secret-token = {{ leader["secret-token"] }}
+{% if config["openid-provider-url"] %}
+openid-provider-url = {{ config["openid-provider-url"] }}
+{% endif %}
+{% if config["openid-logout-url"] %}
+openid-logout-url = {{ config["openid-logout-url"] }}
+{% endif %}
 
 [api]
 stores = main account-1 resource-1 package

=== modified file 'tests/01-begin.py'
--- tests/01-begin.py	2015-03-19 20:14:18 +0000
+++ tests/01-begin.py	2015-03-26 09:04:07 +0000
@@ -83,7 +83,7 @@
     frontend = find_address(juju_status(), "haproxy")
 
     # Make sure the app server is up.
-    # Note: In order to work on a new server or a server with the 
+    # Note: In order to work on a new server or a server with the
     #       first admin user already created, this phrase should match
     #       the new-standalone-user form, the login form, and not
     #       the maintenance page.
@@ -229,7 +229,7 @@
         Specifically that it is reachable and that it presents the new
         user form.
 
-        Note: In order to work on a new server or a server with the 
+        Note: In order to work on a new server or a server with the
           first admin user already created, this phrase should match
           the new-standalone-user form, the login form, and not
           the maintenance page.