-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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] Add consistent _meta
property to all Fleet ES assets
#119380
[Fleet] Add consistent _meta
property to all Fleet ES assets
#119380
Conversation
Pinging @elastic/fleet (Team:Fleet) |
@@ -36,10 +40,14 @@ export const FLEET_GLOBAL_COMPONENT_TEMPLATE_CONTENT = { | |||
}, | |||
}; | |||
|
|||
export const FLEET_FINAL_PIPELINE_VERSION = 1; | |||
export const FLEET_FINAL_PIPELINE_VERSION = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nchaulet - I wasn't 100% sure whether we should increment this pipeline version since we're adding _meta
below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think incrementing the version will work it will trigger to update the pipeline to the latest version on fleet setup.
// If the content is updated you probably need to update the FLEET_FINAL_PIPELINE_VERSION too to allow upgrade of the pipeline | ||
export const FLEET_FINAL_PIPELINE_CONTENT = `--- | ||
version: ${FLEET_FINAL_PIPELINE_VERSION} | ||
_meta: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interpolating the full _meta
object generated by getESAssetMetadata()
proved difficult, so I elected to just interpolate the individual fields here. We don't need package
for this global pipeline, but this may need to be revisited if we ever add more data to our _meta
object.
if (pipeline.extension === 'yml') { | ||
// Convert the YML content to JSON, append the `_meta` value, then convert it back to | ||
// YML and return the resulting YML | ||
const parsedPipelineContent = safeLoad(pipeline.contentForInstallation); | ||
parsedPipelineContent._meta = meta; | ||
|
||
return { | ||
...pipeline, | ||
contentForInstallation: `---\n${safeDump(parsedPipelineContent)}`, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This proved to be a bit of a tricky use case here. I felt that our choices to insert the _meta
field into a pipeline's yml data were either to do some kind of string replacement, or parse the yml -> add the value -> re-convert back into yml. I opted for the second choice here. Open to suggestions here certainly.
|
||
return { | ||
...pipeline, | ||
contentForInstallation: `---\n${safeDump(parsedPipelineContent)}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed to re-add the yml document start marker ---
in order for the ES client to be happy. safeDump
seems to remove it.
return { | ||
installationName: getTransformNameForInstallation( | ||
installablePackage, | ||
path, | ||
installNameSuffix | ||
), | ||
content: getAsset(path).toString('utf-8'), | ||
content, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't content
have to be converted back to string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The eventual putTransform
call that uses this body actually expects an object, but I believe the ES client calls parse strings to JSON generally so that's why this was working before. Elsewhere is the same story - ES client calls accept either a string that will be parsed to JSON or a Record<string, any>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @kpollich |
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…ic#119380) * Add consistent _meta property to all Fleet ES assets * Fix meta logic for ILM + transforms * Fix tests + snapshots * Fix failing tests
…) (#119524) * Add consistent _meta property to all Fleet ES assets * Fix meta logic for ILM + transforms * Fix tests + snapshots * Fix failing tests Co-authored-by: Kyle Pollich <kyle.pollich@elastic.co>
* Add consistent _meta property to all Fleet ES assets * Fix meta logic for ILM + transforms * Fix tests + snapshots * Fix failing tests
…ic#119380) * Add consistent _meta property to all Fleet ES assets * Fix meta logic for ILM + transforms * Fix tests + snapshots * Fix failing tests
Summary
Resolves #111737
Adds/alters the
_meta
property on Elasticsearch assets generated by Fleet to consistently indicate that the assets come from Fleet, and when possible tie them back to an integration.To Test
_meta
field of the formatIn cases like the
.fleet-final-pipeline
ingest pipeline and others, thepackage
value will be omitted.To verify assets, use these Kibana dev tools queries:
Examples