-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Enroll Kibana API uses Service Accounts #76370
Changes from 9 commits
c84c36f
39d6b92
d23a85b
29cf973
0180b16
2e9c380
7f204e7
b9280b2
52a3ac4
89d0f38
8a3e89f
cd73a19
ed7e2fe
4dce318
cca510c
3ab69c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2875,7 +2875,7 @@ public void onFailure(Exception e) { | |
} | ||
} | ||
|
||
@AwaitsFix(bugUrl = "Determine behavior for keystores with multiple keys") | ||
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/75097") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really blocked by this issue? I think it shouldn't be unless we use truststore containing multiple CA certs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, we should have re-enabled this test as part of #73807. Will take care of it on Monday There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, not really. EnrollIT means to test the HLRC behavior where the *DocumentationIT means to test that our documentation snippets are accurate and work |
||
public void testNodeEnrollment() throws Exception { | ||
RestHighLevelClient client = highLevelClient(); | ||
|
||
|
@@ -2918,7 +2918,7 @@ public void onFailure(Exception e) { | |
} | ||
} | ||
|
||
@AwaitsFix(bugUrl = "Determine behavior for keystores with multiple keys") | ||
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/75097") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above ^^ |
||
public void testKibanaEnrollment() throws Exception { | ||
RestHighLevelClient client = highLevelClient(); | ||
|
||
|
@@ -2928,10 +2928,10 @@ public void testKibanaEnrollment() throws Exception { | |
// end::kibana-enrollment-execute | ||
|
||
// tag::kibana-enrollment-response | ||
SecureString password = response.getPassword(); // <1> | ||
SecureString token = response.getToken(); // <1> | ||
String httoCa = response.getHttpCa(); // <2> | ||
// end::kibana-enrollment-response | ||
assertThat(password.length(), equalTo(14)); | ||
assertNotNull(token); | ||
} | ||
|
||
{ | ||
|
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.
@azasypkin would Kibana want to know the token name in addition to the value ? ( https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-create-service-token.html )
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.
A explicit token name might be convenient for kibana, but the token value itself also has the token name embedded. The token value takes the format of
\0\1\0\1elastic/kibana-system/$tokenName:$tokenSecret
and is base64 encoded.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.
Yeah, I think it'd be better to have one (to log it least).
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.
From what I can gather we only need the service account token to authenticate so don't think we need the name at all.
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.
Right, we don't need it in the code, but I thought it'd be beneficial to log it anyway, to know which Kibana triggered generation of which token, for troubleshooting purposes.
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.
To be honest, for consistency, it might make sense to mirror the create service token API anyways:
Learn once, use anywhere, etc.
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.
coo, I'll make the change to return the name too, thanks for weighing in folks