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] Update logic for "Keep policies up to date" defaults in 8.0 #119126

Merged
merged 7 commits into from
Nov 21, 2021

Conversation

kpollich
Copy link
Member

@kpollich kpollich commented Nov 19, 2021

Summary

Ref #111858

Introduces two new constants that control the functionality around our keep_policies_up_to_date field:

  1. KEEP_POLICIES_UP_TO_DATE_PACKAGES - controls packages on which we allow users to configure keep_policies_up_to_date via the integration settings UI
  2. AUTO_UPGRADE_POLICIES_PACKAGES - list of packages for which we default keep_policies_up_to_date to true, and do not allow users to opt out.

Previously, we applied a default of true to all DEFAULT_PACKAGES, but in reality, the only packages that are required to opt into this behavior are APM and Synthetics - which we consider "stack-aligned" due to shipping custom policy editor UI's with Kibana. So, in this PR we've added a migration to set keep_policies_up_to_date: true for these two packages. Previously supplied values for default packages will remain in place, and the migration only touches AUTO_UPGRADE_POLICIES_PACKAGES.

To summarize:

Packages keep_policies_up_date default UI behavior
APM, Synthetics true Setting is shown, but disabled
Default + auto update packages (e.g. system, fleet_server) undefined Setting is shown and enabled
All other packages undefined Setting is not shown

For historical context around "stack aligned package upgrades", see these issues:

In 7.16, we pared down the implementation here to avoid breakages. With 8.0 as the target for this change, it seems appropriate that we ease our concerns for breaking changes.

For some additional context, our initial plan around this functionality was to push the "auto-upgrade policies" opt-in out into the package spec via this change proposal: elastic/package-spec#244. However, it seems after some thorough and helpful conversations with members of the ecosystem team, we won't land on exactly the functionality we initially specced out. Things are still in flight here, so for the time being we've elected to unblock ourselves and continue relying on our hardcoded package lists to support automatic policy upgrades for our various managed packages.

With this PR the list of "managed" packages in 8.0 will be

  • system
  • elastic_agent
  • fleet_server
  • endpoint
  • apm
  • synthetics

Related issues

Testing Instructions

Installation Path

  1. Install the apm integration

image

  1. Verify that the "Keep Policies up to date" switch defaults to the "enabled" setting

image

Upgrade Path

  1. On a 7.16 instance, navigate directly to <kibana>/integrations/detail/apm-0.4.0/overview and add the apm integration

image

  1. Once the integration is added, verify that the "keep policies up to date" switch is in the "disabled" setting

image

  1. Upgrade the 7.16 instance to 8.0+ and verify the setting has switched to the "enabled" position w/ a disabled input, and that the apm integration policy has been upgraded

image

image

@kpollich kpollich added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 labels Nov 19, 2021
@kpollich kpollich requested a review from joshdover November 19, 2021 01:58
@kpollich kpollich requested a review from a team as a code owner November 19, 2021 01:58
@kpollich kpollich self-assigned this Nov 19, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@@ -534,9 +534,9 @@ export async function createInstallation(options: {
const removable = !isUnremovablePackage(pkgName);
const toSaveESIndexPatterns = generateESIndexPatterns(packageInfo.data_streams);

// For default packages, default the `keep_policies_up_to_date` setting to true. For all other
// package, default it to false.
const defaultKeepPoliciesUpToDate = DEFAULT_PACKAGES.some(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original intent here should have been to leave keep_policies_up_to_date set to undefined unless it was explicitly set by the user, but this code introduced in #116993 sets the default value to false instead for any non-default package that's installed.

Because we explicitly set this to false during installation for any non-default package, we can't reliably honor user settings during our migration.

For example, if a user installed apm on their 7.16 Kibana instance, the installation SO would have keep_policies_up_to_date: false. So, we have to make a choice in this PR whether we should discard any user-provided value for keep_policies_up_to_date on managed policies and simply set them all globally to true.

Copy link
Member Author

@kpollich kpollich Nov 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To close the loop here, I spoke with Josh offline and got a few pieces of direction here.

  • The only packages that must have their policies upgraded are APM and Synthetics
  • We shouldn't automatically opt system into keeping policies up to date, because it's widely used and could cause rollout/performance issues in environments with many agents

With those two things in mind, I elected to make the changes detailed in my previous comment.

Copy link
Contributor

@criamico criamico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, LGTM

@@ -72,6 +72,8 @@ export const AUTO_UPDATE_PACKAGES = autoUpdatePackages.map((name) => ({
version: PRECONFIGURATION_LATEST_KEYWORD,
}));

export const MANAGED_PACKAGES = [...DEFAULT_PACKAGES, ...AUTO_UPDATE_PACKAGES];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the naming kind of confusing we already use the managed term for the preconfigured policy (Probably a bad naming) but what about a more explicit name here like KEEP_POLICIES_UP_TO_DATE_PACKAGES

@kpollich
Copy link
Member Author

I pushed up 7a444a4 which makes a few changes to our logic here

  1. The only packages that are opted into automatic policy upgrades are now APM and Synthetics. This set is captured in a new AUTO_UPGRADE_POLICIES_PACKAGES constant
  2. The "Keep integration policies up to date" switch is now disabled for packages that appear in AUTO_UPGRADE_POLICIES_PACKAGES
  3. Packages that allow the keep_policies_up_to_date switch to appear at all are now captured in a better named KEEP_POLICIES_UP_TO_DATE_PACKAGES constant - cc @nchaulet

@kpollich kpollich changed the title [Fleet] Auto-upgrade policies for all managed packages by default [Fleet] Update logic for "Keep policies up to date" defaults in 8.0 Nov 19, 2021
@kpollich
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for changing the naming and all the comments make things a lot more better 🚀

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
fleet 1127 1130 +3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 630.8KB 631.0KB +270.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fleet 104.7KB 105.2KB +433.0B
Unknown metric groups

API count

id before after diff
fleet 1229 1232 +3

History

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

cc @kpollich

@kpollich kpollich merged commit c1a5cee into elastic:main Nov 21, 2021
@kpollich kpollich deleted the upgrade-managed-policies branch November 21, 2021 18:18
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Nov 21, 2021
…lastic#119126)

* Auto-upgrade policies for all managed packages by default

* Fixing unused import in test

* Add migration to installation list

* Update naming + disable input for APM + Synthetics

* Remove unused import + add message for disabled policy update settingg

* Fix failing tests

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

💚 Backport successful

Status Branch Result
8.0

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Nov 21, 2021
…119126) (#119270)

* Auto-upgrade policies for all managed packages by default

* Fixing unused import in test

* Add migration to installation list

* Update naming + disable input for APM + Synthetics

* Remove unused import + add message for disabled policy update settingg

* Fix failing tests

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kyle Pollich <[email protected]>
dmlemeshko pushed a commit that referenced this pull request Nov 29, 2021
…119126)

* Auto-upgrade policies for all managed packages by default

* Fixing unused import in test

* Add migration to installation list

* Update naming + disable input for APM + Synthetics

* Remove unused import + add message for disabled policy update settingg

* Fix failing tests

Co-authored-by: Kibana Machine <[email protected]>
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
…lastic#119126)

* Auto-upgrade policies for all managed packages by default

* Fixing unused import in test

* Add migration to installation list

* Update naming + disable input for APM + Synthetics

* Remove unused import + add message for disabled policy update settingg

* Fix failing tests

Co-authored-by: Kibana Machine <[email protected]>
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:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants