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

Solve Issue #63: Add Support for Token Authentication and User/Pass Authentication in BMC Firmware Update #89

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

abhashsolanki18
Copy link

Issue Addressed: Resolves #63
Changes Made:

  1. Added support for token-based authentication alongside existing user/password authentication in the BMC firmware update process.
  2. Introduced two distinct blocks in roles/bmc_fw_update/tasks/main.yml:
    User/Password Authentication Block: For handling authentication using username and password.
    Token Authentication Block: For handling authentication using tokens.

Details:
Updated the community.general.redfish_info task to support both authentication methods.
Ensured compatibility with existing configurations and backward compatibility.

@glimchb Please review these changes. Your feedback is appreciated!

@glimchb
Copy link
Member

glimchb commented Jul 6, 2024

@abhashsolanki18 thanks for fixing this
Please see commitlint failure.
Otherwise looks good.
If you rebase, those ansible ci failures should be resolved

@abhashsolanki18 abhashsolanki18 force-pushed the main branch 4 times, most recently from 305b124 to 689cb7e Compare July 9, 2024 12:21
Signed-off-by: abhash [email protected]
Signed-off-by: root <[email protected]>
@abhashsolanki18
Copy link
Author

@glimchb i have rebased the commits now it should pass the ci.

Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

@abhashsolanki18

  • looks like linter fails on your changes, please fix
  • i saw one place at the end when you don;t give neither user/pass nor auth token... looks like a bug...

Signed-off-by: Abhash Solanki <[email protected]>
@abhashsolanki18
Copy link
Author

@glimchb
the linters should also exclude ansible 2.17 with python 3.9, fixed that in the latest commit
also fixed the bug you highlighted earlier.

@@ -154,4 +212,5 @@
msg: "{{ bmc_fw_update_version_failure }}"
when:
- bmc_fw_update_reboot is true
- not bmc_fw_update_image_file is search(bmc_fw_update_got_fw_version | regex_search('[0-9-.]+'))
- not bmc_fw_update_image_file is search(bmc_fw_update_got_fw_version | regex_search('[0-9-.]+'))"
Copy link
Member

Choose a reason for hiding this comment

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

last " is typo?

Copy link
Author

Choose a reason for hiding this comment

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

yes, fixed it

This commit resolves multiple YAML linting issues in the `bmc_fw_update` task file.
- Removed extra blank lines
- Ensured all tasks are named
- Improved task key order
- Prefixed variable names appropriately

These changes should ensure the playbook passes `yamllint` and `ansible-lint` checks.

Signed-off-by: Abhash Solanki <[email protected]>
@abhashsolanki18
Copy link
Author

Invalid workflow file: .github/workflows/docker-publish.yml#L13
The workflow is not valid. .github/workflows/docker-publish.yml (Line: 13, Col: 3): Error calling workflow 'opiproject/actions/.github/workflows/docker-publish.yml@main'. The nested job 'build' is requesting 'id-token: write', but is only allowed 'id-token: none'.

@glimchb what is the error about? failing while executing docker workflow as well as openSSF workflow

@glimchb
Copy link
Member

glimchb commented Jul 14, 2024

@glimchb what is the error about? failing while executing docker workflow as well as openSSF workflow

will check it

@@ -23,11 +23,11 @@

- name: Store current fw version
ansible.builtin.set_fact:
bmc_fw_update_cur_fw_version: "{{ vars.get_bmc_facts_all_fw_versions[bmc_fw_update_inventory_name] }}"
bmc_fw_update_cur_fw_version: "{{ get_bmc_facts_before.get_bmc_facts_all_fw_versions[bmc_fw_update_inventory_name] }}"
Copy link
Member

@glimchb glimchb Jul 15, 2024

Choose a reason for hiding this comment

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

this doesn't work in my setup for some reason, let's try to understand why?

TASK [get_bmc_facts : Print all Versions] ***************************************************************************************************************************************************
task path: /opt/roles/get_bmc_facts/tasks/main.yml:54
ok: [172.22.4.2] => {
    "msg": {
        "6d53cf4d_bmc_firmware": "bf-24.01-5-0-gdd4a6159ce7.1704982350.6715245",
        "bluefield_fw_erot": "4-15",
        "dpu_atf": "",
        "dpu_board": "",
        "dpu_bsp": "",
        "dpu_nic": "",
        "dpu_node": "",
        "dpu_ofed": "",
        "dpu_os": "",
        "dpu_sys_image": "",
        "dpu_uefi": ""
    }
}

TASK [bmc_fw_update : Store current fw version] *********************************************************************************************************************************************
task path: /opt/roles/bmc_fw_update/tasks/main.yml:24
fatal: [172.22.4.2]: FAILED! => {
    "msg": "The task includes an option with an undefined variable. The error was: 'dict object' has no attribute 'get_bmc_facts_all_fw_versions'. 'dict object' has no attribute 'get_bmc_facts_all_fw_versions'\n\nThe error appears to be in '/opt/roles/bmc_fw_update/tasks/main.yml': line 24, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n- name: Store current fw version\n  ^ here\n"
}


- name: Print BMC Version
ansible.builtin.debug:
msg: "{{ get_bmc_facts_all_fw_versions }}"
msg: "{{ get_bmc_facts_before.get_bmc_facts_all_fw_versions }}"
Copy link
Member

@glimchb glimchb Jul 15, 2024

Choose a reason for hiding this comment

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

this doesn't work in my setup for some reason, let's try to understand why?

TASK [bmc_fw_update : Store current fw version] *********************************************************************************************************************************************
task path: /opt/roles/bmc_fw_update/tasks/main.yml:24
ok: [172.22.4.2] => {
    "ansible_facts": {
        "bmc_fw_update_cur_fw_version": "bf-24.01-5-0-gdd4a6159ce7.1704982350.6715245"
    },
    "changed": false
}

TASK [bmc_fw_update : Print BMC Version] ****************************************************************************************************************************************************
task path: /opt/roles/bmc_fw_update/tasks/main.yml:28
fatal: [172.22.4.2]: FAILED! => {
    "msg": "The task includes an option with an undefined variable. The error was: 'dict object' has no attribute 'get_bmc_facts_all_fw_versions'. 'dict object' has no attribute 'get_bmc_facts_all_fw_versions'\n\nThe error appears to be in '/opt/roles/bmc_fw_update/tasks/main.yml': line 28, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n- name: Print BMC Version\n  ^ here\n"
}

@glimchb
Copy link
Member

glimchb commented Jul 15, 2024

@abhashsolanki18 I tested your changes and they fail, please fix

I used for user/pass flow:

docker run --rm -it --entrypoint ansible-playbook -v $(pwd):/opt -w /opt/roles ghcr.io/opiproject/ansible-opi-dpu:main ../playbooks/firmware.yml -vvv -i "172.22.4.2," -e dpu_bmc_username='root' -e dpu_bmc_password='123456' -e bmc_fw_update_inventory_name='6d53cf4d_bmc_firmware' -e bmc_fw_update_image_file='/tmp/bf2-bmc-ota-24.04-5-opn.tar'

I hit errors, so I had to make those changes:

diff --git a/roles/bmc_fw_update/defaults/main.yml b/roles/bmc_fw_update/defaults/main.yml
index ba5ffaf..ba8a611 100644
--- a/roles/bmc_fw_update/defaults/main.yml
+++ b/roles/bmc_fw_update/defaults/main.yml
@@ -8,5 +8,5 @@ bmc_fw_update_reboot: true
 bmc_fw_update_job_wait: true
 bmc_fw_update_inventory_name: bmc_firmware
 bmc_fw_update_image_file: /tmp/bf3-bmc-24.04-5_opn.fwpkg
-bmc_fw_update_image_url: https://content.mellanox.com/BlueField/BMC/24.04-5-Apr-2024/{{ bmc_fw_update_image_file | basename }}
+bmc_fw_update_image_url: https://www.mellanox.com/downloads/BlueField/BMC/24.04-5-Apr-2024/{{ bmc_fw_update_image_file | basename }}
 https_port: 443
diff --git a/roles/bmc_fw_update/tasks/main.yml b/roles/bmc_fw_update/tasks/main.yml
index e37647c..93b8d05 100644
--- a/roles/bmc_fw_update/tasks/main.yml
+++ b/roles/bmc_fw_update/tasks/main.yml
@@ -23,11 +23,11 @@

 - name: Store current fw version
   ansible.builtin.set_fact:
-    bmc_fw_update_cur_fw_version: "{{ get_bmc_facts_before.get_bmc_facts_all_fw_versions[bmc_fw_update_inventory_name] }}"
+    bmc_fw_update_cur_fw_version: "{{ vars.get_bmc_facts_all_fw_versions[bmc_fw_update_inventory_name] }}"

 - name: Print BMC Version
   ansible.builtin.debug:
-    msg: "{{ get_bmc_facts_before.get_bmc_facts_all_fw_versions }}"
+    msg: "{{ get_bmc_facts_all_fw_versions }}"

 - name: Check if firmware image exists locally {{ bmc_fw_update_image_file }}
   ansible.builtin.stat:

once fixed, I can test the same with token:

curl -k -H "Content-Type: application/json" -X POST https://172.22.4.2/login -d '{"username" : "root", "password" : "123456"}'
docker run --rm -it --entrypoint ansible-playbook -v $(pwd):/opt -w /opt/roles ghcr.io/opiproject/ansible-opi-dpu:main ../playbooks/firmware.yml -vvv -i "172.22.4.2," -e dpu_bmc_token='123456' -e bmc_fw_update_inventory_name='6d53cf4d_bmc_firmware' -e bmc_fw_update_image_file='/tmp/bf2-bmc-ota-24.04-5-opn.tar'    


- name: Store fw version we installed
ansible.builtin.set_fact:
bmc_fw_update_got_fw_version: "{{ vars.get_bmc_facts_all_fw_versions[bmc_fw_update_inventory_name] }}"
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't work in my setup for some reason, let's try to understand why?

@glimchb glimchb added the Merge Candidate in the open merge window, next candidate for merge label Jul 16, 2024
@abhashsolanki18
Copy link
Author

@abhashsolanki18 I tested your changes and they fail, please fix

I used for user/pass flow:

docker run --rm -it --entrypoint ansible-playbook -v $(pwd):/opt -w /opt/roles ghcr.io/opiproject/ansible-opi-dpu:main ../playbooks/firmware.yml -vvv -i "172.22.4.2," -e dpu_bmc_username='root' -e dpu_bmc_password='123456' -e bmc_fw_update_inventory_name='6d53cf4d_bmc_firmware' -e bmc_fw_update_image_file='/tmp/bf2-bmc-ota-24.04-5-opn.tar'

I hit errors, so I had to make those changes:

diff --git a/roles/bmc_fw_update/defaults/main.yml b/roles/bmc_fw_update/defaults/main.yml
index ba5ffaf..ba8a611 100644
--- a/roles/bmc_fw_update/defaults/main.yml
+++ b/roles/bmc_fw_update/defaults/main.yml
@@ -8,5 +8,5 @@ bmc_fw_update_reboot: true
 bmc_fw_update_job_wait: true
 bmc_fw_update_inventory_name: bmc_firmware
 bmc_fw_update_image_file: /tmp/bf3-bmc-24.04-5_opn.fwpkg
-bmc_fw_update_image_url: https://content.mellanox.com/BlueField/BMC/24.04-5-Apr-2024/{{ bmc_fw_update_image_file | basename }}
+bmc_fw_update_image_url: https://www.mellanox.com/downloads/BlueField/BMC/24.04-5-Apr-2024/{{ bmc_fw_update_image_file | basename }}
 https_port: 443
diff --git a/roles/bmc_fw_update/tasks/main.yml b/roles/bmc_fw_update/tasks/main.yml
index e37647c..93b8d05 100644
--- a/roles/bmc_fw_update/tasks/main.yml
+++ b/roles/bmc_fw_update/tasks/main.yml
@@ -23,11 +23,11 @@

 - name: Store current fw version
   ansible.builtin.set_fact:
-    bmc_fw_update_cur_fw_version: "{{ get_bmc_facts_before.get_bmc_facts_all_fw_versions[bmc_fw_update_inventory_name] }}"
+    bmc_fw_update_cur_fw_version: "{{ vars.get_bmc_facts_all_fw_versions[bmc_fw_update_inventory_name] }}"

 - name: Print BMC Version
   ansible.builtin.debug:
-    msg: "{{ get_bmc_facts_before.get_bmc_facts_all_fw_versions }}"
+    msg: "{{ get_bmc_facts_all_fw_versions }}"

 - name: Check if firmware image exists locally {{ bmc_fw_update_image_file }}
   ansible.builtin.stat:

once fixed, I can test the same with token:

curl -k -H "Content-Type: application/json" -X POST https://172.22.4.2/login -d '{"username" : "root", "password" : "123456"}'
docker run --rm -it --entrypoint ansible-playbook -v $(pwd):/opt -w /opt/roles ghcr.io/opiproject/ansible-opi-dpu:main ../playbooks/firmware.yml -vvv -i "172.22.4.2," -e dpu_bmc_token='123456' -e bmc_fw_update_inventory_name='6d53cf4d_bmc_firmware' -e bmc_fw_update_image_file='/tmp/bf2-bmc-ota-24.04-5-opn.tar'    

will find out whats going wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge Candidate in the open merge window, next candidate for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bmc_fw_update: Support token in addition to user/pass
2 participants