[Upgrade Assistant] Minimize reindex attributes used to create credential hash #123727
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alisonelizabeth
added
bug
|
Pinging @elastic/platform-deployment-management (Team:Deployment Management) |
sebelga
approved these changes
Jan 26, 2022
| createHash('sha256') | ||
| .update(stringify({ id: reindexOp.id, ...reindexOp.attributes })) | ||
| const getHash = (reindexOp: ReindexSavedObject) => { | ||
| const { reindexOptions, ...attributes } = reindexOp.attributes; |
There was a problem hiding this comment.
Can we add a comment to mention that we exclude the reindexOptions for now as it creates an unstable hash. This needs to be investigated (with link to #123752)
There was a problem hiding this comment.
👍 thanks for the reminder! Meant to do this and forgot to push it up.
|
@elasticmachine merge upstream |
sabarasaba
approved these changes
Jan 26, 2022
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
alisonelizabeth
added a commit
to alisonelizabeth/kibana
that referenced
this pull request
Jan 26, 2022
alisonelizabeth
added a commit
that referenced
this pull request
Jan 27, 2022
alisonelizabeth
added a commit
to alisonelizabeth/kibana
that referenced
this pull request
Jan 27, 2022
…tial hash (elastic#123727) (elastic#123864) (cherry picked from commit 94e64e4)
alisonelizabeth
added a commit
that referenced
this pull request
Jan 27, 2022
awahab07
pushed a commit
to awahab07/kibana
that referenced
this pull request
Jan 31, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #123616
It was determined that the reindexOp SO attributes were changing from the time the credentials are initially set to when the reindex worker requests them.
The diff was caused by the addition of the
openAndCloseparameter. I believe this is related to changes made in #59890 (originally implemented via #58890).For now, I am excluding the
reindexOptionsfrom the credential hash. This seems like the least obtrusive change given the time constraints. I've opened #123752 as a follow-up issue to continue investigating and potentially consider updating the credential store with the updated reindexOptions as an alternative.How to test
Steps to reproduce are listed in #123616. Note that Kibana must be stopped/restarted on the third step of the reindexing process - "Reindex documents" - in order to reproduce.
With this change, you should be able to click "Resume reindexing" and the reindexing process will finish successfully.
There are some existing functional tests that run through the reindexing flow, as well as jest tests for the credential store. I'm not sure if there's a good way to test this specific scenario by restarting Kibana.