Skip to content

[Fleet] Update minimun package spec version to 2.3 #214600

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

Merged
jsoriano merged 20 commits into from
Mar 25, 2025
Merged

[Fleet] Update minimun package spec version to 2.3 #214600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e5d635b
[Fleet] Update minimun package spec version to 2.3
jsoriano Mar 14, 2025
31b4860
Set spec.min in tests
jsoriano Mar 17, 2025
5879660
Merge remote-tracking branch 'origin/main' into update-spec-min
jsoriano Mar 17, 2025
6f41b9b
Try to fix schema of spec version constraints
jsoriano Mar 18, 2025
02b20b2
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Mar 18, 2025
04886c4
Try to fix types
jsoriano Mar 18, 2025
8d2ef15
Merge remote-tracking branch 'jsoriano/update-spec-min' into update-s…
jsoriano Mar 18, 2025
1ba63ee
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Mar 18, 2025
c2682f8
Override spec.min in more tests
jsoriano Mar 19, 2025
5a38f21
Merge remote-tracking branch 'jsoriano/update-spec-min' into update-s…
jsoriano Mar 19, 2025
e9b00d9
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Mar 19, 2025
457443a
More spec.min overrides
jsoriano Mar 19, 2025
353c911
Merge remote-tracking branch 'jsoriano/update-spec-min' into update-s…
jsoriano Mar 19, 2025
15be35f
Fix test config
jsoriano Mar 19, 2025
3cef56d
Merge remote-tracking branch 'origin/main' into update-spec-min
jsoriano Mar 20, 2025
4be35e4
Merge branch 'main' into update-spec-min
jsoriano Mar 21, 2025
f81927b
Merge branch 'main' into update-spec-min
jsoriano Mar 21, 2025
68ce1e7
Merge branch 'main' into update-spec-min
jsoriano Mar 21, 2025
f4a6b98
Try to reduce overriden configurations
jsoriano Mar 24, 2025
2fd42fd
Merge branch 'main' into update-spec-min
jsoriano Mar 24, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions x-pack/platform/plugins/shared/fleet/server/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { BULK_CREATE_MAX_ARTIFACTS_BYTES } from './services/artifacts/artifacts'
const DEFAULT_BUNDLED_PACKAGE_LOCATION = path.join(__dirname, '../target/bundled_packages');
const DEFAULT_GPG_KEY_PATH = path.join(__dirname, '../target/keys/GPG-KEY-elasticsearch');

const REGISTRY_SPEC_MIN_VERSION = '2.3';
const REGISTRY_SPEC_MAX_VERSION = '3.3';

export const config: PluginConfigDescriptor = {
Expand Down Expand Up @@ -228,18 +229,25 @@ export const config: PluginConfigDescriptor = {
excludePackages: schema.arrayOf(schema.string(), { defaultValue: [] }),
spec: schema.object(
{
min: schema.maybe(schema.string()),
max: schema.string({ defaultValue: REGISTRY_SPEC_MAX_VERSION }),
min: schema.string({
coerceFromNumber: true,
defaultValue: REGISTRY_SPEC_MIN_VERSION,
}),
max: schema.string({
coerceFromNumber: true,
defaultValue: REGISTRY_SPEC_MAX_VERSION,
}),
},
{
defaultValue: {
min: REGISTRY_SPEC_MIN_VERSION,
max: REGISTRY_SPEC_MAX_VERSION,
},
}
),
capabilities: schema.arrayOf(
schema.oneOf([
// See package-spec for the list of available capiblities https://github.com/elastic/package-spec/blob/dcc37b652690f8a2bca9cf8a12fc28fd015730a0/spec/integration/manifest.spec.yml#L113
// See package-spec for the list of available capabilities https://github.com/elastic/package-spec/blob/dcc37b652690f8a2bca9cf8a12fc28fd015730a0/spec/integration/manifest.spec.yml#L113
schema.literal('apm'),
schema.literal('enterprise_search'),
schema.literal('observability'),
Expand All @@ -256,6 +264,7 @@ export const config: PluginConfigDescriptor = {
capabilities: [],
excludePackages: [],
spec: {
min: REGISTRY_SPEC_MIN_VERSION,
max: REGISTRY_SPEC_MAX_VERSION,
},
},
Expand Down
3 changes: 3 additions & 0 deletions ...ck/platform/plugins/shared/fleet/server/integration_tests/fixtures/cloud_kibana_config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ export const CLOUD_KIBANA_CONFIG = {
internal: {
registry: {
kibanaVersionCheckEnabled: false,
spec: {
min: '1.0',
Copy link
Member

@nchaulet nchaulet Mar 19, 2025

Choose a reason for hiding this comment

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

@jsoriano the goal of that test what to mimic the cloud config, I am worried that if we introduce a change here, we may accidentally break cloud, it is because there no apm package compatible? should it be fixed by publishing one instead?
Copy link
Member Author

@jsoriano jsoriano Mar 19, 2025

Choose a reason for hiding this comment

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

APM packages are not released as GA in the registry since 8.4.2. So not sure if we can assume that there is going to be a package there.
Copy link
Member Author

@jsoriano jsoriano Mar 19, 2025

Choose a reason for hiding this comment

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

Could it be that the GA package is bundled in GA Kibana, but the one in the registry is used for development? 🤔
Copy link
Member

@nchaulet nchaulet Mar 20, 2025

Choose a reason for hiding this comment

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

Yes I think in development we are using the old APM package
Copy link
Member Author

@jsoriano jsoriano Mar 20, 2025

Choose a reason for hiding this comment

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

Created issue about this: #215400
},
},
},
packages: [
Expand Down
3 changes: 3 additions & 0 deletions ...plugins/shared/fleet/server/integration_tests/fixtures/cloud_kibana_config_without_apm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ export const CLOUD_KIBANA_CONFIG_WITHOUT_APM = {
internal: {
registry: {
kibanaVersionCheckEnabled: false,
spec: {
min: '1.0',
},
},
},
packages: [
Expand Down
3 changes: 3 additions & 0 deletions ...s/shared/fleet/server/integration_tests/fixtures/cloud_kibana_config_without_policy_id.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ export const CLOUD_KIBANA_WITHOUT_PACKAGE_POLICY_ID_CONFIG = {
internal: {
registry: {
kibanaVersionCheckEnabled: false,
spec: {
min: '1.0',
},
},
},
packages: [
Expand Down
3 changes: 3 additions & 0 deletions x-pack/platform/plugins/shared/fleet/server/integration_tests/ha_setup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ describe('Fleet setup preconfiguration with multiple instances Kibana', () => {
internal: {
registry: {
kibanaVersionCheckEnabled: false,
spec: {
min: '1.0',
},
},
},
packages: [
Expand Down
12 changes: 10 additions & 2 deletions x-pack/platform/plugins/shared/fleet/server/services/epm/registry/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,21 @@ function setSpecVersion(url: URL) {
const specMax = appContextService.getConfig()?.internal?.registry?.spec?.max;

if (specMin) {
url.searchParams.set('spec.min', specMin);
url.searchParams.set('spec.min', formatSpecVersion(specMin));
}
if (specMax) {
url.searchParams.set('spec.max', specMax);
url.searchParams.set('spec.max', formatSpecVersion(specMax));
}
}

function formatSpecVersion(version: string): string {
// Version can be coerced from number when obtained through flags, in these cases X.0 versions are coerced to X.
Copy link
Member Author

@jsoriano jsoriano Mar 19, 2025

Choose a reason for hiding this comment

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

This is still a problem for versions whose minor ends with 0, for example 2.1, 2.10 and 2.100 will be parsed as 2.1, and coerced to the 2.1 string.

No idea what we can do to avoid flags being parsed as numbers.
Copy link
Contributor

@MichelLosier MichelLosier Mar 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Do we foresee problems with that loss of precision in how this value is used?

Seems like if ideally we want to keep this value as a string, one thought is we could enhance the StringType validator to have an isSemver option, much like it has a hostname one.

I see the repo already has the semver library already as a dependency, so we could lean on the semver.valid() util from that for the implementation of that option
Copy link
Member Author

@jsoriano jsoriano Mar 19, 2025

Choose a reason for hiding this comment

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

Do we foresee problems with that loss of precision in how this value is used?

Yes, it is a problem here because we expect <major>.<minor> format. With the "fix" introduced here it will be only a problem with minors that are multiples of 10.

Seems like if ideally we want to keep this value as a string, one thought is we could enhance the StringType validator to have an isSemver option, much like it has a hostname one.

I have tried some things but it looks like the value is directly parsed as number, so there is some loss.

Maybe it is expected for strings to be used as quotes, though it didn't work well when used in tests.

We can investigate this further in the future, this PR at least doesn't make things worse 🙂
Copy link
Member Author

@jsoriano jsoriano Mar 19, 2025

Choose a reason for hiding this comment

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

Issue created for further investigation: #215251
if (version.indexOf('.') > 0) {
return version;
}
return version + '.0';
}

function setCapabilities(url: URL) {
const capabilities = appContextService.getConfig()?.internal?.registry?.capabilities;
if (capabilities && capabilities.length > 0) {
Expand Down
1 change: 1 addition & 0 deletions x-pack/test/fleet_api_integration/config.base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export default async function ({ readConfigFile, log }: FtrConfigProviderContext
`--xpack.fleet.agentless.api.tls.key=./config/node.key`,
`--xpack.fleet.agentless.api.tls.ca=./config/ca.crt`,
`--xpack.fleet.internal.registry.kibanaVersionCheckEnabled=false`,
`--xpack.fleet.internal.registry.spec.min=1.0`,
`--logging.loggers=${JSON.stringify([
...getKibanaCliLoggers(xPackAPITestsConfig.get('kbnTestServer.serverArgs')),

Expand Down
1 change: 0 additions & 1 deletion x-pack/test/functional/config.base.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export default async function ({ readConfigFile }) {
'--server.restrictInternalApis=false',
// disable fleet task that writes to metrics.fleet_server.* data streams, impacting functional tests
`--xpack.task_manager.unsafe.exclude_task_types=${JSON.stringify(['Fleet-Metrics-Task'])}`,
`--xpack.fleet.internal.registry.kibanaVersionCheckEnabled=false`,
],
},
uiSettings: {
Expand Down
Loading