From 38900b09b930dd19dfb7308de8421b75a195725e Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 2 May 2024 01:31:18 +0100 Subject: [PATCH 01/88] drafted approach to re-do add-admin user action --- src/charm.py | 92 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 88 insertions(+), 4 deletions(-) diff --git a/src/charm.py b/src/charm.py index 92f44fac..cf763e00 100755 --- a/src/charm.py +++ b/src/charm.py @@ -704,19 +704,45 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: email = event.params["email"] password = event.params["password"] container = self.unit.get_container(CONTAINER_NAME) + + # see if user already exists + if container.can_connect(): process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", - "admin:create", + "users:exists", + email, + ], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + ) + try: + process.wait_output() + except ExecError as ex: + # User does not exist, create it + if self._create_user(event, email, password): + event.set_results({"user": f"{email}"}) + else: + event.fail("Container is not ready") + + # make user an admin + + if container.can_connect(): + process = container.exec( + [ + os.path.join(DISCOURSE_PATH, "bin/bundle"), + "exec", + "rake", + "users:make_admin", + email, ], - stdin=f"{email}\n{password}\n{password}\nY\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), - timeout=60, ) try: process.wait_output() @@ -724,10 +750,68 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: except ExecError as ex: event.fail( # Parameter validation errors are printed to stdout - f"Failed to create user with email {email}: {ex.stdout}" # type: ignore + f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore ) + + + # if container.can_connect(): + # process = container.exec( + # [ + # os.path.join(DISCOURSE_PATH, "bin/bundle"), + # "exec", + # "rake", + # "admin:create", + # ], + # stdin=f"{email}\n{password}\n{password}\nY\n", + # working_dir=DISCOURSE_PATH, + # user=CONTAINER_APP_USERNAME, + # environment=self._create_discourse_environment_settings(), + # timeout=60, + # ) + # try: + # process.wait_output() + # event.set_results({"user": f"{email}"}) + # except ExecError as ex: + # event.fail( + # # Parameter validation errors are printed to stdout + # f"Failed to create user with email {email}: {ex.stdout}" # type: ignore + # ) + # else: + # event.fail("Container is not ready") + + def _create_user(self, event, email: str, password: str) -> bool: + """Create a new user in Discourse. + + Args: + email: Email of the user to be created. + password: Password of the user to be created. + + Returns: + bool: True if user creation is successful, False otherwise. + """ + container = self.unit.get_container(CONTAINER_NAME) + if container.can_connect(): + process = container.exec( + [ + os.path.join(DISCOURSE_PATH, "bin/bundle"), + "exec", + "rake", + "users:create", + ], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + timeout=60, + ) + try: + process.wait_output() + return True + except ExecError as ex: + event.fail(f"Failed to create user with email {email}: {ex.stdout}") + return False else: event.fail("Container is not ready") + return False def _config_force_https(self) -> None: """Config Discourse to force_https option based on charm configuration.""" From 7a56ca377645ff117b16bc36f064ab785f74ed4f Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 3 May 2024 02:14:38 +0100 Subject: [PATCH 02/88] fixed linting, small bugs, typos --- requirements.txt | 1 + src/charm.py | 51 ++++++++++++++-------------------------- tests/unit/test_charm.py | 2 +- 3 files changed, 20 insertions(+), 34 deletions(-) diff --git a/requirements.txt b/requirements.txt index 99145740..ee92c4de 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,3 @@ +ops==2.12 ops-lib-pgsql pydantic==1.10.15 diff --git a/src/charm.py b/src/charm.py index cf763e00..590d35b8 100755 --- a/src/charm.py +++ b/src/charm.py @@ -702,7 +702,6 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: event: Event triggering the add_admin_user action. """ email = event.params["email"] - password = event.params["password"] container = self.unit.get_container(CONTAINER_NAME) # see if user already exists @@ -722,15 +721,13 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: ) try: process.wait_output() - except ExecError as ex: + except ExecError: # User does not exist, create it - if self._create_user(event, email, password): + if self._create_user(event, email): event.set_results({"user": f"{email}"}) else: event.fail("Container is not ready") - # make user an admin - if container.can_connect(): process = container.exec( [ @@ -752,34 +749,8 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: # Parameter validation errors are printed to stdout f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore ) - - - # if container.can_connect(): - # process = container.exec( - # [ - # os.path.join(DISCOURSE_PATH, "bin/bundle"), - # "exec", - # "rake", - # "admin:create", - # ], - # stdin=f"{email}\n{password}\n{password}\nY\n", - # working_dir=DISCOURSE_PATH, - # user=CONTAINER_APP_USERNAME, - # environment=self._create_discourse_environment_settings(), - # timeout=60, - # ) - # try: - # process.wait_output() - # event.set_results({"user": f"{email}"}) - # except ExecError as ex: - # event.fail( - # # Parameter validation errors are printed to stdout - # f"Failed to create user with email {email}: {ex.stdout}" # type: ignore - # ) - # else: - # event.fail("Container is not ready") - - def _create_user(self, event, email: str, password: str) -> bool: + + def _create_user(self, event, email: str) -> bool: """Create a new user in Discourse. Args: @@ -790,6 +761,7 @@ def _create_user(self, event, email: str, password: str) -> bool: bool: True if user creation is successful, False otherwise. """ container = self.unit.get_container(CONTAINER_NAME) + password = self._generate_password() if container.can_connect(): process = container.exec( [ @@ -797,6 +769,8 @@ def _create_user(self, event, email: str, password: str) -> bool: "exec", "rake", "users:create", + email, + password, ], working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, @@ -813,6 +787,17 @@ def _create_user(self, event, email: str, password: str) -> bool: event.fail("Container is not ready") return False + def _generate_password(self, length: int = 16) -> str: + """Generate a random password. + + Args: + length: Length of the password to generate. + + Returns: + str: Random password. + """ + return os.urandom(length).hex() + def _config_force_https(self) -> None: """Config Discourse to force_https option based on charm configuration.""" container = self.unit.get_container("discourse") diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 5865b653..67eb9ced 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -313,7 +313,7 @@ def test_db_relation(): def test_add_admin_user(): """ arrange: an email and a password - act: when the _on_add_admin_user_action mtehod is executed + act: when the _on_add_admin_user_action method is executed assert: the underlying rake command to add the user is executed with the appropriate parameters. """ From 25f27f571f5204fcc1788408d58fc5a9e48237bf Mon Sep 17 00:00:00 2001 From: codethulu Date: Sat, 4 May 2024 03:32:58 +0100 Subject: [PATCH 03/88] fixes --- src/charm.py | 81 ++++++++++++++++++++-------------------- tests/unit/test_charm.py | 43 +++++++++++++++++++++ 2 files changed, 84 insertions(+), 40 deletions(-) diff --git a/src/charm.py b/src/charm.py index 590d35b8..6d964c0f 100755 --- a/src/charm.py +++ b/src/charm.py @@ -706,49 +706,50 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: # see if user already exists - if container.can_connect(): - process = container.exec( - [ - os.path.join(DISCOURSE_PATH, "bin/bundle"), - "exec", - "rake", - "users:exists", - email, - ], - working_dir=DISCOURSE_PATH, - user=CONTAINER_APP_USERNAME, - environment=self._create_discourse_environment_settings(), - ) - try: - process.wait_output() - except ExecError: - # User does not exist, create it - if self._create_user(event, email): - event.set_results({"user": f"{email}"}) - else: + if not container.can_connect(): event.fail("Container is not ready") + return - if container.can_connect(): - process = container.exec( - [ - os.path.join(DISCOURSE_PATH, "bin/bundle"), - "exec", - "rake", - "users:make_admin", - email, - ], - working_dir=DISCOURSE_PATH, - user=CONTAINER_APP_USERNAME, - environment=self._create_discourse_environment_settings(), - ) - try: - process.wait_output() + process = container.exec( + [ + os.path.join(DISCOURSE_PATH, "bin/bundle"), + "exec", + "rake", + "users:exists", + email, + ], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + ) + try: + process.wait_output() + except ExecError: + # User does not exist, create the user before making them an admin + if self._create_user(event, email): event.set_results({"user": f"{email}"}) - except ExecError as ex: - event.fail( - # Parameter validation errors are printed to stdout - f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore - ) + + # make user an admin + process = container.exec( + [ + os.path.join(DISCOURSE_PATH, "bin/bundle"), + "exec", + "rake", + "users:make_admin", + email, + ], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + ) + try: + process.wait_output() + event.set_results({"user": f"{email}"}) + except ExecError as ex: + event.fail( + # Parameter validation errors are printed to stdout + f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore + ) def _create_user(self, event, email: str) -> bool: """Create a new user in Discourse. diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 67eb9ced..e9f93554 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -353,6 +353,49 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: charm._on_add_admin_user_action(event) +def test_add_user(): + """ + arrange: an email and a password + act: when the _on_add_user_action method is executed + assert: the underlying rake command to add the user is executed + with the appropriate parameters. + """ + harness = helpers.start_harness() + + # We catch the exec call that we expect to register it and make sure that the + # args passed to it are correct. + expected_exec_call_was_made = False + + def bundle_handler(args: ops.testing.ExecArgs) -> None: + nonlocal expected_exec_call_was_made + expected_exec_call_was_made = True + if ( + args.environment != harness.charm._create_discourse_environment_settings() + or args.working_dir != DISCOURSE_PATH + or args.user != "_daemon_" + or args.stdin != f"{email}\n{password}\n{password}\n" + or args.timeout != 60 + ): + raise ValueError(f"{args.command} wasn't made with the correct args.") + + harness.handle_exec( + SERVICE_NAME, + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "users:create"], + handler=bundle_handler, + ) + + charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) + + email = "sample@email.com" + password = "somepassword" # nosec + event = MagicMock(spec=ActionEvent) + event.params = { + "email": email, + "password": password, + } + charm._create_user(event, email) + + def test_anonymize_user(): """ arrange: set up discourse From 86745c43ccabb89111784d4fbe76be42f62ff2e7 Mon Sep 17 00:00:00 2001 From: codethulu Date: Tue, 14 May 2024 16:56:49 +0100 Subject: [PATCH 04/88] added patches --- discourse_rock/patches/admin_promote.patch | 29 +++++++++ discourse_rock/patches/create_user.patch | 70 ++++++++++++++++++++++ discourse_rock/rockcraft.yaml | 14 ++--- src/charm.py | 59 +++++++++--------- 4 files changed, 134 insertions(+), 38 deletions(-) create mode 100644 discourse_rock/patches/admin_promote.patch create mode 100644 discourse_rock/patches/create_user.patch diff --git a/discourse_rock/patches/admin_promote.patch b/discourse_rock/patches/admin_promote.patch new file mode 100644 index 00000000..cd62595d --- /dev/null +++ b/discourse_rock/patches/admin_promote.patch @@ -0,0 +1,29 @@ +--- a/lib/tasks/admin.rake ++++ b/lib/tasks/admin.rake +@@ -210,3 +210,12 @@ def find_user(username) + + user + end ++ ++desc "Promote a user to admin" ++task "admin:promote", [:email] => [:environment] do |_, args| ++email = args[:email] ++ if !email || email !~ /@/ ++ puts "ERROR: Expecting rake admin:promote[some@email.com]" ++ exit 1 ++ end ++ if User.find_by_email(email).nil? ++ puts "ERROR: User with email #{email} not found" ++ exit 1 ++ end ++ admin = User.find_by_email(email) ++ grant_admin = ask("Do you want to grant Admin privileges to this account? (Y/n) ") ++ if (grant_admin == "" || grant_admin.downcase == "y") ++ admin.grant_admin! ++ admin.change_trust_level!(1) if admin.trust_level < 1 ++ admin.email_tokens.update_all confirmed: true ++ admin.activate ++ ++ say("\nYour account now has Admin privileges!") ++ end ++end \ No newline at end of file diff --git a/discourse_rock/patches/create_user.patch b/discourse_rock/patches/create_user.patch new file mode 100644 index 00000000..3da1d401 --- /dev/null +++ b/discourse_rock/patches/create_user.patch @@ -0,0 +1,70 @@ +--- a/lib/tasks/users.rake ++++ b/lib/tasks/users.rake +@@ -210,3 +210,12 @@ def find_user(username) + + user + end ++ ++desc "Create the user for given credentials" ++task "user:create" => :environment do ++ require "highline/import" ++ ++ begin ++ email = ask("Email: ") ++ existing_user = User.find_by_email(email) ++ ++ # check if user account already exists ++ if existing_user ++ # user already exists, ask for password reset ++ new_user = existing_user ++ reset_password = ++ ask( ++ "User with this email already exists! Do you want to reset the password for this email? (Y/n) ", ++ ) ++ if (reset_password == "" || reset_password.downcase == "y") ++ begin ++ password = ask("Password: ") { |q| q.echo = false } ++ password_confirmation = ask("Repeat password: ") { |q| q.echo = false } ++ passwords_match = password == password_confirmation ++ ++ say("Passwords don't match, try again...") unless passwords_match ++ end while !passwords_match ++ new_user.password = password ++ end ++ else ++ # create new user ++ new_user = User.new ++ new_user.email = email ++ new_user.username = UserNameSuggester.suggest(new_user.email) ++ begin ++ if ENV["RANDOM_PASSWORD"] == "1" ++ password = password_confirmation = SecureRandom.hex ++ else ++ password = ask("Password: ") { |q| q.echo = false } ++ password_confirmation = ask("Repeat password: ") { |q| q.echo = false } ++ end ++ ++ passwords_match = password == password_confirmation ++ ++ say("Passwords don't match, try again...") unless passwords_match ++ end while !passwords_match ++ new_user.password = password ++ end ++ ++ new_user.name = ask("Full name: ") if SiteSetting.full_name_required && new_user.name.blank? ++ ++ # save/update user account ++ saved = new_user.save ++ say(new_user.errors.full_messages.join("\n")) unless saved ++ end while !saved ++ ++ say "\nEnsuring account is active!" ++ new_user.active = true ++ new_user.save ++ ++ if existing_user ++ say("\nAccount updated successfully!") ++ else ++ say("\nAccount created successfully with username #{new_user.username}") ++ end ++end diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 054c01ed..21a59dd0 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -268,10 +268,10 @@ parts: chown -R 584792:584792 var/lib/pebble/default/.npm chown -R 584792:584792 var/lib/pebble/default/.yarn chown 584792:584792 var/lib/pebble/default/.yarnrc -checks: - discourse-setup-completed: - override: replace - level: ready - threshold: 1 - exec: - command: ls /run/discourse-k8s-operator/setup_completed +# checks: +# discourse-setup-completed: +# override: replace +# level: ready +# threshold: 1 +# exec: +# command: ls /run/discourse-k8s-operator/setup_completed diff --git a/src/charm.py b/src/charm.py index 6d964c0f..e22d5d0a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -702,12 +702,13 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: event: Event triggering the add_admin_user action. """ email = event.params["email"] - container = self.unit.get_container(CONTAINER_NAME) + # @ can cause issues cannot escape @ symbol + # user / domain change config - # see if user already exists + container = self.unit.get_container(CONTAINER_NAME) if not container.can_connect(): - event.fail("Container is not ready") + event.fail("Unable to connect to container, container is not ready") return process = container.exec( @@ -716,8 +717,8 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: "exec", "rake", "users:exists", - email, ], + stdin=f"{email}\nY\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), @@ -725,9 +726,7 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: try: process.wait_output() except ExecError: - # User does not exist, create the user before making them an admin - if self._create_user(event, email): - event.set_results({"user": f"{email}"}) + self._create_user(event, email) # make user an admin process = container.exec( @@ -751,7 +750,7 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore ) - def _create_user(self, event, email: str) -> bool: + def _create_user(self, event: ActionEvent, email: str): """Create a new user in Discourse. Args: @@ -763,30 +762,28 @@ def _create_user(self, event, email: str) -> bool: """ container = self.unit.get_container(CONTAINER_NAME) password = self._generate_password() - if container.can_connect(): - process = container.exec( - [ - os.path.join(DISCOURSE_PATH, "bin/bundle"), - "exec", - "rake", - "users:create", - email, - password, - ], - working_dir=DISCOURSE_PATH, - user=CONTAINER_APP_USERNAME, - environment=self._create_discourse_environment_settings(), - timeout=60, + # docker run -t dicouse:llal -- exec bin/bash + # rake users:create email password + # rake --help + process = container.exec( + [ + os.path.join(DISCOURSE_PATH, "bin/bundle"), + "exec", + "rake", + "user:create", + ], + stdin=f"{email}\n{password}\nY\n", + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + timeout=60, + ) + try: + process.wait_output() + except ExecError as ex: + event.fail( + f"Failed to create user with email {email}: {ex.stdout}" # type: ignore ) - try: - process.wait_output() - return True - except ExecError as ex: - event.fail(f"Failed to create user with email {email}: {ex.stdout}") - return False - else: - event.fail("Container is not ready") - return False def _generate_password(self, length: int = 16) -> str: """Generate a random password. From 4edeef811e7a7c0bab2c640aca012241958e874c Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 15 May 2024 14:57:03 +0100 Subject: [PATCH 05/88] blank --- .../{admin_promote.patch => admin_rake.patch} | 0 discourse_rock/patches/anonymize_user.patch | 15 --- discourse_rock/patches/rake.patch | 0 .../{create_user.patch => users_rake.patch} | 2 +- src-docs/charm.py.md | 98 ------------------- src-docs/database.py.md | 79 --------------- src/charm.py | 59 ++--------- tests/unit/test_charm.py | 2 +- 8 files changed, 12 insertions(+), 243 deletions(-) rename discourse_rock/patches/{admin_promote.patch => admin_rake.patch} (100%) delete mode 100644 discourse_rock/patches/anonymize_user.patch create mode 100644 discourse_rock/patches/rake.patch rename discourse_rock/patches/{create_user.patch => users_rake.patch} (98%) delete mode 100644 src-docs/charm.py.md delete mode 100644 src-docs/database.py.md diff --git a/discourse_rock/patches/admin_promote.patch b/discourse_rock/patches/admin_rake.patch similarity index 100% rename from discourse_rock/patches/admin_promote.patch rename to discourse_rock/patches/admin_rake.patch diff --git a/discourse_rock/patches/anonymize_user.patch b/discourse_rock/patches/anonymize_user.patch deleted file mode 100644 index a7c3a22c..00000000 --- a/discourse_rock/patches/anonymize_user.patch +++ /dev/null @@ -1,15 +0,0 @@ ---- a/lib/tasks/users.rake -+++ b/lib/tasks/users.rake -@@ -210,3 +210,12 @@ def find_user(username) - - user - end -+ -+desc "Anonymize user with the given username" -+task "users:anonymize", [:username] => [:environment] do |_, args| -+ username = args[:username] -+ user = find_user(username) -+ system_user = Discourse.system_user -+ UserAnonymizer.new(user, system_user).make_anonymous -+ puts "User #{username} anonymized" -+end diff --git a/discourse_rock/patches/rake.patch b/discourse_rock/patches/rake.patch new file mode 100644 index 00000000..e69de29b diff --git a/discourse_rock/patches/create_user.patch b/discourse_rock/patches/users_rake.patch similarity index 98% rename from discourse_rock/patches/create_user.patch rename to discourse_rock/patches/users_rake.patch index 3da1d401..0a31c49b 100644 --- a/discourse_rock/patches/create_user.patch +++ b/discourse_rock/patches/users_rake.patch @@ -6,7 +6,7 @@ end + +desc "Create the user for given credentials" -+task "user:create" => :environment do ++task "users:create" => :environment do + require "highline/import" + + begin diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md deleted file mode 100644 index 44d3f51f..00000000 --- a/src-docs/charm.py.md +++ /dev/null @@ -1,98 +0,0 @@ - - - - -# module `charm.py` -Charm for Discourse on kubernetes. - -**Global Variables** ---------------- -- **DEFAULT_RELATION_NAME** -- **DATABASE_NAME** -- **DISCOURSE_PATH** -- **THROTTLE_LEVELS** -- **LOG_PATHS** -- **PROMETHEUS_PORT** -- **REQUIRED_S3_SETTINGS** -- **SCRIPT_PATH** -- **SERVICE_NAME** -- **CONTAINER_NAME** -- **CONTAINER_APP_USERNAME** -- **SERVICE_PORT** -- **SETUP_COMPLETED_FLAG_FILE** -- **DATABASE_RELATION_NAME** - - ---- - -## class `DiscourseCharm` -Charm for Discourse on kubernetes. - - - -### function `__init__` - -```python -__init__(*args) -``` - -Initialize defaults and event handlers. - - ---- - -#### property app - -Application that this unit is part of. - ---- - -#### property charm_dir - -Root directory of the charm as it is running. - ---- - -#### property config - -A mapping containing the charm's config and current values. - ---- - -#### property meta - -Metadata of this charm. - ---- - -#### property model - -Shortcut for more simple access the model. - ---- - -#### property unit - -Unit that this execution is responsible for. - - - - ---- - -## class `MissingRedisRelationDataError` -Custom exception to be raised in case of malformed/missing redis relation data. - - - - - ---- - -## class `S3Info` -S3Info(enabled, region, bucket, endpoint) - - - - - diff --git a/src-docs/database.py.md b/src-docs/database.py.md deleted file mode 100644 index 5647b847..00000000 --- a/src-docs/database.py.md +++ /dev/null @@ -1,79 +0,0 @@ - - - - -# module `database.py` -Provide the DatabaseObserver class to handle database relation and state. - -**Global Variables** ---------------- -- **DATABASE_NAME** - - ---- - -## class `DatabaseHandler` -The Database relation observer. - - - -### function `__init__` - -```python -__init__(charm: CharmBase, relation_name) -``` - -Initialize the observer and register event handlers. - - - -**Args:** - - - `charm`: The parent charm to attach the observer to. - - ---- - -#### property model - -Shortcut for more simple access the model. - - - ---- - - - -### function `get_relation_data` - -```python -get_relation_data() → Dict[str, str] -``` - -Get database data from relation. - - - -**Returns:** - - - `Dict`: Information needed for setting environment variables. Returns default if the relation data is not correctly initialized. - ---- - - - -### function `is_relation_ready` - -```python -is_relation_ready() → bool -``` - -Check if the relation is ready. - - - -**Returns:** - - - `bool`: returns True if the relation is ready. - - diff --git a/src/charm.py b/src/charm.py index e22d5d0a..fab9829a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -701,78 +701,50 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: Args: event: Event triggering the add_admin_user action. """ - email = event.params["email"] - # @ can cause issues cannot escape @ symbol - # user / domain change config - container = self.unit.get_container(CONTAINER_NAME) if not container.can_connect(): event.fail("Unable to connect to container, container is not ready") return - process = container.exec( - [ - os.path.join(DISCOURSE_PATH, "bin/bundle"), - "exec", - "rake", - "users:exists", - ], - stdin=f"{email}\nY\n", - working_dir=DISCOURSE_PATH, - user=CONTAINER_APP_USERNAME, - environment=self._create_discourse_environment_settings(), - ) - try: - process.wait_output() - except ExecError: - self._create_user(event, email) + self._create_user(event) - # make user an admin process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", - "users:make_admin", - email, + "admin:create", ], + stdin=f"{event.params["email"]}\n{event.params["password"]}\nN\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), ) try: process.wait_output() - event.set_results({"user": f"{email}"}) + event.set_results({"user": f"{event.params["email"]}"}) except ExecError as ex: event.fail( - # Parameter validation errors are printed to stdout - f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore + f"Failed to make user with email {event.params["email"]} an admin: {ex.stdout}" # type: ignore ) - def _create_user(self, event: ActionEvent, email: str): + def _create_user(self, event: ActionEvent): """Create a new user in Discourse. Args: - email: Email of the user to be created. - password: Password of the user to be created. - - Returns: - bool: True if user creation is successful, False otherwise. + event: Event triggering the create user action. """ + container = self.unit.get_container(CONTAINER_NAME) - password = self._generate_password() - # docker run -t dicouse:llal -- exec bin/bash - # rake users:create email password - # rake --help process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", - "user:create", + "admin:create", ], - stdin=f"{email}\n{password}\nY\n", + stdin=f"{event.params["email"]}\n{event.params["password"]}\nN\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), @@ -785,17 +757,6 @@ def _create_user(self, event: ActionEvent, email: str): f"Failed to create user with email {email}: {ex.stdout}" # type: ignore ) - def _generate_password(self, length: int = 16) -> str: - """Generate a random password. - - Args: - length: Length of the password to generate. - - Returns: - str: Random password. - """ - return os.urandom(length).hex() - def _config_force_https(self) -> None: """Config Discourse to force_https option based on charm configuration.""" container = self.unit.get_container("discourse") diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index e9f93554..712797c8 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -393,7 +393,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: "email": email, "password": password, } - charm._create_user(event, email) + charm._create_user(event) def test_anonymize_user(): From 9608a39dcf5c8ae4870f25336fdb5388ea64efa2 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 15 May 2024 15:46:23 +0100 Subject: [PATCH 06/88] feat: singular rake file to have both create user and promtoe admin actions --- discourse_rock/patches/admin_rake.patch | 29 --------- discourse_rock/patches/rake.patch | 87 +++++++++++++++++++++++++ discourse_rock/patches/users_rake.patch | 70 -------------------- src/charm.py | 8 +-- 4 files changed, 91 insertions(+), 103 deletions(-) delete mode 100644 discourse_rock/patches/admin_rake.patch delete mode 100644 discourse_rock/patches/users_rake.patch diff --git a/discourse_rock/patches/admin_rake.patch b/discourse_rock/patches/admin_rake.patch deleted file mode 100644 index cd62595d..00000000 --- a/discourse_rock/patches/admin_rake.patch +++ /dev/null @@ -1,29 +0,0 @@ ---- a/lib/tasks/admin.rake -+++ b/lib/tasks/admin.rake -@@ -210,3 +210,12 @@ def find_user(username) - - user - end -+ -+desc "Promote a user to admin" -+task "admin:promote", [:email] => [:environment] do |_, args| -+email = args[:email] -+ if !email || email !~ /@/ -+ puts "ERROR: Expecting rake admin:promote[some@email.com]" -+ exit 1 -+ end -+ if User.find_by_email(email).nil? -+ puts "ERROR: User with email #{email} not found" -+ exit 1 -+ end -+ admin = User.find_by_email(email) -+ grant_admin = ask("Do you want to grant Admin privileges to this account? (Y/n) ") -+ if (grant_admin == "" || grant_admin.downcase == "y") -+ admin.grant_admin! -+ admin.change_trust_level!(1) if admin.trust_level < 1 -+ admin.email_tokens.update_all confirmed: true -+ admin.activate -+ -+ say("\nYour account now has Admin privileges!") -+ end -+end \ No newline at end of file diff --git a/discourse_rock/patches/rake.patch b/discourse_rock/patches/rake.patch index e69de29b..55e6e497 100644 --- a/discourse_rock/patches/rake.patch +++ b/discourse_rock/patches/rake.patch @@ -0,0 +1,87 @@ +--- /dev/null ++++ b/lib/tasks/canonical.rake +@@ -0,0 +1,88 @@ ++# frozen_string_literal: true ++ ++desc "Promote a user to admin" ++task "admin:promote", [:email] => [:environment] do |_, args| ++email = args[:email] ++ if !email || email !~ /@/ ++ puts "ERROR: Expecting rake admin:promote[some@email.com]" ++ exit 1 ++ end ++ if User.find_by_email(email).nil? ++ puts "ERROR: User with email #{email} not found" ++ exit 1 ++ end ++ admin = User.find_by_email(email) ++ admin.grant_admin! ++ admin.change_trust_level!(1) if admin.trust_level < 1 ++ admin.email_tokens.update_all confirmed: true ++ admin.activate ++ say("\nYour account now has Admin privileges!") ++end ++ ++desc "Create the user for given credentials" ++task "users:create" => :environment do ++ require "highline/import" ++ ++ begin ++ email = ask("Email: ") ++ existing_user = User.find_by_email(email) ++ ++ # check if user account already exists ++ if existing_user ++ # user already exists, ask for password reset ++ new_user = existing_user ++ reset_password = ++ ask( ++ "User with this email already exists! Do you want to reset the password for this email? (Y/n) ", ++ ) ++ if (reset_password == "" || reset_password.downcase == "y") ++ begin ++ password = ask("Password: ") { |q| q.echo = false } ++ password_confirmation = ask("Repeat password: ") { |q| q.echo = false } ++ passwords_match = password == password_confirmation ++ ++ say("Passwords don't match, try again...") unless passwords_match ++ end while !passwords_match ++ new_user.password = password ++ end ++ else ++ # create new user ++ new_user = User.new ++ new_user.email = email ++ new_user.username = UserNameSuggester.suggest(new_user.email) ++ begin ++ if ENV["RANDOM_PASSWORD"] == "1" ++ password = password_confirmation = SecureRandom.hex ++ else ++ password = ask("Password: ") { |q| q.echo = false } ++ password_confirmation = ask("Repeat password: ") { |q| q.echo = false } ++ end ++ ++ passwords_match = password == password_confirmation ++ ++ say("Passwords don't match, try again...") unless passwords_match ++ end while !passwords_match ++ new_user.password = password ++ end ++ ++ new_user.name = ask("Full name: ") if SiteSetting.full_name_required && new_user.name.blank? ++ ++ # save/update user account ++ saved = new_user.save ++ say(new_user.errors.full_messages.join("\n")) unless saved ++ end while !saved ++ ++ say "\nEnsuring account is active!" ++ new_user.active = true ++ new_user.save ++ ++ if existing_user ++ say("\nAccount updated successfully!") ++ else ++ say("\nAccount created successfully with username #{new_user.username}") ++ end ++end diff --git a/discourse_rock/patches/users_rake.patch b/discourse_rock/patches/users_rake.patch deleted file mode 100644 index 0a31c49b..00000000 --- a/discourse_rock/patches/users_rake.patch +++ /dev/null @@ -1,70 +0,0 @@ ---- a/lib/tasks/users.rake -+++ b/lib/tasks/users.rake -@@ -210,3 +210,12 @@ def find_user(username) - - user - end -+ -+desc "Create the user for given credentials" -+task "users:create" => :environment do -+ require "highline/import" -+ -+ begin -+ email = ask("Email: ") -+ existing_user = User.find_by_email(email) -+ -+ # check if user account already exists -+ if existing_user -+ # user already exists, ask for password reset -+ new_user = existing_user -+ reset_password = -+ ask( -+ "User with this email already exists! Do you want to reset the password for this email? (Y/n) ", -+ ) -+ if (reset_password == "" || reset_password.downcase == "y") -+ begin -+ password = ask("Password: ") { |q| q.echo = false } -+ password_confirmation = ask("Repeat password: ") { |q| q.echo = false } -+ passwords_match = password == password_confirmation -+ -+ say("Passwords don't match, try again...") unless passwords_match -+ end while !passwords_match -+ new_user.password = password -+ end -+ else -+ # create new user -+ new_user = User.new -+ new_user.email = email -+ new_user.username = UserNameSuggester.suggest(new_user.email) -+ begin -+ if ENV["RANDOM_PASSWORD"] == "1" -+ password = password_confirmation = SecureRandom.hex -+ else -+ password = ask("Password: ") { |q| q.echo = false } -+ password_confirmation = ask("Repeat password: ") { |q| q.echo = false } -+ end -+ -+ passwords_match = password == password_confirmation -+ -+ say("Passwords don't match, try again...") unless passwords_match -+ end while !passwords_match -+ new_user.password = password -+ end -+ -+ new_user.name = ask("Full name: ") if SiteSetting.full_name_required && new_user.name.blank? -+ -+ # save/update user account -+ saved = new_user.save -+ say(new_user.errors.full_messages.join("\n")) unless saved -+ end while !saved -+ -+ say "\nEnsuring account is active!" -+ new_user.active = true -+ new_user.save -+ -+ if existing_user -+ say("\nAccount updated successfully!") -+ else -+ say("\nAccount created successfully with username #{new_user.username}") -+ end -+end diff --git a/src/charm.py b/src/charm.py index fab9829a..ce605384 100755 --- a/src/charm.py +++ b/src/charm.py @@ -714,9 +714,9 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", - "admin:create", + "admin:promote", + event.params["email"], ], - stdin=f"{event.params["email"]}\n{event.params["password"]}\nN\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), @@ -742,9 +742,9 @@ def _create_user(self, event: ActionEvent): os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", - "admin:create", + "users:create", ], - stdin=f"{event.params["email"]}\n{event.params["password"]}\nN\n", + stdin=f"{event.params["email"]}\n{event.params["password"]}\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), From af7cc0a76cc9a9ccb4aa2045c68271fb0901a15e Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 15 May 2024 17:18:51 +0100 Subject: [PATCH 07/88] fixed tests and linting --- src-docs/charm.py.md | 98 ++++++++++++++++++++++++++++++++++++++++ src-docs/database.py.md | 79 ++++++++++++++++++++++++++++++++ src/charm.py | 22 +++++---- tests/unit/test_charm.py | 2 +- 4 files changed, 190 insertions(+), 11 deletions(-) create mode 100644 src-docs/charm.py.md create mode 100644 src-docs/database.py.md diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md new file mode 100644 index 00000000..44d3f51f --- /dev/null +++ b/src-docs/charm.py.md @@ -0,0 +1,98 @@ + + + + +# module `charm.py` +Charm for Discourse on kubernetes. + +**Global Variables** +--------------- +- **DEFAULT_RELATION_NAME** +- **DATABASE_NAME** +- **DISCOURSE_PATH** +- **THROTTLE_LEVELS** +- **LOG_PATHS** +- **PROMETHEUS_PORT** +- **REQUIRED_S3_SETTINGS** +- **SCRIPT_PATH** +- **SERVICE_NAME** +- **CONTAINER_NAME** +- **CONTAINER_APP_USERNAME** +- **SERVICE_PORT** +- **SETUP_COMPLETED_FLAG_FILE** +- **DATABASE_RELATION_NAME** + + +--- + +## class `DiscourseCharm` +Charm for Discourse on kubernetes. + + + +### function `__init__` + +```python +__init__(*args) +``` + +Initialize defaults and event handlers. + + +--- + +#### property app + +Application that this unit is part of. + +--- + +#### property charm_dir + +Root directory of the charm as it is running. + +--- + +#### property config + +A mapping containing the charm's config and current values. + +--- + +#### property meta + +Metadata of this charm. + +--- + +#### property model + +Shortcut for more simple access the model. + +--- + +#### property unit + +Unit that this execution is responsible for. + + + + +--- + +## class `MissingRedisRelationDataError` +Custom exception to be raised in case of malformed/missing redis relation data. + + + + + +--- + +## class `S3Info` +S3Info(enabled, region, bucket, endpoint) + + + + + diff --git a/src-docs/database.py.md b/src-docs/database.py.md new file mode 100644 index 00000000..5647b847 --- /dev/null +++ b/src-docs/database.py.md @@ -0,0 +1,79 @@ + + + + +# module `database.py` +Provide the DatabaseObserver class to handle database relation and state. + +**Global Variables** +--------------- +- **DATABASE_NAME** + + +--- + +## class `DatabaseHandler` +The Database relation observer. + + + +### function `__init__` + +```python +__init__(charm: CharmBase, relation_name) +``` + +Initialize the observer and register event handlers. + + + +**Args:** + + - `charm`: The parent charm to attach the observer to. + + +--- + +#### property model + +Shortcut for more simple access the model. + + + +--- + + + +### function `get_relation_data` + +```python +get_relation_data() → Dict[str, str] +``` + +Get database data from relation. + + + +**Returns:** + + - `Dict`: Information needed for setting environment variables. Returns default if the relation data is not correctly initialized. + +--- + + + +### function `is_relation_ready` + +```python +is_relation_ready() → bool +``` + +Check if the relation is ready. + + + +**Returns:** + + - `bool`: returns True if the relation is ready. + + diff --git a/src/charm.py b/src/charm.py index ce605384..ac281b03 100755 --- a/src/charm.py +++ b/src/charm.py @@ -703,11 +703,12 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: """ container = self.unit.get_container(CONTAINER_NAME) + email = event.params["email"] + if not container.can_connect(): event.fail("Unable to connect to container, container is not ready") return - - self._create_user(event) + self._create_user(event) process = container.exec( [ @@ -715,7 +716,7 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: "exec", "rake", "admin:promote", - event.params["email"], + email, ], working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, @@ -723,10 +724,10 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: ) try: process.wait_output() - event.set_results({"user": f"{event.params["email"]}"}) + event.set_results(email) except ExecError as ex: event.fail( - f"Failed to make user with email {event.params["email"]} an admin: {ex.stdout}" # type: ignore + f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore ) def _create_user(self, event: ActionEvent): @@ -735,8 +736,11 @@ def _create_user(self, event: ActionEvent): Args: event: Event triggering the create user action. """ - container = self.unit.get_container(CONTAINER_NAME) + + email = event.params["email"] + password = event.params["password"] + process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), @@ -744,7 +748,7 @@ def _create_user(self, event: ActionEvent): "rake", "users:create", ], - stdin=f"{event.params["email"]}\n{event.params["password"]}\n", + stdin=f"{email}\n{password}\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), @@ -753,9 +757,7 @@ def _create_user(self, event: ActionEvent): try: process.wait_output() except ExecError as ex: - event.fail( - f"Failed to create user with email {email}: {ex.stdout}" # type: ignore - ) + event.fail(f"Failed to create user with email {email}: {ex.stdout}") # type: ignore def _config_force_https(self) -> None: """Config Discourse to force_https option based on charm configuration.""" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 712797c8..7f16b1b6 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -373,7 +373,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: args.environment != harness.charm._create_discourse_environment_settings() or args.working_dir != DISCOURSE_PATH or args.user != "_daemon_" - or args.stdin != f"{email}\n{password}\n{password}\n" + or args.stdin != f"{email}\n{password}\n" or args.timeout != 60 ): raise ValueError(f"{args.command} wasn't made with the correct args.") From 72bf7db59ffc84d8d60e3a36a2cf259cd7c6c081 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 15 May 2024 17:20:32 +0100 Subject: [PATCH 08/88] re-added check to rock --- discourse_rock/rockcraft.yaml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 21a59dd0..054c01ed 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -268,10 +268,10 @@ parts: chown -R 584792:584792 var/lib/pebble/default/.npm chown -R 584792:584792 var/lib/pebble/default/.yarn chown 584792:584792 var/lib/pebble/default/.yarnrc -# checks: -# discourse-setup-completed: -# override: replace -# level: ready -# threshold: 1 -# exec: -# command: ls /run/discourse-k8s-operator/setup_completed +checks: + discourse-setup-completed: + override: replace + level: ready + threshold: 1 + exec: + command: ls /run/discourse-k8s-operator/setup_completed From a2e900fac8c16ab9bb6947f61ace77d05d73adea Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 15 May 2024 17:54:28 +0100 Subject: [PATCH 09/88] bug fixed: changed rockcraft to patch rake.patch --- discourse_rock/rockcraft.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 81092334..7d97802d 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -186,7 +186,7 @@ parts: after: [discourse, patches] override-stage: | git -C srv/discourse/app apply patches/lp1903695.patch - git -C srv/discourse/app apply patches/anonymize_user.patch + git -C srv/discourse/app apply patches/rake.patch git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation # https://github.com/lautis/uglifier/issues/127#issuecomment-352224986 From 9f6c673e7c4a7b2b3fa652b8c9e13a9768e58d3b Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 16 May 2024 10:14:20 +0100 Subject: [PATCH 10/88] fixes --- discourse_rock/patches/lp1903695.patch | 5 +---- discourse_rock/patches/rake.patch | 2 +- requirements.txt | 1 - 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/discourse_rock/patches/lp1903695.patch b/discourse_rock/patches/lp1903695.patch index 5df8612c..330320f0 100644 --- a/discourse_rock/patches/lp1903695.patch +++ b/discourse_rock/patches/lp1903695.patch @@ -1,8 +1,6 @@ -diff --git a/lib/middleware/anonymous_cache.rb b/lib/middleware/anonymous_cache.rb -index d41069c92e..fe968c6d64 100644 --- a/lib/middleware/anonymous_cache.rb +++ b/lib/middleware/anonymous_cache.rb -@@ -347,7 +347,7 @@ module Middleware +@@ -351,7 +351,7 @@ module Middleware return @app.call(env) if defined?(@@disabled) && @@disabled if PAYLOAD_INVALID_REQUEST_METHODS.include?(env[Rack::REQUEST_METHOD]) && @@ -10,4 +8,3 @@ index d41069c92e..fe968c6d64 100644 + env[Rack::RACK_INPUT].respond_to?(:size) && env[Rack::RACK_INPUT].size > 0 return 413, { "Cache-Control" => "private, max-age=0, must-revalidate" }, [] end - diff --git a/discourse_rock/patches/rake.patch b/discourse_rock/patches/rake.patch index 55e6e497..2b66f3d7 100644 --- a/discourse_rock/patches/rake.patch +++ b/discourse_rock/patches/rake.patch @@ -1,5 +1,5 @@ --- /dev/null -+++ b/lib/tasks/canonical.rake ++++ b/lib/tasks/discourse-charm.rake @@ -0,0 +1,88 @@ +# frozen_string_literal: true + diff --git a/requirements.txt b/requirements.txt index 3fed098d..eb63cb92 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,2 @@ ops==2.12.0 -ops-lib-pgsql pydantic==2.7.1 From 76f168d90483cafc5bb69d27a1b9013ce1393eed Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 16 May 2024 15:00:03 +0100 Subject: [PATCH 11/88] fixed bug in lp patch --- discourse_rock/patches/lp1903695.patch | 1 + 1 file changed, 1 insertion(+) diff --git a/discourse_rock/patches/lp1903695.patch b/discourse_rock/patches/lp1903695.patch index 330320f0..67e611f6 100644 --- a/discourse_rock/patches/lp1903695.patch +++ b/discourse_rock/patches/lp1903695.patch @@ -8,3 +8,4 @@ + env[Rack::RACK_INPUT].respond_to?(:size) && env[Rack::RACK_INPUT].size > 0 return 413, { "Cache-Control" => "private, max-age=0, must-revalidate" }, [] end + From 6846e2e163b96c02f5c8f73f4eb53cd79be2d400 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 17 May 2024 09:41:22 +0100 Subject: [PATCH 12/88] fixed rock --- discourse_rock/patches/rake.patch | 4 ++-- discourse_rock/rockcraft.yaml | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/discourse_rock/patches/rake.patch b/discourse_rock/patches/rake.patch index 2b66f3d7..b3234792 100644 --- a/discourse_rock/patches/rake.patch +++ b/discourse_rock/patches/rake.patch @@ -1,6 +1,6 @@ ---- /dev/null +--- a/lib/tasks/discourse-charm.rake +++ b/lib/tasks/discourse-charm.rake -@@ -0,0 +1,88 @@ +@@ -0,0 +1,84 @@ +# frozen_string_literal: true + +desc "Promote a user to admin" diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 7d97802d..3253e809 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -186,6 +186,7 @@ parts: after: [discourse, patches] override-stage: | git -C srv/discourse/app apply patches/lp1903695.patch + touch srv/discourse/app/lib/tasks/discourse-charm.rake git -C srv/discourse/app apply patches/rake.patch git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation From 1747d3c18328c2f80053789ab5c25898b6656ecb Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 22 May 2024 13:36:38 +0100 Subject: [PATCH 13/88] force enable https --- config.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config.yaml b/config.yaml index 8cf75602..2fe3f887 100644 --- a/config.yaml +++ b/config.yaml @@ -41,6 +41,10 @@ options: type: string description: "Enable TLS encryption for smtp connections." default: "true" + smtp_force_https: + type: boolean + description: "Force https for smtp connections." + default: "true" smtp_force_tls: type: string description: "Force implicit TLS as per RFC 8314 3.3." From 3b9e4371998607fe0c759552f73cbcdbb35b48c8 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 22 May 2024 13:55:13 +0100 Subject: [PATCH 14/88] fixes --- config.yaml | 2 +- .../patches/{rake.patch => discourse-charm.patch} | 0 discourse_rock/rockcraft.yaml | 3 +-- src/charm.py | 14 +++++++------- tests/unit/test_charm.py | 2 +- 5 files changed, 10 insertions(+), 11 deletions(-) rename discourse_rock/patches/{rake.patch => discourse-charm.patch} (100%) diff --git a/config.yaml b/config.yaml index 2fe3f887..af253c3d 100644 --- a/config.yaml +++ b/config.yaml @@ -44,7 +44,7 @@ options: smtp_force_https: type: boolean description: "Force https for smtp connections." - default: "true" + default: true smtp_force_tls: type: string description: "Force implicit TLS as per RFC 8314 3.3." diff --git a/discourse_rock/patches/rake.patch b/discourse_rock/patches/discourse-charm.patch similarity index 100% rename from discourse_rock/patches/rake.patch rename to discourse_rock/patches/discourse-charm.patch diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 3253e809..708f2d16 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -186,8 +186,7 @@ parts: after: [discourse, patches] override-stage: | git -C srv/discourse/app apply patches/lp1903695.patch - touch srv/discourse/app/lib/tasks/discourse-charm.rake - git -C srv/discourse/app apply patches/rake.patch + git -C srv/discourse/app apply patches/discourse-charm.patch git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation # https://github.com/lautis/uglifier/issues/127#issuecomment-352224986 diff --git a/src/charm.py b/src/charm.py index ac281b03..7467765c 100755 --- a/src/charm.py +++ b/src/charm.py @@ -708,7 +708,9 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: if not container.can_connect(): event.fail("Unable to connect to container, container is not ready") return - self._create_user(event) + if not self._create_user(event.params["email"], event.params["password"]): + event.fail(f"Failed to create user with email {email}") + return process = container.exec( [ @@ -730,7 +732,7 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore ) - def _create_user(self, event: ActionEvent): + def _create_user(self, email: str, password: str) -> bool: """Create a new user in Discourse. Args: @@ -738,9 +740,6 @@ def _create_user(self, event: ActionEvent): """ container = self.unit.get_container(CONTAINER_NAME) - email = event.params["email"] - password = event.params["password"] - process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), @@ -756,8 +755,9 @@ def _create_user(self, event: ActionEvent): ) try: process.wait_output() - except ExecError as ex: - event.fail(f"Failed to create user with email {email}: {ex.stdout}") # type: ignore + except ExecError: + return False + return True def _config_force_https(self) -> None: """Config Discourse to force_https option based on charm configuration.""" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 7f16b1b6..fa936d53 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -393,7 +393,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: "email": email, "password": password, } - charm._create_user(event) + assert charm._create_user(email, password) def test_anonymize_user(): From b2d0abbb6f29822a6a94d7ab5ce51bf1ad39ad6a Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 22 May 2024 14:09:07 +0100 Subject: [PATCH 15/88] minor changes to rake file --- discourse_rock/patches/discourse-charm.patch | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index b3234792..77735c74 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -5,21 +5,21 @@ + +desc "Promote a user to admin" +task "admin:promote", [:email] => [:environment] do |_, args| -+email = args[:email] ++ email = args[:email] + if !email || email !~ /@/ + puts "ERROR: Expecting rake admin:promote[some@email.com]" + exit 1 + end -+ if User.find_by_email(email).nil? ++ admin = User.find_by_email(email) ++ if admin.nil? + puts "ERROR: User with email #{email} not found" + exit 1 + end -+ admin = User.find_by_email(email) -+ admin.grant_admin! -+ admin.change_trust_level!(1) if admin.trust_level < 1 -+ admin.email_tokens.update_all confirmed: true -+ admin.activate -+ say("\nYour account now has Admin privileges!") ++ admin.grant_admin! ++ admin.change_trust_level!(1) if admin.trust_level < 1 ++ admin.email_tokens.update_all confirmed: true ++ admin.activate ++ say("\nYour account now has Admin privileges!") +end + +desc "Create the user for given credentials" From f6baa423c1bebfbfc0342487df71f36bcc7f97fa Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 22 May 2024 14:10:09 +0100 Subject: [PATCH 16/88] blank --- discourse_rock/patches/discourse-charm.patch | 1 + 1 file changed, 1 insertion(+) diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index 77735c74..bf72b64a 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -84,4 +84,5 @@ + else + say("\nAccount created successfully with username #{new_user.username}") + end ++ new_user.activate +end From 361fdfd7278fce21b5ecbd61f7aa1f9e2dcb847d Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 22 May 2024 14:55:09 +0100 Subject: [PATCH 17/88] fix --- discourse_rock/rockcraft.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 708f2d16..609e4427 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -186,6 +186,7 @@ parts: after: [discourse, patches] override-stage: | git -C srv/discourse/app apply patches/lp1903695.patch + touch srv/discourse/app/lib/tasks/discourse-charm.rake git -C srv/discourse/app apply patches/discourse-charm.patch git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation From ac18402973922dcecd95e372bb86fcbe0cb5df23 Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 23 May 2024 12:40:30 +0100 Subject: [PATCH 18/88] using admin:create instead --- actions.yaml | 10 +++ discourse_rock/patches/discourse-charm.patch | 84 ++------------------ src/charm.py | 44 ++++++---- tests/unit/test_charm.py | 6 +- 4 files changed, 49 insertions(+), 95 deletions(-) diff --git a/actions.yaml b/actions.yaml index bec71e88..e938d558 100644 --- a/actions.yaml +++ b/actions.yaml @@ -10,6 +10,16 @@ add-admin-user: type: string description: User password. required: [email, password] +add-user: + description: Add a new regular user. + params: + email: + type: string + description: User email. + password: + type: string + description: User password. + required: [email, password] anonymize-user: description: Anonymize a user. params: diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index bf72b64a..568750f3 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -1,88 +1,18 @@ --- a/lib/tasks/discourse-charm.rake +++ b/lib/tasks/discourse-charm.rake -@@ -0,0 +1,84 @@ +@@ -0,0 +1,15 @@ +# frozen_string_literal: true + -+desc "Promote a user to admin" -+task "admin:promote", [:email] => [:environment] do |_, args| -+ email = args[:email] ++desc "Check if a user exists for given email address" ++task "users:exists", [:email] => [:environment] do |_, args| ++email = args[:email] + if !email || email !~ /@/ -+ puts "ERROR: Expecting rake admin:promote[some@email.com]" ++ puts "ERROR: Expecting rake users:exists[some@email.com]" + exit 1 + end -+ admin = User.find_by_email(email) -+ if admin.nil? ++ if User.find_by_email(email).nil? + puts "ERROR: User with email #{email} not found" + exit 1 + end -+ admin.grant_admin! -+ admin.change_trust_level!(1) if admin.trust_level < 1 -+ admin.email_tokens.update_all confirmed: true -+ admin.activate -+ say("\nYour account now has Admin privileges!") -+end -+ -+desc "Create the user for given credentials" -+task "users:create" => :environment do -+ require "highline/import" -+ -+ begin -+ email = ask("Email: ") -+ existing_user = User.find_by_email(email) -+ -+ # check if user account already exists -+ if existing_user -+ # user already exists, ask for password reset -+ new_user = existing_user -+ reset_password = -+ ask( -+ "User with this email already exists! Do you want to reset the password for this email? (Y/n) ", -+ ) -+ if (reset_password == "" || reset_password.downcase == "y") -+ begin -+ password = ask("Password: ") { |q| q.echo = false } -+ password_confirmation = ask("Repeat password: ") { |q| q.echo = false } -+ passwords_match = password == password_confirmation -+ -+ say("Passwords don't match, try again...") unless passwords_match -+ end while !passwords_match -+ new_user.password = password -+ end -+ else -+ # create new user -+ new_user = User.new -+ new_user.email = email -+ new_user.username = UserNameSuggester.suggest(new_user.email) -+ begin -+ if ENV["RANDOM_PASSWORD"] == "1" -+ password = password_confirmation = SecureRandom.hex -+ else -+ password = ask("Password: ") { |q| q.echo = false } -+ password_confirmation = ask("Repeat password: ") { |q| q.echo = false } -+ end -+ -+ passwords_match = password == password_confirmation -+ -+ say("Passwords don't match, try again...") unless passwords_match -+ end while !passwords_match -+ new_user.password = password -+ end -+ -+ new_user.name = ask("Full name: ") if SiteSetting.full_name_required && new_user.name.blank? -+ -+ # save/update user account -+ saved = new_user.save -+ say(new_user.errors.full_messages.join("\n")) unless saved -+ end while !saved -+ -+ say "\nEnsuring account is active!" -+ new_user.active = true -+ new_user.save -+ -+ if existing_user -+ say("\nAccount updated successfully!") -+ else -+ say("\nAccount created successfully with username #{new_user.username}") -+ end -+ new_user.activate ++ puts "User with email #{email} exists" +end diff --git a/src/charm.py b/src/charm.py index 7467765c..bc70db06 100755 --- a/src/charm.py +++ b/src/charm.py @@ -702,27 +702,35 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: event: Event triggering the add_admin_user action. """ container = self.unit.get_container(CONTAINER_NAME) - - email = event.params["email"] - if not container.can_connect(): event.fail("Unable to connect to container, container is not ready") return - if not self._create_user(event.params["email"], event.params["password"]): - event.fail(f"Failed to create user with email {email}") - return + + email = event.params["email"] + + user_exists = container.exec( + [os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", "users:exists", email], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + ) + try: + user_exists.wait_output() + except ExecError: + self._on_add_user_action(event) process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", - "admin:promote", - email, + "admin:create", ], + stdin=f"{email}\nn\nY\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), + timeout=60, ) try: process.wait_output() @@ -732,22 +740,28 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore ) - def _create_user(self, email: str, password: str) -> bool: + def _on_add_user_action(self, event: ActionEvent): """Create a new user in Discourse. Args: - event: Event triggering the create user action. + event: Event triggering the add_user action. """ container = self.unit.get_container(CONTAINER_NAME) + if not container.can_connect(): + event.fail("Unable to connect to container, container is not ready") + return + + email = event.params["email"] + password = event.params["password"] process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", - "users:create", + "admin:create", ], - stdin=f"{email}\n{password}\n", + stdin=f"{email}\n{password}\n{password}\nN\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), @@ -755,9 +769,9 @@ def _create_user(self, email: str, password: str) -> bool: ) try: process.wait_output() - except ExecError: - return False - return True + event.set_results(email) + except ExecError as ex: + event.fail(f"Failed to make user with email {email}: {ex.stdout}") # type: ignore def _config_force_https(self) -> None: """Config Discourse to force_https option based on charm configuration.""" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index fa936d53..30b1721f 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -330,7 +330,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: args.environment != harness.charm._create_discourse_environment_settings() or args.working_dir != DISCOURSE_PATH or args.user != "_daemon_" - or args.stdin != f"{email}\n{password}\n{password}\nY\n" + or args.stdin != f"{email}\nn\nY\n" or args.timeout != 60 ): raise ValueError(f"{args.command} wasn't made with the correct args.") @@ -373,7 +373,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: args.environment != harness.charm._create_discourse_environment_settings() or args.working_dir != DISCOURSE_PATH or args.user != "_daemon_" - or args.stdin != f"{email}\n{password}\n" + or args.stdin != f"{email}\n{password}\n{password}\nN\n" or args.timeout != 60 ): raise ValueError(f"{args.command} wasn't made with the correct args.") @@ -393,7 +393,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: "email": email, "password": password, } - assert charm._create_user(email, password) + charm._on_add_user_action(event) def test_anonymize_user(): From e922412182fcd445b9d0b5de58961a8bc15dc4bb Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 23 May 2024 20:19:17 +0100 Subject: [PATCH 19/88] relevant changes --- actions.yaml | 21 ++++------ discourse_rock/patches/discourse-charm.patch | 2 +- src-docs/charm.py.md | 2 +- src/charm.py | 44 ++++++++++++++++---- tests/integration/conftest.py | 3 +- tests/unit/test_charm.py | 17 +++----- 6 files changed, 56 insertions(+), 33 deletions(-) diff --git a/actions.yaml b/actions.yaml index e938d558..f693fd2c 100644 --- a/actions.yaml +++ b/actions.yaml @@ -1,29 +1,26 @@ # Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. -add-admin-user: - description: Add a new admin user. +promote-user: + description: Promote a user to admin. params: email: type: string description: User email. - password: - type: string - description: User password. - required: [email, password] -add-user: - description: Add a new regular user. + required: [email] +create-user: + description: Create a new user. params: email: type: string description: User email. - password: - type: string - description: User password. - required: [email, password] + required: [email] anonymize-user: description: Anonymize a user. params: username: type: string description: The unique identifier of the user to anonymize. + admin: + type: boolean + description: Whether the user should be an admin. required: [username] diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index 568750f3..409cdd25 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -10,7 +10,7 @@ + puts "ERROR: Expecting rake users:exists[some@email.com]" + exit 1 + end -+ if User.find_by_email(email).nil? ++ if User.find_by_email(email) + puts "ERROR: User with email #{email} not found" + exit 1 + end diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index 44d3f51f..a74aec10 100644 --- a/src-docs/charm.py.md +++ b/src-docs/charm.py.md @@ -28,7 +28,7 @@ Charm for Discourse on kubernetes. ## class `DiscourseCharm` Charm for Discourse on kubernetes. - + ### function `__init__` diff --git a/src/charm.py b/src/charm.py index bc70db06..2520f3d8 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,6 +7,8 @@ import hashlib import logging import os.path +import random +import string import typing from collections import defaultdict, namedtuple @@ -117,7 +119,8 @@ def __init__(self, *args): self.framework.observe(self.on.upgrade_charm, self._on_upgrade_charm) self.framework.observe(self.on.discourse_pebble_ready, self._on_discourse_pebble_ready) self.framework.observe(self.on.config_changed, self._on_config_changed) - self.framework.observe(self.on.add_admin_user_action, self._on_add_admin_user_action) + self.framework.observe(self.on.promote_user_action, self._on_promote_user_action) + self.framework.observe(self.on.create_user_action, self._on_create_user_action) self.framework.observe(self.on.anonymize_user_action, self._on_anonymize_user_action) self.redis = RedisRequires(self, self._stored) @@ -695,8 +698,8 @@ def _activate_charm(self) -> None: self._start_service() self.model.unit.status = ActiveStatus() - def _on_add_admin_user_action(self, event: ActionEvent) -> None: - """Add a new admin user to Discourse. + def _on_promote_user_action(self, event: ActionEvent) -> None: + """Promote a user to a specific trust level. Args: event: Event triggering the add_admin_user action. @@ -717,7 +720,8 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: try: user_exists.wait_output() except ExecError: - self._on_add_user_action(event) + event.fail(f"User with email {email} does not exist") + return process = container.exec( [ @@ -740,7 +744,7 @@ def _on_add_admin_user_action(self, event: ActionEvent) -> None: f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore ) - def _on_add_user_action(self, event: ActionEvent): + def _on_create_user_action(self, event: ActionEvent): """Create a new user in Discourse. Args: @@ -752,7 +756,19 @@ def _on_add_user_action(self, event: ActionEvent): return email = event.params["email"] - password = event.params["password"] + password = self._generate_password(16) + + user_exists = container.exec( + [os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", "users:exists", email], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + ) + try: + user_exists.wait_output() + event.fail(f"User with email {email} already exists") + except ExecError: + pass process = container.exec( [ @@ -769,9 +785,23 @@ def _on_add_user_action(self, event: ActionEvent): ) try: process.wait_output() - event.set_results(email) + event.set_results({"email": email, "password": password}) except ExecError as ex: event.fail(f"Failed to make user with email {email}: {ex.stdout}") # type: ignore + + if event.params.get("admin") == True: + self._on_promote_user_action(event) + + def _generate_password(self, length: int) -> str: + """Generate a random password. + + Args: + length: Length of the password to generate. + + Returns: + Random password. + """ + return "".join(random.choices(string.ascii_letters + string.digits, k=length)) def _config_force_https(self) -> None: """Config Discourse to force_https option based on charm configuration.""" diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 6bf8e417..dc30cafd 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -246,9 +246,10 @@ async def admin_credentials_fixture(app: Application) -> types.Credentials: password = secrets.token_urlsafe(16) discourse_unit: Unit = app.units[0] action: Action = await discourse_unit.run_action( - "add-admin-user", email=email, password=password + "create-user", email=email, admin=True ) await action.wait() + password = action.results["password"] admin_credentials = types.Credentials( email=email, username=email.split("@", maxsplit=1)[0], password=password ) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 30b1721f..27561088 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -310,10 +310,10 @@ def test_db_relation(): ), "database name should be set after relation joined" -def test_add_admin_user(): +def test_promote_user(): """ arrange: an email and a password - act: when the _on_add_admin_user_action method is executed + act: when the _on_promote_user_action method is executed assert: the underlying rake command to add the user is executed with the appropriate parameters. """ @@ -344,19 +344,17 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) email = "sample@email.com" - password = "somepassword" # nosec event = MagicMock(spec=ActionEvent) event.params = { "email": email, - "password": password, } - charm._on_add_admin_user_action(event) + charm._on_promote_user_action(event) -def test_add_user(): +def test_create_user(): """ arrange: an email and a password - act: when the _on_add_user_action method is executed + act: when the _on_create_user_action method is executed assert: the underlying rake command to add the user is executed with the appropriate parameters. """ @@ -373,7 +371,6 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: args.environment != harness.charm._create_discourse_environment_settings() or args.working_dir != DISCOURSE_PATH or args.user != "_daemon_" - or args.stdin != f"{email}\n{password}\n{password}\nN\n" or args.timeout != 60 ): raise ValueError(f"{args.command} wasn't made with the correct args.") @@ -387,13 +384,11 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) email = "sample@email.com" - password = "somepassword" # nosec event = MagicMock(spec=ActionEvent) event.params = { "email": email, - "password": password, } - charm._on_add_user_action(event) + charm._on_create_user_action(event) def test_anonymize_user(): From 00720d467ef5f61240050af96faf2a53fd8e80de Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 23 May 2024 20:20:36 +0100 Subject: [PATCH 20/88] removed unnecessary line --- tests/integration/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index dc30cafd..4c50c706 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -243,7 +243,6 @@ async def setup_saml_config(app: Application, model: Model): async def admin_credentials_fixture(app: Application) -> types.Credentials: """Admin user credentials.""" email = f"admin-user{secrets.randbits(32)}@test.internal" - password = secrets.token_urlsafe(16) discourse_unit: Unit = app.units[0] action: Action = await discourse_unit.run_action( "create-user", email=email, admin=True From bfbff5e05fd28f1d038921a986a7c1189c3f2102 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 24 May 2024 01:16:17 +0100 Subject: [PATCH 21/88] minor fixes --- discourse_rock/patches/discourse-charm.patch | 2 +- src/charm.py | 8 ++++---- tests/integration/conftest.py | 4 +--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index 409cdd25..09fa44c7 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -5,7 +5,7 @@ + +desc "Check if a user exists for given email address" +task "users:exists", [:email] => [:environment] do |_, args| -+email = args[:email] ++ email = args[:email] + if !email || email !~ /@/ + puts "ERROR: Expecting rake users:exists[some@email.com]" + exit 1 diff --git a/src/charm.py b/src/charm.py index 2520f3d8..565b1232 100755 --- a/src/charm.py +++ b/src/charm.py @@ -770,6 +770,9 @@ def _on_create_user_action(self, event: ActionEvent): except ExecError: pass + # Admin flag is optional, if it is true, the user will be created as an admin + admin_flag = "Y" if event.params.get("admin") else "N" + process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), @@ -777,7 +780,7 @@ def _on_create_user_action(self, event: ActionEvent): "rake", "admin:create", ], - stdin=f"{email}\n{password}\n{password}\nN\n", + stdin=f"{email}\n{password}\n{password}\n{admin_flag}\n", working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), @@ -788,9 +791,6 @@ def _on_create_user_action(self, event: ActionEvent): event.set_results({"email": email, "password": password}) except ExecError as ex: event.fail(f"Failed to make user with email {email}: {ex.stdout}") # type: ignore - - if event.params.get("admin") == True: - self._on_promote_user_action(event) def _generate_password(self, length: int) -> str: """Generate a random password. diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 4c50c706..8526cd75 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -244,9 +244,7 @@ async def admin_credentials_fixture(app: Application) -> types.Credentials: """Admin user credentials.""" email = f"admin-user{secrets.randbits(32)}@test.internal" discourse_unit: Unit = app.units[0] - action: Action = await discourse_unit.run_action( - "create-user", email=email, admin=True - ) + action: Action = await discourse_unit.run_action("create-user", email=email, admin=True) await action.wait() password = action.results["password"] admin_credentials = types.Credentials( From 8cd79e3eaaa6e3d00fc32e06867f12e3a15c349e Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 24 May 2024 02:05:49 +0100 Subject: [PATCH 22/88] fixed event set results typo --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 565b1232..86f81ca7 100755 --- a/src/charm.py +++ b/src/charm.py @@ -738,7 +738,7 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: ) try: process.wait_output() - event.set_results(email) + event.set_results({"email": email}) except ExecError as ex: event.fail( f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore From 68c90e459e7a74a12c376f818c5325653b63ac40 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 24 May 2024 10:14:02 +0100 Subject: [PATCH 23/88] using secrets instead of random --- src/charm.py | 6 ++++-- tests/integration/test_charm.py | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/charm.py b/src/charm.py index 86f81ca7..8c9bf7bf 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,7 +7,7 @@ import hashlib import logging import os.path -import random +import secrets import string import typing from collections import defaultdict, namedtuple @@ -801,7 +801,9 @@ def _generate_password(self, length: int) -> str: Returns: Random password. """ - return "".join(random.choices(string.ascii_letters + string.digits, k=length)) + choices = string.ascii_letters + string.digits + password = "".join([secrets.choice(choices) for _ in range(length)]) + return password def _config_force_https(self) -> None: """Config Discourse to force_https option based on charm configuration.""" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index b8f4f6d9..c72bbb51 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -251,10 +251,10 @@ async def test_saml_login( # pylint: disable=too-many-locals,too-many-arguments # username can't be "discourse" or it will be renamed username = "ubuntu" email = "ubuntu@canonical.com" - password = "test-discourse-k8s-password" # nosec + password = "test-discourse-k8s-password" # nosecue saml_helper.register_user(username=username, email=email, password=password) - action_result = await run_action(app.name, "add-admin-user", email=email, password=password) + action_result = await run_action(app.name, "create-user", email=email, admin=True) assert "user" in action_result host = app.name From 0010a60e2c03e4a3d47e0c61819671e158f885c8 Mon Sep 17 00:00:00 2001 From: Brendan Bell Date: Sat, 25 May 2024 00:42:56 +0100 Subject: [PATCH 24/88] replace email with user --- src/charm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index 8c9bf7bf..6da67933 100755 --- a/src/charm.py +++ b/src/charm.py @@ -738,7 +738,7 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: ) try: process.wait_output() - event.set_results({"email": email}) + event.set_results({"user": email}) except ExecError as ex: event.fail( f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore @@ -788,7 +788,7 @@ def _on_create_user_action(self, event: ActionEvent): ) try: process.wait_output() - event.set_results({"email": email, "password": password}) + event.set_results({"user": email, "password": password}) except ExecError as ex: event.fail(f"Failed to make user with email {email}: {ex.stdout}") # type: ignore From 13b2d2b8937a8f7f16ff8acbed0a8bd6f4db1045 Mon Sep 17 00:00:00 2001 From: codethulu Date: Sun, 26 May 2024 15:18:55 +0100 Subject: [PATCH 25/88] blank --- config.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/config.yaml b/config.yaml index af253c3d..08e316c3 100644 --- a/config.yaml +++ b/config.yaml @@ -17,6 +17,10 @@ options: type: string description: "External hostname this discourse instance responds to. Defaults to application name." default: "" + force_https: + type: boolean + description: "Force https for smtp connections." + default: true force_saml_login: type: boolean description: "Force SAML login (full screen, no local database logins)." @@ -41,10 +45,6 @@ options: type: string description: "Enable TLS encryption for smtp connections." default: "true" - smtp_force_https: - type: boolean - description: "Force https for smtp connections." - default: true smtp_force_tls: type: string description: "Force implicit TLS as per RFC 8314 3.3." From bc9414600462bf243ebb7274d17b351890ae0690 Mon Sep 17 00:00:00 2001 From: codethulu Date: Sun, 26 May 2024 17:30:10 +0100 Subject: [PATCH 26/88] changed config --- config.yaml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/config.yaml b/config.yaml index 08e316c3..a452f7c8 100644 --- a/config.yaml +++ b/config.yaml @@ -19,7 +19,7 @@ options: default: "" force_https: type: boolean - description: "Force https for smtp connections." + description: Configure Discourse to use https. default: true force_saml_login: type: boolean @@ -138,7 +138,3 @@ options: type: string description: "Throttle level - blocks excessive usage by ip. Accepted values: none, permissive, strict." default: none - force_https: - type: boolean - description: Configure Discourse to use https. - default: false From df3ef14dc6165efb90c04f2b54127daacb1689eb Mon Sep 17 00:00:00 2001 From: Brendan Bell Date: Mon, 27 May 2024 09:46:23 +0100 Subject: [PATCH 27/88] Update config.yaml --- config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.yaml b/config.yaml index a452f7c8..446ced93 100644 --- a/config.yaml +++ b/config.yaml @@ -20,7 +20,7 @@ options: force_https: type: boolean description: Configure Discourse to use https. - default: true + default: false force_saml_login: type: boolean description: "Force SAML login (full screen, no local database logins)." From be80aae9886ccf1607ed445418d6449ec0cb1e20 Mon Sep 17 00:00:00 2001 From: Brendan Bell Date: Mon, 27 May 2024 09:48:42 +0100 Subject: [PATCH 28/88] Update conftest.py --- tests/integration/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 8526cd75..0d2a935a 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -230,11 +230,11 @@ async def setup_saml_config(app: Application, model: Model): discourse_app = model.applications[app.name] original_config: dict = await discourse_app.get_config() original_config = {k: v["value"] for k, v in original_config.items()} - await discourse_app.set_config({"force_https": "true"}) + await discourse_app.set_config({"force_https": true}) yield await discourse_app.set_config( { - "force_https": str(original_config["force_https"]).lower(), + "force_https": original_config["force_https"], } ) From d95c61c5372f88e5416ddfe21c5140f763ea39f3 Mon Sep 17 00:00:00 2001 From: Brendan Bell Date: Mon, 27 May 2024 09:51:06 +0100 Subject: [PATCH 29/88] Update conftest.py --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 0d2a935a..3ce09549 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -230,7 +230,7 @@ async def setup_saml_config(app: Application, model: Model): discourse_app = model.applications[app.name] original_config: dict = await discourse_app.get_config() original_config = {k: v["value"] for k, v in original_config.items()} - await discourse_app.set_config({"force_https": true}) + await discourse_app.set_config({"force_https": True}) yield await discourse_app.set_config( { From fb1a9db855e7719692d76d4e74ffd787683655d4 Mon Sep 17 00:00:00 2001 From: codethulu Date: Tue, 28 May 2024 10:10:57 +0100 Subject: [PATCH 30/88] updat3ed ops --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index eb63cb92..7c62f548 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,2 @@ -ops==2.12.0 +ops==2.13.0 pydantic==2.7.1 From 4c43eaf3132f21e97827532dc877dac60f269111 Mon Sep 17 00:00:00 2001 From: codethulu Date: Tue, 28 May 2024 12:03:31 +0100 Subject: [PATCH 31/88] refactor test --- src/charm.py | 6 ++++-- tests/integration/conftest.py | 28 ++++++++++++++++++++++++++-- tests/integration/test_charm.py | 21 +-------------------- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/charm.py b/src/charm.py index 6da67933..08bc1f96 100755 --- a/src/charm.py +++ b/src/charm.py @@ -242,7 +242,9 @@ def _get_external_hostname(self) -> str: The site hostname defined as part of the site_url configuration or a default value. """ return ( - self.config["external_hostname"] if self.config["external_hostname"] else self.app.name + str(self.config["external_hostname"]) + if self.config["external_hostname"] + else self.app.name ) def _is_setup_completed(self) -> bool: @@ -333,7 +335,7 @@ def _get_saml_config(self) -> typing.Dict[str, typing.Any]: "true" if self.config["force_saml_login"] else "false" ) saml_sync_groups = [ - x.strip() for x in self.config["saml_sync_groups"].split(",") if x.strip() + x.strip() for x in str(self.config["saml_sync_groups"]).split(",") if x.strip() ] if saml_sync_groups: # Per https://github.com/discourse/discourse-saml setting this to `true` diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 3ce09549..711fa2cb 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -18,6 +18,7 @@ from ops.model import WaitingStatus from pytest import Config, fixture from pytest_operator.plugin import Model, OpsTest +from saml_test_helper import SamlK8sTestHelper # pylint: disable=import-error from . import types @@ -230,13 +231,36 @@ async def setup_saml_config(app: Application, model: Model): discourse_app = model.applications[app.name] original_config: dict = await discourse_app.get_config() original_config = {k: v["value"] for k, v in original_config.items()} - await discourse_app.set_config({"force_https": True}) + await discourse_app.set_config({"force_https": "true"}) yield + saml_helper = SamlK8sTestHelper.deploy_saml_idp(model.name) + saml_app: Application = await model.deploy( + "saml-integrator", + channel="latest/edge", + series="jammy", + trust=True, + ) + await model.wait_for_idle() + saml_helper.prepare_pod(model.name, f"{saml_app.name}-0") + saml_helper.prepare_pod(model.name, f"{app.name}-0") + await model.wait_for_idle() + await saml_app.set_config( # type: ignore[attr-defined] + { + "entity_id": saml_helper.entity_id, + "metadata_url": saml_helper.metadata_url, + } + ) + await model.add_relation(app.name, "saml-integrator") + await model.wait_for_idle() + yield saml_helper await discourse_app.set_config( { - "force_https": original_config["force_https"], + "force_https": str(original_config["force_https"]).lower(), } ) + await app.destroy_relation(app.name, "saml-integrator") + await model.applications["saml-integrator"].destroy() + await model.wait_for_idle() @pytest_asyncio.fixture(scope="module", name="admin_credentials") diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index c72bbb51..926d11bb 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -220,32 +220,13 @@ async def test_saml_login( # pylint: disable=too-many-locals,too-many-arguments app: Application, requests_timeout: int, run_action, - model: Model, + saml_helper: SamlK8sTestHelper, ): """ arrange: after discourse charm has been deployed, with all required relation established. act: add an admin user and enable force-https mode. assert: user can login discourse using SAML Authentication. """ - saml_helper = SamlK8sTestHelper.deploy_saml_idp(model.name) - saml_app: Application = await model.deploy( - "saml-integrator", - channel="latest/edge", - series="jammy", - trust=True, - ) - await model.wait_for_idle() - saml_helper.prepare_pod(model.name, f"{saml_app.name}-0") - saml_helper.prepare_pod(model.name, f"{app.name}-0") - await model.wait_for_idle() - await saml_app.set_config( # type: ignore[attr-defined] - { - "entity_id": saml_helper.entity_id, - "metadata_url": saml_helper.metadata_url, - } - ) - await model.add_relation(app.name, "saml-integrator") - await model.wait_for_idle() urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) # discourse need a long password and a valid email # username can't be "discourse" or it will be renamed From bfd05e567ac2a5e6a0547ab4d6c703c697e3fe50 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 29 May 2024 11:25:01 +0100 Subject: [PATCH 32/88] resolved --- docs/how-to/contribute.md | 3 +-- localstack-installation.sh | 0 tests/integration/conftest.py | 3 ++- tests/integration/test_charm.py | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) mode change 100644 => 100755 localstack-installation.sh diff --git a/docs/how-to/contribute.md b/docs/how-to/contribute.md index 4bbe44bc..052aa594 100644 --- a/docs/how-to/contribute.md +++ b/docs/how-to/contribute.md @@ -83,8 +83,7 @@ the registry: ```shell cd [project_dir]/discourse_rock && rockcraft pack rockcraft.yaml - skopeo --insecure-policy copy oci-archive:discourse_1.0_amd64.rock docker-daemon:localhost:32000/discourse:latest - docker push localhost:32000/discourse:latest + skopeo --insecure-policy copy --dest-tls-verify=false oci-archive:discourse_1.0_amd64.rock docker://localhost:32000/discourse:latest ``` ### Deploy diff --git a/localstack-installation.sh b/localstack-installation.sh old mode 100644 new mode 100755 diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 01b1d8a7..dce052a7 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -232,7 +232,7 @@ async def setup_saml_config(app: Application, model: Model): original_config: dict = await discourse_app.get_config() original_config = {k: v["value"] for k, v in original_config.items()} await discourse_app.set_config({"force_https": "true"}) - yield + saml_helper = SamlK8sTestHelper.deploy_saml_idp(model.name) saml_app: Application = await model.deploy( "saml-integrator", @@ -252,6 +252,7 @@ async def setup_saml_config(app: Application, model: Model): ) await model.add_relation(app.name, "saml-integrator") await model.wait_for_idle() + yield saml_helper diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 6e149a13..c5d6e38f 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -220,13 +220,14 @@ async def test_saml_login( # pylint: disable=too-many-locals,too-many-arguments app: Application, requests_timeout: int, run_action, - saml_helper: SamlK8sTestHelper, + setup_saml_config, ): """ arrange: after discourse charm has been deployed, with all required relation established. act: add an admin user and enable force-https mode. assert: user can login discourse using SAML Authentication. """ + saml_helper = setup_saml_config urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) # discourse need a long password and a valid email # username can't be "discourse" or it will be renamed From 14a94b1f793ce3e532c55af2f8f8c46c39bf0b5d Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 29 May 2024 11:44:44 +0100 Subject: [PATCH 33/88] refactored test --- tests/integration/test_charm.py | 91 --------------------------------- 1 file changed, 91 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index c5d6e38f..3b6735dc 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -5,21 +5,17 @@ import logging import re -import socket -import unittest.mock from datetime import datetime, timedelta from pathlib import Path from typing import Dict import pytest import requests -import urllib3.exceptions from boto3 import client from botocore.config import Config from juju.application import Application from ops.model import ActiveStatus from pytest_operator.plugin import Model, OpsTest -from saml_test_helper import SamlK8sTestHelper # pylint: disable=import-error from charm import PROMETHEUS_PORT @@ -213,93 +209,6 @@ async def test_create_category( assert category["color"] == category_info["color"] -@pytest.mark.asyncio -@pytest.mark.abort_on_fail -@pytest.mark.usefixtures("setup_saml_config") -async def test_saml_login( # pylint: disable=too-many-locals,too-many-arguments - app: Application, - requests_timeout: int, - run_action, - setup_saml_config, -): - """ - arrange: after discourse charm has been deployed, with all required relation established. - act: add an admin user and enable force-https mode. - assert: user can login discourse using SAML Authentication. - """ - saml_helper = setup_saml_config - urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) - # discourse need a long password and a valid email - # username can't be "discourse" or it will be renamed - username = "ubuntu" - email = "ubuntu@canonical.com" - password = "test-discourse-k8s-password" # nosecue - saml_helper.register_user(username=username, email=email, password=password) - - action_result = await run_action(app.name, "create-user", email=email, admin=True) - assert "user" in action_result - - host = app.name - original_getaddrinfo = socket.getaddrinfo - - def patched_getaddrinfo(*args): - if args[0] == host: - return original_getaddrinfo("127.0.0.1", *args[1:]) - return original_getaddrinfo(*args) - - with unittest.mock.patch.multiple(socket, getaddrinfo=patched_getaddrinfo): - session = requests.session() - - response = session.get( - f"https://{host}/auth/saml/metadata", - verify=False, - timeout=10, - ) - saml_helper.register_service_provider(name=host, metadata=response.text) - - preference_page = session.get( - f"https://{host}/u/{username}/preferences/account", - verify=False, - timeout=requests_timeout, - ) - assert preference_page.status_code == 404 - - session.get(f"https://{host}", verify=False) - response = session.get( - f"https://{host}/session/csrf", - verify=False, - headers={"Accept": "application/json"}, - timeout=requests_timeout, - ) - csrf_token = response.json()["csrf"] - redirect_response = session.post( - f"https://{host}/auth/saml", - data={"authenticity_token": csrf_token}, - verify=False, - timeout=requests_timeout, - allow_redirects=False, - ) - assert redirect_response.status_code == 302 - redirect_url = redirect_response.headers["Location"] - saml_response = saml_helper.redirect_sso_login( - redirect_url, username=username, password=password - ) - assert f"https://{host}" in saml_response.url - session.post( - saml_response.url, - verify=False, - data={"SAMLResponse": saml_response.data["SAMLResponse"], "SameSite": "1"}, - ) - session.post(saml_response.url, verify=False, data=saml_response.data) - - preference_page = session.get( - f"https://{host}/u/{username}/preferences/account", - verify=False, - timeout=requests_timeout, - ) - assert preference_page.status_code == 200 - - @pytest.mark.asyncio async def test_serve_compiled_assets( discourse_address: str, From 9419b100f7c91b394da68a8482c55819523b2fb1 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 29 May 2024 11:53:00 +0100 Subject: [PATCH 34/88] added saml testing module --- tests/integration/test_saml.py | 102 +++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 tests/integration/test_saml.py diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py new file mode 100644 index 00000000..ffbff78b --- /dev/null +++ b/tests/integration/test_saml.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Discourse integration tests.""" + +import logging +import socket +import unittest.mock + +import pytest +import requests +import urllib3.exceptions +from juju.application import Application # pylint: disable=import-error + +logger = logging.getLogger(__name__) + + +@pytest.mark.asyncio +@pytest.mark.abort_on_fail +@pytest.mark.usefixtures("setup_saml_config") +async def test_saml_login( # pylint: disable=too-many-locals,too-many-arguments + app: Application, + requests_timeout: int, + run_action, + setup_saml_config, +): + """ + arrange: after discourse charm has been deployed, with all required relation established. + act: add an admin user and enable force-https mode. + assert: user can login discourse using SAML Authentication. + """ + saml_helper = setup_saml_config + urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) + # discourse need a long password and a valid email + # username can't be "discourse" or it will be renamed + username = "ubuntu" + email = "ubuntu@canonical.com" + password = "test-discourse-k8s-password" # nosecue + saml_helper.register_user(username=username, email=email, password=password) + + action_result = await run_action(app.name, "create-user", email=email, admin=True) + assert "user" in action_result + + host = app.name + original_getaddrinfo = socket.getaddrinfo + + def patched_getaddrinfo(*args): + if args[0] == host: + return original_getaddrinfo("127.0.0.1", *args[1:]) + return original_getaddrinfo(*args) + + with unittest.mock.patch.multiple(socket, getaddrinfo=patched_getaddrinfo): + session = requests.session() + + response = session.get( + f"https://{host}/auth/saml/metadata", + verify=False, + timeout=10, + ) + saml_helper.register_service_provider(name=host, metadata=response.text) + + preference_page = session.get( + f"https://{host}/u/{username}/preferences/account", + verify=False, + timeout=requests_timeout, + ) + assert preference_page.status_code == 404 + + session.get(f"https://{host}", verify=False) + response = session.get( + f"https://{host}/session/csrf", + verify=False, + headers={"Accept": "application/json"}, + timeout=requests_timeout, + ) + csrf_token = response.json()["csrf"] + redirect_response = session.post( + f"https://{host}/auth/saml", + data={"authenticity_token": csrf_token}, + verify=False, + timeout=requests_timeout, + allow_redirects=False, + ) + assert redirect_response.status_code == 302 + redirect_url = redirect_response.headers["Location"] + saml_response = saml_helper.redirect_sso_login( + redirect_url, username=username, password=password + ) + assert f"https://{host}" in saml_response.url + session.post( + saml_response.url, + verify=False, + data={"SAMLResponse": saml_response.data["SAMLResponse"], "SameSite": "1"}, + ) + session.post(saml_response.url, verify=False, data=saml_response.data) + + preference_page = session.get( + f"https://{host}/u/{username}/preferences/account", + verify=False, + timeout=requests_timeout, + ) + assert preference_page.status_code == 200 From 3e4fa461b524c5c6a7097579d1dc908106c87d57 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 29 May 2024 13:43:57 +0100 Subject: [PATCH 35/88] parameters --- actions.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/actions.yaml b/actions.yaml index f693fd2c..4e7b0e8e 100644 --- a/actions.yaml +++ b/actions.yaml @@ -13,6 +13,9 @@ create-user: email: type: string description: User email. + admin: + type: boolean + description: Whether the user should be an admin. required: [email] anonymize-user: description: Anonymize a user. From db9d82960276271c6a21d020683a881e5fba56b2 Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 30 May 2024 13:36:07 +0100 Subject: [PATCH 36/88] updated workflow --- .github/workflows/integration_test.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 489bd5a0..ebd40e93 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -13,5 +13,6 @@ jobs: trivy-image-config: "trivy.yaml" juju-channel: 3.1/stable channel: 1.28-strict/stable + modules: '["test_charm", "test_saml"]' self-hosted-runner: true self-hosted-runner-label: "edge" From 8933ba4f22507c866621461ceacc0ce1c758f58d Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 30 May 2024 13:41:05 +0100 Subject: [PATCH 37/88] updated descriptions --- src/charm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index 2c5e718b..6290b9ae 100755 --- a/src/charm.py +++ b/src/charm.py @@ -706,7 +706,7 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: """Promote a user to a specific trust level. Args: - event: Event triggering the add_admin_user action. + event: Event triggering the promote_user action. """ container = self.unit.get_container(CONTAINER_NAME) if not container.can_connect(): @@ -752,7 +752,7 @@ def _on_create_user_action(self, event: ActionEvent): """Create a new user in Discourse. Args: - event: Event triggering the add_user action. + event: Event triggering the create_user action. """ container = self.unit.get_container(CONTAINER_NAME) if not container.can_connect(): From 7b44ed2e84290b0e72c1ab6a7f40380efdfc140b Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 30 May 2024 14:27:18 +0100 Subject: [PATCH 38/88] blank --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index ab2c9315..7c62f548 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,2 @@ -ops==2.14.0 +ops==2.13.0 pydantic==2.7.1 From 42f99f3d4665bd6ae2b9a4673fd9d6821d2b88ba Mon Sep 17 00:00:00 2001 From: Brendan Bell Date: Thu, 30 May 2024 16:36:18 +0100 Subject: [PATCH 39/88] Update requirements.txt --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 7c62f548..ab2c9315 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,2 @@ -ops==2.13.0 +ops==2.14.0 pydantic==2.7.1 From 88bcccafb9209796e2c11248e154cf5338950895 Mon Sep 17 00:00:00 2001 From: Brendan Bell Date: Fri, 31 May 2024 14:03:22 +0100 Subject: [PATCH 40/88] Update actions.yaml --- actions.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/actions.yaml b/actions.yaml index 4e7b0e8e..4e1d5445 100644 --- a/actions.yaml +++ b/actions.yaml @@ -23,7 +23,4 @@ anonymize-user: username: type: string description: The unique identifier of the user to anonymize. - admin: - type: boolean - description: Whether the user should be an admin. required: [username] From 97bfa315e7e8553624dc3e34c412b982f8eec28d Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 3 Jun 2024 22:24:08 +0100 Subject: [PATCH 41/88] addressed changes --- discourse_rock/patches/discourse-charm.patch | 7 +-- src/charm.py | 55 ++++++++++++-------- tests/integration/test_charm.py | 25 +++++++++ tests/unit/test_charm.py | 44 ++-------------- 4 files changed, 65 insertions(+), 66 deletions(-) diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index 09fa44c7..c9c35774 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -11,8 +11,9 @@ + exit 1 + end + if User.find_by_email(email) -+ puts "ERROR: User with email #{email} not found" -+ exit 1 ++ puts "User with email #{email} exists" ++ exit 0 + end -+ puts "User with email #{email} exists" ++ puts "ERROR: User with email #{email} not found" ++ exit 2 +end diff --git a/src/charm.py b/src/charm.py index 6290b9ae..0b99d83b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -716,7 +716,7 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: email = event.params["email"] user_exists = container.exec( - [os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", "users:exists", email], + [os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", f"users:exists[{email}]"], working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), @@ -724,6 +724,9 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: try: user_exists.wait_output() except ExecError: + if ex.exit_code == 1: + event.fail(f"Error checking if user with email {email} exists: {ex.stdout}") + return event.fail(f"User with email {email} does not exist") return @@ -763,7 +766,7 @@ def _on_create_user_action(self, event: ActionEvent): password = self._generate_password(16) user_exists = container.exec( - [os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", "users:exists", email], + [os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", f"users:exists[{email}]"], working_dir=DISCOURSE_PATH, user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), @@ -771,7 +774,11 @@ def _on_create_user_action(self, event: ActionEvent): try: user_exists.wait_output() event.fail(f"User with email {email} already exists") - except ExecError: + return + except ExecError as ex: + if ex.exit_code == 1: + event.fail(f"Error checking if user with email {email} exists: {ex.stdout}") + return pass # Admin flag is optional, if it is true, the user will be created as an admin @@ -833,26 +840,30 @@ def _on_anonymize_user_action(self, event: ActionEvent) -> None: """ username = event.params["username"] container = self.unit.get_container("discourse") - if container.can_connect(): - process = container.exec( - [ - "bash", - "-c", - f"./bin/bundle exec rake users:anonymize[{username}]", - ], - working_dir=DISCOURSE_PATH, - user=CONTAINER_APP_USERNAME, - environment=self._create_discourse_environment_settings(), + if not container.can_connect(): + event.fail("Unable to connect to container, container is not ready") + return + + process = container.exec( + [ + os.path.join(DISCOURSE_PATH, "bin/bundle"), + "exec", + "rake", + f"users:anonymize[{username}]", + ], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + ) + try: + process.wait_output() + event.set_results({"user": f"{username}"}) + except ExecError as ex: + event.fail( + # Parameter validation errors are printed to stdout + # Ignore mypy warning when formatting stdout + f"Failed to anonymize user with username {username}:{ex.stdout}" # type: ignore ) - try: - process.wait_output() - event.set_results({"user": f"{username}"}) - except ExecError as ex: - event.fail( - # Parameter validation errors are printed to stdout - # Ignore mypy warning when formatting stdout - f"Failed to anonymize user with username {username}:{ex.stdout}" # type: ignore - ) def _start_service(self): """Start discourse.""" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 3b6735dc..4bc09ef1 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -13,7 +13,9 @@ import requests from boto3 import client from botocore.config import Config +from juju.action import Action from juju.application import Application +from juju.unit import Unit from ops.model import ActiveStatus from pytest_operator.plugin import Model, OpsTest @@ -357,3 +359,26 @@ def _upgrade_finished(): await model.block_until(upgrade_finished(), timeout=10 * 60) check_alive() + + +@pytest.mark.asyncio +async def test_user_creation( + app: Application, +): + """ + arrange: Given discourse application + act: when creating a user + assert: the user should be created successfully. + """ + + email = f"admin-user@test.internal" + discourse_unit: Unit = app.units[0] + action: Action = await discourse_unit.run_action("create-user", email=email) + await action.wait() + assert action.results["user"] == email + + # Re-creating the same user should fail, as the user already exists + email = f"admin-user@test.internal" + action: Action = await discourse_unit.run_action("create-user", email=email) + await action.wait() + assert action.results["user"] == email diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 27561088..08b83173 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -349,46 +349,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: "email": email, } charm._on_promote_user_action(event) - - -def test_create_user(): - """ - arrange: an email and a password - act: when the _on_create_user_action method is executed - assert: the underlying rake command to add the user is executed - with the appropriate parameters. - """ - harness = helpers.start_harness() - - # We catch the exec call that we expect to register it and make sure that the - # args passed to it are correct. - expected_exec_call_was_made = False - - def bundle_handler(args: ops.testing.ExecArgs) -> None: - nonlocal expected_exec_call_was_made - expected_exec_call_was_made = True - if ( - args.environment != harness.charm._create_discourse_environment_settings() - or args.working_dir != DISCOURSE_PATH - or args.user != "_daemon_" - or args.timeout != 60 - ): - raise ValueError(f"{args.command} wasn't made with the correct args.") - - harness.handle_exec( - SERVICE_NAME, - [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "users:create"], - handler=bundle_handler, - ) - - charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) - - email = "sample@email.com" - event = MagicMock(spec=ActionEvent) - event.params = { - "email": email, - } - charm._on_create_user_action(event) + assert expected_exec_call_was_made def test_anonymize_user(): @@ -417,13 +378,14 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: harness.handle_exec( SERVICE_NAME, - ["bash", "-c", f"./bin/bundle exec rake users:anonymize[{username}]"], + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", f"users:anonymize[{username}]"], handler=bundle_handler, ) charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) event = MagicMock(spec=ActionEvent) event.params = {"username": username} charm._on_anonymize_user_action(event) + assert expected_exec_call_was_made def test_handle_pebble_ready_event(): From 879cdeeeb0c3dff54f830e67532a1217ec55b2af Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 3 Jun 2024 22:26:40 +0100 Subject: [PATCH 42/88] make sure action fails --- tests/integration/test_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 4bc09ef1..476271ab 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -381,4 +381,4 @@ async def test_user_creation( email = f"admin-user@test.internal" action: Action = await discourse_unit.run_action("create-user", email=email) await action.wait() - assert action.results["user"] == email + assert action.status == "failed" From 1e0bed25327ccd4aa0b731b16a8d232735301266 Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 3 Jun 2024 22:31:47 +0100 Subject: [PATCH 43/88] linting --- src/charm.py | 2 +- tests/integration/test_charm.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index 0b99d83b..4c001840 100755 --- a/src/charm.py +++ b/src/charm.py @@ -723,7 +723,7 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: ) try: user_exists.wait_output() - except ExecError: + except ExecError as ex: if ex.exit_code == 1: event.fail(f"Error checking if user with email {email} exists: {ex.stdout}") return diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 476271ab..12506c23 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -371,14 +371,14 @@ async def test_user_creation( assert: the user should be created successfully. """ - email = f"admin-user@test.internal" + email = "admin-user@test.internal" discourse_unit: Unit = app.units[0] action: Action = await discourse_unit.run_action("create-user", email=email) await action.wait() assert action.results["user"] == email # Re-creating the same user should fail, as the user already exists - email = f"admin-user@test.internal" + email = "admin-user@test.internal" action: Action = await discourse_unit.run_action("create-user", email=email) await action.wait() assert action.status == "failed" From ba37983caa3659f9ec7130eb5c74e4523f97dc5f Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 3 Jun 2024 22:52:19 +0100 Subject: [PATCH 44/88] linting fix --- tests/integration/test_charm.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 12506c23..6064c751 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -379,6 +379,6 @@ async def test_user_creation( # Re-creating the same user should fail, as the user already exists email = "admin-user@test.internal" - action: Action = await discourse_unit.run_action("create-user", email=email) - await action.wait() - assert action.status == "failed" + break_action: Action = await discourse_unit.run_action("create-user", email=email) + await break_action.wait() + assert break_action.status == "failed" From d70cd8588b89b258c522df1d187f03da11f0b702 Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 3 Jun 2024 22:56:28 +0100 Subject: [PATCH 45/88] linting --- src/charm.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 4c001840..b3edf0da 100755 --- a/src/charm.py +++ b/src/charm.py @@ -779,7 +779,6 @@ def _on_create_user_action(self, event: ActionEvent): if ex.exit_code == 1: event.fail(f"Error checking if user with email {email} exists: {ex.stdout}") return - pass # Admin flag is optional, if it is true, the user will be created as an admin admin_flag = "Y" if event.params.get("admin") else "N" From cb2b7f37e5f63baaf7ec35c801dc255b2c8d298f Mon Sep 17 00:00:00 2001 From: codethulu Date: Tue, 4 Jun 2024 22:16:50 +0100 Subject: [PATCH 46/88] blank --- discourse_rock/patches/discourse-charm.patch | 8 ++------ src/charm.py | 11 +++-------- tests/integration/test_charm.py | 2 +- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index c9c35774..eae201a9 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -1,19 +1,15 @@ --- a/lib/tasks/discourse-charm.rake +++ b/lib/tasks/discourse-charm.rake -@@ -0,0 +1,15 @@ +@@ -0,0 +1,11 @@ +# frozen_string_literal: true + +desc "Check if a user exists for given email address" +task "users:exists", [:email] => [:environment] do |_, args| + email = args[:email] -+ if !email || email !~ /@/ -+ puts "ERROR: Expecting rake users:exists[some@email.com]" -+ exit 1 -+ end + if User.find_by_email(email) + puts "User with email #{email} exists" + exit 0 + end + puts "ERROR: User with email #{email} not found" -+ exit 2 ++ exit 1 +end diff --git a/src/charm.py b/src/charm.py index b3edf0da..3346948a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -723,10 +723,7 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: ) try: user_exists.wait_output() - except ExecError as ex: - if ex.exit_code == 1: - event.fail(f"Error checking if user with email {email} exists: {ex.stdout}") - return + except ExecError: event.fail(f"User with email {email} does not exist") return @@ -775,10 +772,8 @@ def _on_create_user_action(self, event: ActionEvent): user_exists.wait_output() event.fail(f"User with email {email} already exists") return - except ExecError as ex: - if ex.exit_code == 1: - event.fail(f"Error checking if user with email {email} exists: {ex.stdout}") - return + except ExecError: + pass # Admin flag is optional, if it is true, the user will be created as an admin admin_flag = "Y" if event.params.get("admin") else "N" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 6064c751..1829fa86 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -362,7 +362,7 @@ def _upgrade_finished(): @pytest.mark.asyncio -async def test_user_creation( +async def test_create_user( app: Application, ): """ From 5bccdf9db209c00290e7efbe738d91a32c41d874 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 5 Jun 2024 15:19:19 +0100 Subject: [PATCH 47/88] updated rockcraft/patches --- discourse_rock/patches/discourse-charm.patch | 2 +- discourse_rock/rockcraft.yaml | 2 + src/charm.py | 2 - tests/integration/test_charm.py | 33 ++++++++++++++- tests/unit/test_charm.py | 42 ++++++++++++++++++++ 5 files changed, 77 insertions(+), 4 deletions(-) diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index eae201a9..2c97d22b 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -1,6 +1,6 @@ --- a/lib/tasks/discourse-charm.rake +++ b/lib/tasks/discourse-charm.rake -@@ -0,0 +1,11 @@ +@@ -0,0 +1,12 @@ +# frozen_string_literal: true + +desc "Check if a user exists for given email address" diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 609e4427..62f4c85b 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -193,6 +193,8 @@ parts: # https://github.com/lautis/uglifier/issues/127#issuecomment-352224986 sed -i 's/config.assets.js_compressor = :uglifier/config.assets.js_compressor = Uglifier.new(:harmony => true)/g' srv/discourse/app/config/environments/production.rb sed -i '1s/^/require "uglifier"\n/' srv/discourse/app/config/environments/production.rb + prime: + - srv/discourse/app/lib/tasks/discourse-charm.rake scripts: plugin: dump source: scripts diff --git a/src/charm.py b/src/charm.py index 3346948a..ba58036f 100755 --- a/src/charm.py +++ b/src/charm.py @@ -761,7 +761,6 @@ def _on_create_user_action(self, event: ActionEvent): email = event.params["email"] password = self._generate_password(16) - user_exists = container.exec( [os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", f"users:exists[{email}]"], working_dir=DISCOURSE_PATH, @@ -777,7 +776,6 @@ def _on_create_user_action(self, event: ActionEvent): # Admin flag is optional, if it is true, the user will be created as an admin admin_flag = "Y" if event.params.get("admin") else "N" - process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 1829fa86..a4319326 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -378,7 +378,38 @@ async def test_create_user( assert action.results["user"] == email # Re-creating the same user should fail, as the user already exists - email = "admin-user@test.internal" break_action: Action = await discourse_unit.run_action("create-user", email=email) await break_action.wait() assert break_action.status == "failed" + +# promote user +# discourse api call to check admin status + +@pytest.mark.asyncio +async def test_promote_user( + app: Application, + discourse_address: str, +): + """ + arrange: Given discourse application + act: when promoting a user + assert: the user should be promoted successfully. + """ + + email = "test-user@test.internal" + discourse_unit: Unit = app.units[0] + action: Action = await discourse_unit.run_action("create-user", email=email) + await action.wait() + assert action.results["user"] == email + + # use api call to check user status + category_info = {"name": "test", "color": "FFFFFF"} + res = requests.post( + f"{discourse_address}/categories.json", + headers={ + "Api-Key": admin_api_key, + "Api-Username": admin_credentials.username, + }, + json=category_info, + timeout=60, + ) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 08b83173..79ca1a7f 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -352,6 +352,48 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: assert expected_exec_call_was_made +def test_create_user(): + """ + arrange: an email and a password + act: when the _on_create_user_action method is executed + assert: the underlying rake command to add the user is executed + with the appropriate parameters. + """ + harness = helpers.start_harness() + + # We catch the exec call that we expect to register it and make sure that the + # args passed to it are correct. + expected_exec_call_was_made = False + + def bundle_handler(args: ops.testing.ExecArgs) -> None: + nonlocal expected_exec_call_was_made + expected_exec_call_was_made = True + if ( + args.environment != harness.charm._create_discourse_environment_settings() + or args.working_dir != DISCOURSE_PATH + or args.user != "_daemon_" + or args.timeout != 60 + ): + raise ValueError(f"{args.command} wasn't made with the correct args.") + + harness.handle_exec( + SERVICE_NAME, + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "admin:create"], + handler=bundle_handler, + ) + + charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) + + email = "admin-user@test.internal" + password = "test-discourse-k8s-password" # nosec + event = MagicMock(spec=ActionEvent) + event.params = { + "email": email + } + charm._on_create_user_action(event) + assert expected_exec_call_was_made + + def test_anonymize_user(): """ arrange: set up discourse From 4e29b908ae3822da6460f17c90a0e87227022400 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 5 Jun 2024 16:46:12 +0100 Subject: [PATCH 48/88] fixes, new integration test and fix for unit test --- src/charm.py | 16 ++++---- tests/integration/test_charm.py | 70 ++++++++++++++++++++++++--------- tests/unit/test_charm.py | 7 +--- 3 files changed, 63 insertions(+), 30 deletions(-) diff --git a/src/charm.py b/src/charm.py index ba58036f..3cc2ac2c 100755 --- a/src/charm.py +++ b/src/charm.py @@ -748,7 +748,7 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore ) - def _on_create_user_action(self, event: ActionEvent): + def _on_create_user_action(self, event: ActionEvent, skip_validation: bool = False) -> None: """Create a new user in Discourse. Args: @@ -767,12 +767,14 @@ def _on_create_user_action(self, event: ActionEvent): user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), ) - try: - user_exists.wait_output() - event.fail(f"User with email {email} already exists") - return - except ExecError: - pass + # The validation can be skipped if the action is called from a test + if not skip_validation: + try: + user_exists.wait_output() + event.fail(f"User with email {email} already exists") + return + except ExecError: + pass # Admin flag is optional, if it is true, the user will be created as an admin admin_flag = "Y" if event.params.get("admin") else "N" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index a4319326..d769c716 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -382,8 +382,6 @@ async def test_create_user( await break_action.wait() assert break_action.status == "failed" -# promote user -# discourse api call to check admin status @pytest.mark.asyncio async def test_promote_user( @@ -395,21 +393,57 @@ async def test_promote_user( act: when promoting a user assert: the user should be promoted successfully. """ + with requests.session() as sess: + res = sess.get(f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}) + # pylint doesn't see the "ok" member + assert res.status_code == requests.codes.ok, res.text # pylint: disable=no-member + data = res.json() + assert data["csrf"], data + csrf = data["csrf"] + + def get_api_key() -> str: + res = sess.post( + f"{discourse_address}/admin/api/keys", + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": csrf, + "X-Requested-With": "XMLHttpRequest", + }, + json={"key": {"description": "admin-api-key", "username": None}}, + ) + return res.json()["key"]["key"] + + def attempt_login(email: str, password: str) -> str: + res = sess.post( + f"{discourse_address}/session", + headers={ + "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8", + "X-CSRF-Token": csrf, + "X-Requested-With": "XMLHttpRequest", + }, + data={ + "login": email, + "password": password, + "second_factor_method": "1", + "timezone": "Asia/Hong_Kong", + }, + ) + assert "error" not in res.json() - email = "test-user@test.internal" - discourse_unit: Unit = app.units[0] - action: Action = await discourse_unit.run_action("create-user", email=email) - await action.wait() - assert action.results["user"] == email + email = "test-user@test.internal" + discourse_unit: Unit = app.units[0] + action: Action = await discourse_unit.run_action("create-user", email=email) + await action.wait() + assert action.results["user"] == email - # use api call to check user status - category_info = {"name": "test", "color": "FFFFFF"} - res = requests.post( - f"{discourse_address}/categories.json", - headers={ - "Api-Key": admin_api_key, - "Api-Username": admin_credentials.username, - }, - json=category_info, - timeout=60, - ) + attempt_login(email, action.results["password"]) + + # This should fail as the user is not promoted + assert get_api_key() is None + + # Promote the user + action: Action = await discourse_unit.run_action("promote-user", email=email) + await action.wait() + + # Check the user is promoted + assert get_api_key() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 79ca1a7f..4e42ccf5 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -385,12 +385,9 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) email = "admin-user@test.internal" - password = "test-discourse-k8s-password" # nosec event = MagicMock(spec=ActionEvent) - event.params = { - "email": email - } - charm._on_create_user_action(event) + event.params = {"email": email} + charm._on_create_user_action(event, True) assert expected_exec_call_was_made From 10328ed7f07100245c1e8addcbb2ab4395b3f0e2 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 5 Jun 2024 16:59:24 +0100 Subject: [PATCH 49/88] linting fixes --- tests/integration/test_charm.py | 46 ++++++++++++++++----------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index d769c716..e0b35022 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -188,9 +188,9 @@ async def test_create_category( admin_api_key: str, ): """ - arrange: Given discourse application and an admin user - act: if an admin user creates a category - assert: a category should be created normally. + arrange: A discourse application and an admin user + act: If an admin user creates a category + assert: A category should be created normally. """ category_info = {"name": "test", "color": "FFFFFF"} res = requests.post( @@ -216,9 +216,9 @@ async def test_serve_compiled_assets( discourse_address: str, ): """ - arrange: Given discourse application - act: when accessing a page that does not exist - assert: a compiled asset should be served. + arrange: A discourse application + act: Access a page that does not exist + assert: A compiled asset should be served. """ res = requests.get(f"{discourse_address}/404", timeout=60) not_found_page = str(res.content) @@ -237,9 +237,9 @@ async def test_relations( requests_timeout: int, ): """ - arrange: Given discourse application - act: when removing some of its relations - assert: it should have the correct status + arrange: A discourse application + act: Remove some of its relations + assert: It should have the correct status """ def test_discourse_srv_status_ok(): @@ -287,7 +287,7 @@ async def test_upgrade( ops_test: OpsTest, ): """ - arrange: Given discourse application with three units + arrange: A discourse application with three units act: Refresh the application (upgrade) assert: The application upgrades and over all the upgrade, the application replies correctly through the ingress. @@ -366,9 +366,9 @@ async def test_create_user( app: Application, ): """ - arrange: Given discourse application - act: when creating a user - assert: the user should be created successfully. + arrange: A discourse application + act: Create a user + assert: User is created, and re-creating the same user should fail """ email = "admin-user@test.internal" @@ -389,9 +389,9 @@ async def test_promote_user( discourse_address: str, ): """ - arrange: Given discourse application - act: when promoting a user - assert: the user should be promoted successfully. + arrange: A discourse application + act: Promote a user to admin + assert: User cannot access the admin API before being promoted """ with requests.session() as sess: res = sess.get(f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}) @@ -413,7 +413,7 @@ def get_api_key() -> str: ) return res.json()["key"]["key"] - def attempt_login(email: str, password: str) -> str: + def attempt_login(email: str, password: str): res = sess.post( f"{discourse_address}/session", headers={ @@ -432,18 +432,18 @@ def attempt_login(email: str, password: str) -> str: email = "test-user@test.internal" discourse_unit: Unit = app.units[0] - action: Action = await discourse_unit.run_action("create-user", email=email) - await action.wait() - assert action.results["user"] == email + create_action: Action = await discourse_unit.run_action("create-user", email=email) + await create_action.wait() + assert create_action.results["user"] == email - attempt_login(email, action.results["password"]) + attempt_login(email, create_action.results["password"]) # This should fail as the user is not promoted assert get_api_key() is None # Promote the user - action: Action = await discourse_unit.run_action("promote-user", email=email) - await action.wait() + promote_action: Action = await discourse_unit.run_action("promote-user", email=email) + await promote_action.wait() # Check the user is promoted assert get_api_key() From ea6f4957baf8fc6a399e58909c0b9c4bd3fcc1b3 Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 6 Jun 2024 15:24:48 +0100 Subject: [PATCH 50/88] addressed comments --- actions.yaml | 20 ++++++++++---------- discourse_rock/rockcraft.yaml | 3 +-- tests/integration/conftest.py | 18 +++++++++--------- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/actions.yaml b/actions.yaml index 4e1d5445..10370b63 100644 --- a/actions.yaml +++ b/actions.yaml @@ -1,12 +1,12 @@ # Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. -promote-user: - description: Promote a user to admin. +anonymize-user: + description: Anonymize a user. params: - email: + username: type: string - description: User email. - required: [email] + description: The unique identifier of the user to anonymize. + required: [username] create-user: description: Create a new user. params: @@ -17,10 +17,10 @@ create-user: type: boolean description: Whether the user should be an admin. required: [email] -anonymize-user: - description: Anonymize a user. +promote-user: + description: Promote a user to admin. params: - username: + email: type: string - description: The unique identifier of the user to anonymize. - required: [username] + description: User email. + required: [email] \ No newline at end of file diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 62f4c85b..f47aee41 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -186,7 +186,6 @@ parts: after: [discourse, patches] override-stage: | git -C srv/discourse/app apply patches/lp1903695.patch - touch srv/discourse/app/lib/tasks/discourse-charm.rake git -C srv/discourse/app apply patches/discourse-charm.patch git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation @@ -194,7 +193,7 @@ parts: sed -i 's/config.assets.js_compressor = :uglifier/config.assets.js_compressor = Uglifier.new(:harmony => true)/g' srv/discourse/app/config/environments/production.rb sed -i '1s/^/require "uglifier"\n/' srv/discourse/app/config/environments/production.rb prime: - - srv/discourse/app/lib/tasks/discourse-charm.rake + - srv/discourse/app/patches/discourse-charm.patch scripts: plugin: dump source: scripts diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index dce052a7..496e59b5 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -277,14 +277,14 @@ async def admin_api_key_fixture( """Admin user API key""" with requests.session() as sess: # Get CSRF token - res = sess.get(f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}) + response = sess.get(f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}) # pylint doesn't see the "ok" member - assert res.status_code == requests.codes.ok, res.text # pylint: disable=no-member - data = res.json() + assert response.status_code == requests.codes.ok, response.text # pylint: disable=no-member + data = response.json() assert data["csrf"], data csrf = data["csrf"] # Create session & login - res = sess.post( + response = sess.post( f"{discourse_address}/session", headers={ "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8", @@ -299,10 +299,10 @@ async def admin_api_key_fixture( }, ) # pylint doesn't see the "ok" member - assert res.status_code == requests.codes.ok, res.text # pylint: disable=no-member - assert "error" not in res.json() + assert response.status_code == requests.codes.ok, response.text # pylint: disable=no-member + assert "error" not in response.json() # Create global key - res = sess.post( + response = sess.post( f"{discourse_address}/admin/api/keys", headers={ "Content-Type": "application/json", @@ -312,8 +312,8 @@ async def admin_api_key_fixture( json={"key": {"description": "admin-api-key", "username": None}}, ) # pylint doesn't see the "ok" member - assert res.status_code == requests.codes.ok, res.text # pylint: disable=no-member + assert response.status_code == requests.codes.ok, response.text # pylint: disable=no-member - data = res.json() + data = response.json() assert data["key"]["key"], data return data["key"]["key"] From 89303515a9dc8f2973f79dce393e1f2d9cdf77ba Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 6 Jun 2024 15:33:26 +0100 Subject: [PATCH 51/88] using response.ok --- tests/integration/conftest.py | 16 +++++++++------- tests/integration/test_charm.py | 28 ++++++++++++++-------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 496e59b5..a88491a7 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -275,16 +275,18 @@ async def admin_api_key_fixture( admin_credentials: types.Credentials, discourse_address: str ) -> str: """Admin user API key""" - with requests.session() as sess: + with requests.session() as session: # Get CSRF token - response = sess.get(f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}) + response = session.get( + f"{discourse_address}/session/csrf", headers={"Accept": "application/json"} + ) # pylint doesn't see the "ok" member - assert response.status_code == requests.codes.ok, response.text # pylint: disable=no-member + assert response.ok, response.text # pylint: disable=no-member data = response.json() assert data["csrf"], data csrf = data["csrf"] # Create session & login - response = sess.post( + response = session.post( f"{discourse_address}/session", headers={ "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8", @@ -299,10 +301,10 @@ async def admin_api_key_fixture( }, ) # pylint doesn't see the "ok" member - assert response.status_code == requests.codes.ok, response.text # pylint: disable=no-member + assert response.ok, response.text # pylint: disable=no-member assert "error" not in response.json() # Create global key - response = sess.post( + response = session.post( f"{discourse_address}/admin/api/keys", headers={ "Content-Type": "application/json", @@ -312,7 +314,7 @@ async def admin_api_key_fixture( json={"key": {"description": "admin-api-key", "username": None}}, ) # pylint doesn't see the "ok" member - assert response.status_code == requests.codes.ok, response.text # pylint: disable=no-member + assert response.ok, response.text # pylint: disable=no-member data = response.json() assert data["key"]["key"], data diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index e0b35022..1d72c860 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -193,7 +193,7 @@ async def test_create_category( assert: A category should be created normally. """ category_info = {"name": "test", "color": "FFFFFF"} - res = requests.post( + response = requests.post( f"{discourse_address}/categories.json", headers={ "Api-Key": admin_api_key, @@ -202,7 +202,7 @@ async def test_create_category( json=category_info, timeout=60, ) - category_id = res.json()["category"]["id"] + category_id = response.json()["category"]["id"] category = requests.get(f"{discourse_address}/c/{category_id}/show.json", timeout=60).json()[ "category" ] @@ -220,8 +220,8 @@ async def test_serve_compiled_assets( act: Access a page that does not exist assert: A compiled asset should be served. """ - res = requests.get(f"{discourse_address}/404", timeout=60) - not_found_page = str(res.content) + response = requests.get(f"{discourse_address}/404", timeout=60) + not_found_page = str(response.content) asset_matches = re.search( r"(onpopstate-handler).+.js", not_found_page @@ -393,16 +393,18 @@ async def test_promote_user( act: Promote a user to admin assert: User cannot access the admin API before being promoted """ - with requests.session() as sess: - res = sess.get(f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}) + with requests.session() as session: + response = session.get( + f"{discourse_address}/session/csrf", headers={"Accept": "application/json"} + ) # pylint doesn't see the "ok" member - assert res.status_code == requests.codes.ok, res.text # pylint: disable=no-member - data = res.json() + assert response.ok, response.text # pylint: disable=no-member + data = response.json() assert data["csrf"], data csrf = data["csrf"] def get_api_key() -> str: - res = sess.post( + response = session.post( f"{discourse_address}/admin/api/keys", headers={ "Content-Type": "application/json", @@ -411,10 +413,10 @@ def get_api_key() -> str: }, json={"key": {"description": "admin-api-key", "username": None}}, ) - return res.json()["key"]["key"] + return response.json()["key"]["key"] def attempt_login(email: str, password: str): - res = sess.post( + response = session.post( f"{discourse_address}/session", headers={ "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8", @@ -428,7 +430,7 @@ def attempt_login(email: str, password: str): "timezone": "Asia/Hong_Kong", }, ) - assert "error" not in res.json() + assert "error" not in response.json() email = "test-user@test.internal" discourse_unit: Unit = app.units[0] @@ -441,9 +443,7 @@ def attempt_login(email: str, password: str): # This should fail as the user is not promoted assert get_api_key() is None - # Promote the user promote_action: Action = await discourse_unit.run_action("promote-user", email=email) await promote_action.wait() - # Check the user is promoted assert get_api_key() From 373a6fe9f1d359ee2b13f3c85c2a57d40fa628c0 Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 6 Jun 2024 16:49:05 +0100 Subject: [PATCH 52/88] readded touch --- discourse_rock/rockcraft.yaml | 1 + src-docs/charm.py.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index aa9aec4b..6f9077f7 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -186,6 +186,7 @@ parts: after: [discourse, patches] override-stage: | git -C srv/discourse/app apply patches/lp1903695.patch + touch srv/discourse/app/patches/discourse-charm.patch git -C srv/discourse/app apply patches/discourse-charm.patch git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index a74aec10..44d3f51f 100644 --- a/src-docs/charm.py.md +++ b/src-docs/charm.py.md @@ -28,7 +28,7 @@ Charm for Discourse on kubernetes. ## class `DiscourseCharm` Charm for Discourse on kubernetes. - + ### function `__init__` From 2fc84da39a1bc928a42bff24c2826466dcee66da Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 6 Jun 2024 21:46:52 +0100 Subject: [PATCH 53/88] rock and unit test --- discourse_rock/rockcraft.yaml | 33 +++------------------------------ src/charm.py | 19 +++++++++---------- tests/unit/test_charm.py | 20 ++++++++++++++++---- 3 files changed, 28 insertions(+), 44 deletions(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 6f9077f7..6321b819 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -159,42 +159,15 @@ parts: "*": srv/discourse/app/plugins/discourse-templates/ discourse-calendar: plugin: dump - after: [discourse, bundler-config] - source: https://github.com/discourse/discourse-calendar.git - source-commit: 84ef46a38cf02748ecacad16c5d9c6fec12dc8da - source-depth: 1 - organize: - "*": srv/discourse/app/plugins/discourse-calendar/ - discourse-gamification: - plugin: dump - after: [discourse, bundler-config] - source: https://github.com/discourse/discourse-gamification.git - source-commit: 5951fc573702090c0dc95b12d4aa3a053303bd63 - source-depth: 1 - organize: - "*": srv/discourse/app/plugins/discourse-gamification/ - patches: - plugin: dump - after: [discourse] - source: patches - organize: - "*": srv/discourse/app/patches/ - apply-patches: - plugin: nil - build-packages: - - git - after: [discourse, patches] - override-stage: | - git -C srv/discourse/app apply patches/lp1903695.patch - touch srv/discourse/app/patches/discourse-charm.patch - git -C srv/discourse/app apply patches/discourse-charm.patch + touch srv/discourse/app/lib/tasks/discourse-charm.rake + git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation # https://github.com/lautis/uglifier/issues/127#issuecomment-352224986 sed -i 's/config.assets.js_compressor = :uglifier/config.assets.js_compressor = Uglifier.new(:harmony => true)/g' srv/discourse/app/config/environments/production.rb sed -i '1s/^/require "uglifier"\n/' srv/discourse/app/config/environments/production.rb prime: - - srv/discourse/app/patches/discourse-charm.patch + - srv/discourse/app/lib/tasks/discourse-charm.rake scripts: plugin: dump source: scripts diff --git a/src/charm.py b/src/charm.py index 5dcc5a19..80568e15 100755 --- a/src/charm.py +++ b/src/charm.py @@ -743,7 +743,7 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: f"Failed to make user with email {email} an admin: {ex.stdout}" # type: ignore ) - def _on_create_user_action(self, event: ActionEvent, skip_validation: bool = False) -> None: + def _on_create_user_action(self, event: ActionEvent) -> None: """Create a new user in Discourse. Args: @@ -762,15 +762,14 @@ def _on_create_user_action(self, event: ActionEvent, skip_validation: bool = Fal user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), ) - # The validation can be skipped if the action is called from a test - if not skip_validation: - try: - user_exists.wait_output() - event.fail(f"User with email {email} already exists") - return - except ExecError: - pass - + try: + user_exists.wait_output() + print("2") + event.fail(f"User with email {email} already exists") + return + except ExecError: + print("3") + pass # Admin flag is optional, if it is true, the user will be created as an admin admin_flag = "Y" if event.params.get("admin") else "N" process = container.exec( diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 4e42ccf5..b996c624 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -14,6 +14,7 @@ import pytest from ops.charm import ActionEvent from ops.model import ActiveStatus, BlockedStatus, WaitingStatus +from ops.pebble import ExecError from charm import CONTAINER_NAME, DATABASE_NAME, DISCOURSE_PATH, SERVICE_NAME, DiscourseCharm from tests.unit import helpers @@ -365,7 +366,7 @@ def test_create_user(): # args passed to it are correct. expected_exec_call_was_made = False - def bundle_handler(args: ops.testing.ExecArgs) -> None: + def create_bundle_handler(args: ops.testing.ExecArgs) -> None: nonlocal expected_exec_call_was_made expected_exec_call_was_made = True if ( @@ -376,18 +377,29 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: ): raise ValueError(f"{args.command} wasn't made with the correct args.") + def exists_bundle_handler(event: ops.testing.ExecArgs) -> None: + print("1") + raise ExecError(command=event.command, exit_code=1, stdout="", stderr="") + + email = "admin-user@test.internal" + + harness.handle_exec( + SERVICE_NAME, + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", f"users:exists[{email}]"], + handler=exists_bundle_handler + ) + harness.handle_exec( SERVICE_NAME, [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "admin:create"], - handler=bundle_handler, + handler=create_bundle_handler, ) charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) - email = "admin-user@test.internal" event = MagicMock(spec=ActionEvent) event.params = {"email": email} - charm._on_create_user_action(event, True) + charm._on_create_user_action(event) assert expected_exec_call_was_made From fcc9fa3c60ee2c383310a6fd2c73d874ff3ba30d Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 6 Jun 2024 22:17:42 +0100 Subject: [PATCH 54/88] uhm --- discourse_rock/rockcraft.yaml | 30 +++++++++++++++++++++++++++++- src/charm.py | 4 +--- tests/unit/test_charm.py | 4 +++- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 6321b819..605cf9a6 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -159,8 +159,36 @@ parts: "*": srv/discourse/app/plugins/discourse-templates/ discourse-calendar: plugin: dump + after: [discourse, bundler-config] touch srv/discourse/app/lib/tasks/discourse-charm.rake - + source: https://github.com/discourse/discourse-calendar.git + source-commit: 84ef46a38cf02748ecacad16c5d9c6fec12dc8da + source-depth: 1 + organize: + "*": srv/discourse/app/plugins/discourse-calendar/ + discourse-gamification: + plugin: dump + after: [discourse, bundler-config] + source: https://github.com/discourse/discourse-gamification.git + source-commit: 5951fc573702090c0dc95b12d4aa3a053303bd63 + source-depth: 1 + organize: + "*": srv/discourse/app/plugins/discourse-gamification/ + patches: + plugin: dump + after: [discourse] + source: patches + organize: + "*": srv/discourse/app/patches/ + apply-patches: + plugin: nil + build-packages: + - git + after: [discourse, patches] + override-stage: | + git -C srv/discourse/app apply patches/lp1903695.patch + touch srv/discourse/app/patches/discourse-charm.patch + git -C srv/discourse/app apply patches/discourse-charm.patch git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation # https://github.com/lautis/uglifier/issues/127#issuecomment-352224986 diff --git a/src/charm.py b/src/charm.py index 80568e15..18de0a47 100755 --- a/src/charm.py +++ b/src/charm.py @@ -763,12 +763,10 @@ def _on_create_user_action(self, event: ActionEvent) -> None: environment=self._create_discourse_environment_settings(), ) try: - user_exists.wait_output() - print("2") + assert user_exists.wait_output() event.fail(f"User with email {email} already exists") return except ExecError: - print("3") pass # Admin flag is optional, if it is true, the user will be created as an admin admin_flag = "Y" if event.params.get("admin") else "N" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index b996c624..9aa1eaf8 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -353,13 +353,15 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: assert expected_exec_call_was_made -def test_create_user(): +@patch.object(ops.Container, "exec") +def test_create_user(mock_exec): """ arrange: an email and a password act: when the _on_create_user_action method is executed assert: the underlying rake command to add the user is executed with the appropriate parameters. """ + mock_exec.return_value = MagicMock(wait_output=MagicMock(side_effect=ExecError(command="", exit_code=1, stdout="", stderr=""))) harness = helpers.start_harness() # We catch the exec call that we expect to register it and make sure that the From d801445254d7220f053c61b6221a3d76c7a526e2 Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 6 Jun 2024 22:20:41 +0100 Subject: [PATCH 55/88] blank --- discourse_rock/rockcraft.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 605cf9a6..bf9e5836 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -160,7 +160,6 @@ parts: discourse-calendar: plugin: dump after: [discourse, bundler-config] - touch srv/discourse/app/lib/tasks/discourse-charm.rake source: https://github.com/discourse/discourse-calendar.git source-commit: 84ef46a38cf02748ecacad16c5d9c6fec12dc8da source-depth: 1 @@ -187,7 +186,7 @@ parts: after: [discourse, patches] override-stage: | git -C srv/discourse/app apply patches/lp1903695.patch - touch srv/discourse/app/patches/discourse-charm.patch + touch srv/discourse/app/patches/discourse-charm.rake git -C srv/discourse/app apply patches/discourse-charm.patch git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation From 5c5c21280de210e52e47237fc69932162d66240e Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 6 Jun 2024 22:22:24 +0100 Subject: [PATCH 56/88] Dblank --- discourse_rock/rockcraft.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index bf9e5836..7f789e7e 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -160,7 +160,7 @@ parts: discourse-calendar: plugin: dump after: [discourse, bundler-config] - source: https://github.com/discourse/discourse-calendar.git + source: https://github.com/discourse/discourse-calendar.git source-commit: 84ef46a38cf02748ecacad16c5d9c6fec12dc8da source-depth: 1 organize: From af951d14eebbb4f907839732f33d2a3b2d45b02e Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 7 Jun 2024 00:54:38 +0100 Subject: [PATCH 57/88] addressed changes, fixed linting --- tests/integration/test_charm.py | 5 ++--- tests/unit/test_charm.py | 13 ++++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 1d72c860..12389088 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -440,10 +440,9 @@ def attempt_login(email: str, password: str): attempt_login(email, create_action.results["password"]) - # This should fail as the user is not promoted - assert get_api_key() is None + assert get_api_key() is None, "This should fail as the user is not promoted" promote_action: Action = await discourse_unit.run_action("promote-user", email=email) await promote_action.wait() - assert get_api_key() + assert get_api_key(), "This should succeed as the user is promoted" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 9aa1eaf8..77a80622 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -361,7 +361,11 @@ def test_create_user(mock_exec): assert: the underlying rake command to add the user is executed with the appropriate parameters. """ - mock_exec.return_value = MagicMock(wait_output=MagicMock(side_effect=ExecError(command="", exit_code=1, stdout="", stderr=""))) + mock_exec.return_value = MagicMock( + wait_output=MagicMock( + side_effect=ExecError(command=[""], exit_code=1, stdout="", stderr="") + ) + ) harness = helpers.start_harness() # We catch the exec call that we expect to register it and make sure that the @@ -380,15 +384,14 @@ def create_bundle_handler(args: ops.testing.ExecArgs) -> None: raise ValueError(f"{args.command} wasn't made with the correct args.") def exists_bundle_handler(event: ops.testing.ExecArgs) -> None: - print("1") - raise ExecError(command=event.command, exit_code=1, stdout="", stderr="") + raise ExecError(command=event.command, exit_code=1, stdout="", stderr="") email = "admin-user@test.internal" - + harness.handle_exec( SERVICE_NAME, [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", f"users:exists[{email}]"], - handler=exists_bundle_handler + handler=exists_bundle_handler, ) harness.handle_exec( From b9012a870ba2277b26a9af5ca77827a2708a07a2 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 7 Jun 2024 16:58:01 +0100 Subject: [PATCH 58/88] fixed unit test --- tests/unit/test_charm.py | 86 ++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 77a80622..7828bb14 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -8,7 +8,7 @@ import secrets import typing -from unittest.mock import MagicMock, patch +from unittest.mock import DEFAULT, MagicMock, patch import ops import pytest @@ -346,9 +346,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: email = "sample@email.com" event = MagicMock(spec=ActionEvent) - event.params = { - "email": email, - } + event.params = {"email": email} charm._on_promote_user_action(event) assert expected_exec_call_was_made @@ -356,52 +354,64 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: @patch.object(ops.Container, "exec") def test_create_user(mock_exec): """ - arrange: an email and a password + arrange: an email act: when the _on_create_user_action method is executed - assert: the underlying rake command to add the user is executed - with the appropriate parameters. + assert: the create user rake command is executed upon failure of the user existence check. """ - mock_exec.return_value = MagicMock( - wait_output=MagicMock( - side_effect=ExecError(command=[""], exit_code=1, stdout="", stderr="") + mock_wo = MagicMock( + return_value=("", None), ) - ) - harness = helpers.start_harness() + stdout_mock = "CRASH" + harness = helpers.start_harness(run_initial_hooks=False) + harness.set_can_connect(CONTAINER_NAME, True) - # We catch the exec call that we expect to register it and make sure that the - # args passed to it are correct. + email = "admin-user@test.internal" expected_exec_call_was_made = False - def create_bundle_handler(args: ops.testing.ExecArgs) -> None: - nonlocal expected_exec_call_was_made - expected_exec_call_was_made = True - if ( - args.environment != harness.charm._create_discourse_environment_settings() - or args.working_dir != DISCOURSE_PATH - or args.user != "_daemon_" - or args.timeout != 60 - ): - raise ValueError(f"{args.command} wasn't made with the correct args.") + def mock_wo_side_effect(*args, **kwargs): # pylint: disable=unused-argument + """Mock wo side effect. - def exists_bundle_handler(event: ops.testing.ExecArgs) -> None: - raise ExecError(command=event.command, exit_code=1, stdout="", stderr="") + Args: + args: Variable list of positional arguments passed to the parent constructor. + kwargs: a `dict` of the extra arguments passed to the function. - email = "admin-user@test.internal" + Returns: + unittest.mock DEFAULT built-in. - harness.handle_exec( - SERVICE_NAME, - [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", f"users:exists[{email}]"], - handler=exists_bundle_handler, - ) + Raises: + ExecError: Execution error fired if conditions are met. + """ - harness.handle_exec( - SERVICE_NAME, - [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "admin:create"], - handler=create_bundle_handler, - ) + if isinstance(mock_wo.cmd, list) and f"users:exists[{email}]" in mock_wo.cmd: + raise ExecError(command=mock_wo.cmd, exit_code=42, stdout=stdout_mock, stderr="") + + if isinstance(mock_wo.cmd, list) and f"admin:create" in mock_wo.cmd: + nonlocal expected_exec_call_was_made + expected_exec_call_was_made = True - charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) + return DEFAULT + + mock_wo.side_effect = mock_wo_side_effect + def mock_exec_side_effect(*args, **kwargs): # pylint: disable=unused-argument + """Mock execution side effect. + + Args: + args: Variable list of positional arguments passed to the parent constructor. + kwargs: a `dict` of the extra arguments passed to the function. + + Returns: + unittest.mock DEFAULT built-in. + """ + mock_wo.cmd = args[0] + return DEFAULT + + mock_exec.side_effect = mock_exec_side_effect + mock_exec.return_value = (MagicMock( + wait_output=mock_wo, + )) + + charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) event = MagicMock(spec=ActionEvent) event.params = {"email": email} charm._on_create_user_action(event) From c56b177c7cafea101877dc7ac57d29c1e617035a Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 7 Jun 2024 17:48:56 +0100 Subject: [PATCH 59/88] updated rock :wq --- discourse_rock/rockcraft.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 7f789e7e..10200fa7 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -12,6 +12,11 @@ version: "1.0" platforms: amd64: parts: + add-user: + plugin: nil + overlay-script: | + mkdir -p $CRAFT_OVERLAY/run/discourse-k8s-operator + chmod 755 $CRAFT_OVERLAY/run/discourse-k8s-operator tooling: plugin: nil overlay-packages: From 107c78087be87a8ce1079812c55e214a3854d531 Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 10 Jun 2024 14:23:45 +0100 Subject: [PATCH 60/88] added user activation --- actions.yaml | 4 ++++ discourse_rock/patches/discourse-charm.patch | 13 +++++++++++++ discourse_rock/rockcraft.yaml | 5 ----- src/charm.py | 12 ++++++++++++ tests/integration/test_charm.py | 4 ++-- 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/actions.yaml b/actions.yaml index 10370b63..b70f9c07 100644 --- a/actions.yaml +++ b/actions.yaml @@ -16,6 +16,10 @@ create-user: admin: type: boolean description: Whether the user should be an admin. + active: + type: boolean + description: Whether the user should be email-verified and active. + default: true required: [email] promote-user: description: Promote a user to admin. diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index 2c97d22b..c822559f 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -13,3 +13,16 @@ + puts "ERROR: User with email #{email} not found" + exit 1 +end ++ ++desc "Activate a user account" ++task "users:activate", [:email] => [:environment] do |_, args| ++ email = args[:email] ++ user = User.find_by_email(email) ++ if !user ++ puts "User with email #{email} does not exist" ++ exit 1 ++ end ++ user.email_tokens.update_all(confirmed: true) ++ puts "User with email #{email} activated" ++ exit 0 ++end diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 10200fa7..7f789e7e 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -12,11 +12,6 @@ version: "1.0" platforms: amd64: parts: - add-user: - plugin: nil - overlay-script: | - mkdir -p $CRAFT_OVERLAY/run/discourse-k8s-operator - chmod 755 $CRAFT_OVERLAY/run/discourse-k8s-operator tooling: plugin: nil overlay-packages: diff --git a/src/charm.py b/src/charm.py index 18de0a47..9bead044 100755 --- a/src/charm.py +++ b/src/charm.py @@ -783,6 +783,18 @@ def _on_create_user_action(self, event: ActionEvent) -> None: environment=self._create_discourse_environment_settings(), timeout=60, ) + if not event.params.get("admin") and not event.params.get("active"): + container.exec( + [ + os.path.join(DISCOURSE_PATH, "bin/bundle"), + "exec", + "rake", + f"users:activate[{email}]", + ], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + ) try: process.wait_output() event.set_results({"user": email, "password": password}) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 12389088..da4a778d 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -432,9 +432,9 @@ def attempt_login(email: str, password: str): ) assert "error" not in response.json() - email = "test-user@test.internal" + email = "test-promote-user@test.internal" discourse_unit: Unit = app.units[0] - create_action: Action = await discourse_unit.run_action("create-user", email=email) + create_action: Action = await discourse_unit.run_action("create-user", email=email, admin=False) await create_action.wait() assert create_action.results["user"] == email From 952b9a6e3b6cc036407187b40e0aa6dccf0312b8 Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 10 Jun 2024 14:26:04 +0100 Subject: [PATCH 61/88] fixed linting --- tests/integration/test_charm.py | 4 +++- tests/unit/test_charm.py | 14 +++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index da4a778d..de3eb334 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -434,7 +434,9 @@ def attempt_login(email: str, password: str): email = "test-promote-user@test.internal" discourse_unit: Unit = app.units[0] - create_action: Action = await discourse_unit.run_action("create-user", email=email, admin=False) + create_action: Action = await discourse_unit.run_action( + "create-user", email=email, admin=False + ) await create_action.wait() assert create_action.results["user"] == email diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 7828bb14..4a336e4e 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -359,8 +359,8 @@ def test_create_user(mock_exec): assert: the create user rake command is executed upon failure of the user existence check. """ mock_wo = MagicMock( - return_value=("", None), - ) + return_value=("", None), + ) stdout_mock = "CRASH" harness = helpers.start_harness(run_initial_hooks=False) harness.set_can_connect(CONTAINER_NAME, True) @@ -384,8 +384,8 @@ def mock_wo_side_effect(*args, **kwargs): # pylint: disable=unused-argument if isinstance(mock_wo.cmd, list) and f"users:exists[{email}]" in mock_wo.cmd: raise ExecError(command=mock_wo.cmd, exit_code=42, stdout=stdout_mock, stderr="") - - if isinstance(mock_wo.cmd, list) and f"admin:create" in mock_wo.cmd: + + if isinstance(mock_wo.cmd, list) and "admin:create" in mock_wo.cmd: nonlocal expected_exec_call_was_made expected_exec_call_was_made = True @@ -405,11 +405,11 @@ def mock_exec_side_effect(*args, **kwargs): # pylint: disable=unused-argument """ mock_wo.cmd = args[0] return DEFAULT - + mock_exec.side_effect = mock_exec_side_effect - mock_exec.return_value = (MagicMock( + mock_exec.return_value = MagicMock( wait_output=mock_wo, - )) + ) charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) event = MagicMock(spec=ActionEvent) From 4abe096c400f528c017446b946d999f911505ce9 Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 10 Jun 2024 15:12:30 +0100 Subject: [PATCH 62/88] improved and fixed unit test --- src/charm.py | 2 +- tests/unit/test_charm.py | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/charm.py b/src/charm.py index 9bead044..254b348b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -763,7 +763,7 @@ def _on_create_user_action(self, event: ActionEvent) -> None: environment=self._create_discourse_environment_settings(), ) try: - assert user_exists.wait_output() + user_exists.wait_output() event.fail(f"User with email {email} already exists") return except ExecError: diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 4a336e4e..32a547d7 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -385,10 +385,6 @@ def mock_wo_side_effect(*args, **kwargs): # pylint: disable=unused-argument if isinstance(mock_wo.cmd, list) and f"users:exists[{email}]" in mock_wo.cmd: raise ExecError(command=mock_wo.cmd, exit_code=42, stdout=stdout_mock, stderr="") - if isinstance(mock_wo.cmd, list) and "admin:create" in mock_wo.cmd: - nonlocal expected_exec_call_was_made - expected_exec_call_was_made = True - return DEFAULT mock_wo.side_effect = mock_wo_side_effect @@ -404,6 +400,19 @@ def mock_exec_side_effect(*args, **kwargs): # pylint: disable=unused-argument unittest.mock DEFAULT built-in. """ mock_wo.cmd = args[0] + + if isinstance(mock_wo.cmd, list) and "admin:create" in mock_wo.cmd: + nonlocal expected_exec_call_was_made + expected_exec_call_was_made = True + if ( + kwargs["environment"] != harness.charm._create_discourse_environment_settings() + or kwargs["working_dir"] != DISCOURSE_PATH + or kwargs["user"] != "_daemon_" + or email not in kwargs["stdin"] + or kwargs["timeout"] != 60 + ): + raise ValueError(f"{mock_wo.cmd} wasn't made with the correct args.") + return DEFAULT mock_exec.side_effect = mock_exec_side_effect From b1a736f842c18ee5abc389c9c5b04f1d66231197 Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 10 Jun 2024 15:20:21 +0100 Subject: [PATCH 63/88] improved and fixed unit test --- tests/unit/test_charm.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 32a547d7..9fd3fa8b 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -368,6 +368,11 @@ def test_create_user(mock_exec): email = "admin-user@test.internal" expected_exec_call_was_made = False + test_users = { + "email": "standard_user@test.internal", "admin": False, "check": False, + "email": "admin_user@test.internal", "admin": True, "check": False + } + def mock_wo_side_effect(*args, **kwargs): # pylint: disable=unused-argument """Mock wo side effect. @@ -412,6 +417,10 @@ def mock_exec_side_effect(*args, **kwargs): # pylint: disable=unused-argument or kwargs["timeout"] != 60 ): raise ValueError(f"{mock_wo.cmd} wasn't made with the correct args.") + if test_users[0]["email"] in kwargs["stdin"] and "n\nY\n" in kwargs["stdin"]: + test_users[0]["check"] = True + if test_users[1]["email"] in kwargs["stdin"] and "Y\n" in kwargs["stdin"]: + test_users[1]["check"] = True return DEFAULT @@ -421,10 +430,19 @@ def mock_exec_side_effect(*args, **kwargs): # pylint: disable=unused-argument ) charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) - event = MagicMock(spec=ActionEvent) - event.params = {"email": email} - charm._on_create_user_action(event) - assert expected_exec_call_was_made + + for user in test_users: + event = MagicMock(spec=ActionEvent) + event.params = {"email": user["email"], "admin": user["admin"]} + charm._on_create_user_action(event) + assert user["check"] + # assert expected_exec_call_was_made + # expected_exec_call_was_made = False + + # event = MagicMock(spec=ActionEvent) + # event.params = {"email": test_users[0]["email"]} + # charm._on_create_user_action(event) + # assert expected_exec_call_was_made def test_anonymize_user(): From 589dad6fd999add710e37b77e41b576b406dc038 Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 10 Jun 2024 16:05:38 +0100 Subject: [PATCH 64/88] fixes --- tests/unit/test_charm.py | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 9fd3fa8b..32a547d7 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -368,11 +368,6 @@ def test_create_user(mock_exec): email = "admin-user@test.internal" expected_exec_call_was_made = False - test_users = { - "email": "standard_user@test.internal", "admin": False, "check": False, - "email": "admin_user@test.internal", "admin": True, "check": False - } - def mock_wo_side_effect(*args, **kwargs): # pylint: disable=unused-argument """Mock wo side effect. @@ -417,10 +412,6 @@ def mock_exec_side_effect(*args, **kwargs): # pylint: disable=unused-argument or kwargs["timeout"] != 60 ): raise ValueError(f"{mock_wo.cmd} wasn't made with the correct args.") - if test_users[0]["email"] in kwargs["stdin"] and "n\nY\n" in kwargs["stdin"]: - test_users[0]["check"] = True - if test_users[1]["email"] in kwargs["stdin"] and "Y\n" in kwargs["stdin"]: - test_users[1]["check"] = True return DEFAULT @@ -430,19 +421,10 @@ def mock_exec_side_effect(*args, **kwargs): # pylint: disable=unused-argument ) charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) - - for user in test_users: - event = MagicMock(spec=ActionEvent) - event.params = {"email": user["email"], "admin": user["admin"]} - charm._on_create_user_action(event) - assert user["check"] - # assert expected_exec_call_was_made - # expected_exec_call_was_made = False - - # event = MagicMock(spec=ActionEvent) - # event.params = {"email": test_users[0]["email"]} - # charm._on_create_user_action(event) - # assert expected_exec_call_was_made + event = MagicMock(spec=ActionEvent) + event.params = {"email": email} + charm._on_create_user_action(event) + assert expected_exec_call_was_made def test_anonymize_user(): From b0853d9e3a6308b2bec719bf6b9ccd8bf8e2aa5b Mon Sep 17 00:00:00 2001 From: codethulu Date: Mon, 10 Jun 2024 19:23:55 +0100 Subject: [PATCH 65/88] added timeout --- tests/integration/test_charm.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index de3eb334..5131457e 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -395,7 +395,7 @@ async def test_promote_user( """ with requests.session() as session: response = session.get( - f"{discourse_address}/session/csrf", headers={"Accept": "application/json"} + f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}, timeout=60 ) # pylint doesn't see the "ok" member assert response.ok, response.text # pylint: disable=no-member @@ -412,6 +412,7 @@ def get_api_key() -> str: "X-Requested-With": "XMLHttpRequest", }, json={"key": {"description": "admin-api-key", "username": None}}, + timeout=60, ) return response.json()["key"]["key"] @@ -429,6 +430,7 @@ def attempt_login(email: str, password: str): "second_factor_method": "1", "timezone": "Asia/Hong_Kong", }, + timeout=60, ) assert "error" not in response.json() From 4ad16cd6049e002c4ed007eceef9f79d1eb0024f Mon Sep 17 00:00:00 2001 From: codethulu Date: Tue, 11 Jun 2024 09:51:16 +0100 Subject: [PATCH 66/88] flagged test upgrade --- tests/integration/test_charm.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 5131457e..88f386f5 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -280,6 +280,7 @@ def test_discourse_srv_status_ok(): test_discourse_srv_status_ok() +@pytest.mark.skip(reason="Frequent timeouts") async def test_upgrade( app: Application, model: Model, @@ -442,6 +443,8 @@ def attempt_login(email: str, password: str): await create_action.wait() assert create_action.results["user"] == email + logger.info("User created, attempting to login") + attempt_login(email, create_action.results["password"]) assert get_api_key() is None, "This should fail as the user is not promoted" From 1bc23b404ecce1826ad627588771957d67f056bb Mon Sep 17 00:00:00 2001 From: codethulu Date: Tue, 11 Jun 2024 10:48:40 +0100 Subject: [PATCH 67/88] activate --- discourse_rock/rockcraft.yaml | 1 - src/charm.py | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 7f789e7e..f309670f 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -6,7 +6,6 @@ summary: Discourse rock description: Discourse OCI image for the Discourse charm base: ubuntu@22.04 # renovate: base: ubuntu:22.04@sha256:19478ce7fc2ffbce89df29fea5725a8d12e57de52eb9ea570890dc5852aac1ac -run-user: _daemon_ # UID/GID 584792 license: Apache-2.0 version: "1.0" platforms: diff --git a/src/charm.py b/src/charm.py index 254b348b..12e2d86f 100755 --- a/src/charm.py +++ b/src/charm.py @@ -784,7 +784,7 @@ def _on_create_user_action(self, event: ActionEvent) -> None: timeout=60, ) if not event.params.get("admin") and not event.params.get("active"): - container.exec( + activate_process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", @@ -795,6 +795,8 @@ def _on_create_user_action(self, event: ActionEvent) -> None: user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), ) + activate_process.wait_output() + logger.info(f"User {email} activated") try: process.wait_output() event.set_results({"user": email, "password": password}) From e26c0c97d7d396f9287d3bb38b09accd408428f3 Mon Sep 17 00:00:00 2001 From: codethulu Date: Tue, 11 Jun 2024 22:00:42 +0100 Subject: [PATCH 68/88] fixed integration test --- discourse_rock/patches/discourse-charm.patch | 2 +- discourse_rock/rockcraft.yaml | 1 + src/charm.py | 30 +++++++++++--------- tests/integration/test_charm.py | 14 ++++----- tests/integration/test_saml.py | 2 +- 5 files changed, 25 insertions(+), 24 deletions(-) diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index c822559f..68a0ff03 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -1,6 +1,6 @@ --- a/lib/tasks/discourse-charm.rake +++ b/lib/tasks/discourse-charm.rake -@@ -0,0 +1,12 @@ +@@ -0,0 +1,25 @@ +# frozen_string_literal: true + +desc "Check if a user exists for given email address" diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index f309670f..6d104639 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -6,6 +6,7 @@ summary: Discourse rock description: Discourse OCI image for the Discourse charm base: ubuntu@22.04 # renovate: base: ubuntu:22.04@sha256:19478ce7fc2ffbce89df29fea5725a8d12e57de52eb9ea570890dc5852aac1ac +run-user: _daemon_ license: Apache-2.0 version: "1.0" platforms: diff --git a/src/charm.py b/src/charm.py index 12e2d86f..be301abf 100755 --- a/src/charm.py +++ b/src/charm.py @@ -783,26 +783,28 @@ def _on_create_user_action(self, event: ActionEvent) -> None: environment=self._create_discourse_environment_settings(), timeout=60, ) - if not event.params.get("admin") and not event.params.get("active"): - activate_process = container.exec( - [ - os.path.join(DISCOURSE_PATH, "bin/bundle"), - "exec", - "rake", - f"users:activate[{email}]", - ], - working_dir=DISCOURSE_PATH, - user=CONTAINER_APP_USERNAME, - environment=self._create_discourse_environment_settings(), - ) - activate_process.wait_output() - logger.info(f"User {email} activated") try: process.wait_output() event.set_results({"user": email, "password": password}) except ExecError as ex: event.fail(f"Failed to make user with email {email}: {ex.stdout}") # type: ignore + if event.params.get("admin") or not event.params.get("active"): + return + + activate_process = container.exec( + [ + os.path.join(DISCOURSE_PATH, "bin/bundle"), + "exec", + "rake", + f"users:activate[{email}]", + ], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + ) + activate_process.wait_output() + def _generate_password(self, length: int) -> str: """Generate a random password. diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 88f386f5..bdda3889 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -404,7 +404,7 @@ async def test_promote_user( assert data["csrf"], data csrf = data["csrf"] - def get_api_key() -> str: + def get_api_key() -> bool: response = session.post( f"{discourse_address}/admin/api/keys", headers={ @@ -415,7 +415,9 @@ def get_api_key() -> str: json={"key": {"description": "admin-api-key", "username": None}}, timeout=60, ) - return response.json()["key"]["key"] + if "error_type" in response.json(): + return False + return True def attempt_login(email: str, password: str): response = session.post( @@ -437,17 +439,13 @@ def attempt_login(email: str, password: str): email = "test-promote-user@test.internal" discourse_unit: Unit = app.units[0] - create_action: Action = await discourse_unit.run_action( - "create-user", email=email, admin=False - ) + create_action: Action = await discourse_unit.run_action("create-user", email=email) await create_action.wait() assert create_action.results["user"] == email - logger.info("User created, attempting to login") - attempt_login(email, create_action.results["password"]) - assert get_api_key() is None, "This should fail as the user is not promoted" + assert not get_api_key(), "This should fail as the user is not promoted" promote_action: Action = await discourse_unit.run_action("promote-user", email=email) await promote_action.wait() diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py index ffbff78b..ea095361 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -38,7 +38,7 @@ async def test_saml_login( # pylint: disable=too-many-locals,too-many-arguments password = "test-discourse-k8s-password" # nosecue saml_helper.register_user(username=username, email=email, password=password) - action_result = await run_action(app.name, "create-user", email=email, admin=True) + action_result = await run_action(app.name, "create-user", email=email) assert "user" in action_result host = app.name From 4ba9a42eb287553a444756c87b28ecec3d32e9c9 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 12 Jun 2024 10:22:55 +0100 Subject: [PATCH 69/88] changed cindition, added some logging statements --- discourse_rock/rockcraft.yaml | 2 +- tests/integration/test_charm.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 6d104639..cdae23ef 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -6,7 +6,7 @@ summary: Discourse rock description: Discourse OCI image for the Discourse charm base: ubuntu@22.04 # renovate: base: ubuntu:22.04@sha256:19478ce7fc2ffbce89df29fea5725a8d12e57de52eb9ea570890dc5852aac1ac -run-user: _daemon_ +run-user: _daemon_ # UID/GID 584792 license: Apache-2.0 version: "1.0" platforms: diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index bdda3889..8a28b36c 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -415,7 +415,10 @@ def get_api_key() -> bool: json={"key": {"description": "admin-api-key", "username": None}}, timeout=60, ) - if "error_type" in response.json(): + logger.info("get_api_key response json: %s", response.json()) + logger.info("get_api_key response text: %s", response.text) + logger.info("get_api_key response cond: %s", str(response.json().get("key") is None)) + if response.json().get("key") is None: return False return True From 814cb2c0e0978bca4423e2c602df80401f335d50 Mon Sep 17 00:00:00 2001 From: Brendan Bell Date: Wed, 12 Jun 2024 10:45:54 +0100 Subject: [PATCH 70/88] Update rockcraft.yaml --- discourse_rock/rockcraft.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index cdae23ef..7f789e7e 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -6,7 +6,7 @@ summary: Discourse rock description: Discourse OCI image for the Discourse charm base: ubuntu@22.04 # renovate: base: ubuntu:22.04@sha256:19478ce7fc2ffbce89df29fea5725a8d12e57de52eb9ea570890dc5852aac1ac -run-user: _daemon_ # UID/GID 584792 +run-user: _daemon_ # UID/GID 584792 license: Apache-2.0 version: "1.0" platforms: From 45d0bb7e390af20dec2bb2685016eaf4321a3362 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 12 Jun 2024 12:32:49 +0100 Subject: [PATCH 71/88] added try except, figure out whats failing in ci --- tests/integration/test_charm.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 8a28b36c..60d36545 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -415,9 +415,6 @@ def get_api_key() -> bool: json={"key": {"description": "admin-api-key", "username": None}}, timeout=60, ) - logger.info("get_api_key response json: %s", response.json()) - logger.info("get_api_key response text: %s", response.text) - logger.info("get_api_key response cond: %s", str(response.json().get("key") is None)) if response.json().get("key") is None: return False return True @@ -438,6 +435,13 @@ def attempt_login(email: str, password: str): }, timeout=60, ) + try: + "error" not in response.json() + except Exception as e: + logger.error("Error in response: %s", response.text) + raise e + + assert response.ok, response.text # pylint: disable=no-member assert "error" not in response.json() email = "test-promote-user@test.internal" From 80e375584c824c1af84847e6a6d7927fe5bc6525 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 12 Jun 2024 15:34:54 +0100 Subject: [PATCH 72/88] wait for idle --- tests/integration/test_charm.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 60d36545..2884bd3f 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -388,12 +388,23 @@ async def test_create_user( async def test_promote_user( app: Application, discourse_address: str, + model: Model, + requests_timeout: float, ): """ arrange: A discourse application act: Promote a user to admin assert: User cannot access the admin API before being promoted """ + + def test_discourse_srv_status_ok(): + response = requests.get(f"{discourse_address}/srv/status", timeout=requests_timeout) + assert response.status_code == 200 + + # The charm should be active when starting this test + await model.wait_for_idle(status="active") + test_discourse_srv_status_ok() + with requests.session() as session: response = session.get( f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}, timeout=60 From a4bf63711c5626268bb004207787cf1946c641e5 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 12 Jun 2024 18:36:56 +0100 Subject: [PATCH 73/88] pinned redis channel to 28/edge --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index a88491a7..324c7898 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -157,7 +157,7 @@ async def app_fixture( async with ops_test.fast_forward(): await model.wait_for_idle(apps=[postgres_app.name], status="active") - redis_app = await model.deploy("redis-k8s", series="jammy", channel="latest/edge") + redis_app = await model.deploy("redis-k8s", series="jammy", channel="28/edge") await model.wait_for_idle(apps=[redis_app.name], status="active") await model.deploy("nginx-ingress-integrator", series="focal", trust=True) From 5e60183794135f10cd380b5164d8a16d00fa4ba8 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 12 Jun 2024 18:40:41 +0100 Subject: [PATCH 74/88] latest/stable --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 324c7898..fd3414dc 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -157,7 +157,7 @@ async def app_fixture( async with ops_test.fast_forward(): await model.wait_for_idle(apps=[postgres_app.name], status="active") - redis_app = await model.deploy("redis-k8s", series="jammy", channel="28/edge") + redis_app = await model.deploy("redis-k8s", series="jammy", channel="latest/stable") await model.wait_for_idle(apps=[redis_app.name], status="active") await model.deploy("nginx-ingress-integrator", series="focal", trust=True) From f4c5ab7d1ee59a224bd61e7c278efbe99e7ce59c Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 12 Jun 2024 22:11:38 +0100 Subject: [PATCH 75/88] rev 28 --- tests/integration/conftest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index fd3414dc..1024fab6 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -157,7 +157,8 @@ async def app_fixture( async with ops_test.fast_forward(): await model.wait_for_idle(apps=[postgres_app.name], status="active") - redis_app = await model.deploy("redis-k8s", series="jammy", channel="latest/stable") + # Revision 28 is being used due to https://github.com/canonical/redis-k8s-operator/issues/87. + redis_app = await model.deploy("redis-k8s", series="jammy", channel="latest/edge", revision=28) await model.wait_for_idle(apps=[redis_app.name], status="active") await model.deploy("nginx-ingress-integrator", series="focal", trust=True) From 300d64f7f52f95399a8a51426bfeb23f14cbb208 Mon Sep 17 00:00:00 2001 From: codethulu Date: Wed, 12 Jun 2024 23:49:15 +0100 Subject: [PATCH 76/88] moved user testing to separate module --- .github/workflows/integration_test.yaml | 2 +- tests/integration/test_charm.py | 111 --------------------- tests/integration/test_users.py | 124 ++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 112 deletions(-) create mode 100644 tests/integration/test_users.py diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index ebd40e93..9a412adc 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -13,6 +13,6 @@ jobs: trivy-image-config: "trivy.yaml" juju-channel: 3.1/stable channel: 1.28-strict/stable - modules: '["test_charm", "test_saml"]' + modules: '["test_charm", "test_saml", "test_users"]' self-hosted-runner: true self-hosted-runner-label: "edge" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 2884bd3f..23de5966 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -13,9 +13,7 @@ import requests from boto3 import client from botocore.config import Config -from juju.action import Action from juju.application import Application -from juju.unit import Unit from ops.model import ActiveStatus from pytest_operator.plugin import Model, OpsTest @@ -360,112 +358,3 @@ def _upgrade_finished(): await model.block_until(upgrade_finished(), timeout=10 * 60) check_alive() - - -@pytest.mark.asyncio -async def test_create_user( - app: Application, -): - """ - arrange: A discourse application - act: Create a user - assert: User is created, and re-creating the same user should fail - """ - - email = "admin-user@test.internal" - discourse_unit: Unit = app.units[0] - action: Action = await discourse_unit.run_action("create-user", email=email) - await action.wait() - assert action.results["user"] == email - - # Re-creating the same user should fail, as the user already exists - break_action: Action = await discourse_unit.run_action("create-user", email=email) - await break_action.wait() - assert break_action.status == "failed" - - -@pytest.mark.asyncio -async def test_promote_user( - app: Application, - discourse_address: str, - model: Model, - requests_timeout: float, -): - """ - arrange: A discourse application - act: Promote a user to admin - assert: User cannot access the admin API before being promoted - """ - - def test_discourse_srv_status_ok(): - response = requests.get(f"{discourse_address}/srv/status", timeout=requests_timeout) - assert response.status_code == 200 - - # The charm should be active when starting this test - await model.wait_for_idle(status="active") - test_discourse_srv_status_ok() - - with requests.session() as session: - response = session.get( - f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}, timeout=60 - ) - # pylint doesn't see the "ok" member - assert response.ok, response.text # pylint: disable=no-member - data = response.json() - assert data["csrf"], data - csrf = data["csrf"] - - def get_api_key() -> bool: - response = session.post( - f"{discourse_address}/admin/api/keys", - headers={ - "Content-Type": "application/json", - "X-CSRF-Token": csrf, - "X-Requested-With": "XMLHttpRequest", - }, - json={"key": {"description": "admin-api-key", "username": None}}, - timeout=60, - ) - if response.json().get("key") is None: - return False - return True - - def attempt_login(email: str, password: str): - response = session.post( - f"{discourse_address}/session", - headers={ - "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8", - "X-CSRF-Token": csrf, - "X-Requested-With": "XMLHttpRequest", - }, - data={ - "login": email, - "password": password, - "second_factor_method": "1", - "timezone": "Asia/Hong_Kong", - }, - timeout=60, - ) - try: - "error" not in response.json() - except Exception as e: - logger.error("Error in response: %s", response.text) - raise e - - assert response.ok, response.text # pylint: disable=no-member - assert "error" not in response.json() - - email = "test-promote-user@test.internal" - discourse_unit: Unit = app.units[0] - create_action: Action = await discourse_unit.run_action("create-user", email=email) - await create_action.wait() - assert create_action.results["user"] == email - - attempt_login(email, create_action.results["password"]) - - assert not get_api_key(), "This should fail as the user is not promoted" - - promote_action: Action = await discourse_unit.run_action("promote-user", email=email) - await promote_action.wait() - - assert get_api_key(), "This should succeed as the user is promoted" diff --git a/tests/integration/test_users.py b/tests/integration/test_users.py new file mode 100644 index 00000000..8eab7e27 --- /dev/null +++ b/tests/integration/test_users.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Discourse integration tests.""" + +import logging + +import pytest +import requests +from juju.action import Action +from juju.application import Application +from juju.unit import Unit +from pytest_operator.plugin import Model + +logger = logging.getLogger(__name__) + + +@pytest.mark.asyncio +async def test_create_user( + app: Application, +): + """ + arrange: A discourse application + act: Create a user + assert: User is created, and re-creating the same user should fail + """ + + await app.model.wait_for_idle(status="active") + discourse_unit: Unit = app.units[0] + + email = "admin-user@test.internal" + + action: Action = await discourse_unit.run_action("create-user", email=email) + await action.wait() + assert action.results["user"] == email + + # Re-creating the same user should fail, as the user already exists + break_action: Action = await discourse_unit.run_action("create-user", email=email) + await break_action.wait() + assert break_action.status == "failed" + + +@pytest.mark.asyncio +async def test_promote_user( + app: Application, + discourse_address: str, + model: Model, + requests_timeout: float, +): + """ + arrange: A discourse application + act: Promote a user to admin + assert: User cannot access the admin API before being promoted + """ + + def test_discourse_srv_status_ok(): + response = requests.get(f"{discourse_address}/srv/status", timeout=requests_timeout) + assert response.status_code == 200 + + # The charm should be active when starting this test + await model.wait_for_idle(status="active") + test_discourse_srv_status_ok() + + with requests.session() as session: + + def get_api_key(csrf_token: str) -> bool: + response = session.post( + f"{discourse_address}/admin/api/keys", + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": csrf_token, + "X-Requested-With": "XMLHttpRequest", + }, + json={"key": {"description": "admin-api-key", "username": None}}, + ) + if response.json().get("key") is None: + return False + return True + + response = session.get( + f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}, timeout=60 + ) + # pylint doesn't see the "ok" member + assert response.ok, response.text # pylint: disable=no-member + data = response.json() + assert data["csrf"], data + csrf = data["csrf"] + + email = "test-promote-user@test.internal" + discourse_unit: Unit = app.units[0] + create_action: Action = await discourse_unit.run_action("create-user", email=email) + await create_action.wait() + assert create_action.results["user"] == email + + response = session.post( + f"{discourse_address}/session", + headers={ + "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8", + "X-CSRF-Token": csrf, + "X-Requested-With": "XMLHttpRequest", + }, + data={ + "login": email, + "password": create_action.results["password"], + "second_factor_method": "1", + "timezone": "Asia/Hong_Kong", + }, + ) + + try: + "error" not in response.json() + except Exception as e: + logger.error("Error in response: %s", response.text) + raise e + + assert response.ok, response.text # pylint: disable=no-member + assert "error" not in response.json() + + assert not get_api_key(csrf), "This should fail as the user is not promoted" + + promote_action: Action = await discourse_unit.run_action("promote-user", email=email) + await promote_action.wait() + + assert get_api_key(csrf), "This should succeed as the user is promoted" From a2c0d7d08fa833dc525639e94fe34213d68031c6 Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 13 Jun 2024 17:34:33 +0100 Subject: [PATCH 77/88] addressed changes --- discourse_rock/patches/discourse-charm.patch | 4 +-- docs/how-to/contribute.md | 2 +- src/charm.py | 32 +++++++++++++++++--- tests/integration/conftest.py | 3 +- tests/integration/test_users.py | 17 ----------- 5 files changed, 31 insertions(+), 27 deletions(-) diff --git a/discourse_rock/patches/discourse-charm.patch b/discourse_rock/patches/discourse-charm.patch index 68a0ff03..3aac35df 100644 --- a/discourse_rock/patches/discourse-charm.patch +++ b/discourse_rock/patches/discourse-charm.patch @@ -11,7 +11,7 @@ + exit 0 + end + puts "ERROR: User with email #{email} not found" -+ exit 1 ++ exit 2 +end + +desc "Activate a user account" @@ -20,7 +20,7 @@ + user = User.find_by_email(email) + if !user + puts "User with email #{email} does not exist" -+ exit 1 ++ exit 2 + end + user.email_tokens.update_all(confirmed: true) + puts "User with email #{email} activated" diff --git a/docs/how-to/contribute.md b/docs/how-to/contribute.md index 052aa594..7d6de760 100644 --- a/docs/how-to/contribute.md +++ b/docs/how-to/contribute.md @@ -83,7 +83,7 @@ the registry: ```shell cd [project_dir]/discourse_rock && rockcraft pack rockcraft.yaml - skopeo --insecure-policy copy --dest-tls-verify=false oci-archive:discourse_1.0_amd64.rock docker://localhost:32000/discourse:latest + rockcraft.skopeo --insecure-policy copy --dest-tls-verify=false oci-archive:discourse_1.0_amd64.rock docker://localhost:32000/discourse:latest ``` ### Deploy diff --git a/src/charm.py b/src/charm.py index be301abf..268f9668 100755 --- a/src/charm.py +++ b/src/charm.py @@ -718,8 +718,13 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: ) try: user_exists.wait_output() - except ExecError: - event.fail(f"User with email {email} does not exist") + except ExecError as ex: + if ex.exit_code == 2: + event.fail(f"User with email {email} does not exist") + else: + event.fail( + f"Error checking if user with email {email} exists: {ex.stdout}" # type: ignore + ) return process = container.exec( @@ -766,8 +771,12 @@ def _on_create_user_action(self, event: ActionEvent) -> None: user_exists.wait_output() event.fail(f"User with email {email} already exists") return - except ExecError: - pass + except ExecError as ex: + if ex.exit_code == 2: + pass + else: + event.fail(f"Error checking if user with email {email} exists: {ex.stdout}") + return # Admin flag is optional, if it is true, the user will be created as an admin admin_flag = "Y" if event.params.get("admin") else "N" process = container.exec( @@ -788,6 +797,7 @@ def _on_create_user_action(self, event: ActionEvent) -> None: event.set_results({"user": email, "password": password}) except ExecError as ex: event.fail(f"Failed to make user with email {email}: {ex.stdout}") # type: ignore + return if event.params.get("admin") or not event.params.get("active"): return @@ -803,7 +813,19 @@ def _on_create_user_action(self, event: ActionEvent) -> None: user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), ) - activate_process.wait_output() + try: + activate_process.wait_output() + except ExecError as ex: + if ex.exit_code == 2: + event.fail( + f"Could not find user {email} for activation: {ex.stdout}" # type: ignore + ) + return + event.fail( + # Parameter validation errors are printed to stdout + # Ignore mypy warning when formatting stdout + f"Failed to activate user with email {email}: {ex.stdout}" # type: ignore + ) def _generate_password(self, length: int) -> str: """Generate a random password. diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 1024fab6..a88491a7 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -157,8 +157,7 @@ async def app_fixture( async with ops_test.fast_forward(): await model.wait_for_idle(apps=[postgres_app.name], status="active") - # Revision 28 is being used due to https://github.com/canonical/redis-k8s-operator/issues/87. - redis_app = await model.deploy("redis-k8s", series="jammy", channel="latest/edge", revision=28) + redis_app = await model.deploy("redis-k8s", series="jammy", channel="latest/edge") await model.wait_for_idle(apps=[redis_app.name], status="active") await model.deploy("nginx-ingress-integrator", series="focal", trust=True) diff --git a/tests/integration/test_users.py b/tests/integration/test_users.py index 8eab7e27..85db149a 100644 --- a/tests/integration/test_users.py +++ b/tests/integration/test_users.py @@ -10,7 +10,6 @@ from juju.action import Action from juju.application import Application from juju.unit import Unit -from pytest_operator.plugin import Model logger = logging.getLogger(__name__) @@ -44,8 +43,6 @@ async def test_create_user( async def test_promote_user( app: Application, discourse_address: str, - model: Model, - requests_timeout: float, ): """ arrange: A discourse application @@ -53,14 +50,6 @@ async def test_promote_user( assert: User cannot access the admin API before being promoted """ - def test_discourse_srv_status_ok(): - response = requests.get(f"{discourse_address}/srv/status", timeout=requests_timeout) - assert response.status_code == 200 - - # The charm should be active when starting this test - await model.wait_for_idle(status="active") - test_discourse_srv_status_ok() - with requests.session() as session: def get_api_key(csrf_token: str) -> bool: @@ -107,12 +96,6 @@ def get_api_key(csrf_token: str) -> bool: }, ) - try: - "error" not in response.json() - except Exception as e: - logger.error("Error in response: %s", response.text) - raise e - assert response.ok, response.text # pylint: disable=no-member assert "error" not in response.json() From 3121bc5478cd2c6b32a0eb111375cd12df8137cd Mon Sep 17 00:00:00 2001 From: codethulu Date: Thu, 13 Jun 2024 17:42:24 +0100 Subject: [PATCH 78/88] fixed unit test exit code --- tests/unit/test_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 392c6009..2b471864 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -383,7 +383,7 @@ def mock_wo_side_effect(*args, **kwargs): # pylint: disable=unused-argument """ if isinstance(mock_wo.cmd, list) and f"users:exists[{email}]" in mock_wo.cmd: - raise ExecError(command=mock_wo.cmd, exit_code=42, stdout=stdout_mock, stderr="") + raise ExecError(command=mock_wo.cmd, exit_code=2, stdout=stdout_mock, stderr="") return DEFAULT From 125a24bfe8c3fe948d90cd5c1c8280111a952915 Mon Sep 17 00:00:00 2001 From: Brendan Bell Date: Fri, 14 Jun 2024 01:11:08 +0100 Subject: [PATCH 79/88] Update rockcraft.yaml --- discourse_rock/rockcraft.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/discourse_rock/rockcraft.yaml b/discourse_rock/rockcraft.yaml index 7f789e7e..50d4692e 100644 --- a/discourse_rock/rockcraft.yaml +++ b/discourse_rock/rockcraft.yaml @@ -186,7 +186,6 @@ parts: after: [discourse, patches] override-stage: | git -C srv/discourse/app apply patches/lp1903695.patch - touch srv/discourse/app/patches/discourse-charm.rake git -C srv/discourse/app apply patches/discourse-charm.patch git -C srv/discourse/app apply patches/sigterm.patch # The following is a fix for UglifierJS assets compilation From 1c0bc3f1db4893d38ba330dcb58705db633688d3 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 14 Jun 2024 09:55:46 +0100 Subject: [PATCH 80/88] fixed unit tests --- src/charm.py | 60 +++++++++++++++++++++------------------- tests/unit/test_charm.py | 23 ++++++--------- 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/src/charm.py b/src/charm.py index 268f9668..b29ed8c9 100755 --- a/src/charm.py +++ b/src/charm.py @@ -696,20 +696,17 @@ def _activate_charm(self) -> None: if self._is_config_valid() and container.can_connect(): self._start_service() self.model.unit.status = ActiveStatus() - - def _on_promote_user_action(self, event: ActionEvent) -> None: - """Promote a user to a specific trust level. + + def _user_exists(self, email: str) -> bool: + """Check if a user with the given email exists. Args: - event: Event triggering the promote_user action. + email: Email of the user to check. + + Returns: + True if the user exists, False otherwise. """ container = self.unit.get_container(CONTAINER_NAME) - if not container.can_connect(): - event.fail("Unable to connect to container, container is not ready") - return - - email = event.params["email"] - user_exists = container.exec( [os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", f"users:exists[{email}]"], working_dir=DISCOURSE_PATH, @@ -718,13 +715,27 @@ def _on_promote_user_action(self, event: ActionEvent) -> None: ) try: user_exists.wait_output() + return True except ExecError as ex: if ex.exit_code == 2: - event.fail(f"User with email {email} does not exist") - else: - event.fail( - f"Error checking if user with email {email} exists: {ex.stdout}" # type: ignore - ) + return False + raise ex + + def _on_promote_user_action(self, event: ActionEvent) -> None: + """Promote a user to a specific trust level. + + Args: + event: Event triggering the promote_user action. + """ + container = self.unit.get_container(CONTAINER_NAME) + if not container.can_connect(): + event.fail("Unable to connect to container, container is not ready") + return + + email = event.params["email"] + + if not self._user_exists(email): + event.fail(f"User with email {email} does not exist") return process = container.exec( @@ -761,24 +772,14 @@ def _on_create_user_action(self, event: ActionEvent) -> None: email = event.params["email"] password = self._generate_password(16) - user_exists = container.exec( - [os.path.join(DISCOURSE_PATH, "bin/bundle"), "exec", "rake", f"users:exists[{email}]"], - working_dir=DISCOURSE_PATH, - user=CONTAINER_APP_USERNAME, - environment=self._create_discourse_environment_settings(), - ) - try: - user_exists.wait_output() + + if self._user_exists(email): event.fail(f"User with email {email} already exists") return - except ExecError as ex: - if ex.exit_code == 2: - pass - else: - event.fail(f"Error checking if user with email {email} exists: {ex.stdout}") - return + # Admin flag is optional, if it is true, the user will be created as an admin admin_flag = "Y" if event.params.get("admin") else "N" + process = container.exec( [ os.path.join(DISCOURSE_PATH, "bin/bundle"), @@ -813,6 +814,7 @@ def _on_create_user_action(self, event: ActionEvent) -> None: user=CONTAINER_APP_USERNAME, environment=self._create_discourse_environment_settings(), ) + try: activate_process.wait_output() except ExecError as ex: diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 2b471864..6450c19c 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -342,12 +342,8 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: handler=bundle_handler, ) - charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) - email = "sample@email.com" - event = MagicMock(spec=ActionEvent) - event.params = {"email": email} - charm._on_promote_user_action(event) + harness.run_action("promote-user", {"email": email}) assert expected_exec_call_was_made @@ -361,11 +357,13 @@ def test_create_user(mock_exec): mock_wo = MagicMock( return_value=("", None), ) - stdout_mock = "CRASH" + + email = "admin-user@test.internal" + + stdout_mock = f"ERROR: User with email {email} not found" harness = helpers.start_harness(run_initial_hooks=False) harness.set_can_connect(CONTAINER_NAME, True) - email = "admin-user@test.internal" expected_exec_call_was_made = False def mock_wo_side_effect(*args, **kwargs): # pylint: disable=unused-argument @@ -420,10 +418,7 @@ def mock_exec_side_effect(*args, **kwargs): # pylint: disable=unused-argument wait_output=mock_wo, ) - charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) - event = MagicMock(spec=ActionEvent) - event.params = {"email": email} - charm._on_create_user_action(event) + harness.run_action("create-user", {"email": email}) assert expected_exec_call_was_made @@ -456,10 +451,8 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", f"users:anonymize[{username}]"], handler=bundle_handler, ) - charm: DiscourseCharm = typing.cast(DiscourseCharm, harness.charm) - event = MagicMock(spec=ActionEvent) - event.params = {"username": username} - charm._on_anonymize_user_action(event) + + harness.run_action("anonymize-user", {"username": username}) assert expected_exec_call_was_made From db34e09ec8ff55c0d88ed624b70d3e8b3e0d8c54 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 14 Jun 2024 09:56:21 +0100 Subject: [PATCH 81/88] fixed unit tests --- tests/unit/test_charm.py | 79 ++++++++++------------------------------ 1 file changed, 20 insertions(+), 59 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 6450c19c..8725d1c7 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -354,71 +354,32 @@ def test_create_user(mock_exec): act: when the _on_create_user_action method is executed assert: the create user rake command is executed upon failure of the user existence check. """ - mock_wo = MagicMock( - return_value=("", None), - ) - - email = "admin-user@test.internal" - - stdout_mock = f"ERROR: User with email {email} not found" - harness = helpers.start_harness(run_initial_hooks=False) - harness.set_can_connect(CONTAINER_NAME, True) + harness = helpers.start_harness() + # We catch the exec call that we expect to register it and make sure that the + # args passed to it are correct. expected_exec_call_was_made = False - def mock_wo_side_effect(*args, **kwargs): # pylint: disable=unused-argument - """Mock wo side effect. - - Args: - args: Variable list of positional arguments passed to the parent constructor. - kwargs: a `dict` of the extra arguments passed to the function. - - Returns: - unittest.mock DEFAULT built-in. - - Raises: - ExecError: Execution error fired if conditions are met. - """ - - if isinstance(mock_wo.cmd, list) and f"users:exists[{email}]" in mock_wo.cmd: - raise ExecError(command=mock_wo.cmd, exit_code=2, stdout=stdout_mock, stderr="") - - return DEFAULT - - mock_wo.side_effect = mock_wo_side_effect - - def mock_exec_side_effect(*args, **kwargs): # pylint: disable=unused-argument - """Mock execution side effect. - - Args: - args: Variable list of positional arguments passed to the parent constructor. - kwargs: a `dict` of the extra arguments passed to the function. - - Returns: - unittest.mock DEFAULT built-in. - """ - mock_wo.cmd = args[0] - - if isinstance(mock_wo.cmd, list) and "admin:create" in mock_wo.cmd: - nonlocal expected_exec_call_was_made - expected_exec_call_was_made = True - if ( - kwargs["environment"] != harness.charm._create_discourse_environment_settings() - or kwargs["working_dir"] != DISCOURSE_PATH - or kwargs["user"] != "_daemon_" - or email not in kwargs["stdin"] - or kwargs["timeout"] != 60 - ): - raise ValueError(f"{mock_wo.cmd} wasn't made with the correct args.") - - return DEFAULT + def bundle_handler(args: ops.testing.ExecArgs) -> None: + nonlocal expected_exec_call_was_made + expected_exec_call_was_made = True + if ( + args.environment != harness.charm._create_discourse_environment_settings() + or args.working_dir != DISCOURSE_PATH + or args.user != "_daemon_" + or args.stdin != f"{email}\nn\nY\n" + or args.timeout != 60 + ): + raise ValueError(f"{args.command} wasn't made with the correct args.") - mock_exec.side_effect = mock_exec_side_effect - mock_exec.return_value = MagicMock( - wait_output=mock_wo, + harness.handle_exec( + SERVICE_NAME, + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "admin:create"], + handler=bundle_handler, ) - harness.run_action("create-user", {"email": email}) + email = "sample@email.com" + harness.run_action("promote-user", {"email": email}) assert expected_exec_call_was_made From de4b0ac460be293b26b242721975e170dd6a089c Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 14 Jun 2024 09:59:57 +0100 Subject: [PATCH 82/88] fixed all unit tests --- tests/unit/test_charm.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 8725d1c7..9b3c99c1 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -347,8 +347,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: assert expected_exec_call_was_made -@patch.object(ops.Container, "exec") -def test_create_user(mock_exec): +def test_create_user(): """ arrange: an email act: when the _on_create_user_action method is executed @@ -367,19 +366,26 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: args.environment != harness.charm._create_discourse_environment_settings() or args.working_dir != DISCOURSE_PATH or args.user != "_daemon_" - or args.stdin != f"{email}\nn\nY\n" or args.timeout != 60 ): raise ValueError(f"{args.command} wasn't made with the correct args.") + email = "sample@email.com" + harness.handle_exec( SERVICE_NAME, [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "admin:create"], handler=bundle_handler, ) - email = "sample@email.com" - harness.run_action("promote-user", {"email": email}) + stdout = "ERROR: User with email f{email} not found" + harness.handle_exec( + SERVICE_NAME, + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", f"users:exists[{email}]"], + result=ops.testing.ExecResult(exit_code=2, stdout=stdout, stderr=""), + ) + + harness.run_action("create-user", {"email": email}) assert expected_exec_call_was_made From 2a7c8b9cbad69382c41b5993a7d52dc09484e1c6 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 14 Jun 2024 10:06:59 +0100 Subject: [PATCH 83/88] changes --- tests/unit/test_charm.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 9b3c99c1..4b2f435c 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -358,24 +358,24 @@ def test_create_user(): # We catch the exec call that we expect to register it and make sure that the # args passed to it are correct. expected_exec_call_was_made = False + email = "sample@email.com" - def bundle_handler(args: ops.testing.ExecArgs) -> None: + def mock_create_user(args: ops.testing.ExecArgs) -> None: nonlocal expected_exec_call_was_made expected_exec_call_was_made = True if ( args.environment != harness.charm._create_discourse_environment_settings() or args.working_dir != DISCOURSE_PATH + or email not in args.stdin or args.user != "_daemon_" or args.timeout != 60 ): raise ValueError(f"{args.command} wasn't made with the correct args.") - email = "sample@email.com" - harness.handle_exec( SERVICE_NAME, [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "admin:create"], - handler=bundle_handler, + handler=mock_create_user, ) stdout = "ERROR: User with email f{email} not found" From 8b74d84ca46cc0fb99cdedcb903217f185f1d7fc Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 14 Jun 2024 13:25:27 +0100 Subject: [PATCH 84/88] unit test coverage --- src/charm.py | 2 +- tests/unit/test_charm.py | 109 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 102 insertions(+), 9 deletions(-) diff --git a/src/charm.py b/src/charm.py index b29ed8c9..55783bc1 100755 --- a/src/charm.py +++ b/src/charm.py @@ -696,7 +696,7 @@ def _activate_charm(self) -> None: if self._is_config_valid() and container.can_connect(): self._start_service() self.model.unit.status = ActiveStatus() - + def _user_exists(self, email: str) -> bool: """Check if a user with the given email exists. diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 4b2f435c..b9486b09 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -7,16 +7,13 @@ # Protected access check is disabled in tests as we're injecting test data import secrets -import typing -from unittest.mock import DEFAULT, MagicMock, patch +from unittest.mock import MagicMock, patch import ops import pytest -from ops.charm import ActionEvent from ops.model import ActiveStatus, BlockedStatus, WaitingStatus -from ops.pebble import ExecError -from charm import CONTAINER_NAME, DATABASE_NAME, DISCOURSE_PATH, SERVICE_NAME, DiscourseCharm +from charm import CONTAINER_NAME, DATABASE_NAME, DISCOURSE_PATH, SERVICE_NAME from tests.unit import helpers @@ -311,7 +308,7 @@ def test_db_relation(): ), "database name should be set after relation joined" -def test_promote_user(): +def test_promote_user_success(): """ arrange: an email and a password act: when the _on_promote_user_action method is executed @@ -347,7 +344,7 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: assert expected_exec_call_was_made -def test_create_user(): +def test_promote_user_fail(): """ arrange: an email act: when the _on_create_user_action method is executed @@ -366,7 +363,65 @@ def mock_create_user(args: ops.testing.ExecArgs) -> None: if ( args.environment != harness.charm._create_discourse_environment_settings() or args.working_dir != DISCOURSE_PATH - or email not in args.stdin + or email not in str(args.stdin) + or args.user != "_daemon_" + or args.timeout != 60 + ): + raise ValueError(f"{args.command} wasn't made with the correct args.") + + harness.handle_exec( + SERVICE_NAME, + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "admin:create"], + handler=mock_create_user, + ) + + stdout = "ERROR: User with email f{email} not found" + + # Exit code 2 means that the user cannot be found in the rake task. + harness.handle_exec( + SERVICE_NAME, + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", f"users:exists[{email}]"], + result=ops.testing.ExecResult(exit_code=2, stdout=stdout, stderr=""), + ) + try: + harness.run_action("promote-user", {"email": email}) + assert False + except ops.testing.ActionFailed as e: + assert e.message == f"User with email {email} does not exist" + + # Exit code 1 means that the rake task failed. + harness.handle_exec( + SERVICE_NAME, + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", f"users:exists[{email}]"], + result=ops.testing.ExecResult(exit_code=1, stdout=stdout, stderr=""), + ) + try: + harness.run_action("promote-user", {"email": email}) + assert False + except ops.pebble.ExecError as e: + assert "non-zero exit code 1" in str(e) + + +def test_create_user_success(): + """ + arrange: an email + act: when the _on_create_user_action method is executed + assert: the create user rake command is executed upon failure of the user existence check. + """ + harness = helpers.start_harness() + + # We catch the exec call that we expect to register it and make sure that the + # args passed to it are correct. + expected_exec_call_was_made = False + email = "sample@email.com" + + def mock_create_user(args: ops.testing.ExecArgs) -> None: + nonlocal expected_exec_call_was_made + expected_exec_call_was_made = True + if ( + args.environment != harness.charm._create_discourse_environment_settings() + or args.working_dir != DISCOURSE_PATH + or email not in str(args.stdin) or args.user != "_daemon_" or args.timeout != 60 ): @@ -389,6 +444,44 @@ def mock_create_user(args: ops.testing.ExecArgs) -> None: assert expected_exec_call_was_made +def test_create_user_fail(): + """ + arrange: an email + act: when the _on_create_user_action method is executed + assert: the create user rake command is executed upon failure of the user existence check. + """ + harness = helpers.start_harness() + + # We catch the exec call that we expect to register it and make sure that the + # args passed to it are correct. + expected_exec_call_was_made = False + email = "sample@email.com" + + def mock_create_user(args: ops.testing.ExecArgs) -> None: + nonlocal expected_exec_call_was_made + expected_exec_call_was_made = True + if ( + args.environment != harness.charm._create_discourse_environment_settings() + or args.working_dir != DISCOURSE_PATH + or email not in str(args.stdin) + or args.user != "_daemon_" + or args.timeout != 60 + ): + raise ValueError(f"{args.command} wasn't made with the correct args.") + + harness.handle_exec( + SERVICE_NAME, + [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "admin:create"], + handler=mock_create_user, + ) + + try: + harness.run_action("create-user", {"email": email}) + assert False + except ops.testing.ActionFailed as e: + assert e.message == f"User with email {email} already exists" + + def test_anonymize_user(): """ arrange: set up discourse From 3285716d804d3ef0cefe275963d216b139719365 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 14 Jun 2024 14:54:18 +0100 Subject: [PATCH 85/88] refactor --- src/charm.py | 63 +++++++++++++++++---------------- tests/integration/test_users.py | 3 +- 2 files changed, 35 insertions(+), 31 deletions(-) diff --git a/src/charm.py b/src/charm.py index 55783bc1..5696dee6 100755 --- a/src/charm.py +++ b/src/charm.py @@ -721,6 +721,32 @@ def _user_exists(self, email: str) -> bool: return False raise ex + def _activate_user(self, email: str) -> bool: + """Activate a user with the given email. + + Args: + email: Email of the user to activate. + """ + container = self.unit.get_container(CONTAINER_NAME) + activate_process = container.exec( + [ + os.path.join(DISCOURSE_PATH, "bin/bundle"), + "exec", + "rake", + f"users:activate[{email}]", + ], + working_dir=DISCOURSE_PATH, + user=CONTAINER_APP_USERNAME, + environment=self._create_discourse_environment_settings(), + ) + try: + activate_process.wait_output() + return True + except ExecError as ex: + if ex.exit_code == 2: + return False + raise ex + def _on_promote_user_action(self, event: ActionEvent) -> None: """Promote a user to a specific trust level. @@ -795,39 +821,16 @@ def _on_create_user_action(self, event: ActionEvent) -> None: ) try: process.wait_output() - event.set_results({"user": email, "password": password}) except ExecError as ex: event.fail(f"Failed to make user with email {email}: {ex.stdout}") # type: ignore return - if event.params.get("admin") or not event.params.get("active"): - return - - activate_process = container.exec( - [ - os.path.join(DISCOURSE_PATH, "bin/bundle"), - "exec", - "rake", - f"users:activate[{email}]", - ], - working_dir=DISCOURSE_PATH, - user=CONTAINER_APP_USERNAME, - environment=self._create_discourse_environment_settings(), - ) - - try: - activate_process.wait_output() - except ExecError as ex: - if ex.exit_code == 2: - event.fail( - f"Could not find user {email} for activation: {ex.stdout}" # type: ignore - ) + if not event.params.get("admin") and event.params.get("active"): + if not self._activate_user(email): + event.fail(f"Could not find user {email} to activate") return - event.fail( - # Parameter validation errors are printed to stdout - # Ignore mypy warning when formatting stdout - f"Failed to activate user with email {email}: {ex.stdout}" # type: ignore - ) + + event.set_results({"user": email, "password": password}) def _generate_password(self, length: int) -> str: """Generate a random password. @@ -844,7 +847,7 @@ def _generate_password(self, length: int) -> str: def _config_force_https(self) -> None: """Config Discourse to force_https option based on charm configuration.""" - container = self.unit.get_container("discourse") + container = self.unit.get_container(CONTAINER_NAME) force_bool = str(self.config["force_https"]).lower() process = container.exec( [ @@ -865,7 +868,7 @@ def _on_anonymize_user_action(self, event: ActionEvent) -> None: event: Event triggering the anonymize_user action. """ username = event.params["username"] - container = self.unit.get_container("discourse") + container = self.unit.get_container(CONTAINER_NAME) if not container.can_connect(): event.fail("Unable to connect to container, container is not ready") return diff --git a/tests/integration/test_users.py b/tests/integration/test_users.py index 85db149a..ac5474b9 100644 --- a/tests/integration/test_users.py +++ b/tests/integration/test_users.py @@ -17,6 +17,7 @@ @pytest.mark.asyncio async def test_create_user( app: Application, + discourse_address: str, ): """ arrange: A discourse application @@ -27,7 +28,7 @@ async def test_create_user( await app.model.wait_for_idle(status="active") discourse_unit: Unit = app.units[0] - email = "admin-user@test.internal" + email = "test-user@test.internal" action: Action = await discourse_unit.run_action("create-user", email=email) await action.wait() From ef2e9aae8a9c58bbea51544d642da5536e5020ec Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 14 Jun 2024 14:54:48 +0100 Subject: [PATCH 86/88] lint --- tests/integration/test_users.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/test_users.py b/tests/integration/test_users.py index ac5474b9..5a29899f 100644 --- a/tests/integration/test_users.py +++ b/tests/integration/test_users.py @@ -17,7 +17,6 @@ @pytest.mark.asyncio async def test_create_user( app: Application, - discourse_address: str, ): """ arrange: A discourse application From 79d3e575e46df3fad6c6467dacff3c2051d714b2 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 14 Jun 2024 15:05:02 +0100 Subject: [PATCH 87/88] raise w/o arguments --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 5696dee6..0bb9fc24 100755 --- a/src/charm.py +++ b/src/charm.py @@ -719,7 +719,7 @@ def _user_exists(self, email: str) -> bool: except ExecError as ex: if ex.exit_code == 2: return False - raise ex + raise def _activate_user(self, email: str) -> bool: """Activate a user with the given email. From 6df044233c2d2b49c178c53e2e7ae3cdf42dd168 Mon Sep 17 00:00:00 2001 From: codethulu Date: Fri, 14 Jun 2024 15:25:33 +0100 Subject: [PATCH 88/88] removed arguments of raise --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 0bb9fc24..57e9390f 100755 --- a/src/charm.py +++ b/src/charm.py @@ -745,7 +745,7 @@ def _activate_user(self, email: str) -> bool: except ExecError as ex: if ex.exit_code == 2: return False - raise ex + raise def _on_promote_user_action(self, event: ActionEvent) -> None: """Promote a user to a specific trust level.