-
Couldn't load subscription status.
- Fork 128
Allow to run system benchmarks with more than one input #3019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
95d7abb to
3dcbab5
Compare
873010c to
8e5c1b5
Compare
💛 Build succeeded, but was flaky
Failed CI StepsHistory
cc @mrodm |
| // Set default values for scenario fields from package manifest if not set | ||
| if r.scenario.Version == "" { | ||
| r.scenario.Version = pkgManifest.Version | ||
| } | ||
|
|
||
| if r.scenario.Package == "" { | ||
| r.scenario.Package = pkgManifest.Name | ||
| } | ||
|
|
||
| if r.scenario.PolicyTemplate == "" { | ||
| r.scenario.PolicyTemplate = pkgManifest.PolicyTemplates[0].Name | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These checks/assignments were previously inside createPackagePolicy, it feels like they should be set earlier here to setup all the fields in the scenario variable.
| // By default, all policy templates are enabled when creating a package policy. | ||
| // This could lead to errors if other policy templates have required variables. | ||
| // Therefore, all other policy templates and inputs must be disabled since here | ||
| // just the variables for the current input are set. | ||
| // NOTE: This data is retrieved from the local package manifest. | ||
| for _, policyTemplate := range pkgManifest.PolicyTemplates { | ||
| for _, input := range policyTemplate.Inputs { | ||
| if policyTemplate.Name == r.scenario.PolicyTemplate && input.Type == r.scenario.Input { | ||
| continue | ||
| } | ||
| pp.Inputs[fmt.Sprintf("%s-%s", policyTemplate.Name, input.Type)] = kibana.PackagePolicyInput{ | ||
| Enabled: false, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Screenshots with examples of the explanation explained in the comment are posted in the description of the PR.
As mentioned in the comment, setting this policy templates as disabled is based on the manifests read from the files found in the package locally.
It follows the same approach as to choose the default policy template reading from the value from the manifest.
elastic-package/internal/benchrunner/runners/system/runner.go
Lines 430 to 432 in d5f73ab
| if r.scenario.PolicyTemplate == "" { | |
| r.scenario.PolicyTemplate = pkgManifest.PolicyTemplates[0].Name | |
| } |
This would be something to check if it is required to use other package or versions as mentioned in the documentation:
elastic-package/docs/howto/system_benchmarking.md
Lines 231 to 234 in d5f73ab
| | package | string | | The name of the package. If omitted will pick the current package, this is to allow for future definition of benchmarks outside of the packages folders. | | |
| | description | string | | A description for the scenario. | | |
| | version | string | | The version of the package to benchmark. If omitted will pick the current version of the package. | | |
| | policy_template | string | | The policy template to test. If omitted will pick the first one. | |
| Vars: r.scenario.Vars, | ||
| Streams: map[string]kibana.PackagePolicyStream{ | ||
| fmt.Sprintf("%s.%s", pkgManifest.Name, r.scenario.DataStream.Name): { | ||
| fmt.Sprintf("%s.%s", r.scenario.Package, r.scenario.DataStream.Name): { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, this was using directly pkgManifest.Name , is it ok to use r.scenario.Package instead ? @marc-gr
| } | ||
| } | ||
|
|
||
| pp.Package.Name = r.scenario.Package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, this was using directly pkgManifest.Name , is it ok to use r.scenario.Package instead ? @marc-gr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mainly added notes in this documentation file in case developers are running system benchmark with local changes.
Fixes #2904
Add support to run system benchmarks in those packages that contain more than one input. And one of the other inputs not used in the system benchmark define any variable as required.
The current behaviour with the request performed, Fleet enabled all the inputs. If one of the inputs not used in the system benchmark defines that requires any variable, it could fail with an error of missing required variables.
As example, if it runs a system benchmark for the input
httpjsonfor thesentinel_onepackage, it failed since thecelinput contains some required variables and it is enabled:This PR disables all the unused policy templates/inputs, and it just enables and configures the policy_template and input defined in the system benchmark.
Author's Checklist
system_benchmark_sentinel_onecontaining more two inputs, and both of the inputs with required variables (copied fromsentinel_onepackage in the integrations repository).panw,ti_abusech).