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

Test blocking write access to system indices elasticseach#81400 #120705

Closed
wants to merge 8 commits into from

Conversation

tylersmalley
Copy link
Contributor

@tylersmalley tylersmalley commented Dec 8, 2021

elastic/elasticsearch#81400

./gradlew :distribution:archives:linux-tar:assemble && sha512sum distribution/archives/linux-tar/build/distributions/elasticsearch-8.1.0-SNAPSHOT-linux-x86_64.tar.gz > elasticsearch-8.1.0-SNAPSHOT-linux-x86_64.tar.gz.sha512

@joshdover
Copy link
Contributor

It seems most of these remaining failures are happening on the main ES promotion build as well, so not sure they're related to this change.

@tylersmalley
Copy link
Contributor Author

Applied the change in #120985 and updated this branch.

@joshdover joshdover changed the title Test elasticseach#81400 Test blocking write access to system indices elasticseach#81400 Dec 13, 2021
@joshdover
Copy link
Contributor

While the upstream changes to block superuser from writing to system indices is still pending a larger decision, I'd like to get an idea of how much effort is involved from Kibana teams in order to handle this problem.

Essentially all write operations to system indices will need to use Kibana's internal user (aka kibana_system) and have any access to these operations guarded by separate privilege checks. In some cases, tests may be failing simply because test code would need to be updated to use the systemIndicesSuperuser I have added for interacting with these indices from test code (see https://github.com/elastic/kibana/pull/120705/files#diff-716197dcba910e912a201ff1b850ded1a42754bb6c641c9e187515349f6945f4R23-R28 as an example).

@elastic/kibana-reporting-services @elastic/kibana-stack-management Would someone from each of your teams be able to assess how much effort is required to resolve the issue with reporting and upgrade assistant detected in the tests above?

You're also welcome to push commits directly to this branch if the fix is easy and I'll keep this up to date.

@joshdover joshdover self-assigned this Dec 13, 2021
@cjcenizal
Copy link
Contributor

@sabarasaba Could you please take a look at the failing upgrade assistant reindexing should create a new index with the same documents test and address Josh's questions?

@sabarasaba
Copy link
Member

@joshdover UA requires read/delete privileges from the .tasks index. So in this case would be a matter of just changing the test config to use systemIndicesSuperuser as you point out. I cloned your branch locally so that I could patch it up but I wasn't able to make the test fail as it happens in the CI, is there any particular steps I should follow?

@joshdover
Copy link
Contributor

joshdover commented Dec 14, 2021

@sabarasaba

So in this case would be a matter of just changing the test config to use systemIndicesSuperuser as you point out.

systemIndicesSuperuser is only for tests, it doesn't actually exist in production. You'll want to use Kibana's internal user from core: core.elasticsearch.client.asInternalUser. If you have issues with this user, we may need to add delete privileges for this user for the .tasks index.

I cloned your branch locally so that I could patch it up but I wasn't able to make the test fail as it happens in the CI, is there any particular steps I should follow?

You need to be running the specific Elasticsearch branch to make tests fail. How to do this:

# Clone Elasticsearch, must be sibling to kibana checkout
git clone [email protected]:elastic/elasticsearch.git
cd elasticsearch
gh pr checkout 81400

# Back in Kibana checkout, run Elasticsearch from source
yarn es source

# For FTR you can start it with the --esFrom arg
node scripts/functional_tests_server --config=<...> --esFrom=source

@joshdover
Copy link
Contributor

@elasticmachine merge upstream

@joshdover
Copy link
Contributor

@tylersmalley would it be possible to run this with a newer Elasticsearch build based on elastic/elasticsearch#81400?

@tylersmalley
Copy link
Contributor Author

@joshdover yeah, I will work on that today.

@tylersmalley
Copy link
Contributor Author

@elasticmachine merge upstream

@tylersmalley
Copy link
Contributor Author

tylersmalley commented Dec 21, 2021

Updated ES build from e7e196251f2a4bb3ed49f80f8a9fe16405b3da99.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Dec 21, 2021

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Docker CI Group / Fleet Endpoints EPM Endpoints fleet_final_pipeline Should write the correct event.agent_id_status for API key with agent id metadata
  • [job] [logs] Docker CI Group / Fleet Endpoints EPM Endpoints fleet_final_pipeline Should write the correct event.agent_id_status for API key with agent id metadata
  • [job] [logs] Default CI Group #2 / Reporting APIs ILM policy migration APIs "before all" hook for "detects when no migration is needed"
  • [job] [logs] Default CI Group #2 / Reporting APIs ILM policy migration APIs "before all" hook for "detects when no migration is needed"
  • [job] [logs] Jest Tests #6 / setPasswords uses provided passwords
  • [job] [logs] Default CI Group #7 / upgrade assistant reindexing should create a new index with the same documents
  • [job] [logs] Default CI Group #7 / upgrade assistant reindexing should create a new index with the same documents

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
@kbn/test 180 182 +2

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
@kbn/test 4 5 +1
Unknown metric groups

API count

id before after diff
@kbn/test 203 206 +3

History

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

cc @joshdover

@joshdover
Copy link
Contributor

@sabarasaba @elastic/kibana-reporting-services @elastic/kibana-app-services We still need contributions here to make your plugins work correctly with the changes to system indices. See my comment above for info on how to test this: #120705 (comment)

@sabarasaba
Copy link
Member

@elastic/platform-deployment-management part should be good to go now with the next merge upstream due to the changes introduced in this pr

@joshdover joshdover closed this Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants