Skip to content
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

fix(apisix): store stream route name correctly #156

Merged
merged 7 commits into from
Jul 20, 2024
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
16 changes: 10 additions & 6 deletions apps/cli/src/linter/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,7 @@ const serviceSchema = z

upstream: upstreamSchema.optional(),
plugins: pluginsSchema.optional(),
path_prefix: z
.string()
.optional()
.refine((val) => val?.startsWith('/'), {
message: 'Path prefix must start with "/"',
}),
path_prefix: z.string().optional(),
strip_path_prefix: z.boolean().optional(),
hosts: z.array(z.string()).optional(),

Expand All @@ -237,6 +232,15 @@ const serviceSchema = z
message:
'HTTP routes and Stream routes are mutually exclusive and should not exist in the same service',
},
)
.refine(
(val) => {
if (!val.path_prefix) return true;
return val.path_prefix.startsWith('/');
},
{
message: 'Path prefix must start with "/"',
},
);

const sslSchema = z
Expand Down
5 changes: 5 additions & 0 deletions libs/backend-apisix/e2e/assets/apisix_conf/http.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ apisix:
control:
ip: "0.0.0.0"
port: 9092
proxy_mode: http&stream
stream_proxy:
tcp:
- addr: 33060
tls: true
deployment:
admin:
allow_admin:
Expand Down
63 changes: 63 additions & 0 deletions libs/backend-apisix/e2e/sync-and-dump-1.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,69 @@ describe('Sync and Dump - 1', () => {
});
});

describe('Sync and dump service with stream route', () => {
const serviceName = 'test';
const service = {
name: serviceName,
upstream: {
scheme: 'tcp',
nodes: [
{
host: '1.1.1.1',
port: 5432,
weight: 100,
},
],
},
} as ADCSDK.Service;
const route1Name = 'postgres';
const route1 = {
name: route1Name,
server_port: 54320,
} as ADCSDK.StreamRoute;

it('Create resources', async () =>
syncEvents(backend, [
createEvent(ADCSDK.ResourceType.SERVICE, serviceName, service),
createEvent(
ADCSDK.ResourceType.STREAM_ROUTE,
route1Name,
route1,
serviceName,
),
]));

it('Dump', async () => {
const result = (await dumpConfiguration(backend)) as ADCSDK.Configuration;
expect(result.services).toHaveLength(1);
expect(result.services[0]).toMatchObject(service);
expect(result.services[0].stream_routes).toHaveLength(1);
expect(result.services[0].stream_routes[0]).toMatchObject(route1);
});

it('Delete stream route', async () =>
syncEvents(backend, [
deleteEvent(ADCSDK.ResourceType.STREAM_ROUTE, route1Name, serviceName),
]));

it('Dump again (non-route)', async () => {
const result = (await dumpConfiguration(backend)) as ADCSDK.Configuration;
expect(result.services).toHaveLength(1);
expect(result.services[0]).toMatchObject(service);
expect(result.services[0].stream_routes).toBeUndefined();
});

it('Delete service', async () =>
syncEvents(backend, [
deleteEvent(ADCSDK.ResourceType.SERVICE, serviceName),
]));

it('Dump again (service should not exist)', async () => {
const result = (await dumpConfiguration(backend)) as ADCSDK.Configuration;
expect(result.services).toHaveLength(0);
});
});

describe('Sync and dump consumers', () => {
const consumer1Name = 'consumer1';
const consumer1 = {
Expand Down
31 changes: 25 additions & 6 deletions libs/backend-apisix/src/transformer.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
import * as ADCSDK from '@api7/adc-sdk';
import { isEmpty, unset } from 'lodash';
import { filter, isEmpty, unset } from 'lodash';

import * as typing from './typing';

export class ToADC {
private static transformLabels(labels?: ADCSDK.Labels): ADCSDK.Labels {
if (!labels) return undefined;
const filteredLabels = filter(
labels,
(val, key) => key !== '__ADC_NAME',
) as unknown as ADCSDK.Labels;
return Object.values(filteredLabels).length > 0
? filteredLabels
: undefined;
}

public transformRoute(route: typing.Route): ADCSDK.Route {
return ADCSDK.utils.recursiveOmitUndefined({
name: route.name ?? route.id,
Expand Down Expand Up @@ -120,9 +131,9 @@ export class ToADC {
streamRoute: typing.StreamRoute,
): ADCSDK.StreamRoute {
return ADCSDK.utils.recursiveOmitUndefined({
name: streamRoute.name ?? streamRoute.id,
name: streamRoute.labels?.__ADC_NAME ?? streamRoute.id,
description: streamRoute.desc,
labels: streamRoute.labels,
labels: ToADC.transformLabels(streamRoute.labels),

remote_addr: streamRoute.remote_addr,
server_addr: streamRoute.server_addr,
Expand Down Expand Up @@ -343,10 +354,18 @@ export class FromADC {
streamRoute: ADCSDK.StreamRoute,
): typing.StreamRoute {
return ADCSDK.utils.recursiveOmitUndefined({
...streamRoute,
id: undefined,
labels: FromADC.transformLabels(streamRoute.labels),
});
desc: streamRoute.description,
labels: {
...FromADC.transformLabels(streamRoute.labels),
__ADC_NAME: streamRoute.name,
},
plugins: streamRoute.plugins,
remote_addr: streamRoute.remote_addr,
server_addr: streamRoute.server_addr,
server_port: streamRoute.server_port,
sni: streamRoute.sni,
} as typing.StreamRoute);
}

public transformUpstream(upstream: ADCSDK.Upstream): typing.Upstream {
Expand Down
2 changes: 1 addition & 1 deletion libs/backend-apisix/src/typing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export interface StreamRoute {
id: string;
desc?: string;
labels?: Labels;
name: string;
//name: string; // As of 3.9.1, APISIX does not support name on the stream route
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove this unused value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I will add it to APISIX. We can use it later.


remote_addr?: string;
server_addr?: string;
Expand Down
6 changes: 4 additions & 2 deletions schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@
"additionalProperties": false
}
},
"required": [
"active"
],
"additionalProperties": false
},
"nodes": {
Expand Down Expand Up @@ -538,8 +541,7 @@
}
},
"required": [
"name",
"path_prefix"
"name"
],
"additionalProperties": false
}
Expand Down
Loading