Feat: Cleanup and rationalization (#23)

* feat: add log-level argument

* feat: log process args with masking

* chore: disable no-await-in-loop globally

* chore: remove outdated todo list

* chore: handle response logging

* feat: fail fast on any SSL error encountered

* chore: cleanup todos

* feat: move total count to bottom of output

* chore: clean up log messages

.eslintrc

@@ -1,22 +1,28 @@
 {
-  "extends": [
-    "oclif",
-    "oclif-typescript",
-    "prettier"
-  ],
-  "rules": {
-    "no-else-return": "off",
-    "radix": "off",
-    "max-params": [
-      1, 
-      {
-        "max": 8
-      }
+    "extends": [
+        "oclif",
+        "oclif-typescript",
+        "prettier"
     ],
-    "semi": [
-      2,
-      "always"
-    ],
-    "unicorn/numeric-separators-style": ["error", {"onlyIfContainsSeparator": true }]
-  }
-}
+    "rules": {
+        "no-else-return": "off",
+        "radix": "off",
+        "no-await-in-loop": "off",
+        "max-params": [
+            1,
+            {
+                "max": 8
+            }
+        ],
+        "semi": [
+            2,
+            "always"
+        ],
+        "unicorn/numeric-separators-style": [
+            "error",
+            {
+                "onlyIfContainsSeparator": true
+            }
+        ]
+    }
+}

src/commands/azuredevops.ts

@@ -2,6 +2,7 @@ import { Command, Flags } from '@oclif/core';
 import { CLIError } from '@oclif/errors';
 import { vcsFlags } from '../common/flags';
 import { HelpGroup, SourceType, VcsSourceInfo } from '../common/types';
+import { init } from '../common/utils';
 import { AzureApiManager } from '../vcs/azure/azure-api-manager';
 import { AzureRunner } from '../vcs/azure/azure-runner';
 
@@ -49,6 +50,7 @@ export default class AzureDevOps extends Command {
 
     async run(): Promise<void> {
         const { flags } = (await this.parse(AzureDevOps));
+        init(flags);
 
         const sourceInfo = AzureDevOps.getSourceInfo(':' + flags.token, flags['include-public']);
 

src/commands/bitbucket-server.ts

@@ -1,7 +1,7 @@
 import { Flags } from '@oclif/core';
 import { vcsServerFlags } from '../common/flags';
 import { HelpGroup, SourceType, VcsSourceInfo } from '../common/types';
-import { deleteFlagKey, getServerUrl } from '../common/utils';
+import { deleteFlagKey, getServerUrl, init } from '../common/utils';
 import { BitbucketServerApiManager } from '../vcs/bitbucketServer/bitbucket-server-api-manager';
 import { BitbucketServerRunner } from '../vcs/bitbucketServer/bitbucket-server-runner';
 import Bitbucket from './bitbucket';
@@ -12,7 +12,9 @@ export default class BitbucketServer extends Bitbucket {
     static description = `This tool works with Bitbucket server v1 APIs.
     
     About rate limiting: Bitbucket server rate limiting is unique in that you specify a "token bucket size" and "refill rate". To translate this to requests per hour, you must calculate how many requests a client can submit in an hour without being limited. This is basically equal to the refill rate, which is the number of requests per second we can submit indefinitely without being limited. The requests per hour is then the refill rate * 3600.
-    See https://confluence.atlassian.com/bitbucketserver/improving-instance-stability-with-rate-limiting-976171954.html for more information.`
+    See https://confluence.atlassian.com/bitbucketserver/improving-instance-stability-with-rate-limiting-976171954.html for more information.
+    
+    If you run this tool multiple times in quick succession, or if there are other external consumers of this rate limit, you may need to provide a lower value here, because there is no way to check the rate liit status in a stateless way. For Bitbucket Server, you can also control throttling by setting the MAX_REQUESTS_PER_SECOND environment variable. This will cause the tool to submit no more than that many requests per second from the start of execution. This will slow down execution but avoid unexpected rate limit issues.`
 
     static examples = [
         `$ <%= config.bin %> <%= command.id %> --token ATXXX --repos bridgecrewio/checkov,try-bridgecrew/terragoat --hostname github.mycompany.internal`,
@@ -33,6 +35,7 @@ export default class BitbucketServer extends Bitbucket {
     async run(): Promise<void> {
 
         const { flags } = await this.parse(BitbucketServer);
+        init(flags);
 
         const serverUrl = getServerUrl(flags.hostname, flags.port, flags.protocol);
         const baseUrl = `${serverUrl}/rest/api/1.0`;

src/commands/bitbucket.ts

@@ -1,14 +1,14 @@
 import { Command, Flags } from '@oclif/core';
 import { getThrottlingFlag, vcsFlags } from '../common/flags';
 import { HelpGroup, SourceType, VcsSourceInfo } from '../common/types';
+import { init } from '../common/utils';
 import { BitbucketApiManager } from '../vcs/bitbucket/bitbucket-api-manager';
 import { BitbucketRunner } from '../vcs/bitbucket/bitbucket-runner';
 
-// TODO notes about rate limiting
 export default class Bitbucket extends Command {
     static summary = 'Count active contributors for Bitbucket repos'
 
-    static description = `About rate limiting: Bitbucket uses an hourly rate limit that rolls over every minute. Thus, this tool will attempt to submit requests in as much of a burst as possible while respecting the rolling limit.`
+    static description = `About rate limiting: Bitbucket uses an hourly rate limit that rolls over every minute. Thus, this tool will attempt to submit requests in as much of a burst as possible while respecting the rolling limit. If you run this tool multiple times in quick succession, or if there are other external consumers of this rate limit, you may need to provide a lower value here, because there is no way to check the rate liit status in a stateless way. For Bitbucket, you can also control throttling by setting the MAX_REQUESTS_PER_SECOND environment variable. This will cause the tool to submit no more than that many requests per second from the start of execution. This will slow down execution but avoid unexpected rate limit issues.`
 
     static examples = [
         `$ <%= config.bin %> <%= command.id %> --username my_username --token ATBBXXX --repos bridgecrewio/checkov,try-bridgecrew/terragoat`,
@@ -39,6 +39,7 @@ export default class Bitbucket extends Command {
 
     async run(): Promise<void> {
         const { flags } = await this.parse(Bitbucket);
+        init(flags);
 
         const sourceInfo = Bitbucket.getSourceInfo(`${flags.username}:${flags.token}`, flags['include-public']);
 

src/commands/github-server.ts

@@ -1,5 +1,5 @@
 import { SourceType } from '../common/types';
-import { getServerUrl } from '../common/utils';
+import { getServerUrl, init } from '../common/utils';
 import { GithubServerRunner } from '../vcs/githubServer/github-server-runner';
 import { GithubServerApiManager } from '../vcs/githubServer/github-server-api-manager';
 import Github from './github';
@@ -25,6 +25,7 @@ export default class GithubServer extends Github {
     async run(): Promise<void> {
 
         const { flags } = await this.parse(GithubServer);
+        init(flags);
 
         const serverUrl = getServerUrl(flags.hostname, flags.port, flags.protocol);
         const baseUrl = `${serverUrl}/api/v3`;

src/commands/github.ts

@@ -1,6 +1,7 @@
 import { Command, Flags } from '@oclif/core';
 import { vcsFlags } from '../common/flags';
 import { HelpGroup, SourceType, VcsSourceInfo } from '../common/types';
+import { init } from '../common/utils';
 import { GithubApiManager } from '../vcs/github/github-api-manager';
 import { GithubRunner } from '../vcs/github/github-runner';
 
@@ -29,6 +30,7 @@ export default class Github extends Command {
 
     async run(): Promise<void> {
         const { flags } = await this.parse(Github);
+        init(flags);
 
         const sourceInfo = Github.getSourceInfo(flags.token, flags['include-public']);
 

src/commands/gitlab-server.ts

@@ -1,6 +1,6 @@
 import { vcsServerFlags } from '../common/flags';
 import { SourceType } from '../common/types';
-import { getServerUrl } from '../common/utils';
+import { getServerUrl, init } from '../common/utils';
 import { GitlabServerApiManager } from '../vcs/gitlabServer/gitlab-server-api-manager';
 import { GitlabServerRunner } from '../vcs/gitlabServer/gitlab-server-runner';
 import Gitlab from './gitlab';
@@ -23,6 +23,7 @@ export default class GitlabServer extends Gitlab {
     async run(): Promise<void> {
 
         const { flags } = await this.parse(GitlabServer);
+        init(flags);
 
         const serverUrl = getServerUrl(flags.hostname, flags.port, flags.protocol);
         const baseUrl = `${serverUrl}/api/v4`;

src/commands/gitlab.ts

@@ -1,6 +1,7 @@
 import { Command, Flags } from '@oclif/core';
 import { vcsFlags } from '../common/flags';
 import { HelpGroup, SourceType, VcsSourceInfo, } from '../common/types';
+import { init } from '../common/utils';
 import { GitlabApiManager } from '../vcs/gitlab/gitlab-api-manager';
 import { GitlabRunner } from '../vcs/gitlab/gitlab-runner';
 
@@ -31,6 +32,7 @@ export default class Gitlab extends Command {
 
     async run(): Promise<void> {
         const { flags } = await this.parse(Gitlab);
+        init(flags);
 
         const sourceInfo = Gitlab.getSourceInfo(flags.token, flags['include-public']);
 

src/commands/local.ts

@@ -2,7 +2,7 @@ import { Command, Flags } from '@oclif/core';
 import { CLIError } from '@oclif/errors';
 import { commonFlags } from '../common/flags';
 import { HelpGroup, SourceType } from '../common/types';
-import { deleteFlagKey } from '../common/utils';
+import { deleteFlagKey, init } from '../common/utils';
 import { LocalApiManager } from '../vcs/local/local-api-manager';
 import { LocalRunner } from '../vcs/local/local-runner';
 
@@ -47,6 +47,7 @@ export default class Local extends Command {
 
     async run(): Promise<void> {
         const { flags } = (await this.parse(Local));
+        init(flags);
 
         if (!(flags.directories || flags['directory-file'])) {
             throw new CLIError('At least one of --directories or --directory-file is required for running locally.');

src/common/api-manager.ts

@@ -1,10 +1,13 @@
 import { Repo, SourceInfo, VCSCommit } from './types';
 
+export const LOG_API_RESPONSES_ENV = 'LOG_API_RESPONSES';
 export abstract class ApiManager {
     sourceInfo: SourceInfo
+    logApiResponses: boolean
 
     constructor(sourceInfo: SourceInfo) {
         this.sourceInfo = sourceInfo;
+        this.logApiResponses = process.env[LOG_API_RESPONSES_ENV]?.toLowerCase() === 'true';
     }
 
     abstract getCommits(repo: Repo, sinceDate: Date): Promise<VCSCommit[]>

src/common/base-runner.ts

@@ -5,10 +5,8 @@ import { Commit, ContributorMap, Repo, RepoResponse, SourceInfo, VCSCommit, VcsS
 import { DEFAULT_DAYS, getXDaysAgoDate, isSslError, logError, LOGGER } from './utils';
 
 // TODO
-// - all branches? (no?) - remove all branches from the BB call, remote git log --all
 // - document specific permissions needed
 // - document getting a cert chain
-// - some sort of errored repo list that is easy to review
 // - author vs committer - use author always (not committer)
 // - clean up logging
 // - test on windows
@@ -33,7 +31,7 @@ export abstract class BaseRunner {
         this.contributorsByEmail = new Map();
         this.contributorsByRepo = new Map();
         this.flags = flags;
-        this.apiManager = apiManager; 
+        this.apiManager = apiManager;
     }
 
     abstract aggregateCommitContributors(repo: Repo, commits: VCSCommit[]): void
@@ -46,14 +44,14 @@ export abstract class BaseRunner {
         }
 
         const sinceDate = getXDaysAgoDate(this.flags.days);
-
-        // TODO better error handling
+        LOGGER.info(`${this.flags.days} days ago: ${sinceDate.toISOString()}`);
 
         try {
             const repos = await this.getRepoList();
             await this.processRepos(repos, sinceDate);
         } catch (error) {
             if (error instanceof AxiosError && isSslError(error)) {
+                // if we ever encounter an SSL error, we cannot continue. This method expects getRepoList to raise any SSL error it encounters, as soon as it encounters one
                 const sourceInfo = this.sourceInfo as VcsSourceInfo;
                 logError(error, `Received an SSL error while connecting to the server at url ${sourceInfo.url}: ${error.code}: ${error.message}. This is usually caused by a VPN in your environment. Please try using the --ca-cert option to provide a valid certificate chain.`);
             }
@@ -65,17 +63,29 @@ export abstract class BaseRunner {
     }
 
     async processRepos(repos: Repo[], sinceDate: Date): Promise<void> {
+        LOGGER.info(`Getting commits for ${repos.length} ${this.sourceInfo.repoTerm}s`);
+
         for (const repo of repos) {
             try {
-                // eslint-disable-next-line no-await-in-loop
+                LOGGER.debug(`Getting commits for ${this.sourceInfo.repoTerm}s ${repo.owner}/${repo.name}`);
                 const commits: VCSCommit[] = await this.apiManager.getCommits(repo, sinceDate);
+                LOGGER.debug(`Found ${commits.length} commits`);
                 if (commits.length > 0) {
                     this.aggregateCommitContributors(repo, commits);
                 } else if (!this.flags['exclude-empty']) {
                     this.addEmptyRepo(repo);
                 }
             } catch (error) {
-                logError(error as Error, `Failed to get commits for ${this.sourceInfo.repoTerm} ${repo.owner}/${repo.name}`);
+                if (error instanceof AxiosError && isSslError(error)) {
+                    throw error;
+                }
+
+                let message = `Failed to get commits for ${this.sourceInfo.repoTerm} ${repo.owner}/${repo.name}`;
+                if (error instanceof AxiosError) {
+                    message += ` - the API returned an error: ${error.message}`;
+                }
+
+                logError(error as Error, message);
             }
         }
     }
@@ -111,11 +121,11 @@ export abstract class BaseRunner {
         const { username, email, commitDate } = commit;
 
         // handle the 2 maps separately so that we can track commit dates per repo and globally
-        this.upsertContributor(repoContributors, username, email, commitDate);
+        this.upsertContributor(repoContributors, username, email, commitDate, true);
         this.upsertContributor(this.contributorsByEmail, username, email, commitDate);
     }
 
-    upsertContributor(contributorMap: ContributorMap, username: string, email: string, commitDate: string): void {
+    upsertContributor(contributorMap: ContributorMap, username: string, email: string, commitDate: string, logNew = false): void {
         const contributor = contributorMap.get(email);
 
         if (contributor) {
@@ -124,7 +134,10 @@ export abstract class BaseRunner {
                 contributor.lastCommitDate = commitDate;
             }
         } else {
-            LOGGER.debug(`Found new contributor: ${email}, ${username}`);
+            if (logNew) {
+                LOGGER.debug(`Found new contributor: ${email}, ${username}`);
+            }
+
             contributorMap.set(email, {
                 email,
                 usernames: new Set([username]),

src/common/flags.ts

@@ -1,7 +1,8 @@
 import { Flags } from "@oclif/core";
 import { OptionFlag } from "@oclif/core/lib/interfaces";
+import { LOG_API_RESPONSES_ENV } from "./api-manager";
 import { HelpGroup, OutputFormat, Protocol, SortField } from "./types";
-import { DEFAULT_DAYS } from "./utils";
+import { DEFAULT_DAYS, DEFAULT_LOG_LEVEL, DISABLE_LOG_ENV_VAR, LOG_LEVELS } from "./utils";
 
 export const commonFlags = {
     output: Flags.enum({
@@ -36,6 +37,13 @@ export const commonFlags = {
     'include-public': Flags.boolean({
         description: 'The platform only counts contributors in private repos (and "internal" repos for some enterprise systems). If you wish to see contributors in public repos, for informational or other purposes, use this flag. This will also cause Redshirts to skip checking if a repo is public, so it can speed up the runtime if you know you are only supplying private repos, or mostly private repos, using --repos, --orgs, etc.',
         helpGroup: HelpGroup.REPO_SPEC
+    }),
+    'log-level': Flags.enum({
+        description: `Set the log level for the execution. Can also be set with the LOG_LEVEL environment variable. Use 'debug' for granular logging, which will be required for any support cases. You can log individual responses using the environment variable ${LOG_API_RESPONSES_ENV}=true. You can also disable all logging by setting ${DISABLE_LOG_ENV_VAR}=true as an environment variable. This is not recommended, as you may miss important processing messages. All logs will be written to the stderr stream.`,
+        options: LOG_LEVELS,
+        env: 'LOG_LEVEL',
+        default: DEFAULT_LOG_LEVEL,
+        helpGroup: HelpGroup.OUTPUT
     })
 };
 

src/common/output.ts

@@ -14,7 +14,6 @@ export const printSummary = (counter: BaseRunner, outputFormat: string, sortFiel
 
         case OutputFormat.CSV:
             console.log('Repo,Unique contributors');
-            console.log(`Total,${counter.contributorsByEmail.size}`);
 
             const repos = mapIterable(counter.contributorsByRepo.entries(), (value): OutputTableRow => {
                 return {
@@ -29,13 +28,14 @@ export const printSummary = (counter: BaseRunner, outputFormat: string, sortFiel
                 console.log(`${repo.Repo},${repo.Contributors}`);
             }
 
+            console.log(`Total,${counter.contributorsByEmail.size}`);
+
             break;
 
         case OutputFormat.Summary:
             // TODO determine tabular output format (mainly the header with total)
 
             const table = new Table({
-                title: `Total unique contributors: ${counter.contributorsByEmail.size}`,
                 columns: [
                     {
                         name: 'Repo',
@@ -55,6 +55,8 @@ export const printSummary = (counter: BaseRunner, outputFormat: string, sortFiel
 
             table.printTable();
 
+            console.log(`Total unique contributors: ${counter.contributorsByEmail.size}`);
+
             break;
     }
 };