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

[Saved Objects] Adds config flag to toggle hiddenFromHttpApis SO types conditionally #151512

Merged
merged 7 commits into from
Feb 22, 2023
Merged

[Saved Objects] Adds config flag to toggle hiddenFromHttpApis SO types conditionally #151512

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
76a77b4
Adds config flag to toggle hiddenFromHttpApis SO types conditionally
TinaHeiligers Feb 13, 2023
99b4dcf
cleanup draft
TinaHeiligers Feb 16, 2023
497ec8a
Merge branch 'main' into kbn-150471-toggle-hiddenFromHttpApis-accessi…
kibanamachine Feb 20, 2023
879d33b
marks allowHttpApiAccess as internal
TinaHeiligers Feb 20, 2023
b61d065
Merge branch 'main' into kbn-150471-toggle-hiddenFromHttpApis-accessi…
kibanamachine Feb 21, 2023
96bb95a
default allowHttpApiAccess conditionally depending on if Kibana's run…
TinaHeiligers Feb 21, 2023
75e11c7
Consistently name integration tests
TinaHeiligers Feb 21, 2023
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 config/kibana.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@
# configuration option. Default: 100mb
#migrations.maxBatchSizeBytes: 100mb


# The number of times to retry temporary migration failures. Increase the setting
# if migrations fail frequently with a message such as `Unable to complete the [...] step after
# 15 attempts, terminating`. Defaults to 15
Expand Down
9 changes: 6 additions & 3 deletions ...es/core/saved-objects/core-saved-objects-base-server-internal/src/saved_objects_config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export const savedObjectsMigrationConfig: ServiceConfigDescriptor<SavedObjectsMi
const soSchema = schema.object({
maxImportPayloadBytes: schema.byteSize({ defaultValue: 26_214_400 }),
maxImportExportSize: schema.number({ defaultValue: 10_000 }),
// Conditionally set within config, dependening on the env. In prod: allow all access(default), in dev: false
allowHttpApiAccess: schema.boolean({ defaultValue: true }),
Copy link
Contributor Author

@TinaHeiligers TinaHeiligers Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I've gone with defaulting to the "old" behavior where all visible types (those not hidden) use the HTTP APIs. We also need to allow access for self-managed & current cloud offering & cloud tests

Copy link
Member

@Bamieh Bamieh Feb 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is there a specific reason to depend on the environment and not distribution?

Instead of doing the override logic between the config and the env inside the code and having to inject the env into the classes, why not just do it from the config defaults here (like the example below). This makes the logic a lot more resilient and easier to understand. wdyt?

schema.conditional(
      schema.contextRef('dist'),
      true,
      schema.boolean({ defaultValue: false }),
      schema.boolean({ defaultValue: true })
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a specific reason to depend on the environment and not distribution?

I didn't think to use the distro rather, great idea thanks!
I'll change it and see how it improves the logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Bamieh I've made the change you requested in 96bb95a (#151512).
Could you take another look, please?

});

export type SavedObjectsConfigType = TypeOf<typeof soSchema>;
Expand All @@ -50,19 +52,20 @@ export const savedObjectsConfig: ServiceConfigDescriptor<SavedObjectsConfigType>
path: 'savedObjects',
schema: soSchema,
};

export class SavedObjectConfig {
public maxImportPayloadBytes: number;
public maxImportExportSize: number;

public allowHttpApiAccess: boolean; // depend on env: see https://github.com/elastic/dev/issues/2200
public migration: SavedObjectsMigrationConfigType;

constructor(
rawConfig: SavedObjectsConfigType,
rawMigrationConfig: SavedObjectsMigrationConfigType
rawMigrationConfig: SavedObjectsMigrationConfigType,
allowHttpApiAccess: boolean
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Naming is hard and hideFromHttpApis or denyHttpApiAccess sound rather negative, hence the more "positive" choice.

) {
this.maxImportPayloadBytes = rawConfig.maxImportPayloadBytes.getValueInBytes();
this.maxImportExportSize = rawConfig.maxImportExportSize;
this.migration = rawMigrationConfig;
this.allowHttpApiAccess = rawConfig.allowHttpApiAccess || allowHttpApiAccess;
}
}
9 changes: 7 additions & 2 deletions 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,20 +7,23 @@
*/

import { schema } from '@kbn/config-schema';
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, throwIfAnyTypeNotVisibleByAPI } from './utils';

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

export const registerBulkCreateRoute = (
router: InternalSavedObjectRouter,
{ coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/_bulk_create',
Expand Down Expand Up @@ -62,7 +65,9 @@ export const registerBulkCreateRoute = (
const { savedObjects } = await context.core;

const typesToCheck = [...new Set(req.body.map(({ type }) => type))];
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);
if (!allowHttpApiAccess) {
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);
}
const result = await savedObjects.client.bulkCreate(req.body, { overwrite });
return res.ok({ body: result });
})
Expand Down
10 changes: 7 additions & 3 deletions 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,20 +7,23 @@
*/

import { schema } from '@kbn/config-schema';
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, throwIfAnyTypeNotVisibleByAPI } from './utils';

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

export const registerBulkDeleteRoute = (
router: InternalSavedObjectRouter,
{ coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/_bulk_delete',
Expand All @@ -47,8 +50,9 @@ export const registerBulkDeleteRoute = (
const { savedObjects } = await context.core;

const typesToCheck = [...new Set(req.body.map(({ type }) => type))];
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);

if (!allowHttpApiAccess) {
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);
}
const statuses = await savedObjects.client.bulkDelete(req.body, { force });
return res.ok({ body: statuses });
})
Expand Down
10 changes: 7 additions & 3 deletions 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,20 +7,23 @@
*/

import { schema } from '@kbn/config-schema';
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, throwIfAnyTypeNotVisibleByAPI } from './utils';

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

export const registerBulkGetRoute = (
router: InternalSavedObjectRouter,
{ coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/_bulk_get',
Expand All @@ -42,8 +45,9 @@ export const registerBulkGetRoute = (

const { savedObjects } = await context.core;
const typesToCheck = [...new Set(req.body.map(({ type }) => type))];
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);

if (!allowHttpApiAccess) {
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);
}
const result = await savedObjects.client.bulkGet(req.body);
return res.ok({ body: result });
})
Expand Down
9 changes: 7 additions & 2 deletions 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,20 +7,23 @@
*/

import { schema } from '@kbn/config-schema';
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, throwIfAnyTypeNotVisibleByAPI } from './utils';

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

export const registerBulkResolveRoute = (
router: InternalSavedObjectRouter,
{ coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/_bulk_resolve',
Expand All @@ -42,7 +45,9 @@ export const registerBulkResolveRoute = (

const { savedObjects } = await context.core;
const typesToCheck = [...new Set(req.body.map(({ type }) => type))];
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);
if (!allowHttpApiAccess) {
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);
}
const result = await savedObjects.client.bulkResolve(req.body);
return res.ok({ body: result });
})
Expand Down
10 changes: 7 additions & 3 deletions 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,20 +7,23 @@
*/

import { schema } from '@kbn/config-schema';
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, throwIfAnyTypeNotVisibleByAPI } from './utils';

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

export const registerBulkUpdateRoute = (
router: InternalSavedObjectRouter,
{ coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.put(
{
path: '/_bulk_update',
Expand Down Expand Up @@ -55,8 +58,9 @@ export const registerBulkUpdateRoute = (
const { savedObjects } = await context.core;

const typesToCheck = [...new Set(req.body.map(({ type }) => type))];
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);

if (!allowHttpApiAccess) {
throwIfAnyTypeNotVisibleByAPI(typesToCheck, savedObjects.typeRegistry);
}
const savedObject = await savedObjects.client.bulkUpdate(req.body);
return res.ok({ body: savedObject });
})
Expand Down
11 changes: 7 additions & 4 deletions 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,20 +7,23 @@
*/

import { schema } from '@kbn/config-schema';
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, throwIfTypeNotVisibleByAPI } from './utils';

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

export const registerCreateRoute = (
router: InternalSavedObjectRouter,
{ coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.post(
{
path: '/{type}/{id?}',
Expand Down Expand Up @@ -60,9 +63,9 @@ export const registerCreateRoute = (
usageStatsClient.incrementSavedObjectsCreate({ request: req }).catch(() => {});

const { savedObjects } = await context.core;

throwIfTypeNotVisibleByAPI(type, savedObjects.typeRegistry);

if (!allowHttpApiAccess) {
throwIfTypeNotVisibleByAPI(type, savedObjects.typeRegistry);
}
const options = {
id,
overwrite,
Expand Down
10 changes: 7 additions & 3 deletions 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,20 +7,23 @@
*/

import { schema } from '@kbn/config-schema';
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, throwIfTypeNotVisibleByAPI } from './utils';

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

export const registerDeleteRoute = (
router: InternalSavedObjectRouter,
{ coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.delete(
{
path: '/{type}/{id}',
Expand All @@ -42,8 +45,9 @@ export const registerDeleteRoute = (

const usageStatsClient = coreUsageData.getClient();
usageStatsClient.incrementSavedObjectsDelete({ request: req }).catch(() => {});
throwIfTypeNotVisibleByAPI(type, typeRegistry);

if (!allowHttpApiAccess) {
throwIfTypeNotVisibleByAPI(type, typeRegistry);
}
const client = getClient();
const result = await client.delete(type, id, { force });
return res.ok({ body: result });
Expand Down
8 changes: 5 additions & 3 deletions 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,19 +7,21 @@
*/

import { schema } from '@kbn/config-schema';
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';

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

export const registerFindRoute = (
router: InternalSavedObjectRouter,
{ coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger }: RouteDependencies
) => {
const referenceSchema = schema.object({
type: schema.string(),
Expand All @@ -28,7 +30,7 @@ export const registerFindRoute = (
const searchOperatorSchema = schema.oneOf([schema.literal('OR'), schema.literal('AND')], {
defaultValue: 'OR',
});

const { allowHttpApiAccess } = config;
router.get(
{
path: '/_find',
Expand Down Expand Up @@ -95,7 +97,7 @@ export const registerFindRoute = (
return fullType.name;
}
});
if (unsupportedTypes.length > 0) {
if (unsupportedTypes.length > 0 && !allowHttpApiAccess) {
throwOnHttpHiddenTypes(unsupportedTypes);
}

Expand Down
10 changes: 8 additions & 2 deletions 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,20 +7,23 @@
*/

import { schema } from '@kbn/config-schema';
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, throwIfTypeNotVisibleByAPI } from './utils';

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

export const registerGetRoute = (
router: InternalSavedObjectRouter,
{ coreUsageData, logger }: RouteDependencies
{ config, coreUsageData, logger }: RouteDependencies
) => {
const { allowHttpApiAccess } = config;
router.get(
{
path: '/{type}/{id}',
Expand All @@ -39,7 +42,10 @@ export const registerGetRoute = (
usageStatsClient.incrementSavedObjectsGet({ request: req }).catch(() => {});

const { savedObjects } = await context.core;
throwIfTypeNotVisibleByAPI(type, savedObjects.typeRegistry);

if (!allowHttpApiAccess) {
throwIfTypeNotVisibleByAPI(type, savedObjects.typeRegistry);
}

const object = await savedObjects.client.get(type, id);
return res.ok({ body: object });
Expand Down
Loading