-
Notifications
You must be signed in to change notification settings - Fork 87
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
Fix application name in API key #1091
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for fixing this
I wonder if we need to get this also into 7.17 as a 7.17 fleet-server could talk to an Elasticsearch 8.0 (x?) instance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -22,7 +22,7 @@ const testFleetRoles = ` | |||
"fleet-apikey-access": { | |||
"cluster": [], | |||
"applications": [{ | |||
"application": ".fleet", | |||
"application": "fleet", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have a copy here instead of using the one from the actual implementation? Using a copy means we could have a passing test but the actual implementation does not work.
@ruflin it seems to be the same issue mentioned there elastic/beats#29811 And it's causing this e2e-tests to fail. cc @ph |
I can confirm on our side that this fix solved the issue for elastic-package (fresh Docker images baked). The PR was green, didn't see any failing tests. |
@ruflin Yes this needs to go to 7.17. Fleet Server needs to be able to communicate with an 8.0 elasticsearch. |
This PR fixes issue spotted in: elastic/elastic-package#650