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

Openssh PermitRootLogin update from el8 to el9 #864

Merged
merged 5 commits into from
May 6, 2022

Conversation

Jakuje
Copy link
Contributor

@Jakuje Jakuje commented Mar 22, 2022

Again, the first commit overlaps with #860 and #863, but it should be pretty uncontroversial.

We already had a checks for PermitRootLogin configuration in the ugprade from el7toel8, but the el8toel9 requires a bit different corner cases to capture possible issues. For more info, see the comments in the code.

@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 mergable.
If you want to re-run tests or request review, you can use following commands as a comment:

  • leapp-ci build to run copr build and e2e tests in OAMG CI
  • review please to notify leapp developers of review request

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.

@leapp-bot
Copy link
Collaborator

This PR has been linked in issue tracker (#OAMG-6712).

@Jakuje Jakuje force-pushed the openssh-permitrootlogin-8to9 branch 2 times, most recently from eef48a1 to 1fe3ec4 Compare March 22, 2022 14:50
"""
name = 'openssh_permit_root_login'
consumes = (OpenSshConfig, )
produces = (Report, )
tags = (ChecksPhaseTag, IPUWorkflowTag, )

common_resources = [
Copy link
Member

@fernflower fernflower Apr 1, 2022

Choose a reason for hiding this comment

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

Wow, does it work? I was under impression that one can't declare additional class level args in an Actor, the framework would not allow that.
I might be mistaken though, will check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this to the global constant for simplicity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whether it works, I dont know. Tried with copr repo on rhel8, but I am lost at the following step, which fails for me for no obvious reason when I follow the docs:

https://leapp.readthedocs.io/en/latest/el7toel8/actor-rhel7-to-rhel8.html#executing-a-single-actor-that-uses-the-workflow-config

# snactor run --save-output IPUWorkflowConfig
Process Process-101:
Traceback (most recent call last):
  File "/usr/lib64/python3.6/multiprocessing/process.py", line 258, in _bootstrap
    self.run()
  File "/usr/lib64/python3.6/multiprocessing/process.py", line 93, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib/python3.6/site-packages/leapp/repository/actor_definition.py", line 72, in _do_run
    actor_instance.run(*args, **kwargs)
  File "/usr/lib/python3.6/site-packages/leapp/actors/__init__.py", line 289, in run
    self.process(*args)
  File "/usr/share/leapp-repository/repositories/system_upgrade/common/actors/ipuworkflowconfig/actor.py", line 19, in process
    ipuworkflowconfig.produce_ipu_config(self)
  File "/usr/share/leapp-repository/repositories/system_upgrade/common/actors/ipuworkflowconfig/libraries/ipuworkflowconfig.py", line 77, in produce_ipu_config
    target=target_version
  File "/usr/lib/python3.6/site-packages/leapp/models/__init__.py", line 90, in __init__
    getattr(defined_fields[field], init_method)(kwargs, field, self)
  File "/usr/lib/python3.6/site-packages/leapp/models/fields/__init__.py", line 110, in from_initialization
    self._validate_model_value(value=source_value, name=name)
  File "/usr/lib/python3.6/site-packages/leapp/models/fields/__init__.py", line 179, in _validate_model_value
    super(BuiltinField, self)._validate_model_value(value, name)
  File "/usr/lib/python3.6/site-packages/leapp/models/fields/__init__.py", line 60, in _validate_model_value
    raise ModelViolationError('The value of "{name}" field is None, but this is not allowed'.format(name=name))
leapp.models.fields.ModelViolationError: The value of "target" field is None, but this is not allowed

Copy link
Member

@fernflower fernflower Apr 1, 2022

Choose a reason for hiding this comment

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

@pirat89 We need to fix snactor asap (n\n-1 patch broke it but we were putting it off) :(

Sorry for this, but testing with snactor won't work at the moment. This will be among my top priorities for the next week though. In the meantime you could use our vagrant box maybe? @MichalHe has explained the details how to do this here #867 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Created oamg/leapp#771 , all updates will go there.

Copy link
Member

Choose a reason for hiding this comment

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

set these envars before running the snactor

export LEAPP_UPGRADE_PATH_TARGET_RELEASE=9.0
export LEAPP_UPGRADE_PATH_FLAVOUR=default

@fernflower
Copy link
Member

/rerun

@github-actions
Copy link

github-actions bot commented Apr 1, 2022

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

@github-actions
Copy link

github-actions bot commented Apr 1, 2022

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

@github-actions
Copy link

github-actions bot commented Apr 1, 2022

Testing Farm request for RHEL-8.6.0-Nightly/3933138 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@fernflower
Copy link
Member

/rerun

@github-actions
Copy link

github-actions bot commented Apr 1, 2022

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

@github-actions
Copy link

github-actions bot commented Apr 1, 2022

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

@github-actions
Copy link

github-actions bot commented Apr 1, 2022

Testing Farm request for RHEL-8.6.0-Nightly/3938103 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@Rezney
Copy link
Member

Rezney commented May 2, 2022

/rerun

@github-actions
Copy link

github-actions bot commented May 2, 2022

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

@github-actions
Copy link

github-actions bot commented May 2, 2022

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

@github-actions
Copy link

github-actions bot commented May 2, 2022

Testing Farm request for RHEL-8.6.0-Nightly/4355130 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@Rezney
Copy link
Member

Rezney commented May 3, 2022

/rerun

@github-actions
Copy link

github-actions bot commented May 3, 2022

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

@github-actions
Copy link

github-actions bot commented May 3, 2022

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

@github-actions
Copy link

github-actions bot commented May 3, 2022

Testing Farm request for RHEL-8.6.0-Nightly/4359152 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@Jakuje Jakuje force-pushed the openssh-permitrootlogin-8to9 branch 2 times, most recently from 2ecc487 to 2c96f46 Compare May 3, 2022 14:05
@Rezney
Copy link
Member

Rezney commented May 4, 2022

@Jakuje please rebase against the latest master. Thanks.

@Jakuje Jakuje force-pushed the openssh-permitrootlogin-8to9 branch from 2c96f46 to e637a6b Compare May 4, 2022 14:11
.format(get_source_major_version()))

def process7to8(self, config):
# When the configuration does not contain the PermitRootLogin directive and
Copy link
Member

Choose a reason for hiding this comment

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

@Jakuje do we not consider a commented PermitRootLogin directive anymore for the inhibitor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not think so. Why would we?

Copy link
Member

Choose a reason for hiding this comment

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

@Jakuje
Well, currently it works like that (in master). I was under impression that if the config file was modified and there is not explicitly set "PermitrootLogin", we trigger the inhibitor. Even if "PermitrootLogin" is commented, not removed. Did I get it wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. That is kept in 7to8. If PermitRootLogin is commented or removed, it is like it would not be there at all (it should be a job of the parser though). This part of code is just moved from repos/system_upgrade/el7toel8/actors/opensshpermitrootlogincheck/actor.py:44 so it should not change on 7to8 execution.

@Jakuje Jakuje force-pushed the openssh-permitrootlogin-8to9 branch from e637a6b to 772a31d Compare May 5, 2022 18:15
Jakuje added 5 commits May 6, 2022 10:02
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Neither of the inhibitor are useful if the configuration file was not
modified and the upgrade of the file will be handled by RPM, keeping the
root logins enabled by pulling the new configuration file from new
package.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
In the past, both of the inhibitors were triggered when the
configuration file did not contain any PermitRootLogin configuration
option. But this really does not make any sense to report the second
inhibitor if the first one is already raised.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
The original code was quite confusing. This reuses the global_value
function and checks for the only rare corner case we want to consider
allowing without inhibiting the upgrade.

The test coverage is still passing with the new code.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
@Jakuje Jakuje force-pushed the openssh-permitrootlogin-8to9 branch from 772a31d to 5acf661 Compare May 6, 2022 08:02
@Rezney Rezney merged commit 5862f8b into oamg:master May 6, 2022
@fernflower fernflower mentioned this pull request May 23, 2022
@pirat89 pirat89 added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Jul 22, 2022
pirat89 added a commit to pirat89/leapp-repository that referenced this pull request Aug 23, 2022
## Packaging
- Provide and require leapp-repository-dependencies 7 (oamg#952)
- Provide `leapp-command(<CMD>)` for each CLI command provided by leapp-repository (oamg#947)
- Require dracut, kmod, procps-ng on RHEL 8+ (oamg#952)
- Require leapp-framework >= 3.1 (oamg#905, oamg#927)

## Upgrade handling
### Fixes
-  Do not create the upgrade bootloader entry when the dnf dry-run actor fails  (oamg#912)
- Do not inhibit in-place upgrades in case LUKS volumes are Ceph OSDs (oamg#735)
- Fix & improve application of custom selinux rules to be less error prone and do not override changes done by RPM scriptlets (oamg#925)
- Fix detection of deprecated devices (and drivers) regarding the PCI address (oamg#881)
- Fix detection of deprecated kernel modules (oamg#874)
- Fix the false positive NFS storage detection on NFS servers (oamg#888)
- Fix the issues on systems with the LANGUAGE environment variable (oamg#887)
- Fix the root directory scan to deal with non-utf8 filenames (oamg#927)
- Skip comment lines when parsing the GRUB configuration file (oamg#883)
- Stop propagating the “debug” and ”enforcing=0” kernel cmdline options into the target kernel cmdline options (oamg#938, oamg#950)
- [IPU 7 -> 8] Fix the upgrade of the Satellite server (oamg#875, oamg#878, oamg#879 oamg#890, oamg#899, oamg#916, 934)
- [IPU 7 -> 8] Fix SSSD: Prune old cache files (the format of data is incompatible) (oamg#922)
- [IPU 8 -> 9] Enable the CRB repository for the upgrade only if enabled on the source system (oamg#942)
- [IPU 8 -> 9] Drop obsoleted actor blocking upgrade on z16 (oamg#892)
- [IPU 8 -> 9] Fix cloud provider detection on AWS (oamg#920)
- [IPU 8 -> 9] Fix detention of the latest kernel on RHEL 8+ systems (oamg#909)
- [IPU 8 -> 9] Fix issues caused by leapp artifacts from previous in-place upgrades (oamg#889)
- [IPU 8 -> 9] Fix issues with false positive switch to emergency console during the upgrade (oamg#906)
- [IPU 8 -> 9] Fix swap page size on aarch64 (oamg#937, oamg#948)
- [IPU 8 -> 9] Fix the VDO scanner to skip partitions unrelated to VDO and adjust error messages (oamg#919)

### Enhancements
- Add 8.7 & 9.1 Beta & GA product certificates (oamg#891)
- Detect /var/lib/leapp being mounted in a non-persistent fashion (oamg#921)
- Detect /var/lib/leapp mounted with the noexec option (oamg#908)
- Improve the report msg when NFS partitions are discovered providing info about concrete mountpoints (oamg#806)
- Inform about necessary migrations related to bacula-director (oamg#896)
- [IPU 7 -> 8] The default upgrade path for RHEL SAP is 7.9 -> 8.6 (oamg#939)
- [IPU 7 -> 8] Detect and fix missing newline at the end of /etc/default/grub (oamg#945)
- [IPU 7 -> 8] Handle upgrades of SAP Apps systems on Azure (oamg#926)
- [IPU 7 -> 8] Handle upgrades on RHUI Google Cloud (oamg#897, oamg#946)
- [IPU 8 -> 9] Support upgrade path RHEL 8.7 -> 9.0 and RHEL SAP 8.6 -> 9.0 (oamg#903, oamg#894)
- [IPU 8 -> 9] Add actors covering removal of NIS components on RHEL 9 (oamg#851)
- [IPU 8 -> 9] Add checks for obsolete .NET versions (oamg#867)
- [IPU 8 -> 9] Allow specifying the report schema v1.2.0 (oamg#872)
- [IPU 8 -> 9] Check and handle upgrades with custom crypto policies (oamg#898)
- [IPU 8 -> 9] Check and migrate OpenSSH configuration (oamg#864, oamg#860)
- [IPU 8 -> 9] Check and migrate multipath configuration the upgrade (oamg#886)
- [IPU 8 -> 9] Check minimum memory requirements (oamg#935)
- [IPU 8 -> 9] Enable Base and SAP In-place upgrades on Azure (oamg#943)
- [IPU 8 -> 9] Enable in-place upgrades in Azure RHEL 8 base images using RHUI (oamg#918)
- [IPU 8 -> 9] Handle upgrades of SAP systems on AWS (oamg#924)
- [IPU 8 -> 9] Inhibit upgrade when NVIDIA driver is detected (oamg#880)
- [IPU 8 -> 9] Migrate blocklisted CAs (oamg#882)
- [IPU 8 -> 9] Migrate the OpenSSL configuration (oamg#900)
- [IPU 8 -> 9] Report changes around SCP and SFTP (oamg#863, oamg#893)

## Additional changes interesting for devels
- Extend LsblkEntry model in StorageInfo by kernel name and size of partition in bytes (oamg#919)
- Mass refactoring: Fix imports in actors and libraries to follow project guidelines (oamg#932)
- Mass refactoring: Replace use of deprecated `reporting.(Tags|Flags)` by `reporting.Groups` (oamg#932)
- PESEventScanner actor has been fully refactored  (oamg#856, oamg#941)
- Use library function is_inhibitor to check for failures (oamg#905)

Signed-off-by: Petr Stodulka <pstodulk@redhat.com>
@pirat89 pirat89 mentioned this pull request Aug 23, 2022
pirat89 added a commit that referenced this pull request Aug 23, 2022
## Packaging
- Provide and require leapp-repository-dependencies 7 (#952)
- Provide `leapp-command(<CMD>)` for each CLI command provided by leapp-repository (#947)
- Require dracut, kmod, procps-ng on RHEL 8+ (#952)
- Require leapp-framework >= 3.1 (#905, #927)

## Upgrade handling
### Fixes
-  Do not create the upgrade bootloader entry when the dnf dry-run actor fails  (#912)
- Do not inhibit in-place upgrades in case LUKS volumes are Ceph OSDs (#735)
- Fix & improve application of custom selinux rules to be less error prone and do not override changes done by RPM scriptlets (#925)
- Fix detection of deprecated devices (and drivers) regarding the PCI address (#881)
- Fix detection of deprecated kernel modules (#874)
- Fix the false positive NFS storage detection on NFS servers (#888)
- Fix the issues on systems with the LANGUAGE environment variable (#887)
- Fix the root directory scan to deal with non-utf8 filenames (#927)
- Skip comment lines when parsing the GRUB configuration file (#883)
- Stop propagating the “debug” and ”enforcing=0” kernel cmdline options into the target kernel cmdline options (#938, #950)
- [IPU 7 -> 8] Fix the upgrade of the Satellite server (#875, #878, #879 #890, #899, #916, 934)
- [IPU 7 -> 8] Fix SSSD: Prune old cache files (the format of data is incompatible) (#922)
- [IPU 8 -> 9] Enable the CRB repository for the upgrade only if enabled on the source system (#942)
- [IPU 8 -> 9] Drop obsoleted actor blocking upgrade on z16 (#892)
- [IPU 8 -> 9] Fix cloud provider detection on AWS (#920)
- [IPU 8 -> 9] Fix detention of the latest kernel on RHEL 8+ systems (#909)
- [IPU 8 -> 9] Fix issues caused by leapp artifacts from previous in-place upgrades (#889)
- [IPU 8 -> 9] Fix issues with false positive switch to emergency console during the upgrade (#906)
- [IPU 8 -> 9] Fix swap page size on aarch64 (#937, #948)
- [IPU 8 -> 9] Fix the VDO scanner to skip partitions unrelated to VDO and adjust error messages (#919)

### Enhancements
- Add 8.7 & 9.1 Beta & GA product certificates (#891)
- Detect /var/lib/leapp being mounted in a non-persistent fashion (#921)
- Detect /var/lib/leapp mounted with the noexec option (#908)
- Improve the report msg when NFS partitions are discovered providing info about concrete mountpoints (#806)
- Inform about necessary migrations related to bacula-director (#896)
- [IPU 7 -> 8] The default upgrade path for RHEL SAP is 7.9 -> 8.6 (#939)
- [IPU 7 -> 8] Detect and fix missing newline at the end of /etc/default/grub (#945)
- [IPU 7 -> 8] Handle upgrades of SAP Apps systems on Azure (#926)
- [IPU 7 -> 8] Handle upgrades on RHUI Google Cloud (#897, #946)
- [IPU 8 -> 9] Support upgrade path RHEL 8.7 -> 9.0 and RHEL SAP 8.6 -> 9.0 (#903, #894)
- [IPU 8 -> 9] Add actors covering removal of NIS components on RHEL 9 (#851)
- [IPU 8 -> 9] Add checks for obsolete .NET versions (#867)
- [IPU 8 -> 9] Allow specifying the report schema v1.2.0 (#872)
- [IPU 8 -> 9] Check and handle upgrades with custom crypto policies (#898)
- [IPU 8 -> 9] Check and migrate OpenSSH configuration (#864, #860)
- [IPU 8 -> 9] Check and migrate multipath configuration the upgrade (#886)
- [IPU 8 -> 9] Check minimum memory requirements (#935)
- [IPU 8 -> 9] Enable Base and SAP In-place upgrades on Azure (#943)
- [IPU 8 -> 9] Enable in-place upgrades in Azure RHEL 8 base images using RHUI (#918)
- [IPU 8 -> 9] Handle upgrades of SAP systems on AWS (#924)
- [IPU 8 -> 9] Inhibit upgrade when NVIDIA driver is detected (#880)
- [IPU 8 -> 9] Migrate blocklisted CAs (#882)
- [IPU 8 -> 9] Migrate the OpenSSL configuration (#900)
- [IPU 8 -> 9] Report changes around SCP and SFTP (#863, #893)

## Additional changes interesting for devels
- Extend LsblkEntry model in StorageInfo by kernel name and size of partition in bytes (#919)
- Mass refactoring: Fix imports in actors and libraries to follow project guidelines (#932)
- Mass refactoring: Replace use of deprecated `reporting.(Tags|Flags)` by `reporting.Groups` (#932)
- PESEventScanner actor has been fully refactored  (#856, #941)
- Use library function is_inhibitor to check for failures (#905)

Signed-off-by: Petr Stodulka <pstodulk@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

5 participants