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] Make integration names globally unique #115212

Merged
merged 5 commits into from
Nov 3, 2021

Conversation

criamico
Copy link
Contributor

@criamico criamico commented Oct 15, 2021

Summary

Implements #72948

Closes #116475

Currently, when installing an integration package, the name has to be unique per agent policy. This allows to have several integrations across policies having the same name, which can be confusing for the user.

This PR changes the logic in PackagePolicyService to ensure that the name is globally unique and updates the component step_define_package_policy to change the naming displayed in the form to be incremented based on all the package policies.

Repro steps

Test case 1 (Adding an integration with an already used name)

  • Add an integration (for instance apache)
  • Go to the integration's policies tab ( app/integrations/detail/apache-1.1.1/policies), and click on Add Apache
  • When the editor opens up put the same name as before.
  • You should see a popup "There is already a package with the same name"
  • Repeat this case both with the same agent policy and with a different one

Test case 2 (Adding an integration with a unique name)

  • Click on Add Apache
  • In the editor choose a different name
  • The integration should install regularly

Test case 3 (Pre-filled name increments globally)

  • Click on Add Apache
  • Regardless of which agent policy you choose, check that the pre-filled name in the form is the highest number already present, incremented by one
  • For instance, if you had Apache-2 it should now be Apache-3:
    Screenshot 2021-10-20 at 16 26 37
  • If you just install it should work regularly

Test case 4 (Upgrading a policy)

  • Upgrade a policy (with or without conflicts)
  • The name shouldn't change

Test case 5 (Duplicating an agent policy)

  • Duplicate an agent policy, then open it
  • All the installed integration should now have name with format Name (copy)

Screenshot 2021-10-29 at 16 22 57

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@criamico criamico self-assigned this Oct 15, 2021
@criamico criamico added v7.15.0 v8.0.0 Team:Fleet Team label for Observability Data Collection Fleet team auto-backport Deprecated - use backport:version if exact versions are needed release_note:enhancement labels Oct 15, 2021
@criamico criamico linked an issue Oct 15, 2021 that may be closed by this pull request
@criamico criamico marked this pull request as ready for review October 15, 2021 15:49
@criamico criamico requested a review from a team as a code owner October 15, 2021 15:49
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@criamico criamico requested a review from jen-huang October 15, 2021 16:07
@criamico criamico force-pushed the 72948_Make_integration_names_unique branch from 1847cd0 to 1dededf Compare October 15, 2021 16:08
@jen-huang jen-huang added v7.16.0 and removed v7.15.0 labels Oct 15, 2021
@criamico
Copy link
Contributor Author

@elasticmachine merge upstream

@jen-huang jen-huang removed their request for review October 18, 2021 21:24
@criamico criamico added the WIP Work in progress label Oct 19, 2021
@criamico criamico force-pushed the 72948_Make_integration_names_unique branch from 74a21be to 8eab9fc Compare October 19, 2021 13:52
@criamico criamico removed the v7.16.0 label Oct 19, 2021
@criamico criamico force-pushed the 72948_Make_integration_names_unique branch from 49b60af to c10685f Compare October 20, 2021 14:21
@criamico criamico removed the WIP Work in progress label Oct 20, 2021
@nchaulet
Copy link
Member

I think we could have an issue with that feature and preconfigured policy, we generate a package policy name for preconfigured policy and not sure how it will work with that feature

@criamico
Copy link
Contributor Author

@nchaulet thanks for pointing it out. How can I test the preconfigured policy?

@nchaulet
Copy link
Member

@criamico You can add some policies to your kibana config file, like this for example

xpack.fleet.agentPolicies:
  - name: Test agent policy 2
    id: test-policy-2
    monitoring_enabled: []
    package_policies:
      - package:
          name: fleet_server
        name: Fleet Server
        inputs:
          - type: fleet-server
            keep_enabled: true
            ```

@kibanamachine
Copy link
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

History

  • 💔 Build #163288 failed c10685f000dbc9b1b8f82a648538b5a30e1f80c2
  • 💔 Build #162790 failed 09d42c9619cfaacffc33c81473c719ea0c6c8c86
  • 💔 Build #162703 failed 8eab9fceface3948be34dd2839537eeb5ff1b1f1
  • 💔 Build #161911 failed 74a21be59141af6ef91e4c1c9c50557e35b2127f
  • 💔 Build #161538 failed 1dededf14e28ae8814ae699a06de26b97e323c73

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

cc @criamico

@criamico criamico force-pushed the 72948_Make_integration_names_unique branch from 766e9e8 to 50907dc Compare October 21, 2021 08:05
@criamico criamico force-pushed the 72948_Make_integration_names_unique branch from c02f5b7 to 565251b Compare November 3, 2021 13:58
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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 615.7KB 615.9KB +185.0B

History

  • 💔 Build #3929 failed c02f5b7268f70278ace71625d3cc6ae43a5db455
  • 💔 Build #3639 failed 6fd058c9a1c209cb5ba82fdf022b7126f845b28f
  • 💔 Build #3574 failed 368381bd33854eb44f71888cd9b4c2124b788e39
  • 💔 Build #2898 failed 03b95b65ed4d35a95b8d171d544be28c666af95f
  • 💔 Build #2833 failed 2f158d3de23fd90ff24d892a269f3d66cae9f89c

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

cc @criamico

@criamico criamico merged commit 7432b9b into elastic:main Nov 3, 2021
@criamico criamico deleted the 72948_Make_integration_names_unique branch November 3, 2021 16:41
@kibanamachine
Copy link
Contributor

The following labels were identified as gaps in your version labels and will be added automatically:

  • v8.1.0

If any of these should not be on your pull request, please manually remove them.

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 3, 2021
* [Fleet] Make integration names globally unique

* Fix Jest tests

* Append (copy) to names of integration packages belonging to duplicated policy

* Update current policy maintaining its name

* Fix failing tests
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 3, 2021
* [Fleet] Make integration names globally unique

* Fix Jest tests

* Append (copy) to names of integration packages belonging to duplicated policy

* Update current policy maintaining its name

* Fix failing tests
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0
7.16

The backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Nov 4, 2021
* [Fleet] Make integration names globally unique

* Fix Jest tests

* Append (copy) to names of integration packages belonging to duplicated policy

* Update current policy maintaining its name

* Fix failing tests

Co-authored-by: Cristina Amico <[email protected]>
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 4, 2021
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

2 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

1 similar comment
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 15, 2021
kibanamachine added a commit that referenced this pull request Nov 15, 2021
* [Fleet] Make integration names globally unique

* Fix Jest tests

* Append (copy) to names of integration packages belonging to duplicated policy

* Update current policy maintaining its name

* Fix failing tests

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

Hi @criamico
We have revalidated this PR on 7.16.0 BC-5 Kibana Cloud environment and found it working fine.

Test case 1 (Adding an integration with an already used name)

We received below error as expected on adding same name integration:
5

Test case 2 (Adding an integration with a unique name)

We are able to add integration with a unique name.

Test case 3 (Pre-filled name increments globally)

Integration names are incremented globally.
Like:
First Policy- System Integration- system-1
Second Policy- System Integration- system-2
--- and so on Validated till 12.

This is Working fine.

Test case 4 (Upgrading a policy)

No issues are observed on upgrading integration.

Test case 5 (Duplicating an agent policy)

Integration name of duplicated policy shows up like system-1 (copy)
4

Build details

Build: 46061
Commit: f13296db8798dd0cd39ab6cc4a61a35a2a2b05cc

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:fix Team:Fleet Team label for Observability Data Collection Fleet team v7.16.0 v8.0.0 v8.1.0
Projects
None yet
8 participants