kibana icon indicating copy to clipboard operation
kibana copied to clipboard

[Fleet] Improve enrollment API keys OpenAPI spec

Open jillguyonnet opened this issue 1 year ago • 5 comments

Summary

Relates https://github.com/elastic/kibana/issues/155550 Relates https://github.com/elastic/kibana/issues/190402

Users have reported unclear expectations from Fleet Enrollment API Keys API. These include:

  • Enrollment token names must be unique (impacts token creation).
  • Deleting an enrollment token doesn't actually delete, it revokes (invalidates it, marks it as inactive).
  • Revoked tokens are still returned (with "active": false) by GET /api/fleet/enrollment_api_keys.
  • It's not clear what happens to the API key after it's been invalidated.

This PR adds some details to the OpenAPI spec.

I believe, however, that some improvements could also be brought to the documentation itself in order to fully address the above concerns. The Fleet enrollment tokens doc page already describes enrollment token creation and deletion, perhaps the following information could be added:

  • Why the token name must be unique.
  • Add an explanation around revoking tokens (marked as inactive, will be removed after expiration) with a link to the retention period setting in Elasticsearch.
  • A mention that inactive tokens are still returned by the GET API and visible in the UI until they are cleaned up.

A final note around the variations in terminology (not a big issue, I think):

  • "enrollment tokens" in the docs vs. "enrollment API keys" in the OpenAPI spec (I thought it would be best to keep the latter consistent)
  • "revoke token" vs. "invalidate API key"

jillguyonnet avatar Aug 28 '24 09:08 jillguyonnet

:robot: GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

obltmachine avatar Aug 28 '24 09:08 obltmachine

Pinging @elastic/fleet (Team:Fleet)

elasticmachine avatar Aug 28 '24 13:08 elasticmachine

Thanks @kilfoyle! I've pushed the suggested change. If you could approve again now that the PR has been marked as ready, I think that would pick up the review from @elastic/platform-docs.

Also, could you let me know your thoughts above my suggestions regarding Fleet docs? If you agree with them, should a separate issue be filed or is the original one enough?

jillguyonnet avatar Aug 28 '24 13:08 jillguyonnet

@elasticmachine merge upstream

jillguyonnet avatar Aug 28 '24 13:08 jillguyonnet

@jillguyonnet Oops! I didn't read carefully. :-)

I've opened a separate docs issue for your suggestions: https://github.com/elastic/ingest-docs/issues/1270

For Why the token name must be unique. do you have a suggestion of what I can add for that? That is, I'm only aware that they have to be unique but I'm not sure exactly why.

For the terminology, the docs need to match the UI, so perhaps the best thing would be just to add a note on the Fleet enrollment tokens doc page explaining that enrollment token = enrollment API key. Would that work?

And you're right - reapproving made the platform-docs review requirement disappear. 🎉


Add: I should be able to get to the docs issue in the next 2 - 3 weeks.

kilfoyle avatar Aug 28 '24 13:08 kilfoyle

Thanks @kilfoyle, I will link your issue to the original one 👍

Regarding why the token name must be unique, I've asked for a clarification in https://github.com/elastic/kibana/issues/155550#issuecomment-2314866970, so maybe stay tuned for a more precise answer 🙂

For the terminology, the docs need to match the UI, so perhaps the best thing would be just to add a note on the Fleet enrollment tokens doc page explaining that enrollment token = enrollment API key. Would that work?

Yes, great suggestion 👍 Can you add it to https://github.com/elastic/ingest-docs/issues/1270?

jillguyonnet avatar Aug 28 '24 14:08 jillguyonnet

@jillguyonnet I've updated the ingest-docs issue with the details you mentioned. Thanks a lot for checking on why the token name must be unique. I'll follow along in that thread.

kilfoyle avatar Aug 28 '24 14:08 kilfoyle

@elasticmachine merge upstream

jillguyonnet avatar Aug 29 '24 07:08 jillguyonnet

:green_heart: Build Succeeded

Metrics [docs]

✅ unchanged

History

  • :broken_heart: Build #230442 failed 4cac144f66e2fb793a74562f6a054f0ffff184fc
  • :broken_heart: Build #230440 failed 5ca402e381b3f5bd76c1733a3f532551aba0201c

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

cc @jillguyonnet

kibana-ci avatar Aug 29 '24 08:08 kibana-ci