-
Notifications
You must be signed in to change notification settings - Fork 444
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
cisco_umbrella: add dashboards #9551
Conversation
85e3fdc
to
c4ae040
Compare
Audit logs are omitted as these are not correctly handled due to the presence of new-lines in CSV records.
🚀 Benchmarks reportTo see the full report comment with |
487ffe3
to
4cfa09e
Compare
/test |
4cfa09e
to
59bb594
Compare
…elpful This is not done for global_settings as it is hierarchically organised in a way that cannot be expressed here.
59bb594
to
62646e4
Compare
💚 Build Succeeded
History
cc @efd6 |
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
"before_values": { | ||
"deviceKey": "1111::2222222\\nlabel: 1787\\nbundle: minimal\\nphishing: 1\\ncreatedAt: 2023-03-16 09:37:31\\ntimeZoneName: GMT\\noriginId: 222222222\\ndeviceTypeId: 1\\nmaxBlockedDomains: 25\\nmaxNoredirectDomains: 25\\nmaxWhitelistDomains: 10\\norganizationId: 1111111\\noriginTypeId: 34\\nmodifiedAt: 2023-03-16 09:37:31\\n" | ||
}, |
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 don't think parsing is working as expected. This still has 1 key deviceKey
with rest of the value in it.
If it has the correct split, shouldn't this look like:
"before_values": {
"deviceKey": "1111::2222222",
"label": 1787,
"bundle": "minimal",
"phishing": 1
......
},
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.
Same for the after_values
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 expected. If you look at the equivalent in the system tests this does work. The issue is that Cisco have used actual CRLF in the strings, not \r\n
escape sequences. This is possible to do in system tests, but it it not possible to represent in pipeline tests.
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 👍🏼
Package cisco_umbrella - 1.23.0 containing this change is available at https://epr.elastic.co/search?package=cisco_umbrella |
Proposed commit message
See title.
Checklist
changelog.yml
file.Author's Checklist
How to test this PR locally
Related issues
Screenshots