Consider throwing an error when more than one benchmark was selected

[ofiriro3]

Motivation Currently, when we try to extract the benchmark from the package policy, we use a tolerant approach. I.E when one or more benchmarks were selected, we select Kubernetes vanilla as the default input type. We think that we should consider a stricter approach here - if more than one benchmark were selected, we should throw an error.

In-depth The fleet extension should not let the user choose more than one benchmark, so by selecting a default benchmark for the user, we hide bugs that we might have in our flow - we wish to detect these bugs if there are any. Moreover, we cannot use Vanilla as the default value for CSPM, it can cause some unexpected behavior.

Relevant code:

export const getBenchmarkFromPackagePolicy = (
  inputs: PackagePolicy['inputs'] | NewPackagePolicy['inputs']
): BenchmarkId => {
  const enabledInputs = inputs.filter(isEnabledBenchmarkInputType);

  // Use the only enabled input
  if (enabledInputs.length === 1) {
    return getInputType(enabledInputs[0].type);
  }

fyi @kfirpeled @orouz

[elasticmachine]

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security Posture)

[kfirpeled]

Currently not a high priority, we don't have any users that use our API Delaying to 8.8

[orouz]

just to clarify - getBenchmarkFromPackagePolicy is used in our code in-

server: https://github.com/elastic/kibana/blob/bdd12a638e77269f83563c925380b563b4f8093e/x-pack/plugins/cloud_security_posture/server/routes/benchmarks/benchmarks.ts#L67

client: https://github.com/elastic/kibana/blob/bdd12a638e77269f83563c925380b563b4f8093e/x-pack/plugins/cloud_security_posture/public/pages/rules/use_csp_rules.ts#L39

and in both cases, if getBenchmarkFromPackagePolicy returns a default value we pick: https://github.com/elastic/kibana/blob/bdd12a638e77269f83563c925380b563b4f8093e/x-pack/plugins/cloud_security_posture/common/utils/helpers.ts#L60

it's likely a bug, as we can't provide a default input without knowing the policy templtae

[ofiriro3]

Don't we think it is a critical bug if a customer will try to create a CSPM integration and he will end up with a Vanilla benchmark?

@kfirpeled @orouz ?

[tehilashn]

@ofiriro3 - how can the user select more than one benchmark? since it's not possible via the UI then I don't think it's a critical bug.

[ofiriro3]

cc

Hi @tehilashn thanks for your response.

how can the user select more than one benchmark? since it's not possible via the UI then I don't think it's a critical bug.

It can be created by using an API call and not the UI.

I just think if I do a risk assessment, I would consider the following:

1. If our system works as expected and doesn't allow a customer to choose multiple benchmarks, then returning an error won't matter and will just make our code less error-prone.
2. If our system does not work as expected, and this default value is being set on some scenarios we are covering a serious bug.

The fix is pretty straightforward and does not involve any complexity.

But again, that is only my opinion, we can do it on 8.8 if you guys think it is not urgent enough.

[tehilashn]

Thanks @ofiriro3 - I don't think it urgent for 8.7. Let's postpone for now.