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

Report txt: fix printing of non-ascii details #844

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

pirat89
Copy link
Member

@pirat89 pirat89 commented Oct 11, 2023

Previous commit introduced couple of issues regarding details of reports that could lead to situations like:

  • remediation instructions has not been printed when non-ascii characters have been present
  • possible unwanted empty line when remediation has been specified but relative symlinks hasn't
  • if the URL title contained non-ascii character, it has been broken too (on py2)

This should handle all mentioned problems when generating the txt file.

@github-actions
Copy link

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

  • review please @oamg/developers to notify leapp developers of the review request
  • /packit copr-build to submit a public copr build using packit

To launch regression testing public members of oamg organization can leave the following comment:

  • /rerun to schedule basic regression tests using this pr build and leapp-repository*master* as artifacts
  • /rerun 42 to schedule basic regression tests using this pr build and leapp-repository*PR42* as artifacts
  • /rerun-sst to schedule sst tests using this pr build and leapp-repository*master* as artifacts
  • /rerun-sst 42 to schedule sst tests using this pr build and leapp-repository*PR42* as artifacts

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

@pirat89
Copy link
Member Author

pirat89 commented Oct 11, 2023

/rerun

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/6516691

@pirat89 pirat89 marked this pull request as ready for review October 11, 2023 11:59
@github-actions
Copy link

Testing Farm request for RHEL-8.6-rhui/6513877;6516691 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-rhui/6513877;6516691 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

Previous commit introduced couple of issues regarding details of
reports that could lead to situations like:
* remediation instructions has not been printed when non-ascii
  characters have been present
* possible unwanted empty line when remediation has been specified
  but relative symlinks hasn't
* if the URL title contained non-ascii character, it has been broken
  too (on py2)

This should handle all mentioned problems when generating the txt
file.
@pirat89 pirat89 force-pushed the fix-report-unicode branch from b89d92b to 0809701 Compare October 11, 2023 15:19
@pirat89
Copy link
Member Author

pirat89 commented Oct 11, 2023

@oamg/developers

tested manual on RHEL 7 & RHEL 8:

----------------------------------------
Risk Factor: high
Title: [snip]
Summary: [snip]
Related links:
    - Difference in Python versions and support in RHEL 8: https://red.ht/rhel-8-python
    - kurník řepa: https://red.ht/rhel-8-python
Remediation: [hint] Please run "alternatives --set pythořn /usr/bin/python3" after upgra%čde
Key: 0c98585b1d8d252eb540bf61560094f3495351f5

Risk Factor: high (inhibitor)
Title: Upgrade requires links in root directory to be relative
Summary: After rebooting, parts of the upgrade process can fail if symbolic links in / point to absolute paths.
Please change these links to relative ones.
Remediation: [command] sh -c ln -snf bořek /bořislav && ln -snf root/tlučhuba /kuřislav
Key: 3d895ad37ceaf4157864d439edb6bd75562061fa

(yes, these invalid commands etc is the change I wanted, so the output looks like it should)

@pirat89 pirat89 requested review from PeterMocary and a team October 11, 2023 15:57
@pirat89 pirat89 added the bug label Oct 11, 2023
@pirat89
Copy link
Member Author

pirat89 commented Oct 11, 2023

Note there are no any unit-tests for this and I am skipping to add them now to unblock upstream testing of upgrades.

Copy link
Member

@PeterMocary PeterMocary left a comment

Choose a reason for hiding this comment

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

LGTM!

  • - remediation gets printed even with non-ascii characters
  • - unwanted empty line is no longer present when external links aren't specified
  • - link with non-ascii characters gets printed just fine

Tested on RHEL8 and RHEL7 as follows:

Risk Factor: high (inhibitor)
Title: Local repository detected
Summary: The following local repository has been found: my-repo (their baseurl starts with file:///). Currently leapp does not support this option.
Related links:
    - Customizing your Red Hat Enterprise Linux in-place upgrade ľščťžýáíéôúäň: ľščťžýáíéôúäň https://red.ht/ipu-customisation-repos-known-issues
Remediation: [hint] By using Apache HTTP Server you can expose your local repository via http. See the linked article for details. ľščťžýáíéôúäň
Key: 3450b32bfad8e1837b73b18e56c556d1052d5e4b
----------------------------------------
Risk Factor: high
Title: Remote root logins globally allowed using password
Summary: RHEL9 no longer allows remote root logins, but the server configuration explicitly overrides this default. The configuration file will not be updated and root is still going to be allowed to login with password. This is not recommended and considered as a security risk.
Remediation: [hint] If you depend on remote root logins using passwords, consider setting up a different user for remote administration. Otherwise you can ignore this message.
Key: e738f78bc8f3a84411a4210e3b609057139d1855

leapp/utils/report.py Show resolved Hide resolved
@PeterMocary PeterMocary merged commit 0eac9d8 into oamg:master Oct 11, 2023
14 checks passed
@pirat89 pirat89 added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Oct 12, 2023
pirat89 added a commit to pirat89/leapp that referenced this pull request Feb 13, 2024
## Framework
### Enhancements
- Generated txt report file now contains also External links when part of a report (oamg#842, oamg#844)

## stdlib
### Fixes
- The `run` function is now compatible with Python 3.12 (oamg#845)
@pirat89 pirat89 mentioned this pull request Feb 13, 2024
pirat89 added a commit that referenced this pull request Feb 13, 2024
## Framework
### Enhancements
- Generated txt report file now contains also External links when part of a report (#842, #844)

## stdlib
### Fixes
- The `run` function is now compatible with Python 3.12 (#845)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants