[Fleet] Fix preconfiguration variable values #119749
Conversation
nchaulet
added
bug
|
@elasticmachine merge upstream |
|
Pinging @elastic/fleet (Team:Fleet) |
|
@elasticmachine merge upstream |
| @@ -1171,7 +1169,110 @@ export function overridePackageInputs( | |||
| return resultingPackagePolicy; | |||
| } | |||
|
|
|||
| function deepMergeVars(original: any, override: any): any { | |||
| export function preconfigurePackageInputs( | |||
There was a problem hiding this comment.
could these new functions be moved to separate files? this service is more than 1000 lines long.
There was a problem hiding this comment.
Yes I think it will make sense to refactorize this whole service, I am going to create a follow up issue to that if it's correct for you?
There was a problem hiding this comment.
I created that issue here #119844
| ], | ||
| }; | ||
|
|
||
| const packageInfo: PackageInfo = { |
There was a problem hiding this comment.
it takes a lot of code to set up these objects. could we extract default values to a common place and only override the specific parts for a test?
There was a problem hiding this comment.
If we refactorize the package policy we will have to refactorize the tests and it will probably a better time to create these fixture functions, what do you think?
There was a problem hiding this comment.
Definitely a welcome change. Having a single code path try to account for multiple use cases around applying policy values (upgrades vs preconfiguration) was a poor implementation choice out of the gate. Having two distinct implementations here is much more stable and avoids nasty bugs with our policy merging logic.
The other review comments look good to me, so I'll approve for once those are addressed.
🚀
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @nchaulet |
💔 Backport failed
Successful backport PRs will be merged automatically after passing CI. To backport manually run: |
Co-authored-by: Nicolas Chaulet <[email protected]>
# Conflicts: # x-pack/plugins/fleet/server/services/preconfiguration.ts
Summary
Resolve #119748
When we introduced the package upgrade we reused and modified the
overrideInputsfunction that was created to be used for preconfiguration function but we want to different behavior for preconfiguration and upgrade. This resulted in a bug where the values provided by the user for preconfiguration are not used.That PR fix that by splitting the
overrideInputsin two different functionupdatePackageInputsandpreconfigurePackageInputs.