eliminate dmidecode in component.py because it requires root privileges #17509
Conversation
|
@jeff-yin flagging this one too for review by someone at Dell. |
|
@justindthomas please assign @arunlk-dell and @vpsubramaniam as reviewers too, thanks! |
| @@ -16,7 +16,7 @@ | |||
| except ImportError as e: | |||
| raise ImportError(str(e) + "- required module not found") | |||
| def get_bios_version(): | |||
| return subprocess.check_output(['dmidecode', '-s', 'system-version']).strip().decode() | |||
| return subprocess.check_output(['cat', '/sys/class/dmi/id/bios_version']).strip().decode() | |||
There was a problem hiding this comment.
@vpsubramaniam can you check this change and see whether it is Kosher to get the bios version this way?
There was a problem hiding this comment.
For reference, here's what those commands look like on my N3248TE-ON.
Last login: Thu Dec 14 23:39:12 2023 from 10.20.37.104
jdt@sonic:~$ sudo dmidecode -s system-version
3.45.0.9-4
jdt@sonic:~$ cat /sys/class/dmi/id/bios_version
3.45.0.9-4
jdt@sonic:~$ show ver
SONiC Software Version: SONiC.jdt.0-865e34c64
SONiC OS Version: 12
Distribution: Debian 12.4
Kernel: 6.1.0-11-2-amd64
Build commit: 865e34c64
Build date: Mon Dec 11 10:57:26 UTC 2023
Built by: justin@sonic-build
There was a problem hiding this comment.
@vpsubramaniam can you check this change and see whether it is Kosher to get the bios version this way?
Yes Jeff, for getting the BIOS version we can use this file as well.
|
Is there a meeting I need to attend or something to get things like this merged? This seems like a pretty trivial change, but has a significantly positive usability impact (i.e., operations that shouldn't require privileges no longer error out). I have 4 PRs that have been sitting for months with some reviews, but apparently not enough. How do I move these forward? |
|
@zhangyanzhao @lguohan -- these changes have been approved by Dell staff. Can someone merge? |
|
@yxieca can you please help to check this PR to see if we can merge it? Thanks. |
|
/Semgrep |
Why I did it
As described in #16878, every
showcommand on the N3248TE-ON results in an error aboutdmidecodenot being found. This is because that command is called in theget_bios_versionfunction ofcomponent.py, which is used in a lot of places (many which should not require privileged access).The error message prior to this change:
Work item tracking
How I did it
On this platform, at least, the BIOS version can be obtained from
/sys/class/dmi/id/bios_versionwithout requiringrootprivileges. I adjustedcomponent.pyto use that instead ofdmidecode.How to verify it
With that change in place, you can see that commands no longer complain about
dmidecode:Tested branch (Please provide the tested image version)
master
Description for the changelog
Removed call to
dmidecodefor every component command on the N3248TE-ON platform.