fix(k8s): set fsGroup on pods with volumes
A freshly provisioned PVC mounts root:root, so any image that drops privileges cannot write to it. docs-mcp-server runs as uid 1000 and died on first start with SQLITE_CANTOPEN; its own Dockerfile says to chown the volume 1000:1000. Pods with volumes now carry securityContext.fsGroup, defaulting to 1000 and overridable per volume. fsGroupChangePolicy is OnRootMismatch so Kubernetes does not walk and re-chown the whole volume on every start. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BMXdb2qZbPSh8Q7XpTyjB
This commit is contained in:
@@ -776,6 +776,10 @@ Notes:
|
|||||||
- Claims are `ReadWriteOnce`, so a server with volumes is limited to one
|
- Claims are `ReadWriteOnce`, so a server with volumes is limited to one
|
||||||
replica. Asking for more is rejected at validation rather than leaving the
|
replica. Asking for more is rejected at validation rather than leaving the
|
||||||
extra replicas unschedulable.
|
extra replicas unschedulable.
|
||||||
|
- A fresh claim mounts `root:root`, so mcpctl sets the pod's `fsGroup` to
|
||||||
|
**1000** — the conventional non-root uid in node/python images. Override with
|
||||||
|
`fsGroup` on the volume if an image uses a different one; containers running
|
||||||
|
as root are unaffected either way.
|
||||||
- `storageClass` defaults to `MCPD_VOLUME_STORAGE_CLASS`, and is omitted
|
- `storageClass` defaults to `MCPD_VOLUME_STORAGE_CLASS`, and is omitted
|
||||||
entirely when neither is set. **Set it explicitly on any cluster with more
|
entirely when neither is set. **Set it explicitly on any cluster with more
|
||||||
than one default StorageClass**, where an omitted class binds
|
than one default StorageClass**, where an omitted class binds
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ const VolumeSpecSchema = z.object({
|
|||||||
mountPath: z.string().min(1).regex(/^\//, 'mountPath must be absolute'),
|
mountPath: z.string().min(1).regex(/^\//, 'mountPath must be absolute'),
|
||||||
sizeGb: z.number().int().min(1).max(1024).default(10),
|
sizeGb: z.number().int().min(1).max(1024).default(10),
|
||||||
storageClass: z.string().optional(),
|
storageClass: z.string().optional(),
|
||||||
|
fsGroup: z.number().int().min(0).max(65535).optional(),
|
||||||
});
|
});
|
||||||
|
|
||||||
const ServerSpecSchema = z.object({
|
const ServerSpecSchema = z.object({
|
||||||
|
|||||||
@@ -381,6 +381,7 @@ export class InstanceService {
|
|||||||
mountPath: string;
|
mountPath: string;
|
||||||
sizeGb?: number;
|
sizeGb?: number;
|
||||||
storageClass?: string;
|
storageClass?: string;
|
||||||
|
fsGroup?: number;
|
||||||
}>;
|
}>;
|
||||||
if (volumes.length > 0) {
|
if (volumes.length > 0) {
|
||||||
spec.volumes = volumes.map((v) => ({
|
spec.volumes = volumes.map((v) => ({
|
||||||
@@ -390,6 +391,7 @@ export class InstanceService {
|
|||||||
...(v.storageClass !== undefined && v.storageClass !== ''
|
...(v.storageClass !== undefined && v.storageClass !== ''
|
||||||
? { storageClass: v.storageClass }
|
? { storageClass: v.storageClass }
|
||||||
: {}),
|
: {}),
|
||||||
|
...(v.fsGroup !== undefined ? { fsGroup: v.fsGroup } : {}),
|
||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
// Package-based servers: command = [packageName, ...args] (entrypoint handles execution)
|
// Package-based servers: command = [packageName, ...args] (entrypoint handles execution)
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import type { ContainerSpec, ContainerVolume } from '../orchestrator.js';
|
import type { ContainerSpec, ContainerVolume } from '../orchestrator.js';
|
||||||
import { DEFAULT_MEMORY_LIMIT, DEFAULT_NANO_CPUS } from '../orchestrator.js';
|
import { DEFAULT_MEMORY_LIMIT, DEFAULT_NANO_CPUS, DEFAULT_VOLUME_FS_GROUP } from '../orchestrator.js';
|
||||||
|
|
||||||
const MCPCTL_LABEL = 'mcpctl.managed';
|
const MCPCTL_LABEL = 'mcpctl.managed';
|
||||||
|
|
||||||
@@ -37,6 +37,7 @@ export interface K8sPodManifest {
|
|||||||
automountServiceAccountToken: boolean;
|
automountServiceAccountToken: boolean;
|
||||||
nodeSelector?: Record<string, string>;
|
nodeSelector?: Record<string, string>;
|
||||||
volumes?: Array<{ name: string; persistentVolumeClaim: { claimName: string } }>;
|
volumes?: Array<{ name: string; persistentVolumeClaim: { claimName: string } }>;
|
||||||
|
securityContext?: { fsGroup: number; fsGroupChangePolicy: string };
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -95,13 +96,24 @@ function sanitizeName(name: string): string {
|
|||||||
return name.toLowerCase().replace(/[^a-z0-9-]/g, '-').replace(/^-+|-+$/g, '').slice(0, 63);
|
return name.toLowerCase().replace(/[^a-z0-9-]/g, '-').replace(/^-+|-+$/g, '').slice(0, 63);
|
||||||
}
|
}
|
||||||
|
|
||||||
function buildPodVolumes(spec: ContainerSpec): Pick<K8sPodManifest['spec'], 'volumes'> {
|
function buildPodVolumes(
|
||||||
|
spec: ContainerSpec,
|
||||||
|
): Pick<K8sPodManifest['spec'], 'volumes' | 'securityContext'> {
|
||||||
if (!spec.volumes || spec.volumes.length === 0) return {};
|
if (!spec.volumes || spec.volumes.length === 0) return {};
|
||||||
return {
|
return {
|
||||||
volumes: spec.volumes.map((v) => ({
|
volumes: spec.volumes.map((v) => ({
|
||||||
name: sanitizeName(v.claimName),
|
name: sanitizeName(v.claimName),
|
||||||
persistentVolumeClaim: { claimName: sanitizeName(v.claimName) },
|
persistentVolumeClaim: { claimName: sanitizeName(v.claimName) },
|
||||||
})),
|
})),
|
||||||
|
securityContext: {
|
||||||
|
// Without this a fresh PVC mounts root:root and any image that dropped
|
||||||
|
// privileges cannot write to it. First declared fsGroup wins — a pod has
|
||||||
|
// exactly one, so per-volume groups are not expressible here.
|
||||||
|
fsGroup: spec.volumes[0]?.fsGroup ?? DEFAULT_VOLUME_FS_GROUP,
|
||||||
|
// Only chown when the top-level ownership is already wrong; the default
|
||||||
|
// ("Always") walks the whole volume on every single start.
|
||||||
|
fsGroupChangePolicy: 'OnRootMismatch',
|
||||||
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -24,6 +24,16 @@ export interface ContainerVolume {
|
|||||||
* where an omitted class binds nondeterministically.
|
* where an omitted class binds nondeterministically.
|
||||||
*/
|
*/
|
||||||
storageClass?: string;
|
storageClass?: string;
|
||||||
|
/**
|
||||||
|
* Group that owns the mounted volume (Kubernetes `fsGroup`).
|
||||||
|
*
|
||||||
|
* A fresh PVC mounts as root:root, so any image that drops privileges cannot
|
||||||
|
* write to it — docs-mcp-server fails with SQLITE_CANTOPEN, and its own
|
||||||
|
* Dockerfile tells you to `chown 1000:1000` the volume. Defaults to
|
||||||
|
* DEFAULT_VOLUME_FS_GROUP, which covers images using the conventional uid
|
||||||
|
* 1000; containers still running as root are unaffected either way.
|
||||||
|
*/
|
||||||
|
fsGroup?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface ContainerSpec {
|
export interface ContainerSpec {
|
||||||
@@ -117,3 +127,10 @@ export interface InteractiveExec {
|
|||||||
/** Default resource limits */
|
/** Default resource limits */
|
||||||
export const DEFAULT_MEMORY_LIMIT = 512 * 1024 * 1024; // 512 MB
|
export const DEFAULT_MEMORY_LIMIT = 512 * 1024 * 1024; // 512 MB
|
||||||
export const DEFAULT_NANO_CPUS = 500_000_000; // 0.5 CPU
|
export const DEFAULT_NANO_CPUS = 500_000_000; // 0.5 CPU
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Group applied to mounted volumes when a server does not specify one.
|
||||||
|
* 1000 is the conventional non-root uid/gid in node, python and distro base
|
||||||
|
* images, which is what the images that drop privileges actually use.
|
||||||
|
*/
|
||||||
|
export const DEFAULT_VOLUME_FS_GROUP = 1000;
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ export const VolumeSpecSchema = z.object({
|
|||||||
mountPath: z.string().min(1).max(200).regex(/^\//, 'mountPath must be absolute'),
|
mountPath: z.string().min(1).max(200).regex(/^\//, 'mountPath must be absolute'),
|
||||||
sizeGb: z.number().int().min(1).max(1024).default(10),
|
sizeGb: z.number().int().min(1).max(1024).default(10),
|
||||||
storageClass: z.string().max(100).optional(),
|
storageClass: z.string().max(100).optional(),
|
||||||
|
fsGroup: z.number().int().min(0).max(65535).optional(),
|
||||||
});
|
});
|
||||||
|
|
||||||
export type VolumeSpecInput = z.infer<typeof VolumeSpecSchema>;
|
export type VolumeSpecInput = z.infer<typeof VolumeSpecSchema>;
|
||||||
|
|||||||
@@ -177,6 +177,24 @@ describe('volumes', () => {
|
|||||||
const pod = generatePodSpec(baseSpec, 'mcpctl-servers');
|
const pod = generatePodSpec(baseSpec, 'mcpctl-servers');
|
||||||
expect(pod.spec.volumes).toBeUndefined();
|
expect(pod.spec.volumes).toBeUndefined();
|
||||||
expect(pod.spec.containers[0]!.volumeMounts).toBeUndefined();
|
expect(pod.spec.containers[0]!.volumeMounts).toBeUndefined();
|
||||||
|
expect(pod.spec.securityContext).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('sets fsGroup so a non-root image can write to a fresh claim', () => {
|
||||||
|
// A PVC mounts root:root. Without fsGroup, any image that drops privileges
|
||||||
|
// (docs-mcp-server runs as uid 1000) fails on first write.
|
||||||
|
const pod = generatePodSpec(volumeSpec, 'mcpctl-servers');
|
||||||
|
expect(pod.spec.securityContext?.fsGroup).toBe(1000);
|
||||||
|
// "Always" re-chowns the entire volume on every start.
|
||||||
|
expect(pod.spec.securityContext?.fsGroupChangePolicy).toBe('OnRootMismatch');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('honours an explicit fsGroup', () => {
|
||||||
|
const pod = generatePodSpec(
|
||||||
|
{ ...baseSpec, volumes: [{ claimName: 'c', mountPath: '/d', sizeGb: 1, fsGroup: 65534 }] },
|
||||||
|
'ns',
|
||||||
|
);
|
||||||
|
expect(pod.spec.securityContext?.fsGroup).toBe(65534);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('carries volumes into a deployment pod template', () => {
|
it('carries volumes into a deployment pod template', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user