Generalize REST/client API for multiple users #1989

martinpitt · 2025-02-03T11:24:23Z

Preparation and heavy lifting for https://issues.redhat.com/browse/COCKPIT-1086 . But this PR does not yet change visible behaviour or pixels.

test: Check podman socket going away while cockpit session is running #1988

Replacing an array variable with its first entry rightfully gives us the type check finger wagging.

Change `connnect()` and `call()` to accept a `uid` parameter instead of an address/superuser pair. Compute the Unix address within rest.js, move `getAddress()` from client for that. For now, just accept `0` for root and `null` for "current cockpit user", until we actually use that. Drop the `system` argument from `debug()`, and let the caller specify an appropriate string. We were only calling `debug()` from rest.js so far anyway, so that isn't very intrusive. Keep `isSystem` for the client.js API for the time being. That will be the next step, but let's not change everything at once.

martinpitt · 2025-02-06T09:38:08Z

Yay -- that was a hard piece of work, so let's record d00421a as a milestone where everything is green. I'll commit fixups from now on. There is still some things which I don't like, such as the repeated uid ?? "user" pattern; and the higher-level states still use isSystem a lot.

Similarly to the previous commit for rest.js, change the API to accept an user id instead of an `isSystem` boolean. Change the Container, Pod, and Image properties accordingly. However, keep the UI logic and higher-level `isSystem` flags/helpers as-is for now, as this is intrusive enough. Clean up the often repeated "id + isSystem" index key calculation for the app's containers/images/pods states, and centralize it into a `makeKey()` helper. Put the uid in front, so that they have more natural sorting (in the UI we sort by owner first), and add a `-` in between so that they are easier to read.

cockpituous · 2025-02-06T14:11:59Z

src/Containers.jsx

@@ -607,7 +607,7 @@ class Containers extends React.Component {
                const lcf = this.props.textFilter.toLowerCase();
                filtered = filtered.filter(id => this.props.containers[id].Name.toLowerCase().indexOf(lcf) >= 0 ||
                    (this.props.containers[id].Pod &&
-                     this.props.pods[this.props.containers[id].Pod + this.props.containers[id].isSystem.toString()].Name.toLowerCase().indexOf(lcf) >= 0) ||
+                     this.props.pods[utils.makeKey(this.props.containers[id].uid, this.props.containers[id].Pod)].Name.toLowerCase().indexOf(lcf) >= 0) ||


This added line is not executed by any test.

cockpituous · 2025-02-06T14:11:59Z

src/Containers.jsx

@@ -641,7 +641,7 @@ class Containers extends React.Component {
            filtered.forEach(id => {
                const container = this.props.containers[id];
                if (container)
-                    (partitionedContainers[container.Pod ? (container.Pod + container.isSystem.toString()) : 'no-pod'] || []).push(container);
+                    (partitionedContainers[container.Pod ? utils.makeKey(container.uid, container.Pod) : 'no-pod'] || []).push(container);


This added line is not executed by any test.

cockpituous · 2025-02-06T14:11:59Z

src/Containers.jsx

-                         ((this.props.ownerFilter === "system" && !pod.isSystem) ||
-                            (this.props.ownerFilter !== "system" && pod.isSystem))))
+                         ((this.props.ownerFilter === "system" && pod.uid !== 0) ||
+                            (this.props.ownerFilter !== "system" && pod.uid === 0))))


This added line is not executed by any test.

cockpituous · 2025-02-06T14:11:59Z

src/Containers.jsx

-                                        if (this.props.pods[a].isSystem !== this.props.pods[b].isSystem)
-                                            return this.props.pods[a].isSystem ? 1 : -1;
+                                        if (this.props.pods[a].uid !== this.props.pods[b].uid)
+                                            return this.props.pods[a].uid === 0 ? 1 : -1;


This added line is not executed by any test.

cockpituous · 2025-02-06T14:11:59Z

src/ImageRunModal.jsx

@@ -667,7 +668,7 @@ export class ImageRunModal extends React.Component {

    async validateContainerName(containerName) {
        try {
-            await client.containerExists(this.isSystem(), containerName);
+            await client.containerExists(this.isSystem() ? 0 : null, containerName);


This added line is not executed by any test.

cockpituous · 2025-02-06T14:12:00Z

src/app.jsx

                    }
                })
                .catch(ex => {
-                    console.warn("Failed to do Update Pod:", JSON.stringify(ex));
+                    console.warn("Failed to do updatePod for uid", uid, ":", JSON.stringify(ex));


This added line is not executed by any test.

cockpituous · 2025-02-06T14:12:00Z

src/rest.js

+        console.warn("$XDG_RUNTIME_DIR is not present. Cannot use user service.");
+        return { path: "", superuser: null };


These 2 added lines are not executed by any test.

cockpituous · 2025-02-06T14:12:00Z

src/rest.js

+    if (uid === 0)
+        return { path: PODMAN_SYSTEM_ADDRESS, superuser: "require" };
+
+    throw new Error(`getAddress: uid ${uid} not supported`);


This added line is not executed by any test.

cockpituous · 2025-02-06T14:12:00Z

src/rest.js

    const connection = {};
    const decoder = new TextDecoder();
+    const user_str = (uid === null) ? "user" : (uid === 0) ? "root" : `uid ${uid}`;


This added line is not executed by any test.

cockpituous · 2025-02-06T14:12:00Z

src/util.js

    if (window.debugging === "all" || window.debugging?.includes("podman"))
-        console.debug("podman", system ? "system" : "user", ...args);
+        console.debug("podman", ...args);


This added line is not executed by any test.

martinpitt force-pushed the generalize-isSystem branch 5 times, most recently from 96e6c60 to 89c7bd8 Compare February 5, 2025 09:56

This comment was marked as resolved.

Sign in to view

martinpitt force-pushed the generalize-isSystem branch from 89c7bd8 to d00421a Compare February 6, 2025 08:33

martinpitt added 2 commits February 6, 2025 10:36

Clean up variable aliasing in updatePod

3d7adfd

Replacing an array variable with its first entry rightfully gives us the type check finger wagging.

martinpitt force-pushed the generalize-isSystem branch 3 times, most recently from 277d09e to d1f4652 Compare February 6, 2025 13:20

martinpitt force-pushed the generalize-isSystem branch from d1f4652 to bac14d1 Compare February 6, 2025 13:59

cockpituous reviewed Feb 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generalize REST/client API for multiple users #1989

Generalize REST/client API for multiple users #1989

martinpitt commented Feb 3, 2025 •

edited

Loading

This comment was marked as resolved.

martinpitt commented Feb 6, 2025

cockpituous Feb 6, 2025

cockpituous Feb 6, 2025

cockpituous Feb 6, 2025

cockpituous Feb 6, 2025

cockpituous Feb 6, 2025

cockpituous Feb 6, 2025

cockpituous Feb 6, 2025

cockpituous Feb 6, 2025

cockpituous Feb 6, 2025

cockpituous Feb 6, 2025

		console.warn("$XDG_RUNTIME_DIR is not present. Cannot use user service.");
		return { path: "", superuser: null };

Generalize REST/client API for multiple users #1989

Are you sure you want to change the base?

Generalize REST/client API for multiple users #1989

Conversation

martinpitt commented Feb 3, 2025 • edited Loading

This comment was marked as resolved.

martinpitt commented Feb 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpitt commented Feb 3, 2025 •

edited

Loading