Optional ssl for fleet logstash output

[MichelLosier]

Summary

Resolves: #145266

• Allows SSL configuration to be disabled for Fleet agent logstash output
  • Adds an SSL toggle in the logstash output form.
  • On is the default state of the form
  • When off:
    • Authentication form section is removed
    • Logstash input config has SSL related fields removed
    • Submitting update removes SSL fields and related SSL secrets in output config
    • Shows a call out to proceed with caution
      • Borrows language from this doc
• Adds testing in Fleet API
  • Ensure SSL fields are removed if SSL field is null
  • Remove secrets if secrets are omitted in an update
• Fix: Fleet API output update usage (HTTP PUT) of saved object client is aligned as full object update instead of partial
  • Sets the mergeAttributes: false option on the saved object client

Reviewer asks

• QA
  • Will need validation of SSL toggling for Fleet managed agents using logstash output
• @kilfoyle: Just need a review of the new warning callout message for the SSL switch

Current / Default State

SSL disabled state: form

SSL disabled state: Logstash config input with SSL fields removed

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
• Flaky Test Runner was used on any tests changed
• The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

• See some risk examples
• Flipping an existing agent output for logstash to no SSL could put agent in an unexpected state

[elasticmachine]

Pinging @elastic/fleet (Team:Fleet)

[MichelLosier]

Our fleet API route for updating outputs uses a PUT HTTP verb, but our update of the saved object was performing partial updates and merging existing fields with fields included in the update. This had the effect of persisting the secrets field when it was omitted in the update. This change makes sure this is a full replace of the object.

[nchaulet]

Fleet APIs are not following strictly REST guidelines and all of our PUT APIs are in reality PATCH, like most of the Kibana APIs

[MichelLosier]

Gotcha! I'll refactor towards the PATCH behavior instead. Looks like anyways this change broke a bunch of tests which I missed.

[MichelLosier]

The idea here is that if we don't have any pre-existing output, we assume its a new configuration and set the default state to true. Otherwise if we are editing an existing output, we set the enable state based on the presence of existing SSL settings.

This is fine for editing an existing Logstash config, but if you are editing an existing output of a different type like Kafka where SSL values may not have been setup, and changing it to Logstash, it can default to false which is not ideal. We have a failing cypress test around this.

[MichelLosier]

Addressed in 30be164

[nchaulet]

@MichelLosier I have been trying to save a logstash output without ssl configuration and it seems it's stuck without any error message, looking at the original issue it seems to me we should be able to save the logstash even with an incomplete ssl configuration, maybe I missed something

[MichelLosier]

Thanks for catching that, seeing that too. It seems this happens now if its a brand new output config, or if its an existing output already with SSL disabled and you are trying to save again with SSL still disabled. Looking into it.

[MichelLosier]

58161af should address that issue, following a similar pattern with the Kafka output form. Seems what was missing here was not running validation for SSL fields if the SSL switch was off, otherwise as you observed, it would quietly fail validation.

[nchaulet]

@MichelLosier looks better just tested on create and it seems to work, but on update it seems I still have issues when providing only a server certificate (that should be a scenario we accept)

[MichelLosier]

From my understanding that looks to be expected, if the new SSL switch is turned off this allows you to submit a configuration that doesn't use the SSL fields and omits that part of the form altogether. Otherwise, if the SSL switch is turned on, the SSL certificate and key values are required which is the current state onmain.

Worth noting: I see on main if you omit those SSL fields the validation fails silently on a new output config submission, but validation warnings are visible when editing an existing output and clearing those fields and trying to submit.

It sounds like though from here when SSL is in use we do want to make sure a key and cert is defined and used at both the agent and ES pipeline end because its also serving as an auth mechanism. Our template config for elastic_agent input for ES sets ssl_client_authentication to required which makes the client providing a key and cert required here.

[nchaulet]

It seems the issue we want to solve is to allow user to use a logstash output without ssl client verification, in that scenario they may want to provide a server certificate authority but no client certificate.

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 58161af

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	1.7MB	1.7MB	+1.1KB

History

• 💔 Build #288812 failed b8a1b11
• 💔 Build #288783 failed aa0881b
• 💔 Build #288736 failed 7596fe3

[MichelLosier]

Right! From the original ticket, as I understood it there seemed to be two asks:

1. Ability to not use SSL in this setup, there seemed to be a long history around this use case as well
2. Ability to not need ssl client verification

For 2, I split into its own ticket: #216243, since there was concern about the complexity introduced to the form. I think my initial proposal of an approach we can take for that case made some incorrect requirements, and I have just revised it and it seems a bit more simpler.

So for this PR I was just scoping to the story of using / not using SSL.

For supporting turning off client verification, I agree that having the client certificate and key fields as optional would support that, but I think we would need to have the elastic agent pipeline config field ssl_client_authentication updated to reflect that configuration. What are your thoughts on what is proposed here: #216243

[nchaulet]

Thanks got it, I was missing that second issue context :) I am wondering if the second ticket could be simplified by providing all the option in the pipeline with comments, and avoiding a dropdown, but not related to that one

[nchaulet]

LGTM 🚀

[MichelLosier]

I am wondering if the second ticket could be simplified by providing all the option in the pipeline with comments, and avoiding a dropdown, but not related to that one

Yeah, I like this option! I'll add it as a consideration for #216243