Skip to content
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 support for allow_routing data streams option #131609

Closed
wants to merge 1 commit into from

Conversation

joshdover
Copy link
Contributor

@joshdover joshdover commented May 5, 2022

Summary

Explores adding support for elastic/package-spec#327, but it turns out we need to do #115032 first.

Doesn't work yet.

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

@joshdover joshdover added the Team:Fleet Team label for Observability Data Collection Fleet team label May 5, 2022
@joshdover joshdover force-pushed the fleet/allow-routing branch from 6dfe4fb to 4ceb955 Compare May 5, 2022 10:59
@@ -98,6 +98,7 @@ export async function storedPackagePoliciesToAgentPermissions(
type: stream.data_stream.type,
dataset:
stream.compiled_stream?.data_stream?.dataset ?? stream.data_stream.dataset,
allow_routing: stream.data_stream.allow_routing,
Copy link
Member

Choose a reason for hiding this comment

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

If this is going to give privileges on data streams outside of the ones of this package, shall we warn the user when this privilege is given? Otherwise any package could use this setting to inadvertently obtain too much privileges.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like a good idea. Do you have a suggestion on how and were to warn the user?

Copy link
Member

Choose a reason for hiding this comment

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

I guess that this would need to be done when installing a package with this flag enabled.

@joshdover joshdover force-pushed the fleet/allow-routing branch from 4ceb955 to d67bef4 Compare October 11, 2022 12:48
@kibana-ci
Copy link
Collaborator

kibana-ci commented Oct 11, 2022

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #19 / endpoint Endpoint permissions: "after all" hook in "Endpoint permissions:"
  • [job] [logs] FTR Configs #19 / endpoint Endpoint permissions: "after all" hook in "Endpoint permissions:"
  • [job] [logs] FTR Configs #19 / endpoint Endpoint permissions: "before all" hook in "Endpoint permissions:"
  • [job] [logs] FTR Configs #19 / endpoint Endpoint permissions: "before all" hook in "Endpoint permissions:"
  • [job] [logs] FTR Configs #19 / endpoint For each artifact list under management When on the Blocklist entries list "after all" hook for "should be able to delete the existing Blocklist entry"
  • [job] [logs] FTR Configs #19 / endpoint For each artifact list under management When on the Blocklist entries list "after all" hook for "should be able to delete the existing Blocklist entry"
  • [job] [logs] FTR Configs #19 / endpoint For each artifact list under management When on the Blocklist entries list "before all" hook for "should not show page title if there is no Blocklist entry"
  • [job] [logs] FTR Configs #19 / endpoint For each artifact list under management When on the Blocklist entries list "before all" hook for "should not show page title if there is no Blocklist entry"
  • [job] [logs] FTR Configs #19 / endpoint For each artifact list under management When on the Host isolation exceptions entries list "after all" hook for "should be able to delete the existing Host isolation exceptions entry"
  • [job] [logs] FTR Configs #19 / endpoint For each artifact list under management When on the Host isolation exceptions entries list "after all" hook for "should be able to delete the existing Host isolation exceptions entry"
  • [job] [logs] FTR Configs #19 / endpoint For each artifact list under management When on the Host isolation exceptions entries list "before all" hook for "should not show page title if there is no Host isolation exceptions entry"
  • [job] [logs] FTR Configs #19 / endpoint For each artifact list under management When on the Host isolation exceptions entries list "before all" hook for "should not show page title if there is no Host isolation exceptions entry"
  • [job] [logs] FTR Configs #19 / endpoint Response Actions Responder "before all" hook in "Response Actions Responder"
  • [job] [logs] FTR Configs #19 / endpoint Response Actions Responder "before all" hook in "Response Actions Responder"
  • [job] [logs] FTR Configs #19 / endpoint When on the Trusted Apps list "after all" hook for "should be able to add a new trusted app and remove it"
  • [job] [logs] FTR Configs #19 / endpoint When on the Trusted Apps list "after all" hook for "should be able to add a new trusted app and remove it"
  • [job] [logs] FTR Configs #19 / endpoint When on the Trusted Apps list "before all" hook for "should not show page title if there is no trusted app"
  • [job] [logs] FTR Configs #19 / endpoint When on the Trusted Apps list "before all" hook for "should not show page title if there is no trusted app"
  • [job] [logs] Jest Tests #9 / Fleet - packageToPackagePolicy packageToPackagePolicy returns package policy with multiple policy templates (aka has integrations
  • [job] [logs] FTR Configs #45 / Fleet Endpoints Package Policy - create Simplified package policy should work with valid values
  • [job] [logs] FTR Configs #45 / Fleet Endpoints Package Policy - create Simplified package policy should work with valid values
  • [job] [logs] FTR Configs #45 / Fleet Endpoints Package Policy - update "before all" hook for "should work with valid values on "regular" policies"
  • [job] [logs] FTR Configs #45 / Fleet Endpoints Package Policy - update "before all" hook for "should work with valid values on "regular" policies"
  • [job] [logs] FTR Configs #45 / Fleet Endpoints Package Policy - upgrade when upgrading from an integration package to an input package where a required variable has been added "before each" hook for "returns a diff with errors"
  • [job] [logs] FTR Configs #45 / Fleet Endpoints Package Policy - upgrade when upgrading from an integration package to an input package where a required variable has been added "before each" hook for "returns a diff with errors"
  • [job] [logs] Jest Tests #9 / toPackagePolicy With nginx package should work

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
fleet 893 894 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fleet 112.8KB 112.9KB +62.0B
Unknown metric groups

API count

id before after diff
fleet 996 997 +1

ESLint disabled in files

id before after diff
apm 14 13 -1
observability 8 7 -1
total -2

ESLint disabled line counts

id before after diff
apm 81 79 -2
observability 45 44 -1
securitySolution 411 407 -4
synthetics 60 54 -6
ux 10 9 -1
total -14

Total ESLint disabled count

id before after diff
apm 95 92 -3
observability 53 51 -2
securitySolution 484 480 -4
synthetics 66 60 -6
ux 13 12 -1
total -16

History

  • 💔 Build #42757 failed 4ceb9551d43c9b80a8c83d78ad149ef4a404f563
  • 💔 Build #42739 failed 6dfe4fbc1b0fc8fb69a988689cdf1c73e671ba96

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@joshdover
Copy link
Contributor Author

I updated this PR with the latest changes from #115032

I didn't quite get it to work yet with the correct permissions ending up in the resulting policy, but I think there's an issue with my test package. I will follow up on this by EOW.

@felixbarny
Copy link
Member

Thanks for picking this up, Josh. What are the next steps here?

@joshdover
Copy link
Contributor Author

There's some conflicts between this and other work for input packages that we're working through. It also seems there's additional code paths that are fetching details from the registry that we need to remove to unblock this and input packages. I'm working with @hop-dev in #143198 to find a solution.

@joshdover
Copy link
Contributor Author

Already implemented

@joshdover joshdover closed this Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants