Support configuring dynamic dataset and namespace in input packages - #1140
Support configuring dynamic dataset and namespace in input packages#1140jsoriano wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThis PR adds two new optional Elasticsearch configuration fields to the input package manifest specification: ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/input/manifest.spec.yml`:
- Around line 158-161: Add a version patch to gate the newly added fields
dynamic_dataset and dynamic_namespace so they are removed for older spec format
versions; update the manifest.spec.yml patching logic (the
elasticsearch_dynamic_dataset / elasticsearch_dynamic_namespace additions under
dynamic_dataset and dynamic_namespace refs) to include a backward-compatibility
version patch that deletes those properties for legacy format_version values
(also apply same patching for the related additions in the 173-220 range) so
older spec versions do not expose the new schema surface.
In `@spec/integration/data_stream/manifest.spec.yml`:
- Around line 250-255: Move the shared field definitions for
elasticsearch_dynamic_dataset and elasticsearch_dynamic_namespace out of the
data_stream spec and into the integration manifest's shared definitions section;
then replace the local definitions with $ref entries that point (relatively) to
those centralized definitions so both the data_stream and input specs reuse the
same definition. Ensure the centralized definitions include the same description
and type boolean, remove the duplicate blocks from the original file (the
elasticsearch_dynamic_dataset and elasticsearch_dynamic_namespace blocks), and
update any occurrences to reference the manifest definitions via relative $ref
paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f7aaf640-8075-4647-b668-f534bff0282d
📒 Files selected for processing (4)
spec/changelog.ymlspec/input/manifest.spec.ymlspec/integration/data_stream/manifest.spec.ymltest/packages/good_input/manifest.yml
| dynamic_dataset: | ||
| $ref: "../integration/data_stream/manifest.spec.yml#/definitions/elasticsearch_dynamic_dataset" | ||
| dynamic_namespace: | ||
| $ref: "../integration/data_stream/manifest.spec.yml#/definitions/elasticsearch_dynamic_namespace" |
There was a problem hiding this comment.
Add a version patch to gate the new input fields by spec version.
dynamic_dataset and dynamic_namespace are added, but older versions are not patched to remove them. This leaks new schema surface into legacy format_versions.
Proposed patch
versions:
+ - before: 3.6.1
+ patch:
+ - op: remove
+ path: "/properties/elasticsearch/properties/dynamic_dataset"
+ - op: remove
+ path: "/properties/elasticsearch/properties/dynamic_namespace"
- before: 3.6.0
patch:
- op: remove
path: "/properties/var_groups" # removes var_groups from package rootAs per coding guidelines, "Version patches enable backward compatibility by removing features from older spec versions."
Also applies to: 173-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/input/manifest.spec.yml` around lines 158 - 161, Add a version patch to
gate the newly added fields dynamic_dataset and dynamic_namespace so they are
removed for older spec format versions; update the manifest.spec.yml patching
logic (the elasticsearch_dynamic_dataset / elasticsearch_dynamic_namespace
additions under dynamic_dataset and dynamic_namespace refs) to include a
backward-compatibility version patch that deletes those properties for legacy
format_version values (also apply same patching for the related additions in the
173-220 range) so older spec versions do not expose the new schema surface.
| elasticsearch_dynamic_dataset: | ||
| description: When set to true, agents running this integration are granted data stream privileges for all datasets of its type | ||
| type: boolean | ||
| elasticsearch_dynamic_namespace: | ||
| description: When set to true, agents running this integration are granted data stream privileges for all namespaces of its type | ||
| type: boolean |
There was a problem hiding this comment.
Move shared dynamic field definitions to the integration manifest definitions file.
These definitions are now shared by multiple specs (spec/integration/data_stream/manifest.spec.yml and spec/input/manifest.spec.yml), but they are defined in the data_stream spec file instead of spec/integration/manifest.spec.yml.
As per coding guidelines, "When a field is used in multiple locations, define it once in spec/integration/manifest.spec.yml and reference it using relative paths."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/integration/data_stream/manifest.spec.yml` around lines 250 - 255, Move
the shared field definitions for elasticsearch_dynamic_dataset and
elasticsearch_dynamic_namespace out of the data_stream spec and into the
integration manifest's shared definitions section; then replace the local
definitions with $ref entries that point (relatively) to those centralized
definitions so both the data_stream and input specs reuse the same definition.
Ensure the centralized definitions include the same description and type
boolean, remove the duplicate blocks from the original file (the
elasticsearch_dynamic_dataset and elasticsearch_dynamic_namespace blocks), and
update any occurrences to reference the manifest definitions via relative $ref
paths.
|
I think it was actually intentional to have all input packages work as if they always had dynamic dataset and namespace. To support dynamic routing. See discussions about this in: |
💚 Build Succeeded
cc @jsoriano |
2 participants
What does this PR do?
Add support to configure dynamic dataset and namespace in input packages.
Why is it important?
There can be generic inputs that can store documents, or their documents can be rerouted, to multiple datasets or namespaces.
Currently Kibana is adding permissions for input packages depending on the kind of input, instead of that, it should give the minimum set of permissions, and developers should use
dynamic_dataset/dynamic_namespaceif permissions for more datasets are required.Checklist
test/packagesthat prove my change is effective.spec/changelog.yml.Related issues