-
Notifications
You must be signed in to change notification settings - Fork 10
feat(apisix-standalone): separate inline upstream #319
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import { DifferV3 } from '@api7/adc-differ'; | ||
import * as ADCSDK from '@api7/adc-sdk'; | ||
|
||
import { BackendAPISIXStandalone } from '../../src'; | ||
import { | ||
config as configCache, | ||
rawConfig as rawConfigCache, | ||
} from '../../src/cache'; | ||
import { server1, token1 } from '../support/constants'; | ||
import { dumpConfiguration, restartAPISIX, syncEvents } from '../support/utils'; | ||
|
||
const cacheKey = 'default'; | ||
describe('Service E2E - inline upstream', () => { | ||
let backend: BackendAPISIXStandalone; | ||
|
||
beforeAll(async () => { | ||
await restartAPISIX(); | ||
backend = new BackendAPISIXStandalone({ | ||
server: server1, | ||
token: token1, | ||
cacheKey, | ||
}); | ||
}); | ||
|
||
it('Initialize cache', () => | ||
expect(dumpConfiguration(backend)).resolves.not.toThrow()); | ||
|
||
const serviceName = 'test'; | ||
it('Create service', async () => { | ||
const events = DifferV3.diff( | ||
{ | ||
services: [ | ||
{ | ||
name: serviceName, | ||
upstream: { | ||
nodes: [ | ||
{ | ||
host: '127.0.0.1', | ||
port: 9180, | ||
weight: 100, | ||
}, | ||
], | ||
}, | ||
}, | ||
], | ||
}, | ||
await dumpConfiguration(backend), | ||
); | ||
return syncEvents(backend, events); | ||
}); | ||
|
||
it('Check configuration', () => { | ||
expect(rawConfigCache.get(cacheKey)?.upstreams).not.toBeUndefined(); | ||
expect(rawConfigCache.get(cacheKey)!.upstreams).toHaveLength(1); | ||
expect(rawConfigCache.get(cacheKey)!.upstreams![0].id).toEqual( | ||
ADCSDK.utils.generateId(serviceName), | ||
); | ||
expect(rawConfigCache.get(cacheKey)!.upstreams![0].name).toEqual( | ||
serviceName, | ||
); | ||
|
||
expect(configCache.get(cacheKey)?.services).not.toBeUndefined(); | ||
expect( | ||
configCache.get(cacheKey)?.services![0].upstream, | ||
).not.toBeUndefined(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,3 +1,4 @@ | ||||||
import { DifferV3 } from '@api7/adc-differ'; | ||||||
import * as ADCSDK from '@api7/adc-sdk'; | ||||||
import axios, { | ||||||
type AxiosError, | ||||||
|
@@ -68,19 +69,35 @@ export class Operator extends ADCSDK.backend.BackendEventSource { | |||||
? ADCSDK.ResourceType.CONSUMER | ||||||
: (event.resourceType as typing.UsedResourceTypes); | ||||||
const resourceKey = typing.APISIXStandaloneKeyMap[resourceType]; | ||||||
const upstreamResourceKey = | ||||||
typing.APISIXStandaloneKeyMap[ADCSDK.ResourceType.UPSTREAM]; | ||||||
|
||||||
if (event.type === ADCSDK.EventType.CREATE) { | ||||||
if (!newConfig[resourceKey]) newConfig[resourceKey] = []; | ||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- infer error | ||||||
newConfig[resourceKey].push(this.fromADC(event, timestamp) as any); | ||||||
increaseVersion[resourceType] = true; | ||||||
|
||||||
// Emit inline upstream | ||||||
if (event.resourceType === ADCSDK.ResourceType.SERVICE) { | ||||||
if (!newConfig[upstreamResourceKey]) | ||||||
newConfig[upstreamResourceKey] = []; | ||||||
newConfig[upstreamResourceKey].push({ | ||||||
...this.fromADCUpstream( | ||||||
(event.newValue as ADCSDK.Service).upstream!, | ||||||
), | ||||||
id: event.resourceId, | ||||||
modifiedIndex: timestamp, | ||||||
name: event.resourceName, | ||||||
}); | ||||||
increaseVersion[ADCSDK.ResourceType.UPSTREAM] = true; | ||||||
} | ||||||
} else if ( | ||||||
event.type === ADCSDK.EventType.UPDATE || | ||||||
event.type === ADCSDK.EventType.DELETE | ||||||
) { | ||||||
/* eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- | ||||||
* Since this resource should be updated, it must already exist in the old configuration */ | ||||||
const resources = newConfig[resourceKey]!; | ||||||
if (!newConfig[resourceKey]) newConfig[resourceKey] = []; | ||||||
const resources = newConfig[resourceKey]; | ||||||
const eventGeneratedId = this.generateIdFromEvent(event); | ||||||
const index = resources?.findIndex((item) => | ||||||
'id' in item ? item.id : item.username === eventGeneratedId, | ||||||
|
@@ -97,6 +114,60 @@ export class Operator extends ADCSDK.backend.BackendEventSource { | |||||
} | ||||||
increaseVersion[resourceType] = true; | ||||||
} | ||||||
|
||||||
// Emit inline upstream | ||||||
if (event.resourceType === ADCSDK.ResourceType.SERVICE) { | ||||||
if (event.type === ADCSDK.EventType.UPDATE) { | ||||||
const baseUpstream = { | ||||||
id: event.resourceId, | ||||||
name: event.resourceName, | ||||||
}; | ||||||
const newUpstream = (event.newValue as ADCSDK.Service)?.upstream; | ||||||
const oldUpstream = (event.oldValue as ADCSDK.Service)?.upstream; | ||||||
const events = DifferV3.diff( | ||||||
{ | ||||||
...(newUpstream && { | ||||||
upstreams: [Object.assign(baseUpstream, newUpstream)], | ||||||
}), | ||||||
}, | ||||||
{ | ||||||
...(oldUpstream && { | ||||||
upstreams: [Object.assign(baseUpstream, oldUpstream)], | ||||||
}), | ||||||
}, | ||||||
); | ||||||
if (events.length > 0) { | ||||||
if (!newConfig[upstreamResourceKey]) | ||||||
newConfig[upstreamResourceKey] = []; | ||||||
const resources = newConfig[upstreamResourceKey]; | ||||||
const index = resources?.findIndex( | ||||||
(item) => item.id === eventGeneratedId, | ||||||
); | ||||||
if (!isNil(index) && index != -1) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
If findIndex is the only assignment method, then the value of index can only be a number. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||||||
resources[index] = { | ||||||
...this.fromADCUpstream( | ||||||
(event.newValue as ADCSDK.Service).upstream!, | ||||||
), | ||||||
id: event.resourceId, | ||||||
modifiedIndex: timestamp, | ||||||
name: event.resourceName, | ||||||
}; | ||||||
increaseVersion[ADCSDK.ResourceType.UPSTREAM] = true; | ||||||
} | ||||||
} | ||||||
} else { | ||||||
if (!newConfig[upstreamResourceKey]) | ||||||
newConfig[upstreamResourceKey] = []; | ||||||
const resources = newConfig[upstreamResourceKey]; | ||||||
const index = resources?.findIndex( | ||||||
(item) => item.id === eventGeneratedId, | ||||||
); | ||||||
if (!isNil(index) && index != -1) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
ditto |
||||||
resources.splice(index, 1); | ||||||
increaseVersion[ADCSDK.ResourceType.UPSTREAM] = true; | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
}), | ||||||
// filtering of new consumer configurations to ensure | ||||||
|
@@ -232,14 +303,15 @@ export class Operator extends ADCSDK.backend.BackendEventSource { | |||||
} | ||||||
case ADCSDK.ResourceType.SERVICE: { | ||||||
const res = event.newValue as ADCSDK.Service; | ||||||
const id = this.generateIdFromEvent(event); | ||||||
return { | ||||||
modifiedIndex, | ||||||
id: this.generateIdFromEvent(event), | ||||||
id, | ||||||
name: res.name, | ||||||
desc: res.description, | ||||||
labels: this.fromADCLabels(res.labels), | ||||||
hosts: res.hosts, | ||||||
upstream: this.fromADCUpstream(res.upstream!), | ||||||
upstream_id: id, | ||||||
plugins: res.plugins, | ||||||
} satisfies typing.Service as typing.Service; | ||||||
} | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are no other considerations, this change is more concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
newConfig
is derived from the old configuration as a deep clone, not an empty object. Therefore, the key is not necessarily empty, and it is not feasible to override it directly.