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] Support input level variable for granular integrations #112272

Closed
nchaulet opened this issue Sep 15, 2021 · 16 comments
Closed

[Fleet] Support input level variable for granular integrations #112272

nchaulet opened this issue Sep 15, 2021 · 16 comments
Labels
enhancement New value added to drive a business result Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@nchaulet
Copy link
Member

nchaulet commented Sep 15, 2021

Description

For package that contains multiple integrations (AWS, Azure, ...) we do not support input level variables in the policy editor, (we got type errors if the package contains input level variables)
Looks like it's expected:

// If non-integration package, collect input-level vars, otherwise skip them,
// we do not support input-level vars for packages with integrations yet)

We should probably support that.

Related to elastic/integrations#1570

@nchaulet nchaulet added the Team:Fleet Team label for Observability Data Collection Fleet team label Sep 15, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@joshdover
Copy link
Contributor

@jen-huang I'm guessing we need some UX work here to define how to display these variables? @endorama is blocked on adding metrics to the GCP integration (which currently only supports logs) on this problem.

@endorama I think we also need to define the upgrade issue you're facing in moving from input variables to policy_template ones. Could you create a simple example of the structure of the variables before/after the change you're trying to make? This would help us determine how we can solve this problem by ensuring we're aligned on the exact upgrade path.

@endorama
Copy link
Member

Variables in policy_template (excerpt):

policy_templates:
  - name: gcp
    title: Google Cloud Platform (GCP) logs
    description: Collect logs from Google Cloud Platform (GCP) instances
    inputs:
      - type: gcp-pubsub
        vars:
          - name: alternative_host
            type: text
            title: Alternative host
            multi: false
            required: false
            show_user: false
          - name: project_id
            type: text
            title: Project Id
            description: Your Google Cloud project ID where the resources exist.
            multi: false
            required: true
            show_user: true
            default: SET_PROJECT_NAME
          - name: credentials_file
            type: text
            title: Credentials File
            description: The path to the JSON file with the private key. Make sure that the Elastic Agent has at least read-only privileges to this file.
            multi: false
            required: false
            show_user: true
          - name: credentials_json
            type: text
            title: Credentials JSON
            description: The content of the JSON file you downloaded from Google Cloud Platform.
            multi: false
            required: false
            show_user: true
        title: "Collect Google Cloud Platform (GCP) audit, firewall and vpcflow logs (input: gcp-pubsub)"
        description: "Collecting audit, firewall and vpcflow logs from Google Cloud Platform (GCP) instances (input: gcp-pubsub)"

Variables moved from input to package root (excerpt):

vars:
  - name: project_id
    type: text
    title: Project Id
    multi: false
    required: true
    show_user: true
    default: SET_PROJECT_NAME
  - name: credentials_file
    type: text
    title: Credentials File
    multi: false
    required: false
    show_user: true
  - name: credentials_json
    type: text
    title: Credentials Json
    multi: false
    required: false
    show_user: true
policy_templates:
  - name: audit
    title: Google Cloud Platform (GCP) Audit logs
    description: Collect audit logs from Google Cloud Platform (GCP) with Elastic Agent
    categories:
      - security
    data_streams:
      - audit
    inputs:
      - type: gcp-pubsub
        title: "Collect Google Cloud Platform (GCP) audit logs (input: gcp-pubsub)"
        description: "Collecting audit logs from Google Cloud Platform (GCP) instances (input: gcp-pubsub)"
        input_group: logs
    screenshots:
      - src: /img/filebeat-gcp-audit.png
        title: filebeat gcp audit
        size: 1702x996
        type: image/png

@jen-huang
Copy link
Contributor

jen-huang commented Apr 19, 2022

@endorama For other packages that have moved to exporting multiple integrations, it didn't seem necessary to support input-level vars as package-level and data stream-level vars seemed to suffice:

AWS Cloudwatch integration examples:

Cloudwatch policy_template, associated with multiple data streams, multiple inputs, no input-level vars:
https://github.com/elastic/integrations/blob/7547305e7db53231ef7c1dc19e706c7061c61352/packages/aws/manifest.yml#L128

cloudwatch_logs and cloudwatch_metrics data streams, with stream-level vars per input type
https://github.com/elastic/integrations/blob/7547305e7db53231ef7c1dc19e706c7061c61352/packages/aws/data_stream/cloudwatch_logs/manifest.yml
https://github.com/elastic/integrations/blob/7547305e7db53231ef7c1dc19e706c7061c61352/packages/aws/data_stream/cloudwatch_metrics/manifest.yml

For the GCP package changes, is the gcp-pubsub input associated with multiple data streams? And each data stream shares some common vars, hence the need for input-level vars? If not, I wonder if stream-level vars would suffice?

@endorama
Copy link
Member

@jen-huang

For other packages that have moved to exporting multiple integrations, it didn't seem necessary to support input-level vars as package-level and data stream-level vars seemed to suffice:

This is correct!

For the GCP package changes, is the gcp-pubsub input associated with multiple data streams? And each data stream shares some common vars, hence the need for input-level vars?

It is, but this is not the reason we need this.

If not, I wonder if stream-level vars would suffice?

Unfortunately no. The reason GCP migration is blocked is elastic/integrations#2987
As you say other packages have used a mix of package level and data streams level variables. GCP package is different because it uses input level variables only. Adding metrics data streams require moving variables to package level and data stream level, but this produce a breaking change in the configuration for which the upgrade path is not working as expected.
We have possible workarounds but we wanted to have an estimation of the effort required to implement input-templates before reaching a decision.

@jen-huang
Copy link
Contributor

Adding metrics data streams require moving variables to package level and data stream level, but this produce a breaking change in the configuration for which the upgrade path is not working as expected.

Reading elastic/integrations#2987, the upgrade path refers to upgrading the GCP policies once the package is upgraded. There is no machine-readable "migration" schema available in packages between versions, which means that Fleet's mechanism for upgrading policies is naive. Apart from minor package updates, it is very easy to run into a conflict with upgrading policies. Certainly with this kind of big migration to exporting multiple integrations: all the previous packages that did this migration resulting in policy upgrade conflicts.

Even if we introduce support for input-level variables and templates, it does not guarantee that GCP policy upgrade will be conflict-free as it is possible that some of Fleet's data models will need to be changed.

So the case for supporting input-level vars in order to produce conflict-free upgrades is not a strong one from Fleet's perspective. A stronger argument would be that these vars should live on the input level from a UX perspective, but are instead having to be moved to package/data stream-level as workaround.

If we were to add this support, it would probably take 2 weeks or so, but it is currently not on our foreseeable roadmap due to other product priorities that are higher.

@endorama
Copy link
Member

@jen-huang thanks a lot for the answer! 🎉

@joshdover
Copy link
Contributor

joshdover commented Apr 26, 2022

I have a few clarifications that would help me understand what's going on here, as this is an area of Fleet & Integrations I'm still unfamiliar with:

Am I right in understanding this as a summary of where we are?

  • In the ideal end state we'd like to get to, the existing package-level and stream-level variable support in Fleet UI fit the GCP use case well.
  • If we move the variables that are currently defined at the policy template level to the package-level, it will be a "breaking change" that would require the user to re-enter these configuration values. To be clear, data collection will not break, it's just not an ideal user experience.
  • We could move ahead using the existing support for package and stream-level variables and accept that users will have to re-enter these values.
  • In an ideal world, a package could define machine-readable migrations when variables move to new levels. However there could also be other, lower effort options to improve the user experience:
    • We could show the user a list of variables that were used in the previous version that are no longer used to allow them to easily copy
    • We could suggest copying variables from the previous version if they are now unused, but have the same name
  • There's no reason that we necessarily need to solve this issue (supporting input-level variables for packages that contain multiple policy templates / integrations).

@ravikesarwani
Copy link
Contributor

@endorama did you get a chance to take a look at the above comments?

From product perspective for GCP I would say the experience would be fine if:

  1. We don’t break the existing data collection.
  2. We tell them that they need to re-enter some of the credential variable again when they they upgrade to the new package.

The overall usage of GCP is low and hence doing the right thing and keeping a single package for both logs and metrics would be desirable.

@endorama
Copy link
Member

@joshdover

In elastic/integrations#2987 it's mentioned that Fleet UI support Policy Template level variables is yet to be implemented. When we say "policy template level variables" is that the same as "input level variables" (the topic of this issue)?

Yes! Actually that wording is imprecise, sorry for that, as the issue is with "input level variables" within policy templates. (I admit inputs in this context got me confused).

It appears Fleet UI supports these today, but it only works for packages that contain a single policy template?

Yes.

In the ideal end state we'd like to get to, the existing package-level and stream-level variable support in Fleet UI fit the GCP use case well.

correct

If we move the variables that are currently defined at the policy template level to the package-level, it will be a "breaking change" that would require the user to re-enter these configuration values. To be clear, data collection will not break, it's just not an ideal user experience.

Yes, the main issue here is that trying out this some variable just disappear and are no longer fillable (they don't show up in the UI anymore, so there is no way to insert their value).

In an ideal world, a package could define machine-readable migrations when variables move to new levels. However there could also be other, lower effort options to improve the user experience:

I'm not sure that's ideal due to the complexity involved and due to general human intervention in updates with breaking changes (if just to approve them).
Given a migration path that works reliably and is easy enough I don't think we need to overthink this and is ok for users to do them manually. Your proposed improvements would already simplify the migration to an acceptable level IMO so 👍

There's no reason that we necessarily need to solve this issue (supporting input-level variables for packages that contain multiple policy templates / integrations).

One reason is that the Package Specification support this use case and this incongruence degrades the developer experience, but to the point discussed we can move forward without this issue being solved if the upgrade path works correctly.

@jen-huang
Copy link
Contributor

We already have a "conflict resolution" UX. This kicks in when a user upgrades to a package version with breaking changes. In this case, the associated policies cannot be upgraded automatically, the user needs to click Upgrade for those policies and are presented with a policy editor that allows them to view the previous configuration to copy values from. The below screenshots show the experience upgrading from a pre-integrations AWS policy to post-integrations:

image

image

@endorama
Copy link
Member

This experience was not working for my use case, but I don't want to steer the conversation there in this issue, so I will open a separate one with details.

@ravikesarwani
Copy link
Contributor

@jen-huang Is there someone in your team that you would recommend Edo work with closely so that we can get to the bottom of these issues and if applicable create appropriate issue to get that fixed? This "conflict resolution" flow I guess is critical and potentially may be needed for other packages as we continue to make enhancements and architectural changes around all things Agent & Fleet.

@endorama Can you please link the issue here when opened so that we have some level of continuation on the discussion and folks can follow up as needed.

@jen-huang
Copy link
Contributor

@endorama @ravikesarwani - @kpollich worked on the conflict resolution experience previously, so he will follow up to help investigate the gaps that we're missing.

@mukeshelastic
Copy link

As Ravi mentioned above, if the upgrade conflict resolution doesn't work as expected then then this is a generic problem applicable to all packages and not just GCP. So thanks @kpollich @endorama so much for collaborating to get to the bottom of the upgrade problems.

@jen-huang
Copy link
Contributor

Given that #131251 was resolved (thanks Kyle!) and unblocks the next step for GCP package (elastic/integrations#490 (comment)), I'm going to close this issue.

@jen-huang jen-huang closed this as not planned Won't fix, can't repro, duplicate, stale May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

No branches or pull requests

8 participants