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

Check config file not empty after modify it in hostcfgd. #36

Merged
merged 6 commits into from
Feb 2, 2023

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Jan 16, 2023

What I did
Check /etc/pam.d/sshd integrity after modify it in hostcfgd.

Why I did it
Found some incident that /etc/pam.d/sshd become empty file during OR upgrade.

How I verified it
Pass all UT.
Add new UT to cover new code.

Details if related

@liuh-80 liuh-80 marked this pull request as ready for review January 16, 2023 02:07
@liuh-80 liuh-80 requested a review from qiluo-msft January 16, 2023 02:07
scripts/hostcfgd Outdated
@@ -781,11 +792,29 @@ class AaaCfg(object):
interface_ip = ipv4_addr
return interface_ip


def check_file_integrity(self, filename):
Copy link
Contributor

@qiluo-msft qiluo-msft Jan 18, 2023

Choose a reason for hiding this comment

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

check_file_integrity

Strictly speaking, this file is only check file non-empty. If this is expected and good enough, please change the function name to narrow scope.

Otherwise, you may find some command line to verify the config. For example, sshd -T could be used to verify /etc/ssh/sshd_config. I am not sure if you can find the verification command for any other command. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed by change function name.

scripts/hostcfgd Outdated
@@ -781,11 +792,29 @@ class AaaCfg(object):
interface_ip = ipv4_addr
return interface_ip


def check_file_integrity(self, filename):
rc, out = exec_cmd(cmd="du -b {}".format(filename), verbose=False)
Copy link
Contributor

@qiluo-msft qiluo-msft Jan 18, 2023

Choose a reason for hiding this comment

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

exec_cmd

This is python. So prefer python API than equivalent command line invoke #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@liuh-80 liuh-80 changed the title Check config file integrity after modify it in hostcfgd. Check config file not empty after modify it in hostcfgd. Jan 18, 2023
scripts/hostcfgd Outdated
syslog.syslog(syslog.LOG_ERR, "file size check failed: {} is missing".format(filename))
return

# du command result is: 'size\tpath'
Copy link
Contributor

@qiluo-msft qiluo-msft Jan 20, 2023

Choose a reason for hiding this comment

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

du command result is: 'size\tpath'

The comment is not relevant any more. Please remove it. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@liuh-80 liuh-80 merged commit 2807404 into sonic-net:master Feb 2, 2023
@liuh-80 liuh-80 deleted the dev/liuh/fix-empty-sshd branch February 2, 2023 06:24
@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 7, 2023

Remove the [Request for 202205 branch] label, because for release <= 202205 need manually cherry-pick

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 7, 2023

Manually cherry-pick PR created: sonic-net/sonic-buildimage#14115

qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Mar 27, 2023
…4385)

**What I did**
Check /etc/pam.d/sshd integrity after modify it in hostcfgd.

**Why I did it**
Found some incident that /etc/pam.d/sshd become empty file during OR upgrade. 

**How I verified it**
Pass all UT.
Add new UT to cover new code.

**Details if related**
This is a manually cherry-pick PR for sonic-net/sonic-host-services#36
StormLiangMS pushed a commit that referenced this pull request Apr 20, 2023
**What I did**
Check /etc/pam.d/sshd integrity after modify it in hostcfgd.

**Why I did it**
Found some incident that /etc/pam.d/sshd become empty file during OR upgrade. 

**How I verified it**
Pass all UT.
Add new UT to cover new code.

**Details if related**
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.

3 participants