elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.8k stars 8.19k forks source link

[Fleet] Add support for url, email, and timezone var types #116914

Open P1llus opened 2 years ago

P1llus commented 2 years ago

Support for 3 new var types were added in https://github.com/elastic/package-spec/pull/241 but are not yet supported in Kibana Fleet. We need to add support for this on both our API and the policy editor UI for validating these field types and offering an appropriate input widget.

Original Description

When an integration package is created, there is multiple field types we can use that is exposed to us, like text, yml, password, multi, bool etc, however we would like to request a few additions to this:

  1. We would like an url field type. Many of our integrations requires that http/https is used in the field, we do not have any good way of hardcoding this while still giving the user a choice. This field should either alert the user that they need to add http/https if its missing, or it should have a dropdown choice to the left of the user input, with a choice between http or https.

  2. An email field type, straight forward validates if the input is an email address or not.

  3. Timezone dropdown. This field is more optional, but a large amount of packages asks the user to input a timezone to override the local timezone, however there is multiple formats and ways to do this, and it would be much better for the end user if we could get a dropdown of all the available timezones in the format like this, I am sure we already have library functionality for this right? America/Chicago

This would greatly reduce the possibility of errors, typos or wrong formats used for timezones.

An example of how packages define fields so that there is no uncertainties on which fields we are talking about:

streams:
  - input: logfile
    template_path: logfile.yml.hbs
    title: DHCP Logs
    description: Collects Microsoft DHCP logs.
    vars:
      - name: tz_offset
        type: text
        title: Timezone Offset
        multi: false
        required: true
        show_user: true
        default: local
        description: >-
          By default, datetimes in the logs will be interpreted as relative to the timezone configured in the host where the agent is running. If ingesting logs from a host on a different timezone, use this field to set the timezone offset so that datetimes are correctly parsed. Acceptable timezone formats are: a canonical ID (e.g. "Europe/Amsterdam"), abbreviated (e.g. "EST") or an HH:mm differential (e.g. "-05:00") from UCT.
      - name: paths
        type: text
        title: Paths
        multi: true
        show_user: true
        default:
          - 'C:\Windows\System32\DHCP\DhcpSrvLog-*.log'
      - name: preserve_original_event
        required: true
        show_user: true
        title: Preserve original event
        description: Preserves a raw copy of the original event, added to the field `event.original`.
        type: bool
        multi: false
        default: false
      - name: tags
        type: text
        title: Tags
        multi: true
        required: true
        show_user: false
        default:
          - forwarded
          - microsoft_dhcp
      - name: processors
        type: yaml
        title: Processors
        multi: false
        required: false
        show_user: false
        description: >
          Processors are used to reduce the number of fields in the exported event or to enhance the event with metadata.  This executes in the agent before the logs are parsed. See [Processors](https://www.elastic.co/guide/en/beats/filebeat/current/filtering-and-enhancing-data.html) for details.

These are usually found here: image

jamiehynds commented 2 years ago

Having personally made the mistake of excluding the protocol for a URL (on more than one occasion!) during integration configuration, this type of validation is invaluable. Simple validation can act as a first line of defence, and avoid users having to dig into Beats/Agent debug logs, in cases where the error is caused by a trivial input error. +1 for me!

elasticmachine commented 2 years ago

Pinging @elastic/fleet (Team:Fleet)

jsoriano commented 2 years ago

@P1llus I have started a draft to see how this could look in the spec: https://github.com/elastic/package-spec/pull/241

Regarding the url type, as package developer, would you expect to have a setting to specify the schemes allowed?

If this is expected, I wonder if for this it would be better to have just a type, or explicit validators.

With only the type it could be something like this:

          - name: hosts
            type: url
            url_allowed_schemes: ['http', 'https']
            title: Hosts
            multi: true
            required: true
            show_user: true
            default:
            - http://127.0.0.1

Explicit validators would be more complex, but could be open to more things:

          - name: hosts
            type: text
            validators:
            - type: url
              allowed_schemes: ['http', 'https']
            title: Hosts
            multi: true
            required: true
            show_user: true
            default:
            - http://127.0.0.1

If we are not going to have many options for the types I think I would prefer the more simple approach with types.

cc @mtojek

P1llus commented 2 years ago

@jsoriano for our specific usecase it would only be http/https always, so we would not need to specify any allowed types or custom validators, though it might be something worth considering in the future. One thought we might want to have another validator for things like S3 inputs, bucket arns etc? But from our standpoint it would be okay with the first example you have.

Another thought would be if we want to only notify the user about this, or actually enforce it? Do we want to have a parameter to turn that on or off? I can't think of a usecase that we would need to bypass this though.

P1llus commented 2 years ago

To add to this, we could always add a custom field type as well at some point, which takes a raw regex value? But it would be good to have these predefined formats as well

jsoriano commented 2 years ago

@jsoriano for our specific usecase it would only be http/https always, so we would not need to specify any allowed types or custom validators, though it might be something worth considering in the future.

In principle an url can have any scheme (or none), it doesn't need to be http/https (it could be redis:// for example in the redis integration). So if for your use case you want to be able to check that the url has one of these schemes, this will need to be done with an additional setting.

One thought we might want to have another validator for things like S3 inputs, bucket arns etc? But from our standpoint it would be okay with the first example you have.

Additional validators could be used for other things, for example to check that a string has a specific format, but not sure if we really need this and this would add more complexity.

P1llus commented 2 years ago

@jsoriano for our specific usecase it would only be http/https always, so we would not need to specify any allowed types or custom validators, though it might be something worth considering in the future.

In principle an url can have any scheme (or none), it doesn't need to be http/https (it could be redis:// for example in the redis integration). So if for your use case you want to be able to check that the url has one of these schemes, this will need to be done with an additional setting.

That makes sense yeah. We could also just have a dropdown of values to the left of the text field if we want to? And able to add a list of the different choices?

For us, the integrations using httpjson does not work without a scheme, which is why it's important for us to have the constraints, as it's common to forget to add it.

jsoriano commented 2 years ago

We could also just have a dropdown of values to the left of the text field if we want to? And able to add a list of the different choices?

I guess that this will depend in the UX implementation :slightly_smiling_face: By now I am adding the url_allowed_schemes setting in https://github.com/elastic/package-spec/pull/241 to support this needing.

jsoriano commented 2 years ago

To add to this, we could always add a custom field type as well at some point, which takes a raw regex value? But it would be good to have these predefined formats as well

@P1llus regular expressions have appeared also in https://github.com/elastic/package-spec/pull/241#discussion_r741453291, but there are some open questions, see https://github.com/elastic/package-spec/pull/245.

jsoriano commented 2 years ago

Change merged in the spec: https://github.com/elastic/package-spec/pull/241

jlind23 commented 1 year ago

@kpollich @juliaElastic is this issue still relevant?

juliaElastic commented 1 year ago

Yes, I think these types are not implemented yet in Fleet UI.

P1llus commented 1 year ago

This would still be a useful feature indeed!