[Security Solution][Detections] fixes tests related to prebuilt rules update

[vitaliidm]

Summary

fixes tests related to prebuilt rules update tests failures(#138574 (comment)):

• removed hardcoded rule version value
• required_fields are not tested agains empty array, but agains actual immutable rule value

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 3b4e036

Metrics [docs]

✅ unchanged

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

cc @vitaliidm

[elasticmachine]

Pinging @elastic/security-detections-response (Team:Detections and Resp)

[elasticmachine]

Pinging @elastic/security-solution (Team: SecuritySolution)

[banderror]

Why required_fields are special so that we need to check them explicitly? What about other rule fields like related_integrations, index, etc? It feels like this expected object should be built based on immutableRule. Something like this:

        const expected = {
          ...immutableRule,
          immutable: true,
          actions: ...
          // and other properties that we expect to be changed

[vitaliidm]

It feels like this expected object should be built based on immutableRule

Agree on that. Here is a quick explanation why it's done like this

Scenarios written in this file are using immutable rule, fetching it by hardcoded ruleId.

       const immutableRule = await getRule(supertest, log, '9a1a2dae-0b5f-4c3d-8305-a268d404c306');
        const hookAction = await createNewAction(supertest, log);

Then rule mock data is created from getSimpleRule, that uses immutable rule id,
Existing immutable rule is getting updated with that mock, and then assertions are made against it.

        const newRuleToUpdate = getSimpleRule(immutableRule.rule_id);
        const ruleToUpdate = getRuleWithWebHookAction(hookAction.id, false, newRuleToUpdate);
        const updatedRule = await updateRule(supertest, log, ruleToUpdate);
        const bodyToCompare = removeServerGeneratedProperties(updatedRule);

I'm not sure, what is rationale for doing this. But I didn't want for a fix that unblocks another PR, start refactoring tests there, given BC4 soon.

We may look further, after release, as current implementation is not obvious for me, at least.
If you prefer I can look to refactor it now, but I'm not sure if no side effects will be discovered

[banderror]

@vitaliidm We should definitely unblock #138574 as soon as we can, so yea, let's merge this PR as is.

However, I think we should fix this test and make it more robust so it doesn't fail in the future for similar reasons. The fix could be safely backported to the 8.4 branch because it's a test 🙂 We don't have to wait for the release to happen, the test can be reworked in a follow-up PR right away.

[vitaliidm]

thanks @banderror
Here is a ticket to track it: #138757
I think it would make to schedule it in the next release, so next prebuilt update won't cause similar failures

[banderror]

@vitaliidm Thank you for taking over this PR with rule updates (#138574) 🙏 My suggestion is to merge this PR as is and rework the test in a follow-up one.

[banderror]

@vitaliidm We should definitely unblock #138574 as soon as we can, so yea, let's merge this PR as is.

However, I think we should fix this test and make it more robust so it doesn't fail in the future for similar reasons. The fix could be safely backported to the 8.4 branch because it's a test 🙂 We don't have to wait for the release to happen, the test can be reworked in a follow-up PR right away.

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.4

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

Questions ?

Please refer to the Backport tool documentation