Skip to content

Conversation

harshilgajera-crest and others added 7 commits February 5, 2025 15:20
Updated 2  fields in Network Resolution model.

- Added expected values for reply_code_id which has corresponding
reply_code.
- Added expected values for reply_code.
Added support for cim v5.3.2.

- Updated data-models with new child data set in various models.
- Updated required fields with updated values as per v5.3.2.
- Added optional fields as per v5.3.2

Detailed comparison and analysis between v4.15.0 and v5.3.2 can be found
here:
https://docs.google.com/spreadsheets/d/1ZFDC0Efn-bHvcU1Qy78s95GCfWyxt6IUhTv94j3yagk/edit#gid=1147250948
Added new data models:

- Compute_Inventory
- Data_Access
- Databases
- Event_Signatures
- Interprocess Messaging
- JVM
- Performance
- Ticket_Management

Updated version in requirement_test_datamodel_tag_constants.py file
This PR removes the feature of generating cim-field-report.
ref: [ADDON-73385](https://splunk.atlassian.net/browse/ADDON-73385)

NOTE:
- moved unit test file (test_report.py) of cim-compliance report
generation test from test_tools folder to test_utilities
Added changes based on CIM 6.0.0 and CIM 5.3.2 changes

Added Session ID in Authentication Model:
https://cd.splunkdev.com/EnterpriseSecurity/sa-commoninformationmodel/-/merge_requests/545

Signature field description change in Intrusion Detection Model:
https://cd.splunkdev.com/EnterpriseSecurity/sa-commoninformationmodel/-/merge_requests/542

Protocol Version description change in Network Traffic Model:
https://cd.splunkdev.com/EnterpriseSecurity/sa-commoninformationmodel/-/merge_requests/544

Power field description change in Performance Model:
https://cd.splunkdev.com/EnterpriseSecurity/sa-commoninformationmodel/-/merge_requests/543
chore:  Downgrading Ubuntu version as python 3.7 is no longer supported on
latest
@dvarasani-crest dvarasani-crest changed the title chore(release): making a new release chore(release): making a new release v6.0.0 Feb 5, 2025
renovate bot and others added 4 commits February 5, 2025 15:58
- Update datamodel definition for CIM v5.3.2 according to the
[sheet](https://docs.google.com/spreadsheets/d/1ZFDC0Efn-bHvcU1Qy78s95GCfWyxt6IUhTv94j3yagk/edit?gid=1873743384#gid=1873743384)
- Add datamodel definition for CIM v6.0.0
- Update copyright year
This PR fixes the issue with the token replacement for the fields
defined under `other_mappings` for the sample event.

- Updated the e2e tests to cover the token replacement scenario for
`other_mappings`
"dest_nt_domain",
"src_nt_domain",
"src_user",
"src_user_name",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we removing this? looks like it's still here https://docs.splunk.com/Documentation/CIM/6.0.2/User/Change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was decided that the CIM datamodel jsons should be considered as single source of truth (reference) and the fields user_name and src_user_name are optional and not marked as recommended.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as before, I'm not sure what to do here. this is the technically correct call, but the whole point of filing CIM tickets for changes is to actually change CIM and not just the docs. this seems like the worst possible outcome where we spend time fixing things, telling customers what to expect, then don't actually fix them and yet our tests say the docs are wrong.

"type": "optional",
"expected_values": ["lockout"],
"condition": "status=failure",
"type": "conditional",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the meaning of type= conditional

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It means that the field is required under certain condition and the condition is also defined.

"name": "parent_process",
"name": "original_file_name",
"type": "optional",
"comment": "Original name of the file, not including path."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original name of the file, not including path. Sometimes this field is similar to process name but the two do not always match, such as process_name=pwsh and original_file_name=powershell.exe to detect renamed instances of any process executing. per https://docs.splunk.com/Documentation/CIM/6.0.0/User/Endpoint

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This align with the description here. Also, these comments are not being used anywhere in the PSA tests.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, the correct response seems to be to get the CIM app updated rather than reject the progress made on giving customers and ourselves clarity. I understand that the CIM app is the ultimate arbiter of what is "correct", but I don't understand where the disconnect is happening between us raising CIM tickets and them being implemented, but only in the docs and not in actual code.

{
"name": "file_size",
"type": "optional",
"type": "required",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@dvarasani-crest dvarasani-crest Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was decided here with alexei to keep this field for the "Filesystem" dataset as required

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexeisuv, what is the reasoning here, it doesn't align with our docs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justin-splunk @dvarasani-crest The Endpoint.json says it is the recommended field, which means the PSA should fail if it is not extracted. The flag "recommended" is what historically was used in the CIM jsons for the required fields.

So we changed it from "optional" to "required" in the PSA json, because I thought the PSA terminology does not have the "recommended" but "required" instead, is it not so? If this is not correct, then can you clarify the difference between the recommended and required in the PSA specifically?

P.S. On another note, we may need to remove the "recommended" flag from this field in the CIM json. I do not see this field (file_size) is so critical to be recommended/required, but it is up to you, Justin, since you are dealing a lot with the Endpoint DM in your TAs.

Copy link

@justin-splunk justin-splunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple questions. also, 6.0.1 and 6.0.2 have been released, why aren't we working on that version?

alexeisuv
alexeisuv previously approved these changes Feb 13, 2025
This PR updates the condition for the object_category field of the
Change DataModel.
Added e2e tests to cover this check.
props.conf had invalid syntax using line breakers (issue reported here:
https://splunk.atlassian.net/browse/ADDON-72549
). The incorrect format is causing warnings in the splunkd log, which
could be easily prevented by correcting the props conf file like in this
[example](https://github.com/splunk/splunk-add-on-for-microsoft-cloud-services/pull/1196/files).

Related Jira:
https://splunk.atlassian.net/browse/ADDON-77954

Example local run: 
<img width="1707" alt="image"
src="https://github.com/user-attachments/assets/33e5247b-0f93-494c-9685-a48ddfb43a67"
/>
alexeisuv
alexeisuv previously approved these changes Feb 19, 2025
Reverts #901
We decided to keep the check for conf files in app inspect (the check is
already implemented, but requires a fix) :
https://splunk.atlassian.net/browse/A3-1577

Also upgraded pip for test-splunk-external job:
https://github.com/splunk/pytest-splunk-addon/actions/runs/13541590970/job/37843909257#step:3:19
@dvarasani-crest dvarasani-crest merged commit a84484b into main Mar 7, 2025
21 of 22 checks passed
@dvarasani-crest dvarasani-crest deleted the release/v6.0.0 branch March 7, 2025 10:48
@github-actions github-actions bot locked and limited conversation to collaborators Mar 7, 2025
@srv-rr-github-token
Copy link
Contributor

🎉 This PR is included in version 5.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@srv-rr-github-token
Copy link
Contributor

🎉 This PR is included in version 6.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants