[Fleet] Package policy saving failure when secrets enabled for the first time

[jillguyonnet]

Part of #154715

Followup to #157176

Context

With the merging of #157176 input variables can be saved and stored as secrets from Fleet UI. There is one remaining bug that happens when a package policy with secret variables was created before the feature was enabled and edited afterwards.

Steps to reproduce

Cf. #157176 (comment)

1. Disable the secretsStorage feature flag.
   • For this, make sure the value secretsStorage is not present in xpack.fleet.enableExperimental in your kibana.dev.yml.
2. Run the test package registry as described in [Fleet] View & Edit Secret Variables #157176 and run Kibana.
   • Note: to "point kibana to the registry", add xpack.fleet.registryUrl: http://localhost:8080 to your kibana.dev.yml.
3. Add the "Package with secrets" integration, fill at least one secret variable and save the package policy.
4. Enable the secretsStorage feature flag (xpack.fleet.enableExperimental: ['secretsStorage']).
5. Edit the package policy and change the value of a secret variable.
6. Click Save integration. The first time, the request will fail with 404 Bad Request and the following error message:

   KQLSyntaxError: Expected "(", NOT, value but ")" found.
   ingest-package-policies.attributes.secret_references.id: ( or  or )
   

7. Click Save integration again. This time, the request will succeed and the package policy will be saved correctly.

Details and observations

Server error

The following error appears in Kibana logs:

[ERROR][plugins.fleet] KQLSyntaxError: Expected "(", NOT, value, whitespace but ")" found.
ingest-package-policies.attributes.secret_references.id: ()
----------------------------------------------------------^: Bad Request

Error stack trace:

KQLSyntaxError: Expected "(", NOT, value, whitespace but ")" found.
ingest-package-policies.attributes.secret_references.id: ()
----------------------------------------------------------^
    at Object.fromKueryExpression (ast.ts:52:13)
    at validateConvertFilterToKueryNode (filter_utils.ts:26:44)
    at performFind (find.ts:124:51)
    at SavedObjectsRepository.find (repository.ts:321:29)
    at SavedObjectsClient.find (saved_objects_client.ts:102:35)
    at PackagePolicyClientImpl.list (package_policy.ts:619:44)
    at findPackagePoliciesUsingSecrets (secrets.ts:152:54)
    at deleteSecretsIfNotReferenced (secrets.ts:115:45)
    at PackagePolicyClientImpl.update (package_policy.ts:841:22)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at updatePackagePolicyHandler (handlers.ts:366:34)
    at Router.handle (router.ts:206:30)
    at handler (router.ts:161:13)
    at exports.Manager.execute (/Users/jillguyonnet/Code/jillguyonnet/kibana/node_modules/@hapi/hapi/lib/toolkit.js:60:28)
    at Object.internals.handler (/Users/jillguyonnet/Code/jillguyonnet/kibana/node_modules/@hapi/hapi/lib/handler.js:46:20)
    at exports.execute (/Users/jillguyonnet/Code/jillguyonnet/kibana/node_modules/@hapi/hapi/lib/handler.js:31:20)
    at Request._lifecycle (/Users/jillguyonnet/Code/jillguyonnet/kibana/node_modules/@hapi/hapi/lib/request.js:371:32)
    at Request._execute (/Users/jillguyonnet/Code/jillguyonnet/kibana/node_modules/@hapi/hapi/lib/request.js:281:9)

This error originates here. Note that the catch block is responsible for creating the Bad Request response. The error itself seems to be due to invalid parsing of the initial filter query param.

API requests

There are no differences in the API request payloads between the first failed request and the second successful requests. Only the responses (404 vs. 200) are different due to the above thrown error.

Screenshots

First request fails:

Second request succeeds:

[elasticmachine]

Pinging @elastic/fleet (Team:Fleet)

[joshdover]

Related to this - are we handling the case where an integration has a field that was not a secret in version X and then changes the field to secret in version X+1? Any existing policies should be upgraded and have the previous values of this field now saved as secrets and removed from the Saved Object.

[jillguyonnet]

I cannot reproduce the bug on latest main. I remember it being pretty consistent last time, so maybe one of the recent changes smoothed it out.

@joshdover I'm not sure whether that scenario was explicitly listed, but I can try to test it.

[juliaElastic]

@jillguyonnet The feature flag was just enabled by default, have you tried setting to false when trying to reproduce the bug?

EDIT: I just tried it, and couldn't reproduce it either, so I think we fixed this issue with the changes.

[juliaElastic]

@jillguyonnet The scenario that Josh described is part of this issue, I can assign to you: #162045