[Fleet] Fixes to preconfigure API #96094

Zacqary · 2021-04-01T19:58:48Z

Summary

Fixes passing the is_managed flag when creating a new policy
Fixes passing the is_default and is_default_fleet_server flags
Adds the force flag to the packages schema instead of passing it by default; this restores the ability for the package registry to throw an error when trying to install and outdated package, unless the user passes force: true intentionally
Reconciles the preconfiguration API's naming convention for versioned packages, splitting the package name with - instead of :

Testing examples

Send a PUT to /api/fleet/preconfiguration with the following payloads to test different scenarios:

`force` flag

{
   "packages": [
      {
         "name": "apache",
         "version": "0.3.4",
         "force": true
      }
   ]
}

Also remove force: true and ensure that this request fails.

`is_managed`

{
    "agentPolicies": [
        {
            "name": "Preconfigured Managed Policy",
            "id": 1,
            "namespace": "test",
            "is_managed": true,
            "package_policies": [
                {
                    "package": {
                        "name": "system"
                    },
                    "name": "System Integration"
                }
            ]
        }
    ]
}

After the policy is created, go to the policy in the Fleet UI and try to add an integration. Ensure that it throws an error related to being a managed policy.

`is_default`/`is_default_fleet_server`

Before running this, restart yarn es snapshot so that you're working with a fresh cluster. Then run this request BEFORE opening the Fleet app for the first time.

{
    "agentPolicies": [
        {
            "name": "Preconfigured Default Policy",
            "id": 1,
            "namespace": "test",
            "is_default": true,
            "package_policies": [
                {
                    "package": {
                        "name": "system"
                    },
                    "name": "System Integration"
                }
            ]
        },
        {
          "name": "Preconfigured Default Fleet Policy",
          "id": 1,
          "namespace": "test",
          "is_default_fleet_server": true,
          "package_policies": [
              {
                  "package": {
                      "name": "system"
                  },
                  "name": "System Integration"
              }
          ]
      }
    ]
}

After running this, open Fleet and ensure that these are the only two policies created, instead of Fleet creating new Default policies.

Confirm by running this test scenario with only one of the Default or Default Fleet Server policies. When Fleet initializes, it should create a default policy for whichever one you did not preconfigure.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2021-04-01T19:58:51Z

Pinging @elastic/fleet (Feature:Fleet)

elasticmachine · 2021-04-01T19:58:51Z

Pinging @elastic/fleet (Team:Fleet)

ruflin

Can you share a full json example with all the flags in the PR description? This will help to have it directly documented and makes testing easier.

ruflin · 2021-04-06T06:54:17Z

x-pack/plugins/fleet/server/services/preconfiguration.ts

@@ -45,7 +45,7 @@ export async function ensurePreconfiguredPackagesAndPolicies(
    // If there are multiple packages with duplicate versions, separate them with semicolons, e.g
    // package-a:1.0.0, package-a:2.0.0; package-b:1.0.0, package-b:2.0.0
    const duplicateList = duplicatePackages
-      .map(([, versions]) => versions.map((v) => `${v.name}:${v.version}`).join(', '))
+      .map(([, versions]) => versions.map((v) => `${v.name}-${v.version}`).join(', '))


I'm starting to wonder if what you have with the : is actually better. I know I used the - in some of my tests but : looks pretty intuitive. What is the reason you changed it?

- is used everywhere else in the codebase, such as this example in EPM's package registry: https://github.com/elastic/kibana/blob/master/x-pack/plugins/fleet/server/services/epm/registry/index.ts#L65

I like : better too but I think consistency is the most important.

I wonder if we mix two things here. The part you linked is the name of the .zip file I think. The reason we have - there is because it is pretty uncomment to have : in a file name. So it is apache-1.2.3.zip instead of apache:1.2.3.zip as file name. We also copied here what is used for the other stack downloads. I would think for configuration we could use the :. But as packages are not allowed to have any - in the name it is also something we could change later on and adjust all the places where it is used. So good to move forward with your change 👍

…e-fixes

jfsiii

Haven't run locally, but it reads well & has tests. Left two comments but they can be added anytime. Giving a 🚢 if you need it.

jfsiii · 2021-04-06T23:14:58Z

x-pack/plugins/fleet/server/services/preconfiguration.ts

@@ -45,7 +45,7 @@ export async function ensurePreconfiguredPackagesAndPolicies(
    // If there are multiple packages with duplicate versions, separate them with semicolons, e.g
    // package-a:1.0.0, package-a:2.0.0; package-b:1.0.0, package-b:2.0.0
    const duplicateList = duplicatePackages
-      .map(([, versions]) => versions.map((v) => `${v.name}:${v.version}`).join(', '))
+      .map(([, versions]) => versions.map((v) => `${v.name}-${v.version}`).join(', '))


Can use pkgToPkgKey

kibana/x-pack/plugins/fleet/server/services/epm/registry/index.ts

Line 74 in f1d6629

export const pkgToPkgKey = ({ name, version }: { name: string; version: string }) =>

to ensure the correct value and avoid the duplication

jfsiii · 2021-04-06T23:15:30Z

x-pack/plugins/fleet/server/services/preconfiguration.ts

@@ -123,7 +137,7 @@ export async function ensurePreconfiguredPackagesAndPolicies(
      id: p.policy.id,
      updated_at: p.policy.updated_at,
    })),
-    packages: preconfiguredPackages.map((pkg) => `${pkg.name}:${pkg.version}`),
+    packages: preconfiguredPackages.map((pkg) => `${pkg.name}-${pkg.version}`),


Same comment re: pkgToPkgKey

kibana/x-pack/plugins/fleet/server/services/epm/registry/index.ts

Line 74 in f1d6629

export const pkgToPkgKey = ({ name, version }: { name: string; version: string }) =>

ruflin

I tested a few scenarios and it does what it should.

One thing I stumbled over this that I set is_managed but then was still able to delete the policy. Not sure if this is related to this PR or something still being worked on. @jfsiii probably knows.

jfsiii · 2021-04-07T13:02:22Z

@ruflin managed policies should not be able to be deleted

kibana/x-pack/test/fleet_api_integration/apis/agent_policy/agent_policy.ts

Line 323 in 4d43a4f

describe('POST /api/fleet/agent_policies/delete', () => {

Can you add more detail about the steps you took so Zacqary or I can confirm/fix?

ruflin · 2021-04-07T13:39:12Z

@jfsiii Tried to take it out of my copy / paste history and did not run it yet again. But I think that is the command I used:

curl -X PUT http://elastic:changeme@localhost:5601/api/fleet/setup/preconfiguration -H "kbn-xsrf: foo" -H "Content-Type:application/json" -d'
{
    "agentPolicies": [
        {
            "name": "Fun is fun",
            "id": "foo-bar2",
            "namespace": "test",
            "is_managed": true,
            "package_policies": [
                {
                    "package": {
                        "name": "system"
                    },
                    "name": "System Integration"
                }
            ]
        }
    ]
}'

jfsiii · 2021-04-07T13:42:01Z

@ruflin thanks. That would create the policy. How did you delete it? Via the UI or another curl command?

ruflin · 2021-04-07T13:45:16Z

I used the UI.

Zacqary · 2021-04-07T16:02:49Z

@ruflin Even though you were able to delete the policy, did it still prevent you from adding a new integration from the UI? I just want to make sure the is_managed flag is actually being applied.

kibanamachine · 2021-04-07T18:00:44Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💚 Build #117606 succeeded 3f2d4f7
💚 Build #116926 succeeded 3a9bc9b

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

cc @Zacqary

Zacqary · 2021-04-07T18:07:32Z

@ruflin Just tested and I wasn't able to reproduce your issue. Deleting a managed preconfigured policy threw a Cannot delete managed policy <uuid> error.

ruflin · 2021-04-07T18:29:18Z

@Zacqary Probably an error on my end. I'll try again on master.

ruflin · 2021-04-07T18:55:21Z

I tested again and indeed I get the error. What confused me was that the Delete button was available. In contrast on the Default policy is greyed out. We should update the UI to also grey out the button.

Zacqary · 2021-04-07T19:36:07Z

@ruflin Tracking that here: #96492

jfsiii · 2021-04-07T20:43:33Z

@ruflin I am working on that right now #96492 (comment)

Zacqary added 4 commits April 1, 2021 13:25

Fix is_managed flag

5005f32

Fix preconfiguring default-flagged policies

b757371

Add force flag for installing older package versions

c1d9ca2

Fix package key convention

3a9bc9b

Zacqary added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Fleet Fleet team's agent central management project Team:Fleet Team label for Observability Data Collection Fleet team v7.13.0 labels Apr 1, 2021

Zacqary requested a review from a team as a code owner April 1, 2021 19:58

Zacqary self-assigned this Apr 1, 2021

ruflin reviewed Apr 6, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master' into 95788-preconfigur…

3f2d4f7

…e-fixes

jfsiii approved these changes Apr 6, 2021

View reviewed changes

ruflin approved these changes Apr 7, 2021

View reviewed changes

ruflin mentioned this pull request Apr 7, 2021

[Fleet] configure default policy for agent and fleet server from kibana config #96370

Closed

Use pkgToPkgKey in preconfiguration service

56ffd44

Zacqary merged commit c8e23ad into elastic:master Apr 7, 2021

Zacqary deleted the 95788-preconfigure-fixes branch April 7, 2021 18:07

Zacqary added a commit to Zacqary/kibana that referenced this pull request Apr 7, 2021

[Fleet] Fixes to preconfigure API (elastic#96094)

c5117ad

Zacqary mentioned this pull request Apr 7, 2021

[7.x] [Fleet] Fixes to preconfigure API (#96094) #96470

Merged

Zacqary added a commit that referenced this pull request Apr 7, 2021

[Fleet] Fixes to preconfigure API (#96094) (#96470)

764010b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] Fixes to preconfigure API #96094

[Fleet] Fixes to preconfigure API #96094

Zacqary commented Apr 1, 2021 •

edited

Loading

elasticmachine commented Apr 1, 2021

elasticmachine commented Apr 1, 2021

ruflin left a comment

ruflin Apr 6, 2021

Zacqary Apr 6, 2021

ruflin Apr 7, 2021

jfsiii left a comment

jfsiii Apr 6, 2021

jfsiii Apr 6, 2021

ruflin left a comment

jfsiii commented Apr 7, 2021

ruflin commented Apr 7, 2021

jfsiii commented Apr 7, 2021

ruflin commented Apr 7, 2021

Zacqary commented Apr 7, 2021

kibanamachine commented Apr 7, 2021

Zacqary commented Apr 7, 2021

ruflin commented Apr 7, 2021

ruflin commented Apr 7, 2021

Zacqary commented Apr 7, 2021

jfsiii commented Apr 7, 2021

[Fleet] Fixes to preconfigure API #96094

[Fleet] Fixes to preconfigure API #96094

Conversation

Zacqary commented Apr 1, 2021 • edited Loading

Summary

Testing examples

force flag

is_managed

is_default/is_default_fleet_server

Checklist

elasticmachine commented Apr 1, 2021

elasticmachine commented Apr 1, 2021

ruflin left a comment

Choose a reason for hiding this comment

ruflin Apr 6, 2021

Choose a reason for hiding this comment

Zacqary Apr 6, 2021

Choose a reason for hiding this comment

ruflin Apr 7, 2021

Choose a reason for hiding this comment

jfsiii left a comment

Choose a reason for hiding this comment

jfsiii Apr 6, 2021

Choose a reason for hiding this comment

jfsiii Apr 6, 2021

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

jfsiii commented Apr 7, 2021

ruflin commented Apr 7, 2021

jfsiii commented Apr 7, 2021

ruflin commented Apr 7, 2021

Zacqary commented Apr 7, 2021

kibanamachine commented Apr 7, 2021

💚 Build Succeeded

Metrics [docs]

History

Zacqary commented Apr 7, 2021

ruflin commented Apr 7, 2021

ruflin commented Apr 7, 2021

Zacqary commented Apr 7, 2021

jfsiii commented Apr 7, 2021

Zacqary commented Apr 1, 2021 •

edited

Loading

`force` flag

`is_managed`

`is_default`/`is_default_fleet_server`