fix: address release v2 reviews
This commit is contained in:
1 parent
4905ea6738
commit
32732680e5
55 files changed
+925
-294
No files matched your search
@@ -27,4 +27,37 @@ describe("getTrackedFeatureProperties", () => {
|
||||
userId: "user-1",
|
||||
});
|
||||
});
|
||||
|
||||
test("discards setup values outside the closed telemetry contract", () => {
|
||||
expect(
|
||||
getTrackedFeatureProperties(
|
||||
"setup:widget-completed",
|
||||
{
|
||||
entryPoint: "https://private.example",
|
||||
intent: "secret-id",
|
||||
outcome: "credential",
|
||||
elapsedMs: Number.POSITIVE_INFINITY,
|
||||
hasBoardContext: "true",
|
||||
canResolveInline: 1,
|
||||
},
|
||||
"session-user",
|
||||
),
|
||||
).toEqual({});
|
||||
});
|
||||
|
||||
test("keeps known widget intents and bounded elapsed times", () => {
|
||||
expect(
|
||||
getTrackedFeatureProperties(
|
||||
"setup:widget-completed",
|
||||
{ entryPoint: "board", intent: "weather", outcome: "completed", elapsedMs: 0, canResolveInline: false },
|
||||
"session-user",
|
||||
),
|
||||
).toEqual({ entryPoint: "board", intent: "weather", outcome: "completed", elapsedMs: 0, canResolveInline: false });
|
||||
});
|
||||
|
||||
test("keeps the app intent used by Universal Create", () => {
|
||||
expect(
|
||||
getTrackedFeatureProperties("setup:intent-selected", { entryPoint: "header", intent: "app" }, "user-1"),
|
||||
).toEqual({ entryPoint: "header", intent: "app" });
|
||||
});
|
||||
});
|
||||
@@ -1,11 +1,30 @@
|
||||
const setupPropertyKeys = new Set([
|
||||
"entryPoint",
|
||||
"intent",
|
||||
"outcome",
|
||||
"elapsedMs",
|
||||
"hasBoardContext",
|
||||
"canResolveInline",
|
||||
import { widgetKinds } from "@homarr/definitions";
|
||||
|
||||
const setupEntryPoints = new Set(["header", "spotlight", "board", "assistant", "docker", "management"]);
|
||||
const setupIntents = new Set([
|
||||
...widgetKinds,
|
||||
"widget",
|
||||
"app",
|
||||
"integration",
|
||||
"container",
|
||||
"board",
|
||||
"workshop",
|
||||
"customWidget",
|
||||
"add-compatible-widget",
|
||||
]);
|
||||
const setupOutcomes = new Set(["completed", "blocked", "continued"]);
|
||||
const maximumSetupElapsedMs = 24 * 60 * 60 * 1_000;
|
||||
|
||||
const isValidSetupProperty = (key: string, value: unknown) => {
|
||||
if (key === "entryPoint") return typeof value === "string" && setupEntryPoints.has(value);
|
||||
if (key === "intent") return typeof value === "string" && setupIntents.has(value);
|
||||
if (key === "outcome") return typeof value === "string" && setupOutcomes.has(value);
|
||||
if (key === "elapsedMs") {
|
||||
return typeof value === "number" && Number.isFinite(value) && value >= 0 && value <= maximumSetupElapsedMs;
|
||||
}
|
||||
if (key === "hasBoardContext" || key === "canResolveInline") return typeof value === "boolean";
|
||||
return false;
|
||||
};
|
||||
|
||||
export const getTrackedFeatureProperties = (
|
||||
feature: string,
|
||||
@@ -15,8 +34,6 @@ export const getTrackedFeatureProperties = (
|
||||
if (!feature.startsWith("setup:")) return { ...properties, userId };
|
||||
|
||||
return Object.fromEntries(
|
||||
Object.entries(properties ?? {}).filter(
|
||||
([key, value]) => setupPropertyKeys.has(key) && ["string", "number", "boolean"].includes(typeof value),
|
||||
),
|
||||
Object.entries(properties ?? {}).filter(([key, value]) => isValidSetupProperty(key, value)),
|
||||
);
|
||||
};
|
||||
@@ -562,7 +562,7 @@ export const boardRouter = createTRPCRouter({
|
||||
mcp: {
|
||||
enabled: true,
|
||||
description:
|
||||
"Create a new board with a name, column count (1-24), and isPublic flag. Returns { boardId }. Requires board-create permission",
|
||||
"Create a new board with a name, column count (1-24), and isPublic flag. Returns { boardId, name, layoutId }. Requires board-create permission",
|
||||
},
|
||||
})
|
||||
.input(boardCreateSchema)
|
||||
|
||||
@@ -26,6 +26,11 @@ export const getDockerReconciliationAsync = async (db: Database) => {
|
||||
}),
|
||||
db.query.apps.findMany(),
|
||||
]);
|
||||
const normalizedIntegrations = integrations.map((integration) => ({
|
||||
integration,
|
||||
normalizedUrl: normalizeDockerServiceUrl(integration.url),
|
||||
}));
|
||||
const normalizedApps = apps.map((app) => ({ app, normalizedUrl: normalizeDockerServiceUrl(app.href) }));
|
||||
const containerNameCounts = new Map<string, number>();
|
||||
for (const container of data.containers) {
|
||||
const name = normalizeName(container.name);
|
||||
@@ -44,13 +49,16 @@ export const getDockerReconciliationAsync = async (db: Database) => {
|
||||
urlCandidates.map(({ url }) => normalizeDockerServiceUrl(url)).filter((url) => url !== null),
|
||||
);
|
||||
const uniqueName = containerNameCounts.get(normalizeName(container.name)) === 1;
|
||||
const sameKindIntegrations = match ? integrations.filter(({ kind }) => kind === match.kind) : [];
|
||||
const exactIntegrationMatches = sameKindIntegrations.filter(({ url }) => {
|
||||
const normalizedUrl = normalizeDockerServiceUrl(url);
|
||||
return normalizedUrl ? candidateUrls.has(normalizedUrl) : false;
|
||||
});
|
||||
const sameKindIntegrations = match
|
||||
? normalizedIntegrations.filter(({ integration }) => integration.kind === match.kind)
|
||||
: [];
|
||||
const exactIntegrationMatches = sameKindIntegrations
|
||||
.filter(({ normalizedUrl }) => normalizedUrl !== null && candidateUrls.has(normalizedUrl))
|
||||
.map(({ integration }) => integration);
|
||||
const nameIntegrationMatches = uniqueName
|
||||
? sameKindIntegrations.filter(({ name }) => normalizeName(name) === normalizeName(container.name))
|
||||
? sameKindIntegrations
|
||||
.filter(({ integration }) => normalizeName(integration.name) === normalizeName(container.name))
|
||||
.map(({ integration }) => integration)
|
||||
: [];
|
||||
const exactIntegration = exactIntegrationMatches.length === 1 ? exactIntegrationMatches[0] : undefined;
|
||||
const nameIntegration =
|
||||
@@ -58,10 +66,9 @@ export const getDockerReconciliationAsync = async (db: Database) => {
|
||||
? nameIntegrationMatches[0]
|
||||
: undefined;
|
||||
const integration = exactIntegration ?? nameIntegration;
|
||||
const exactAppMatches = apps.filter(({ href }) => {
|
||||
const normalizedUrl = normalizeDockerServiceUrl(href);
|
||||
return normalizedUrl ? candidateUrls.has(normalizedUrl) : false;
|
||||
});
|
||||
const exactAppMatches = normalizedApps
|
||||
.filter(({ normalizedUrl }) => normalizedUrl !== null && candidateUrls.has(normalizedUrl))
|
||||
.map(({ app }) => app);
|
||||
const nameAppMatches = uniqueName
|
||||
? apps.filter(({ name }) => normalizeName(name) === normalizeName(container.name))
|
||||
: [];
|
||||
|
||||
@@ -19,18 +19,40 @@ const dockerContainerTargetSchema = z.object({
|
||||
endpointId: z.string().min(1),
|
||||
id: z.string().min(1),
|
||||
});
|
||||
const dockerContainerTargetsSchema = z.array(dockerContainerTargetSchema).min(1).max(100);
|
||||
|
||||
export const dockerRouter = createTRPCRouter({
|
||||
reconcileServices: permissionRequiredProcedure
|
||||
.requiresPermission("admin")
|
||||
.meta({
|
||||
mcp: {
|
||||
enabled: true,
|
||||
description:
|
||||
"Reconcile Docker container inventory with Homarr integrations and apps, including URL matches and recommended setup or repair actions. Requires admin permission.",
|
||||
},
|
||||
})
|
||||
.concat(dockerMiddleware())
|
||||
.query(async ({ ctx }) => await getDockerReconciliationAsync(ctx.db)),
|
||||
getServiceHealth: permissionRequiredProcedure
|
||||
.requiresPermission("admin")
|
||||
.meta({
|
||||
mcp: {
|
||||
enabled: true,
|
||||
description:
|
||||
"Get Docker endpoint and discovered-service health projected across Docker, integration, app, and widget setup layers. Requires admin permission.",
|
||||
},
|
||||
})
|
||||
.concat(dockerMiddleware())
|
||||
.query(async ({ ctx }) => await getDockerServiceHealthAsync(ctx.db)),
|
||||
refreshInventory: permissionRequiredProcedure
|
||||
.requiresPermission("admin")
|
||||
.meta({
|
||||
mcp: {
|
||||
enabled: true,
|
||||
description:
|
||||
"Invalidate the cached Docker inventory so the next Docker query performs a fresh user-requested discovery. Requires admin permission.",
|
||||
},
|
||||
})
|
||||
.concat(dockerMiddleware())
|
||||
.mutation(() => {
|
||||
dockerContainersRequestHandler.invalidateCache();
|
||||
@@ -65,7 +87,7 @@ export const dockerRouter = createTRPCRouter({
|
||||
},
|
||||
})
|
||||
.concat(dockerMiddleware())
|
||||
.input(z.object({ targets: z.array(dockerContainerTargetSchema) }))
|
||||
.input(z.object({ targets: dockerContainerTargetsSchema }))
|
||||
.mutation(async ({ input }) => {
|
||||
return await performDockerContainerActionsAsync(input.targets, "start");
|
||||
}),
|
||||
@@ -78,7 +100,7 @@ export const dockerRouter = createTRPCRouter({
|
||||
},
|
||||
})
|
||||
.concat(dockerMiddleware())
|
||||
.input(z.object({ targets: z.array(dockerContainerTargetSchema) }))
|
||||
.input(z.object({ targets: dockerContainerTargetsSchema }))
|
||||
.mutation(async ({ input }) => {
|
||||
return await performDockerContainerActionsAsync(input.targets, "stop");
|
||||
}),
|
||||
@@ -91,7 +113,7 @@ export const dockerRouter = createTRPCRouter({
|
||||
},
|
||||
})
|
||||
.concat(dockerMiddleware())
|
||||
.input(z.object({ targets: z.array(dockerContainerTargetSchema) }))
|
||||
.input(z.object({ targets: dockerContainerTargetsSchema }))
|
||||
.mutation(async ({ input }) => {
|
||||
return await performDockerContainerActionsAsync(input.targets, "restart");
|
||||
}),
|
||||
@@ -105,7 +127,7 @@ export const dockerRouter = createTRPCRouter({
|
||||
},
|
||||
})
|
||||
.concat(dockerMiddleware())
|
||||
.input(z.object({ targets: z.array(dockerContainerTargetSchema) }))
|
||||
.input(z.object({ targets: dockerContainerTargetsSchema }))
|
||||
.mutation(async ({ input }) => {
|
||||
return await performDockerContainerActionsAsync(input.targets, "remove");
|
||||
}),
|
||||
|
||||
@@ -280,7 +280,7 @@ export const integrationRouter = createTRPCRouter({
|
||||
mcp: {
|
||||
enabled: true,
|
||||
description:
|
||||
"Create a new integration (connection to an external service). REQUIRED fields: name, url (http/https), kind, secrets, attemptSearchEngineCreation. The 'secrets' field is REQUIRED and must be a non-empty array — call integration_getKinds first to see which secret kinds each integration type needs. Example for Radarr: secrets=[{kind:'apiKey', value:'your-radarr-api-key'}]. Example for Proxmox: secrets=[{kind:'tokenId', value:'...'}, {kind:'personalAccessToken', value:'...'}, {kind:'realm', value:'pam'}]. The connection is tested before saving — if secrets are wrong, an error is returned. Set attemptSearchEngineCreation to false unless explicitly requested. The 'app' field is optional — pass {id:'...'} to link to an existing app, or omit it.",
|
||||
"Create a new integration (connection to an external service). REQUIRED fields: name, url (http/https), kind, secrets, attemptSearchEngineCreation. The 'secrets' field is REQUIRED and must be a non-empty array — call integration_getKinds first to see which secret kinds each integration type needs. Example for Radarr: secrets=[{kind:'apiKey', value:'your-radarr-api-key'}]. Example for Proxmox: secrets=[{kind:'tokenId', value:'...'}, {kind:'personalAccessToken', value:'...'}, {kind:'realm', value:'pam'}]. The connection is tested before saving — if secrets are wrong, an error is returned. Set attemptSearchEngineCreation to false unless explicitly requested. The 'app' field is optional — pass {id:'...'} to link to an existing app, or omit it. Returns integration details and appId, which is null when no app is linked or created.",
|
||||
},
|
||||
})
|
||||
.input(integrationCreateSchema)
|
||||
|
||||
@@ -99,6 +99,30 @@ describe("KubernetesClientRegistry", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("aborts a timed-out node metrics request", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const client = createRegistry().getClient("home");
|
||||
let observedSignal: AbortSignal | undefined;
|
||||
vi.spyOn(client.metricsProbeApi, "listClusterCustomObject").mockImplementation(async (_request, options) => {
|
||||
const middleware = (
|
||||
options as { middleware?: { pre?: (request: unknown) => { toPromise: () => Promise<unknown> } }[] }
|
||||
).middleware?.[0];
|
||||
await middleware?.pre?.({ setSignal: (signal: AbortSignal) => (observedSignal = signal) }).toPromise();
|
||||
return await new Promise<never>(() => undefined);
|
||||
});
|
||||
|
||||
const resultPromise = client.getNodeMetricsAsync(50);
|
||||
const rejection = expect(resultPromise).rejects.toThrow("Kubernetes context probe timed out");
|
||||
await vi.advanceTimersByTimeAsync(50);
|
||||
|
||||
await rejection;
|
||||
expect(observedSignal?.aborted).toBe(true);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects unknown context IDs", () => {
|
||||
expect(() => createRegistry().getClient("missing")).toThrow(KubernetesContextNotFoundError);
|
||||
});
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import * as fs from "fs";
|
||||
import type { RequestContext, ResponseContext } from "@kubernetes/client-node";
|
||||
import { CoreV1Api, CustomObjectsApi, KubeConfig, Metrics, NetworkingV1Api, VersionApi } from "@kubernetes/client-node";
|
||||
import type { NodeMetricsList, RequestContext, ResponseContext } from "@kubernetes/client-node";
|
||||
import { CoreV1Api, CustomObjectsApi, KubeConfig, NetworkingV1Api, VersionApi } from "@kubernetes/client-node";
|
||||
import { of } from "@kubernetes/client-node/dist/gen/rxjsStub.js";
|
||||
|
||||
import { env } from "../../env";
|
||||
@@ -55,7 +55,6 @@ export class KubernetesClient {
|
||||
public kubeConfig: KubeConfig;
|
||||
public coreApi: CoreV1Api;
|
||||
public networkingApi: NetworkingV1Api;
|
||||
public metricsApi: Metrics;
|
||||
public metricsProbeApi: CustomObjectsApi;
|
||||
public versionApi: VersionApi;
|
||||
|
||||
@@ -63,11 +62,22 @@ export class KubernetesClient {
|
||||
this.kubeConfig = kubeConfig;
|
||||
this.coreApi = kubeConfig.makeApiClient(CoreV1Api);
|
||||
this.networkingApi = kubeConfig.makeApiClient(NetworkingV1Api);
|
||||
this.metricsApi = new Metrics(kubeConfig);
|
||||
this.metricsProbeApi = kubeConfig.makeApiClient(CustomObjectsApi);
|
||||
this.versionApi = kubeConfig.makeApiClient(VersionApi);
|
||||
}
|
||||
|
||||
public async getNodeMetricsAsync(timeoutMs = kubernetesContextProbeTimeoutMs): Promise<NodeMetricsList> {
|
||||
const result: unknown = await withKubernetesProbeTimeoutAsync(
|
||||
async (signal) =>
|
||||
await this.metricsProbeApi.listClusterCustomObject(
|
||||
{ group: "metrics.k8s.io", version: "v1beta1", plural: "nodes" },
|
||||
abortableKubernetesRequest(signal),
|
||||
),
|
||||
timeoutMs,
|
||||
);
|
||||
return result as NodeMetricsList;
|
||||
}
|
||||
|
||||
public static getInstance(contextId: string): KubernetesClient {
|
||||
return KubernetesClient.getRegistry().getClient(contextId);
|
||||
}
|
||||
@@ -125,14 +135,7 @@ export class KubernetesClientRegistry {
|
||||
async (signal) => await client.versionApi.getCode(undefined, abortableKubernetesRequest(signal)),
|
||||
timeoutMs,
|
||||
),
|
||||
withKubernetesProbeTimeoutAsync(
|
||||
async (signal) =>
|
||||
await client.metricsProbeApi.listClusterCustomObject(
|
||||
{ group: "metrics.k8s.io", version: "v1beta1", plural: "nodes" },
|
||||
abortableKubernetesRequest(signal),
|
||||
),
|
||||
timeoutMs,
|
||||
),
|
||||
client.getNodeMetricsAsync(timeoutMs),
|
||||
]);
|
||||
const status =
|
||||
api.status === "rejected" ? "unavailable" : metrics.status === "rejected" ? "degraded" : "available";
|
||||
|
||||
@@ -16,14 +16,15 @@ export const clusterRouter = createTRPCRouter({
|
||||
.concat(kubernetesMiddleware())
|
||||
.input(kubernetesContextInput)
|
||||
.query(async ({ input }): Promise<KubernetesCluster> => {
|
||||
const { coreApi, metricsApi, versionApi, kubeConfig } = getKubernetesClient(input.contextId);
|
||||
const client = getKubernetesClient(input.contextId);
|
||||
const { coreApi, versionApi, kubeConfig } = client;
|
||||
|
||||
try {
|
||||
const [versionInfo, nodes, listPodForAllNamespaces, nodeMetricsClient] = await Promise.all([
|
||||
versionApi.getCode(),
|
||||
coreApi.listNode(),
|
||||
coreApi.listPodForAllNamespaces(),
|
||||
metricsApi.getNodeMetrics().catch(() => null),
|
||||
client.getNodeMetricsAsync().catch(() => null),
|
||||
]);
|
||||
|
||||
let totalCPUCapacity = 0;
|
||||
|
||||
@@ -5,6 +5,13 @@ import { KubernetesClient } from "../kubernetes-client";
|
||||
export const contextsRouter = createTRPCRouter({
|
||||
getContexts: permissionRequiredProcedure
|
||||
.requiresPermission("admin")
|
||||
.meta({
|
||||
mcp: {
|
||||
enabled: true,
|
||||
description:
|
||||
"List configured Kubernetes contexts with availability, metrics status, and default-context metadata. Requires admin permission.",
|
||||
},
|
||||
})
|
||||
.concat(kubernetesMiddleware())
|
||||
.query(async () => await KubernetesClient.getContextsAsync()),
|
||||
});
|
||||
@@ -10,6 +10,13 @@ import { getKubernetesClient, kubernetesContextInput } from "../kubernetes-conte
|
||||
export const ingressesRouter = createTRPCRouter({
|
||||
getIngresses: permissionRequiredProcedure
|
||||
.requiresPermission("admin")
|
||||
.meta({
|
||||
mcp: {
|
||||
enabled: true,
|
||||
description:
|
||||
"List Kubernetes ingress inventory for a context. Get the required contextId from kubernetes_contexts_getContexts. Requires admin permission.",
|
||||
},
|
||||
})
|
||||
.concat(kubernetesMiddleware())
|
||||
.input(kubernetesContextInput)
|
||||
.query(async ({ input }): Promise<KubernetesIngress[]> => {
|
||||
|
||||
@@ -15,12 +15,13 @@ export const nodesRouter = createTRPCRouter({
|
||||
.concat(kubernetesMiddleware())
|
||||
.input(kubernetesContextInput)
|
||||
.query(async ({ input }): Promise<KubernetesNode[]> => {
|
||||
const { coreApi, metricsApi } = getKubernetesClient(input.contextId);
|
||||
const client = getKubernetesClient(input.contextId);
|
||||
const { coreApi } = client;
|
||||
|
||||
try {
|
||||
const [nodes, nodeMetricsClient] = await Promise.all([
|
||||
coreApi.listNode(),
|
||||
metricsApi.getNodeMetrics().catch(() => null),
|
||||
client.getNodeMetricsAsync().catch(() => null),
|
||||
]);
|
||||
const cpuResourceParser = new CpuResourceParser();
|
||||
const memoryResourceParser = new MemoryResourceParser();
|
||||
|
||||
@@ -14,6 +14,13 @@ const logger = createLogger({ module: "podsRouter" });
|
||||
export const podsRouter = createTRPCRouter({
|
||||
getPods: permissionRequiredProcedure
|
||||
.requiresPermission("admin")
|
||||
.meta({
|
||||
mcp: {
|
||||
enabled: true,
|
||||
description:
|
||||
"List Kubernetes pod inventory and inferred workload types for a context. Get the required contextId from kubernetes_contexts_getContexts. Requires admin permission.",
|
||||
},
|
||||
})
|
||||
.concat(kubernetesMiddleware())
|
||||
.input(kubernetesContextInput)
|
||||
.query(async ({ input }): Promise<KubernetesPod[]> => {
|
||||
|
||||
@@ -184,6 +184,20 @@ describe("All procedures should only be accessible for users with admin permissi
|
||||
});
|
||||
|
||||
describe("Docker action targeting", () => {
|
||||
test.each(["startAll", "stopAll", "restartAll", "removeAll"] as const)(
|
||||
"%s requires a bounded non-empty target list",
|
||||
async (procedure) => {
|
||||
const caller = createAdminCaller();
|
||||
|
||||
await expect(caller[procedure]({ targets: [] })).rejects.toThrow();
|
||||
await expect(
|
||||
caller[procedure]({
|
||||
targets: Array.from({ length: 101 }, (_, index) => ({ endpointId: "endpoint", id: String(index) })),
|
||||
}),
|
||||
).rejects.toThrow();
|
||||
},
|
||||
);
|
||||
|
||||
test("enforces endpoint capabilities before running a lifecycle action", async () => {
|
||||
hasDockerEndpointCapabilityMock.mockReturnValueOnce(false);
|
||||
|
||||
|
||||
Reference in new issue
Block a user