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

Refactor workflows to include report GUID tracking for scan processes… #556

Merged
merged 8 commits into from
Jan 2, 2025

Conversation

kooomix
Copy link
Contributor

@kooomix kooomix commented Jan 2, 2025

User description

… and remove unused delay constant


PR Type

Enhancement


Description

  • Added report GUID tracking across scan processes

  • Removed unused NOTIFICATIONS_SVC_DELAY constant

  • Inherited BaseKubescape functionality in Workflows class


Changes walkthrough 📝

Relevant files
Enhancement
slack_workflows.py
Add report GUID tracking to slack notification workflow   

tests_scripts/workflows/slack_workflows.py

  • Removed unused NOTIFICATIONS_SVC_DELAY constant
  • Added report GUID tracking between scans using get_report_guid()
  • Updated scan workflow to track report GUIDs across initial, first and
    second scans
  • +12/-2   
    teams_workflows.py
    Add report GUID tracking to teams notification workflow   

    tests_scripts/workflows/teams_workflows.py

  • Removed unused NOTIFICATIONS_SVC_DELAY constant
  • Added report GUID tracking between scans using get_report_guid()
  • Updated scan workflow to track report GUIDs across initial, first and
    second scans
  • +11/-2   
    workflows.py
    Inherit BaseKubescape functionality in Workflows class     

    tests_scripts/workflows/workflows.py

    • Added BaseKubescape as parent class to Workflows class
    +3/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Parameter Mismatch

    The parameter name 'report_guid_old' is used in teams_workflows.py while 'old_report_guid' is used in slack_workflows.py. This inconsistency should be unified across files.

    cluster_name=self.cluster, wait_to_result=True, framework_name=self.fw_name, report_guid_old=report_guid_init

    Copy link

    codiumai-pr-agent-free bot commented Jan 2, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add proper error handling for critical operations that track scan processes

    Add error handling for get_report_guid() calls to handle potential failures in
    retrieving report GUIDs, as these are critical for tracking scan processes.

    tests_scripts/workflows/slack_workflows.py [95-97]

    -report_guid_init = self.get_report_guid(
    -    cluster_name=self.cluster, wait_to_result=True, framework_name="AllControls"
    -)
    +try:
    +    report_guid_init = self.get_report_guid(
    +        cluster_name=self.cluster, wait_to_result=True, framework_name="AllControls"
    +    )
    +except Exception as e:
    +    Logger.logger.error(f"Failed to retrieve initial report GUID: {str(e)}")
    +    raise
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding error handling for report GUID retrieval is important since this operation is critical for scan tracking and failure could impact test reliability. The suggestion provides appropriate error logging and re-raising.

    8
    General
    ✅ Standardize parameter naming across similar function calls to maintain code consistency
    Suggestion Impact:Changed parameter name from 'report_guid_old' to 'old_report_guid' in two get_report_guid() calls in teams_workflows.py

    code diff:

    -            cluster_name=self.cluster, wait_to_result=True, framework_name=self.fw_name, report_guid_old=report_guid_init
    +            cluster_name=self.cluster, wait_to_result=True, framework_name=self.fw_name, old_report_guid=report_guid_init
             )
             TestUtil.sleep(NOTIFICATIONS_SVC_DELAY_FIRST_SCAN, "waiting for first scan to be saved in notification service")
             
    @@ -119,7 +121,7 @@
             Logger.logger.info('Stage 11: Trigger second scan')
             self.backend.create_kubescape_job_request(cluster_name=self.cluster, framework_list=[self.fw_name])
             report_guid_second = self.get_report_guid(
    -            cluster_name=self.cluster, wait_to_result=True, framework_name=self.fw_name, report_guid_old=report_guid_first
    +            cluster_name=self.cluster, wait_to_result=True, framework_name=self.fw_name, old_report_guid=report_guid_first
             )

    Fix inconsistent parameter naming in get_report_guid() calls. The parameter is named
    'old_report_guid' in slack_workflows.py but 'report_guid_old' in teams_workflows.py.
    Standardize the parameter name across both files.

    tests_scripts/workflows/teams_workflows.py [101-103]

     report_guid_first = self.get_report_guid(
    -    cluster_name=self.cluster, wait_to_result=True, framework_name=self.fw_name, report_guid_old=report_guid_init
    +    cluster_name=self.cluster, wait_to_result=True, framework_name=self.fw_name, old_report_guid=report_guid_init
     )
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Inconsistent parameter naming between files can lead to confusion and maintenance issues. The suggestion correctly identifies the need to standardize between 'old_report_guid' and 'report_guid_old' parameters.

    7

    Copy link

    github-actions bot commented Jan 2, 2025

    Failed to generate code suggestions for PR

    @kooomix kooomix merged commit dc2be19 into master Jan 2, 2025
    2 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant