Skip to content

Commit fd7b922

Browse files
authored
fix(backend-api7): duplicate removal of upstream and credentials (#343)
1 parent 555ab97 commit fd7b922

File tree

5 files changed

+134
-79
lines changed

5 files changed

+134
-79
lines changed

libs/backend-api7/e2e/resources/service-upstream.e2e-spec.ts

Lines changed: 65 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
1+
import { Differ } from '@api7/adc-differ';
12
import * as ADCSDK from '@api7/adc-sdk';
3+
import { unset } from 'lodash';
24
import { gte } from 'semver';
35

46
import { BackendAPI7 } from '../../src';
57
import {
68
conditionalDescribe,
7-
createEvent,
8-
deleteEvent,
99
dumpConfiguration,
1010
semverCondition,
1111
sortResult,
1212
syncEvents,
13-
updateEvent,
1413
} from '../support/utils';
1514

1615
conditionalDescribe(semverCondition(gte, '3.5.0'))(
@@ -20,30 +19,15 @@ conditionalDescribe(semverCondition(gte, '3.5.0'))(
2019

2120
beforeAll(() => {
2221
backend = new BackendAPI7({
23-
server: process.env.SERVER,
24-
token: process.env.TOKEN,
22+
server: process.env.SERVER!,
23+
token: process.env.TOKEN!,
2524
tlsSkipVerify: true,
2625
gatewayGroup: process.env.GATEWAY_GROUP,
26+
cacheKey: 'e2e-service-upstream',
2727
});
2828
});
2929

3030
describe('Service multiple upstreams', () => {
31-
const serviceName = 'test';
32-
const service = {
33-
name: serviceName,
34-
upstream: {
35-
type: 'roundrobin',
36-
nodes: [
37-
{
38-
host: 'httpbin.org',
39-
port: 443,
40-
weight: 100,
41-
},
42-
],
43-
},
44-
path_prefix: '/test',
45-
strip_path_prefix: true,
46-
} satisfies ADCSDK.Service;
4731
const upstreamND1Name = 'nd-upstream1';
4832
const upstreamND1 = {
4933
name: upstreamND1Name,
@@ -70,31 +54,42 @@ conditionalDescribe(semverCondition(gte, '3.5.0'))(
7054
},
7155
],
7256
} satisfies ADCSDK.Upstream;
57+
const serviceName = 'test';
58+
const service = {
59+
name: serviceName,
60+
upstream: {
61+
type: 'roundrobin',
62+
nodes: [
63+
{
64+
host: 'httpbin.org',
65+
port: 443,
66+
weight: 100,
67+
},
68+
],
69+
},
70+
path_prefix: '/test',
71+
strip_path_prefix: true,
72+
upstreams: [upstreamND1, upstreamND2],
73+
} satisfies ADCSDK.Service;
7374

7475
it('Create service and upstreams', async () =>
75-
syncEvents(backend, [
76-
createEvent(ADCSDK.ResourceType.SERVICE, serviceName, service),
77-
createEvent(
78-
ADCSDK.ResourceType.UPSTREAM,
79-
upstreamND1Name,
80-
upstreamND1,
81-
serviceName,
76+
syncEvents(
77+
backend,
78+
Differ.diff(
79+
{ services: [service] },
80+
await dumpConfiguration(backend),
8281
),
83-
createEvent(
84-
ADCSDK.ResourceType.UPSTREAM,
85-
upstreamND2Name,
86-
upstreamND2,
87-
serviceName,
88-
),
89-
]));
82+
));
9083

9184
it('Dump', async () => {
9285
const result = await dumpConfiguration(backend);
9386
expect(result.services).toHaveLength(1);
94-
expect(result.services[0]).toMatchObject(service);
95-
expect(result.services[0].upstreams).toHaveLength(2);
87+
const testService = service;
88+
unset(testService, 'upstreams');
89+
expect(result.services![0]).toMatchObject(testService);
90+
expect(result.services![0].upstreams).toHaveLength(2);
9691

97-
const upstreams = sortResult(result.services[0].upstreams, 'name');
92+
const upstreams = sortResult(result.services![0].upstreams!, 'name');
9893
expect(upstreams[0]).toMatchObject(upstreamND1);
9994
expect(upstreams[1]).toMatchObject(upstreamND2);
10095
});
@@ -104,43 +99,55 @@ conditionalDescribe(semverCondition(gte, '3.5.0'))(
10499
retry_timeout: 100,
105100
} as ADCSDK.Upstream;
106101
it('Update service non-default upstream 1', async () =>
107-
syncEvents(backend, [
108-
updateEvent(
109-
ADCSDK.ResourceType.UPSTREAM,
110-
upstreamND1Name,
111-
newUpstreamND1,
112-
serviceName,
102+
syncEvents(
103+
backend,
104+
Differ.diff(
105+
{
106+
services: [
107+
{
108+
...service,
109+
upstreams: [newUpstreamND1, upstreamND2],
110+
},
111+
],
112+
},
113+
await dumpConfiguration(backend),
113114
),
114-
]));
115+
));
115116

116117
it('Dump (updated non-default upstream 1)', async () => {
117118
const result = await dumpConfiguration(backend);
118119
expect(result.services).toHaveLength(1);
119120

120-
const upstreams = sortResult(result.services[0].upstreams, 'name');
121+
const upstreams = sortResult(result.services![0].upstreams!, 'name');
121122
expect(upstreams[0]).toMatchObject(newUpstreamND1);
122123
});
123124

124-
it('Delete non-default upstream 2', async () =>
125-
syncEvents(backend, [
126-
deleteEvent(
127-
ADCSDK.ResourceType.UPSTREAM,
128-
upstreamND2Name,
129-
serviceName,
125+
it('Delete non-default upstream 2', async () => {
126+
await syncEvents(
127+
backend,
128+
Differ.diff(
129+
{
130+
services: [
131+
{
132+
...service,
133+
upstreams: [newUpstreamND1],
134+
},
135+
],
136+
},
137+
await dumpConfiguration(backend),
130138
),
131-
]));
139+
);
140+
});
132141

133142
it('Dump (non-default upstream 2 should not exist)', async () => {
134143
const result = await dumpConfiguration(backend);
135144
expect(result.services).toHaveLength(1);
136-
expect(result.services[0].upstreams).toHaveLength(1);
137-
expect(result.services[0].upstreams[0]).toMatchObject(newUpstreamND1);
145+
expect(result.services![0].upstreams).toHaveLength(1);
146+
expect(result.services![0].upstreams![0]).toMatchObject(newUpstreamND1);
138147
});
139148

140149
it('Delete', async () =>
141-
syncEvents(backend, [
142-
deleteEvent(ADCSDK.ResourceType.SERVICE, serviceName),
143-
]));
150+
syncEvents(backend, Differ.diff({}, await dumpConfiguration(backend))));
144151

145152
it('Dump again (service should not exist)', async () => {
146153
const result = await dumpConfiguration(backend);

libs/backend-api7/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
}
1010
},
1111
"devDependencies": {
12-
"@api7/adc-sdk": "workspace:*"
12+
"@api7/adc-sdk": "workspace:*",
13+
"@api7/adc-differ": "workspace:*"
1314
},
1415
"nx": {
1516
"name": "backend-api7",

libs/backend-api7/src/operator.ts

Lines changed: 63 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
catchError,
1212
concatMap,
1313
filter,
14+
forkJoin,
1415
from,
1516
map,
1617
mergeMap,
@@ -133,39 +134,83 @@ export class Operator extends ADCSDK.backend.BackendEventSource {
133134
);
134135

135136
const event$ = from(events);
136-
return event$.pipe(
137+
return forkJoin({
137138
// Aggregate services that need to be deleted
138-
filter(
139-
(event) =>
140-
event.resourceType === ADCSDK.ResourceType.SERVICE &&
141-
event.type === ADCSDK.EventType.DELETE,
139+
deletedServiceIds: event$.pipe(
140+
filter(
141+
(event) =>
142+
event.resourceType === ADCSDK.ResourceType.SERVICE &&
143+
event.type === ADCSDK.EventType.DELETE,
144+
),
145+
map((event) => event.resourceId),
146+
toArray(),
147+
),
148+
// Aggregate consumers that need to be deleted
149+
deletedConsumerIds: event$.pipe(
150+
filter(
151+
(event) =>
152+
event.resourceType === ADCSDK.ResourceType.CONSUMER &&
153+
event.type === ADCSDK.EventType.DELETE,
154+
),
155+
map((event) => event.resourceId),
156+
toArray(),
142157
),
143-
map((event) => event.resourceId),
144-
toArray(),
158+
}).pipe(
145159
// Switch to a new event pipe for event filtering and grouping.
146160
// It will use the deleted service ID that has been aggregated.
147-
switchMap((deletedServiceIds) =>
161+
switchMap(({ deletedServiceIds, deletedConsumerIds }) =>
148162
event$.pipe(
149-
// If an event wants to delete a route, but its parent service
150-
// will also be deleted, this operation can be ignored.
151-
// The deletion service will cascade the deletion of the route.
152163
filter(
153164
(event) =>
165+
// If an event wants to delete a route, but its parent service
166+
// will also be deleted, this operation can be ignored.
167+
// The deletion service will cascade the deletion of the route.
154168
!(
155169
isRouteLike(event) &&
156170
event.type === ADCSDK.EventType.DELETE &&
171+
event.parentId &&
172+
deletedServiceIds.includes(event.parentId)
173+
) &&
174+
// If an event wants to delete an upstream, but its parent service
175+
// will also be deleted, this operation can be ignored.
176+
// The deletion service will cascade the deletion of the upstream.
177+
// This does not affect inline upstream processing, which is always
178+
// handled on the server side.
179+
!(
180+
event.resourceType === ADCSDK.ResourceType.UPSTREAM &&
181+
event.type === ADCSDK.EventType.DELETE &&
182+
event.parentId &&
157183
deletedServiceIds.includes(event.parentId)
158184
),
159185
),
186+
// If an event wants to delete a consumer credential, but its parent
187+
// consumer will also be deleted, this operation can be ignored.
188+
// The deletion consumer will cascade the deletion of the credential.
189+
filter(
190+
(event) =>
191+
!(
192+
event.resourceType ===
193+
ADCSDK.ResourceType.CONSUMER_CREDENTIAL &&
194+
event.type === ADCSDK.EventType.DELETE &&
195+
event.parentId &&
196+
deletedConsumerIds.includes(event.parentId)
197+
),
198+
),
160199
// Grouping events by resource type and operation type.
161200
// The sequence of events should not be broken in this process,
162201
// and the correct behavior of the API will depend on the order
163202
// of execution.
164-
reduce((groups, event) => {
165-
const key = `${event.resourceType}.${event.type}`;
166-
(groups[key] = groups[key] || []).push(event);
167-
return groups;
168-
}, {}),
203+
reduce(
204+
(groups, event) => {
205+
const key = `${event.resourceType}.${event.type}` as const;
206+
(groups[key] ??= []).push(event);
207+
return groups;
208+
},
209+
{} as Record<
210+
`${ADCSDK.ResourceType}.${ADCSDK.EventType}`,
211+
Array<ADCSDK.Event>
212+
>,
213+
),
169214
// Strip group name and convert to two-dims arrays
170215
// {"service.create": [1], "consumer.create": [2]} => [[1], [2]]
171216
mergeMap<
@@ -208,13 +253,13 @@ export class Operator extends ADCSDK.backend.BackendEventSource {
208253
(event.newValue as ADCSDK.Route).id = event.resourceId;
209254
return fromADC.transformRoute(
210255
event.newValue as ADCSDK.Route,
211-
event.parentId,
256+
event.parentId!,
212257
);
213258
case ADCSDK.ResourceType.STREAM_ROUTE:
214259
(event.newValue as ADCSDK.StreamRoute).id = event.resourceId;
215260
return fromADC.transformStreamRoute(
216261
event.newValue as ADCSDK.StreamRoute,
217-
event.parentId,
262+
event.parentId!,
218263
);
219264
case ADCSDK.ResourceType.SSL:
220265
(event.newValue as ADCSDK.SSL).id = event.resourceId;
@@ -225,7 +270,6 @@ export class Operator extends ADCSDK.backend.BackendEventSource {
225270
event.newValue as ADCSDK.ConsumerCredential,
226271
);
227272
case ADCSDK.ResourceType.UPSTREAM:
228-
//(event.newValue as ADCSDK.ConsumerCredential).id = event.resourceId;
229273
return fromADC.transformUpstream(event.newValue as ADCSDK.Upstream);
230274
}
231275
}

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
"yaml": "^2.4.2",
7272
"zod": "^4.0.10"
7373
},
74-
"packageManager": "pnpm@10.15.0+sha512.486ebc259d3e999a4e8691ce03b5cac4a71cbeca39372a9b762cb500cfdf0873e2cb16abe3d951b1ee2cf012503f027b98b6584e4df22524e0c7450d9ec7aa7b",
74+
"packageManager": "pnpm@10.17.1+sha512.17c560fca4867ae9473a3899ad84a88334914f379be46d455cbf92e5cf4b39d34985d452d2583baf19967fa76cb5c17bc9e245529d0b98745721aa7200ecaf7a",
7575
"pnpm": {
7676
"onlyBuiltDependencies": [
7777
"@parcel/watcher",

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)