HLD for cli sessions feature by i-davydenko · Pull Request #1367 · sonic-net/SONiC

i-davydenko · 2023-06-23T13:52:36Z

Prepared HLD documents for CLI sessions feature

Module name	PR	state	context
sonic-buildimage	Dev cli sessions
sonic-host-services	cli-sessions
sonic-utilities	SONIC CLI for CLI-Sessions feature #3175

Updated ssh_config.md with auto-logout and max-syslogins parameters.
Created serial-console-HLD.md

Signed-off-by: Ivan Davydenko [email protected]

Prepare HLD documents for CLI sessions feature Update ssh_config.md with auto-logout and max-syslogins parameters. Create serial-console-HLD.md

linux-foundation-easycla · 2023-06-23T13:52:40Z

The committers listed above are authorized under a signed CLA.

✅ login: i-davydenko (2c4c01d, 34a7364, 3db77dc, 9e32292, 8e0296d, 90a8d3e)

venkatmahalingam · 2023-06-27T15:22:04Z

doc/ssh_config/ssh_config.md

                    }
 				}
+				leaf auto_logout {
+					description "inactivity timeoout (min unit)";


min unit -> in minutes?

venkatmahalingam · 2023-06-27T15:25:34Z

doc/ssh_config/ssh_config.md

+					default 15;
+					type uint32 {
+						range 0..35000;
+					}


Please add description for 0 value to indicate no auto logout.

venkatmahalingam · 2023-06-27T15:26:39Z

doc/ssh_config/ssh_config.md

+				leaf max_syslogins {
+					description "limit of concurrent system logins";
+					default 100;
+					type uint32 {


uint32 -> uint8?

I prefer to leave it as 32, to not limit it in case we will extend the upper limit in future.

venkatmahalingam · 2023-06-27T15:27:22Z

doc/ssh_config/ssh_config.md

+				leaf auto_logout {
+					description "inactivity timeoout (min unit)";
+					default 15;
+					type uint32 {


uint32 -> uint16?

Same here, prefer not to limit by u16 in case we will extend in future.

doc/ssh_config/ssh_config.md

doc/console/serial-console-HLD.md

lguohan · 2023-06-27T16:02:39Z

doc/console/serial-console-HLD.md

+| Policy             | Action                                                                | Param values        | Default      |
+|--------------------|-----------------------------------------------------------------------|---------------------|--------------|
+| auto logout        | Inactivity timeout for serial-console session                         | 0-35000 (min)       | 15           |
+| sysrq capabilities | Enabling or disabling SysRq functionality for serial-consoles         | enabled/disabled    | disabled     |


i saw some test are using sysrq in sonic-mgmt.

tests/scripts/sai_qualify/sai_warmboot.sh: echo 1 > /proc/sys/kernel/sysrq

can we check if this won't affect the tst?

@vaibhavhd for visibility.

This will overwrite any existing configuration and perform reboot (in the next line: 'echo b > /proc/sysrq-trigger' )
So the test wont break by proposed changes.
Thanks.

lguohan · 2023-06-27T16:04:41Z

doc/console/serial-console-HLD.md

+/* end of module sonic-serial-console */
+
+```
+###  6.4. <a name='ConfigDBEnhancements'></a>Config DB Enhancements


can we use the native markdown format instead of http marker? there are other cases, please correct as well.

zhangyanzhao · 2023-06-27T16:06:17Z

community review recording https://zoom.us/rec/share/4Wwy3t9ogqaeABDgjvYQ6IOwAovCEqdamwVzTeR9dxlvMK2GlOhNhrkcWfOgpeyc.dXhqq6g4IUDmdjxa. The starting point of this HLD is in the middle of this recording.

zhangyanzhao · 2023-06-27T16:10:14Z

MSFT register as reviewer for this feature.

liuh-80 · 2023-06-28T05:33:18Z

doc/ssh_config/ssh_config.md

 | login timeout  |     SSH session timeout                                                   |     1-600 (secs)        |     120          |
 | ports          |     Port numbers for SSH                                                  |     1-65535             |     22           |
+| auto logout    |     Inactivity timeout for SSH session                                    |     0-35000 (min)       |     15           |
+| max sessions   |     Max number of concurrent logins                                       |     3-100               |     100          |


Currently we already have template files and hostcfgd code to support generate session limit by hwsku:

sonic-net/sonic-buildimage#10177

In the public repo this is a empty template.

So for the implementation detail of these limit, please consider reuse and improve existed code and template.

Also for the auto logout, currently there are code in this file do some pre-configurition:

https://github.com/sonic-net/sonic-buildimage/blob/master/build_debian.sh

So the implementation should not break those code, or need migrate those config to the new design.

Agree, we will reuse existing infra to keep all limits.conf file updates in one pllace.
Added section to clarify this. Thank you for commens.

Update serial-console.hld according to the comments.

Update ssh_config HLD according to the community comments.

qiluo-msft · 2023-08-14T00:28:27Z

doc/console/serial-console-HLD.md

+
+###  1.2. Definitions/Abbreviations 
+
+	serial - secure shell


secure shell

this is for ssh? #Closed

No, this section covers the serial connection.
Fixed in 9e32292.

qiluo-msft · 2023-08-14T00:30:49Z

doc/console/serial-console-HLD.md

+### 3.1 Flow description
+When the feature is enabled, by modifying the DB manually, user will set serial-console configurations by modifing CONFIG_DB in SERIAL_CONSOLE table.
+
+The hostcfgd daemon will be extended to listen to confogurations from SERIAL_CONSOLE table and restarts the serial_console.service. Serial console script will read SERIAL_CONSOLE table and update config files accordingly.


confogurations

typo #Closed

Thanks, fixed in 9e32292.

qiluo-msft · 2023-08-14T00:35:50Z

doc/console/serial-console-HLD.md

+### 3.1 Flow description
+When the feature is enabled, by modifying the DB manually, user will set serial-console configurations by modifing CONFIG_DB in SERIAL_CONSOLE table.
+
+The hostcfgd daemon will be extended to listen to confogurations from SERIAL_CONSOLE table and restarts the serial_console.service. Serial console script will read SERIAL_CONSOLE table and update config files accordingly.


serial_console.service

Is it a vanilla Debian service, or part of this HLD? If latter, please explain what is its functionality?

Why single hostcfgd service is not enough? #Closed

This is new service, not a vanilla Debian one.

The rationale behind it creation is to update serial console config files (sysrq config, $TMOUT, and others that could be added in the future) before user allowed to start serial connection.

We will achieve this by starting serial-console.service Before=getty-pre.target

Is it possible to move its functionality into hostcfgd service?

I believe hostcfgd starts too late for this purpose, and it will impact the users config if the serial/ssh was started too early. Thats why I think in this case moving logic to hostcfgd is not a good solution.

Fix typos.

Fix typos

Fix HLD with missed "POLICIES" container in config/cli tree.

qiluo-msft

LGTM. Please check with other active reviewers.

liat-grozovik · 2023-08-24T10:13:10Z

@zhangyanzhao as other reviewers has no further comment, i believe we should move to the next step and merge
@i-davydenko please fix the easyCLA to unblock it

Meir-renford · 2023-10-02T13:32:36Z

@venkatmahalingam can you please approve this PR if there are no more comments from your side?

zhangyanzhao · 2024-02-05T06:43:33Z

@i-davydenko can you please help to add the code PRs to this HLD by refering to #806 ? Thanks.

liat-grozovik · 2024-04-17T10:12:26Z

@i-davydenko please add a table with all the PRs under this feature in the PR description.

i-davydenko · 2024-04-17T13:12:55Z

@liat-grozovik Done

zhangyanzhao · 2024-05-22T16:08:11Z

code PR review is on-going. Still target 202405 release

zhangyanzhao · 2024-06-04T17:27:20Z

code PRs are not approved yet, move to backlog for future release

sonic-net/SONiC#1367 Why I did it Give ability to: 1. configure limit for active login sessions. 2. configure ssh-server / serial console autologout timeout 3. configure sysrq-capabilities (enable / disable) Work item tracking Microsoft ADO (number only): How I did it Add new service that responsible for serial configuration; Update existing flows for extended ssh-server configurations in hostcfgd; Add YANG model to support new configuration. How to verify it Which release branch to backport (provide reason below if selected) Tested branch (Please provide the tested image version) Description for the changelog Link to config_db schema for YANG module changes [ssh_server](https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md#ssh_server) [serial_console](https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md#serial_console)

HLD: sonic-net/SONiC#1367 | Module name | PR | state | context | | ------------- | ------------- | ----|-----| | [sonic-buildimage](https://github.com/sonic-net/sonic-buildimage) | [Dev cli sessions](sonic-net/sonic-buildimage#17623) | ![GitHub issue/pull request detail](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-buildimage/17623) | ![GitHub pull request check contexts](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-buildimage/17623) | | [sonic-host-services](https://github.com/sonic-net/sonic-host-services) | [cli-sessions](sonic-net/sonic-host-services#99) | ![GitHub issue/pull request detail](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-host-services/99) | ![GitHub pull request check contexts](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-host-services/99) | | [sonic-utilities](https://github.com/sonic-net/sonic-utilities) | [SONIC CLI for CLI-Sessions feature #3175](#3175) | ![GitHub issue/pull request detail](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-utilities/3175) | ![GitHub pull request check contexts](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-utilities/3175) | #### What I did Implement next commands for CLI-sessions feature: - config serial-console inactivity-timeout - config serial-console sysrq-capabilities - show serial-console - config ssh max-sessions - config ssh inactivity-timeout - show ssh #### How I did it Write handlers in config/main.py for serial-console and ssh commands to cover configuration set; Write handlers in show/main.py for serial-console and ssh to cover show commands. #### How to verify it Manual tests

HLD: sonic-net/SONiC#1367 | Module name | PR | state | context | | ------------- | ------------- | ----|-----| | [sonic-buildimage](https://github.com/sonic-net/sonic-buildimage) | [Dev cli sessions](sonic-net/sonic-buildimage#17623) | ![GitHub issue/pull request detail](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-buildimage/17623) | ![GitHub pull request check contexts](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-buildimage/17623) | | [sonic-host-services](https://github.com/sonic-net/sonic-host-services) | [cli-sessions](sonic-net/sonic-host-services#99) | ![GitHub issue/pull request detail](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-host-services/99) | ![GitHub pull request check contexts](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-host-services/99) | | [sonic-utilities](https://github.com/sonic-net/sonic-utilities) | [SONIC CLI for CLI-Sessions feature sonic-net#3175](sonic-net#3175) | ![GitHub issue/pull request detail](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-utilities/3175) | ![GitHub pull request check contexts](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-utilities/3175) | #### What I did Implement next commands for CLI-sessions feature: - config serial-console inactivity-timeout - config serial-console sysrq-capabilities - show serial-console - config ssh max-sessions - config ssh inactivity-timeout - show ssh #### How I did it Write handlers in config/main.py for serial-console and ssh commands to cover configuration set; Write handlers in show/main.py for serial-console and ssh to cover show commands. #### How to verify it Manual tests

HLD for cli sessions feature (#1)

2c4c01d

Prepare HLD documents for CLI sessions feature Update ssh_config.md with auto-logout and max-syslogins parameters. Create serial-console-HLD.md

venkatmahalingam reviewed Jun 27, 2023

View reviewed changes

i-davydenko commented Jun 27, 2023

View reviewed changes

doc/ssh_config/ssh_config.md Show resolved Hide resolved

i-davydenko commented Jun 27, 2023

View reviewed changes

doc/console/serial-console-HLD.md Show resolved Hide resolved

i-davydenko commented Jun 27, 2023

View reviewed changes

doc/console/serial-console-HLD.md Show resolved Hide resolved

lguohan requested a review from liuh-80 June 27, 2023 15:54

i-davydenko commented Jun 27, 2023

View reviewed changes

doc/console/serial-console-HLD.md Show resolved Hide resolved

lguohan requested a review from qiluo-msft June 27, 2023 15:54

i-davydenko commented Jun 27, 2023

View reviewed changes

doc/console/serial-console-HLD.md Show resolved Hide resolved

lguohan reviewed Jun 27, 2023

View reviewed changes

liuh-80 reviewed Jun 28, 2023

View reviewed changes

i-davydenko added 2 commits August 4, 2023 15:50

Update serial-console-HLD.md

34a7364

Update serial-console.hld according to the comments.

Update ssh_config.md

3db77dc

Update ssh_config HLD according to the community comments.

qiluo-msft requested a review from liuh-80 August 14, 2023 00:26

qiluo-msft reviewed Aug 14, 2023

View reviewed changes

qiluo-msft requested review from Blueve and qiluo-msft August 14, 2023 00:31

qiluo-msft reviewed Aug 14, 2023

View reviewed changes

i-davydenko added 3 commits August 14, 2023 11:29

Update serial-console-HLD.md

9e32292

Fix typos.

Update ssh_config.md

8e0296d

Fix typos

Update serial-console-HLD.md

90a8d3e

Fix HLD with missed "POLICIES" container in config/cli tree.

qiluo-msft approved these changes Aug 21, 2023

View reviewed changes

liat-grozovik merged commit 10af3b0 into sonic-net:master Oct 10, 2023

This was referenced Oct 30, 2023

Dev cli sessions i-davydenko/sonic-buildimage#1

Merged

Dev cli sessions i-davydenko/sonic-host-services#1

Merged

This was referenced Dec 15, 2023

[cli-sessions] sonic-net/sonic-host-services#98

Closed

Dev cli sessions sonic-net/sonic-buildimage#17528

Closed

Dev cli sessions i-davydenko/sonic-buildimage#2

Merged

[cli-sessions] i-davydenko/sonic-host-services#2

Merged

i-davydenko mentioned this pull request Dec 26, 2023

Dev cli sessions sonic-net/sonic-buildimage#17623

Merged

i-davydenko mentioned this pull request Feb 22, 2024

SONIC CLI for CLI-Sessions feature sonic-net/sonic-utilities#3175

Merged

SvyatDemn mentioned this pull request Mar 19, 2024

added testplan for cli session feature sonic-net/sonic-mgmt#12041

Open

8 tasks

liat-grozovik assigned i-davydenko Apr 17, 2024

i-davydenko mentioned this pull request Apr 17, 2024

[cli-sessions] sonic-net/sonic-host-services#99

Merged

Conversation

i-davydenko commented Jun 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linux-foundation-easycla bot commented Jun 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhangyanzhao commented Jun 27, 2023

Uh oh!

zhangyanzhao commented Jun 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

liat-grozovik commented Aug 24, 2023

Uh oh!

Meir-renford commented Oct 2, 2023

Uh oh!

zhangyanzhao commented Feb 5, 2024

Uh oh!

liat-grozovik commented Apr 17, 2024

Uh oh!

i-davydenko commented Jun 23, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jun 23, 2023 •

edited

Loading

zhangyanzhao commented Jun 27, 2023 •

edited

Loading

qiluo-msft Aug 14, 2023 •

edited

Loading

qiluo-msft Aug 14, 2023 •

edited

Loading

qiluo-msft Aug 14, 2023 •

edited

Loading