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

Adapt password typing for slower arch #20097

Closed
wants to merge 1 commit into from

Conversation

mloviska
Copy link
Contributor

@mloviska mloviska commented Aug 30, 2024

In https://openqa.opensuse.org/tests/4444816#step/firstrun/15 the password typing results in incomplete typing string which yields a cracklib error that the string is too short.

Verification runs

@mloviska
Copy link
Contributor Author

@Vogtinator, the password for the user fails as it is too short, but the same one works just fine for root. Is this worth to file a bug that root screen accepts even short passwords?

@Vogtinator
Copy link
Member

@Vogtinator, the password for the user fails as it is too short, but the same one works just fine for root. Is this worth to file a bug that root screen accepts even short passwords?

Currently it's intentional behaviour, similar to YaST, which just shows a warning that can be ignored. When the root pw dialog in jeos-firstboot gets rewritten some time in the future it'll probably do the same.

@Vogtinator
Copy link
Member

In https://openqa.opensuse.org/tests/4444816#step/firstrun/15 the password typing results in incomplete typing string which yields a cracklib error that the string is too short.

Hm, that particular failure is because of the username. I also wonder how ssh can drop input, that shouldn't happen...

@mloviska
Copy link
Contributor Author

mloviska commented Sep 2, 2024

Hm, that particular failure is because of the username. I also wonder how ssh can drop input, that shouldn't happen...

The username needle was matched, I see this error after the community jeos password was set. Which contains only 5 characters

image

@Vogtinator
Copy link
Member

The username needle was matched,

Ok, there's just no screenshot for the fully typed in dialog or the error message.

I see this error after the community jeos password was set. Which contains only 5 characters

IMO it shouldn't ever set "linux" as password, only use it for login if necessary.

@mloviska mloviska force-pushed the fix_ui_user_pass_arm branch from 94f6ce2 to b60e751 Compare September 2, 2024 09:35
@@ -45,7 +45,7 @@ sub run {
# Handle default credentials for ssh login
# On SLE we use an image preprocessed by openQA where the default
# $testapi::password was set
$testapi::password = $default_password;
local $testapi::password = $default_password;
}
# 'root-ssh' console will wait for SUT to be reachable from ssh
select_console('root-ssh');
Copy link
Member

Choose a reason for hiding this comment

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

I guess it should just reset $testapi::password after the select_console?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alright, even the original $testapi::password is changed outside this module.
@ggardet, is it possible that the password somewhere in the testapi code is changed in RPi tests?

Copy link
Collaborator

@ggardet ggardet Sep 2, 2024

Choose a reason for hiding this comment

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

Default password is linux on JeOS images. I guess we can change it in firstboot config interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that password fails the cracklib-check and it seems that the osado's default password in testapi also defaults to linux

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 have done a PoC, https://openqa.opensuse.org/tests/4457380#step/firstrun/14 shows that $testapi::password is set to linux. In the PoC run I have changed it, the UI test object passed. Later the password was restored to linux (default) hence the test fails in another point.
If we keep linux as default, I would rather skip this test object by setting WIZARD_SKIP_USER in community jeos RPi test runs

@pevik
Copy link
Contributor

pevik commented Sep 4, 2024

@mloviska are verifications just outdated or problem not fixed yet? I tried your fork to verify os-autoinst/opensuse-jobgroups#516, but it failed as well.

@mloviska mloviska force-pushed the fix_ui_user_pass_arm branch 2 times, most recently from 73d4023 to 2c2d093 Compare September 5, 2024 11:05
@Vogtinator
Copy link
Member

Vogtinator commented Sep 23, 2024

I had a look. What currently happens is this:

The TW job group hardcodes PASSWORD=linux for all RPi tests. This is set as initial $testapi::password by main_common.pm. When consoles are added by init_consoles, $testapi::password is passed to the various ssh instances and stored there. In prepare_firstboot, the initial $testapi::password is still linux due to the test variable, but it also does $testapi::password = "linux" early on.

My initial attempt was to remove the PASSWORD=linux setting (os-autoinst/opensuse-jobgroups#520) and rely on prepare_firstboot to set the image password, but that doesn't work because the consoles already have the password stored at that point. This could be rectified by doing init_consoles again or overwriting the password parameters, but this results in the messy situation that there can be intermediate states, especially if there is no reboot with console reconnection.

Not sure how this can be addressed the best way. IMO the PASSWORD=linux variable makes sense as it's image specific, but belongs into the medium configuration instead of the job group. Maybe prepare_firstboot should drop $default_password and rely on the variable instead, and set $testapi::password to a stronger one just before jeos-firstboot is started and update the consoles?

@mloviska
Copy link
Contributor Author

Not sure how this can be addressed the best way. IMO the PASSWORD=linux variable makes sense as it's image specific, but belongs into the medium configuration instead of the job group. Maybe prepare_firstboot should drop $default_passwordand rely on the variable instead, and set$testapi::password` to a stronger one just before jeos-firstboot is started and update the consoles?

Or we can simply skip this dialog as it is tested everywhere. Anyway, I am fine with both ways but not sure who should make the call when it comes to community images.

@Vogtinator
Copy link
Member

Not sure how this can be addressed the best way. IMO the PASSWORD=linux variable makes sense as it's image specific, but belongs into the medium configuration instead of the job group. Maybe prepare_firstboot should drop $default_passwordand rely on the variable instead, and set$testapi::password` to a stronger one just before jeos-firstboot is started and update the consoles?

Or we can simply skip this dialog as it is tested everywhere. Anyway, I am fine with both ways but not sure who should make the call when it comes to community images.

That's also an option, but means a separate code path for creation of the $testapi::username user again. It's the easiest option for now though.

@mloviska
Copy link
Contributor Author

That's also an option, but means a separate code path for creation of the $testapi::username user again. It's the easiest option for now though.

The code path is already present. We just need to set WIZARD_SKIP_USER=1 in the medium

@Vogtinator
Copy link
Member

There's also #20212 with a similar issue FTR

@mloviska
Copy link
Contributor Author

There's also #20212 with a similar issue FTR

I would rather blame yaml schedule over here :D

@mloviska
Copy link
Contributor Author

That's also an option, but means a separate code path for creation of the $testapi::username user again. It's the easiest option for now though.

The code path is already present. We just need to set WIZARD_SKIP_USER=1 in the medium

PoC https://openqa.opensuse.org/tests/4522171#details, no code changes

@pevik
Copy link
Contributor

pevik commented Oct 11, 2024

@mloviska
Copy link
Contributor Author

PoC https://openqa.opensuse.org/tests/4522171#details, no code changes

I don't see CASEDIR of your fork in https://openqa.opensuse.org/tests/4522171#settings, https://openqa.opensuse.org/tests/4522171/file/autoinst-log.txt points to d9fcc37.

Why is there a need of casedir when there are no code changes. Does the problem still persists?

@pevik
Copy link
Contributor

pevik commented Oct 11, 2024

PoC https://openqa.opensuse.org/tests/4522171#details, no code changes

I don't see CASEDIR of your fork in https://openqa.opensuse.org/tests/4522171#settings, https://openqa.opensuse.org/tests/4522171/file/autoinst-log.txt points to d9fcc37.

Why is there a need of casedir when there are no code changes. Does the problem still persists?

Ah, I'm sorry, now I understand "no code changes meaning". No, problem don't persist any more (got fixed by WIZARD_SKIP_USER=1 in JeOS-for-RPi flavor in https://openqa.opensuse.org/admin/products as you mentioned).

I wonder if we need this PR for other jobs. If yes, shouldn't be the second commit removed? If not, it should be closed, right?

In https://openqa.opensuse.org/tests/4444816#step/firstrun/15 the
password typing results in incomplete typing string which yields a
cracklib error that the string is too short.

- ticket: https://progress.opensuse.org/issues/165800
@mloviska mloviska force-pushed the fix_ui_user_pass_arm branch from 2c2d093 to c956ecb Compare October 11, 2024 14:00
@mloviska mloviska removed the WIP Work in progress label Oct 11, 2024
@mloviska
Copy link
Contributor Author

I wonder if we need this PR for other jobs. If yes, shouldn't be the second commit removed? If not, it should be closed, right?

I guess it is not necessary to have, but it is Hacktober afterall, every PR counts :D

@mloviska
Copy link
Contributor Author

mloviska commented Oct 11, 2024

VRs:

@mloviska mloviska closed this Dec 4, 2024
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.

4 participants