-
Notifications
You must be signed in to change notification settings - Fork 2
feat(garm): add get-credentials action #254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
380cdd0
37f4360
c650a3d
bee811c
c087ce6
2d65888
41cfabb
d7c09d6
f62e050
6803af9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,4 +43,7 @@ build/ | |
|
|
||
| # Editor/IDE | ||
| .idea/ | ||
| .vscode/ | ||
| .vscode/ | ||
|
|
||
| # Git worktrees | ||
| .worktrees/ | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -520,6 +520,44 @@ def test_maybe_first_run_skips_on_missing_credential_key(): | |||||||||
| mock_client_cls.assert_not_called() | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def test_get_credentials_action_returns_credentials_when_available(): | ||||||||||
| """ | ||||||||||
| arrange: Admin credentials secret exists and contains valid content. | ||||||||||
| act: Call _on_get_credentials_action(). | ||||||||||
| """ | ||||||||||
| charm = MagicMock() | ||||||||||
| event = MagicMock() | ||||||||||
| credentials = { | ||||||||||
| "username": "admin", | ||||||||||
| "password": "s3cr3t", | ||||||||||
| "email": "admin@garm.local", | ||||||||||
| "full-name": "GARM Admin", | ||||||||||
| } | ||||||||||
| charm._get_admin_credentials.return_value = credentials | ||||||||||
|
|
||||||||||
| GarmCharm._on_get_credentials_action(charm, event) | ||||||||||
|
|
||||||||||
| event.set_results.assert_called_once_with(credentials) | ||||||||||
| event.fail.assert_not_called() | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add assertion that the credentials returned is as expected
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's what the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i see, is there a reason why we cannot use scenario? ideally it's better to test the state, not interaction
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No particular reason aside from that I like to keep tests simple if it's only a single method. And using scenario we'd still only call the hook method so it's a bit more overhead. But I can switch to scenario if needed, wdyt? |
||||||||||
|
|
||||||||||
|
|
||||||||||
| def test_get_credentials_action_fails_when_credentials_unavailable(): | ||||||||||
| """ | ||||||||||
| arrange: Admin credentials secret does not exist yet. | ||||||||||
| act: Call _on_get_credentials_action(). | ||||||||||
| assert: event.fail is called with a message containing "not yet available" and | ||||||||||
| event.set_results is not called. | ||||||||||
| """ | ||||||||||
| charm = MagicMock() | ||||||||||
| event = MagicMock() | ||||||||||
| charm._get_admin_credentials.return_value = None | ||||||||||
|
|
||||||||||
| GarmCharm._on_get_credentials_action(charm, event) | ||||||||||
|
|
||||||||||
| event.fail.assert_called_once() | ||||||||||
| fail_message = event.fail.call_args[0][0] | ||||||||||
| assert "not yet available" in fail_message | ||||||||||
| event.set_results.assert_not_called() | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are zero blank lines between the end of this test and the next top-level
Suggested change
|
||||||||||
| def test_reconcile_scalesets_skips_when_no_admin_credentials(): | ||||||||||
| """ | ||||||||||
| arrange: Admin credentials secret is unavailable. | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # How to retrieve GARM admin credentials | ||
|
Thanhphan1147 marked this conversation as resolved.
|
||
|
|
||
| The `get-credentials` action retrieves the GARM admin credentials generated by the charm during start-up. Use it when you need to authenticate via the GARM API or when registering new profiles using `garm-cli`. | ||
|
Thanhphan1147 marked this conversation as resolved.
|
||
|
|
||
| ## Prerequisites | ||
|
|
||
| The GARM charm must be fully initialized. The unit status must be `active` before running this action. If the charm has not completed its first-run setup, the credentials will not yet be available and the action will fail. | ||
|
|
||
| ## Run the action | ||
|
|
||
| ```bash | ||
| juju run garm/0 get-credentials | ||
| ``` | ||
|
|
||
| ## Action output | ||
|
|
||
| The action returns four fields: | ||
|
|
||
| | Field | Description | | ||
| |-------|-------------| | ||
| | `username` | The GARM admin username. | | ||
| | `password` | The GARM admin password. | | ||
| | `email` | The email address associated with the admin account. | | ||
| | `full-name` | The full name associated with the admin account. | | ||
|
|
||
| Example output: | ||
|
|
||
| ```{terminal} | ||
| :output-only: | ||
|
|
||
| Running operation 1 with 1 task | ||
| - task 2 on unit-garm-0 | ||
|
|
||
| Waiting for task 2... | ||
| email: admin@garm.local | ||
| full-name: GARM Admin | ||
| password: <generated-password> | ||
| username: admin | ||
| ``` | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This docstring is missing its
assert:section — it only hasarrange:/act:, while the test body performs two assertions. The sibling test added in the same commit (test_get_credentials_action_fails_when_credentials_unavailable) includes all three sections, as does every other test in this file.Suggestion: