Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
12 changes: 9 additions & 3 deletions src/server/lib/agentSession/__tests__/forwardedEnv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('forwardedEnv', () => {
(SecretProcessor as unknown as jest.Mock).mockImplementation(() => ({
processEnvSecrets: jest.fn().mockResolvedValue({
secretRefs: [],
secretNames: [],
expectedKeysPerSecret: {},
warnings: [],
}),
waitForSecretSync: jest.fn().mockResolvedValue(undefined),
Expand Down Expand Up @@ -192,7 +192,9 @@ describe('forwardedEnv', () => {
key: 'npmToken',
},
],
secretNames: ['agent-env-session-123-aws-secrets'],
expectedKeysPerSecret: {
'agent-env-session-123-aws-secrets': ['PRIVATE_REGISTRY_TOKEN'],
},
warnings: [],
});
const waitForSecretSync = jest.fn().mockResolvedValue(undefined);
Expand Down Expand Up @@ -236,7 +238,11 @@ describe('forwardedEnv', () => {
namespace: 'test-ns',
buildUuid: 'build-123',
});
expect(waitForSecretSync).toHaveBeenCalledWith(['agent-env-session-123-aws-secrets'], 'test-ns', 30000);
expect(waitForSecretSync).toHaveBeenCalledWith(
{ 'agent-env-session-123-aws-secrets': ['PRIVATE_REGISTRY_TOKEN'] },
'test-ns',
30000
);
expect(result.secretProviders).toEqual(['aws']);
});

Expand Down
6 changes: 4 additions & 2 deletions src/server/lib/agentSession/forwardedEnv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,14 @@ export async function resolveForwardedAgentEnv(
throw new Error(secretResult.warnings.join(' '));
}

if (secretResult.secretNames.length > 0) {
const secretNames = Object.keys(secretResult.expectedKeysPerSecret);

if (secretNames.length > 0) {
const providerTimeouts = Object.values(secretProviders)
.map((provider) => provider.secretSyncTimeout)
.filter((timeout): timeout is number => timeout !== undefined);
const timeout = providerTimeouts.length > 0 ? Math.max(...providerTimeouts) * 1000 : 60000;
await secretProcessor.waitForSecretSync(secretResult.secretNames, namespace, timeout);
await secretProcessor.waitForSecretSync(secretResult.expectedKeysPerSecret, namespace, timeout);
}

return {
Expand Down
131 changes: 121 additions & 10 deletions src/server/services/__tests__/secretProcessor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,33 +56,53 @@ describe('SecretProcessor', () => {
});

describe('waitForSecretSync', () => {
it('resolves when secret exists with data', async () => {
it('resolves when secret contains expected keys', async () => {
const mockReadNamespacedSecret = jest.fn().mockResolvedValue({
body: {
data: { key: 'dmFsdWU=' },
data: { API_TOKEN: 'dmFsdWU=' },
},
});

jest.spyOn(processor as any, 'getK8sClient').mockReturnValue({
readNamespacedSecret: mockReadNamespacedSecret,
});

await expect(processor.waitForSecretSync(['my-secret'], 'test-ns', 5000)).resolves.toBeUndefined();
await expect(
processor.waitForSecretSync({ 'my-secret': ['API_TOKEN'] }, 'test-ns', 5000)
).resolves.toBeUndefined();

expect(mockReadNamespacedSecret).toHaveBeenCalledWith('my-secret', 'test-ns');
});

it('treats empty secret values as present keys', async () => {
const mockReadNamespacedSecret = jest.fn().mockResolvedValue({
body: {
data: { EMPTY_TOKEN: '' },
},
});

jest.spyOn(processor as any, 'getK8sClient').mockReturnValue({
readNamespacedSecret: mockReadNamespacedSecret,
});

await expect(
processor.waitForSecretSync({ 'my-secret': ['EMPTY_TOKEN'] }, 'test-ns', 5000)
).resolves.toBeUndefined();
});

it('throws error on timeout when secret does not exist', async () => {
const mockReadNamespacedSecret = jest.fn().mockRejectedValue({ statusCode: 404 });

jest.spyOn(processor as any, 'getK8sClient').mockReturnValue({
readNamespacedSecret: mockReadNamespacedSecret,
});

await expect(processor.waitForSecretSync(['my-secret'], 'test-ns', 1000)).rejects.toThrow('Secret sync timeout');
await expect(processor.waitForSecretSync({ 'my-secret': ['API_TOKEN'] }, 'test-ns', 1000)).rejects.toThrow(
/Secret sync timeout.*missing keys=\[API_TOKEN\]/
);
});

it('throws error on timeout when secret has no data', async () => {
it('throws with missing keys listed when Secret data is empty', async () => {
const mockReadNamespacedSecret = jest.fn().mockResolvedValue({
body: { data: null },
});
Expand All @@ -91,7 +111,89 @@ describe('SecretProcessor', () => {
readNamespacedSecret: mockReadNamespacedSecret,
});

await expect(processor.waitForSecretSync(['my-secret'], 'test-ns', 1000)).rejects.toThrow('Secret sync timeout');
await expect(processor.waitForSecretSync({ 'my-secret': ['API_TOKEN'] }, 'test-ns', 1000)).rejects.toThrow(
/Secret sync timeout.*missing keys=\[API_TOKEN\]/
);
});

it('waits until an existing secret contains newly requested keys', async () => {
const mockReadNamespacedSecret = jest
.fn()
.mockResolvedValueOnce({
body: { data: { EXISTING_TOKEN: 'b2xk' } },
})
.mockResolvedValueOnce({
body: { data: { EXISTING_TOKEN: 'b2xk', NEW_TOKEN: 'bmV3' } },
});

jest.spyOn(processor as any, 'getK8sClient').mockReturnValue({
readNamespacedSecret: mockReadNamespacedSecret,
});
jest.spyOn(processor as any, 'sleep').mockResolvedValue(undefined);

await expect(
processor.waitForSecretSync({ 'sample-service-aws-secrets': ['EXISTING_TOKEN', 'NEW_TOKEN'] }, 'test-ns', 5000)
).resolves.toBeUndefined();

expect(mockReadNamespacedSecret).toHaveBeenCalledTimes(2);
});

it('waits through not found and partial keys until all requested keys land', async () => {
const mockReadNamespacedSecret = jest
.fn()
.mockRejectedValueOnce({ statusCode: 404 })
.mockResolvedValueOnce({
body: { data: { FIRST_TOKEN: 'Zmlyc3Q=' } },
})
.mockResolvedValueOnce({
body: { data: { FIRST_TOKEN: 'Zmlyc3Q=', SECOND_TOKEN: 'c2Vjb25k' } },
});

jest.spyOn(processor as any, 'getK8sClient').mockReturnValue({
readNamespacedSecret: mockReadNamespacedSecret,
});
jest.spyOn(processor as any, 'sleep').mockResolvedValue(undefined);

await processor.waitForSecretSync(
{ 'sample-service-aws-secrets': ['FIRST_TOKEN', 'SECOND_TOKEN'] },
'test-ns',
5000
);

expect(mockReadNamespacedSecret).toHaveBeenCalledTimes(3);
});

it('waits until the last requested key lands', async () => {
const mockReadNamespacedSecret = jest
.fn()
.mockResolvedValueOnce({
body: { data: { FIRST_TOKEN: 'Zmlyc3Q=' } },
})
.mockResolvedValueOnce({
body: { data: { FIRST_TOKEN: 'Zmlyc3Q=', SECOND_TOKEN: 'c2Vjb25k' } },
})
.mockResolvedValueOnce({
body: {
data: {
FIRST_TOKEN: 'Zmlyc3Q=',
SECOND_TOKEN: 'c2Vjb25k',
THIRD_TOKEN: 'dGhpcmQ=',
},
},
});

jest.spyOn(processor as any, 'getK8sClient').mockReturnValue({
readNamespacedSecret: mockReadNamespacedSecret,
});
jest.spyOn(processor as any, 'sleep').mockResolvedValue(undefined);

await processor.waitForSecretSync(
{ 'sample-service-aws-secrets': ['FIRST_TOKEN', 'SECOND_TOKEN', 'THIRD_TOKEN'] },
'test-ns',
5000
);

expect(mockReadNamespacedSecret).toHaveBeenCalledTimes(3);
});

it('waits for multiple secrets', async () => {
Expand All @@ -103,7 +205,7 @@ describe('SecretProcessor', () => {
readNamespacedSecret: mockReadNamespacedSecret,
});

await processor.waitForSecretSync(['secret-1', 'secret-2'], 'test-ns', 5000);
await processor.waitForSecretSync({ 'secret-1': ['key'], 'secret-2': ['key'] }, 'test-ns', 5000);

expect(mockReadNamespacedSecret).toHaveBeenCalledWith('secret-1', 'test-ns');
expect(mockReadNamespacedSecret).toHaveBeenCalledWith('secret-2', 'test-ns');
Expand All @@ -126,6 +228,9 @@ describe('SecretProcessor', () => {

expect(result.secretRefs).toHaveLength(1);
expect(result.secretRefs[0].envKey).toBe('DB_PASSWORD');
expect(result.expectedKeysPerSecret).toEqual({
'api-server-aws-secrets': ['DB_PASSWORD'],
});
expect(result.warnings).toHaveLength(0);
});

Expand All @@ -141,6 +246,7 @@ describe('SecretProcessor', () => {
});

expect(result.secretRefs).toHaveLength(0);
expect(result.expectedKeysPerSecret).toEqual({});
expect(result.warnings).toHaveLength(1);
expect(result.warnings[0]).toContain('disabled');
});
Expand All @@ -162,6 +268,7 @@ describe('SecretProcessor', () => {
});

expect(result.secretRefs).toHaveLength(0);
expect(result.expectedKeysPerSecret).toEqual({});
expect(result.warnings).toHaveLength(1);
expect(result.warnings[0]).toContain('not configured');
});
Expand Down Expand Up @@ -222,6 +329,7 @@ describe('SecretProcessor', () => {
});

expect(result.secretRefs).toHaveLength(0);
expect(result.expectedKeysPerSecret).toEqual({});
expect(result.warnings).toHaveLength(0);
});

Expand All @@ -240,11 +348,12 @@ describe('SecretProcessor', () => {
});

expect(result.secretRefs).toHaveLength(1);
expect(result.expectedKeysPerSecret).toEqual({});
expect(result.warnings).toHaveLength(1);
expect(result.warnings[0]).toContain('Failed to apply ExternalSecret');
});

it('returns secret names for mounting', async () => {
it('returns expected keys by secret for mounting', async () => {
const env = {
AWS_SECRET: '{{aws:path:key}}',
GCP_SECRET: '{{gcp:path:key}}',
Expand All @@ -261,8 +370,10 @@ describe('SecretProcessor', () => {
namespace: 'ns',
});

expect(result.secretNames).toContain('api-server-aws-secrets');
expect(result.secretNames).toContain('api-server-gcp-secrets');
expect(result.expectedKeysPerSecret).toEqual({
'api-server-aws-secrets': ['AWS_SECRET'],
'api-server-gcp-secrets': ['GCP_SECRET'],
});
});
});
});
14 changes: 10 additions & 4 deletions src/server/services/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1156,17 +1156,23 @@ export default class DeployService extends BaseService {
);
}

if (secretResult.secretNames.length > 0) {
getLogger().info(`Build: waiting for secrets to sync secrets=[${secretResult.secretNames.join(', ')}]`);
const secretNames = Object.keys(secretResult.expectedKeysPerSecret);

if (secretNames.length > 0) {
getLogger().info(`Build: waiting for secrets to sync secrets=[${secretNames.join(', ')}]`);

const providerTimeouts = Object.values(secretProviders)
.map((p) => p.secretSyncTimeout)
.filter((t): t is number => t !== undefined);
const timeout = providerTimeouts.length > 0 ? Math.max(...providerTimeouts) * 1000 : 60000;

try {
await secretProcessor.waitForSecretSync(secretResult.secretNames, deploy.build.namespace, timeout);
buildSecretNames = secretResult.secretNames;
await secretProcessor.waitForSecretSync(
secretResult.expectedKeysPerSecret,
deploy.build.namespace,
timeout
);
buildSecretNames = secretNames;
getLogger().info(`Build: secrets synced count=${buildSecretNames.length}`);
} catch (error) {
getLogger().error({ error }, `Build: secret sync failed service=${deployable.name}`);
Expand Down
31 changes: 21 additions & 10 deletions src/server/services/secretProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export interface ProcessEnvSecretsOptions {

export interface ProcessEnvSecretsResult {
secretRefs: SecretRefWithEnvKey[];
secretNames: string[];
expectedKeysPerSecret: Record<string, string[]>;
warnings: string[];
}

Expand All @@ -57,28 +57,39 @@ export class SecretProcessor {
return this.k8sClient;
}

async waitForSecretSync(secretNames: string[], namespace: string, timeoutMs?: number): Promise<void> {
// ExternalSecret updates reuse Secret names, so wait for this apply's keys instead of any existing data.
async waitForSecretSync(
expectedKeysPerSecret: Record<string, string[]>,
namespace: string,
timeoutMs?: number
): Promise<void> {
const timeout = timeoutMs ?? DEFAULT_SECRET_SYNC_TIMEOUT;
const pollInterval = 1000;
const startTime = Date.now();

const k8sClient = this.getK8sClient();

for (const secretName of secretNames) {
for (const [secretName, expectedKeys] of Object.entries(expectedKeysPerSecret)) {
let synced = false;
let missingKeys = expectedKeys;

while (!synced) {
if (Date.now() - startTime > timeout) {
throw new Error(`Secret sync timeout: ${secretName} not ready after ${timeout}ms`);
throw new Error(
`Secret sync timeout: ${secretName} missing keys=[${missingKeys.join(', ')}] after ${timeout}ms`
);
}

try {
const response = await k8sClient.readNamespacedSecret(secretName, namespace);
if (response.body.data && Object.keys(response.body.data).length > 0) {
const data = response.body.data || {};
missingKeys = expectedKeys.filter((key) => !Object.prototype.hasOwnProperty.call(data, key));

if (missingKeys.length === 0) {
getLogger().info(`Secret: synced name=${secretName} namespace=${namespace}`);
synced = true;
} else {
getLogger().debug(`Secret: waiting for data name=${secretName}`);
getLogger().debug(`Secret: waiting for keys name=${secretName} missing=[${missingKeys.join(', ')}]`);
await this.sleep(pollInterval);
}
} catch (error: any) {
Expand Down Expand Up @@ -120,11 +131,11 @@ export class SecretProcessor {
}

if (validRefs.length === 0) {
return { secretRefs: [], secretNames: [], warnings };
return { secretRefs: [], expectedKeysPerSecret: {}, warnings };
}

const grouped = groupSecretRefsByProvider(validRefs);
const secretNames: string[] = [];
const expectedKeysPerSecret: Record<string, string[]> = {};

for (const [provider, refs] of Object.entries(grouped)) {
const providerConfig = this.secretProviders![provider];
Expand All @@ -141,14 +152,14 @@ export class SecretProcessor {

try {
await applyExternalSecret(manifest, namespace);
secretNames.push(secretName);
expectedKeysPerSecret[secretName] = [...new Set(refs.map((ref) => ref.envKey))];
} catch (error) {
const errorMsg = (error as any)?.message || (error as any)?.stderr || String(error);
const warning = `Failed to apply ExternalSecret for ${serviceName}: ${errorMsg}`;
warnings.push(warning);
}
}

return { secretRefs: validRefs, secretNames, warnings };
return { secretRefs: validRefs, expectedKeysPerSecret, warnings };
}
}
Loading