Banner HLD by SviatoslavBoichuk · Pull Request #1361 · sonic-net/SONiC

SviatoslavBoichuk · 2023-06-12T10:10:08Z

This document provides general information about Banner implementation in SONiC

The scope of this document is to cover definition, design and implementation of SONiC Banner feature and Banner CLI.

PR title	state	context
Banner YANG Model/SystemD service
Banner Config handler
Banner Config DB
Banner CLI

linux-foundation-easycla · 2023-06-12T10:10:12Z

The committers listed above are authorized under a signed CLA.

✅ login: SviatoslavBoichuk (e82c388)

linux-foundation-easycla · 2023-06-13T08:26:38Z

❌ The email address for the commit (784d489) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please submit a support request ticket.

venkatmahalingam · 2023-06-27T15:18:28Z

doc/banner/banner_hld.md

Add default values.

venkatmahalingam · 2023-06-27T15:19:52Z

doc/banner/banner_hld.md

Instead of adding 3 containers i.e pre_login, post_login & post_logout, can we have the BANNER table name and key as "global" and three 3 attributes.

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

doc/banner/banner_hld.md

Rename the field name as motd and add descriptions for all three fields.

yxieca · 2023-06-27T15:27:13Z

Questions:

Does banner feature support dynamic contents, e.g. uptime, date, login IP, etc.
How does banner feature support contents with multiple lines?

zhangyanzhao · 2023-06-27T16:05:31Z

community review recording https://zoom.us/rec/share/4Wwy3t9ogqaeABDgjvYQ6IOwAovCEqdamwVzTeR9dxlvMK2GlOhNhrkcWfOgpeyc.dXhqq6g4IUDmdjxa

SviatoslavBoichuk · 2023-08-04T09:34:24Z

Questions:

Does banner feature support dynamic contents, e.g. uptime, date, login IP, etc.

Answer: No, it is not supported.

How does banner feature support contents with multiple lines?

Answer: yes, I updated HLD - please see detail there.

yxieca · 2023-08-07T16:44:32Z

Questions:

Does banner feature support dynamic contents, e.g. uptime, date, login IP, etc.
Answer: No, it is not supported.

What does it take to get something like this? https://gist.github.com/bearlike/d5c90dd24e43e0c5cf89342ac4331358

liat-grozovik · 2023-08-24T10:17:40Z

@SviatoslavBoichuk pls refer to the latest questions and if no further open issues, i will go a head and merge it.
when do we expect code PRs to be avaialble?

SviatoslavBoichuk · 2023-08-25T07:20:24Z

Questions:

Does banner feature support dynamic contents, e.g. uptime, date, login IP, etc.
Answer: No, it is not supported.
What does it take to get something like this? https://gist.github.com/bearlike/d5c90dd24e43e0c5cf89342ac4331358

Originally feature was designed to provide opportunity for users to configure the Banner messages (even if it is just simple string) via CLI commands and save it to Redis Config DB, so it will be restored after SONiC upgrade (better user experiance).

In case of the scripts - I don't see easy way to provide the same functionality for user as I described above.

SviatoslavBoichuk · 2023-08-25T07:23:45Z

@SviatoslavBoichuk pls refer to the latest questions and if no further open issues, i will go a head and merge it. when do we expect code PRs to be avaialble?

Provided reply to the latest questions. The code PRs planned to be published ~ middle of September (in worst case end of September).

dharmaraj-gurusamy · 2023-09-21T14:44:16Z

doc/banner/banner_hld.md

Please explain the purpose or use case of logout banner requirement.

Mention the Linux banner files (/etc/banner, /etc/motd, etc) where the banners are maintained for completeness

Purpose to have logout message is - full interface (login-motd-logout)

Updated HLD and mentioned Linux files

Sorry, still not convinced about the use case for logout banner. Linux does not support logout banner natively, seems to be something new being brought as SONiC specific. I would recommend to add logout banner, if valid use case is available.

Logout message supported by bash (bash.bash_logout file). The logout message will be written from ConfDB to new file: /etc/logout_message. And "bash.bash_logout" will be modified to read logout message from "/etc/logout_message" and display it (via 'echo'). Some vendors, for example Mellanox (https://docs.nvidia.com/networking/display/onyxv3102002/ui+commands#src-80577389_UICommands-Banner) support logout banner, so it might be useful for users.

dharmaraj-gurusamy · 2023-09-21T14:47:55Z

doc/banner/banner_hld.md

Is enable / disable of banner feature is supported? If so, unable to locate the respective commands.

Updated HLD with feature state enable/disable command (please see section 2.3.1 Config command group)

It would be great if below details are added into HLD

The behavior when feature state is disabled ==> Is it intended to revert back to default SONiC banner? If not, what is the procedure to revert back to default banner?

The behavior of when feature state is enabled ==> Is user configured banner will be applied into system only when state is enabled?

The behavior of banner configuration when feature state is disabled

The provisions to revert back to default banners independently, looks like currently the feature state disabled option is meant for that (if the assumption is correct). Both banners will be reverted to default. I would suggest to add default option in both command syntax (config login banner default, config motd banner default) and remove the feature enable / disable option. This will provide flexibility to user to manage the banners independently and consistent with other configuration commands to set default values.

Updated Flows section in HLD with explanation regarding feature state.
Regarding reverting back to default SONiC banners. AFAIK SONiC OS doesn't have an infra to restore configuration to default values, and whatever configuration you changed - to revert it back to "default" you need to configured "default" value again manually. The same is for this feature.

dharmaraj-gurusamy · 2023-09-21T14:53:21Z

doc/banner/banner_hld.md

I would recommend to update the banner files directly from hostcfgd service. This will avoid introducing another service (banner-config) which is intended to fetch the data from redis & update the banner files.
Is there any advantage of introducing new host service?

The hostcfgd (after reboot) start later then network service - means banner might not be updated when user login to the system.
It is main goal to use the systemd service as single point where the banners will be updated and it will start before network service.

dharmaraj-gurusamy · 2023-09-21T14:55:40Z

doc/banner/banner_hld.md

I hope that expected result shall be the default logout banner message after login + logout sequence

dharmaraj-gurusamy · 2023-09-21T14:57:36Z

doc/banner/banner_hld.md

I hope that expected result shall be the configured logout banner message after login + logout sequence

liat-grozovik · 2023-10-10T16:19:01Z

@yxieca @venkatmahalingam @dharmaraj-gurusamy any further comments? if not we wish to merge this PR and share the code PRs soon

dharmaraj-gurusamy · 2023-10-11T14:33:12Z

doc/banner/banner_hld.md

I hope that Debian uses /etc/login.warn file as default login banner file & Linux flavors use /etc/issue and /etc/issue.net files. Please correct me, if wrong. If so, any reason why it is not being updated & introducing new files?

Don't see /etc/login.warn file is used by SONiC. The messages is configured to /etc/motd, /etc/issue.net files.
The Banner feature is going use the same files. We introduce only 1 new file: /etc/logout_message for logout banner.
This file will be used in "bash.bash_logout" script to read and display configured logout message.

dharmaraj-gurusamy · 2023-10-11T14:36:21Z

doc/banner/banner_hld.md

Sorry, still not convinced about the use case for logout banner. Linux does not support logout banner natively, seems to be something new being brought as SONiC specific. I would recommend to add logout banner, if valid use case is available.

dharmaraj-gurusamy · 2023-10-11T14:46:26Z

doc/banner/banner_hld.md

It would be great if below details are added into HLD

The behavior when feature state is disabled ==> Is it intended to revert back to default SONiC banner? If not, what is the procedure to revert back to default banner?

The behavior of when feature state is enabled ==> Is user configured banner will be applied into system only when state is enabled?

The behavior of banner configuration when feature state is disabled

The provisions to revert back to default banners independently, looks like currently the feature state disabled option is meant for that (if the assumption is correct). Both banners will be reverted to default. I would suggest to add default option in both command syntax (config login banner default, config motd banner default) and remove the feature enable / disable option. This will provide flexibility to user to manage the banners independently and consistent with other configuration commands to set default values.

liat-grozovik · 2023-11-20T07:55:29Z

@SviatoslavBoichuk
do you have already the code PRs available? can you please add them to the PR description according to the tracking template? i will then be able to merge the HLD

SviatoslavBoichuk · 2023-11-20T09:49:42Z

@SviatoslavBoichuk do you have already the code PRs available? can you please add them to the PR description according to the tracking template? i will then be able to merge the HLD

Hi @liat-grozovik , I update description with code PRs.

qiluo-msft · 2024-01-14T05:59:19Z

doc/banner/banner_hld.md

+
+This feature require access to SONiC DB. All messages (MOTD, login and logout) saved into the SONiC config database. Hostcfgd will listen for the configuration changes in corresponding tables and restart banner-config service. Banner config service - it is simple SystemD service which runs before we get SSH connection. It reads configured messages from database and apply it to Linux.
+
+**The Linux files will be used:**


I am wondering why should user not directly update the linux files. What is the value added by this HLD?

Is there a really scenarios that we need to config banner per device? If no, maybe a single command line to update these "Linux files" will be good enough. No dependency on database, hostcfgd, no need to have another service.

The banner messages are user configuration.
We would like to provide to user simple CLI commands to configure messages.
Saved banners to Config DB will be restored after upgrade, so user don't need to set them again.

skg-net · 2024-02-06T00:38:06Z

@SviatoslavBoichuk Can you please update the Quality Metric (Alpha/Beta/GA) for the feature either in this PR comments or in HLD itself based on https://github.com/sonic-net/SONiC/blob/master/doc/SONiC%20feature%20quality%20definition.md
Thanks

- Why I did it Added Banner feature related services according to HLD: sonic-net/SONiC#1361 - How I did it Added banner-config systemd service, YANG model for new ConfDB table and YANG model tests - How to verify it Manual test Co-authored-by: Sviatoslav Boichuk <[email protected]>

- Why I did it Added Banner feature related Config DB table according to HLD: sonic-net/SONiC#1361 - How I did it Added Banner table name to schema.h. Signed-off-by: Yevhen Fastiuk <[email protected]>

…79) - Why I did it Added Banner feature related hostcfgd service according to HLD: sonic-net/SONiC#1361 This PR depends on the following PR: sonic-net/sonic-swss-common#826 - How I did it Added BannerCfg service to hostcfgd. It handles Config DB changes and restart "banner-config" systemd service which will update banners. - How to verify it Manual test Signed-off-by: Yevhen Fastiuk <[email protected]> Co-authored-by: Yevhen Fastiuk <[email protected]>

…onfiguration (#3021) What I did Added CLI commands for Banner feature according to HLD: sonic-net/SONiC#1361 How I did it Added CLI commands to: Enable/disable Banner feature Configure Banner messages: login/motd/logout Related show command How to verify it Manual testing

…onfiguration (sonic-net#3021) What I did Added CLI commands for Banner feature according to HLD: sonic-net/SONiC#1361 How I did it Added CLI commands to: Enable/disable Banner feature Configure Banner messages: login/motd/logout Related show command How to verify it Manual testing

SviatoslavBoichuk closed this Jun 13, 2023

SviatoslavBoichuk reopened this Jun 13, 2023

SviatoslavBoichuk force-pushed the banner_hld branch from 784d489 to 398f1e6 Compare June 13, 2023 08:29

SviatoslavBoichuk force-pushed the banner_hld branch from 398f1e6 to f1fdf14 Compare June 20, 2023 09:35

venkatmahalingam reviewed Jun 27, 2023

View reviewed changes

SviatoslavBoichuk force-pushed the banner_hld branch from f1fdf14 to f8ed1f4 Compare August 4, 2023 08:38

SviatoslavBoichuk force-pushed the banner_hld branch from f8ed1f4 to bdfa859 Compare August 4, 2023 11:47

yxieca previously approved these changes Aug 7, 2023

View reviewed changes

SviatoslavBoichuk force-pushed the banner_hld branch from bdfa859 to bb18a23 Compare September 11, 2023 07:31

dharmaraj-gurusamy suggested changes Sep 21, 2023

View reviewed changes

SviatoslavBoichuk dismissed yxieca’s stale review via 5fb87d3 October 4, 2023 10:49

SviatoslavBoichuk force-pushed the banner_hld branch from 5fb87d3 to f2e2c91 Compare October 4, 2023 10:54

dharmaraj-gurusamy reviewed Oct 11, 2023

View reviewed changes

This was referenced Oct 20, 2023

[Banner] Added CLI commands to configure Banner and display current configuration sonic-net/sonic-utilities#3021

Merged

[Banner] Add new feature including config, yang and tests sonic-net/sonic-buildimage#16957

Merged

[SONiC] Banner HLD

e82c388

SviatoslavBoichuk force-pushed the banner_hld branch from f2e2c91 to e82c388 Compare October 22, 2023 07:00

venkatmahalingam approved these changes Nov 20, 2023

View reviewed changes

dharmaraj-gurusamy approved these changes Nov 20, 2023

View reviewed changes

liat-grozovik approved these changes Dec 3, 2023

View reviewed changes

liat-grozovik merged commit 6c0b90b into sonic-net:master Dec 3, 2023

liat-grozovik changed the title ~~[SONiC] Banner HLD~~ Banner HLD Dec 3, 2023

This was referenced Dec 15, 2023

[Banner] Added BannerCfg class to hostcfgd to handle Banner messages sonic-net/sonic-host-services#79

Merged

[Banner] Added BANNER_MESSAGE table to ConfigDB sonic-net/sonic-swss-common#826

Merged

qiluo-msft reviewed Jan 14, 2024

View reviewed changes

Yuval-Mellanox assigned fastiuk Jul 17, 2024


		This feature require access to SONiC DB. All messages (MOTD, login and logout) saved into the SONiC config database. Hostcfgd will listen for the configuration changes in corresponding tables and restart banner-config service. Banner config service - it is simple SystemD service which runs before we get SSH connection. It reads configured messages from database and apply it to Linux.

		The Linux files will be used:

Conversation

SviatoslavBoichuk commented Jun 12, 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 12, 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 13, 2023

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!

venkatmahalingam Jun 27, 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!

yxieca commented Jun 27, 2023

Uh oh!

zhangyanzhao commented Jun 27, 2023

Uh oh!

SviatoslavBoichuk commented Aug 4, 2023

Uh oh!

yxieca commented Aug 7, 2023

Uh oh!

liat-grozovik commented Aug 24, 2023

Uh oh!

SviatoslavBoichuk commented Aug 25, 2023

Uh oh!

SviatoslavBoichuk commented Aug 25, 2023

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!

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!

liat-grozovik commented Oct 10, 2023

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!

liat-grozovik commented Nov 20, 2023

Uh oh!

SviatoslavBoichuk commented Nov 20, 2023

Uh oh!

Choose a reason for hiding this comment

SviatoslavBoichuk commented Jun 12, 2023 •

edited

Loading

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

edited

Loading

venkatmahalingam Jun 27, 2023 •

edited

Loading