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
diff --git a/config.yaml b/config.yaml
index 85f0677..931dc5f 100644
--- a/config.yaml
+++ b/config.yaml
@@ -71,13 +71,17 @@ options:
71 type: string71 type: string
72 description: "Throttle level - blocks excessive usage by ip. Valid values: none, permissive, strict"72 description: "Throttle level - blocks excessive usage by ip. Valid values: none, permissive, strict"
73 default: none73 default: none
74 saml_sync_groups:
75 type: string
76 description: "Comma-separated list of groups to sync from SAML provider."
77 default: ""
74 saml_target_url:78 saml_target_url:
75 type: string79 type: string
76 description: "SAML authentication target url"80 description: "SAML authentication target url"
77 default: ""81 default: ""
78 force_saml_login:82 force_saml_login:
79 type: boolean83 type: boolean
80 description: "Force saml login (full screen, no local database logins)"84 description: "Force SAML login (full screen, no local database logins)"
81 default: false85 default: false
82 max_body_size:86 max_body_size:
83 type: int87 type: int
diff --git a/src/charm.py b/src/charm.py
index 0ef9f01..4085867 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -93,6 +93,15 @@ def get_saml_config(config):
93 if fingerprint:93 if fingerprint:
94 saml_config['DISCOURSE_SAML_CERT_FINGERPRINT'] = fingerprint94 saml_config['DISCOURSE_SAML_CERT_FINGERPRINT'] = fingerprint
9595
96 saml_sync_groups = [x.strip() for x in config['saml_sync_groups'].split(',') if x.strip()]
97 if saml_sync_groups:
98 # Per https://github.com/discourse/discourse-saml setting this to `true`
99 # means the assigned groups will be completely synced including adding
100 # AND removing groups based on the SAML provider.
101 saml_config['DISCOURSE_SAML_GROUPS_FULLSYNC'] = "false"
102 saml_config['DISCOURSE_SAML_SYNC_GROUPS'] = "true"
103 saml_config['DISCOURSE_SAML_SYNC_GROUPS_LIST'] = "|".join(saml_sync_groups)
104
96 return saml_config105 return saml_config
97106
98107
@@ -186,8 +195,11 @@ def check_for_config_problems(config, stored):
186 if not THROTTLE_LEVELS.get(config['throttle_level']):195 if not THROTTLE_LEVELS.get(config['throttle_level']):
187 errors.append('throttle_level must be one of: ' + ' '.join(THROTTLE_LEVELS.keys()))196 errors.append('throttle_level must be one of: ' + ' '.join(THROTTLE_LEVELS.keys()))
188197
189 if config['force_saml_login'] and config['saml_target_url'] == '':198 if config['force_saml_login'] and not config['saml_target_url']:
190 errors.append('force_saml_login can not be true without a saml_target_url')199 errors.append("'force_saml_login' cannot be true without a 'saml_target_url'")
200
201 if config['saml_sync_groups'] and not config['saml_target_url']:
202 errors.append("'saml_sync_groups' cannot be specified without a 'saml_target_url'")
191203
192 return errors204 return errors
193205
diff --git a/tests/unit/fixtures/config_invalid_bad_throttle_mode.yaml b/tests/unit/fixtures/config_invalid_bad_throttle_mode.yaml
index 46d7ee6..3e0ecf0 100644
--- a/tests/unit/fixtures/config_invalid_bad_throttle_mode.yaml
+++ b/tests/unit/fixtures/config_invalid_bad_throttle_mode.yaml
@@ -19,7 +19,10 @@ config:
19 smtp_port: 58719 smtp_port: 587
20 smtp_username: apikey20 smtp_username: apikey
21 tls_secret_name: discourse_local21 tls_secret_name: discourse_local
22 saml_sync_groups: ''
22 saml_target_url: https://login.ubuntu.com/+saml23 saml_target_url: https://login.ubuntu.com/+saml
23 force_saml_login: true24 force_saml_login: true
24 throttle_level: scream25 throttle_level: scream
26config_problems:
27 - 'throttle_level must be one of: none permissive strict'
25missing_fields: []28missing_fields: []
diff --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
index 8be167e..86a5002 100644
--- a/tests/unit/fixtures/config_invalid_force_saml_no_saml_target.yaml
+++ b/tests/unit/fixtures/config_invalid_force_saml_no_saml_target.yaml
@@ -19,7 +19,12 @@ config:
19 smtp_port: 58719 smtp_port: 587
20 smtp_username: apikey20 smtp_username: apikey
21 tls_secret_name: discourse_local21 tls_secret_name: discourse_local
22 saml_sync_groups: 'canonical, ubuntu-core'
22 saml_target_url: ''23 saml_target_url: ''
23 force_saml_login: true24 force_saml_login: true
24 throttle_level: scream25 throttle_level: scream
26config_problems:
27 - 'throttle_level must be one of: none permissive strict'
28 - "'force_saml_login' cannot be true without a 'saml_target_url'"
29 - "'saml_sync_groups' cannot be specified without a 'saml_target_url'"
25missing_fields: []30missing_fields: []
diff --git a/tests/unit/fixtures/config_invalid_missing_cors.yaml b/tests/unit/fixtures/config_invalid_missing_cors.yaml
index d135c3f..85ece42 100644
--- a/tests/unit/fixtures/config_invalid_missing_cors.yaml
+++ b/tests/unit/fixtures/config_invalid_missing_cors.yaml
@@ -17,8 +17,11 @@ config:
17 smtp_password: OBV10USLYF4K317 smtp_password: OBV10USLYF4K3
18 smtp_port: 58718 smtp_port: 587
19 smtp_username: apikey19 smtp_username: apikey
20 saml_sync_groups: ''
20 saml_target_url: https://login.ubuntu.com/+saml21 saml_target_url: https://login.ubuntu.com/+saml
21 force_saml_login: true22 force_saml_login: true
22 throttle_level: none23 throttle_level: none
24config_problems:
25 - 'Required configuration missing: cors_origin'
23missing_fields: 26missing_fields:
24 - 'cors_origin'27 - 'cors_origin'
diff --git a/tests/unit/fixtures/config_invalid_missing_db_name.yaml b/tests/unit/fixtures/config_invalid_missing_db_name.yaml
index 865ae83..128c381 100644
--- a/tests/unit/fixtures/config_invalid_missing_db_name.yaml
+++ b/tests/unit/fixtures/config_invalid_missing_db_name.yaml
@@ -15,8 +15,11 @@ config:
15 smtp_password: OBV10USLYF4K315 smtp_password: OBV10USLYF4K3
16 smtp_port: 58716 smtp_port: 587
17 smtp_username: apikey17 smtp_username: apikey
18 saml_sync_groups: ''
18 saml_target_url: https://login.ubuntu.com/+saml19 saml_target_url: https://login.ubuntu.com/+saml
19 force_saml_login: true20 force_saml_login: true
20 throttle_level: none21 throttle_level: none
22config_problems:
23 - 'Required configuration missing: db_name'
21missing_fields: 24missing_fields:
22 - 'db_name'25 - 'db_name'
diff --git a/tests/unit/fixtures/config_valid_complete.yaml b/tests/unit/fixtures/config_valid_complete.yaml
index 946388d..800255e 100644
--- a/tests/unit/fixtures/config_valid_complete.yaml
+++ b/tests/unit/fixtures/config_valid_complete.yaml
@@ -21,6 +21,7 @@ config:
21 smtp_port: 58721 smtp_port: 587
22 smtp_username: apikey22 smtp_username: apikey
23 tls_secret_name: discourse_local23 tls_secret_name: discourse_local
24 saml_sync_groups: 'canonical, ubuntu-core'
24 saml_target_url: https://login.ubuntu.com/+saml25 saml_target_url: https://login.ubuntu.com/+saml
25 force_saml_login: true26 force_saml_login: true
26 throttle_level: none27 throttle_level: none
@@ -44,6 +45,9 @@ pod_config:
44 DISCOURSE_SMTP_USER_NAME: apikey45 DISCOURSE_SMTP_USER_NAME: apikey
45 DISCOURSE_REFRESH_MAXMIND_DB_DURING_PRECOMPILE_DAYS: '0'46 DISCOURSE_REFRESH_MAXMIND_DB_DURING_PRECOMPILE_DAYS: '0'
46 DISCOURSE_SERVE_STATIC_ASSETS: 'true'47 DISCOURSE_SERVE_STATIC_ASSETS: 'true'
48 DISCOURSE_SAML_GROUPS_FULLSYNC: 'false'
49 DISCOURSE_SAML_SYNC_GROUPS: 'true'
50 DISCOURSE_SAML_SYNC_GROUPS_LIST: 'canonical|ubuntu-core'
47 DISCOURSE_SAML_TARGET_URL: https://login.ubuntu.com/+saml51 DISCOURSE_SAML_TARGET_URL: https://login.ubuntu.com/+saml
48 DISCOURSE_SAML_FULL_SCREEN_LOGIN: "true"52 DISCOURSE_SAML_FULL_SCREEN_LOGIN: "true"
49 DISCOURSE_SAML_CERT_FINGERPRINT: 32:15:20:9F:A4:3C:8E:3E:8E:47:72:62:9A:86:8D:0E:E6:CF:45:D553 DISCOURSE_SAML_CERT_FINGERPRINT: 32:15:20:9F:A4:3C:8E:3E:8E:47:72:62:9A:86:8D:0E:E6:CF:45:D5
@@ -73,6 +77,9 @@ pod_spec:
73 DISCOURSE_SMTP_USER_NAME: apikey77 DISCOURSE_SMTP_USER_NAME: apikey
74 DISCOURSE_REFRESH_MAXMIND_DB_DURING_PRECOMPILE_DAYS: '0'78 DISCOURSE_REFRESH_MAXMIND_DB_DURING_PRECOMPILE_DAYS: '0'
75 DISCOURSE_SERVE_STATIC_ASSETS: 'true'79 DISCOURSE_SERVE_STATIC_ASSETS: 'true'
80 DISCOURSE_SAML_GROUPS_FULLSYNC: 'false'
81 DISCOURSE_SAML_SYNC_GROUPS: 'true'
82 DISCOURSE_SAML_SYNC_GROUPS_LIST: 'canonical|ubuntu-core'
76 DISCOURSE_SAML_TARGET_URL: https://login.ubuntu.com/+saml83 DISCOURSE_SAML_TARGET_URL: https://login.ubuntu.com/+saml
77 DISCOURSE_SAML_FULL_SCREEN_LOGIN: "true"84 DISCOURSE_SAML_FULL_SCREEN_LOGIN: "true"
78 DISCOURSE_SAML_CERT_FINGERPRINT: 32:15:20:9F:A4:3C:8E:3E:8E:47:72:62:9A:86:8D:0E:E6:CF:45:D585 DISCOURSE_SAML_CERT_FINGERPRINT: 32:15:20:9F:A4:3C:8E:3E:8E:47:72:62:9A:86:8D:0E:E6:CF:45:D5
diff --git a/tests/unit/fixtures/config_valid_no_saml_fingerprint.yaml b/tests/unit/fixtures/config_valid_no_saml_fingerprint.yaml
index cc76606..c1887db 100644
--- a/tests/unit/fixtures/config_valid_no_saml_fingerprint.yaml
+++ b/tests/unit/fixtures/config_valid_no_saml_fingerprint.yaml
@@ -20,6 +20,7 @@ config:
20 smtp_port: 58720 smtp_port: 587
21 smtp_username: apikey21 smtp_username: apikey
22 tls_secret_name: discourse_local22 tls_secret_name: discourse_local
23 saml_sync_groups: ''
23 saml_target_url: https://login.launchpad.net/+saml24 saml_target_url: https://login.launchpad.net/+saml
24 force_saml_login: true25 force_saml_login: true
25 throttle_level: none26 throttle_level: none
diff --git a/tests/unit/fixtures/config_valid_no_saml_target_url.yaml b/tests/unit/fixtures/config_valid_no_saml_target_url.yaml
index 1364998..cbae7b7 100644
--- a/tests/unit/fixtures/config_valid_no_saml_target_url.yaml
+++ b/tests/unit/fixtures/config_valid_no_saml_target_url.yaml
@@ -20,6 +20,7 @@ config:
20 smtp_port: 58720 smtp_port: 587
21 smtp_username: apikey21 smtp_username: apikey
22 tls_secret_name: discourse_local22 tls_secret_name: discourse_local
23 saml_sync_groups: ''
23 saml_target_url: ''24 saml_target_url: ''
24 force_saml_login: false25 force_saml_login: false
25 throttle_level: none26 throttle_level: none
diff --git a/tests/unit/fixtures/config_valid_no_tls.yaml b/tests/unit/fixtures/config_valid_no_tls.yaml
index 8179e1a..cfb7541 100644
--- a/tests/unit/fixtures/config_valid_no_tls.yaml
+++ b/tests/unit/fixtures/config_valid_no_tls.yaml
@@ -21,6 +21,7 @@ config:
21 smtp_port: 58721 smtp_port: 587
22 smtp_username: apikey22 smtp_username: apikey
23 tls_secret_name: ''23 tls_secret_name: ''
24 saml_sync_groups: ''
24 saml_target_url: https://login.ubuntu.com/+saml25 saml_target_url: https://login.ubuntu.com/+saml
25 force_saml_login: true26 force_saml_login: true
26 throttle_level: none27 throttle_level: none
diff --git a/tests/unit/fixtures/config_valid_with_tls.yaml b/tests/unit/fixtures/config_valid_with_tls.yaml
index a4eff75..37dc673 100644
--- a/tests/unit/fixtures/config_valid_with_tls.yaml
+++ b/tests/unit/fixtures/config_valid_with_tls.yaml
@@ -20,6 +20,7 @@ config:
20 smtp_port: 58720 smtp_port: 587
21 smtp_username: apikey21 smtp_username: apikey
22 tls_secret_name: discourse-local22 tls_secret_name: discourse-local
23 saml_sync_groups: ''
23 saml_target_url: https://login.ubuntu.com/+saml24 saml_target_url: https://login.ubuntu.com/+saml
24 force_saml_login: true25 force_saml_login: true
25 throttle_level: permissive26 throttle_level: permissive
diff --git a/tests/unit/fixtures/config_valid_without_tls.yaml b/tests/unit/fixtures/config_valid_without_tls.yaml
index 6dff7ab..dd331c8 100644
--- a/tests/unit/fixtures/config_valid_without_tls.yaml
+++ b/tests/unit/fixtures/config_valid_without_tls.yaml
@@ -19,6 +19,7 @@ config:
19 smtp_password:19 smtp_password:
20 smtp_port: 58720 smtp_port: 587
21 smtp_username: apikey21 smtp_username: apikey
22 saml_sync_groups: ''
22 saml_target_url: https://login.ubuntu.com/+saml23 saml_target_url: https://login.ubuntu.com/+saml
23 force_saml_login: false24 force_saml_login: false
24 throttle_level: strict25 throttle_level: strict
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 40da7db..2012479 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -13,6 +13,7 @@ import yaml
13from types import SimpleNamespace13from types import SimpleNamespace
1414
15from charm import (15from charm import (
16 check_for_config_problems,
16 check_for_missing_config_fields,17 check_for_missing_config_fields,
17 create_discourse_pod_config,18 create_discourse_pod_config,
18 get_pod_spec,19 get_pod_spec,
@@ -86,6 +87,12 @@ class TestDiscourseK8sCharmHooksDisabled(unittest.TestCase):
86 self.configs[config_key]['missing_fields'],87 self.configs[config_key]['missing_fields'],
87 'Invalid config {} does not fail as expected.'.format(config_key),88 'Invalid config {} does not fail as expected.'.format(config_key),
88 )89 )
90 config_problems = check_for_config_problems(self.configs[config_key]['config'], stored)
91 self.assertEqual(
92 config_problems,
93 self.configs[config_key]['config_problems'],
94 'Invalid config {} does not fail with expected problems'.format(config_key),
95 )
8996
90 def test_charm_config_process(self):97 def test_charm_config_process(self):
91 """Test that the entire config process for the charm works."""98 """Test that the entire config process for the charm works."""

Subscribers

People subscribed via source and target branches