Skip to content

Conversation

@liangxin1300
Copy link
Collaborator

@liangxin1300 liangxin1300 commented Mar 12, 2025

Ensure the trace directory is specified either through the argument or by using the value of the heartbeat_dir option. Eliminate any inconsistencies that might arise from heartbeat_dir (from etc/crm/crm.conf) or from HA_VARLIB (from /usr/lib/ocf/heartbeat/ocf-directories).

@liangxin1300 liangxin1300 force-pushed the 20250312_cleanup_cluster_glue branch from 0246fa5 to 6f149cd Compare March 13, 2025 07:21
@liangxin1300 liangxin1300 changed the title Dev: config: Change default value of 'heartbeat_dir' to '/var/lib/pac… Dev: Drop heartbeat_dir option Mar 13, 2025
@liangxin1300 liangxin1300 force-pushed the 20250312_cleanup_cluster_glue branch 3 times, most recently from 511b29e to 5bf8df2 Compare March 13, 2025 07:38
@codecov
Copy link

codecov bot commented Mar 13, 2025

Codecov Report

Attention: Patch coverage is 80.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 69.96%. Comparing base (ee7aaf4) to head (ee920d1).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
crmsh/ui_resource.py 79.16% 5 Missing ⚠️
crmsh/utils.py 83.33% 1 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
integration 54.17% <80.00%> (+0.02%) ⬆️
unit 52.57% <3.33%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
crmsh/config.py 81.48% <ø> (ø)
crmsh/utils.py 66.39% <83.33%> (+0.05%) ⬆️
crmsh/ui_resource.py 72.83% <79.16%> (+0.14%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@liangxin1300 liangxin1300 marked this pull request as ready for review March 13, 2025 08:06
@liangxin1300 liangxin1300 force-pushed the 20250312_cleanup_cluster_glue branch 2 times, most recently from c80ba6f to ee920d1 Compare March 14, 2025 06:02
crmsh/utils.py Outdated
"""
source a script and get the value of a variable
"""
cmd = f"source {script_path} && echo ${var_name}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

DANGEROUS:

  1. Sourcing a script is vulnerable to shell injection.
  2. script_path and var_name is not quoted.

Although it is safe when sourcing trusted code, this should not be defined as a common utility function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about:

Suggested change
cmd = f"source {script_path} && echo ${var_name}"
cmd = ['/bin/bash', '-c', f'source {script_path} && echo ${var_name}']
rc, out, _ = ShellUtils().get_stdout_stderr(cmd, shell=False)

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree Nicholas for the security risk, and not to source the script. Instead, it is safe for the use case to directly use python re to parse {HA_VARLIB:=/var/lib/heartbeat}.

If "rpm -ql resource-agents" is not available, then crmsh falls back to the hard code of /usr/lib/ocf/lib/heartbeat/ocf-directories.

@liangxin1300 liangxin1300 force-pushed the 20250312_cleanup_cluster_glue branch from ee920d1 to 71d0bdb Compare March 18, 2025 13:45
@liangxin1300 liangxin1300 force-pushed the 20250312_cleanup_cluster_glue branch from 71d0bdb to b1cd65f Compare March 19, 2025 14:19
@liangxin1300 liangxin1300 changed the title Dev: Drop heartbeat_dir option Dev: ui_resource: Refactor do_trace function Mar 19, 2025
@liangxin1300 liangxin1300 force-pushed the 20250312_cleanup_cluster_glue branch from b1cd65f to 320b795 Compare March 19, 2025 14:22
Ensure the trace directory is specified either through the argument or
by using the value of the heartbeat_dir option. Eliminate any
inconsistencies that might arise from heartbeat_dir (from
etc/crm/crm.conf) or from HA_VARLIB (from
/usr/lib/ocf/heartbeat/ocf-directories).
@liangxin1300 liangxin1300 force-pushed the 20250312_cleanup_cluster_glue branch from 320b795 to 7b8e201 Compare March 19, 2025 14:50
@liangxin1300 liangxin1300 merged commit dd765c5 into ClusterLabs:master Mar 21, 2025
32 checks passed
liangxin1300 added a commit that referenced this pull request Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants