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

[S6000] Fix 'show interface status' CLI needs sudo permission #20384

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

prgeor
Copy link
Contributor

@prgeor prgeor commented Sep 30, 2024

Why I did it

CLI crashes without sudo permission on Dell 6000 platform

sonic@sonic-dev:~$ show interfaces status
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/sonic_platform_base/sonic_eeprom/eeprom_base.py", line 244, in read_eeprom_bytes
    F = self.open_eeprom()
  File "/usr/local/lib/python3.9/dist-packages/sonic_platform_base/sonic_eeprom/eeprom_base.py", line 232, in open_eeprom
    return io.open(eeprom_file, "rb")
PermissionError: [Errno 13] Permission denied: '/sys/bus/i2c/devices/i2c-10/10-0053/eeprom'
 
During handling of the above exception, another exception occurred:
 
Traceback (most recent call last):
  File "/usr/local/bin/intfutil", line 836, in <module>
    main()
  File "/usr/local/bin/intfutil", line 819, in main
    interface_stat.display_intf_status()
  File "/usr/local/bin/intfutil", line 448, in display_intf_status
    self.get_intf_status()
  File "/usr/local/lib/python3.9/dist-packages/utilities_common/multi_asic.py", line 157, in wrapped_run_on_all_asics
    func(self,  *args, **kwargs)
  File "/usr/local/bin/intfutil", line 535, in get_intf_status
    self.table += self.generate_intf_status()
  File "/usr/local/bin/intfutil", line 479, in generate_intf_status
    port_oper_speed_get(self.db, key),
  File "/usr/local/bin/intfutil", line 202, in port_oper_speed_get
    return appl_db_port_status_get(db, intf_name, PORT_SPEED)
  File "/usr/local/bin/intfutil", line 167, in appl_db_port_status_get
    optics_type = port_optics_get(appl_db, intf_name, PORT_OPTICS_TYPE)
  File "/usr/local/bin/intfutil", line 224, in port_optics_get
    if is_rj45_port(intf_name):
  File "/usr/local/lib/python3.9/dist-packages/utilities_common/platform_sfputil_helper.py", line 120, in is_rj45_port
    platform_chassis = sonic_platform.platform.Platform().get_chassis()
  File "/usr/local/lib/python3.9/dist-packages/sonic_platform/platform.py", line 24, in __init__
    self._chassis = Chassis()
  File "/usr/local/lib/python3.9/dist-packages/sonic_platform/chassis.py", line 93, in __init__
    self._eeprom = EepromS6000()
  File "/usr/local/lib/python3.9/dist-packages/sonic_platform/eeprom.py", line 307, in __init__
    self.eeprom_data = self.read_eeprom()
  File "/usr/local/lib/python3.9/dist-packages/sonic_platform/eeprom.py", line 374, in read_eeprom
    return self.read_eeprom_bytes(self._EEPROM_MAX_LEN)
  File "/usr/local/lib/python3.9/dist-packages/sonic_platform_base/sonic_eeprom/eeprom_base.py", line 267, in read_eeprom_bytes
    raise IOError("Failed to read eeprom : %s" % (str(e)))
OSError: Failed to read eeprom : [Errno 13] Permission denied: '/sys/bus/i2c/devices/i2c-10/10-0053/eeprom'
Work item tracking
  • Microsoft ADO (number only):29676167

How I did it

Add check to read eeprom only if user has root permission

How to verify it

Verified the CLI "show interface status" does not crash if user is not root

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305
  • 202311
  • 202405

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Prince George <prgeor@microsoft.com>
@prgeor
Copy link
Contributor Author

prgeor commented Sep 30, 2024

@StormLiangMS @yxieca @bingwang-ms please help cherry pick as necessary. Branch label added.

Copy link
Contributor

@santhosh-kt santhosh-kt left a comment

Choose a reason for hiding this comment

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

LGTM

@prgeor
Copy link
Contributor Author

prgeor commented Oct 4, 2024

'/azpw ms_conflict'

@assrinivasan
Copy link
Contributor

/azpw ms_conflict

1 similar comment
@StormLiangMS
Copy link
Contributor

/azpw ms_conflict

@yxieca yxieca merged commit 3a3ae37 into sonic-net:master Oct 11, 2024
11 of 12 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Oct 11, 2024
…net#20384)

Why I did it
CLI crashes without sudo permission on Dell 6000 platform

How I did it
Add check to read eeprom only if user has root permission

How to verify it
Verified the CLI "show interface status" does not crash if user is not root

Signed-off-by: Prince George <prgeor@microsoft.com>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #20467

mssonicbld pushed a commit that referenced this pull request Oct 11, 2024
Why I did it
CLI crashes without sudo permission on Dell 6000 platform

How I did it
Add check to read eeprom only if user has root permission

How to verify it
Verified the CLI "show interface status" does not crash if user is not root

Signed-off-by: Prince George <prgeor@microsoft.com>
sschlafman pushed a commit to sschlafman/sonic-buildimage that referenced this pull request Oct 15, 2024
…net#20384)

Why I did it
CLI crashes without sudo permission on Dell 6000 platform

How I did it
Add check to read eeprom only if user has root permission

How to verify it
Verified the CLI "show interface status" does not crash if user is not root

Signed-off-by: Prince George <prgeor@microsoft.com>
@assrinivasan
Copy link
Contributor

assrinivasan commented Oct 17, 2024

Hello @bingwang-ms request you to cherry-pick this PR to 202405 branch. Thanks.

@assrinivasan
Copy link
Contributor

Hello @yxieca please help merge this PR to 202311 branch. Thanks.

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Oct 18, 2024
…net#20384)

Why I did it
CLI crashes without sudo permission on Dell 6000 platform

How I did it
Add check to read eeprom only if user has root permission

How to verify it
Verified the CLI "show interface status" does not crash if user is not root

Signed-off-by: Prince George <prgeor@microsoft.com>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #20537

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Oct 18, 2024
…net#20384)

Why I did it
CLI crashes without sudo permission on Dell 6000 platform

How I did it
Add check to read eeprom only if user has root permission

How to verify it
Verified the CLI "show interface status" does not crash if user is not root

Signed-off-by: Prince George <prgeor@microsoft.com>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #20538

mssonicbld pushed a commit that referenced this pull request Oct 18, 2024
Why I did it
CLI crashes without sudo permission on Dell 6000 platform

How I did it
Add check to read eeprom only if user has root permission

How to verify it
Verified the CLI "show interface status" does not crash if user is not root

Signed-off-by: Prince George <prgeor@microsoft.com>
mssonicbld pushed a commit that referenced this pull request Oct 18, 2024
Why I did it
CLI crashes without sudo permission on Dell 6000 platform

How I did it
Add check to read eeprom only if user has root permission

How to verify it
Verified the CLI "show interface status" does not crash if user is not root

Signed-off-by: Prince George <prgeor@microsoft.com>
aidan-gallagher pushed a commit to aidan-gallagher/sonic-buildimage that referenced this pull request Nov 16, 2024
…net#20384)

Why I did it
CLI crashes without sudo permission on Dell 6000 platform

How I did it
Add check to read eeprom only if user has root permission

How to verify it
Verified the CLI "show interface status" does not crash if user is not root

Signed-off-by: Prince George <prgeor@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants