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

[Fleet] Fix agent policy mappings for space awareness #201689

Merged
merged 10 commits into from
Nov 28, 2024
Merged

[Fleet] Fix agent policy mappings for space awareness #201689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d129bc1
[Fleet] Fix agent policy mappings for space awareness
nchaulet Nov 25, 2024
f9b3035
[CI] Auto-commit changed files from 'node scripts/check_mappings_upda…
kibanamachine Nov 25, 2024
4b3c2a5
fix test
nchaulet Nov 25, 2024
5a71934
[CI] Auto-commit changed files from 'node scripts/jest_integration -u…
kibanamachine Nov 25, 2024
d2d77a5
Merge branch 'fix-agent-policies-space-aware-mappings' of github.com:…
nchaulet Nov 26, 2024
fbacaea
change agent policy mappings to dynamic:false
nchaulet Nov 27, 2024
d27b010
fix mappings
nchaulet Nov 27, 2024
1660392
[CI] Auto-commit changed files from 'node scripts/check_mappings_upda…
kibanamachine Nov 27, 2024
7e83e00
[CI] Auto-commit changed files from 'node scripts/jest_integration -u…
kibanamachine Nov 27, 2024
c78a9d1
Merge branch 'main' into fix-agent-policies-space-aware-mappings
elasticmachine Nov 27, 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
2 changes: 2 additions & 0 deletions packages/kbn-check-mappings-update-cli/current_mappings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1593,6 +1593,7 @@
}
},
"fleet-agent-policies": {
"dynamic": false,
"properties": {
"advanced_settings": {
"index": false,
Expand Down Expand Up @@ -1962,6 +1963,7 @@
"properties": {}
},
"ingest-agent-policies": {
"dynamic": false,
"properties": {
"advanced_settings": {
"index": false,
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
"file": "6b65ae5899b60ebe08656fd163ea532e557d3c98",
"file-upload-usage-collection-telemetry": "06e0a8c04f991e744e09d03ab2bd7f86b2088200",
"fileShare": "5be52de1747d249a221b5241af2838264e19aaa1",
"fleet-agent-policies": "f57d3b70e4175a19a18f18ee72a379ceec82e1fc",
"fleet-agent-policies": "908765a33aab066f4ac09446686b2d884aceed00",
"fleet-fleet-server-host": "69be15f6b6f2a2875ad3c7050ddea7a87f505417",
"fleet-message-signing-keys": "93421f43fed2526b59092a4e3c65d64bc2266c0f",
"fleet-package-policies": "0206c20f27286787b91814a2e7872f06dc1e8e47",
Expand All @@ -121,7 +121,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
"infra-custom-dashboards": "1a5994f2e05bb8a1609825ddbf5012f77c5c67f3",
"infrastructure-monitoring-log-view": "5f86709d3c27aed7a8379153b08ee5d3d90d77f5",
"infrastructure-ui-source": "113182d6895764378dfe7fa9fa027244f3a457c4",
"ingest-agent-policies": "5e95e539826a40ad08fd0c1d161da0a4d86ffc6d",
"ingest-agent-policies": "c1818c4119259908875b4c777ae62b11ba0585cd",
"ingest-download-sources": "279a68147e62e4d8858c09ad1cf03bd5551ce58d",
"ingest-outputs": "55988d5f778bbe0e76caa7e6468707a0a056bdd8",
"ingest-package-policies": "60d43f475f91417d14d9df05476acf2e63e99435",
Expand Down
32 changes: 32 additions & 0 deletions x-pack/plugins/fleet/server/saved_objects/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import _ from 'lodash';

import { getSavedObjectTypes } from '.';

describe('space aware models', () => {
it('should have the same mappings for space and non-space aware agent policies', () => {
const soTypes = getSavedObjectTypes();

const legacyMappings = _.omit(
soTypes['ingest-agent-policies'].mappings,
'properties.monitoring_diagnostics',
'properties.monitoring_http',
'properties.monitoring_pprof_enabled'
);

expect(legacyMappings).toEqual(soTypes['fleet-agent-policies'].mappings);
});
it('should have the same mappings for space and non-space aware package policies', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

const soTypes = getSavedObjectTypes();

expect(soTypes['ingest-package-policies'].mappings).toEqual(
soTypes['fleet-package-policies'].mappings
);
});
});
20 changes: 20 additions & 0 deletions x-pack/plugins/fleet/server/saved_objects/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export const getSavedObjectTypes = (
importableAndExportable: false,
},
mappings: {
dynamic: false,
properties: {
name: { type: 'keyword' },
schema_version: { type: 'version' },
Expand Down Expand Up @@ -305,6 +306,14 @@ export const getSavedObjectTypes = (
},
],
},
'5': {
changes: [
{
type: 'mappings_addition',
addedMappings: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
addedMappings: {},
addedMappings: {
dynamic: false,
},

You should also specify the change in mappings here. It's unfortunate that we have this duplication of the mappings definitions, but we didn't see a good way around it.

https://github.com/elastic/kibana/blob/skip-transform-on-mappings-change/docs/developer/architecture/core/saved-objects-service.asciidoc?plain=1#L324-L325

Copy link
Member Author

@nchaulet nchaulet Nov 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@rudolf I tried this but I got a type error Type 'boolean' is not assignable to type 'SavedObjectsFieldMapping' Should we ignore that error?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I see, you're right, looks like a limitation in the modelVersions API in that you can't actually specify a change that sets the top-level dynamic option.

},
],
},
},
},
[AGENT_POLICY_SAVED_OBJECT_TYPE]: {
Expand All @@ -316,6 +325,7 @@ export const getSavedObjectTypes = (
importableAndExportable: false,
},
mappings: {
dynamic: false,
properties: {
name: { type: 'keyword' },
schema_version: { type: 'version' },
Expand Down Expand Up @@ -350,6 +360,16 @@ export const getSavedObjectTypes = (
global_data_tags: { type: 'flattened', index: false },
},
},
modelVersions: {
'1': {
changes: [
{
type: 'mappings_addition',
addedMappings: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
addedMappings: {},
addedMappings: {
dynamic: false,
},

},
],
},
},
},
[OUTPUT_SAVED_OBJECT_TYPE]: {
name: OUTPUT_SAVED_OBJECT_TYPE,
Expand Down
Loading