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

Secret Scanning Alerts migration - update to new location types #1306

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

theztefan
Copy link

@theztefan theztefan commented Nov 26, 2024

PR implements #1305

  • Did you write/update appropriate tests
  • Release notes updated (if appropriate)
  • Appropriate logging output
  • Issue linked
  • Docs updated (or issue created)
  • New package licenses are added to ThirdPartyNotices.txt (if applicable)

Summary of code changes

  • Updated related models src/Octoshift/Models/GithubSecretScanningAlert.cs
    • GithubSecretScanningAlert to include resolution comment
    • GithubSecretScanningAlertLocation to include types of location and related info
  • Updated src/Octoshift/Services/GithubApi.cs to work with new model definition
    • to populate new fields objects when fetching alerts
    • to include resolution_comment when updating alert
  • Refactored src/Octoshift/Services/SecretScanningAlertService.cs in regards to matching the locations of alerts criteria to include the new location types; and the algorithm for matching alerts between source and target repositories.
    Instead of iterating thru all alerts in a list to match an alert from source and target we now use dictionary for the lookup. We use this lookup to match all locations on all alerts; The criteria for matching alerts follows the similar structure as before but included couple of guards/pre-flight checks to further speed up the matching process. Performance optimization has benefits when migrating repositories with big number of alerts.

Copy link

github-actions bot commented Nov 26, 2024

Unit Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 76fab6f.

♻️ This comment has been updated with latest results.

@theztefan theztefan marked this pull request as ready for review November 27, 2024 14:06
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.

1 participant