Skip to content

[Fleet] Fix package policy merge logic for boolean values#123974

Merged
kpollich merged 1 commit intoelastic:mainfrom
kpollich:123945-fix-merge-logic-for-boolean-vars
Jan 27, 2022
Merged

[Fleet] Fix package policy merge logic for boolean values#123974
kpollich merged 1 commit intoelastic:mainfrom
kpollich:123945-fix-merge-logic-for-boolean-vars

Conversation

@kpollich
Copy link
Copy Markdown
Member

@kpollich kpollich commented Jan 27, 2022

Summary

Fixes #123945

Fixes a bug where Fleet's "merge" logic when upgrading package policies was unintentionally ignoring explicitly false boolean values on the original policy objects. In cases where a value was set to false on the original policy and defaulted to true in the package's manifest, we'd overwrite the false value with true.

To test

Create a policy on an older version of APM, e.g. 7.16.2. Flip the RUM Enabled switch to false and save your policy. Then, upgrade it to the latest version of APM. The RUM Enabled switch should remain false after the upgrade.

Checklist

@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 v7.17.0 labels Jan 27, 2022
@kpollich kpollich requested a review from nchaulet January 27, 2022 20:13
@kpollich kpollich requested a review from a team as a code owner January 27, 2022 20:13
@kpollich kpollich self-assigned this Jan 27, 2022
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jan 27, 2022

Pinging @elastic/fleet (Team:Fleet)

nchaulet
nchaulet approved these changes Jan 27, 2022
View reviewed changes
Copy link
Copy Markdown
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.

🚀

@kpollich kpollich enabled auto-merge (squash) January 27, 2022 20:22
auto-merge was automatically disabled January 27, 2022 20:33

Pull Request is not mergeable

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Jan 27, 2022

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

cc @kpollich

@kpollich kpollich merged commit 159825a into elastic:main Jan 27, 2022
@kibanamachine kibanamachine mentioned this pull request Jan 27, 2022
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 27, 2022
@kpollich @kibanamachine
Fix package policy merge logic for boolean values (elastic#123974)
5f0b9af 
(cherry picked from commit 159825a)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 27, 2022
@kpollich @kibanamachine
Fix package policy merge logic for boolean values (elastic#123974)
c5e230d 
(cherry picked from commit 159825a)
@kibanamachine kibanamachine mentioned this pull request Jan 27, 2022
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jan 27, 2022

💚 All backports created successfully

Status Branch Result
✅ 8.0
✅ 7.17

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

juliaElastic pushed a commit that referenced this pull request Jan 28, 2022
@kibanamachine @kpollich
Fix package policy merge logic for boolean values (#123974) (#123979)
f9acef8 
(cherry picked from commit 159825a)

Co-authored-by: Kyle Pollich <kyle.pollich@elastic.co>
juliaElastic pushed a commit that referenced this pull request Jan 28, 2022
@kibanamachine @kpollich
Fix package policy merge logic for boolean values (#123974) (#123980)
60a9838 
(cherry picked from commit 159825a)

Co-authored-by: Kyle Pollich <kyle.pollich@elastic.co>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 28, 2022
@jloleysens
Merge branch 'main' of github.com:elastic/kibana into screenshotting/…
6f3a9cd 
…fix-potential-race-condition-when-screenshotting

* 'main' of github.com:elastic/kibana: (75 commits)
  [Reporting] Logging improvements while generating reports (elastic#123802)
  [Uptime] Default alert connectors email settings (elastic#123244)
  Update comparison series styles to match the main series (elastic#123858)
  [RAC][Uptime] remove extra dot from the uptime alert connector message (elastic#124000)
  [Exploratory view] Allow ability add extra actions in lens embeddable (elastic#123713)
  [SecuritySolution][Investigations] Add message about missing index in data view in analyzer (elastic#122859)
  [TSVB] Formatting in the left axis is not respected when I have two separate axis (elastic#123903)
  [Discover] Remove services from component dependencies (elastic#121691)
  Stop IM rule execution if there are no events (elastic#123811)
  [Security Solution][Endpoint] Update Fleet Trusted Apps and Host Isolation Exception cards to use exception list summary API (elastic#123900)
  [Security Solution][Exceptions] Switches modal to flyout component (elastic#123408)
  [Workplace Search] Fix bug where modal visible after deleting a group (elastic#123976)
  [Alerting] Remove state variables from action variable menu (elastic#123702)
  replace deprecated api usage (elastic#123970)
  Fix package policy merge logic for boolean values (elastic#123974)
  [Security Solution][Endpoint][Policy] Remove GET policy list api route (elastic#123873)
  Reenable alert_add test suite (elastic#123862)
  [Fleet] Remove usage of IFieldType in Fleet (elastic#123960)
  [Lists] Add an instance of `ExceptionListClient` with server extension points turned off to context object provided to callbacks (elastic#123885)
  [Maps] Add execution context (elastic#123651)
  ...

# Conflicts:
#	x-pack/plugins/screenshotting/server/browsers/chromium/driver_factory/index.ts
@joshdover joshdover mentioned this pull request Jan 28, 2022
Open
@kpollich kpollich deleted the 123945-fix-merge-logic-for-boolean-vars branch January 28, 2022 12:27
awahab07 pushed a commit to awahab07/kibana that referenced this pull request Jan 31, 2022
@kpollich @awahab07
Fix package policy merge logic for boolean values (elastic#123974)
7664669
@dikshachauhan-qasource
Copy link
Copy Markdown

dikshachauhan-qasource commented Feb 7, 2022

Hi @kpollich

We have validated this PR in lieu of #123945 on staging and QA environment both and found it as ** NOT Working fine**

Step used:

  • Created a 7.17 Kibana deployment.
  • Enable APM integrations and observed it is added to Elastic agent on cloud policy with version 7.16.0
  • Upgraded APm integration to 7.17.0 version with few changes.
  • Then upgraded kibana to 8.0 BC1
  • Later, we observed error at fleet and fleet while upgrading kibana

Issue already reported under #124785. We will revalidate this PR once related ticket is fixed.

Thanks
QAS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nchaulet nchaulet nchaulet approved these changes

Assignees

@kpollich kpollich

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 v7.17.0 v8.0.0 v8.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

APM Integration - custom config options not preserved on update

6 participants

@kpollich @elasticmachine @kibana-ci @kibanamachine @dikshachauhan-qasource @nchaulet