feat: Implement --skip-if-exceeding-quota and `--test-exceeding-quo…#929
feat: Implement --skip-if-exceeding-quota and `--test-exceeding-quo…#929ankit2995 wants to merge 2 commits intofortify:feat/v3.x/aviator/26.2from
--skip-if-exceeding-quota and `--test-exceeding-quo…#929Conversation
…ta` flags for audit command - Added new gRPC RPC `GetApplicationByToken` to retrieve application quota. - Introduced `--skip-if-exceeding-quota` to skip audits if open issues exceed quota. - Introduced `--test-exceeding-quota` for dry-run mode to report potential skips without auditing. - Enhanced `AviatorSSCAuditHelper` with methods to get available quota and top unaudited categories. - Updated FCLI client and server-side implementations to support new features. - Modified bulk audit YAML to include new options and track quota-skipped statistics. - Added comprehensive error handling and logging for quota checks. - Updated documentation and messages for new command options.
rsenden
left a comment
There was a problem hiding this comment.
Other than the individual code review comments, I noticed that the Action column in the sample screenshot contains multiple lines with dynamically generated content. In general, we try to have the Action column contain only a single string like AUDITED or QUOTA_EXCEEDED, to allow callers (like bulkaudit.yaml) to easily check the outcome.
Given that this command only processes a single application version, maybe we should change default output type to DetailsNoQuery, and then have separate fields for each detail element, like openIssues, availableQuota, topUnauditedCategories, ..., next to the simple action value?
| @Option(names = {"--folder"}, split = ",") @DisableTest(DisableTest.TestType.MULTI_OPT_PLURAL_NAME) private List<String> folderNames; | ||
| @Option(names = {"--skip-if-exceeding-quota"}) private boolean skipIfExceedingQuota; | ||
| @Option(names = {"--test-exceeding-quota"}) private boolean testExceedingQuota; | ||
| @Option(names = {"--default-quota-fallback"}, hidden = true) private boolean defaultQuotaFallback; |
There was a problem hiding this comment.
We (almost) never use hidden options in fcli, and given that this is explicitly used by bulkaudit.yaml, maybe we should just make this a non-hidden option?
| @Option(names = {"--tag-mapping"}) private String tagMapping; | ||
| @Option(names = {"--no-filterset"}) private boolean noFilterSet; | ||
| @Option(names = {"--folder"}, split = ",") @DisableTest(DisableTest.TestType.MULTI_OPT_PLURAL_NAME) private List<String> folderNames; | ||
| @Option(names = {"--skip-if-exceeding-quota"}) private boolean skipIfExceedingQuota; |
There was a problem hiding this comment.
Quite a long option name; nothing better comes to mind right now but maybe worth giving this another thought, to see whether some creativity can result in shorter option names?
| @Option(names = {"--no-filterset"}) private boolean noFilterSet; | ||
| @Option(names = {"--folder"}, split = ",") @DisableTest(DisableTest.TestType.MULTI_OPT_PLURAL_NAME) private List<String> folderNames; | ||
| @Option(names = {"--skip-if-exceeding-quota"}) private boolean skipIfExceedingQuota; | ||
| @Option(names = {"--test-exceeding-quota"}) private boolean testExceedingQuota; |
There was a problem hiding this comment.
PR description states 'dry run'; does this do anything other than checking quota? Maybe we should just have --dry-run or similar, if that's clear enough and properly describes the intent? Can this option be used on its own, or does it also require --skip-if-exceeding-quota to be specified? Just wondering, would it make sense to have this as a separate command (together with --default-quota-fallback)?
| return AviatorSSCAuditHelper.buildResultNode(av, "N/A", "SKIPPED (no auditable issues)"); | ||
| } | ||
|
|
||
| // Quota check: only when --skip-if-exceeding-quota or --test-exceeding-quota is active |
There was a problem hiding this comment.
Not sure how long the original method was, but for sure the method becomes much longer with this change. Can we please split into multiple methods?
…ta` flags for audit command
GetApplicationByTokento retrieve application quota.--skip-if-exceeding-quotato skip audits if open issues exceed quota.--test-exceeding-quotafor dry-run mode to report potential skips without auditing.AviatorSSCAuditHelperwith methods to get available quota and top unaudited categories.