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

Deprecation Info API cannot check filtered settings #37845

Closed
gwbrown opened this issue Jan 25, 2019 · 4 comments
Closed

Deprecation Info API cannot check filtered settings #37845

gwbrown opened this issue Jan 25, 2019 · 4 comments
Assignees
Labels

Comments

@gwbrown
Copy link
Contributor

gwbrown commented Jan 25, 2019

As noted in #37793 (comment), the Deprecation Info API uses the NodeInfo API to check for problems with settings. This means that issues involving filtered settings cannot be detected by the Deprecation Info API.

This issue was masked by the fact that we only have unit tests for the node deprecation checks, which do not go through the filtering process.

This impacts a few different checks:

We do not want to allow filtered settings to be accessed via the NodeInfo API, as there is a reason these settings are filtered: they typically contain sensitive information.

One possible solution is to change how the Deprecation API operates: instead of retrieving the settings from the nodes and performing the checks on the master node, we could have each node perform the checks on its own settings and send the results back to the master. This would allow filtered settings to be examined in deprecation checks, but add significant complexity and create problems in mixed clusters unless it is done carefully.

Another possible solution is to simply say that these issues cannot be detected via the Deprecation Info API, and the user will have to check the deprecation log to find out about these issues. We have done this for one potential issue, the changed file path for file-based discovery, which is not detectable through the NodeInfo API: #36190. If we go this route, we may want to consider returning a WARNING level issue (or resurrecting INFO) if Watcher/Security are enabled reminding users to check the deprecation logs and informing them of these changes we cannot detect.

/cc @bleskes for awareness

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@tvernum
Copy link
Contributor

tvernum commented Jan 25, 2019

We could run these sorts of check only on the local node. That's not ideal obviously, but might be preferable to removing them entirely.
For the the SSL checks, I'd expect more than 90% of clusters have very similar settings per node (the cert + key might be different, but the protocols, etc would not be).
And for the few users with different settings, it's likely that the node they run this one would be representative of the upgrade issues they might run into for the overall cluster.

@bleskes
Copy link
Contributor

bleskes commented Jan 25, 2019

Ideally I would go to move the node depreation API to use a dedicated sub-action that derives from TransportBroadcastByNodeAction. I think that's the right thing to do long term in order to achieve good results.

Looking at the code I don't think it will be major bwc hassle because we currently only check the local node new NodesInfoRequest("_local") so it's very contained.

Last - we should be very careful about which information we expose and whether users have the right to see them (not saying they don't but rather that we should be careful about).

@gwbrown
Copy link
Contributor Author

gwbrown commented Feb 2, 2019

Resolved by #38065

@gwbrown gwbrown closed this as completed Feb 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants