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

[Fleet] Create enrollment API keys as current user #96464

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Apr 7, 2021

Summary

As part of the move to fleet server we will remove the current fleet_enroll user. That user was used to create API keys.
In that PR we change that to use the current user to create API keys instead.

I also removed useless argument to the invalidate calls

Also I added metatada to the enrollment API key.

How to test

This PR should not change anything.

  • the fleet agent setup should create API keys
  • you should be able to create and invalidate API keys.

Automated test

this part of the code is covered by the fleet api integration tests

@nchaulet nchaulet added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Apr 7, 2021
@nchaulet nchaulet self-assigned this Apr 7, 2021
@nchaulet nchaulet requested a review from a team as a code owner April 7, 2021 17:28
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@nchaulet nchaulet requested a review from jen-huang April 7, 2021 17:29
@nchaulet nchaulet force-pushed the feature-create-enrollment-keys-using-current-user branch from 22946bd to e336deb Compare April 7, 2021 17:32
@nchaulet nchaulet requested a review from jfsiii April 8, 2021 01:13
resources: ['*'],

const { body: key } = await esClient.security
.createApiKey({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add some metadata to these enrollment keys?

Maybe it would be nice to have this defined as a constant outside which makes it easier to document and reference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we could add some metadata, what do you have in mind here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In elastic/fleet-server#195 we have:

"metadata" : {
        "agent_id" : "e4dede19-759e-45d5-b08f-0e78dec888e5",
        "managed_by" : "fleet-server",
        "managed" : true,
        "type" : "output"
      }

We could do something similar here:

"metadata" : {
        "managed_by" : "fleet",
        "managed" : true,
        "type" : "enroll" (or similar)
        "policy_id": "1234"
      }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great add this here 9833563

@nchaulet nchaulet requested review from ruflin and Zacqary April 8, 2021 13:02
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not test this locally but overall LGTM. Would be good if someone could do a more granular check.

body: {
name,
// @ts-expect-error Metadata in api keys
metadata: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nchaulet Do you know if metadata is the key that has to be used here or could we just use any key?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @nchaulet

@nchaulet nchaulet merged commit 1de77cc into elastic:master Apr 12, 2021
@nchaulet nchaulet deleted the feature-create-enrollment-keys-using-current-user branch April 12, 2021 12:27
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 12, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

phillipb added a commit to phillipb/kibana that referenced this pull request Apr 12, 2021
…to-metrics-tab

* 'master' of github.com:elastic/kibana: (44 commits)
  [Exploratory View]Additional metrics for kpi over time (elastic#96532)
  [Fleet] UI changes on hosted policy detail view (elastic#96337)
  Stacked line charts incorrectly shows one term as 100% (elastic#96203)
  [Fleet] Create enrollment API keys as current user (elastic#96464)
  [Lens] Make table and metric show on top Chart switcher (elastic#96601)
  skip flaky suite (elastic#96691)
  [Lens] Hide "Show more errors" once expanded (elastic#96605)
  [Discover] Unskip histogram hiding test (elastic#95759)
  skip flyout test, add linked issue elastic#96708
  skip copy_to_space_flyout_internal.test.tsx elastic#96708
  fix config validation (elastic#96502)
  Document telemetry fields for stack security features (elastic#96638)
  [Partial Results] Move inspector adapter integration into search source (elastic#96241)
  [RAC] Rule registry plugin (elastic#95903)
  [APM] Run precommit tasks sequentially (elastic#96551)
  [Maps] fix Kibana does not recognize a valid geo_shape index when attempting to create a Tracking Containment alert (elastic#96633)
  [Security Solution] [Cases] Small UI bugfixes (elastic#96511)
  [Actions UI] Changed PagerDuty action form UI to fill payload fields according to the API docs for Resolve and Acknowledge events. (elastic#96363)
  App Search: Result Component Updates (elastic#96184)
  [Alerting] Preconfigured alert history index connector (elastic#94909)
  ...
kibanamachine added a commit that referenced this pull request Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants