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] Refactor existing Agent 'Policies - Integration - Edit' view to support using a UI extension point if one is available #82482

Closed
paul-tavares opened this issue Nov 3, 2020 · 9 comments · Fixed by #84534
Assignees
Labels
Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Fleet Team label for Observability Data Collection Fleet team v7.11.0

Comments

@paul-tavares
Copy link
Contributor

paul-tavares commented Nov 3, 2020

Part of: Integration Specific Policy Views
Dependent on: #82478

Description

Refactor the Edit Integration Policy View to check for if a registered UI extension point exists for the given Integration and if so, render that custom UI extension on the Edit form

In addition,
Remove the Endpoint specific exception from the Create Integration form:

name:
// For Endpoint packages, the user must fill in the name, thus we don't attempt to generate
// a default one here.
// FIXME: Improve package policies name uniqueness - https://github.com/elastic/kibana/issues/72948
packageInfo.name !== 'endpoint'
? `${packageInfo.name}-${
pkgPoliciesWithMatchingNames.length
? pkgPoliciesWithMatchingNames[pkgPoliciesWithMatchingNames.length - 1] + 1
: 1
}`
: '',

And instead have the Endpoint UI Extension that is registered for the Create flow blank out the default Integration Name generated by Fleet.

Some Questions to address

  • Should the custom UI extension replace the out-of-the-box view?
    • or supplement it by rendering below the existing form?
    • Should perhaps the decision to either render only the custom UI extension -vs- supplement the out-of-the-box form be an option that the plugin registering the extension can control?
@paul-tavares paul-tavares added Team:Fleet Team label for Observability Data Collection Fleet team Team:Defend Workflows “EDR Workflows” sub-team of Security Solution labels Nov 3, 2020
@elasticmachine
Copy link
Contributor

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

@paul-tavares
Copy link
Contributor Author

@elastic/ingest-management I would like some feedback/thoughts on the questions I posed above in this Issue.
Personally, I like the flexibility of deferring the decision to the provider of the UI Extension by introducing options to the UI registration (would apply to Integration Policy Edit and Create) - the default would be to "supplement" the current out-of-the-box form.

@nchaulet
Copy link
Member

I like the idea of having that decision configurable with an option too

@jen-huang
Copy link
Contributor

I think the decision for this is best informed by what we anticipate custom integration form designs to look like. Based on my current awareness of current design discussions, it seems like the majority of future custom integration form views would replace the entire form, rather than adding supplemental views below the form.

This makes sense from a UX perspective too: it would be confusing to have two UI areas which affect the same user-configured field - this is not entirely relevant for endpoint package, which ships no inputs, but we can anticipate that most packages do ship inputs.

My vote is to build it now by rendering only the custom UI extension, with no options, to avoid over-engineering for an unknown future. We can always add options later when the appropriate use case comes up.

@kevinlog
Copy link
Contributor

kevinlog commented Nov 12, 2020

My vote is to build it now by rendering only the custom UI extension, with no options, to avoid over-engineering for an unknown future. We can always add options later when the appropriate use case comes up.

I'm fine with this as long as we still have the "Integration Settings" available at the top of the form, i.e. the Integration Name, Description and namespace. If these are common for all Integrations, I think it should remain until we see a reason to remove it. See the screenshot below.

image

FYI @paul-tavares @jen-huang @nchaulet

@jen-huang
Copy link
Contributor

@paul-tavares 👍 Agreed, the form should only replace what is below Integration Settings.

@paul-tavares
Copy link
Contributor Author

Thanks @jen-huang and @kevinlog .
I'm good with the UI Extension replacing only the Configuration form inputs (those built from the package Policy template inputs) below the Integration Policy defintion (name, description, namespace ++ advanced options dropdown).

Initially I was thinking that option to "replace" would actually also replace that set of integration options as well - so that the Integration specific view can also control those fields. Doing so would void us having integration specific code in Fleet - as is the case for when creating an Endpoint Policy, where we don't want the name to be auto-generated into the input -

name:
// For Endpoint packages, the user must fill in the name, thus we don't attempt to generate
// a default one here.
// FIXME: Improve package policies name uniqueness - https://github.com/elastic/kibana/issues/72948
packageInfo.name !== 'endpoint'
? `${packageInfo.name}-${
pkgPoliciesWithMatchingNames.length
? pkgPoliciesWithMatchingNames[pkgPoliciesWithMatchingNames.length - 1] + 1
: 1
}`
: '',

I'm good with going with the current approach and just replacing the settings and we can iterate on it if needed.

@jen-huang
Copy link
Contributor

@paul-tavares WRT to the package policy name for Endpoint - I'd love to see that bit of code removed too. I wonder if it might be possible to move that hack to the custom endpoint form instead by triggering a change of the name on initial component mount (useEffect) 🤔

@paul-tavares
Copy link
Contributor Author

@jen-huang that seems like its something we can do - to use useEffect only during component initial rendering (on the Create form) to blank out the name. I will add that to the description of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Fleet Team label for Observability Data Collection Fleet team v7.11.0
Projects
None yet
6 participants