Add flags to allow for routing to dynamic datasets and namespaces #327
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
|
I made a different proposal directly in the spec to use existing Elasticsearch primitives. Instead of having something |
|
@joshdover @jsoriano @aspacca Would be good to get some additional opinions on the discussion in #327 (comment) Most important, lets get this moving forward. |
There was a problem hiding this comment.
One suggestion on the description but overall LGTM.
@mtojek, question: do we have a way to specify new fields in the package-spec that aren’t considered “stable”? It’d be nice to be able to add fields to the spec to allow experimentation with new features before we make a hard commitment to it. We may want to explore other ways of configuring a feature in the future.
That said, I’m ok with this one going forward anyways, since it supporting it from the Fleet side should be quite trivial, but just something I think we should be thinking about. Being able to mark a package-spec field as beta or experimental could be valuable.
Co-authored-by: Josh Dover <[email protected]>
|
@joshdover It would be great if the PR for this feature on the Fleet side could be opened in parallel, so it can be merged at the same time. Otherwise I'm worried we have the case that it is in the for but not in Fleet yet. |
I'm afraid that we don't have it at the moment as it seems to be hard to ensure compatibility with stacks. Consider the case, when the field is beta, but its type changes once it's GA. It's hard to predict the consequences of it. |
There was a problem hiding this comment.
LGTM from the Ecosystem team, let's wait for @joshdover's official approval.
I started implementing this in elastic/kibana#131609 but found that it’s going to be dependent on elastic/kibana#115032 which I am assigned to for 8.3 and may be able to finish up this week. The issue is that we’re not reading the data stream information directly from the packages today, we’re using the registry API’s summary during package installation. We want to stop duplicating all of these fields that are only used during install and pull this info directly from the package instead. I’ll follow up finishing this up after the blocking task is done. The amount of work is very small. |
|
Are the dependent tasks still expected to land in 8.3? |
They got delayed, but I have requested they are included for 8.4. |
| @@ -1,5 +1,6 @@ | |||
| title: Nginx access logs | |||
| type: logs | |||
| allow_routing: true | |||
There was a problem hiding this comment.
As this is related to elasticsearch configuration, I think it could be at the root level of elasticsearch, even if it doesn't belong to the elasticsearch.privileges object #327 (comment).
We did something similar for the time series mode in https://github.com/elastic/package-spec/pull/357/files.
There was a problem hiding this comment.
@felixbarny is this change still required? Wdyt about placing this parameter under elasticsearch object?
There was a problem hiding this comment.
Yes, this is still required. Are all the blockers resolved now? I don’t care so much about where the flag is. Happy to change it to be under elasticsearch if you think that’s a better place.
There was a problem hiding this comment.
This blocker was solved elastic/kibana#115032, not sure if there were any others.
Yes, please update the branch to put the new setting under elasticsearch. We removed the versions directory, so you will also need to move the changes to the new structure.
There was a problem hiding this comment.
I've moved it and renamed and split the property so that it's both more self-explanatory and flexible.
Also moves to elasticsearch.privileges
felixbarny
changed the title
🌐 Coverage report
|
I guess it is this one elastic/kibana#134971 |
|
We've had a use case come up that this is close to solving but doesn't quite hit the mark. Users may want to add custom routing to ingest pipelines created for input-type packages to route to more specific pipelines. To do this, it'd be required that the API key granted to the Agent running the integration has adequate privileges. Ideally, this wouldn't be turned on by default, and instead would be an option in the integration policy, rather than always on for all instances of an input package. @jsoriano what do you think about extending this package-spec option to be |
Is this something that we would want for some packages but not for others? If we add the flag to the package spec, we are making package developers to take the decision on this, but this looks like something that the user should decide. If a user adds custom routing using the integration policy editor, Fleet could (should?) ensure that the Agent receives an API key with adequate privileges. This doesn't look like a responsibility of the package developer. |
What does this PR do?
Adds
data_stream.elasticsearch.dynamic_dataset/data_stream.elasticsearch.dynamic_namespace.When set to true, instead of granting permissions to the exact data stream the integration is sending data to (for example
logs-router-default), it grants permissions for all datasets and namespaces, respectively, for thedata_stream.type(for example (logs-*-*).Why is it important?
This is required for event routing. Instead of creating an API key that has permissions for an exact data stream, such as
logs-generic-default, in event routing situations, the target data stream is determined within an ingest node pipeline.See also
Checklist
test/packagesthat prove my change is effective.versions/N/changelog.yml.Related issues