Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cockpit/cockpit-image-builder.spec
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ BuildRequires: nodejs
Requires: cockpit
Requires: cockpit-files
Requires: osbuild-composer >= 131
Requires: image-builder >= 33

%description
The image-builder-frontend generates custom images suitable for
Expand Down
1 change: 1 addition & 0 deletions fec.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ module.exports = {
// to false
cockpit: false,
'cockpit/fsinfo': false,
'long-running-process': false,
'os-release': false,
},
},
Expand Down
17 changes: 16 additions & 1 deletion playwright/test.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { readFileSync } from 'node:fs';
import { readFileSync, writeFileSync } from 'node:fs';
import os from 'node:os';
import path from 'node:path';

import TOML from '@ltd/j-toml';
import { expect, test } from '@playwright/test';
Expand All @@ -8,6 +10,12 @@ import { closePopupsIfExist, isHosted } from './helpers/helpers';
import { ensureAuthenticated } from './helpers/login';
import { ibFrame, navigateToLandingPage } from './helpers/navHelpers';

const awsCredentials = `
[default]
aws_access_key_id = supersecret
aws_secret_access_key = secretsquirrel
`;

test.describe.serial('test', () => {
const blueprintName = uuidv4();
test('create blueprint', async ({ page }) => {
Expand Down Expand Up @@ -266,6 +274,13 @@ test.describe.serial('test', () => {
return;
}

// this needs to be set so ibcli will list aws targets
// as an available image type
writeFileSync(
path.join(os.homedir(), '.aws', 'credentials'),
awsCredentials,
);

await ensureAuthenticated(page);
await closePopupsIfExist(page);
// Navigate to IB landing page and get the frame
Expand Down
14 changes: 14 additions & 0 deletions schutzbot/Containerfile-Playwright
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
FROM ubuntu:latest AS builder
RUN apt update
RUN apt install -y git golang libgpgme-dev libbtrfs-dev libdevmapper-dev podman
RUN git clone https://github.com/osbuild/image-builder-cli.git ibcli
WORKDIR ibcli
RUN ls -al
RUN go mod tidy && go mod vendor
RUN go build -o /opt ./cmd/image-builder/

FROM mcr.microsoft.com/playwright:v1.51.1-noble
RUN apt update
# Needed for running image-builder
RUN apt install -y libgpgme-dev libbtrfs-dev libdevmapper-dev
COPY --from=builder /opt /usr/bin
4 changes: 3 additions & 1 deletion schutzbot/playwright_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ sudo useradd admin -p "$(openssl passwd foobar)"
sudo usermod -aG wheel admin
echo "admin ALL=(ALL:ALL) NOPASSWD: ALL" | sudo tee "/etc/sudoers.d/admin-nopasswd"

sudo podman build --tag playwright -f $(pwd)/schutzbot/Containerfile-Playwright .

function upload_artifacts {
if [ -n "${TMT_TEST_DATA:-}" ]; then
mv playwright-report "$TMT_TEST_DATA"/playwright-report
Expand Down Expand Up @@ -88,5 +90,5 @@ sudo podman run \
--privileged \
--rm \
--init \
mcr.microsoft.com/playwright:v1.51.1-noble \
localhost/playwright \
/bin/sh -c "cd tests && npx -y [email protected] test --workers=${PW_WORKERS}"
19 changes: 13 additions & 6 deletions src/AppCockpit.tsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,35 @@
import '@patternfly/react-core/dist/styles/base.css';
import '@patternfly/patternfly/patternfly-addons.css';

import React from 'react';
import React, { useEffect, useState } from 'react';

import 'cockpit-dark-theme';
import { Page, PageSection } from '@patternfly/react-core';
import { Spinner } from '@redhat-cloud-services/frontend-components';
import NotificationsProvider from '@redhat-cloud-services/frontend-components-notifications/NotificationsProvider';
import cockpit from 'cockpit';
import { createRoot } from 'react-dom/client';
import { Provider } from 'react-redux';
import { HashRouter } from 'react-router-dom';

import './AppCockpit.scss';
import { NotReady, RequireAdmin } from './Components/Cockpit';
import { RequireAdmin } from './Components/Cockpit';
import { Router } from './Router';
import { onPremStore as store } from './store';
import { useGetComposerSocketStatus } from './Utilities/useComposerStatus';
import { useIsCockpitAdmin } from './Utilities/useIsCockpitAdmin';

const Application = () => {
const { enabled, started } = useGetComposerSocketStatus();
const isAdmin = useIsCockpitAdmin();
const [ready, setReady] = useState(false);

if (!started || !enabled) {
return <NotReady enabled={enabled} />;
useEffect(() => {
if (cockpit) {
setReady(true);
}
}, [cockpit, ready, setReady]);

if (!ready) {
return <Spinner centered />;
}

if (!isAdmin) {
Expand Down
3 changes: 3 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export const CENTOS_9 = 'centos-9';
export const CENTOS_10 = 'centos-10';
export const FEDORA_41 = 'fedora-41';
export const FEDORA_42 = 'fedora-42';
export const FEDORA_43 = 'fedora-43';
export const X86_64 = 'x86_64';
export const AARCH64 = 'aarch64';

Expand Down Expand Up @@ -97,6 +98,7 @@ export const ON_PREM_RELEASES = new Map([
[CENTOS_10, 'CentOS Stream 10'],
[FEDORA_41, 'Fedora Linux 41'],
[FEDORA_42, 'Fedora Linux 42'],
[FEDORA_43, 'Fedora Linux 43'],
[RHEL_10, 'Red Hat Enterprise Linux (RHEL) 10'],
]);

Expand Down Expand Up @@ -310,3 +312,4 @@ export const PAGINATION_COUNT = 0;
export const SEARCH_INPUT = '';

export const BLUEPRINTS_DIR = '.cache/cockpit-image-builder/';
export const ARTIFACTS_DIR = '/var/lib/osbuild-composer/artifacts';
121 changes: 85 additions & 36 deletions src/store/cockpit/cockpitApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import path from 'path';
import TOML, { Section } from '@ltd/j-toml';
import cockpit from 'cockpit';
import { fsinfo } from 'cockpit/fsinfo';
import { LongRunningProcess } from 'long-running-process';
import { v4 as uuidv4 } from 'uuid';

// We have to work around RTK query here, since it doesn't like splitting
Expand All @@ -19,17 +20,21 @@ import { v4 as uuidv4 } from 'uuid';
// bit so that the `cockpitApi` doesn't become a monolith.
import { contentSourcesApi } from './contentSourcesApi';
import {
ComposeStatus,
composeStatus,
datastreamDistroLookup,
getBlueprintsPath,
getCloudConfigs,
mapToOnpremRequest,
paginate,
readComposes,
updateComposeStatus,
} from './helpers';
import type {
CockpitCreateBlueprintApiArg,
CockpitCreateBlueprintRequest,
CockpitUpdateBlueprintApiArg,
GetCockpitComposeStatusApiResponse,
ImageStatus,
UpdateWorkerConfigApiArg,
WorkerConfigFile,
WorkerConfigResponse,
Expand All @@ -39,6 +44,7 @@ import {
mapHostedToOnPrem,
mapOnPremToHosted,
} from '../../Components/Blueprints/helpers/onPremToHostedBlueprintMapper';
import { ARTIFACTS_DIR } from '../../constants';
import {
BlueprintItem,
ComposeBlueprintApiArg,
Expand All @@ -61,7 +67,6 @@ import {
GetComposesApiArg,
GetComposesApiResponse,
GetComposeStatusApiArg,
GetComposeStatusApiResponse,
GetOscapCustomizationsApiArg,
GetOscapCustomizationsApiResponse,
GetOscapProfilesApiArg,
Expand Down Expand Up @@ -359,7 +364,7 @@ export const cockpitApi = contentSourcesApi.injectEndpoints({
ComposeBlueprintApiResponse,
ComposeBlueprintApiArg
>({
queryFn: async ({ id: filename }, _, __, baseQuery) => {
queryFn: async ({ id: filename }) => {
try {
const blueprintsDir = await getBlueprintsPath();
const file = cockpit.file(
Expand All @@ -386,29 +391,63 @@ export const cockpitApi = contentSourcesApi.injectEndpoints({
image_requests: [ir],
};

const composeResp = await baseQuery({
url: '/compose',
method: 'POST',
body: JSON.stringify(
// since this is the request that gets sent to the cloudapi
// backend, we need to modify it slightly
mapToOnpremRequest(
const uuid = uuidv4();
const composeDir = path.join(blueprintsDir, filename, uuid);
await cockpit.spawn(['mkdir', '-p', composeDir], {});

const ibBpPath = path.join(composeDir, 'bp.json');
await cockpit
.file(ibBpPath)
.replace(
Copy link
Member

Choose a reason for hiding this comment

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

Oh why rewrite the blueprint here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need it so we can pass it to ibcli we need to save the on-prem version of the blueprint not the hosted version.
So we need to keep track of both.

JSON.stringify(
mapHostedToOnPrem(blueprint as CreateBlueprintRequest),
crcComposeRequest.distribution,
[ir],
null,
2,
),
null,
2,
),
headers: {
'content-type': 'application/json',
},
});
);

// save the blueprint request early, since any errors
// in this function cause pretty big headaches with
// the images table
await cockpit
.file(path.join(blueprintsDir, filename, composeResp.data?.id))
.file(path.join(composeDir, 'request.json'))
.replace(JSON.stringify(crcComposeRequest, null, 2));
composes.push({ id: composeResp.data?.id });

const user = await cockpit.user();
const cmd = [
// the image build fails if we don't set
// this for some reason
`HOME=${user.home}`,
Copy link
Member

Choose a reason for hiding this comment

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

Might be because the systemd process is running as root? Not sure how we could drop it though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I put a note there and created a feature request for cockpit, basically the env variables aren't being set for the systemd service.

The solution is to either import the long-running-process script and modify it or add the feature upstream.

'/usr/bin/image-builder',
'build',
'--with-buildlog',
'--blueprint',
ibBpPath,
'--output-dir',
path.join(ARTIFACTS_DIR, uuid),
'--output-name',
uuid,
'--distro',
crcComposeRequest.distribution,
ir.image_type,
];

const process = new LongRunningProcess(
`cockpit-image-builder-${uuid}.service`,
updateComposeStatus(composeDir),
);

// this is a workaround because the process
// can't be started when in `init` state
process.state = 'stopped';

process.run(['bash', '-ec', cmd.join(' ')]).catch(async () => {
Copy link

Choose a reason for hiding this comment

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

it seems cockpit works with arrays of strings, so would process.run(cmd) work? The only other change required would be HOME= needs to move into the "options" of process.run(), something like progress.run(cmd, environ={"HOME": ${user.home}}) (assuming python syntax here, I have no clue about typescript). As long as we control all inputs its problably fine as is but once there is user controlled input this turns into a command injection quickly and it seems we can just start with using the array of strings which makes command injections (or files with spaces) a non-issue.

It seems this could then also use the same trick that https://github.com/cockpit-project/cockpit/blob/main/examples/long-running-process/index.js#L20 is using, i.e. just monitor the jounal to get the output of ibcli, with that we could add image-builder build --progress=json (we currently only have verbose,term,debug there) and then you could render your own progress this way. When we design the json we need to make sure that its self-contained, i.e. that we must assume that at any point a client could connect or reconect and that the client will not have state. This is not a property of the underlying osbuild json stream which will often use "context" which needs to be captured and remembered by the consumer of the json (but no worries, just spelling it out here, doing this will not be hard especially if we start simple).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ya I didn't like that either and adding HOME= is not ideal. But I ran into two issues here:

  1. the list of strings didn't work as expected and it was really hard to debug why
  2. adding the HOME as an env variable in the options also never worked and got ignored.

But those are both valid points and I will dig into it deeper and try figure out why that wasn't working.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With the json progress that seems nice and we could probably implement that in the getComposeStatus endpoint

Copy link

Choose a reason for hiding this comment

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

@kingsleyzissou before you spend/waste too much time in debugging this please leave and maybe we can have a joint session where you can help me to reproduce so that I can dig into it a bit (if you want).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh sorry I actually missed this comment. I came back to it today in preparation for Friday's call.

I filed a feature request in cockpit:
cockpit-project/cockpit#22454

await cockpit
Copy link
Member

Choose a reason for hiding this comment

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

Could we get image-builder-cli to write some sort of results file? Like I wonder if we can infer the results from the build log, saves us from having to write the result file + in that case I think we can safely close the tab and still get a proper compose status.

Or could we attach updateComposeStatus against all running systemd services on load?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could we get image-builder-cli to write some sort of results file

Yeah that's what we're doing, there is a callback that is handling this too and should be attached to the systemd service afaict. I'll have to double check this.

.file(path.join(composeDir, 'result'))
.replace(ComposeStatus.FAILURE);
});

composes.push({ id: uuid });
}

return {
Expand Down Expand Up @@ -452,37 +491,47 @@ export const cockpitApi = contentSourcesApi.injectEndpoints({
},
}),
getComposeStatus: builder.query<
GetComposeStatusApiResponse,
GetCockpitComposeStatusApiResponse,
GetComposeStatusApiArg
>({
queryFn: async (queryArg, _, __, baseQuery) => {
queryFn: async (queryArg) => {
try {
const resp = await baseQuery({
url: `/composes/${queryArg.composeId}`,
method: 'GET',
});
const blueprintsDir = await getBlueprintsPath();
const info = await fsinfo(blueprintsDir, ['entries'], {
superuser: 'try',
});
const entries = Object.entries(info?.entries || {});
for (const bpEntry of entries) {
for await (const bpEntry of entries) {
const bpComposes = await readComposes(bpEntry[0]);
if (!bpComposes.some((c) => c.id === queryArg.composeId)) {
continue;
}

const request = await cockpit
.file(path.join(blueprintsDir, bpEntry[0], queryArg.composeId))
.file(
path.join(
blueprintsDir,
bpEntry[0],
queryArg.composeId,
'request.json',
),
)
.read();

const status = await composeStatus(
queryArg.composeId,
path.join(blueprintsDir, bpEntry[0], queryArg.composeId),
);

return {
data: {
image_status: resp.data?.image_status,
image_status: status as ImageStatus,
request: JSON.parse(request),
},
};
}
return {
data: {
image_status: '',
request: {},
},
};

throw new Error('Compose not found');
} catch (error) {
return { error };
}
Expand Down
Loading