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.

Sign up for GitHub

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

Jump to bottom

[Fleet] Better display of fleet requirements #65027

Merged
merged 3 commits into from
May 4, 2020
Merged

[Fleet] Better display of fleet requirements #65027

merged 3 commits into from
May 4, 2020

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented May 3, 2020

Summary

Resolve #63637 #64519

To run fleet an user should:

  • Have API key enabled
  • If Kibana is in production mode have TLS enabled

If this requirements are not met we display an error message when the user try to acced fleet

API Change

GET /api/ingest_maanger/fleet/setup
{
 isReady: false,
 missing_requirements=["api_keys", "tls_required", "fleet_admin_user"]
}

UI Change

Screen Shot 2020-05-03 at 5 56 49 PM

Screen Shot 2020-05-03 at 5 54 20 PM

Screen Shot 2020-05-03 at 6 01 10 PM

@nchaulet nchaulet added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.8.0 Team:Fleet Team label for Observability Data Collection Fleet team labels May 3, 2020
@nchaulet nchaulet requested a review from a team May 3, 2020 22:03
@nchaulet nchaulet self-assigned this May 3, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

jfsiii
jfsiii approved these changes May 4, 2020
View reviewed changes
Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

Tested & confirmed locally.

starting ES with yarn es snapshot -E xpack.security.authc.api_key.enabled=true yields a working Fleet.

Using yarn es snapshot only produces the messages shown in the description.

I commented w/ some changes re: Cloud I'd like to add, but I think the worst case is it'll fail silently and can be addressed in a follow-up.

x-pack/plugins/ingest_manager/server/plugin.ts
@@ -184,6 +196,9 @@ export class IngestManagerPlugin
security: this.security,
config$: this.config$,
savedObjects: core.savedObjects,
isProductionMode: this.isProductionMode,
serverInfo: this.serverInfo,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should also use this.serverInfo in registerInstallScriptRoutes a few lines above

Copy link
Member Author

Choose a reason for hiding this comment

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

I am working on this routes, for the enrollment instructions, if you are ok with that I will do the change in an other PR

x-pack/plugins/ingest_manager/server/plugin.ts
this.licensing$ = deps.licensing.license$;
if (deps.security) {
this.security = deps.security;
}
this.cloud = deps.cloud;
Copy link
Contributor

Choose a reason for hiding this comment

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

Security and cloud seem to be similar (both optional) but, iiuc, we treat them differently here.

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 the if for security is probably not necessary here

Copy link
Contributor

Choose a reason for hiding this comment

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

The if is because it's an optional dependency. We do the same guard with features, iirc.

Can be done in a follow up PR

x-pack/plugins/ingest_manager/server/plugin.ts

export interface IngestManagerSetupDeps {
licensing: LicensingPluginSetup;
security?: SecurityPluginSetup;
features?: FeaturesPluginSetup;
encryptedSavedObjects: EncryptedSavedObjectsPluginSetup;
cloud?: CloudSetup;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're adding this to our dependencies, I believe we'd need to add to kibana.json as an optional dependency

kibana/x-pack/plugins/ingest_manager/kibana.json

Line 8 in 5a99642

"optionalPlugins": ["security", "features"]

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@nchaulet nchaulet merged commit 5e972e1 into elastic:master May 4, 2020
nchaulet added a commit to nchaulet/kibana that referenced this pull request May 4, 2020
@nchaulet
[Fleet] Better display of fleet requirements (elastic#65027)
95b6e08 
# Conflicts:
#	x-pack/plugins/ingest_manager/public/applications/ingest_manager/index.tsx
@nchaulet nchaulet mentioned this pull request May 4, 2020
Merged
nchaulet added a commit that referenced this pull request May 4, 2020
@nchaulet
[7.x] [Fleet] Better display of fleet requirements (#65027) (#65124)
e3de86b
@nchaulet nchaulet mentioned this pull request May 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jfsiii jfsiii jfsiii approved these changes

Assignees

@nchaulet nchaulet

Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Fleet] Enforce production mode
4 participants
@nchaulet @elasticmachine @kibanamachine @jfsiii