Landscape Charm

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

  1. openid-configuration
  2. Merge into trunk
Proposed by Данило Шеган
Status: Merged
Approved by: Данило Шеган
Approved revision: 241
Merged at revision: 239
Proposed branch: lp:~danilo/landscape-charm/openid-configuration
Merge into: lp:~landscape/landscape-charm/trunk
Diff against target: 245 lines (+96/-9)
9 files modified
config.yaml (+10/-0)
hooks/lib/relations/landscape.py (+2/-0)
hooks/lib/relations/tests/test_landscape.py (+6/-4)
hooks/lib/services.py (+1/-0)
hooks/lib/tests/sample.py (+5/-0)
hooks/lib/tests/test_services.py (+33/-1)
hooks/lib/tests/test_templates.py (+31/-2)
templates/service.conf (+6/-0)
tests/01-begin.py (+2/-2)
To merge this branch: bzr merge lp:~danilo/landscape-charm/openid-configuration
Reviewer Review Type Date Requested Status
Benji York (community) Approve
Free Ekanayaka (community) Approve
🤖 Landscape Builder test results Approve
Review via email: mp+254077@code.launchpad.net

Commit message

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.

Description of the change

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

To post a comment you must log in.
Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

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

review: Needs Information
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :
Download full text (4.0 KiB)

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...

Read more...

review: Needs Fixing (test results)
Revision history for this message
Данило Шеган (danilo) wrote :

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

Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

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)
Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

Thanks Danilo, looks good to me. +1

review: Approve
Revision history for this message
Benji York (benji) wrote :

This branch looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
14 description: License file for LDS.14 description: License file for LDS.
15 type: string15 type: string
16 default:16 default:
17 openid-provider-url:
18 default:
19 type: string
20 description: |
21 OpenID provider URL to use for authentication to Landscape.
22 openid-logout-url:
23 default:
24 type: string
25 description: |
26 OpenID provider URL to use to log out of Landscape.
17 admin-name:27 admin-name:
18 description: |28 description: |
19 Name of the account administrator. If this, admin-email and29 Name of the account administrator. If this, admin-email and
2030
=== 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 @@
18 interface = "landscape-ha"18 interface = "landscape-ha"
19 required_keys = [19 required_keys = [
20 "database-password", # Password for the 'landscape' database user.20 "database-password", # Password for the 'landscape' database user.
21 "secret-token", # Landscape-wide secret token.
21 ]22 ]
2223
23 def __init__(self, leader_context):24 def __init__(self, leader_context):
@@ -39,6 +40,7 @@
39 interface = "landscape-ha"40 interface = "landscape-ha"
40 required_keys = [41 required_keys = [
41 "database-password", # Password for the 'landscape' database user.42 "database-password", # Password for the 'landscape' database user.
43 "secret-token", # Landscape-wide secret token.
42 ]44 ]
4345
44 def __init__(self, leader_context):46 def __init__(self, leader_context):
4547
=== 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 @@
19 be set on the cluster relation in order for the relation to be19 be set on the cluster relation in order for the relation to be
20 considered ready.20 considered ready.
21 """21 """
22 self.assertEqual(22 self.assertItemsEqual(
23 ["database-password"], LandscapeRequirer.required_keys)23 ["database-password", "secret-token"],
24 LandscapeRequirer.required_keys)
2425
25 def test_is_leader(self):26 def test_is_leader(self):
26 """27 """
@@ -85,8 +86,9 @@
85 The L{LandscapeProvider} class defines all keys that are required to86 The L{LandscapeProvider} class defines all keys that are required to
86 be set before we actually modify the relation.87 be set before we actually modify the relation.
87 """88 """
88 self.assertEqual(89 self.assertItemsEqual(
89 ["database-password"], LandscapeRequirer.required_keys)90 ["database-password", "secret-token"],
91 LandscapeRequirer.required_keys)
9092
91 def test_provide_data(self):93 def test_provide_data(self):
92 """94 """
9395
=== 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 @@
49 LandscapeRequirer(leader_context),49 LandscapeRequirer(leader_context),
50 PostgreSQLRequirer(),50 PostgreSQLRequirer(),
51 RabbitMQRequirer(),51 RabbitMQRequirer(),
52 {"config": hookenv.config()},
52 ],53 ],
53 "data_ready": [54 "data_ready": [
54 render_template(55 render_template(
5556
=== 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 @@
17 "hostname": "10.0.3.170",17 "hostname": "10.0.3.170",
18 "password": "guessme",18 "password": "guessme",
19}19}
20
21SAMPLE_CONFIG_OPENID_DATA = {
22 "openid-provider-url": "http://openid-host/",
23 "openid-logout-url": "http://openid-host/logout",
24}
2025
=== 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 @@
3from lib.tests.helpers import HookenvTest3from lib.tests.helpers import HookenvTest
4from lib.tests.stubs import ClusterStub, HostStub, SubprocessStub4from lib.tests.stubs import ClusterStub, HostStub, SubprocessStub
5from lib.tests.sample import (5from lib.tests.sample import (
6 SAMPLE_DB_UNIT_DATA, SAMPLE_LEADER_CONTEXT_DATA, SAMPLE_AMQP_UNIT_DATA)6 SAMPLE_DB_UNIT_DATA, SAMPLE_LEADER_CONTEXT_DATA, SAMPLE_AMQP_UNIT_DATA,
7 SAMPLE_CONFIG_OPENID_DATA)
7from lib.services import ServicesHook, SERVICE_CONF, DEFAULT_FILE8from lib.services import ServicesHook, SERVICE_CONF, DEFAULT_FILE
89
910
@@ -91,6 +92,7 @@
91 "db": [SAMPLE_DB_UNIT_DATA],92 "db": [SAMPLE_DB_UNIT_DATA],
92 "leader": SAMPLE_LEADER_CONTEXT_DATA,93 "leader": SAMPLE_LEADER_CONTEXT_DATA,
93 "amqp": [SAMPLE_AMQP_UNIT_DATA],94 "amqp": [SAMPLE_AMQP_UNIT_DATA],
95 "config": {},
94 }96 }
9597
96 self.assertEqual(98 self.assertEqual(
@@ -105,6 +107,36 @@
105 self.assertEqual(107 self.assertEqual(
106 ["/usr/bin/lsctl", "restart"], call2[0])108 ["/usr/bin/lsctl", "restart"], call2[0])
107109
110 def test_ready_with_openid_configuration(self):
111 """
112 OpenID configuration is passed in to service.conf generation if
113 it is set in the hook configuration.
114 """
115 self.hookenv.relations = {
116 "db": {
117 "db:1": {
118 "postgresql/0": SAMPLE_DB_UNIT_DATA,
119 },
120 },
121 "amqp": {
122 "amqp:1": {
123 "rabbitmq-server/0": SAMPLE_AMQP_UNIT_DATA,
124 },
125 },
126 }
127 self.hookenv.config().update(SAMPLE_CONFIG_OPENID_DATA)
128 self.hook()
129 context = {
130 "db": [SAMPLE_DB_UNIT_DATA],
131 "leader": SAMPLE_LEADER_CONTEXT_DATA,
132 "amqp": [SAMPLE_AMQP_UNIT_DATA],
133 "config": SAMPLE_CONFIG_OPENID_DATA,
134 }
135
136 self.assertEqual(
137 ("service.conf", SERVICE_CONF, context, "landscape", "root", 416),
138 self.renders[0])
139
108 def test_remote_leader_not_ready(self):140 def test_remote_leader_not_ready(self):
109 """141 """
110 If we're not the leader unit and we didn't yet get relation data from142 If we're not the leader unit and we didn't yet get relation data from
111143
=== 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 @@
1111
12 template_filename = "service.conf"12 template_filename = "service.conf"
1313
14 def test_render_stores(self):14 def test_render(self):
15 """15 """
16 The service.conf template renders data about PostgreSQL configuration.16 The service.conf template renders data about generic Landscape
17 configuration which includes PostgreSQL configuration, AMQP
18 configuration, secret token and no OpenID settings by default.
17 """19 """
18 context = {20 context = {
19 "db": [SAMPLE_DB_UNIT_DATA],21 "db": [SAMPLE_DB_UNIT_DATA],
20 "amqp": [SAMPLE_AMQP_UNIT_DATA],22 "amqp": [SAMPLE_AMQP_UNIT_DATA],
21 "leader": SAMPLE_LEADER_CONTEXT_DATA,23 "leader": SAMPLE_LEADER_CONTEXT_DATA,
24 "config": {},
22 }25 }
23 buffer = StringIO(self.template.render(context))26 buffer = StringIO(self.template.render(context))
24 config = ConfigParser()27 config = ConfigParser()
@@ -31,3 +34,29 @@
31 self.assertEqual("guessme", config.get("broker", "password"))34 self.assertEqual("guessme", config.get("broker", "password"))
32 self.assertEqual(35 self.assertEqual(
33 "landscape-token", config.get("landscape", "secret-token"))36 "landscape-token", config.get("landscape", "secret-token"))
37 self.assertFalse(config.has_option("landscape", "openid-provider-url"))
38 self.assertFalse(config.has_option("landscape", "openid-logout-url"))
39
40 def test_render_with_openid(self):
41 """
42 When OpenID configuration is present in the leader context,
43 openid-related options are set.
44 """
45 context = {
46 "db": [SAMPLE_DB_UNIT_DATA],
47 "amqp": [SAMPLE_AMQP_UNIT_DATA],
48 "leader": SAMPLE_LEADER_CONTEXT_DATA,
49 "config": {
50 "openid-provider-url": "http://openid-host/",
51 "openid-logout-url": "http://openid-host/logout",
52 },
53 }
54 buffer = StringIO(self.template.render(context))
55 config = ConfigParser()
56 config.readfp(buffer)
57 self.assertEqual(
58 "http://openid-host/",
59 config.get("landscape", "openid-provider-url"))
60 self.assertEqual(
61 "http://openid-host/logout",
62 config.get("landscape", "openid-logout-url"))
3463
=== 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 @@
38reprepro-binary = /opt/canonical/landscape/scripts/reprepro-wrapper.sh38reprepro-binary = /opt/canonical/landscape/scripts/reprepro-wrapper.sh
39repository-path = /var/lib/landscape/landscape-repository39repository-path = /var/lib/landscape/landscape-repository
40secret-token = {{ leader["secret-token"] }}40secret-token = {{ leader["secret-token"] }}
41{% if config["openid-provider-url"] %}
42openid-provider-url = {{ config["openid-provider-url"] }}
43{% endif %}
44{% if config["openid-logout-url"] %}
45openid-logout-url = {{ config["openid-logout-url"] }}
46{% endif %}
4147
42[api]48[api]
43stores = main account-1 resource-1 package49stores = main account-1 resource-1 package
4450
=== 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 @@
83 frontend = find_address(juju_status(), "haproxy")83 frontend = find_address(juju_status(), "haproxy")
8484
85 # Make sure the app server is up.85 # Make sure the app server is up.
86 # Note: In order to work on a new server or a server with the 86 # Note: In order to work on a new server or a server with the
87 # first admin user already created, this phrase should match87 # first admin user already created, this phrase should match
88 # the new-standalone-user form, the login form, and not88 # the new-standalone-user form, the login form, and not
89 # the maintenance page.89 # the maintenance page.
@@ -229,7 +229,7 @@
229 Specifically that it is reachable and that it presents the new229 Specifically that it is reachable and that it presents the new
230 user form.230 user form.
231231
232 Note: In order to work on a new server or a server with the 232 Note: In order to work on a new server or a server with the
233 first admin user already created, this phrase should match233 first admin user already created, this phrase should match
234 the new-standalone-user form, the login form, and not234 the new-standalone-user form, the login form, and not
235 the maintenance page.235 the maintenance page.

Subscribers

People subscribed via source and target branches