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

cli: Add --json support to --check #472

Closed
wants to merge 1 commit into from

Conversation

travier
Copy link
Member

@travier travier commented Apr 16, 2024

This will let higher level tools (Plasma Discover for example) more easily read the output of bootc update --check --json.


Needs ostreedev/ostree-rs-ext#618

This will let higher level tools (Plasma Discover for example) more
easily read the output of `bootc update --check --json`.

Signed-off-by: Timothée Ravier <tim@siosm.fr>
@cgwalters
Copy link
Collaborator

The original idea here is that the result of --check is cached, so you can just run bootc status --json to get that data after invoking bootc upgrade --check. So I don't believe we need this, agreed?

@travier
Copy link
Member Author

travier commented Apr 17, 2024

Thanks for pointing that. Will have to give bootc status --json a try then.

@cgwalters
Copy link
Collaborator

However, right now we don't expose the diff in the status. I think it would likely make sense to do that. There's a bit of a messy thing here as we would need to probably change cached_update to be its own struct.

Also this relates to ostreedev/ostree-rs-ext#618 in that in the end we'd need to create a new ContainerImageDiff structure here anyways (because the API types for this crate need to be abstracted from those of ostree).

@travier
Copy link
Member Author

travier commented Apr 17, 2024

If I understand correctly, looking at ostreedev/ostree-rs-ext#618, all manifest diff operations cache the result in the repo, thus we need this to run as root with a writable /sysroot.

But does bootc offer an unprivileged path right now? (for usage by GNOME Software/Plasma Discover for example)

$ ostree container image pull --check test /sysroot/ostree/repo ostree-image-signed:registry:quay.io/travier/fedora-kinoite:latest
error: Preparing import: Fetching manifest: Writing cached pending manifest: Unable to write detached metadata: open(O_TMPFILE): Read-only file system

@cgwalters
Copy link
Collaborator

But does bootc offer an unprivileged path right now?

No. Historically the polkit/unprivileged stuff in both rpm-ostree and things like PackageKit are big, complex beasts that add a lot of security-sensitive code paths. My thinking here is that for now such a thing is probably best as a separate project on top of bootc. Or maybe we could add it as an optionally-installed secondary component here.

One thing I would say on this is that I also think many of the same concerns should apply to "workload" containers such as podman; ideally we share code there. Access to the podman (and docker) remote API is equivalent to root in practice right now, note. It may make sense to have a single component which can initiate upgrades for bootc as well as wrap e.g. https://docs.podman.io/en/latest/markdown/podman-auto-update.1.html (i.e. no ability to change system state, just do updates)

@travier
Copy link
Member Author

travier commented Apr 17, 2024

Creating a small daemon with a very limited, unprivileged DBus API would make sense. For GNOME Software/Plasma Discover, we mostly need "check for update" & "update" commands. And maybe something for "rebase/switch", but it would be hard to make this one unprivileged.

@travier
Copy link
Member Author

travier commented Apr 17, 2024

Might be needed to coordinate with the dnf team here. They already have a DBus daemon that does (a larger super set of) that.

Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

However, right now we don't expose the diff in the status.

This is what we should do.

@cgwalters
Copy link
Collaborator

I believe #472 (comment) covers this, please reopen if anyone disagrees.

@cgwalters
Copy link
Collaborator

#932 tracks the diff

@travier travier deleted the main-update-check-json branch December 16, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants