-
Notifications
You must be signed in to change notification settings - Fork 523
Improve system test coverage for all inputs - 2 #16675
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
Improve system test coverage for all inputs - 2 #16675
Conversation
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
| @@ -1,3 +1,4 @@ | |||
| deployer: tf | |||
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.
Can you fix indentation of assert.hit_count on this file?
Also add another event and update assert.hit_count: 2 (other inputs too)
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.
@moxarth-rathod, this still holds. Can you update it?
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.
@kcreddy done.
| type: flattened | ||
| - name: storage | ||
| type: flattened |
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.
Can you check azure* integrations and add explicit mappings here instead of flattened?
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.
I ran into an issue during the elastic package check because adding the required fields in the Beats YAML for system tests pushed the total field count beyond the 2048 limit. To resolve this, I had to flatten some of the fields.
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.
You could increase the total_fields.limit setting inside the manifest.yml to get around this: Example
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.
Thanks for the suggestion. I tried increasing the total_fields.limit setting in the manifest.yml, but it did not have the intended effect and the issue still persists.
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.
May I know the full error you are getting?
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.
@moxarth-rathod, I just tested your latest commit with total_fields.limit setting as follows:
elasticsearch:
index_template:
settings:
index:
mapping:
total_fields:
limit: 3000and updated beats.yml to reflect all fields:
- name: azure
type: group
fields:
- name: storage
type: group
fields:
- name: container.name
type: keyword
multi_fields:
- name: text
type: text
description: The name of the Azure Blob Storage container
- name: blob.name
type: keyword
description: The name of the Azure Blob Storage blob object
multi_fields:
- name: text
type: text
- name: blob.content_type
type: keyword
description: The content type of the Azure Blob Storage blob object
- name: subscription_id
type: keyword
description: |
Azure subscription ID
- name: correlation_id
type: keyword
description: |
Correlation ID
- name: tenant_id
type: keyword
description: |
tenant ID
- name: resource
type: group
fields:
- name: id
type: keyword
description: |
Resource ID
- name: group
type: keyword
description: |
Resource group
- name: provider
type: keyword
description: |
Resource type/namespace
- name: namespace
type: keyword
description: |
Resource type/namespace
- name: name
type: keyword
description: |
Name
- name: authorization_rule
type: keyword
description: |
Authorization ruleAnd I got all tests passed
--- Test results for package: symantec_endpoint_security - START ---
╭────────────────────────────┬─────────────┬───────────┬───────────┬────────┬───────────────╮
│ PACKAGE │ DATA STREAM │ TEST TYPE │ TEST NAME │ RESULT │ TIME ELAPSED │
├────────────────────────────┼─────────────┼───────────┼───────────┼────────┼───────────────┤
│ symantec_endpoint_security │ event │ system │ azure │ PASS │ 52.463933958s │
│ symantec_endpoint_security │ event │ system │ default │ PASS │ 2m5.3946485s │
│ symantec_endpoint_security │ event │ system │ gcs │ PASS │ 46.601027458s │
╰────────────────────────────┴─────────────┴───────────┴───────────┴────────┴───────────────╯
--- Test results for package: symantec_endpoint_security - END ---
Done
Could it be that you are setting this value to low value, say 2000?
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.
The issue is not related to the system tests. It occurs during the package build process, elastic-package check command still reports the same error.
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.
Yeah, this is coming from package-spec. I will create an issue for this. Thanks!
For now you can make them flattened.
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.
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.
Thanks!
|
@ShourieG, can you please review GCS mock service used in the system tests? |
ShourieG
left a comment
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.
LGTM for now, I will create a docker image for the mock service and upload to docker registry soon, after that we can remove the redundant mock service code from all the packages and use the image directly
kcreddy
left a comment
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.
Waiting on #16675 (comment)
💚 Build Succeeded
History
|
Proposed commit message
Checklist
changelog.ymlfile.Related issues