Skip to content
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

adding RN106 and RN107 #4643

Merged
merged 26 commits into from
Nov 19, 2024
Merged

adding RN106 and RN107 #4643

merged 26 commits into from
Nov 19, 2024

Conversation

dantavori
Copy link
Contributor

@dantavori dantavori commented Nov 4, 2024

Related Issues

fixes: https://jira-dc.paloaltonetworks.com/browse/CIAC-12004

Description

Test PR: demisto/content#37074
Changes:

  • NGINXApiModule -> should raise RN106 for ExportIndicators pack and RN107 for TAXII Server 2 integration, but not EDL which has a RN
  • SplunkPy - should raise RN107 (RN exists but not entry) due to Splunk Generic playbook change
  • MacOS - should raise RN106 due to modeling rule schema change
  • CortexXDR - should raise RN106 due to integration description change
  • Mattermost - should raise RN106 due to integration script change
  • Tripwire - only TPB was changed so shouldn't raise a validation error
    Note that in case of RN106, RN107 won't be raised
  • ServiceNow_v2 - has a RN entry so no validation error
  • ignored files which shouldn't raise validation errors: Packs/EWS/CONTRIBUTORS.json, Packs/Gem/Author_image.png, Packs/Whois/.pack-ignore, Packs/Slack/Integrations/SlackV3/SlackV3_test.py
Packs/SplunkPy/Playbooks/playbook-Splunk_Generic.yml: [RN107] - No release note entry was found for the playbook "Splunk Generic" in the SplunkPy pack. Please rerun the update-release-notes command without -u to generate an updated template. If you are trying to exclude an item from the release notes, please refer to the documentation found here - https://xsoar.pan.dev/docs/integrations/changelog#excluding-items
Packs/TAXIIServer/Integrations/TAXII2Server/TAXII2Server.yml: [RN107] - No release note entry was found for the integration "TAXII2 Server" in the TAXIIServer pack. Please rerun the update-release-notes command without -u to generate an updated template. If you are trying to exclude an item from the release notes, please refer to the documentation found here - https://xsoar.pan.dev/docs/integrations/changelog#excluding-items
Packs/MacOS: [RN106] - Release notes were not found. Please run `demisto-sdk update-release-notes -i Packs/MacOS -u (major|minor|revision|documentation)` to generate release notes according to the new standard. You can refer to the documentation found here: https://xsoar.pan.dev/docs/integrations/changelog for more information.
Packs/CortexXDR: [RN106] - Release notes were not found. Please run `demisto-sdk update-release-notes -i Packs/CortexXDR -u (major|minor|revision|documentation)` to generate release notes according to the new standard. You can refer to the documentation found here: https://xsoar.pan.dev/docs/integrations/changelog for more information.
Packs/Mattermost: [RN106] - Release notes were not found. Please run `demisto-sdk update-release-notes -i Packs/Mattermost -u (major|minor|revision|documentation)` to generate release notes according to the new standard. You can refer to the documentation found here: https://xsoar.pan.dev/docs/integrations/changelog for more information.
Packs/ExportIndicators: [RN106] - Release notes were not found. Please run `demisto-sdk update-release-notes -i Packs/ApiModules -u (major|minor|revision|documentation)` to generate release notes according to the new standard. You can refer to the documentation found here: https://xsoar.pan.dev/docs/integrations/changelog for more information.

new validate job:
https://gitlab.xdr.pan.local/xdr/cortex-content/content/-/jobs/10466680
old validate job: https://gitlab.xdr.pan.local/xdr/cortex-content/content/-/jobs/10429679

for some reason, old validate raised RN106 for SplunkPy (instead of RN107).

Copy link

github-actions bot commented Nov 4, 2024

Changelog(s) in markdown:

  • Moved RN106 to the new validate format. The validation ensures there are no missing release notes. #4643

Copy link

Changelog(s) in markdown:

  • Moved RN106 and RN107 to the new validate format. These validations ensure there are no missing release notes. #4643

@dantavori dantavori changed the title adding RN106 adding RN106 and RN107 Nov 12, 2024
Copy link

Changelog(s) in markdown:

  • Moved RN106 and RN107 to the new validate format. These validations ensure there are no missing release notes. #4643
  • Fixed an issue where the git statuses of content objects' related files were not calculated correctly. #4643

@dantavori dantavori marked this pull request as ready for review November 13, 2024 15:21
Comment on lines +979 to +1009
def _check_file_status(
self,
file_path: str,
remote: str,
branch: str,
feature_branch_or_hash: Optional[str] = None,
) -> str:
"""Get the git status of a given file path
Args:
file_path (str): the file path to check
remote (str): the used git remote
branch (str): the used git branch
feature_branch_or_hash (str | None): compare against a specific branch or commit
Returns:
str: the git status of the file (M, A, R, D).
"""
current_branch_or_hash = self.get_current_git_branch_or_hash()
if not feature_branch_or_hash:
feature_branch_or_hash = self.get_current_git_branch_or_hash()

if remote:
diff_line = self.repo.git.diff(
"--name-status",
f"{remote}/{branch}...{current_branch_or_hash}",
f"{remote}/{branch}...{feature_branch_or_hash}",
"--",
file_path,
)

# if remote does not exist we are checking against the commit sha1
else:
diff_line = self.repo.git.diff(
"--name-status", f"{branch}...{current_branch_or_hash}", "--", file_path
"--name-status", f"{branch}...{feature_branch_or_hash}", "--", file_path
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to be able to get related files' git statuses per specific commits (for our use-case: current branch / master), I allowed passing a specific commit

Comment on lines +4362 to +4373
def get_pack_latest_rn_version(pack_path: str, git_sha: Optional[str] = None) -> str:
"""
Extract all the Release notes from the pack and return the highest version of release note in the Pack.

Return:
(str): The lastest version of RN.
"""
list_of_files = glob.glob(pack_path + "/ReleaseNotes/*")
if git_sha:
git_util = GitUtil.from_content_path()
list_of_files = git_util.list_files_in_dir(f"{pack_path}/ReleaseNotes", git_sha)
else:
list_of_files = glob.glob(f"{pack_path}/ReleaseNotes/*")
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 change makes the Pack's latest_rn_version field correct per commit

Comment on lines +595 to +598
git_sha=self.git_sha,
prev_ver=self.old_base_content_object.git_sha
if self.old_base_content_object
else None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

now we can compare these versions to get the actual git status. we should do it for other related files.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you open a ticket for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had two different tests with a pack named ApiModule and the test failed on FileExistsError

Copy link
Contributor

@YuvHayun YuvHayun left a comment

Choose a reason for hiding this comment

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

Amazing job!
Approved with minor comments.

Comment on lines +508 to +509
git_util = GitUtil.from_content_path()
current_git_sha = git_util.get_current_commit_hash()
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I fully understand all the change here, but are we sure it won't affect the way we load changed files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. renaming git_sha arg to prev_ver just to avoid confusion between prev and current versions
  2. passed current_git_sha to the object's git_sha. from what I saw, the git_sha attribute is used only in:
    a) related_files.py (which is what I fixed)
    b) integration and script parsers to get the code - if git_sha not passed, it uses the current working dir (equivalent to current git sha), so allegedly nothing was changed.
  3. passed prev_ver to old object's git_sha.

With 2+3, we can calculate the git_status of related files correctly for each object (see changes in related_files.py and git_utils.py)

demisto_sdk/commands/validate/tools.py Show resolved Hide resolved
@dantavori dantavori merged commit febe16b into master Nov 19, 2024
25 checks passed
@dantavori dantavori deleted the convert_RN106 branch November 19, 2024 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants