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.

Sign up for GitHub

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

[HTTP/OAS] Make SO CRUD and resolve APIs internal on serverless #184408

Merged
merged 11 commits into from
Jun 11, 2024
Merged

[HTTP/OAS] Make SO CRUD and resolve APIs internal on serverless #184408

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
eafb16b
make new RouteAccess type and export it
jloleysens May 29, 2024
454edfb
pass down and use build flavour => isServerless and map to access lev…
jloleysens May 29, 2024
9268761
added docs and access level for _import, _export and resolve import e…
jloleysens May 29, 2024
8166ec4
added functional test
jloleysens May 29, 2024
76691f3
fix types issues
jloleysens May 29, 2024
629fd36
Merge branch 'main' into http/make-so-apis-public-on-stateful
jloleysens May 30, 2024
bd8aa8a
Merge branch 'main' into http/make-so-apis-public-on-stateful
jloleysens Jun 4, 2024
3ea77ab
Merge branch 'main' into http/make-so-apis-public-on-stateful
jloleysens Jun 6, 2024
3f02c23
Merge branch 'main' into http/make-so-apis-public-on-stateful
jloleysens Jun 7, 2024
fba5c73
Merge branch 'main' into http/make-so-apis-public-on-stateful
jloleysens Jun 10, 2024
dcb6b3e
Merge branch 'main' into http/make-so-apis-public-on-stateful
jloleysens Jun 10, 2024
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
1 change: 1 addition & 0 deletions packages/core/http/core-http-server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export type {
RouteValidatorFullConfigRequest,
RouteValidatorFullConfigResponse,
LazyValidator,
RouteAccess,
} from './src/router';
export {
validBodyOutput,
Expand Down
1 change: 1 addition & 0 deletions packages/core/http/core-http-server/src/router/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export type {
RouteConfigOptionsBody,
RouteContentType,
SafeRouteMethod,
RouteAccess,
} from './route';
export { validBodyOutput } from './route';
export type {
Expand Down
12 changes: 11 additions & 1 deletion packages/core/http/core-http-server/src/router/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,16 @@ export interface RouteConfigOptionsBody {
parse?: boolean | 'gunzip';
}

/**
* Route access level.
*
* Public routes are stable and intended for external access and are subject to
* stricter change management and have long term maintenance windows.
*
* @remark On serverless access to internal routes is restricted.
*/
export type RouteAccess = 'public' | 'internal';

/**
* Additional route options.
* @public
Expand Down Expand Up @@ -133,7 +143,7 @@ export interface RouteConfigOptions<Method extends RouteMethod> {
*
* Defaults to 'internal' If not declared,
*/
access?: 'public' | 'internal';
access?: RouteAccess;

/**
* Additional metadata tag strings to attach to the route.
Expand Down
8 changes: 7 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/bulk_create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { schema } from '@kbn/config-schema';
import type { RouteAccess } from '@kbn/core-http-server';
import { SavedObjectConfig } from '@kbn/core-saved-objects-base-server-internal';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { Logger } from '@kbn/logging';
Expand All @@ -21,16 +22,21 @@ interface RouteDependencies {
config: SavedObjectConfig;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}

export const registerBulkCreateRoute = (
router: InternalSavedObjectRouter,
{ config, coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger, access }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/_bulk_create',
options: {
access,
description: `Create saved objects`,
},
validate: {
query: schema.object({
overwrite: schema.boolean({ defaultValue: false }),
Expand Down
8 changes: 7 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/bulk_delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { schema } from '@kbn/config-schema';
import type { RouteAccess } from '@kbn/core-http-server';
import { SavedObjectConfig } from '@kbn/core-saved-objects-base-server-internal';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { Logger } from '@kbn/logging';
Expand All @@ -21,16 +22,21 @@ interface RouteDependencies {
config: SavedObjectConfig;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}

export const registerBulkDeleteRoute = (
router: InternalSavedObjectRouter,
{ config, coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger, access }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/_bulk_delete',
options: {
access,
description: `Remove saved objects`,
},
validate: {
body: schema.arrayOf(
schema.object({
Expand Down
8 changes: 7 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/bulk_get.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { schema } from '@kbn/config-schema';
import type { RouteAccess } from '@kbn/core-http-server';
import { SavedObjectConfig } from '@kbn/core-saved-objects-base-server-internal';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { Logger } from '@kbn/logging';
Expand All @@ -21,16 +22,21 @@ interface RouteDependencies {
config: SavedObjectConfig;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}

export const registerBulkGetRoute = (
router: InternalSavedObjectRouter,
{ config, coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger, access }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/_bulk_get',
options: {
access,
description: `Get saved objects`,
},
validate: {
body: schema.arrayOf(
schema.object({
Expand Down
8 changes: 7 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/bulk_resolve.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { schema } from '@kbn/config-schema';
import type { RouteAccess } from '@kbn/core-http-server';
import { SavedObjectConfig } from '@kbn/core-saved-objects-base-server-internal';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { Logger } from '@kbn/logging';
Expand All @@ -21,16 +22,21 @@ interface RouteDependencies {
config: SavedObjectConfig;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}

export const registerBulkResolveRoute = (
router: InternalSavedObjectRouter,
{ config, coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger, access }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/_bulk_resolve',
options: {
access,
description: `Resolve saved objects`,
},
validate: {
body: schema.arrayOf(
schema.object({
Expand Down
8 changes: 7 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/bulk_update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { schema } from '@kbn/config-schema';
import type { RouteAccess } from '@kbn/core-http-server';
import { SavedObjectConfig } from '@kbn/core-saved-objects-base-server-internal';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { Logger } from '@kbn/logging';
Expand All @@ -21,16 +22,21 @@ interface RouteDependencies {
config: SavedObjectConfig;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}

export const registerBulkUpdateRoute = (
router: InternalSavedObjectRouter,
{ config, coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger, access }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.put(
{
path: '/_bulk_update',
options: {
access,
Copy link
Contributor

Choose a reason for hiding this comment

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

@jloleysens Unless I'm missing some intricate detail, access will be internal according to https://github.com/elastic/kibana/pull/184408/files#diff-311e61dcad4f336ae3320406e2a114cc844823d4526d9c52c35c022d47516843R144.
The PR title had me worried you were making ALL the SO APIs public!

Copy link
Contributor Author

@jloleysens jloleysens May 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Access will be set dynamically based on build flavour. Serverless will remain internal and stateful will remain public. This pr just brings those to bear in the code.

description: `Update saved objects`,
},
validate: {
body: schema.arrayOf(
schema.object({
Expand Down
8 changes: 7 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { schema } from '@kbn/config-schema';
import type { RouteAccess } from '@kbn/core-http-server';
import { SavedObjectConfig } from '@kbn/core-saved-objects-base-server-internal';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { Logger } from '@kbn/logging';
Expand All @@ -21,16 +22,21 @@ interface RouteDependencies {
config: SavedObjectConfig;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}

export const registerCreateRoute = (
router: InternalSavedObjectRouter,
{ config, coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger, access }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/{type}/{id?}',
options: {
access,
description: `Create a saved object`,
},
validate: {
params: schema.object({
type: schema.string(),
Expand Down
8 changes: 7 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { schema } from '@kbn/config-schema';
import type { RouteAccess } from '@kbn/core-http-server';
import { SavedObjectConfig } from '@kbn/core-saved-objects-base-server-internal';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { Logger } from '@kbn/logging';
Expand All @@ -21,16 +22,21 @@ interface RouteDependencies {
config: SavedObjectConfig;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}

export const registerDeleteRoute = (
router: InternalSavedObjectRouter,
{ config, coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger, access }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.delete(
{
path: '/{type}/{id}',
options: {
access,
description: `Delete a saved object`,
},
validate: {
params: schema.object({
type: schema.string(),
Expand Down
4 changes: 4 additions & 0 deletions packages/core/saved-objects/core-saved-objects-server-internal/src/routes/export.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ export const registerExportRoute = (
router.post(
{
path: '/_export',
options: {
access: 'public',
Copy link
Contributor

Choose a reason for hiding this comment

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

✅

description: `Export saved objects`,
},
validate: {
body: schema.object({
type: schema.maybe(schema.oneOf([schema.string(), schema.arrayOf(schema.string())])),
Expand Down
9 changes: 8 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/find.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,24 @@
*/

import { schema } from '@kbn/config-schema';
import type { RouteAccess } from '@kbn/core-http-server';
import { SavedObjectConfig } from '@kbn/core-saved-objects-base-server-internal';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { Logger } from '@kbn/logging';
import type { InternalSavedObjectRouter } from '../internal_types';
import { catchAndReturnBoomErrors, throwOnHttpHiddenTypes } from './utils';
import { logWarnOnExternalRequest } from './utils';

interface RouteDependencies {
config: SavedObjectConfig;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}

export const registerFindRoute = (
router: InternalSavedObjectRouter,
{ config, coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger, access }: RouteDependencies
) => {
const referenceSchema = schema.object({
type: schema.string(),
Expand All @@ -34,6 +37,10 @@ export const registerFindRoute = (
router.get(
{
path: '/_find',
options: {
access,
description: `Search for saved objects`,
},
validate: {
query: schema.object({
per_page: schema.number({ min: 0, defaultValue: 20 }),
Expand Down
8 changes: 7 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/get.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { schema } from '@kbn/config-schema';
import type { RouteAccess } from '@kbn/core-http-server';
import { SavedObjectConfig } from '@kbn/core-saved-objects-base-server-internal';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { Logger } from '@kbn/logging';
Expand All @@ -21,16 +22,21 @@ interface RouteDependencies {
config: SavedObjectConfig;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}

export const registerGetRoute = (
router: InternalSavedObjectRouter,
{ config, coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger, access }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.get(
{
path: '/{type}/{id}',
options: {
access,
description: `Get a saved object`,
},
validate: {
params: schema.object({
type: schema.string(),
Expand Down
2 changes: 2 additions & 0 deletions packages/core/saved-objects/core-saved-objects-server-internal/src/routes/import.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export const registerImportRoute = (
{
path: '/_import',
options: {
access: 'public',
Copy link
Contributor

Choose a reason for hiding this comment

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

✅

description: `Import saved objects`,
body: {
maxBytes: maxImportPayloadBytes,
output: 'stream',
Expand Down
33 changes: 22 additions & 11 deletions packages/core/saved-objects/core-saved-objects-server-internal/src/routes/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export function registerRoutes({
migratorPromise,
kibanaVersion,
kibanaIndex,
isServerless,
}: {
http: InternalHttpServiceSetup;
coreUsageData: InternalCoreUsageDataSetup;
Expand All @@ -49,20 +50,30 @@ export function registerRoutes({
migratorPromise: Promise<IKibanaMigrator>;
kibanaVersion: string;
kibanaIndex: string;
isServerless: boolean;
}) {
const router =
http.createRouter<InternalSavedObjectsRequestHandlerContext>('/api/saved_objects/');
registerGetRoute(router, { config, coreUsageData, logger });
registerResolveRoute(router, { config, coreUsageData, logger });
registerCreateRoute(router, { config, coreUsageData, logger });
registerDeleteRoute(router, { config, coreUsageData, logger });
registerFindRoute(router, { config, coreUsageData, logger });
registerUpdateRoute(router, { config, coreUsageData, logger });
registerBulkGetRoute(router, { config, coreUsageData, logger });
registerBulkCreateRoute(router, { config, coreUsageData, logger });
registerBulkResolveRoute(router, { config, coreUsageData, logger });
registerBulkUpdateRoute(router, { config, coreUsageData, logger });
registerBulkDeleteRoute(router, { config, coreUsageData, logger });

const internalOnServerless = isServerless ? 'internal' : 'public';
Copy link
Contributor

Choose a reason for hiding this comment

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

no. We cannot use internalOnServerless as is to globally define access for ALL the routes. If we do then we'd make every single one public on non-serverless!
We can't do that and still say we're removing them in v9.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 , this is only used for the APIs documented as public, but that we want internal on serverless. I can add a comment to that effect.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jloleysens I'll approve this PR as long as we're not contradicting our public docs that clearly state which APIs are deprecated.


registerGetRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
Copy link
Contributor

Choose a reason for hiding this comment

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

no

registerResolveRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
registerCreateRoute(router, {
config,
coreUsageData,
logger,
access: internalOnServerless,
});
registerDeleteRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
registerFindRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
registerUpdateRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
registerBulkGetRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
registerBulkCreateRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
registerBulkResolveRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
registerBulkUpdateRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
registerBulkDeleteRoute(router, { config, coreUsageData, logger, access: internalOnServerless });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not add tests for the existing public behaviour as I assume these tests already give us that coverage:

kibana/src/core/server/integration_tests/saved_objects/routes/bulk_get.test.ts

Line 85 in e3ca2c5

const result = await supertest(httpSetup.server.listener)


registerExportRoute(router, { config, coreUsageData });
registerImportRoute(router, { config, coreUsageData });
registerResolveImportErrorsRoute(router, { config, coreUsageData });
Expand Down
Loading