canonical / ubuntu-pro-client

Ubuntu Pro Client for offerings from Canonical
https://canonical-ubuntu-pro-client.readthedocs-hosted.com/en/latest/
GNU General Public License v3.0
52 stars 73 forks source link

Feature: Extra big warning on known-bad platforms when enabling realtime-kernel or fips #3115

Closed orndorffgrant closed 2 months ago

orndorffgrant commented 4 months ago

Please describe the scenario where the new feature would be useful

You can easily enable the generic realtime-kernel on a machine that needs a kernel with special patches/features. Then a number of bad things could happen. In the worst case, the machine won't even boot.

Generally we want pro offerings to be easily available for experimentation; however, some of the cases where the generic realtime-kernel won't work are already known and can be enumerated. In these cases, we should have a very large warning explaining the risk. The user can still bypass it if they really want to, but it should default to "no".

Describe the solution you'd like

We should store a "denylist" of kernel flavors on the pro backend in the resource affordances metadata for realtime-kernel. The metadata will look something like this:

warnPlatforms:
    - kernelFlavor: mtk
    - kernelFlavor: raspi
    - kernelFlavor: fips

The data is a list of objects, which leaves room for expanding this feature to be more granular in the future.

By keeping this data on the backend, it will allow us to update it in the future at any time without being tied to a future pro-client release.

When the pro-client detects that it is on one of the specified kernel flavors while enabling realtime-kernel, it will display the following warning prompt.

WARNING: YOU ARE CURRENTLY RUNNING THE $kernelFlavor KERNEL WHICH MEANS YOUR PLATFORM IS UNSUPPORTED
THE GENERIC KERNEL IS NOT TESTED ON THIS PLATFORM.
IT MAY BREAK HARDWARE SUPPORT AND MAY PREVENT YOUR MACHINE FROM BOOTING.

Do you accept the risk and wish to continue installing the generic realtime kernel? (y/N): 

All of this can be applied to fips in the same way.

Current behavior

Currently, there is one generic warning message about the always-present risk of switching kernels. This warning should not go away. The new warning will be in addition to the current warning.

Stakeholders

Please check the description and provide feedback on if I got anything wrong. Especially please provide feedback on the initial proposed warning message and also let me know of any known kernel flavors where we should display this warning. @EdoardoBarbieriCanonical @hkhonming @khbecker @lucasmoura

khbecker commented 4 months ago

This looks good. However, for the case of noble raspi, can we suggest instead that they enable the raspi variant of the realtime kernel if they forgot the variant argument?

hkhonming commented 4 months ago

The messaging looks fine to me but I am wondering where we should be doing a "acceptlist" rather than a "denylist".

e.g. I can really provide an exhaustive list today(e.g. just from the kernel dashboard) but it won't handle any future platform. e.g. if I work on a new linux- platform, I will then need to make sure I submit the pull request to update the denylist, as well as allowing time for the pro client to be updated before I push out the linux- .

And programatically, as long as you are no running the generic kernel and then updated to the generic realtime, you must be missing the delta between generic and derivative kernel, where that warning message looks correct for all of the derivative kernel though the impact is certainly vary depending on the deltas of your derivative kernel.

hkhonming commented 4 months ago

Just to provide more information on the test result with various pro features anbox-cloud: snaps only, no impact usg: user spaces & apt source list only, no impact

fips-preview: This result to the SAME issue as realtime-kernel. It will install linux-fips and same problem we encounter here will be applicable. And the worst thing is that the pro cli only say it will install crypto packages but it didn't mention a kernel update.

orndorffgrant commented 3 months ago

Sorry for the long delay here. We decided to first focus on the general problem of auto-selecting a variant and notifying the user of which variant has been selected. This will neatly solves the noble raspi problem mentioned (@khbecker), and in every other case will look like this:

No variant specified. To specify a variant, use the variant option.
Auto-selecting *generic* variant. Proceed? (y/N)

I think that is a small improvement relevant to this issue, because if you are on a system that is not generic and pro enable realtime-kernel tells you it is using the generic variant, you at least will have a clue that it may not be a great fit. It doesn't really resolve this issue though.

Thanks for the additional thoughts and info @hkhonming - I think it's a good point that if we are only issuing a warning (not blocking), then it is okay to show the warning liberally. @khbecker what do you think of showing a warning during enabling generic realtime-kernel when the current kernel is anything other than -generic?

hkhonming commented 3 months ago

@orndorffgrant , I understand your point and the compromise on this small improvement.

But at the same time I still want to suggest to refine the warning and tell the user that they are NOT using "generic" variant. I believe the generic variant is a common sense in Canonical but for a normal users(especially with fewer kernel knowledge), they doesn't really know the kernel is different.

orndorffgrant commented 3 months ago

@hkhonming Yes I completely I agree. I was only explaining the reason for prioritizing that other feature over this issue.

I think we should include a warning like you describe. I would just like @khbecker to confirm that makes sense to him as well.

khbecker commented 3 months ago

@orndorffgrant Yes, that makes sense to me. It is good to improve the warning text so users who aren't on generic will know they may break everything.