Skip to content

harden agent session runtime and prewarm matching #179

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.

Sign up for GitHub

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

Merged
vmelikyan merged 1 commit into from
Apr 19, 2026
Merged

harden agent session runtime and prewarm matching #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 62 additions & 0 deletions src/server/lib/agentSession/__tests__/devModeManager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ describe('DevModeManager', () => {
template: {
spec: {
nodeSelector: { 'kubernetes.io/hostname': 'agent-node-a' },
securityContext: { fsGroup: 1000, fsGroupChangePolicy: 'OnRootMismatch' },
volumes: [{ name: 'workspace', persistentVolumeClaim: { claimName: 'agent-pvc-abc' } }],
containers: [
expect.objectContaining({
Expand Down Expand Up @@ -186,6 +187,43 @@ describe('DevModeManager', () => {
});
});

it('preserves existing pod securityContext fields while normalizing shared workspace ownership', async () => {
mockReadDeployment.mockResolvedValue({
body: {
spec: {
template: {
spec: {
securityContext: {
fsGroup: 2000,
runAsNonRoot: false,
supplementalGroups: [2000],
},
containers: [{ name: 'web-app' }],
},
},
},
},
});

const opts: DevModeOptions = {
namespace: 'test-ns',
deploymentName: 'my-app',
serviceName: 'my-app',
pvcName: 'agent-pvc-abc',
devConfig: { image: 'node:20-slim', command: 'pnpm dev', workDir: '/workspace' },
};

await manager.enableDevMode(opts);

const patchBody = mockPatchDeployment.mock.calls[0][2];
expect(patchBody.spec.template.spec.securityContext).toEqual({
fsGroup: 1000,
fsGroupChangePolicy: 'OnRootMismatch',
runAsNonRoot: false,
supplementalGroups: [2000],
});
});

it('mounts the shared workspace root when workDir points at a service subdirectory', async () => {
const opts: DevModeOptions = {
namespace: 'test-ns',
Expand Down Expand Up @@ -325,6 +363,7 @@ describe('DevModeManager', () => {
replicas: 3,
template: {
spec: {
securityContext: { fsGroup: 2000, supplementalGroups: [2000] },
containers: [{ name: 'web-app', image: 'registry.example/my-app:built' }],
},
},
Expand All @@ -343,6 +382,10 @@ describe('DevModeManager', () => {
const snapshot = await manager.enableDevMode(opts);

expect(snapshot.deployment.replicas).toBe(3);
expect(snapshot.deployment.securityContext).toEqual({
fsGroup: 2000,
supplementalGroups: [2000],
});
});

it('pins an attached HorizontalPodAutoscaler to a single replica during dev mode', async () => {
Expand Down Expand Up @@ -413,6 +456,7 @@ describe('DevModeManager', () => {
volumeMounts: [{ name: 'config-volume', mountPath: '/config' }],
},
],
securityContext: { fsGroup: 2000 },
volumes: [{ name: 'config-volume' }],
},
},
Expand All @@ -424,6 +468,7 @@ describe('DevModeManager', () => {
replicas: 1,
template: {
spec: {
securityContext: { fsGroup: 1000, fsGroupChangePolicy: 'OnRootMismatch' },
containers: [
{
name: 'web-app',
Expand Down Expand Up @@ -458,6 +503,7 @@ describe('DevModeManager', () => {
{ op: 'remove', path: '/spec/template/spec/volumes/0' },
{ op: 'remove', path: '/spec/replicas' },
{ op: 'remove', path: '/spec/template/spec/nodeSelector' },
{ op: 'replace', path: '/spec/template/spec/securityContext', value: { fsGroup: 2000 } },
],
undefined,
undefined,
Expand Down Expand Up @@ -536,6 +582,7 @@ describe('DevModeManager', () => {
volumeMounts: [{ name: 'config-volume', mountPath: '/config' }],
volumes: [{ name: 'config-volume', emptyDir: {} }],
nodeSelector: { 'app-long': 'deployments-m7i' },
securityContext: { fsGroup: 2000 },
}),
'lifecycle.goodrx.com/dev-mode-hpa-snapshot': JSON.stringify({
hpaName: 'grpc-echo-hpa',
Expand All @@ -548,6 +595,7 @@ describe('DevModeManager', () => {
replicas: 1,
template: {
spec: {
securityContext: { fsGroup: 1000, fsGroupChangePolicy: 'OnRootMismatch' },
nodeSelector: {
'app-long': 'deployments-m7i',
'kubernetes.io/hostname': 'agent-node-a',
Expand Down Expand Up @@ -630,6 +678,11 @@ describe('DevModeManager', () => {
path: '/spec/template/spec/nodeSelector',
value: { 'app-long': 'deployments-m7i' },
},
{
op: 'replace',
path: '/spec/template/spec/securityContext',
value: { fsGroup: 2000 },
},
],
undefined,
undefined,
Expand Down Expand Up @@ -669,6 +722,7 @@ describe('DevModeManager', () => {
replicas: 1,
template: {
spec: {
securityContext: { fsGroup: 1000, fsGroupChangePolicy: 'OnRootMismatch' },
containers: [
{
name: 'grpc-echo',
Expand Down Expand Up @@ -703,6 +757,7 @@ describe('DevModeManager', () => {
{ op: 'remove', path: '/spec/template/spec/containers/0/volumeMounts/0' },
{ op: 'remove', path: '/spec/template/spec/volumes/0' },
{ op: 'remove', path: '/spec/template/spec/nodeSelector' },
{ op: 'remove', path: '/spec/template/spec/securityContext' },
],
undefined,
undefined,
Expand All @@ -726,6 +781,7 @@ describe('DevModeManager', () => {
replicas: 1,
template: {
spec: {
securityContext: { fsGroup: 1000, fsGroupChangePolicy: 'OnRootMismatch' },
containers: [
{
name: 'lc-apps',
Expand Down Expand Up @@ -777,6 +833,7 @@ describe('DevModeManager', () => {
volumeMounts: null,
volumes: null,
nodeSelector: { 'app-long': 'deployments-m7i' },
securityContext: { fsGroup: 2000 },
},
service: {
serviceName: 'grpc-echo-service',
Expand Down Expand Up @@ -809,6 +866,11 @@ describe('DevModeManager', () => {
path: '/spec/template/spec/nodeSelector',
value: { 'app-long': 'deployments-m7i' },
},
{
op: 'replace',
path: '/spec/template/spec/securityContext',
value: { fsGroup: 2000 },
},
],
undefined,
undefined,
Expand Down
20 changes: 7 additions & 13 deletions src/server/lib/agentSession/__tests__/podFactory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,12 @@ describe('podFactory', () => {
describe('buildSessionWorkspacePodSpec', () => {
it('creates a pod with init and main containers', () => {
const pod = buildSessionWorkspacePodSpec(baseOpts);
expect(pod.spec!.initContainers).toHaveLength(4);
expect(pod.spec!.initContainers).toHaveLength(3);
expect(pod.spec!.containers).toHaveLength(2);
expect(pod.spec!.initContainers!.map((container) => container.name)).toEqual([
'prepare-workspace',
'init-workspace',
'seed-runtime-config',
'prepare-editor-workspace',
]);
expect(pod.spec!.containers!.map((container) => container.name)).toEqual(['editor', 'workspace-gateway']);
});
Expand Down Expand Up @@ -819,23 +818,21 @@ describe('podFactory', () => {
],
});

expect(getInitContainer(pod, 'prepare-editor-workspace')).toEqual(
expect(getInitContainer(pod, 'seed-runtime-config')).toEqual(
expect.objectContaining({
command: [
'sh',
'-c',
expect.stringContaining(`cat > '${SESSION_WORKSPACE_EDITOR_PROJECT_FILE}' << 'WORKSPACE_EOF'`),
],
volumeMounts: [{ name: 'tmp', mountPath: '/tmp' }],
volumeMounts: expect.arrayContaining([{ name: 'tmp', mountPath: '/tmp' }]),
})
);
expect(getInitContainer(pod, 'prepare-editor-workspace').command?.[2]).toContain('"name": "org/repo"');
expect(getInitContainer(pod, 'prepare-editor-workspace').command?.[2]).toContain(
expect(getInitContainer(pod, 'seed-runtime-config').command?.[2]).toContain('"name": "org/repo"');
expect(getInitContainer(pod, 'seed-runtime-config').command?.[2]).toContain(
'"path": "/workspace/repos/org/repo"'
);
expect(getInitContainer(pod, 'prepare-editor-workspace').command?.[2]).toContain(
'"path": "/workspace/repos/org/api"'
);
expect(getInitContainer(pod, 'seed-runtime-config').command?.[2]).toContain('"path": "/workspace/repos/org/api"');
expect(getContainer(pod, 'workspace-gateway').env).toEqual(
expect.arrayContaining([{ name: 'LIFECYCLE_SESSION_PRIMARY_REPO_PATH', value: '/workspace/repos/org/repo' }])
);
Expand All @@ -847,10 +844,7 @@ describe('podFactory', () => {
skipWorkspaceBootstrap: true,
});

expect(pod.spec!.initContainers?.map((container) => container.name)).toEqual([
'seed-runtime-config',
'prepare-editor-workspace',
]);
expect(pod.spec!.initContainers?.map((container) => container.name)).toEqual(['seed-runtime-config']);
});

it('does not set runtimeClassName when gVisor not requested', () => {
Expand Down
8 changes: 4 additions & 4 deletions src/server/lib/agentSession/__tests__/runtimeConfig.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import {

const DEFAULT_READINESS = {
timeoutMs: 60000,
pollMs: 2000,
pollMs: 1000,
};

const DEFAULT_RESOURCES = {
Expand Down Expand Up @@ -321,19 +321,19 @@ describe('runtimeConfig', () => {

it('merges direct readiness overrides over runtime defaults', () => {
expect(
mergeAgentSessionReadiness(resolveAgentSessionReadinessFromDefaults({ timeoutMs: 60000, pollMs: 2000 }), {
mergeAgentSessionReadiness(resolveAgentSessionReadinessFromDefaults({ timeoutMs: 60000, pollMs: 1000 }), {
timeoutMs: 120000,
})
).toEqual({
timeoutMs: 120000,
pollMs: 2000,
pollMs: 1000,
});
});

it('merges service readiness overrides over runtime defaults', () => {
expect(
mergeAgentSessionReadinessForServices(
resolveAgentSessionReadinessFromDefaults({ timeoutMs: 60000, pollMs: 2000 }),
resolveAgentSessionReadinessFromDefaults({ timeoutMs: 60000, pollMs: 1000 }),
[{ timeoutMs: 120000 }, { timeoutMs: 90000, pollMs: 500 }, undefined, { pollMs: 1000 }]
)
).toEqual({
Expand Down
48 changes: 48 additions & 0 deletions src/server/lib/agentSession/devModeManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ const DEV_MODE_DEPLOYMENT_SNAPSHOT_ANNOTATION = 'lifecycle.goodrx.com/dev-mode-d
const DEV_MODE_SERVICE_SNAPSHOT_ANNOTATION = 'lifecycle.goodrx.com/dev-mode-service-snapshot';
const DEV_MODE_HPA_SNAPSHOT_ANNOTATION = 'lifecycle.goodrx.com/dev-mode-hpa-snapshot';
const SAME_NODE_SELECTOR_KEY = 'kubernetes.io/hostname';
const SHARED_WORKSPACE_FS_GROUP = 1000;
const SHARED_WORKSPACE_FS_GROUP_CHANGE_POLICY: NonNullable<k8s.V1PodSecurityContext['fsGroupChangePolicy']> =
'OnRootMismatch';

export interface DevModeOptions {
namespace: string;
Expand All @@ -45,6 +48,7 @@ export interface DevModeDeploymentSnapshot {
volumeMounts: k8s.V1VolumeMount[] | null;
volumes: k8s.V1Volume[] | null;
nodeSelector: Record<string, string> | null;
securityContext: k8s.V1PodSecurityContext | null;
}

export interface DevModeServiceSnapshot {
Expand All @@ -70,6 +74,7 @@ interface AppliedDeploymentTemplate {
template?: {
spec?: {
nodeSelector?: Record<string, string>;
securityContext?: k8s.V1PodSecurityContext;
containers?: Array<{
name?: string;
command?: string[];
Expand Down Expand Up @@ -106,6 +111,23 @@ function selectorMatches(
return Object.entries(expected).every(([key, value]) => selector[key] === value);
}

function buildSharedWorkspacePodSecurityContext(
securityContext?: k8s.V1PodSecurityContext | null
): k8s.V1PodSecurityContext {
return {
...(securityContext || {}),
fsGroup: SHARED_WORKSPACE_FS_GROUP,
fsGroupChangePolicy: SHARED_WORKSPACE_FS_GROUP_CHANGE_POLICY,
};
}

function hasSharedWorkspacePodSecurityContext(securityContext?: k8s.V1PodSecurityContext | null): boolean {
return (
securityContext?.fsGroup === SHARED_WORKSPACE_FS_GROUP &&
securityContext.fsGroupChangePolicy === SHARED_WORKSPACE_FS_GROUP_CHANGE_POLICY
);
}

export class DevModeManager {
private kc: k8s.KubeConfig;
private appsApi: k8s.AppsV1Api;
Expand Down Expand Up @@ -376,6 +398,7 @@ export class DevModeManager {
const deploymentName = existing.metadata?.name || opts.deploymentName;
const workDir = devConfig.workDir || '/workspace';
const existingContainerName = existing.spec?.template?.spec?.containers?.[0]?.name || deploymentName;
const existingPodSecurityContext = existing.spec?.template?.spec?.securityContext;
const nodeSelector = requiredNodeName
? {
...(existing.spec?.template?.spec?.nodeSelector || {}),
Expand All @@ -397,6 +420,7 @@ export class DevModeManager {
template: {
spec: {
...(nodeSelector ? { nodeSelector } : {}),
securityContext: buildSharedWorkspacePodSecurityContext(existingPodSecurityContext),
volumes: [{ name: 'workspace', persistentVolumeClaim: { claimName: pvcName } }],
containers: [
{
Expand Down Expand Up @@ -590,6 +614,12 @@ export class DevModeManager {
liveSpec?.nodeSelector,
desiredTemplate.spec?.template?.spec?.nodeSelector
);
this.appendValuePatch(
patch,
'/spec/template/spec/securityContext',
liveSpec?.securityContext,
desiredTemplate.spec?.template?.spec?.securityContext
);

if (patch.length === 0) {
return;
Expand Down Expand Up @@ -696,6 +726,7 @@ export class DevModeManager {
volumeMounts: liveContainer?.volumeMounts ? deepClone(liveContainer.volumeMounts) : null,
volumes: liveSpec?.volumes ? deepClone(liveSpec.volumes) : null,
nodeSelector: liveSpec?.nodeSelector ? deepClone(liveSpec.nodeSelector) : null,
securityContext: liveSpec?.securityContext ? deepClone(liveSpec.securityContext) : null,
};
}

Expand Down Expand Up @@ -824,6 +855,12 @@ export class DevModeManager {
existing.spec?.template?.spec?.nodeSelector,
snapshot.nodeSelector
);
this.appendValuePatch(
patch,
'/spec/template/spec/securityContext',
existing.spec?.template?.spec?.securityContext,
snapshot.securityContext
);

if (patch.length === 0) {
return;
Expand Down Expand Up @@ -992,6 +1029,17 @@ export class DevModeManager {
}
}

const liveSecurityContext = liveSpec?.securityContext;
if (isLikelyDevPatched && hasSharedWorkspacePodSecurityContext(liveSecurityContext)) {
const securityContextKeys = Object.keys(liveSecurityContext || {});
if (securityContextKeys.length === 2) {
patch.push({ op: 'remove', path: '/spec/template/spec/securityContext' });
} else {
patch.push({ op: 'remove', path: '/spec/template/spec/securityContext/fsGroup' });
patch.push({ op: 'remove', path: '/spec/template/spec/securityContext/fsGroupChangePolicy' });
}
}

return patch;
}
}
2 changes: 1 addition & 1 deletion src/server/lib/agentSession/editorServiceFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export async function createSessionWorkspaceService(

const { body: result } = await coreApi.createNamespacedService(namespace, service);
logger.info(
`Session: workspace editor ready serviceName=${serviceName} namespace=${namespace} port=${SESSION_WORKSPACE_EDITOR_PORT}`
`Session: workspace service created serviceName=${serviceName} namespace=${namespace} port=${SESSION_WORKSPACE_EDITOR_PORT}`
);
return result;
}
Expand Down
Loading
Loading