Skip to content

Commit ca70e83

Browse files
sethvargosethvargo
authored
Fix updateMask computing too deep (#430)
Also fix an issue where integration tests were not actually running. Closes #429
1 parent 7dc254c commit ca70e83

File tree

5 files changed

+138
-103
lines changed

5 files changed

+138
-103
lines changed

‎.github/workflows/integration.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ jobs:
4040
- id: 'deploy'
4141
uses: './'
4242
with:
43-
name: 'https-trigger-${{ github.run_number }}'
43+
name: 'integration-https-trigger-${{ github.run_number }}-${{ github.run_attempt }}'
4444
runtime: 'nodejs22'
4545
entry_point: 'helloWorld'
4646
source_dir: './tests/test-node-func/'
@@ -70,7 +70,7 @@ jobs:
7070
- id: 'deploy'
7171
uses: './'
7272
with:
73-
name: 'event-trigger-${{ github.run_number }}'
73+
name: 'integration-event-trigger-${{ github.run_number }}-${{ github.run_attempt }}'
7474
runtime: 'nodejs22'
7575
entry_point: 'helloWorld'
7676
source_dir: './tests/test-node-func/'

‎.github/workflows/unit.yml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,16 @@ jobs:
4646
if: ${{ matrix.os == 'ubuntu-latest' }}
4747
run: 'npm run lint'
4848

49-
- uses: 'google-github-actions/auth@v2'
49+
- id: 'auth'
50+
uses: 'google-github-actions/auth@v2'
5051
if: ${{ github.event_name == 'push' || github.repository == github.event.pull_request.head.repo.full_name && github.actor != 'dependabot[bot]' }}
5152
with:
53+
project_id: '${{ vars.PROJECT_ID }}'
5254
workload_identity_provider: '${{ vars.WIF_PROVIDER_NAME }}'
53-
service_account: '${{ vars.SERVICE_ACCOUNT_EMAIL }}'
5455

5556
- name: 'npm test'
5657
env:
58+
TEST_AUTHENTICATED: '${{ !!steps.auth.outputs.auth_token }}'
5759
TEST_PROJECT_ID: '${{ vars.PROJECT_ID }}'
5860
TEST_SERVICE_ACCOUNT_EMAIL: '${{ vars.SERVICE_ACCOUNT_EMAIL }}'
5961
TEST_SECRET_VERSION_NAME: '${{ vars.SECRET_VERSION_NAME }}'

‎src/client.ts‎

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -598,22 +598,53 @@ export class CloudFunctionsClient {
598598
computeUpdateMask(cf: CloudFunction): string {
599599
const keys: string[] = [];
600600

601-
const iter = (obj: object, root?: string) => {
602-
for (const [k, v] of Object.entries(obj)) {
603-
if (v === undefined) {
604-
continue;
605-
}
606-
607-
const pth = root ? root + '.' + k : k;
608-
if (typeof v === 'object' && !Array.isArray(v)) {
609-
iter(v, pth);
610-
} else {
611-
keys.push(pth);
612-
}
613-
}
614-
};
615-
616-
iter(cf);
601+
if (cf.name !== undefined) keys.push('name');
602+
if (cf.description !== undefined) keys.push('description');
603+
if (cf.environment !== undefined) keys.push('environment');
604+
if (cf.kmsKeyName !== undefined) keys.push('kmsKeyName');
605+
if (cf.labels !== undefined) keys.push('labels');
606+
607+
if (cf.buildConfig?.runtime !== undefined) keys.push('buildConfig.runtime');
608+
if (cf.buildConfig?.entryPoint !== undefined) keys.push('buildConfig.entryPoint');
609+
if (cf.buildConfig?.source !== undefined) keys.push('buildConfig.source');
610+
if (cf.buildConfig?.dockerRepository !== undefined) keys.push('buildConfig.dockerRepository');
611+
if (cf.buildConfig?.environmentVariables !== undefined)
612+
keys.push('buildConfig.environmentVariables');
613+
if (cf.buildConfig?.serviceAccount !== undefined) keys.push('buildConfig.serviceAccount');
614+
if (cf.buildConfig?.workerPool !== undefined) keys.push('buildConfig.workerPool');
615+
616+
if (cf.serviceConfig?.allTrafficOnLatestRevision !== undefined)
617+
keys.push('serviceConfig.allTrafficOnLatestRevision');
618+
if (cf.serviceConfig?.availableCpu !== undefined) keys.push('serviceConfig.availableCpu');
619+
if (cf.serviceConfig?.availableMemory !== undefined) keys.push('serviceConfig.availableMemory');
620+
if (cf.serviceConfig?.environmentVariables !== undefined)
621+
keys.push('serviceConfig.environmentVariables');
622+
if (cf.serviceConfig?.ingressSettings !== undefined) keys.push('serviceConfig.ingressSettings');
623+
if (cf.serviceConfig?.maxInstanceCount !== undefined)
624+
keys.push('serviceConfig.maxInstanceCount');
625+
if (cf.serviceConfig?.maxInstanceRequestConcurrency !== undefined)
626+
keys.push('serviceConfig.maxInstanceRequestConcurrency');
627+
if (cf.serviceConfig?.minInstanceCount !== undefined)
628+
keys.push('serviceConfig.minInstanceCount');
629+
if (cf.serviceConfig?.secretEnvironmentVariables !== undefined)
630+
keys.push('serviceConfig.secretEnvironmentVariables');
631+
if (cf.serviceConfig?.secretVolumes !== undefined) keys.push('serviceConfig.secretVolumes');
632+
if (cf.serviceConfig?.serviceAccountEmail !== undefined)
633+
keys.push('serviceConfig.serviceAccountEmail');
634+
if (cf.serviceConfig?.timeoutSeconds !== undefined) keys.push('serviceConfig.timeoutSeconds');
635+
if (cf.serviceConfig?.vpcConnector !== undefined) keys.push('serviceConfig.vpcConnector');
636+
if (cf.serviceConfig?.vpcConnectorEgressSettings !== undefined)
637+
keys.push('serviceConfig.vpcConnectorEgressSettings');
638+
639+
if (cf.eventTrigger?.triggerRegion !== undefined) keys.push('eventTrigger.triggerRegion');
640+
if (cf.eventTrigger?.eventType !== undefined) keys.push('eventTrigger.eventType');
641+
if (cf.eventTrigger?.eventFilters !== undefined) keys.push('eventTrigger.eventFilters');
642+
if (cf.eventTrigger?.pubsubTopic !== undefined) keys.push('eventTrigger.pubsubTopic');
643+
if (cf.eventTrigger?.serviceAccountEmail !== undefined)
644+
keys.push('eventTrigger.serviceAccountEmail');
645+
if (cf.eventTrigger?.retryPolicy !== undefined) keys.push('eventTrigger.retryPolicy');
646+
if (cf.eventTrigger?.channel !== undefined) keys.push('eventTrigger.channel');
647+
if (cf.eventTrigger?.service !== undefined) keys.push('eventTrigger.service');
617648

618649
return keys.join(',');
619650
}

‎tests/client.test.ts‎

Lines changed: 85 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@ import { zipDir } from '../src/util';
2929

3030
const { TEST_PROJECT_ID, TEST_SERVICE_ACCOUNT_EMAIL, TEST_SECRET_VERSION_NAME } = process.env;
3131
const TEST_LOCATION = 'us-central1';
32-
const TEST_FUNCTION_NAME = 'test-' + crypto.randomBytes(12).toString('hex');
32+
const TEST_SEED = crypto.randomBytes(12).toString('hex').toLowerCase();
33+
const TEST_SEED_UPPER = TEST_SEED.toUpperCase();
34+
const TEST_FUNCTION_NAME = `unit-${TEST_SEED}`;
3335

3436
test(
3537
'lifecycle',
3638
{
3739
concurrency: true,
38-
skip: skipIfMissingEnv('TEST_PROJECT_ID', 'TEST_LOCATION'),
40+
skip: skipIfMissingEnv('TEST_AUTHENTICATED'),
3941
},
4042
async (suite) => {
4143
// Always try to delete the function
@@ -52,7 +54,7 @@ test(
5254
}
5355
});
5456

55-
suite.test('can create, read, update, and delete', async () => {
57+
await suite.test('can create, read, update, and delete', async () => {
5658
const secret = new SecretName(TEST_SECRET_VERSION_NAME);
5759

5860
const client = new CloudFunctionsClient({
@@ -75,39 +77,50 @@ test(
7577
name: TEST_FUNCTION_NAME,
7678
description: 'test function',
7779
environment: Environment.GEN_2,
78-
labels: { key1: 'value1', key2: 'value2' },
80+
labels: {
81+
[`label1-${TEST_SEED}`]: `value1_${TEST_SEED}`,
82+
[`label2-${TEST_SEED}`]: `value2_${TEST_SEED}`,
83+
},
7984

8085
buildConfig: {
8186
runtime: 'nodejs22',
8287
entryPoint: 'helloWorld',
83-
source: sourceUploadResp,
84-
environmentVariables: { BUILDKEY1: 'VALUE1', BUILDKEY2: 'VALUE2' },
88+
source: {
89+
storageSource: sourceUploadResp.storageSource,
90+
},
91+
environmentVariables: {
92+
[`BUILD_ENV_KEY1_${TEST_SEED_UPPER}`]: `VALUE1_${TEST_SEED}`,
93+
[`BUILD_ENV_KEY2_${TEST_SEED_UPPER}`]: `VALUE2_${TEST_SEED}`,
94+
},
8595
},
8696

8797
serviceConfig: {
8898
allTrafficOnLatestRevision: true,
8999
availableCpu: '1',
90-
availableMemory: '512m',
91-
environmentVariables: { KEY1: 'VALUE1', KEY2: 'VALUE2' },
100+
availableMemory: '512Mi',
101+
environmentVariables: {
102+
[`SERVICE_ENV_KEY1_${TEST_SEED_UPPER}`]: `VALUE1_${TEST_SEED}`,
103+
[`SERVICE_ENV_KEY2_${TEST_SEED_UPPER}`]: `VALUE2_${TEST_SEED}`,
104+
},
92105
ingressSettings: IngressSettings.ALLOW_ALL,
93106
maxInstanceCount: 5,
94107
minInstanceCount: 2,
95108
secretEnvironmentVariables: [
96109
{
97-
key: 'SECRET1',
110+
key: `SECRET1_${TEST_SEED_UPPER}`,
98111
projectId: secret.project,
99112
secret: secret.name,
100113
version: secret.version,
101114
},
102115
],
103116
secretVolumes: [
104117
{
105-
mountPath: '/etc/secrets/one',
118+
mountPath: `/etc/secrets/one_${TEST_SEED}`,
106119
projectId: secret.project,
107120
secret: secret.name,
108121
versions: [
109122
{
110-
path: '/value1',
123+
path: 'value1',
111124
version: secret.version,
112125
},
113126
],
@@ -119,45 +132,55 @@ test(
119132
};
120133

121134
// Create
122-
const createResp = await client.create(cf);
135+
const createResp = await client.create(cf, {
136+
onDebug: (f) => {
137+
process.stdout.write('\n\n\n\n');
138+
process.stdout.write(f());
139+
process.stdout.write('\n\n\n\n');
140+
},
141+
});
123142
assert.ok(createResp?.url);
124143

125144
// Read
126145
const getResp = await client.get(cf.name);
127146
assert.ok(getResp.name.endsWith(TEST_FUNCTION_NAME)); // The response is the fully-qualified name
128147
assert.deepStrictEqual(getResp.description, 'test function');
129-
assert.deepStrictEqual(getResp.labels, { key1: 'value1', key2: 'value2' });
130-
assert.deepStrictEqual(getResp.buildConfig.runtime, 'nodejs20');
148+
assert.deepStrictEqual(getResp.labels, {
149+
[`label1-${TEST_SEED}`]: `value1_${TEST_SEED}`,
150+
[`label2-${TEST_SEED}`]: `value2_${TEST_SEED}`,
151+
});
152+
assert.deepStrictEqual(getResp.buildConfig.runtime, 'nodejs22');
131153
assert.deepStrictEqual(getResp.buildConfig.environmentVariables, {
132-
BUILDKEY1: 'VALUE1',
133-
BUILDKEY2: 'VALUE2',
154+
[`BUILD_ENV_KEY1_${TEST_SEED_UPPER}`]: `VALUE1_${TEST_SEED}`,
155+
[`BUILD_ENV_KEY2_${TEST_SEED_UPPER}`]: `VALUE2_${TEST_SEED}`,
134156
});
135157
assert.deepStrictEqual(getResp.buildConfig.entryPoint, 'helloWorld');
136-
assert.deepStrictEqual(getResp.serviceConfig.availableCpu, 1);
137-
assert.deepStrictEqual(getResp.serviceConfig.availableMemory, 512);
158+
assert.deepStrictEqual(getResp.serviceConfig.availableCpu, '1');
159+
assert.deepStrictEqual(getResp.serviceConfig.availableMemory, '512Mi');
138160
assert.deepStrictEqual(getResp.serviceConfig.environmentVariables, {
139-
KEY1: 'VALUE1',
140-
KEY2: 'VALUE2',
161+
LOG_EXECUTION_ID: 'true', // inserted by GCP
162+
[`SERVICE_ENV_KEY1_${TEST_SEED_UPPER}`]: `VALUE1_${TEST_SEED}`,
163+
[`SERVICE_ENV_KEY2_${TEST_SEED_UPPER}`]: `VALUE2_${TEST_SEED}`,
141164
});
142165
assert.deepStrictEqual(getResp.serviceConfig.ingressSettings, 'ALLOW_ALL');
143166
assert.deepStrictEqual(getResp.serviceConfig.maxInstanceCount, 5);
144167
assert.deepStrictEqual(getResp.serviceConfig.minInstanceCount, 2);
145168
assert.deepStrictEqual(getResp.serviceConfig.secretEnvironmentVariables, [
146169
{
147-
key: 'SECRET1',
170+
key: `SECRET1_${TEST_SEED_UPPER}`,
148171
projectId: secret.project,
149172
secret: secret.name,
150173
version: secret.version,
151174
},
152175
]);
153176
assert.deepStrictEqual(getResp.serviceConfig.secretVolumes, [
154177
{
155-
mountPath: '/etc/secrets/one',
178+
mountPath: `/etc/secrets/one_${TEST_SEED}`,
156179
projectId: secret.project,
157180
secret: secret.name,
158181
versions: [
159182
{
160-
path: '/value1',
183+
path: 'value1',
161184
version: secret.version,
162185
},
163186
],
@@ -175,38 +198,49 @@ test(
175198
const cf2: CloudFunction = {
176199
name: TEST_FUNCTION_NAME,
177200
description: 'test function2',
178-
labels: { key3: 'value3', key4: 'value4' },
201+
labels: {
202+
[`label3-${TEST_SEED}`]: `value3_${TEST_SEED}`,
203+
[`label4-${TEST_SEED}`]: `value4_${TEST_SEED}`,
204+
},
179205

180206
buildConfig: {
181207
runtime: 'nodejs20',
182208
entryPoint: 'helloWorld',
183-
source: sourceUploadUpdateResp,
184-
environmentVariables: { BUILDKEY3: 'VALUE3', BUILDKEY4: 'VALUE4' },
209+
source: {
210+
storageSource: sourceUploadResp.storageSource,
211+
},
212+
environmentVariables: {
213+
[`BUILD_ENV_KEY3_${TEST_SEED_UPPER}`]: `VALUE3_${TEST_SEED}`,
214+
[`BUILD_ENV_KEY4_${TEST_SEED_UPPER}`]: `VALUE4_${TEST_SEED}`,
215+
},
185216
},
186217

187218
serviceConfig: {
188219
allTrafficOnLatestRevision: true,
189-
availableMemory: '256Mi',
190-
environmentVariables: { KEY3: 'VALUE3', KEY4: 'VALUE4' },
220+
availableMemory: '1Gi',
221+
environmentVariables: {
222+
[`SERVICE_ENV_KEY3_${TEST_SEED_UPPER}`]: `VALUE3_${TEST_SEED}`,
223+
[`SERVICE_ENV_KEY4_${TEST_SEED_UPPER}`]: `VALUE4_${TEST_SEED}`,
224+
},
191225
ingressSettings: IngressSettings.ALLOW_INTERNAL_AND_GCLB,
192226
maxInstanceCount: 3,
193227
minInstanceCount: 1,
194228
secretEnvironmentVariables: [
195229
{
196-
key: 'SECRET2',
230+
key: `SECRET2_${TEST_SEED_UPPER}`,
197231
projectId: secret.project,
198232
secret: secret.name,
199233
version: secret.version,
200234
},
201235
],
202236
secretVolumes: [
203237
{
204-
mountPath: '/etc/secrets/two',
238+
mountPath: `/etc/secrets/two_${TEST_SEED}`,
205239
projectId: secret.project,
206240
secret: secret.name,
207241
versions: [
208242
{
209-
path: '/value2',
243+
path: 'value2',
210244
version: secret.version,
211245
},
212246
],
@@ -217,40 +251,50 @@ test(
217251
},
218252
};
219253

220-
const patchResp = await client.patch(cf2);
254+
const patchResp = await client.patch(cf2, {
255+
onDebug: (f) => {
256+
process.stdout.write('\n\n\n\n');
257+
process.stdout.write(f());
258+
process.stdout.write('\n\n\n\n');
259+
},
260+
});
221261
assert.ok(patchResp.name.endsWith(TEST_FUNCTION_NAME)); // The response is the fully-qualified name
222262
assert.deepStrictEqual(patchResp.description, 'test function2');
223-
assert.deepStrictEqual(patchResp.labels, { key3: 'value3', key4: 'value4' });
263+
assert.deepStrictEqual(patchResp.labels, {
264+
[`label3-${TEST_SEED}`]: `value3_${TEST_SEED}`,
265+
[`label4-${TEST_SEED}`]: `value4_${TEST_SEED}`,
266+
});
224267
assert.deepStrictEqual(patchResp.buildConfig.runtime, 'nodejs20');
225268
assert.deepStrictEqual(patchResp.buildConfig.entryPoint, 'helloWorld');
226269
assert.deepStrictEqual(patchResp.buildConfig.environmentVariables, {
227-
BUILDKEY3: 'VALUE3',
228-
BUILDKEY4: 'VALUE4',
270+
[`BUILD_ENV_KEY3_${TEST_SEED_UPPER}`]: `VALUE3_${TEST_SEED}`,
271+
[`BUILD_ENV_KEY4_${TEST_SEED_UPPER}`]: `VALUE4_${TEST_SEED}`,
229272
});
230-
assert.deepStrictEqual(patchResp.serviceConfig.availableMemory, '256');
273+
assert.deepStrictEqual(patchResp.serviceConfig.availableMemory, '1Gi');
231274
assert.deepStrictEqual(patchResp.serviceConfig.environmentVariables, {
232-
KEY3: 'VALUE3',
233-
KEY4: 'VALUE4',
275+
LOG_EXECUTION_ID: 'true', // inserted by GCP
276+
[`SERVICE_ENV_KEY3_${TEST_SEED_UPPER}`]: `VALUE3_${TEST_SEED}`,
277+
[`SERVICE_ENV_KEY4_${TEST_SEED_UPPER}`]: `VALUE4_${TEST_SEED}`,
234278
});
235279
assert.deepStrictEqual(patchResp.serviceConfig.ingressSettings, 'ALLOW_INTERNAL_AND_GCLB');
236280
assert.deepStrictEqual(patchResp.serviceConfig.maxInstanceCount, 3);
237281
assert.deepStrictEqual(patchResp.serviceConfig.minInstanceCount, 1);
238282
assert.deepStrictEqual(patchResp.serviceConfig.secretEnvironmentVariables, [
239283
{
240-
key: 'SECRET2',
284+
key: `SECRET2_${TEST_SEED_UPPER}`,
241285
projectId: secret.project,
242286
secret: secret.name,
243287
version: secret.version,
244288
},
245289
]);
246290
assert.deepStrictEqual(patchResp.serviceConfig.secretVolumes, [
247291
{
248-
mountPath: '/etc/secrets/two',
292+
mountPath: `/etc/secrets/two_${TEST_SEED}`,
249293
projectId: secret.project,
250294
secret: secret.name,
251295
versions: [
252296
{
253-
path: '/value2',
297+
path: 'value2',
254298
version: secret.version,
255299
},
256300
],

0 commit comments

Comments
 (0)