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] Do not provide instruction to setup an http Fleet server #129371

Merged
merged 6 commits into from
Apr 7, 2022
Merged

[Fleet] Do not provide instruction to setup an http Fleet server #129371

merged 6 commits into from
Apr 7, 2022

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Apr 4, 2022

Summary

Resolve #122625

Change the fleet server on prem instruction to not provide instructions to setup an http fleet server and do not allow to add an http fleet server url in these instructions (it's still possible to do it via the settings tab or via the kibana preconfigure output)

Details:

  • change the validation to not allow http fleet server url (only in the instructions)
  • remove the --fleet-server-insecure-http flag

@nchaulet nchaulet added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team auto-backport Deprecated - use backport:version if exact versions are needed v8.2.0 v8.3.0 labels Apr 4, 2022
@nchaulet nchaulet requested a review from joshdover April 4, 2022 15:44
@nchaulet nchaulet self-assigned this Apr 4, 2022
@nchaulet nchaulet requested a review from a team as a code owner April 4, 2022 15:44
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

joshdover
joshdover reviewed Apr 4, 2022
View reviewed changes
Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

Can we change the UI to not require the user to enter the protocol at all? Instead I think we should have https:// be added to the "Fleet Server host" prepend prop or completely replace it.

We should also verify what the behavior is of showing the --insecure option on the regular Elastic Agent enrollment command given to the user. Do we need to display it somehow when a Fleet Server https host is configured, but it was known to be configured with Quick Start?

I'd also like @jen-huang to 👍 this before moving forward.

@nchaulet
Copy link
Member Author

nchaulet commented Apr 4, 2022 •
edited
Loading

Can we change the UI to not require the user to enter the protocol at all? Instead I think we should have https:// be added to the "Fleet Server host" prepend prop or completely replace it.

Actually we already use the prepend prop here for the input label, we could probably use it for the protocol but it will be a little more design change

Do we need to display it somehow when a Fleet Server https host is configured, but it was known to be configured with Quick Start?

We already display it in the Fleet server instruction (giving the user the flag insecure-http was probably a bug) we do not save that I do not think we will be able to show that in the add agent flyout
Screen Shot 2022-04-04 at 12 02 26 PM

@nchaulet
Copy link
Member Author

nchaulet commented Apr 4, 2022

Just tested and using --insecure against self generated fleet server ssl certificate works well

@nchaulet nchaulet requested a review from jen-huang April 4, 2022 16:57
@juliaElastic
Copy link
Contributor

We should revert this flag from docs as well: elastic/observability-docs#1441

@nchaulet nchaulet mentioned this pull request Apr 5, 2022
Merged
@nchaulet nchaulet requested a review from joshdover April 5, 2022 17:14
@nchaulet
Copy link
Member Author

nchaulet commented Apr 6, 2022

@elasticmachine merge upstream

@nchaulet
Copy link
Member Author

nchaulet commented Apr 6, 2022

@elasticmachine merge upstream

@nchaulet
Copy link
Member Author

nchaulet commented Apr 6, 2022

Looks like Cypress tests are failing because some of the packages removed here elastic/package-storage#3992 are returning random 404 from the snapshot registry

jen-huang
jen-huang approved these changes Apr 6, 2022
View reviewed changes
Copy link
Contributor

@jen-huang jen-huang left a comment

Choose a reason for hiding this comment

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

Small copy nit, otherwise LGTM

joshdover
joshdover approved these changes Apr 7, 2022
View reviewed changes
Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

LGTM. I think it would make sense to open an enhancement issue to allow us to track whether or not the Fleet Server is using a self-signed certificate so we could automatically tell the user to use the --insecure flag on Agent enrollment when necessary, along with some warnings in the UI about this not being a production-ready configuration.

@nchaulet
Copy link
Member Author

nchaulet commented Apr 7, 2022

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 684.6KB 684.5KB -25.0B

History

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

cc @nchaulet

@nchaulet nchaulet merged commit eff1e0c into elastic:main Apr 7, 2022
@nchaulet nchaulet deleted the fix-122625-fleet-server-https branch April 7, 2022 14:32
@kibanamachine kibanamachine mentioned this pull request Apr 7, 2022
Merged
kibanamachine pushed a commit that referenced this pull request Apr 7, 2022
@nchaulet
[Fleet] Do not provide instruction to setup an http Fleet server (#12…
4d6830b 
…9371)

(cherry picked from commit eff1e0c)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.2

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 7, 2022
@kibanamachine @nchaulet
[Fleet] Do not provide instruction to setup an http Fleet server (#12…
689af7e 
…9371) (#129756)

(cherry picked from commit eff1e0c)

Co-authored-by: Nicolas Chaulet <[email protected]>
@amolnater-qasource
Copy link

Hi @nchaulet
We have revalidated this PR in lieu of reported issue at #122625
We have revalidated this on 8.2 Snapshot self-managed environment and found it fixed now.

  • User is not able to add fleet server host with http from Add Agent flyout.
  • However as expected, user is able to edit from settings tab and able to update the host url to http.

Build details:
BUILD: 51835
COMMIT: 6fcd2d0

Screenshot:
9

Thanks

@amolnater-qasource amolnater-qasource mentioned this pull request Apr 11, 2022
Closed
@nchaulet nchaulet mentioned this pull request Apr 18, 2022
Merged
3 tasks
@linghengqian linghengqian mentioned this pull request Aug 30, 2024
Closed
@kilfoyle kilfoyle mentioned this pull request Aug 30, 2024
Merged
@mergify mergify bot mentioned this pull request Sep 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshdover joshdover joshdover approved these changes

@jen-huang jen-huang jen-huang approved these changes

Assignees

@nchaulet nchaulet

Labels
auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.2.0 v8.3.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Self-Managed][Fleet]: User is able to add both http and https in Fleet Server host field.
8 participants
@nchaulet @elasticmachine @juliaElastic @kibana-ci @kibanamachine @amolnater-qasource @joshdover @jen-huang