feat(sdn-controller): add patch routes for vif and network (#9936)

This commit is contained in:
Grandalf
2026-06-30 11:29:29 +02:00
committed by GitHub
parent 7bbe139b36
commit 3ad3ecb244
10 changed files with 395 additions and 87 deletions

View File

@@ -4,5 +4,6 @@ export default {
read: true,
update: {
tags: true,
other_config: true,
},
}

View File

@@ -10,5 +10,6 @@ export default {
lockingMode: true,
rateLimit: true,
txChecksumming: true,
other_config: true,
},
}

View File

@@ -16,11 +16,16 @@ export function buildOpenApiSchema(def: Record<string, FieldDefinition>): OpenAP
if (field.type === 'enum') {
property.type = 'string'
property.enum = field.enum
} else if (field.type === 'object') {
property.type = 'object'
const nested = buildOpenApiSchema(field.fields)
property.properties = nested.properties
if (nested.required?.length) property.required = nested.required
} else {
property.type = field.type
}
if (field.example !== undefined) {
if ('example' in field && field.example !== undefined) {
property.example = field.example
}

View File

@@ -278,11 +278,14 @@ function buildZodSchema(def: Record<string, FieldDefinition>): z.ZodObject<Recor
case 'enum':
schema = z.enum(field.enum as [string, ...string[]])
break
case 'object':
schema = buildZodSchema(field.fields)
break
default:
throw new Error(`Unsupported type: ${(field as { type: unknown }).type}`)
}
if (field.example) schema = schema.meta({ example: field.example })
if ('example' in field && field.example !== undefined) schema = schema.meta({ example: field.example })
if (field.optional) schema = schema.optional()

View File

@@ -48,8 +48,15 @@ export type FieldDefinition =
example?: string
optional?: boolean
}
| {
type: 'object'
fields: Record<string, FieldDefinition>
optional?: boolean
}
export type ParamFieldDefinition = Exclude<FieldDefinition, { type: 'boolean' }>
export type ParamFieldDefinition = Exclude<FieldDefinition, { type: 'boolean' } | { type: 'object' }>
export type QueryFieldDefinition = Exclude<FieldDefinition, { type: 'object' }>
export interface RouteDefinition {
method: 'get' | 'post' | 'put' | 'delete' | 'patch'
@@ -57,7 +64,7 @@ export interface RouteDefinition {
description?: string
tags?: string[]
params?: Record<string, ParamFieldDefinition>
query?: Record<string, FieldDefinition>
query?: Record<string, QueryFieldDefinition>
body?: Record<string, FieldDefinition>
responses?: Array<{
status: number

View File

@@ -12,6 +12,7 @@
> Users must be able to say: "Nice enhancement, I'm eager to test it"
- [REST API] Expose `GET /backup-repositories/:id/health` and `POST /backup-repositories/:id/actions/benchmark` routes (PR [#9847](https://github.com/vatesfr/xen-orchestra/pull/9847))
- [sdn-controller] Add `POST /rest/v0/plugins/sdn-controller/networks/:id/actions/update_traffic_rule` and `POST /rest/v0/plugins/sdn-controller/vifs/:id/actions/update_traffic_rule` (PR [#9936](https://github.com/vatesfr/xen-orchestra/pull/9936))
### Bug fixes
@@ -34,12 +35,13 @@
> Keep this list alphabetically ordered to avoid merge conflicts
<!--packages-start-->
- @vates/types minor
- @xen-orchestra/acl minor
- @xen-orchestra/backups patch
- @xen-orchestra/proxy minor
- @xen-orchestra/rest-api minor
- xo-server minor
- xo-server-sdn-controller minor
<!--packages-end-->

View File

@@ -116,12 +116,12 @@ Actions are written using the exact string you pass in a privilege. A parent act
| `vdi` | `read`, `create`, `delete`, `boot`, `export-content`, `import-content`, `migrate-send`, `update:tags` |
| `vdi-snapshot` | `read`, `delete`, `export`, `update:tags` |
| `vdi-unmanaged` | `read` |
| `vif` | `connect`, `create`, `delete`, `disconnect`, `read`, `update:allowedIpv4Addresses`, `update:allowedIpv6Addresses`, `update:lockingMode`, `update:rateLimit`, `update:txChecksumming` |
| `vif` | `connect`, `create`, `delete`, `disconnect`, `read`, `update:allowedIpv4Addresses`, `update:allowedIpv6Addresses`, `update:lockingMode`, `update:rateLimit`, `update:txChecksumming`,`update:other_config` |
| `vbd` | `read`, `create`, `delete`, `connect`, `disconnect` |
| `sr` | `read`, `delete`, `forget`, `migrate-receive`, `reclaim-space`, `scan`, `import:vdi`, `import:vm`, `update:tags` |
| `host` | `read`, `allow-vm`, `join-pool`, `export:logs`,`migrate-receive`, `update:tags`, `disable`, `enable`, `evacuate` |
| `pool` | `add-host`, `read`, `emergency-shutdown`, `rolling-reboot`, `rolling-update`, `create:network`, `create:vm`, `update:tags` |
| `network` | `read`, `create`, `delete`, `update:tags` |
| `network` | `read`, `create`, `delete`, `update:tags`,`update:other_config` |
| `pif` | `read`, `update:management` |
| `pbd` | `read`, `plug`, `unplug` |
| `pci` | `read` |

View File

@@ -30,6 +30,7 @@
"dependencies": {
"@vates/async-each": "^1.0.3",
"@vates/coalesce-calls": "^0.1.0",
"@vates/types": "^1.26.0",
"@xen-orchestra/log": "^0.7.2",
"@xen-orchestra/openflow": "^0.1.4",
"ipaddr.js": "^2.0.1",

View File

@@ -14,8 +14,8 @@ import { PrivateNetwork } from './private-network/private-network'
import { TlsHelper } from './utils/tls-helper'
import { instantiateController } from './openflow-controller'
import { randomBytes } from 'crypto'
import { invalidParameters } from 'xo-common/api-errors.js'
import { invalidParameters, noSuchObject } from 'xo-common/api-errors.js'
import { SDN_CONTROLLER_OF_RULES_KEY } from '@vates/types'
// =============================================================================
const log = createLogger('xo:xo-server-sdn-controller')
@@ -255,6 +255,28 @@ function isControllerNeeded(xapi) {
return !(controller?.protocol === PROTOCOL && controller.address === '' && controller.port === 0)
}
function validateRuleFields(obj, errors, prefix = '') {
if (!obj.direction || typeof obj.direction !== 'string') {
errors.push(`${prefix}direction is required and must be a string`)
}
if (!obj.ipRange || typeof obj.ipRange !== 'string') {
errors.push(`${prefix}ipRange is required and must be a string`)
}
if (!obj.protocol || typeof obj.protocol !== 'string') {
errors.push(`${prefix}protocol is required and must be a string`)
}
if (obj.port != null && !Number.isInteger(obj.port)) {
errors.push(`${prefix}port must be an integer`)
}
}
function validateRuleWithAllow(obj, errors, prefix = '') {
if (obj.allow === undefined || typeof obj.allow !== 'boolean') {
errors.push(`${prefix}allow is required and must be a boolean`)
}
validateRuleFields(obj, errors, prefix)
}
// =============================================================================
class SDNController extends EventEmitter {
@@ -466,25 +488,7 @@ class SDNController extends EventEmitter {
':id/actions/add_traffic_rule': {
_post: async (req, res, next) => {
const validationErrors = []
if (req.body.allow === undefined || typeof req.body.allow !== 'boolean') {
validationErrors.push('allow is required and must be a boolean')
}
if (!req.body.direction || typeof req.body.direction !== 'string') {
validationErrors.push('direction is required and must be a string')
}
if (!req.body.ipRange || typeof req.body.ipRange !== 'string') {
validationErrors.push('ipRange is required and must be a string')
}
if (!req.body.protocol || typeof req.body.protocol !== 'string') {
validationErrors.push('protocol is required and must be a string')
}
if (req.body.port && Number.isInteger(req.body.port) === false) {
validationErrors.push('port must be an integer')
}
validateRuleWithAllow(req.body, validationErrors)
if (validationErrors.length > 0) {
throw invalidParameters(validationErrors)
}
@@ -497,7 +501,7 @@ class SDNController extends EventEmitter {
networkId: req.params.id,
}
if (req.body.port) {
if (req.body.port != null) {
rule.port = req.body.port
}
@@ -514,22 +518,7 @@ class SDNController extends EventEmitter {
':id/actions/delete_traffic_rule': {
_post: async (req, res, next) => {
const validationErrors = []
if (!req.body.direction || typeof req.body.direction !== 'string') {
validationErrors.push('direction is required and must be a string')
}
if (!req.body.ipRange || typeof req.body.ipRange !== 'string') {
validationErrors.push('ipRange is required and must be a string')
}
if (!req.body.protocol || typeof req.body.protocol !== 'string') {
validationErrors.push('protocol is required and must be a string')
}
if (req.body.port && Number.isInteger(req.body.port) === false) {
validationErrors.push('port must be an integer')
}
validateRuleFields(req.body, validationErrors)
if (validationErrors.length > 0) {
throw invalidParameters(validationErrors)
}
@@ -541,7 +530,7 @@ class SDNController extends EventEmitter {
networkId: req.params.id,
}
if (req.body.port) {
if (req.body.port != null) {
rule.port = req.body.port
}
@@ -560,30 +549,7 @@ class SDNController extends EventEmitter {
':id/actions/add_traffic_rule': {
_post: async (req, res, next) => {
const validationErrors = []
if (!req.body) {
validationErrors.push('body is required')
throw invalidParameters(validationErrors)
}
if (req.body.allow === undefined || typeof req.body.allow !== 'boolean') {
validationErrors.push('allow is required and must be a boolean')
}
if (!req.body.direction || typeof req.body.direction !== 'string') {
validationErrors.push('direction is required and must be a string')
}
if (!req.body.ipRange || typeof req.body.ipRange !== 'string') {
validationErrors.push('ipRange is required and must be a string')
}
if (!req.body.protocol || typeof req.body.protocol !== 'string') {
validationErrors.push('protocol is required and must be a string')
}
if (req.body.port && Number.isInteger(req.body.port) === false) {
validationErrors.push('port must be an integer')
}
validateRuleWithAllow(req.body ?? {}, validationErrors)
if (validationErrors.length > 0) {
throw invalidParameters(validationErrors)
}
@@ -613,22 +579,7 @@ class SDNController extends EventEmitter {
':id/actions/delete_traffic_rule': {
_post: async (req, res, next) => {
const validationErrors = []
if (!req.body.direction || typeof req.body.direction !== 'string') {
validationErrors.push('direction is required and must be a string')
}
if (!req.body.ipRange || typeof req.body.ipRange !== 'string') {
validationErrors.push('ipRange is required and must be a string')
}
if (!req.body.protocol || typeof req.body.protocol !== 'string') {
validationErrors.push('protocol is required and must be a string')
}
if (req.body.port && Number.isInteger(req.body.port) === false) {
validationErrors.push('port must be an integer')
}
validateRuleFields(req.body, validationErrors)
if (validationErrors.length > 0) {
throw invalidParameters(validationErrors)
}
@@ -658,7 +609,188 @@ class SDNController extends EventEmitter {
},
'/plugins/sdn-controller'
)
this._cleaners.push(
this._xo.registerRestRoutes(
[
{
endpoint: '/networks/{id}/actions/update_traffic_rule',
description:
'Update a rule on a network, needs the exact old rule fields.\n\nRequired privilege:\n - resource: network, action: update:other_config',
method: 'post',
tags: ['sdn-controller'],
params: {
id: { type: 'string', example: 'b97f4e69-d275-4b25-9dc9-c1ac4e9b3fa5' },
},
query: {
sync: { type: 'boolean', optional: true },
},
body: {
oldRule: {
type: 'object',
fields: {
allow: { type: 'boolean', example: true },
direction: { type: 'string', example: 'to' },
ipRange: { type: 'string', example: '192.168.0.0/24' },
protocol: { type: 'string', example: 'tcp' },
port: { type: 'number', example: 443, optional: true },
},
},
newRule: {
type: 'object',
fields: {
allow: { type: 'boolean', example: true, optional: true },
direction: { type: 'string', example: 'to', optional: true },
ipRange: { type: 'string', example: '10.0.0.0/8', optional: true },
protocol: { type: 'string', example: 'tcp', optional: true },
port: { type: 'number', example: 80, optional: true },
},
},
},
responses: [
{
status: 204,
description: 'Rule updated successfully',
},
{
status: 404,
description: 'Old rule does not exist on this network',
},
],
middlewares: [
{ name: 'json' },
{ name: 'acl', acls: { resource: 'network', action: 'update:other_config', objectId: 'params.id' } },
],
callback: ({ req, createAction }) => {
return createAction(
async () => {
const { oldRule, newRule: partialNewRule } = req.body
const networkId = req.params.id
const network = this._xo.getObject(networkId, 'network')
const networkRules = JSON.parse(network.other_config[SDN_CONTROLLER_OF_RULES_KEY] || '[]').map(
JSON.parse
)
if (
!networkRules.some(
rule =>
rule.allow === oldRule.allow &&
rule.direction === oldRule.direction &&
rule.ipRange === oldRule.ipRange &&
rule.port === oldRule.port &&
rule.protocol.toLowerCase() === oldRule.protocol.toLowerCase()
)
) {
throw noSuchObject(JSON.stringify(oldRule), 'traffic-rule')
}
const newRule = { ...oldRule, ...partialNewRule }
await this._deleteNetworkOfRule({ ...oldRule, networkId })
await this._addNetworkRule({ ...newRule, networkId })
},
{
sync: req.query.sync ?? false,
statusCode: 204,
taskProperties: {
name: 'update network traffic rule',
objectId: req.params.id,
params: req.body,
objectType: 'network',
},
}
)
},
},
{
endpoint: '/vifs/{id}/actions/update_traffic_rule',
description:
'Update a rule on a VIF, needs the exact old rule fields.\n\nRequired privilege:\n - resource: vif, action: update:other_config',
method: 'post',
tags: ['sdn-controller'],
params: {
id: { type: 'string', example: 'b97f4e69-d275-4b25-9dc9-c1ac4e9b3fa5' },
},
query: {
sync: { type: 'boolean', optional: true },
},
body: {
oldRule: {
type: 'object',
fields: {
allow: { type: 'boolean', example: true },
direction: { type: 'string', example: 'from' },
ipRange: { type: 'string', example: '192.168.0.0/24' },
protocol: { type: 'string', example: 'tcp' },
port: { type: 'number', example: 443, optional: true },
},
},
newRule: {
type: 'object',
fields: {
allow: { type: 'boolean', example: true, optional: true },
direction: { type: 'string', example: 'to', optional: true },
ipRange: { type: 'string', example: '10.0.0.0/8', optional: true },
protocol: { type: 'string', example: 'tcp', optional: true },
port: { type: 'number', example: 80, optional: true },
},
},
},
responses: [
{
status: 204,
description: 'Rule updated successfully',
},
{
status: 404,
description: 'Old rule does not exist on this VIF',
},
],
middlewares: [
{ name: 'json' },
{ name: 'acl', acls: { resource: 'vif', action: 'update:other_config', objectId: 'params.id' } },
],
callback: ({ req, createAction }) => {
return createAction(
async () => {
const { oldRule, newRule: partialNewRule } = req.body
const vifId = req.params.id
const vif = this._xo.getObject(vifId, 'VIF')
const rawVifRules = vif.other_config[SDN_CONTROLLER_OF_RULES_KEY]
const vifRules = rawVifRules !== undefined ? JSON.parse(rawVifRules).map(JSON.parse) : []
if (
!vifRules.some(
rule =>
rule.allow === oldRule.allow &&
rule.direction === oldRule.direction &&
rule.ipRange === oldRule.ipRange &&
rule.port === oldRule.port &&
rule.protocol.toLowerCase() === oldRule.protocol.toLowerCase()
)
) {
throw noSuchObject(JSON.stringify(oldRule), 'traffic-rule')
}
const newRule = { ...oldRule, ...partialNewRule }
await this._deleteRule({ ...oldRule, vifId })
await this._addRule({ ...newRule, vifId })
},
{
sync: req.query.sync ?? false,
statusCode: 204,
taskProperties: {
name: 'update vif traffic rule',
objectId: req.params.id,
params: req.body,
objectType: 'VIF',
},
}
)
},
},
],
'/plugins/sdn-controller'
)
)
forOwn(this._xo.getAllXapis(), xapi => {
if (xapi.status === 'connected') {
this._handleConnectedXapi(xapi)

View File

@@ -658,7 +658,10 @@ describe('RestApi', () => {
const response = await get(port, '/param-validation/baz')
assert.equal(response.status, 422)
})
it('rejects object in route param', async () => {
const response = await get(port, '/param-validation/{type}')
assert.equal(response.status, 422)
})
it('accepts valid required query param', async () => {
const response = await get(port, '/query-required-validation?name=hello')
assert.equal(response.status, 200)
@@ -691,6 +694,124 @@ describe('RestApi', () => {
const response = await post(port, '/body-validation', { body: {} })
assert.equal(response.status, 422)
})
describe('object type', () => {
before(() => {
restApi.registerRestRoutes(
[
{
endpoint: '/body-object-validation',
method: 'post',
middlewares: [{ name: 'json' }],
body: {
meta: {
type: 'object',
fields: {
key: { type: 'string' },
count: { type: 'number' },
},
},
},
callback: ({ req }) => ({ meta: req.body.meta }),
},
],
''
)
})
it('accepts valid nested object in body', async () => {
const response = await post(port, '/body-object-validation', { body: { meta: { key: 'hello', count: 42 } } })
assert.equal(response.status, 200)
assert.deepEqual(await response.json(), { meta: { key: 'hello', count: 42 } })
})
it('rejects invalid type for nested field in body', async () => {
const response = await post(port, '/body-object-validation', { body: { meta: { key: 123, count: 42 } } })
assert.equal(response.status, 422)
})
it('rejects missing required nested field in body', async () => {
const response = await post(port, '/body-object-validation', { body: { meta: { key: 'hello' } } })
assert.equal(response.status, 422)
})
it('rejects missing required object field in body', async () => {
const response = await post(port, '/body-object-validation', { body: {} })
assert.equal(response.status, 422)
})
it('accepts optional object field absent from body', async () => {
const unregister = restApi.registerRestRoutes(
[
{
endpoint: '/body-optional-object-test',
method: 'post',
middlewares: [{ name: 'json' }],
body: {
meta: {
type: 'object',
fields: { key: { type: 'string' } },
optional: true,
},
},
callback: ({ req }) => ({ hasBody: req.body.meta !== undefined }),
},
],
'/'
)
try {
const response = await post(port, '/body-optional-object-test', { body: {} })
assert.equal(response.status, 200)
assert.deepEqual(await response.json(), { hasBody: false })
} finally {
unregister()
}
})
it('rejects object type in route params at runtime', async () => {
const unregister = restApi.registerRestRoutes(
[
{
endpoint: '/runtime-param-object-test/{data}',
method: 'get',
params: {
data: { type: 'object', fields: { key: { type: 'string' } } },
},
callback: ({ req }) => ({ data: req.params.data }),
},
],
'/'
)
try {
const response = await get(port, '/runtime-param-object-test/hello')
assert.equal(response.status, 422)
} finally {
unregister()
}
})
it('rejects object type in query params at runtime', async () => {
const unregister = restApi.registerRestRoutes(
[
{
endpoint: '/runtime-query-object-test',
method: 'get',
query: {
data: { type: 'object', fields: { key: { type: 'string' } } },
},
callback: ({ req }) => ({ data: req.query.data }),
},
],
'/'
)
try {
const response = await get(port, '/runtime-query-object-test?data=hello')
assert.equal(response.status, 422)
} finally {
unregister()
}
})
})
})
describe('middlewares', () => {
@@ -1226,6 +1347,41 @@ describe('RestApi', () => {
assert.ok(pathEntry !== undefined)
assert.equal(pathEntry.get.responses['200'].description, 'A custom OK')
})
it('object field in response schema appears in swagger spec with nested properties', async () => {
restApi.registerRestRoutes(
[
{
endpoint: '/swagger-response-object',
method: 'get',
responses: [
{
status: 200,
description: 'A response with a nested object',
schema: {
data: {
type: 'object',
fields: {
id: { type: 'string' },
count: { type: 'number' },
},
},
},
},
],
callback: () => ({}),
},
],
'/'
)
const spec = await (await fetchSwagger(port)).json()
const pathEntry = spec.paths['/swagger-response-object']
assert.ok(pathEntry !== undefined)
const responseSchema = pathEntry.get.responses['200'].content?.['application/json']?.schema
assert.equal(responseSchema?.properties?.data?.type, 'object')
assert.ok(responseSchema?.properties?.data?.properties?.id !== undefined)
assert.ok(responseSchema?.properties?.data?.properties?.count !== undefined)
})
})
})
})