Skip to content
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

Redfish: Added support for displaying and setting account types #6871

Conversation

mraineri
Copy link
Contributor

@mraineri mraineri commented Jul 6, 2023

SUMMARY

Redfish allows for clients to specify what a user account is allowed to access on a device with the AccountTypes and OEMAccountTypes properties. This pull request adds support for setting and displaying the account types (and OEM account types) for Redfish user accounts when invoking AddUser and ListUsers. These are optional parameters; users can still provide the simple "UserName, Password, RoleId" combination when creating user accounts.

Fixes #6823

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

redfish_utils.py, redfish_info.py, redfish_command.py

ADDITIONAL INFORMATION

Playbook with new options when making a new user account

---
- hosts: all
  gather_facts: false
  vars:
    username: REDACTED
    password: REDACTED
    baseuri: REDACTED
    default_uri_timeout: 5
    default_uri_retries: 5
  tasks:
  - name: Set add new account
    community.general.redfish_command:
      category: Accounts
      command: AddUser
      baseuri: "{{ baseuri }}"
      username: "{{ username }}"
      password: "{{ password }}"
      new_username: test
      new_password: test123!
      roleid: ReadOnly
      account_types:
      - Redfish
      - SNMP
      - OEM
      oem_account_types:
      - IPMI
      - SOL
      - WSMAN
      - UI
      - RACADM
    retries: "{{ default_uri_retries }}"
    register: redfish_results
  - debug:
      var: redfish_results

Sample output from the ListUsers operation after invoking the above playbook

ok: [localhost] => {
    "redfish_results": {
        "ansible_facts": {
            "discovered_interpreter_python": "/usr/bin/python3"
        },
        "changed": false,
        "failed": false,
        "redfish_facts": {
            "user": {
                "entries": [
                    {
                        "AccountTypes": [
                            "Redfish",
                            "SNMP",
                            "OEM"
                        ],
                        "Enabled": true,
                        "Id": "2",
                        "Locked": false,
                        "Name": "User Account",
                        "OEMAccountTypes": [
                            "IPMI",
                            "SOL",
                            "WSMAN",
                            "UI",
                            "RACADM"
                        ],
                        "RoleId": "Administrator",
                        "UserName": "root"
                    },
                    {
                        "AccountTypes": [
                            "Redfish",
                            "SNMP",
                            "OEM"
                        ],
                        "Enabled": false,
                        "Id": "3",
                        "Locked": false,
                        "Name": "User Account",
                        "OEMAccountTypes": [
                            "IPMI",
                            "SOL",
                            "WSMAN",
                            "UI",
                            "RACADM"
                        ],
                        "RoleId": "ReadOnly",
                        "UserName": "test"
                    }
                ],
                "ret": true
            }
        }
    }
}

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module module module_utils module_utils needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR plugins plugin (any type) labels Jul 6, 2023
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-7 labels Jul 6, 2023
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Jul 6, 2023
Signed-off-by: Mike Raineri <[email protected]>
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 6, 2023
@mraineri
Copy link
Contributor Author

mraineri commented Jul 6, 2023

@sseekamp FYI. The service I have is pretty static in terms of what it will put in place for account types, so it essentially ignores whatever the user is requesting and fills in its own values (at least on the version of firmware I have available it appears to do this). As far as I can tell from traces, it's following the spec though. It would be good for you to try it out on one of your systems, but I'm happy from the traces I've instrumented.

Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@felixfontein
Copy link
Collaborator

@mraineri do you want to wait on an answer by @sseekamp, or should we just merge this? Or wait until the end of the week and merge then if nobody objected by then? (The next release will happen on Monday.)

@mraineri
Copy link
Contributor Author

I'm okay with merging the pull request now. Everything on paper and from the traces looks good to me, and since these are optional parameters, we won't break anyone who is adding users with the existing commands today.

@felixfontein felixfontein merged commit 9adc82d into ansible-collections:main Jul 12, 2023
@patchback
Copy link

patchback bot commented Jul 12, 2023

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/9adc82d5d1f9b2e0dd7fd79f4818974fc55b5f16/pr-6871

Backported as #6919

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jul 12, 2023
patchback bot pushed a commit that referenced this pull request Jul 12, 2023
* Redfish: Added support for displaying and setting account types

Signed-off-by: Mike Raineri <[email protected]>

* Update 6823-redfish-add-account-type-management.yml

* CI fixes

Signed-off-by: Mike Raineri <[email protected]>

---------

Signed-off-by: Mike Raineri <[email protected]>
(cherry picked from commit 9adc82d)
@felixfontein
Copy link
Collaborator

@mraineri sounds good. Thanks for your contribution!
@russoz thanks for reviewing!

@mraineri mraineri deleted the Redfish-Account-Types-Configuration branch July 12, 2023 19:43
felixfontein pushed a commit that referenced this pull request Jul 12, 2023
…playing and setting account types (#6919)

Redfish: Added support for displaying and setting account types (#6871)

* Redfish: Added support for displaying and setting account types

Signed-off-by: Mike Raineri <[email protected]>

* Update 6823-redfish-add-account-type-management.yml

* CI fixes

Signed-off-by: Mike Raineri <[email protected]>

---------

Signed-off-by: Mike Raineri <[email protected]>
(cherry picked from commit 9adc82d)

Co-authored-by: Mike Raineri <[email protected]>
valeriopoggi pushed a commit to valeriopoggi/community.general that referenced this pull request Jul 17, 2023
…ble-collections#6871)

* Redfish: Added support for displaying and setting account types

Signed-off-by: Mike Raineri <[email protected]>

* Update 6823-redfish-add-account-type-management.yml

* CI fixes

Signed-off-by: Mike Raineri <[email protected]>

---------

Signed-off-by: Mike Raineri <[email protected]>
@sseekamp
Copy link
Contributor

Thanks @mraineri - I'll test shortly!

@sseekamp
Copy link
Contributor

@mraineri apologies for revising a super old thread here. This change is great for new accounts, but we need to modify existing accounts and it isn't behaving as I expected.

Is this a case of needing an explicit "modify_accounttypes" method?

Example - run successfully, but does not update AccountTypes as expected:

   - name: Change account types
      community.general.redfish_command:
        category: Accounts
        command: AddUser
        baseuri: "{{ bmc_address }}"
        username: "admin"
        password: "{{ bmc_password }}"
        new_username: existing_user
        account_roleid: "Administrator"
        account_id: "5"
        account_types:
          - Redfish
          - KVMIP
      register: redfish_results

I can do a raw curl and update the accounttypes array successfully on the same node

@mraineri
Copy link
Contributor Author

@sseekamp AddUser is strictly for creating an entirely new user. We'd need to add a new "UpdateUserAccountTypes" command to redfish_command.py (like how there are other "UpdateUserXYZ" commands under Accounts). It could use account_types and oem_account_types just like in the AddUser command for inputs.

Let's open a new issue to track it.

@sseekamp
Copy link
Contributor

Perfect - thanks sir! I'll get that rolling!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request module_utils module_utils module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redfish Account settings
5 participants