[Fleet] Restrict integration changes for managed policies #90675

jfsiii · 2021-02-08T20:30:23Z

Summary

closes #90445
refs #89617

Cannot add integrations to managed policy

Cannot delete integrations from managed policy

Checklist

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

…olicies

elasticmachine · 2021-02-09T20:50:22Z

Pinging @elastic/fleet (Feature:Fleet)

nchaulet

Should we verify in the delete method of the packagePolicy service too that the policy is not managed?

nchaulet · 2021-02-09T21:54:57Z

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

    }
+    if (parentAgentPolicy.is_managed) {


(not blocking) I have the feeling that is time to refactor L64-L80 to a new function something like _validateParentAgentPolicy

Yeah, I wanted to consolidate the two sources of the "There is already a package named..." errors

kibana/x-pack/plugins/fleet/server/services/package_policy.ts

Lines 297 to 304 in 45f420c

if (

(parentAgentPolicy.package_policies as PackagePolicy[]).find(

(siblingPackagePolicy) =>

siblingPackagePolicy.id !== id && siblingPackagePolicy.name === packagePolicy.name

)

) {

throw new Error('There is already a package with the same name on this agent policy');

}

And perhaps give it a specific error name/type. I plan on circling back to that after I get the other features in.

I'll open a ticket

nchaulet · 2021-02-09T21:57:51Z

x-pack/test/fleet_api_integration/apis/package_policy/create.ts

@@ -39,6 +39,52 @@ export default function ({ getService }: FtrProviderContext) {
        .send({ agentPolicyId });
    });

+    it('should fail for managed agent policies', async function () {
+      if (server.enabled) {


nitpick we have a helper that does the same thing but I found it less verbose https://github.com/elastic/kibana/blob/master/x-pack/test/fleet_api_integration/apis/epm/data_stream.ts/#L37

Thanks for the reminder. I was just copy/pasting from the rest of the file. I'll update

jfsiii · 2021-02-09T23:19:46Z

@elasticmachine merge upstream

…olicies

jfsiii · 2021-02-09T23:32:46Z

Should we verify in the delete method of the packagePolicy service too that the policy is not managed?

@nchaulet I put the check in the AgentPolicyService because Package Policy delete calls it

kibana/x-pack/plugins/fleet/server/services/package_policy.ts

Line 359 in a810ce6

await agentPolicyService.unassignPackagePolicies(

And in the Package Policy service we only have the Agent Policy id string, not the whole object. Which we do have in the AgentPolicyService.

simitt · 2021-02-10T07:51:54Z

Integrations cannot be added ~~, unless with a force flag~~
Integrations cannot be removed ~~, unless with a force flag~~

Does that mean there is no possibility to force adding/removing an integration? @ruflin has a more concrete plan on the policy set up than I do, but so far I was under the impression that a force flag could be used for the initial set up of the managed policy.

jfsiii · 2021-02-10T12:35:23Z

@simitt you cannot current bypass the guards for managed policies. However, you can work around them by doing something like

Create a new policy (which aren't managed by default) or update existing policy to unmanaged
Add / delete integrations
Set policy to managed

I'll open an issue to discuss the force flag (we have it some other places already and it doesn't mean quite the same thing) but I think the "make it unmanaged" workaround means everything is still possible without adding API which we might be inconsistent or changed later.

simitt · 2021-02-10T13:45:33Z

thanks @jfsiii - the described workflow should work for now.

…ithub.com:jfsiii/kibana into 90445-no-integration-changes-for-managed-policies

kibanamachine · 2021-02-10T21:29:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 28526e0

Metrics [docs]

✅ unchanged

History

💛 Build #105422 was flaky a810ce6
💔 Build #105363 failed 45f420c
💚 Build #104885 succeeded 6d9f231

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

nchaulet

🚀

* master: (44 commits) [APM] Add experimental support for Data Streams (elastic#89650) [Search Session] Control "Kibana / Search Sessions" management section by privileges (elastic#90818) [Lens] Median as default function (elastic#90952) Implement custom global header banner (elastic#87438) [Fleet] Reduce permissions. (elastic#90302) Update dependency @elastic/charts to v24.5.1 (elastic#89822) [Create index pattern] Can't create single character index without wildcard (elastic#90919) [ts/build_ts_refs] add support for --clean flag (elastic#91060) Don't clean when running e2e tests (elastic#91057) Fixes track_total_hits in the body not having an effect when using search strategy (elastic#91068) [Security Solution][Detections] Adds list plugin Saved Objects to Security feature privilege (elastic#90895) Removing the code plugin entirely for 8.0 (elastic#77940) chore(NA): move the instruction to remove yarn global bazelisk package into the first place on install bazel tools (elastic#91026) [jest/ci] remove max-old-space-size override to use 4gb default (elastic#91020) [Fleet] Restrict integration changes for managed policies (elastic#90675) [CI] Fix auto-backport condditions so that it doesn't trigger for other labels (elastic#91042) [DOCS] Uses variable to refer to query profiler (elastic#90976) [App Search] Relevance Tuning logic listeners (elastic#89461) [Metrics UI] Fix saving/loading saved views from URL (elastic#90216) Limit cardinality of transaction.name (elastic#90955) ...

) ## Summary - [x] Integrations cannot be added ~~, unless with a force flag~~ - [x] API - [x] UI - [x] tests - [x] Integrations cannot be removed ~~, unless with a force flag~~ - [x] API - [x] UI - [x] tests closes elastic#90445 refs elastic#89617 ### Cannot add integrations to managed policy <img height="400" alt="Screen Shot 2021-02-08 at 1 56 32 PM" src="https://user-images.githubusercontent.com/57655/107277261-25c48300-6a22-11eb-936a-0a7361667093.png"> ### Cannot delete integrations from managed policy <img alt="Screen Shot 2021-02-08 at 3 05 16 PM" src="https://user-images.githubusercontent.com/57655/107277318-337a0880-6a22-11eb-836f-fc66b510d257.png"> ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

…91150) ## Summary - [x] Integrations cannot be added ~~, unless with a force flag~~ - [x] API - [x] UI - [x] tests - [x] Integrations cannot be removed ~~, unless with a force flag~~ - [x] API - [x] UI - [x] tests closes #90445 refs #89617 ### Cannot add integrations to managed policy <img height="400" alt="Screen Shot 2021-02-08 at 1 56 32 PM" src="https://user-images.githubusercontent.com/57655/107277261-25c48300-6a22-11eb-936a-0a7361667093.png"> ### Cannot delete integrations from managed policy <img alt="Screen Shot 2021-02-08 at 3 05 16 PM" src="https://user-images.githubusercontent.com/57655/107277318-337a0880-6a22-11eb-836f-fc66b510d257.png"> ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

John Schulz added 2 commits February 8, 2021 14:13

service layer changes to limit add integrations

30225be

More restrictions on updating integrations on managed policies

6d9f231

jfsiii 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.12.0 labels Feb 8, 2021

jfsiii self-assigned this Feb 8, 2021

jfsiii mentioned this pull request Feb 8, 2021

[Fleet] Fleet server cannot be removed from a managed agent policy #89617

Closed

John Schulz added 3 commits February 9, 2021 10:15

Merge branch 'master' into 90445-no-integration-changes-for-managed-p…

90d93ac

…olicies

Tests for 'cannot add integrations to managed policies'

66ba36a

Cannot update integrations on a managed policy

45f420c

jfsiii marked this pull request as ready for review February 9, 2021 20:50

nchaulet reviewed Feb 9, 2021

View reviewed changes

Merge branch 'master' into 90445-no-integration-changes-for-managed-p…

a810ce6

…olicies

John Schulz added 2 commits February 10, 2021 14:22

"can't remove managed policy integrations" tests

3399029

Merge branch '90445-no-integration-changes-for-managed-policies' of g…

28526e0

…ithub.com:jfsiii/kibana into 90445-no-integration-changes-for-managed-policies

jfsiii requested review from mostlyjason and nchaulet February 10, 2021 21:19

nchaulet approved these changes Feb 10, 2021

View reviewed changes

jfsiii merged commit c92af5a into elastic:master Feb 10, 2021

jfsiii mentioned this pull request Feb 11, 2021

[7.x] [Fleet] Restrict integration changes for managed policies (#90675) #91150

Merged

jfsiii mentioned this pull request Feb 23, 2021

[Fleet] Reduce restrictions for removing integrations from managed policies #92426

Open

2 tasks

jfsiii deleted the 90445-no-integration-changes-for-managed-policies branch April 6, 2021 17:10

jfsiii mentioned this pull request Apr 12, 2021

[Fleet] support force flag to add/remove package_policies #96713

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] Restrict integration changes for managed policies #90675

[Fleet] Restrict integration changes for managed policies #90675

jfsiii commented Feb 8, 2021 •

edited

Loading

elasticmachine commented Feb 9, 2021

nchaulet left a comment

nchaulet Feb 9, 2021

jfsiii Feb 9, 2021

nchaulet Feb 9, 2021

jfsiii Feb 9, 2021

jfsiii commented Feb 9, 2021

jfsiii commented Feb 9, 2021

simitt commented Feb 10, 2021

jfsiii commented Feb 10, 2021

simitt commented Feb 10, 2021

kibanamachine commented Feb 10, 2021

nchaulet left a comment

	if (
	(parentAgentPolicy.package_policies as PackagePolicy[]).find(
	(siblingPackagePolicy) =>
	siblingPackagePolicy.id !== id && siblingPackagePolicy.name === packagePolicy.name
	)
	) {
	throw new Error('There is already a package with the same name on this agent policy');
	}

[Fleet] Restrict integration changes for managed policies #90675

[Fleet] Restrict integration changes for managed policies #90675

Conversation

jfsiii commented Feb 8, 2021 • edited Loading

Summary

Cannot add integrations to managed policy

Cannot delete integrations from managed policy

Checklist

elasticmachine commented Feb 9, 2021

nchaulet left a comment

Choose a reason for hiding this comment

nchaulet Feb 9, 2021

Choose a reason for hiding this comment

jfsiii Feb 9, 2021

Choose a reason for hiding this comment

nchaulet Feb 9, 2021

Choose a reason for hiding this comment

jfsiii Feb 9, 2021

Choose a reason for hiding this comment

jfsiii commented Feb 9, 2021

jfsiii commented Feb 9, 2021

simitt commented Feb 10, 2021

jfsiii commented Feb 10, 2021

simitt commented Feb 10, 2021

kibanamachine commented Feb 10, 2021

💚 Build Succeeded

Metrics [docs]

History

nchaulet left a comment

Choose a reason for hiding this comment

jfsiii commented Feb 8, 2021 •

edited

Loading