Skip to content

Fix transform index spec#964

Merged
jsoriano merged 5 commits into
elastic:mainfrom
jsoriano:version-patch-lookup-index
Sep 2, 2025
Merged

Fix transform index spec#964
jsoriano merged 5 commits into
elastic:mainfrom
jsoriano:version-patch-lookup-index

Conversation

@jsoriano

@jsoriano jsoriano commented Aug 25, 2025

Copy link
Copy Markdown
Contributor

While attempting to follow with #936 (comment), I found that no spec was being applied to the transform index manifests.

What does this PR do?

Fix transform index spec.

We removed spec from the manifests spec in #936 by mistake, add it back.

A safeguard is added to avoid having spec files without spec.

Some settings are moved to its own definitions so they can be more easily referenced, without this change some settings could not be referenced, not sure why.

Also, drop index.mode from spec versions older than 3.4, as lookup mode is not supported in old versions of the stack.

Why is it important?

Properly validate transform index definitions.

Checklist

  • I have added test packages to test/packages that prove my change is effective. There are tests covering this.
  • I have added an entry in spec/changelog.yml. Fixes unreleased changes.

Related issues

@jsoriano jsoriano requested a review from JohannesMahne August 25, 2025 16:10
@jsoriano jsoriano self-assigned this Aug 25, 2025
@jsoriano jsoriano requested a review from a team as a code owner August 25, 2025 16:10
}
if len(schema.Spec) == 0 {
return nil, fmt.Errorf("no spec found in schema file (path: %s)", l.source)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Safeguard added to avoid introducing this kind of issues again.

JohannesMahne
JohannesMahne previously approved these changes Aug 26, 2025

@JohannesMahne JohannesMahne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for picking this up and fixing it.

@jsoriano

Copy link
Copy Markdown
Contributor Author

test integrations

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

Created or updated PR in integrations repository to test this version. Check elastic/integrations#15050

@jsoriano

jsoriano commented Sep 1, 2025

Copy link
Copy Markdown
Contributor Author

Some settings are moved to definitions so they can be referenced and reused.

@jsoriano

jsoriano commented Sep 1, 2025

Copy link
Copy Markdown
Contributor Author

test integrations

@elasticmachine

Copy link
Copy Markdown

💚 Build Succeeded

History

cc @jsoriano

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

Created or updated PR in integrations repository to test this version. Check elastic/integrations#15108

number_of_shards:
$ref: "../../data_stream/manifest.spec.yml#/definitions/index_template_setting_number_of_shards"
number_of_routing_shards:
description: Number used when splitting shards.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure that this description field is in the final JSON schema (after merging the $ref). But in any case, at least that description could help the developers as it is part of the code.

@jsoriano jsoriano merged commit 4c347e5 into elastic:main Sep 2, 2025
3 checks passed
@jsoriano jsoriano deleted the version-patch-lookup-index branch September 2, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Suspected invalid 'required' JSONSchema attribute

4 participants