charm-k8s-discourse

Merge ~mthaddon/charm-k8s-discourse/+git/charm-k8s-discourse:group-sync into charm-k8s-discourse:master

  1. Git
  2. lp:~mthaddon/charm-k8s-discourse/+git/charm-k8s-discourse
  3. group-sync
  4. Merge into master
Proposed by Tom Haddon
Status: Merged
Approved by: Jay Kuri
Approved revision: 4324b038a039dbc4574f0ede96cdc254147a8f4e
Merged at revision: e668060d9f7f0debb5df0915ef370fc4eb3e5a98
Proposed branch: ~mthaddon/charm-k8s-discourse/+git/charm-k8s-discourse:group-sync
Merge into: charm-k8s-discourse:master
Diff against target: 238 lines (+52/-3)
13 files modified
config.yaml (+5/-1)
src/charm.py (+14/-2)
tests/unit/fixtures/config_invalid_bad_throttle_mode.yaml (+3/-0)
tests/unit/fixtures/config_invalid_force_saml_no_saml_target.yaml (+5/-0)
tests/unit/fixtures/config_invalid_missing_cors.yaml (+3/-0)
tests/unit/fixtures/config_invalid_missing_db_name.yaml (+3/-0)
tests/unit/fixtures/config_valid_complete.yaml (+7/-0)
tests/unit/fixtures/config_valid_no_saml_fingerprint.yaml (+1/-0)
tests/unit/fixtures/config_valid_no_saml_target_url.yaml (+1/-0)
tests/unit/fixtures/config_valid_no_tls.yaml (+1/-0)
tests/unit/fixtures/config_valid_with_tls.yaml (+1/-0)
tests/unit/fixtures/config_valid_without_tls.yaml (+1/-0)
tests/unit/test_charm.py (+7/-0)
Reviewer Review Type Date Requested Status
🤖 prod-jenkaas-is (community) continuous-integration Approve
Canonical IS Reviewers Pending
Discourse Charm Maintainers Pending
Review via email: mp+405338@code.launchpad.net

Commit message

Add support for group sync via SAML provider

To post a comment you must log in.
Revision history for this message
🤖 prod-jenkaas-is (prod-jenkaas-is) wrote :

A CI job is currently in progress. A follow up comment will be added when it completes.

Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
🤖 prod-jenkaas-is (prod-jenkaas-is) wrote :

PASSED: Continuous integration, rev:4324b038a039dbc4574f0ede96cdc254147a8f4e
https://jenkins.canonical.com/is/job/lp-charm-k8s-discourse-ci/23/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/is/job/lp-charm-test/129/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/266959/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-discourse-ci/23//rebuild

review: Approve (continuous-integration)
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision e668060d9f7f0debb5df0915ef370fc4eb3e5a98

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/config.yaml b/config.yaml
2index 85f0677..931dc5f 100644
3--- a/config.yaml
4+++ b/config.yaml
5@@ -71,13 +71,17 @@ options:
6 type: string
7 description: "Throttle level - blocks excessive usage by ip. Valid values: none, permissive, strict"
8 default: none
9+ saml_sync_groups:
10+ type: string
11+ description: "Comma-separated list of groups to sync from SAML provider."
12+ default: ""
13 saml_target_url:
14 type: string
15 description: "SAML authentication target url"
16 default: ""
17 force_saml_login:
18 type: boolean
19- description: "Force saml login (full screen, no local database logins)"
20+ description: "Force SAML login (full screen, no local database logins)"
21 default: false
22 max_body_size:
23 type: int
24diff --git a/src/charm.py b/src/charm.py
25index 0ef9f01..4085867 100755
26--- a/src/charm.py
27+++ b/src/charm.py
28@@ -93,6 +93,15 @@ def get_saml_config(config):
29 if fingerprint:
30 saml_config['DISCOURSE_SAML_CERT_FINGERPRINT'] = fingerprint
31
32+ saml_sync_groups = [x.strip() for x in config['saml_sync_groups'].split(',') if x.strip()]
33+ if saml_sync_groups:
34+ # Per https://github.com/discourse/discourse-saml setting this to `true`
35+ # means the assigned groups will be completely synced including adding
36+ # AND removing groups based on the SAML provider.
37+ saml_config['DISCOURSE_SAML_GROUPS_FULLSYNC'] = "false"
38+ saml_config['DISCOURSE_SAML_SYNC_GROUPS'] = "true"
39+ saml_config['DISCOURSE_SAML_SYNC_GROUPS_LIST'] = "|".join(saml_sync_groups)
40+
41 return saml_config
42
43
44@@ -186,8 +195,11 @@ def check_for_config_problems(config, stored):
45 if not THROTTLE_LEVELS.get(config['throttle_level']):
46 errors.append('throttle_level must be one of: ' + ' '.join(THROTTLE_LEVELS.keys()))
47
48- if config['force_saml_login'] and config['saml_target_url'] == '':
49- errors.append('force_saml_login can not be true without a saml_target_url')
50+ if config['force_saml_login'] and not config['saml_target_url']:
51+ errors.append("'force_saml_login' cannot be true without a 'saml_target_url'")
52+
53+ if config['saml_sync_groups'] and not config['saml_target_url']:
54+ errors.append("'saml_sync_groups' cannot be specified without a 'saml_target_url'")
55
56 return errors
57
58diff --git a/tests/unit/fixtures/config_invalid_bad_throttle_mode.yaml b/tests/unit/fixtures/config_invalid_bad_throttle_mode.yaml
59index 46d7ee6..3e0ecf0 100644
60--- a/tests/unit/fixtures/config_invalid_bad_throttle_mode.yaml
61+++ b/tests/unit/fixtures/config_invalid_bad_throttle_mode.yaml
62@@ -19,7 +19,10 @@ config:
63 smtp_port: 587
64 smtp_username: apikey
65 tls_secret_name: discourse_local
66+ saml_sync_groups: ''
67 saml_target_url: https://login.ubuntu.com/+saml
68 force_saml_login: true
69 throttle_level: scream
70+config_problems:
71+ - 'throttle_level must be one of: none permissive strict'
72 missing_fields: []
73diff --git a/tests/unit/fixtures/config_invalid_force_saml_no_saml_target.yaml b/tests/unit/fixtures/config_invalid_force_saml_no_saml_target.yaml
74index 8be167e..86a5002 100644
75--- a/tests/unit/fixtures/config_invalid_force_saml_no_saml_target.yaml
76+++ b/tests/unit/fixtures/config_invalid_force_saml_no_saml_target.yaml
77@@ -19,7 +19,12 @@ config:
78 smtp_port: 587
79 smtp_username: apikey
80 tls_secret_name: discourse_local
81+ saml_sync_groups: 'canonical, ubuntu-core'
82 saml_target_url: ''
83 force_saml_login: true
84 throttle_level: scream
85+config_problems:
86+ - 'throttle_level must be one of: none permissive strict'
87+ - "'force_saml_login' cannot be true without a 'saml_target_url'"
88+ - "'saml_sync_groups' cannot be specified without a 'saml_target_url'"
89 missing_fields: []
90diff --git a/tests/unit/fixtures/config_invalid_missing_cors.yaml b/tests/unit/fixtures/config_invalid_missing_cors.yaml
91index d135c3f..85ece42 100644
92--- a/tests/unit/fixtures/config_invalid_missing_cors.yaml
93+++ b/tests/unit/fixtures/config_invalid_missing_cors.yaml
94@@ -17,8 +17,11 @@ config:
95 smtp_password: OBV10USLYF4K3
96 smtp_port: 587
97 smtp_username: apikey
98+ saml_sync_groups: ''
99 saml_target_url: https://login.ubuntu.com/+saml
100 force_saml_login: true
101 throttle_level: none
102+config_problems:
103+ - 'Required configuration missing: cors_origin'
104 missing_fields:
105 - 'cors_origin'
106diff --git a/tests/unit/fixtures/config_invalid_missing_db_name.yaml b/tests/unit/fixtures/config_invalid_missing_db_name.yaml
107index 865ae83..128c381 100644
108--- a/tests/unit/fixtures/config_invalid_missing_db_name.yaml
109+++ b/tests/unit/fixtures/config_invalid_missing_db_name.yaml
110@@ -15,8 +15,11 @@ config:
111 smtp_password: OBV10USLYF4K3
112 smtp_port: 587
113 smtp_username: apikey
114+ saml_sync_groups: ''
115 saml_target_url: https://login.ubuntu.com/+saml
116 force_saml_login: true
117 throttle_level: none
118+config_problems:
119+ - 'Required configuration missing: db_name'
120 missing_fields:
121 - 'db_name'
122diff --git a/tests/unit/fixtures/config_valid_complete.yaml b/tests/unit/fixtures/config_valid_complete.yaml
123index 946388d..800255e 100644
124--- a/tests/unit/fixtures/config_valid_complete.yaml
125+++ b/tests/unit/fixtures/config_valid_complete.yaml
126@@ -21,6 +21,7 @@ config:
127 smtp_port: 587
128 smtp_username: apikey
129 tls_secret_name: discourse_local
130+ saml_sync_groups: 'canonical, ubuntu-core'
131 saml_target_url: https://login.ubuntu.com/+saml
132 force_saml_login: true
133 throttle_level: none
134@@ -44,6 +45,9 @@ pod_config:
135 DISCOURSE_SMTP_USER_NAME: apikey
136 DISCOURSE_REFRESH_MAXMIND_DB_DURING_PRECOMPILE_DAYS: '0'
137 DISCOURSE_SERVE_STATIC_ASSETS: 'true'
138+ DISCOURSE_SAML_GROUPS_FULLSYNC: 'false'
139+ DISCOURSE_SAML_SYNC_GROUPS: 'true'
140+ DISCOURSE_SAML_SYNC_GROUPS_LIST: 'canonical|ubuntu-core'
141 DISCOURSE_SAML_TARGET_URL: https://login.ubuntu.com/+saml
142 DISCOURSE_SAML_FULL_SCREEN_LOGIN: "true"
143 DISCOURSE_SAML_CERT_FINGERPRINT: 32:15:20:9F:A4:3C:8E:3E:8E:47:72:62:9A:86:8D:0E:E6:CF:45:D5
144@@ -73,6 +77,9 @@ pod_spec:
145 DISCOURSE_SMTP_USER_NAME: apikey
146 DISCOURSE_REFRESH_MAXMIND_DB_DURING_PRECOMPILE_DAYS: '0'
147 DISCOURSE_SERVE_STATIC_ASSETS: 'true'
148+ DISCOURSE_SAML_GROUPS_FULLSYNC: 'false'
149+ DISCOURSE_SAML_SYNC_GROUPS: 'true'
150+ DISCOURSE_SAML_SYNC_GROUPS_LIST: 'canonical|ubuntu-core'
151 DISCOURSE_SAML_TARGET_URL: https://login.ubuntu.com/+saml
152 DISCOURSE_SAML_FULL_SCREEN_LOGIN: "true"
153 DISCOURSE_SAML_CERT_FINGERPRINT: 32:15:20:9F:A4:3C:8E:3E:8E:47:72:62:9A:86:8D:0E:E6:CF:45:D5
154diff --git a/tests/unit/fixtures/config_valid_no_saml_fingerprint.yaml b/tests/unit/fixtures/config_valid_no_saml_fingerprint.yaml
155index cc76606..c1887db 100644
156--- a/tests/unit/fixtures/config_valid_no_saml_fingerprint.yaml
157+++ b/tests/unit/fixtures/config_valid_no_saml_fingerprint.yaml
158@@ -20,6 +20,7 @@ config:
159 smtp_port: 587
160 smtp_username: apikey
161 tls_secret_name: discourse_local
162+ saml_sync_groups: ''
163 saml_target_url: https://login.launchpad.net/+saml
164 force_saml_login: true
165 throttle_level: none
166diff --git a/tests/unit/fixtures/config_valid_no_saml_target_url.yaml b/tests/unit/fixtures/config_valid_no_saml_target_url.yaml
167index 1364998..cbae7b7 100644
168--- a/tests/unit/fixtures/config_valid_no_saml_target_url.yaml
169+++ b/tests/unit/fixtures/config_valid_no_saml_target_url.yaml
170@@ -20,6 +20,7 @@ config:
171 smtp_port: 587
172 smtp_username: apikey
173 tls_secret_name: discourse_local
174+ saml_sync_groups: ''
175 saml_target_url: ''
176 force_saml_login: false
177 throttle_level: none
178diff --git a/tests/unit/fixtures/config_valid_no_tls.yaml b/tests/unit/fixtures/config_valid_no_tls.yaml
179index 8179e1a..cfb7541 100644
180--- a/tests/unit/fixtures/config_valid_no_tls.yaml
181+++ b/tests/unit/fixtures/config_valid_no_tls.yaml
182@@ -21,6 +21,7 @@ config:
183 smtp_port: 587
184 smtp_username: apikey
185 tls_secret_name: ''
186+ saml_sync_groups: ''
187 saml_target_url: https://login.ubuntu.com/+saml
188 force_saml_login: true
189 throttle_level: none
190diff --git a/tests/unit/fixtures/config_valid_with_tls.yaml b/tests/unit/fixtures/config_valid_with_tls.yaml
191index a4eff75..37dc673 100644
192--- a/tests/unit/fixtures/config_valid_with_tls.yaml
193+++ b/tests/unit/fixtures/config_valid_with_tls.yaml
194@@ -20,6 +20,7 @@ config:
195 smtp_port: 587
196 smtp_username: apikey
197 tls_secret_name: discourse-local
198+ saml_sync_groups: ''
199 saml_target_url: https://login.ubuntu.com/+saml
200 force_saml_login: true
201 throttle_level: permissive
202diff --git a/tests/unit/fixtures/config_valid_without_tls.yaml b/tests/unit/fixtures/config_valid_without_tls.yaml
203index 6dff7ab..dd331c8 100644
204--- a/tests/unit/fixtures/config_valid_without_tls.yaml
205+++ b/tests/unit/fixtures/config_valid_without_tls.yaml
206@@ -19,6 +19,7 @@ config:
207 smtp_password:
208 smtp_port: 587
209 smtp_username: apikey
210+ saml_sync_groups: ''
211 saml_target_url: https://login.ubuntu.com/+saml
212 force_saml_login: false
213 throttle_level: strict
214diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
215index 40da7db..2012479 100644
216--- a/tests/unit/test_charm.py
217+++ b/tests/unit/test_charm.py
218@@ -13,6 +13,7 @@ import yaml
219 from types import SimpleNamespace
220
221 from charm import (
222+ check_for_config_problems,
223 check_for_missing_config_fields,
224 create_discourse_pod_config,
225 get_pod_spec,
226@@ -86,6 +87,12 @@ class TestDiscourseK8sCharmHooksDisabled(unittest.TestCase):
227 self.configs[config_key]['missing_fields'],
228 'Invalid config {} does not fail as expected.'.format(config_key),
229 )
230+ config_problems = check_for_config_problems(self.configs[config_key]['config'], stored)
231+ self.assertEqual(
232+ config_problems,
233+ self.configs[config_key]['config_problems'],
234+ 'Invalid config {} does not fail with expected problems'.format(config_key),
235+ )
236
237 def test_charm_config_process(self):
238 """Test that the entire config process for the charm works."""

Subscribers

People subscribed via source and target branches