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

Adaptations to work inside a container #669

Merged
merged 2 commits into from
Jul 28, 2022
Merged

Conversation

ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Jul 25, 2022

Problem

yast2-bootloader does not work inside a container out of the box (see bsc#1199840). There are several things that need to be fixed:

  1. storage-ng probing must work. See Fix libstorage-ng probing yast-in-container#10
  2. The command grub2-mkpasswd-pbkdf2 is executed locally (ie. in the container) instead of the target system due to historical reasons that do not longer apply.
  3. Problems installing the needed packages.

Solution

As mentioned, the first problem is addressed in a separate pull request at another repository.

This fixes the second problem by making sure grub2-mkpasswd-pbkdf2 is executed in the target system.

Problem 3 remains to be fixed (and likely will be addressed by an upcoming pull request at yast-yast2).

Testing

  • Tested manually that containerized execution gets fixed by this.
  • Tested manually that installation is not broken by this because now grub2-mkpasswd-pbkdf2 is executed during the proposal phase, when Execute.locally and Execute.on_target are still equivalent.

Notes

During manual testing I found the grub2 password is logged plain at the YaST logs. Reported as bsc#1201962

It was executed locally (ie. on the inst-sys during installation) due to
bsc#900039. But that does not longer apply.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 88.369% when pulling 2efaf63 on ancorgs:container into 16b0cfc on yast:master.

@@ -131,7 +131,7 @@ def disable
end

def encrypt(password)
result = Yast::Execute.locally("/usr/bin/grub2-mkpasswd-pbkdf2",
result = Yast::Execute.on_target("/usr/bin/grub2-mkpasswd-pbkdf2",
Copy link
Member

Choose a reason for hiding this comment

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

well, another option I see is to have that mkpasswd installed locally as it does not need to be on target system if it is decided to split it from grub2. We need on target system mainly tools used to create config and install stage1 due to new kernel and new grub2 modules - so grub2-mkconfig and grub2-install.

In general I am not against change if it is properly tested during upgrade from old SLE...do we still support migration from SLE11?

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 decided to go this way because is simpler and I don't think grub2-mkpasswd-pbkdf2 will be split from the grub2 package in the foreseeable future.

That tool is included in the grub2 package even for SLE-11. So if you are upgrading such a system the tool is already in the target.

Copy link
Member

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Bearing in mind your comment at https://github.com/yast/yast-bootloader/pull/669/files#r929612013, it LGTM.

Please, do not forget to report the bug mentioned in the description

During manual testing I found the grub2 password is logged plain at the YaST logs. Bug to be reported.

@ancorgs ancorgs merged commit 7fb7d38 into yast:master Jul 28, 2022
@yast-bot
Copy link
Contributor

✔️ Public Jenkins job #115 successfully finished
✔️ Created OBS submit request #991562

@yast-bot
Copy link
Contributor

✔️ Internal Jenkins job #76 successfully finished
✔️ Created IBS submit request #276728

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.

5 participants