Skip to content

feat: Implement --skip-if-exceeding-quota and `--test-exceeding-quo…#929

Open
ankit2995 wants to merge 2 commits intofortify:feat/v3.x/aviator/26.2from
ankit2995:ankit/aviator_quota_overflow
Open

feat: Implement --skip-if-exceeding-quota and `--test-exceeding-quo…#929
ankit2995 wants to merge 2 commits intofortify:feat/v3.x/aviator/26.2from
ankit2995:ankit/aviator_quota_overflow

Conversation

@ankit2995
Copy link
Contributor

@ankit2995 ankit2995 commented Feb 26, 2026

…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.
Screenshot 2026-02-27 131148

…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.
@ankit2995 ankit2995 marked this pull request as ready for review February 27, 2026 07:06
Copy link
Contributor

@rsenden rsenden left a comment

Choose a reason for hiding this comment

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

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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
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 how long the original method was, but for sure the method becomes much longer with this change. Can we please split into multiple methods?

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.

3 participants