diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8eec5d0f07..ce5ca911ae 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -72,7 +72,7 @@ Once you've set up your GitHub app and cloned this repo, copy the content from ` + `PRIVATE_KEY_PATH`: You'll also need to generate a new private key on your GitHub app page, download it, move it to the source root of this repo, and set `PRIVATE_KEY_PATH=.pem` + `ATLASSIAN_URL`: The URL for the Jira instance you're testing it. If you don't have one now, please set the value of this variable after going through the step 1 of "Configuring the Jira instance" section of this document. + `STORAGE_SECRET`: It needs to be set to a 32 char secret (anything else fails). You can generate one by running `openssl rand -hex 32` in your terminal and paste directly to your .env file. -+ `INSTANCE_NAME`: Your Jira app name - will show as "Github (instance-name)" ++ `INSTANCE_NAME`: Your Jira app name - will show as "GitHub (instance-name)" + `WEBHOOK_PROXY_URL`: `https://DOMAIN/github/events` ### Running the app @@ -102,7 +102,7 @@ Go to your Jira instance that you created earlier and do the following steps: ### Setting up the App -In your Jira instance, in the `Manage Apps` section, click on your App's button, then click on `Get Started`. This will bring you to the App's dashboard. Click the `Add an Organization` button and follow the steps to install the App on Github and allow it permission to view your repos. +In your Jira instance, in the `Manage Apps` section, click on your App's button, then click on `Get Started`. This will bring you to the App's dashboard. Click the `Add an Organization` button and follow the steps to install the App on GitHub and allow it permission to view your repos. After this is done, you should see your repos starting to sync in the App's dashboard. @@ -130,4 +130,4 @@ That being said, here are the steps needed to create a Pull Request for us to re 1. Commit and Push your changes - verify it passes all checks. 1. Submit your pull request with a detailed message about what's changed. 1. Wait for us to review and answer questions/make changes where requested. -1. Once merged, celebrate with your drink of choice because you've just helped thousands (if not millions) of people get a better experience in both Jira and Github! :beers: +1. Once merged, celebrate with your drink of choice because you've just helped thousands (if not millions) of people get a better experience in both Jira and GitHub! :beers: diff --git a/README.md b/README.md index c9223b25b3..40776503ad 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,8 @@ For general support inquiries, [please contact the Atlassian Support team](https - [See GitHub CI/CD data in Jira](#see-github-cicd-data-in-jira) - [How the integration works](#how-the-integration-works) - [Migrate from the DVCS connector](#migrate-from-the-dvcs-connector) +- [Enterprise Features](#enterprise-features) + - [IP Allow List](#ip-allow-list) - [Need help?](#need-help) - [Contribute](#contribute) - [License](#license) @@ -134,7 +136,7 @@ If an issue body contains a valid Jira issue key on your instance, the integrati This makes it so Jira issues can be linked inside a comment without it interrupting the flow of the comment as a whole. ### See GitHub CI/CD data in Jira -GitHub Actions is a feature from GitHub for automation such as CI/CD. If you’re setting this up for the first time, follow [GitHub Actions Documentation - GitHub Docs](https://docs.github.com/en/actions). If you already have GitHub Actions and want to see CI/CD data from Github in Jira, include the Jira issue key in your commit message, branch name, or PR. +GitHub Actions is a feature from GitHub for automation such as CI/CD. If you’re setting this up for the first time, follow [GitHub Actions Documentation - GitHub Docs](https://docs.github.com/en/actions). If you already have GitHub Actions and want to see CI/CD data from GitHub in Jira, include the Jira issue key in your commit message, branch name, or PR. ### How the integration works When a workflow (e.g. GitHub Action) or development event (e.g. pull request, commit, branch) runs, the app receives a webhook from GitHub. The app then extract the issue key from the respective branch/commit/PR and send this information to Jira. @@ -146,6 +148,13 @@ Existing users of Jira's built-in DVCS connector that meet the [requirements](#r 2. From the left sidebar in Jira, select **Jira Settings > Applications > DVCS accounts**. 3. Follow the prompt to upgrade your GitHub connection. +## Enterprise Features + +### IP Allow List + +GitHub has the ability to limit who can communicate with your organization's GitHub API which we now fully support. +To enable this feature or to debug any issues, please refer to our [GitHub IP Allow List documentation](./docs/ip-allowlist.md). + ## Need help? Take a look through the troubleshooting steps in our [support guide](./SUPPORT.md). diff --git a/db/config.json b/db/config.json index e93c5f48a1..a23a83a651 100644 --- a/db/config.json +++ b/db/config.json @@ -1,7 +1,12 @@ { "development": { "dialect": "postgres", - "use_env_variable": "DATABASE_URL" + "use_env_variable": "DATABASE_URL", + "pool": { + "max": 15, + "min": 0, + "idle": 1000 + } }, "test": { "dialect": "postgres", diff --git a/docs/builds.md b/docs/builds.md new file mode 100644 index 0000000000..71b3a1ce13 --- /dev/null +++ b/docs/builds.md @@ -0,0 +1,76 @@ +# GitHub Actions - Builds + +GitHub for Jira supports builds via [GitHub Actions workflow syntax](https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions). +To set this up, you will need to create a .github folder at the root of a given repository and then make a child directory +called workflows. Inside of .github/workflows you will need to create a build.yml file. This is where you will specify the workflow for your builds. + +Following is an example of a build.yml file: + +``` +# This is a basic workflow to help you get started with Actions + +name: CI + +# Controls when the action will run. +on: + # Triggers the workflow on push or pull request events but only for the main branch + push: + branches: + - main + pull_request: + branches: + - main + - feature/** + + # Allows you to run this workflow manually from the Actions tab + workflow_dispatch: + +# A workflow run is made up of one or more jobs that can run sequentially or in parallel +jobs: + # This workflow contains a single job called "build" + build: + # The type of runner that the job will run on + runs-on: ubuntu-latest + + # Steps represent a sequence of tasks that will be executed as part of the job + steps: + # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it + - uses: actions/checkout@v2 + + # Runs a single command using the runners shell + - name: Run a one-line script + run: echo Hello, world! + + # Runs a set of commands using the runners shell + - name: Run a multi-line script + run: | + echo Add other actions to build, + echo test, and deploy your project. + sleep 60s +``` + +Once you have a similar file in any of your repositories that are connected to your Jira via the app, you will start to see builds data +in the development panel in Jira. + +![Builds data in Jira](./images/builds-data-jira-dev-panel.png) + +#### Supporting Multiple Commits with Issue Keys + +One important thing to note in the above example is the branches being targeted under `pull_requests` `branches`: + +``` +pull_request: + branches: + - main + - feature/** +``` + +When you open a pull request in a connected repository, the GitHub for Jira app can compare two points in your git history is we have access to a base branch and a head branch. If the app has both of these, it can make a request to GitHub to compare your +commits. On the flipside, if you only listed `main`, for instance, there would be no way for the app to ask GitHub for a comparison. +Instead, the best it can do is use the data GitHub sends in the response when the `workflow_run` event is triggered, +which only includes the most recent commit. This means that if a developer were to make multiple commits, perhaps on multiple branches, and +reference various Jira issue keys in each commit, GitHub would only send the data to Jira about the latest commit. In turn, this +would mean that we could only extract any issue keys from that single message. Although there may be numerous Jira issues +involved, in this scenario, you would only see builds data for any issue keys from the latest commit message. + +When you list branches under `pull_request` you'll need to be very specific about the branches you want the app to target. If any branch is created that isn't listed here, the app won't be able to compare your commits and send the most accurate data to Jira. diff --git a/docs/images/builds-data-jira-dev-panel.png b/docs/images/builds-data-jira-dev-panel.png new file mode 100644 index 0000000000..be7057cbf4 Binary files /dev/null and b/docs/images/builds-data-jira-dev-panel.png differ diff --git a/docs/images/github-ip-allowlist.png b/docs/images/github-ip-allowlist.png new file mode 100644 index 0000000000..8a780857a3 Binary files /dev/null and b/docs/images/github-ip-allowlist.png differ diff --git a/docs/ip-allowlist.md b/docs/ip-allowlist.md new file mode 100644 index 0000000000..c616eaca24 --- /dev/null +++ b/docs/ip-allowlist.md @@ -0,0 +1,65 @@ +# GitHub IP Allow List Configuration + +If your organization is using [GitHub's Organization IP Allow List](https://docs.github. +com/en/organizations/keeping-your-organization-secure/managing-allowed-ip-addresses-for-your-organization), it needs to +be configured properly for this GitHub app to be able to communicate with your organization's GitHub API. + +There are 2 methods to remedy this based on your company's security policy: + +1. a simple method which will enable _all_ GitHub apps with IPs specified in them to have access to your GitHub org's + APIs (_recommended_) +2. a manual way by adding each CIDR ranges possible for this app to communicate from. + +**We recommend using the first method** as you only have to set it once and never have to think about it again. If there +are any IP changes on our end in the future, we can just change the list on our end and it will automatically propagate +to your organization. But for this to happen, you must trust every GitHub app installed or else risk a potential +security breach by an app adding an attacker's IP to your allow list. + +If you'd like to have complete control over your IP Allow List, then you can enter the CIDR ranges manually in your +GitHub organization. But it does come with the drawback that if the CIDR ranges ever change or a new one needs to be +added, you will have to manually update those as well. Furthermore, we don't have a way to easily send a message to all +GitHub org admins about a change like this and it could be possible that the integration might break because of the +change. + +### Simple Method + +As an admin go to your GitHub org page `https://github.com/`, press on the `Settings` tab, then in the sidebar +select the `Organization security` option. Scroll down to the `IP allow list` section. Both +checkboxes `Enable IP allow list` and `Enable IP allow list configuration for installed GitHub Apps` should be selected +and saved independently. + +![](images/github-ip-allowlist.png) + +That's it! + +### Manual Method + +As an admin your GitHub org page `https://github.com/`, press on the `Settings` tab, then in the sidebar +select the `Organization security` option. Scroll down to the `IP allow list` section until you can see the list of IP +addresses with a `+ Add` button. From here, you need +to [add the whole list of CIDR ranges specified in this Atlassian document](https://support.atlassian.com/organization-administration/docs/ip-addresses-and-domains-for-atlassian-cloud +-products/#AtlassiancloudIPrangesanddomains-OutgoingConnections). + +For simplicity, here's the list of CIDR ranges, but it might not be up to date: + +``` +13.52.5.96/28 +13.236.8.224/28 +18.136.214.96/28 +18.184.99.224/28 +18.234.32.224/28 +18.246.31.224/28 +52.215.192.224/28 +104.192.137.240/28 +104.192.138.240/28 +104.192.140.240/28 +104.192.142.240/28 +104.192.143.240/28 +185.166.143.240/28 +185.166.142.240/28 +``` + +## If problems persist + +Feel free to [contact Atlassian support](https://support.atlassian.com/contact/#/? +inquiry_category=technical_issues&is_cloud=true&product_key=third-party-product) for guidance and extra help. \ No newline at end of file diff --git a/docs/legacy-features.md b/docs/legacy-features.md index 71a602ad84..8900448b48 100644 --- a/docs/legacy-features.md +++ b/docs/legacy-features.md @@ -6,17 +6,17 @@ See - (Atlassian docs)[https://confluence.atlassian.com/adminjiracloud/connect-jira-cloud-to-github-814188429.html] -- (Github docs)[https://help.github.com/articles/integrating-jira-with-your-organization-s-projects/] +- (GitHub docs)[https://help.github.com/articles/integrating-jira-with-your-organization-s-projects/] ## Installation From within Jira Software, customers would need to 1. go to the DVCS Accounts page within settings -2. set up the Github connection via oAuth client ID and Code Secret +2. set up the GitHub connection via oAuth client ID and Code Secret 3. Sync all repos that were added. -Note: The same method is used for Github & Github Enterprise, but an additional Host URL is required on installation. When using Atlassian Connect to host the Github application, it will need to deal with Github + Github EE itself. +Note: The same method is used for GitHub & GitHub Enterprise, but an additional Host URL is required on installation. When using Atlassian Connect to host the Github application, it will need to deal with Github + Github EE itself. ![](https://user-images.githubusercontent.com/173/32561336-abd01cb6-c471-11e7-8719-13e165cd3dcd.png) diff --git a/package-lock.json b/package-lock.json index 73fcfed745..47aee89f47 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3119,6 +3119,15 @@ "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.4.1.tgz", "integrity": "sha512-ZwrFkGJxUR3EIoXtO+yVE69Eb7KlixbaeAWfBQB9vVsNn/o+Yw69gBWSSDK825hQNdN+wF8zELf3dFNl/kxkUA==" }, + "cookie-parser": { + "version": "1.4.6", + "resolved": "https://registry.npmjs.org/cookie-parser/-/cookie-parser-1.4.6.tgz", + "integrity": "sha512-z3IzaNjdwUC2olLIB5/ITd0/setiaFMLYiZJle7xg5Fe9KWAceil7xszYfHHBtDFYLSgJduS2Ty0P1uJdPDJeA==", + "requires": { + "cookie": "0.4.1", + "cookie-signature": "1.0.6" + } + }, "cookie-session": { "version": "2.0.0-rc.1", "resolved": "https://registry.npmjs.org/cookie-session/-/cookie-session-2.0.0-rc.1.tgz", diff --git a/package.json b/package.json index 1dbf3cc7a1..0bcff67176 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@atlassian/github-for-jira", "version": "0.0.0", - "description": "Github integration for Atlassian Jira", + "description": "GitHub integration for Atlassian Jira", "repository": "https://github.com/atlassian/github-for-jira.git", "engines": { "node": ">= 14.16 <15", @@ -73,6 +73,7 @@ "bull": "^3.12.1", "bunyan": "^1.8.15", "bunyan-format": "^0.2.1", + "cookie-parser": "^1.4.6", "cookie-session": "^2.0.0-rc.1", "csurf": "^1.11.0", "date-fns": "^1.29.0", diff --git a/src/backfill-queue-supplier.ts b/src/backfill-queue-supplier.ts new file mode 100644 index 0000000000..883e74ea02 --- /dev/null +++ b/src/backfill-queue-supplier.ts @@ -0,0 +1,50 @@ +import Queue from "bull"; +import {SqsQueue} from "./sqs"; +import {BackfillMessagePayload} from "./sqs/backfill"; +import {BackfillQueue} from "./sync/installation"; +import {LoggerWithTarget} from "probot/lib/wrap-logger"; +import {getLogger} from "./config/logger"; +import {booleanFlag, BooleanFlags} from "./config/feature-flags"; + +const fallbackLogger = getLogger("queue-supplier-default"); + +/** + * A temp class to support switching between Redis and SQS. Will be gone with the feature flag and the + * Redis queue. + */ +class BackfillQueueSupplier { + private redisQueue: Queue.Queue; + private sqsQueue: SqsQueue; + + setRedisQueue(redisQueue: Queue.Queue) { + this.redisQueue = redisQueue; + } + + setSQSQueue(sqsQueue: SqsQueue) { + this.sqsQueue = sqsQueue; + } + + async supply(): Promise { + if (!this.redisQueue) { + return Promise.reject(new Error("Redis queue wasn't provided")); + } + if (!this.sqsQueue) { + return Promise.reject(new Error("SQS queue wasn't provided")); + } + return { + schedule: async (payload, delayMsec?: number, log?: LoggerWithTarget) => { + if (await booleanFlag(BooleanFlags.USE_SQS_FOR_BACKFILL, false, payload.jiraHost)) { + await this.sqsQueue.sendMessage(payload, (delayMsec || 0) / 1000, (log || fallbackLogger)); + } else { + if (delayMsec) { + await this.redisQueue.add(payload, {delay: delayMsec}); + } else { + await this.redisQueue.add(payload); + } + } + } + }; + } +} + +export default new BackfillQueueSupplier(); diff --git a/src/config/errors.ts b/src/config/errors.ts index 34e46df095..8989090d3d 100644 --- a/src/config/errors.ts +++ b/src/config/errors.ts @@ -1,5 +1,6 @@ export enum Errors { MISSING_JIRA_HOST = "Jira Host url is missing", MISSING_GITHUB_TOKEN = "Github Auth token is missing", + IP_ALLOWLIST_MISCONFIGURED = "IP Allowlist Misconfigured", } diff --git a/src/config/feature-flags.ts b/src/config/feature-flags.ts index d641d8cb2b..6267a5199b 100644 --- a/src/config/feature-flags.ts +++ b/src/config/feature-flags.ts @@ -3,6 +3,7 @@ import LaunchDarkly, { LDUser } from "launchdarkly-node-server-sdk"; import { getLogger } from "./logger"; import envVars from "./env"; import crypto from "crypto"; +import {LoggerWithTarget} from "probot/lib/wrap-logger"; const logger = getLogger("feature-flags"); @@ -16,21 +17,22 @@ export enum BooleanFlags { EXPOSE_QUEUE_METRICS = "expose-queue-metrics", PROCESS_PUSHES_IMMEDIATELY = "process-pushes-immediately", SIMPLER_PROCESSOR = "simpler-processor", - NEW_GITHUB_CONFIG_PAGE = "new-github-config-page", - NEW_CONNECT_AN_ORG_PAGE = "new-connect-an-org-page", NEW_GITHUB_ERROR_PAGE = "new-git-hub-error-page", - NEW_SETUP_PAGE = "new-setup-page", NEW_BACKFILL_PROCESS_ENABLED = "new-backfill-process-enabled", - USE_DEDUPLICATOR_FOR_BACKFILLING = "use-deduplicator-for-backfilling", // When cleaning up the SEND_PUSH_TO_SQS feature flag, please also clean up the PRIORITIZE_PUSHES // feature flag, because it doesn't make sense with SQS any more. SEND_PUSH_TO_SQS = "send-push-events-to-sqs", PRIORITIZE_PUSHES = "prioritize-pushes", USE_NEW_GITHUB_CLIENT__FOR_PR = "git-hub-client-for-pullrequests", NEW_REPO_SYNC_STATE = "new-repo-sync-state", - PAYLOAD_SIZE_METRIC = "payload-size-metrics", - USE_BACKFILL_QUEUE_SUPPLIER = "use-backfill-queue-supplier", + SUPPORT_BRANCH_AND_MERGE_WORKFLOWS_FOR_DEPLOYMENTS = "support-branch-and-merge-workflows-for-deployments", + TRACE_LOGGING = "trace-logging", + USE_SQS_FOR_BACKFILL = "use-sqs-for-backfill", + SUPPORT_BRANCH_AND_MERGE_WORKFLOWS_FOR_BUILDS = "support-branch-and-merge-workflows-for-builds", + USE_NEW_GITHUB_CLIENT_FOR_PUSH = "use-new-github-client-for-push", + USE_NEW_GITHUB_CLIENT_TO_COUNT_REPOS = "use-new-github-client-to-count-repos", REPO_SYNC_STATE_AS_SOURCE = "repo-sync-state-as-source" + } export enum StringFlags { @@ -69,3 +71,14 @@ export const booleanFlag = async (flag: BooleanFlags, defaultValue: boolean, jir export const stringFlag = async (flag: StringFlags, defaultValue: string, jiraHost?: string): Promise => String(await getLaunchDarklyValue(flag, defaultValue, jiraHost)); + +export const isBlocked = async (installationId: number, logger: LoggerWithTarget): Promise => { + try { + const blockedInstallationsString = await stringFlag(StringFlags.BLOCKED_INSTALLATIONS, "[]"); + const blockedInstallations: number[] = JSON.parse(blockedInstallationsString); + return blockedInstallations.includes(installationId); + } catch (e) { + logger.error({ err: e, installationId }, "Cannot define if isBlocked") + return false; + } +}; diff --git a/src/config/interfaces.ts b/src/config/interfaces.ts index 188412e0c4..82720b2145 100644 --- a/src/config/interfaces.ts +++ b/src/config/interfaces.ts @@ -36,3 +36,188 @@ export interface FailedInstallations { id: number; deleted: boolean; } + +interface WorkflowRunAuthorAndCommitter { + name: string; + email: string; +} + +interface WorkflowRunHeadCommit { + id: string; + tree_id: string; + message: string; + timestamp: Date; + author: WorkflowRunAuthorAndCommitter; + committer: WorkflowRunAuthorAndCommitter; +} + +interface WorkflowRunRepositoryOwner { + login: string; + id: number; + node_id: string; + avatar_url: string; + gravatar_id: string; + url: string; + html_url: string; + followers_url: string; + following_url: string; + gists_url: string; + starred_url: string; + subscriptions_url: string; + organizations_url: string; + repos_url: string; + events_url: string; + received_events_url: string; + type: string; + site_admin: boolean; +} + +interface WorkflowRunRepository { + id: number; + node_id: string; + name: string; + full_name: string; + private: boolean; + owner: WorkflowRunRepositoryOwner; + html_url: string; + description: string | null; + fork: boolean; + url: string; + forks_url: string; + keys_url: string; + collaborators_url: string; + teams_url: string; + hooks_url: string; + issue_events_url: string; + events_url: string; + assignees_url: string; + branches_url: string; + tags_url: string; + blobs_url: string; + git_tags_url: string; + git_refs_url: string; + trees_url: string; + statuses_url: string; + languages_url: string; + stargazers_url: string; + contributors_url: string; + subscribers_url: string; + subscription_url: string; + commits_url: string; + git_commits_url: string; + comments_url: string; + issue_comment_url: string; + contents_url: string; + compare_url: string; + merges_url: string; + archive_url: string; + downloads_url: string; + issues_url: string; + pulls_url: string; + milestones_url: string; + notifications_url: string; + labels_url: string; + releases_url: string; + deployments_url: string; +} + +interface WorkflowRunPullRequestsHeadAndBaseRepo { + id: number; + url: string; + name: string; +} + +interface WorkflowRunPullRequestsHeadAndBase { + ref: string; + sha: string; + repo: WorkflowRunPullRequestsHeadAndBaseRepo; +} + + +interface WorkflowRunPullRequests { + url: string; + id: number; + number: number; + head: WorkflowRunPullRequestsHeadAndBase; + base: WorkflowRunPullRequestsHeadAndBase; +} + +interface WorkflowRun { + id: number; + name: string; + node_id: string; + head_branch: string; + head_sha: string; + run_number: number; + event: string; + status: string; + conclusion: string | undefined; + workflow_id: number; + check_suite_id: number; + check_suite_node_id: string; + url: string; + html_url: string; + pull_requests: WorkflowRunPullRequests[]; + created_at: string; + updated_at: string; + run_attempt: number; + run_started_at: string; + jobs_url: string; + logs_url: string; + check_suite_url: string; + artifacts_url: string; + cancel_url: string; + rerun_url: string; + previous_attempt_url: string | null; + workflow_url: string; + head_commit: WorkflowRunHeadCommit; + repository: WorkflowRunRepository; + head_repository: WorkflowRunRepository; +} + +interface Workflow { + id: string; + node_id: string; + name: string; + path: string; + state: string; + created_at: string; + updated_at: string; + url: string; + html_url: string; + badge_url: string; +} +interface WorkflowSender { + login: string; + id: number; + node_id: string; + avatar_url: string; + gravatar_id: string; + url: string; + html_url: string; + followers_url: string; + following_url: string; + gists_url: string; + starred_url: string; + subscriptions_url: string; + organizations_url: string; + repos_url: string; + events_url: string; + received_events_url: string; + type: string; + site_admin: boolean; +} + +interface WorkflowInstallation { + id: number; + node_id: string; +} + +export interface WorkflowPayload { + action: string; + workflow_run: WorkflowRun; + workflow: Workflow; + repository: WorkflowRunRepository; + sender: WorkflowSender; + installation: WorkflowInstallation; +} diff --git a/src/config/statsd.ts b/src/config/statsd.ts index 2092c3f16f..377f0bfcb1 100644 --- a/src/config/statsd.ts +++ b/src/config/statsd.ts @@ -2,7 +2,7 @@ import { StatsCb, StatsD, Tags } from "hot-shots"; import { getLogger } from "./logger"; import { NextFunction, Request, Response } from "express"; import { isNodeDev, isNodeTest } from "../util/isNodeEnv"; -import {metricHttpRequest} from "./metric-names"; +import { metricHttpRequest } from "./metric-names"; export const globalTags = { environment: isNodeTest() ? "test" : process.env.MICROS_ENV || "", @@ -11,7 +11,7 @@ export const globalTags = { region: process.env.MICROS_AWS_REGION || "us-west-1" }; -const RESPONSE_TIME_HISTOGRAM_BUCKETS = "100_1000_2000_3000_5000_10000_30000_60000"; +const RESPONSE_TIME_HISTOGRAM_BUCKETS = "100_1000_2000_3000_5000_10000_30000_60000"; const logger = getLogger("config.statsd"); const statsd = new StatsD({ @@ -55,27 +55,33 @@ export const elapsedTimeMetrics = ( ) => { const elapsedTimeInMs = hrtimer(); const method = req.method; + (req.log || logger).debug({ + method: req.method, + path: req.path, + body: JSON.stringify(req.body), + query: req.query + }, `${method} request initialized for path ${req.path}`); res.once("finish", () => { - - const pathTag = req.baseUrl ? req.baseUrl + (req.route?.path || "*") : (req.route?.path || "/*"); const elapsedTime = elapsedTimeInMs(); - const statusCode = `${res.statusCode}`; - const tags = { path: pathTag, method, statusCode}; - (req.log || logger).debug(`Request executed in ${elapsedTime} with status ${statusCode} path ${pathTag}`) + const statusCode = `${res.statusCode}`; + const path = (req.baseUrl || "") + (req.route?.path || "/*"); + const tags = { path, method, statusCode }; + (req.log || logger).debug(`${method} request executed in ${elapsedTime} with status ${statusCode} path ${path}`); //Count response time metric statsd.histogram(metricHttpRequest.duration, elapsedTime, tags); //Publish bucketed histogram metric for the call duration statsd.histogram(metricHttpRequest.duration, elapsedTime, - {...tags, + { + ...tags, gsd_histogram: RESPONSE_TIME_HISTOGRAM_BUCKETS } ); //Count requests count - statsd.increment(metricHttpRequest.executed, tags) + statsd.increment(metricHttpRequest.executed, tags); }); next(); diff --git a/src/config/tracer.ts b/src/config/tracer.ts new file mode 100644 index 0000000000..9314b5e502 --- /dev/null +++ b/src/config/tracer.ts @@ -0,0 +1,24 @@ +import Logger from "bunyan"; + +export class Tracer { + private readonly startTimestamp: number; + private readonly logsEnabled: boolean; + private readonly logger: Logger; + + constructor(log: Logger, name: string, logsEnabled: boolean) { + this.logger = log.child({ tracer: name }); + this.startTimestamp = new Date().getTime(); + this.logsEnabled = logsEnabled; + } + + trace(message: string) { + if (this.logsEnabled) { + this.logger.info(`time: ${this.getElapsedMs()}ms - ${message}`); + } + } + + private getElapsedMs() { + const now = new Date().getTime(); + return now - this.startTimestamp; + } +} diff --git a/src/frontend/app.ts b/src/frontend/app.ts index a94f92ba01..53bc528c34 100644 --- a/src/frontend/app.ts +++ b/src/frontend/app.ts @@ -14,7 +14,7 @@ import deleteGitHubSubscription from "./delete-github-subscription"; import getJiraConfiguration from "./get-jira-configuration"; import deleteJiraConfiguration from "./delete-jira-configuration"; import getGithubClientMiddleware from "./github-client-middleware"; -import getJiraConnect from "../jira/connect"; +import getJiraConnect, { postInstallUrl } from "../jira/connect"; import postJiraInstall from "../jira/install"; import postJiraUninstall from "../jira/uninstall"; import extractInstallationFromJiraCallback from "../jira/extract-installation-from-jira-callback"; @@ -34,6 +34,7 @@ import { isNodeProd, isNodeTest } from "../util/isNodeEnv"; import { registerHandlebarsPartials } from "../util/handlebars/partials"; import { registerHandlebarsHelpers } from "../util/handlebars/helpers"; import { Errors } from "../config/errors"; +import cookieParser from "cookie-parser"; // Adding session information to request declare global { @@ -45,7 +46,6 @@ declare global { session: { jiraHost?: string; githubToken?: string; - [key: string]: unknown; }; } } @@ -72,13 +72,6 @@ const csrfProtection = csrf( : undefined ); -const saveSessionVariables = (req: Request, _: Response, next: NextFunction) => { - req.log.info("Setting session variables 'jiraHost'"); - // set jirahost after token if no errors - req.session.jiraHost = req.query.xdm_e as string; - next(); -}; - export default (octokitApp: App): Express => { const githubClientMiddleware = getGithubClientMiddleware(octokitApp); @@ -91,6 +84,7 @@ export default (octokitApp: App): Express => { // Parse URL-encoded bodies for Jira configuration requests app.use(bodyParser.urlencoded({ extended: false })); app.use(bodyParser.json()); + app.use(cookieParser()); // We run behind ngrok.io so we need to trust the proxy always // TODO: look into the security of this. Maybe should only be done for local dev? @@ -102,7 +96,8 @@ export default (octokitApp: App): Express => { maxAge: 30 * 24 * 60 * 60 * 1000, // 30 days signed: true, sameSite: "none", - secure: true + secure: true, + httpOnly: false }) ); @@ -140,6 +135,42 @@ export default (octokitApp: App): Express => { ) ); + app.get(["/session", "/session/*"], (req: Request, res: Response, next: NextFunction) => { + if (!req.params[0]) { + return next(new Error("Missing redirect url for session. Needs to be in format `/session/:redirectUrl`")); + } + + return res.render("session.hbs", { + title: "Logging you into GitHub", + APP_URL: process.env.APP_URL, + redirectUrl: new URL(req.params[0], process.env.APP_URL).href, + nonce: res.locals.nonce + }); + }); + + // Saves the jiraHost cookie to the secure session if available + app.use((req: Request, res: Response, next: NextFunction) => { + if (req.cookies.jiraHost) { + // Save jirahost to secure session + req.session.jiraHost = req.cookies.jiraHost; + // delete jirahost from cookies. + res.clearCookie("jiraHost"); + } + + if (req.path == postInstallUrl && req.method == "GET") { + // Only save xdm_e query when on the GET post install url (iframe url) + res.locals.jiraHost = req.query.xdm_e as string; + } else if ((req.path == postInstallUrl && req.method != "GET") || req.path == "/jira/sync") { + // Only save the jiraHost from the body for specific routes that use it + res.locals.jiraHost = req.body?.jiraHost; + } else { + // Save jiraHost from session for any other URLs + res.locals.jiraHost = req.session.jiraHost; + } + + next(); + }); + app.use(githubClientMiddleware); app.use("/", healthcheck); @@ -153,8 +184,8 @@ export default (octokitApp: App): Express => { app.get("/jira/atlassian-connect.json", getJiraConnect); // Maintenance mode view - app.use(async (req, res, next) => { - if (await booleanFlag(BooleanFlags.MAINTENANCE_MODE, false, req.session.jiraHost)) { + app.use(async (req: Request, res: Response, next: NextFunction) => { + if (await booleanFlag(BooleanFlags.MAINTENANCE_MODE, false, res.locals.jiraHost)) { return getMaintenance(req, res); } next(); @@ -206,7 +237,6 @@ export default (octokitApp: App): Express => { "/jira/configuration", csrfProtection, verifyJiraJwtTokenMiddleware, - saveSessionVariables, getJiraConfiguration ); @@ -220,7 +250,7 @@ export default (octokitApp: App): Express => { // Set up event handlers // TODO: remove enabled and disabled events once the descriptor is updated in marketplace - app.post("/jira/events/disabled",(_: Request, res: Response) => { + app.post("/jira/events/disabled", (_: Request, res: Response) => { return res.sendStatus(204); }); app.post("/jira/events/enabled", (_: Request, res: Response) => { @@ -235,9 +265,9 @@ export default (octokitApp: App): Express => { }); // Add Sentry Context - app.use((err: Error, req: Request, _: Response, next: NextFunction) => { + app.use((err: Error, req: Request, res: Response, next: NextFunction) => { Sentry.withScope((scope: Sentry.Scope): void => { - const jiraHost = req.session.jiraHost; + const jiraHost = res.locals.jiraHost; if (jiraHost) { scope.setTag("jiraHost", jiraHost); } @@ -249,6 +279,17 @@ export default (octokitApp: App): Express => { next(err); }); }); + + // Error endpoints to test out different error pages + app.get(["/error", "/error/:message", "/error/:message/:name"], (req: Request, res: Response, next: NextFunction) => { + res.locals.showError = true; + const error = new Error(req.params.message); + if(req.params.name) { + error.name = req.params.name + } + next(error); + }); + // The error handler must come after controllers and before other error middleware app.use(Sentry.Handlers.errorHandler()); @@ -256,10 +297,16 @@ export default (octokitApp: App): Express => { app.use(async (err: Error, req: Request, res: Response, next: NextFunction) => { req.log.error({ payload: req.body, err, req, res }, "Error in frontend app."); - if (!isNodeProd()) { + if (!isNodeProd() && !res.locals.showError) { return next(err); } + // Check for IP Allowlist error from Github and set the message explicitly + // to be shown to the user in the error page + if (err.name == "HttpError" && err.message?.includes("organization has an IP allow list enabled")) { + err.message = Errors.IP_ALLOWLIST_MISCONFIGURED; + } + // TODO: move this somewhere else, enum? const errorCodes = { Unauthorized: 401, @@ -268,7 +315,8 @@ export default (octokitApp: App): Express => { }; const messages = { - [Errors.MISSING_JIRA_HOST]: "Session information missing - please enable all cookies in your browser settings." + [Errors.MISSING_JIRA_HOST]: "Session information missing - please enable all cookies in your browser settings.", + [Errors.IP_ALLOWLIST_MISCONFIGURED]: `The GitHub org you are trying to connect is currently blocking our requests. To configure the GitHub IP Allow List correctly, please follow these instructions.` }; const errorStatusCode = errorCodes[err.message] || 500; @@ -277,10 +325,7 @@ export default (octokitApp: App): Express => { statsd.increment(metricError.githubErrorRendered, tags); - const newErrorPgFlagIsOn = await booleanFlag(BooleanFlags.NEW_GITHUB_ERROR_PAGE, true, req.session.jiraHost); - const errorPageVersion = newErrorPgFlagIsOn ? "github-error.hbs" : "github-error-OLD.hbs"; - - return res.status(errorStatusCode).render(errorPageVersion, { + return res.status(errorStatusCode).render("error.hbs", { title: "GitHub + Jira integration", message, nonce: res.locals.nonce, diff --git a/src/frontend/delete-github-subscription.ts b/src/frontend/delete-github-subscription.ts index 7a88013e93..ad918c4c9e 100644 --- a/src/frontend/delete-github-subscription.ts +++ b/src/frontend/delete-github-subscription.ts @@ -10,7 +10,7 @@ export default async (req: Request, res: Response): Promise => { return; } - if (!req.body.installationId || !req.body.jiraHost) { + if (!req.body.installationId || !res.locals.jiraHost) { res.status(400) .json({ err: "installationId and jiraHost must be provided to delete a subscription." @@ -63,7 +63,7 @@ export default async (req: Request, res: Response): Promise => { return; } - const subscription = await Subscription.getSingleInstallation(req.body.jiraHost, req.body.installationId); + const subscription = await Subscription.getSingleInstallation(res.locals.jiraHost, req.body.installationId); if(!subscription) { req.log.warn({req, res}, "Cannot find Subscription"); res.status(404).send("Cannot find Subscription."); diff --git a/src/frontend/delete-jira-configuration.ts b/src/frontend/delete-jira-configuration.ts index ba6f0c41df..72a0e51da4 100644 --- a/src/frontend/delete-jira-configuration.ts +++ b/src/frontend/delete-jira-configuration.ts @@ -6,26 +6,28 @@ import { Request, Response } from "express"; * Handle the when a user deletes an entry in the UI */ export default async (req: Request, res: Response): Promise => { - const jiraHost = req.session.jiraHost; - if(!jiraHost) { - req.log.warn("Missing Jira Host"); + const { jiraHost } = res.locals; + const installationId = Number(req.body.installationId); + if (!jiraHost) { + req.log.error("Missing Jira Host"); + res.status(401).send("Missing jiraHost in body"); return; } - req.log.info({ installationId: req.body.installationId }, "Received delete jira configuration request"); + req.log.info({ installationId }, "Received delete jira configuration request"); const subscription = await Subscription.getSingleInstallation( jiraHost, - req.body.installationId + installationId ); - if(!subscription) { + if (!subscription) { res.status(404).send("Cannot find Subscription"); return; } - const jiraClient = await getJiraClient(jiraHost, req.body.installationId, req.log); - await jiraClient.devinfo.installation.delete(req.body.installationId); + const jiraClient = await getJiraClient(jiraHost, installationId, req.log); + await jiraClient.devinfo.installation.delete(installationId); await subscription.destroy(); res.sendStatus(204); diff --git a/src/frontend/get-github-configuration.ts b/src/frontend/get-github-configuration.ts index 6790bee47c..804690dc07 100644 --- a/src/frontend/get-github-configuration.ts +++ b/src/frontend/get-github-configuration.ts @@ -7,6 +7,9 @@ import { GitHubAPI } from "probot"; import { Octokit } from "@octokit/rest"; import { booleanFlag, BooleanFlags } from "../config/feature-flags"; import { Errors } from "../config/errors"; +import { Tracer } from "../config/tracer"; +import GitHubClient from "../github/client/github-client"; +import Logger from "bunyan"; const getConnectedStatus = ( installationsWithSubscriptions: any, @@ -52,7 +55,11 @@ const installationConnectedStatus = async ( return mergeByLogin(installationsWithAdmin, connectedStatuses); }; -async function getInstallationsWithAdmin(installations: Octokit.AppsListInstallationsForAuthenticatedUserResponseInstallationsItem[], login: string, isAdmin: (args: { org: string, username: string, type: string }) => Promise): Promise { +async function getInstallationsWithAdmin(jiraHost: string, + log: Logger, + installations: Octokit.AppsListInstallationsForAuthenticatedUserResponseInstallationsItem[], + login: string, + isAdmin: (args: { org: string, username: string, type: string }) => Promise): Promise { const installationsWithAdmin: InstallationWithAdmin[] = []; for (const installation of installations) { @@ -64,21 +71,37 @@ async function getInstallationsWithAdmin(installations: Octokit.AppsListInstalla type: installation.target_type }); - const authedApp = await app.auth(installation.id); - enhanceOctokit(authedApp); + if(await booleanFlag(BooleanFlags.USE_NEW_GITHUB_CLIENT_TO_COUNT_REPOS, true, jiraHost)){ + const githubClient = new GitHubClient(installation.id, log); + const numberOfReposPromise = githubClient.getNumberOfReposForInstallation(); - const repositories = authedApp.paginate( - authedApp.apps.listRepos.endpoint.merge({ per_page: 100 }), - (res) => res.data - ); + const [admin, numberOfRepos] = await Promise.all([checkAdmin, numberOfReposPromise]); - const [admin, numberOfRepos] = await Promise.all([checkAdmin, repositories]); + log.info("Number of repos in the org received via GraphQL: " + numberOfRepos); - installationsWithAdmin.push({ - ...installation, - numberOfRepos: numberOfRepos.length || 0, - admin - }); + installationsWithAdmin.push({ + ...installation, + numberOfRepos: numberOfRepos || 0, + admin + }); + }else { + + const authedApp = await app.auth(installation.id); + enhanceOctokit(authedApp); + + const repositories = authedApp.paginate( + authedApp.apps.listRepos.endpoint.merge({ per_page: 100 }), + (res) => res.data + ); + + const [admin, numberOfRepos] = await Promise.all([checkAdmin, repositories]); + + installationsWithAdmin.push({ + ...installation, + numberOfRepos: numberOfRepos.length || 0, + admin + }); + } } return installationsWithAdmin; } @@ -110,50 +133,77 @@ const removeFailedConnectionsFromDb = async (req: Request, installations: any, j }; export default async (req: Request, res: Response, next: NextFunction): Promise => { - const { jiraHost, githubToken } = req.session; + const { githubToken } = req.session; + const { jiraHost } = res.locals; + const log = req.log.child({ jiraHost }); + if (!githubToken) { return next(new Error(Errors.MISSING_GITHUB_TOKEN)); } + const traceLogsEnabled = await booleanFlag(BooleanFlags.TRACE_LOGGING, false); + const tracer = new Tracer(log, "get-github-configuration", traceLogsEnabled); + + tracer.trace("found github token"); + if (!jiraHost) { return next(new Error(Errors.MISSING_JIRA_HOST)); } - req.log.info({ installationId: req.body.installationId }, "Received get jira configuration request"); + tracer.trace(`found jira host: ${jiraHost}`); const github: GitHubAPI = res.locals.github; const client: GitHubAPI = res.locals.client; const isAdmin = res.locals.isAdmin; + tracer.trace(`isAdmin: ${isAdmin}`); + const { data: { login } } = await github.users.getAuthenticated(); + tracer.trace(`got login name: ${login}`); + // Remove any failed installations before a user attempts to reconnect - const allInstallations = await getAllInstallations(client, req.log, jiraHost) + const allInstallations = await getAllInstallations(client, log, jiraHost) await removeFailedConnectionsFromDb(req, allInstallations, jiraHost) + tracer.trace(`removed failed installations`); + try { + // we can get the jira client Key from the JWT's `iss` property // so we'll decode the JWT here and verify it's the right key before continuing const installation = await Installation.getForHost(jiraHost); if (!installation) { - req.log.warn({ req, res }, "Missing installation"); + tracer.trace(`missing installation`); + log.warn({ req, res }, "Missing installation"); res.status(404).send(`Missing installation for host '${jiraHost}'`); return; } + + tracer.trace(`found installation in DB with id ${installation.id}`); + const { data: { installations } } = (await github.apps.listInstallationsForAuthenticatedUser()); - const installationsWithAdmin = await getInstallationsWithAdmin(installations, login, isAdmin); + + tracer.trace(`got user's installations from GitHub`); + + const installationsWithAdmin = await getInstallationsWithAdmin(jiraHost, log, installations, login, isAdmin); + + tracer.trace(`got user's installations with admin status from GitHub`); + const { data: info } = (await client.apps.getAuthenticated()); + + tracer.trace(`got user's authenticated apps from GitHub`); + const connectedInstallations = await installationConnectedStatus( jiraHost, client, installationsWithAdmin, - req.log + log ); - const newConnectAnOrgPgFlagIsOn = await booleanFlag(BooleanFlags.NEW_CONNECT_AN_ORG_PAGE, true, jiraHost); - const connectAnOrgPageVersion = newConnectAnOrgPgFlagIsOn ? "github-configuration.hbs" : "github-configuration-OLD.hbs"; + tracer.trace(`got connected installations`); - res.render(connectAnOrgPageVersion, { + res.render("github-configuration.hbs", { csrfToken: req.csrfToken(), installations: connectedInstallations, jiraHost: jiraHost, @@ -162,10 +212,14 @@ export default async (req: Request, res: Response, next: NextFunction): Promise< clientKey: installation.clientKey, login }); + + tracer.trace(`rendered page`); + } catch (err) { // If we get here, there was either a problem decoding the JWT // or getting the data we need from GitHub, so we'll show the user an error. - req.log.error({ err, req, res }, "Error while getting github configuration page"); + tracer.trace(`Error while getting github configuration page`); + log.error({ err, req, res }, "Error while getting github configuration page"); return next(err); } }; diff --git a/src/frontend/get-github-setup.ts b/src/frontend/get-github-setup.ts index 22ba1ff120..2816b328d2 100644 --- a/src/frontend/get-github-setup.ts +++ b/src/frontend/get-github-setup.ts @@ -1,10 +1,5 @@ -import { jiraTopleveldomainOptions } from "./validations"; import { Request, Response } from "express"; -import { booleanFlag, BooleanFlags } from "../config/feature-flags"; -import { - getJiraMarketplaceUrl, - getGitHubConfigurationUrl, -} from "../util/getUrl"; +import { getGitHubConfigurationUrl, getJiraMarketplaceUrl } from "../util/getUrl"; /* Handles redirects for both the installation flow from Jira and @@ -16,31 +11,16 @@ import { */ export default async (req: Request, res: Response): Promise => { req.log.info("Received get github setup page request"); - - if (req.headers.referer) { - const { host: githubHost } = req; - const { jwt, jiraHost } = req.session; - - res.redirect(getGitHubConfigurationUrl(githubHost, jwt, jiraHost)); - } else { - const jiraHost = req.session?.jiraHost; - const marketplaceUrl = jiraHost - ? getJiraMarketplaceUrl(jiraHost) - : undefined; - - if (await booleanFlag(BooleanFlags.NEW_SETUP_PAGE, true, jiraHost)) { - res.render("github-setup.hbs", { - csrfToken: req.csrfToken(), - nonce: res.locals.nonce, - jiraHost: jiraHost, - marketplaceUrl, // only used is jiraHost is present - }); - } else { - res.render("github-setup-OLD.hbs", { - jiraTopleveldomainOptions: jiraTopleveldomainOptions(), - csrfToken: req.csrfToken(), - nonce: res.locals.nonce, - }); - } + const { jiraHost } = res.locals; + if (req.headers.referer && jiraHost) { + return res.redirect(getGitHubConfigurationUrl(jiraHost)); } + const marketplaceUrl = jiraHost ? getJiraMarketplaceUrl(jiraHost) : undefined; + + res.render("github-setup.hbs", { + csrfToken: req.csrfToken(), + nonce: res.locals.nonce, + jiraHost, + marketplaceUrl // only used is jiraHost is present + }); }; diff --git a/src/frontend/get-github-subscriptions.ts b/src/frontend/get-github-subscriptions.ts index c1f1a26854..8e9f87b3c4 100644 --- a/src/frontend/get-github-subscriptions.ts +++ b/src/frontend/get-github-subscriptions.ts @@ -29,7 +29,7 @@ export default async (req: Request, res: Response, next: NextFunction): Promise< nonce: res.locals.nonce, installation, info, - host: req.session.jiraHost, + host: res.locals.jiraHost, subscriptions, hasSubscriptions: subscriptions.length > 0 }); diff --git a/src/frontend/get-jira-configuration.ts b/src/frontend/get-jira-configuration.ts index 9837ba3a68..c05e61fa68 100644 --- a/src/frontend/get-jira-configuration.ts +++ b/src/frontend/get-jira-configuration.ts @@ -1,14 +1,15 @@ import format from "date-fns/format"; import moment from "moment"; -import SubscriptionClass from "../models/subscription"; +import SubscriptionClass, { SyncStatus } from "../models/subscription"; import { RepoSyncState, Subscription } from "../models"; import { NextFunction, Request, Response } from "express"; import statsd from "../config/statsd"; import { metricError } from "../config/metric-names"; -import { booleanFlag, BooleanFlags } from "../config/feature-flags"; import { FailedInstallations } from "../config/interfaces"; +import { booleanFlag, BooleanFlags } from "../config/feature-flags"; +import Logger from "bunyan"; -function mapSyncStatus(syncStatus: string): string { +function mapSyncStatus(syncStatus: SyncStatus = SyncStatus.PENDING): string { switch (syncStatus) { case "ACTIVE": return "IN PROGRESS"; @@ -19,7 +20,7 @@ function mapSyncStatus(syncStatus: string): string { } } -export async function getInstallation(client, subscription, reqLog?) { +export async function getInstallation(client, subscription: SubscriptionClass, reqLog?: Logger) { const id = subscription.gitHubInstallationId; try { const response = await client.apps.getInstallation({ installation_id: id }); @@ -38,7 +39,7 @@ export async function getInstallation(client, subscription, reqLog?) { return response.data; } catch (err) { - reqLog.error( + reqLog?.error( { installationId: id, error: err, uninstalled: err.code === 404 }, "Failed connection" ); @@ -93,7 +94,7 @@ export default async ( next: NextFunction ): Promise => { try { - const jiraHost = req.session.jiraHost; + const jiraHost = res.locals.jiraHost; if (!jiraHost) { req.log.warn({ jiraHost, req, res }, "Missing jiraHost"); @@ -126,20 +127,10 @@ export default async ( repoSyncState: data.repoSyncState })); - const newConfigPgFlagIsOn = await booleanFlag( - BooleanFlags.NEW_GITHUB_CONFIG_PAGE, - true, - jiraHost - ); - - const configPageVersion = newConfigPgFlagIsOn - ? "jira-configuration.hbs" - : "jira-configuration-OLD.hbs"; - const hasConnections = connections.length > 0 || failedConnections.length > 0; - res.render(configPageVersion, { + res.render("jira-configuration.hbs", { host: jiraHost, connections, failedConnections, diff --git a/src/frontend/github-oauth.ts b/src/frontend/github-oauth.ts index 3ac0e93a8e..5a36835371 100644 --- a/src/frontend/github-oauth.ts +++ b/src/frontend/github-oauth.ts @@ -2,10 +2,11 @@ import crypto from "crypto"; import url from "url"; import express, { NextFunction, Request, RequestHandler, Response, Router } from "express"; import axios from "axios"; -import { getJiraHostFromRedirectUrl } from "../util/getUrl"; import { getLogger } from "../config/logger"; +import { booleanFlag, BooleanFlags } from "../config/feature-flags"; +import { Tracer } from "../config/tracer"; -const host = process.env.GHE_HOST || "github.com"; +const githubHost = process.env.GHE_HOST || "github.com"; const logger = getLogger("github-oauth"); export interface OAuthOptions { @@ -30,19 +31,29 @@ export default (opts: OAuthOptions): GithubOAuth => { logger.info(opts, "Configuring github auth"); - function login(req: Request, res: Response): void { + async function login(req: Request, res: Response): Promise { + + const traceLogsEnabled = await booleanFlag(BooleanFlags.TRACE_LOGGING, false); + const tracer = new Tracer(logger.child(opts), "login", traceLogsEnabled); + // TODO: We really should be using an Auth library for this, like @octokit/github-auth // Create unique state for each oauth request const state = crypto.randomBytes(8).toString("hex"); + req.session["timestamp_before_oauth"] = new Date().getTime(); + // Save the redirect that may have been specified earlier into session to be retrieved later req.session[state] = res.locals.redirect || `/github/configuration${url.parse(req.originalUrl).search || ""}`; - const redirectUrl = `https://${host}/login/oauth/authorize?client_id=${opts.githubClient}${ + const redirectUrl = `https://${githubHost}/login/oauth/authorize?client_id=${opts.githubClient}${ opts.scopes?.length ? `&scope=${opts.scopes.join(" ")}` : "" }&redirect_uri=${redirectURI}&state=${state}`; - req.log.info({ redirectUrl, postLoginUrl: req.session[state] }, `Received request for ${opts.loginURI}, redirecting to Github OAuth flow`); + req.log.info({ + redirectUrl, + postLoginUrl: req.session[state] + }, `Received request for ${opts.loginURI}, redirecting to Github OAuth flow`); + tracer.trace(`redirecting to ${redirectUrl}`); res.redirect(redirectUrl); } @@ -59,8 +70,18 @@ export default (opts: OAuthOptions): GithubOAuth => { state } = req.query as Record; + const traceLogsEnabled = await booleanFlag(BooleanFlags.TRACE_LOGGING, false); + const tracer = new Tracer(logger.child(opts), "callback", traceLogsEnabled); + + const timestampBefore = req.session["timestamp_before_oauth"] as number; + if (timestampBefore) { + const timestampAfter = new Date().getTime(); + tracer.trace(`callback called after spending ${timestampAfter - timestampBefore} ms on GitHub servers`); + } + // Show the oauth error if there is one if (error) { + tracer.trace(`OAuth Error: ${error}`); return next(`OAuth Error: ${error} URL: ${error_uri} ${error_description}`); @@ -70,17 +91,21 @@ export default (opts: OAuthOptions): GithubOAuth => { const redirectUrl = req.session[state] as string; delete req.session[state]; + tracer.trace(`extracted redirectUrl from session: ${redirectUrl}`); req.log.info({ query: req.query }, `Received request to ${opts.callbackURI}`); // Check if state is available and matches a previous request if (!state || !redirectUrl) return next("Missing matching Auth state parameter"); if (!code) return next("Missing OAuth Code"); - req.log.info({ jiraHost: getJiraHostFromRedirectUrl(redirectUrl, req.log) }, "Jira Host attempting to auth with GitHub"); + const { jiraHost } = res.locals; + + req.log.info({ jiraHost }, "Jira Host attempting to auth with GitHub"); + tracer.trace(`extracted jiraHost from redirect url: ${jiraHost}`); try { const response = await axios.get( - `https://${host}/login/oauth/access_token`, + `https://${githubHost}/login/oauth/access_token`, { params: { client_id: opts.githubClient, @@ -99,12 +124,16 @@ export default (opts: OAuthOptions): GithubOAuth => { req.session.githubToken = response.data.access_token; if (!req.session.githubToken) { + tracer.trace(`didn't get access token from GitHub`); return next(new Error("Missing Access Token from Github OAuth Flow.")); } + tracer.trace(`got access token from GitHub, redirecting to ${redirectUrl}`); + req.log.info({ redirectUrl }, "Github OAuth code valid, redirecting to internal URL"); return res.redirect(redirectUrl); } catch (e) { + tracer.trace(`Cannot retrieve access token from Github`); return next(new Error("Cannot retrieve access token from Github")); } } @@ -116,11 +145,16 @@ export default (opts: OAuthOptions): GithubOAuth => { return { router: router, - checkGithubAuth: (req: Request, res: Response, next: NextFunction) => { + checkGithubAuth: async (req: Request, res: Response, next: NextFunction) => { + const traceLogsEnabled = await booleanFlag(BooleanFlags.TRACE_LOGGING, false); + const tracer = new Tracer(logger.child(opts), "checkGithubAuth", traceLogsEnabled); + if (!req.session.githubToken) { + tracer.trace("found github token in session, calling login()"); res.locals.redirect = req.originalUrl; return login(req, res); } + tracer.trace("found github token in session"); return next(); } }; diff --git a/src/frontend/post-github-configuration.ts b/src/frontend/post-github-configuration.ts index f20107b98f..971e70eefd 100644 --- a/src/frontend/post-github-configuration.ts +++ b/src/frontend/post-github-configuration.ts @@ -6,7 +6,7 @@ import { Request, Response } from "express"; * Handle the when a user adds a repo to this installation */ export default async (req: Request, res: Response): Promise => { - if (!req.session.githubToken || !req.session.jiraHost) { + if (!req.session.githubToken || !res.locals.jiraHost) { res.sendStatus(401); return; } @@ -55,7 +55,7 @@ export default async (req: Request, res: Response): Promise => { await Subscription.install({ clientKey: getHashedKey(req.body.clientKey), installationId: req.body.installationId, - host: req.session.jiraHost + host: res.locals.jiraHost }); res.sendStatus(200); diff --git a/src/frontend/post-github-setup.ts b/src/frontend/post-github-setup.ts index 2966fb3960..8703a62895 100644 --- a/src/frontend/post-github-setup.ts +++ b/src/frontend/post-github-setup.ts @@ -1,56 +1,33 @@ import axios from "axios"; -import { jiraTopleveldomainOptions, validJiraDomains } from "./validations"; +import { validJiraDomains } from "./validations"; import { Request, Response } from "express"; import { getJiraMarketplaceUrl } from "../util/getUrl"; -import { booleanFlag, BooleanFlags } from "../config/feature-flags"; - -interface JiraTopleveldomain { - value: string; - selected: boolean; -} interface SetupPagePayload { error: string; domain: string; nonce: string; csrfToken: string; - jiraTopleveldomainOptions?: JiraTopleveldomain[]; } const renderGitHubSetupPageVersion = async ( domain: string, - topLevelDomain: string, req: Request, - res: Response, - jiraSiteUrl?: string + res: Response ) => { - const newGithubSetupPgFlagIsOn = await booleanFlag( - BooleanFlags.NEW_SETUP_PAGE, - true, - jiraSiteUrl - ); - - const newGitHubSetupPagePayload: SetupPagePayload = { + const gitHubSetupPagePayload: SetupPagePayload = { error: "The entered Jira Cloud site is not valid.", domain, nonce: res.locals.nonce, csrfToken: req.csrfToken(), }; - const oldGitHubSetupPagePayload = { ...newGitHubSetupPagePayload }; - - oldGitHubSetupPagePayload.jiraTopleveldomainOptions = - jiraTopleveldomainOptions(topLevelDomain); - - return newGithubSetupPgFlagIsOn - ? res.render("github-setup.hbs", newGitHubSetupPagePayload) - : res.render("github-setup-OLD.hbs", oldGitHubSetupPagePayload); + res.render("github-setup.hbs", gitHubSetupPagePayload) }; const validateJiraSite = ( jiraSiteUrl: string, domain: string, - topLevelDomain: string, req: Request, res: Response ): void => { @@ -74,35 +51,27 @@ const validateJiraSite = ( .catch((error) => { // If Jira site is not valid, it returns a 404 req.log.error({ error }, "Invalid Jira site entered."); - renderGitHubSetupPageVersion(domain, topLevelDomain, req, res); + renderGitHubSetupPageVersion(domain, req, res); }); }; export default async (req: Request, res: Response): Promise => { - // TODO - clean up after NEW_SETUP_PAGE is removed - const { jiraDomain, jiraDomainMain, jiraDomainModal, jiraTopleveldomain } = + const { jiraDomain, jiraDomainMain, jiraDomainModal } = req.body; const domain = jiraDomain || jiraDomainMain || jiraDomainModal; - const topLevelDomain = jiraTopleveldomain || "atlassian.net"; + const topLevelDomain = "atlassian.net"; const jiraSiteUrl = `https://${domain}.${topLevelDomain}`; req.log.info(`Received github setup page request for jira ${jiraSiteUrl}`); - const newGithubSetupPgFlagIsOn = await booleanFlag( - BooleanFlags.NEW_SETUP_PAGE, - true - ); - - const siteUrlIncludesProtocol = newGithubSetupPgFlagIsOn - ? !validJiraDomains(domain, "atlassian.net") - : !validJiraDomains(domain, topLevelDomain); + const siteUrlIncludesProtocol = !validJiraDomains(domain, topLevelDomain) if (siteUrlIncludesProtocol) { res.status(400); - renderGitHubSetupPageVersion(domain, topLevelDomain, req, res, jiraSiteUrl); + renderGitHubSetupPageVersion(domain, req, res); } - validateJiraSite(jiraSiteUrl, domain, topLevelDomain, req, res); + validateJiraSite(jiraSiteUrl, domain, req, res); }; diff --git a/src/frontend/validations.ts b/src/frontend/validations.ts index 1a21818cee..232204a275 100644 --- a/src/frontend/validations.ts +++ b/src/frontend/validations.ts @@ -7,13 +7,6 @@ export const validJiraDomains = (jiraDomain: string, jiraTopleveldomain: string) !!jiraDomain && !!jiraTopleveldomain && domainRegexp.test(jiraDomain) && jiraTopleveldomains.includes(jiraTopleveldomain); - -export const jiraTopleveldomainOptions = (jiraTopleveldomain?: string): JiraTopleveldomain[] => - jiraTopleveldomains.map(value => ({ - value, - selected: value === jiraTopleveldomain - })); - export interface JiraTopleveldomain { value: string; selected: boolean; diff --git a/src/frontend/verify-jira-jwt-middleware.ts b/src/frontend/verify-jira-jwt-middleware.ts index 6d191022c3..56495db5ee 100644 --- a/src/frontend/verify-jira-jwt-middleware.ts +++ b/src/frontend/verify-jira-jwt-middleware.ts @@ -1,16 +1,17 @@ import { Installation } from "../models"; import { NextFunction, Request, Response } from "express"; -import { TokenType, verifyAsymmetricJwtTokenMiddleware, verifySymmetricJwtTokenMiddleware, sendError } from "../jira/util/jwt"; +import { sendError, TokenType, verifyAsymmetricJwtTokenMiddleware, verifySymmetricJwtTokenMiddleware } from "../jira/util/jwt"; const verifyJiraJwtMiddleware = (tokenType: TokenType) => async ( req: Request, res: Response, next: NextFunction ): Promise => { - const jiraHost = (req.query.xdm_e as string) || req.body?.jiraHost; + const { jiraHost } = res.locals; - if(!jiraHost) { + if (!jiraHost) { sendError(res, 401, "Unauthorised"); + return; } const installation = await Installation.getForHost(jiraHost); diff --git a/src/github/client/errors.ts b/src/github/client/errors.ts new file mode 100644 index 0000000000..a7b6e0bb5f --- /dev/null +++ b/src/github/client/errors.ts @@ -0,0 +1,29 @@ +import { AxiosError } from "axios"; + +export class GithubClientError extends Error { + status?: number; + cause: AxiosError; + constructor(message: string, cause: AxiosError, status?: number) { + super(message); + this.status = status; + this.cause = { ...cause, config: {} }; + } +} + +export class RateLimitingError extends GithubClientError { + /** + * The value of the x-ratelimit-reset header, i.e. the epoch seconds when the rate limit is refreshed. + */ + rateLimitReset: number; + rateLimitRemaining: number; + constructor(resetEpochSeconds: number, rateLimitRemaining: number, error: AxiosError, status?: number) { + super("Rate limiting error", error, status); + this.rateLimitReset = resetEpochSeconds; + this.rateLimitRemaining = rateLimitRemaining; + } +} +export class BlockedIpError extends GithubClientError { + constructor(error: AxiosError, status?: number) { + super("Blocked by GitHub allowlist", error, status); + } +} diff --git a/src/github/client/github-client.ts b/src/github/client/github-client.ts index 745561d5de..797b94c59d 100644 --- a/src/github/client/github-client.ts +++ b/src/github/client/github-client.ts @@ -1,29 +1,46 @@ +import Logger from 'bunyan'; import { Octokit } from "@octokit/rest"; import axios, { AxiosInstance, AxiosRequestConfig, AxiosResponse } from "axios"; import AppTokenHolder from "./app-token-holder"; import InstallationTokenCache from "./installation-token-cache"; import AuthToken from "./auth-token"; import { GetPullRequestParams } from "./types"; - +import { handleFailedRequest, instrumentFailedRequest, instrumentRequest, setRequestStartTime } from "./interceptors"; +import { metricHttpRequest } from "../../config/metric-names"; +import { getLogger } from "../../config/logger"; +import { urlParamsMiddleware } from "../../util/axios/common-middleware"; + /** * A GitHub client that supports authentication as a GitHub app. * * @see https://docs.github.com/en/developers/apps/building-github-apps/authenticating-with-github-apps */ export default class GitHubClient { - private readonly axios: AxiosInstance; private readonly appTokenHolder: AppTokenHolder; private readonly installationTokenCache: InstallationTokenCache; private readonly githubInstallationId: number; + private logger: Logger; constructor( githubInstallationId: number, + logger: Logger, baseURL = "https://api.github.com" ) { + this.logger = logger || getLogger("github.client.axios"); this.axios = axios.create({ baseURL }); + this.axios.interceptors.request.use(urlParamsMiddleware) + this.axios.interceptors.request.use(setRequestStartTime); + this.axios.interceptors.response.use( + undefined, + handleFailedRequest(this.logger) + ); + this.axios.interceptors.response.use( + instrumentRequest(metricHttpRequest.github), + instrumentFailedRequest(metricHttpRequest.github) + ); this.appTokenHolder = AppTokenHolder.getInstance(); this.installationTokenCache = InstallationTokenCache.getInstance(); this.githubInstallationId = githubInstallationId; @@ -69,37 +86,85 @@ export default class GitHubClient { return new AuthToken(tokenResponse.token, new Date(tokenResponse.expires_at)); } - private async get(url, params = {}): Promise> { + private async get(url, params = {}, urlParams = {}): Promise> { const response = await this.axios.get(url, { ...await this.installationAuthenticationHeaders(), - params: { + params: { installationId: this.githubInstallationId, - ...params - } + ...params, + }, + urlParams, }); - //TODO: error handling return response; } + private async graphql(query: string): Promise { + return await this.axios.post("https://api.github.com/graphql", + { + query + }, + { + ...await this.installationAuthenticationHeaders(), + params: { + installationId: this.githubInstallationId, + } + }); + } + /** * Lists pull requests for the given repository. */ - public async getPullRequests(owner:string, repo: string, pullRequestParams: GetPullRequestParams): Promise> { - return await this.get(`/repos/${owner}/${repo}/pulls`, pullRequestParams); + public async getPullRequests(owner: string, repo: string, pullRequestParams: GetPullRequestParams): Promise> { + return await this.get(`/repos/:owner/:repo/pulls`, pullRequestParams, { + owner, + repo + }); } /** * Get a single pull request for the given repository. */ + // TODO: add a unit test public async getPullRequest(owner: string, repo: string, pullNumber: string): Promise> { - return await this.get(`/repos/${owner}/${repo}/pulls/${pullNumber}`); + return await this.get(`/repos/:owner/:repo/pulls/:pullNumber`, {}, { + owner, + repo, + pullNumber + }); } /** * Get publicly available information for user with given username. */ + // TODO: add a unit test public getUserByUsername = async (username: string): Promise> => { - return await this.get(`/users/${username}`); + return await this.get(`/users/:username`, {}, { + username + }); + } + + /** + * Get a single commit for the given repository. + */ + public getCommit = async (owner: string, repo: string, ref: string): Promise> => { + return await this.get(`/repos/:owner/:repo/commits/:ref`, {}, { + owner, + repo, + ref + }); + } + + public async getNumberOfReposForInstallation(): Promise { + const response = await this.graphql(` + query { + viewer { + repositories { + totalCount + } + } + }`) + + return response?.data?.data?.viewer?.repositories?.totalCount as number; } } diff --git a/src/github/client/interceptors.ts b/src/github/client/interceptors.ts new file mode 100644 index 0000000000..bcc20b7b3c --- /dev/null +++ b/src/github/client/interceptors.ts @@ -0,0 +1,111 @@ +import { BlockedIpError, GithubClientError, RateLimitingError } from "./errors"; +import Logger from "bunyan"; +import url from "url"; +import statsd from "../../config/statsd"; +import { metricError } from "../../config/metric-names"; + +/** + * Extract the path name from a URL. + */ +const extractPath = (someUrl = ""): string => + url.parse(someUrl).pathname || ""; + +const RESPONSE_TIME_HISTOGRAM_BUCKETS = "100_1000_2000_3000_5000_10000_30000_60000"; + +/** + * Enrich the config object to include the time that the request started. + * + * @param {import("axios").AxiosRequestConfig} config - The Axios request configuration object. + * @returns {import("axios").AxiosRequestConfig} The enriched config object. + */ +export const setRequestStartTime = (config) => { + config.requestStartTime = new Date(); + return config; +}; + +//TODO Move to util/axios/common-middleware.ts and use with Jira Client +const sendResponseMetrics = (response, metricName: string, status?: string | number) => { + status = `${status || response.status}`; + const requestDurationMs = Number( + Date.now() - (response.config?.requestStartTime || 0) + ); + + // using client tag to separate GH client from Octokit + const tags = { + client: "axios", + method: response.config?.method?.toUpperCase(), + path: extractPath(response.config?.originalUrl), + status: status + }; + + statsd.histogram(metricName, requestDurationMs, tags); + tags["gsd_histogram"] = RESPONSE_TIME_HISTOGRAM_BUCKETS; + statsd.histogram(metricName, requestDurationMs, tags); + return response; +} + + +export const instrumentRequest = (metricName) => + (response) => { + if(!response) { + return; + } + return sendResponseMetrics(response, metricName); + }; + +/** + * Submit statsd metrics on failed requests. + * + * @param {import("axios").AxiosError} error - The Axios error response object. + * @param metricName - Name for the response metric + * @returns {Promise} a rejected promise with the error inside. + */ +export const instrumentFailedRequest = (metricName) => + (error) => { + if(error instanceof RateLimitingError) { + sendResponseMetrics(error.cause?.response, metricName, "rateLimiting") + } else if(error instanceof BlockedIpError) { + sendResponseMetrics(error.cause?.response, metricName, "blockedIp"); + statsd.increment(metricError.blockedByGitHubAllowlist); + } else if(error instanceof GithubClientError) { + sendResponseMetrics(error.cause?.response, metricName); + } else { + sendResponseMetrics(error.response, metricName); + } + return Promise.reject(error); + }; + + +export const handleFailedRequest = (logger: Logger) => + (error) => { + const response = error.response; + if(response) { + const status = response?.status; + const errorMessage = `Error executing Axios Request ` + error.message; + + const rateLimitResetHeaderValue:string = response.headers?.["x-ratelimit-reset"]; + const rateLimitRemainingHeaderValue:string = response.headers?.["x-ratelimit-reset"]; + if(response.headers?.["x-ratelimit-remaining"] == "0" && rateLimitResetHeaderValue) { + logger.warn({ err: error }, "Rate limiting error"); + const rateLimitReset: number = parseInt(rateLimitResetHeaderValue); + return Promise.reject(new RateLimitingError(rateLimitReset, 0, error, status)); + } + + if (status === 403 && response.data.message.includes("has an IP allow list enabled")) { + logger.error({ err: error }, "Blocked by GitHub allowlist"); + return Promise.reject(new BlockedIpError(error, status)); + } + + if (status === 403) { + const rateLimitRemaining: number = parseInt(rateLimitRemainingHeaderValue); + logger.warn({ err: error }, "Github returned 403 without ratelimit header"); + return Promise.reject(new RateLimitingError(Date.now() / 1000 + 60 * 60, rateLimitRemaining, error, status)); + } + + const isWarning = status && (status >= 300 && status < 500 && status !== 400); + + isWarning? logger.warn(errorMessage) : logger.error({err: error}, errorMessage); + return Promise.reject(new GithubClientError(errorMessage, error, status)); + } + return Promise.reject(error); + } diff --git a/src/github/deployment.ts b/src/github/deployment.ts index 1ce9ea6d63..ba5cf9581f 100644 --- a/src/github/deployment.ts +++ b/src/github/deployment.ts @@ -4,7 +4,7 @@ import { CustomContext } from "./middleware"; import { DeploymentsResult } from "../jira/client"; export default async (context: CustomContext, jiraClient): Promise => { - const jiraPayload = await transformDeployment(context); + const jiraPayload = await transformDeployment(context.github, context.payload, jiraClient.baseURL, context.log); if (!jiraPayload) { context.log( diff --git a/src/github/middleware.ts b/src/github/middleware.ts index 6312447663..8bbade0fea 100644 --- a/src/github/middleware.ts +++ b/src/github/middleware.ts @@ -78,10 +78,8 @@ export default ( const webhookEvent = extractWebhookEventNameFromContext(context); // Metrics for webhook payload size - if (await booleanFlag(BooleanFlags.PAYLOAD_SIZE_METRIC, false)) { - emitWebhookPayloadMetrics(webhookEvent, - Buffer.byteLength(JSON.stringify(context.payload), "utf-8")); - } + emitWebhookPayloadMetrics(webhookEvent, + Buffer.byteLength(JSON.stringify(context.payload), "utf-8")); const webhookReceived = getCurrentTime(); context.webhookReceived = webhookReceived; diff --git a/src/github/push.ts b/src/github/push.ts index b8e658a883..422e73fda6 100644 --- a/src/github/push.ts +++ b/src/github/push.ts @@ -4,6 +4,7 @@ import { Context } from "probot/lib/context"; import { booleanFlag, BooleanFlags } from "../config/feature-flags"; import { getCurrentTime } from '../util/webhooks'; import _ from "lodash"; +import GitHubClient from "./client/github-client"; export default async (context: Context, jiraClient): Promise => { const webhookReceived = getCurrentTime(); @@ -38,7 +39,9 @@ export default async (context: Context, jiraClient): Promise => { // If there's less than 20 commits (the number of commits the github API returns per call), just process it immediately if(payload.commits?.length < 20 && await booleanFlag(BooleanFlags.PROCESS_PUSHES_IMMEDIATELY, true, jiraClient.baseURL)) { context.log.info("Processing push straight away"); - await processPush(context.github, createJobData(payload, jiraClient.baseURL), context.log); + // TODO: this path is not used, opportunistically adding this line to make TypeScript happy without much testing + const githubNew = new GitHubClient(payload.installation?.id, context.log); + await processPush(context.github, githubNew, createJobData(payload, jiraClient.baseURL), context.log); return; } diff --git a/src/github/workflow.ts b/src/github/workflow.ts index 02988db9d2..7f5150ae0c 100644 --- a/src/github/workflow.ts +++ b/src/github/workflow.ts @@ -3,17 +3,18 @@ import { CustomContext } from "./middleware"; import { emitWebhookProcessedMetrics } from "../util/webhooks"; export default async (context: CustomContext, jiraClient): Promise => { - const jiraPayload = transformWorkflow(context); + const { github, payload, log: logger } = context; + const jiraPayload = await transformWorkflow(github, payload, jiraClient.baseURL, logger); if (!jiraPayload) { - context.log( + logger.info( { noop: "no_jira_payload_workflow_run" }, "Halting further execution for workflow since jiraPayload is empty" ); return; } - context.log(`Sending workflow event to Jira: ${jiraClient.baseURL}`); + logger.info(`Sending workflow event to Jira: ${jiraClient.baseURL}`); const jiraResponse = await jiraClient.workflow.submit(jiraPayload); const { webhookReceived, name, log } = context; diff --git a/src/interfaces/jira.ts b/src/interfaces/jira.ts index 1091bff7d6..2176b9c401 100644 --- a/src/interfaces/jira.ts +++ b/src/interfaces/jira.ts @@ -83,7 +83,7 @@ export interface JiraDeployment { displayName: string; url: string; description: string; - lastUpdated: number; + lastUpdated: Date; state: string; pipeline: { id: string; diff --git a/src/jira/client/axios.ts b/src/jira/client/axios.ts index f06baa5e1d..c93b42acbf 100644 --- a/src/jira/client/axios.ts +++ b/src/jira/client/axios.ts @@ -6,6 +6,7 @@ import statsd from "../../config/statsd"; import { getLogger } from "../../config/logger"; import { metricHttpRequest } from "../../config/metric-names"; import { createQueryStringHash, encodeSymmetric } from "atlassian-jwt"; +import {urlParamsMiddleware} from "../../util/axios/common-middleware"; const instance = process.env.INSTANCE_NAME; const iss = `com.github.integration${instance ? `.${instance}` : ""}`; @@ -148,44 +149,6 @@ function getSuccessMiddleware(logger: Logger) { ); } -/** - * Enrich the Axios Request Config with a URL object. - */ - -// TODO: non-standard and probably should be done through string interpolation -function getUrlMiddleware() { - return ( - /** - * @param {import("axios").AxiosRequestConfig} config - The outgoing request configuration. - * @returns {import("axios").AxiosRequestConfig} The enriched axios request config. - */ - (config) => { - // eslint-disable-next-line prefer-const - let { query, pathname, ...rest } = url.parse(config.url, true); - config.urlParams = config.urlParams || {}; - - for (const param in config.urlParams) { - if (pathname?.includes(`:${param}`)) { - pathname = pathname.replace(`:${param}`, config.urlParams[param]); - } else { - query[param] = config.urlParams[param]; - } - } - - config.urlParams.baseUrl = config.baseURL; - - return { - ...config, - originalUrl: config.url, - url: url.format({ - ...rest, - pathname, - query - }) - }; - } - ); -} /* * The Atlassian API uses JSON Web Tokens (JWT) for authentication along with @@ -285,7 +248,7 @@ export default ( ); instance.interceptors.request.use(getAuthMiddleware(secret)); - instance.interceptors.request.use(getUrlMiddleware()); + instance.interceptors.request.use(urlParamsMiddleware); instance.interceptors.response.use( getSuccessMiddleware(logger), diff --git a/src/jira/client/index.ts b/src/jira/client/index.ts index 72e500e5c2..01c8bc91e0 100644 --- a/src/jira/client/index.ts +++ b/src/jira/client/index.ts @@ -142,11 +142,11 @@ async function getJiraClient( }, // Add methods for handling installationId properties that exist in Jira installation: { - exists: (gitHubInstallationId: string) => + exists: (gitHubInstallationId: string | number) => instance.get( `/rest/devinfo/0.10/existsByProperties?installationId=${gitHubInstallationId}` ), - delete: (gitHubInstallationId: string) => + delete: (gitHubInstallationId: string | number) => instance.delete( `/rest/devinfo/0.10/bulkByProperties?installationId=${gitHubInstallationId}` ) diff --git a/src/jira/connect.ts b/src/jira/connect.ts index 5af5d13393..afdce505f5 100644 --- a/src/jira/connect.ts +++ b/src/jira/connect.ts @@ -4,6 +4,7 @@ import { EnvironmentEnum } from "../interfaces/common"; const instance = process.env.INSTANCE_NAME; const isProd = (instance === EnvironmentEnum.production); +export const postInstallUrl = "/jira/configuration"; export default async (_: Request, res: Response): Promise => { const appKey = `com.github.integration${instance ? `.${instance}` : ""}`; @@ -82,7 +83,7 @@ export default async (_: Request, res: Response): Promise => { name: { value: "GitHub Configuration" }, - url: "/jira/configuration", + url: postInstallUrl, conditions: adminPageDisplayConditions }, webSections: [ diff --git a/src/models/installation.ts b/src/models/installation.ts index bf366725ce..25eb133361 100644 --- a/src/models/installation.ts +++ b/src/models/installation.ts @@ -34,25 +34,21 @@ export default class Installation extends Sequelize.Model { } static async getForHost(host: string): Promise { - const payload: any = { + return Installation.findOne( { where: { jiraHost: host }, order: [["id", "DESC"]] - }; - - return Installation.findOne(payload); + }); } static async getAllForHost(host: string): Promise { - const payload: any = { + return Installation.findAll({ where: { jiraHost: host }, order: [["id", "DESC"]] - }; - - return Installation.findAll(payload); + }); } /** diff --git a/src/models/subscription.ts b/src/models/subscription.ts index f2af290401..62a01296e5 100644 --- a/src/models/subscription.ts +++ b/src/models/subscription.ts @@ -1,11 +1,10 @@ import Sequelize, { Op, WhereOptions } from "sequelize"; -import { Job } from "bull"; import _ from "lodash"; import logger from "../config/logger"; import { queues } from "../worker/queues"; import { booleanFlag, BooleanFlags } from "../config/feature-flags"; -import sqsQueues from "../sqs/queues"; import RepoSyncState from "./reposyncstate"; +import backfillQueueSupplier from '../backfill-queue-supplier'; export enum SyncStatus { PENDING = "PENDING", @@ -187,26 +186,12 @@ export default class Subscription extends Sequelize.Model { }); } - static async startNewSyncProcess(subscription: Subscription) { - //TODO Add a sync start logic - await sqsQueues.backfill.sendMessage({ - installationId: subscription.gitHubInstallationId, - jiraHost: subscription.jiraHost - }); - } - static async findOrStartSync( subscription: Subscription, syncType?: string - ): Promise { + ): Promise { const { gitHubInstallationId: installationId, jiraHost } = subscription; - if (await booleanFlag(BooleanFlags.NEW_BACKFILL_PROCESS_ENABLED, false, subscription.jiraHost)) { - await this.startNewSyncProcess(subscription); - } - - // If repo sync state is empty - // start a sync job from scratch if (!subscription.repoSyncState || syncType === "full") { subscription.changed("repoSyncState", true); await subscription.update({ @@ -222,13 +207,19 @@ export default class Subscription extends Sequelize.Model { await RepoSyncState.resetSyncFromSubscription(subscription); } logger.info("Starting Jira sync"); - return queues.discovery.add({ installationId, jiraHost }); + await queues.discovery.add({ installationId, jiraHost }); + return; } // Otherwise, just add a job to the queue for this installation // This will automatically pick back up from where it left off // if something got stuck - return queues.installation.add({ installationId, jiraHost }); + if (await booleanFlag(BooleanFlags.USE_SQS_FOR_BACKFILL, false, jiraHost)) { + const backfillQueue = await backfillQueueSupplier.supply(); + await backfillQueue.schedule({installationId, jiraHost}, 0, logger); + } else { + await queues.installation.add({ installationId, jiraHost }); + } } /* diff --git a/src/sqs/backfill.ts b/src/sqs/backfill.ts index 4851f467a4..73752e3f43 100644 --- a/src/sqs/backfill.ts +++ b/src/sqs/backfill.ts @@ -1,6 +1,9 @@ import {MessageHandler} from "./index" -import {booleanFlag, BooleanFlags} from "../config/feature-flags"; - +import app from "../worker/app"; +import {BackfillQueue, processInstallation} from "../sync/installation"; +import * as Sentry from "@sentry/node"; +import AxiosErrorEventDecorator from "../models/axios-error-event-decorator"; +import SentryScopeProxy from "../models/sentry-scope-proxy"; export type BackfillMessagePayload = { installationId: number, @@ -8,10 +11,36 @@ export type BackfillMessagePayload = { startTime?: string } -export const backfillQueueMessageHandler : MessageHandler = async (context) => { - if(!await booleanFlag(BooleanFlags.NEW_BACKFILL_PROCESS_ENABLED, false, context.payload.jiraHost)) { - context.log.info("New backfill process disabled, dropping the message "); - return +type BackfillQueueSupplier = () => Promise; + +export const backfillQueueMessageHandlerFactory: (queueSupplier: BackfillQueueSupplier) => MessageHandler = + (queueSupplier) => async (context,) => { + const sentry = new Sentry.Hub(Sentry.getCurrentHub().getClient()); + sentry.configureScope((scope) => + scope.addEventProcessor(AxiosErrorEventDecorator.decorate) + ); + sentry.configureScope((scope) => + scope.addEventProcessor(SentryScopeProxy.processEvent) + ); + + try { + const processor = await processInstallation(app, queueSupplier); + await processor({ + data: context.payload, + sentry: sentry + }, context.log); + } catch (err) { + sentry.setExtra("job", { + id: context.message.MessageId, + attemptsMade: parseInt(context.message.Attributes?.ApproximateReceiveCount || "1"), + timestamp: new Date(), + data: context.payload + }); + + sentry.setTag("jiraHost", context.payload.jiraHost); + sentry.setTag("queue", "sqs-backfill"); + sentry.captureException(err); + + throw err; + } } - context.log.info("Porcessing backfill message"); -} diff --git a/src/sqs/error-handlers.ts b/src/sqs/error-handlers.ts new file mode 100644 index 0000000000..be0f377b05 --- /dev/null +++ b/src/sqs/error-handlers.ts @@ -0,0 +1,75 @@ +import {Context, ErrorHandler, ErrorHandlingResult} from "./index"; +import {JiraClientError} from "../jira/client/axios"; +import {Octokit} from "@octokit/rest"; +import {RateLimitingError as OldRateLimitingError} from "../config/enhance-octokit"; +import {emitWebhookFailedMetrics} from "../util/webhooks"; +import {PushQueueMessagePayload} from "./push"; +import {RateLimitingError} from "../github/client/errors"; + +/** + * Sometimes we can get errors from Jira and GitHub which does not indicate a failured webhook. For example: + * Jira site is gone, we'll get 404 + * GitHub App is not installed anymore we'll get 401 + * etc. + * + * In such cases webhook processing doesn't make sense anymore and we need to silently discard these errors + */ +const UNRETRYABLE_STATUS_CODES = [401, 404, 403]; + +const RATE_LIMITING_DELAY_BUFFER_SEC = 10; +const EXPONENTIAL_BACKOFF_BASE_SEC = 60; +const EXPONENTIAL_BACKOFF_MULTIPLIER = 3; + +export const jiraOctokitErrorHandler : ErrorHandler = async (error: JiraClientError | Octokit.HookError | OldRateLimitingError | RateLimitingError | Error, + context: Context) : Promise => { + + const maybeResult = maybeHandleNonFailureCase(error, context); + if (maybeResult) { + return maybeResult; + } + + const errorHandlingResult = handleFailureCase(error, context); + + if (!errorHandlingResult.retryable || context.lastAttempt ) { + context.log.error({error}, "Webhook push processing failed and won't be retried anymore"); + emitWebhookFailedMetrics("push") + } + + return errorHandlingResult; +} + +function maybeHandleNonFailureCase(error: Error, context: Context): ErrorHandlingResult | undefined { + if (error instanceof JiraClientError && + error.status && + UNRETRYABLE_STATUS_CODES.includes(error.status)) { + context.log.warn(`Received ${error.status} from Jira. Unretryable. Discarding the message`); + return {retryable: false} + } + + //If error is Octokit.HookError, then we need to check the response status + //Unfortunately we can't check if error is instance of Octokit.HookError because it is not a calss, so we'll just rely on status + //TODO Add error handling for the new GitHub client when it will be done + const maybeErrorWithStatus : any = error; + if (maybeErrorWithStatus.status && UNRETRYABLE_STATUS_CODES.includes(maybeErrorWithStatus.status)) { + context.log.warn({err: maybeErrorWithStatus}, `Received error with ${maybeErrorWithStatus.status} status. Unretryable. Discarding the message`); + return {retryable: false} + } + + return undefined; +} + +function handleFailureCase(error: Error, context: Context): ErrorHandlingResult { + if (error instanceof OldRateLimitingError) { + const delaySec = error.rateLimitReset + RATE_LIMITING_DELAY_BUFFER_SEC - (new Date().getTime() / 1000); + return {retryable: true, retryDelaySec: delaySec} + } + + if (error instanceof RateLimitingError) { + const delaySec = error.rateLimitReset + RATE_LIMITING_DELAY_BUFFER_SEC - (new Date().getTime() / 1000); + return {retryable: true, retryDelaySec: delaySec} + } + + //In case if error is unknown we should use exponential backoff + const delaySec = EXPONENTIAL_BACKOFF_BASE_SEC * Math.pow(EXPONENTIAL_BACKOFF_MULTIPLIER, context.receiveCount); + return {retryable: true, retryDelaySec: delaySec} +} diff --git a/src/sqs/index.ts b/src/sqs/index.ts index e9131cf183..4d3b516efd 100644 --- a/src/sqs/index.ts +++ b/src/sqs/index.ts @@ -12,6 +12,7 @@ import { v4 as uuidv4 } from "uuid"; import statsd from "../config/statsd"; import { Tags } from "hot-shots"; import {sqsQueueMetrics} from "../config/metric-names"; +import {LoggerWithTarget} from "probot/lib/wrap-logger"; const logger = getLogger("sqs") @@ -33,7 +34,7 @@ export type Context = { /** * Context logger, which has parameters for the processing context (like message id, execution id, etc) */ - log: Logger; + log: LoggerWithTarget; /** * How many times this messages attempted to be processed, including the current attempt (always greater 0) @@ -131,7 +132,7 @@ type ListenerContext = { /** * Logger which contains listener debug parameters */ - log: Logger; + log: LoggerWithTarget; } const EXTRA_VISIBILITY_TIMEOUT_DELAY = 2; @@ -151,7 +152,7 @@ export class SqsQueue { readonly errorHandler: ErrorHandler; readonly messageHandler: MessageHandler; readonly sqs: SQS; - readonly log: Logger; + readonly log: LoggerWithTarget; readonly metricsTags: Tags; /** diff --git a/src/sqs/push.ts b/src/sqs/push.ts index c6ba2d84b4..23dc396dfc 100644 --- a/src/sqs/push.ts +++ b/src/sqs/push.ts @@ -1,11 +1,9 @@ -import {Context, ErrorHandler, ErrorHandlingResult, MessageHandler} from "./index" -import enhanceOctokit, {RateLimitingError} from "../config/enhance-octokit"; +import {Context, MessageHandler} from "./index" +import enhanceOctokit from "../config/enhance-octokit"; import {processPush} from "../transforms/push"; import app from "../worker/app"; import {wrapLogger} from "probot/lib/wrap-logger"; -import {JiraClientError} from "../jira/client/axios"; -import { Octokit } from "@octokit/rest"; -import {emitWebhookFailedMetrics} from "../util/webhooks"; +import GitHubClient from "../github/client/github-client"; export type PayloadRepository = { id: number, @@ -30,75 +28,14 @@ export const pushQueueMessageHandler : MessageHandler = const payload = context.payload; - let github; + let githubOld; try { - github = await app.auth(payload.installationId); + githubOld = await app.auth(payload.installationId); } catch (err) { context.log.warn({ err, payload }, "Could not authenticate for the supplied InstallationId"); return; } - enhanceOctokit(github); - await processPush(github, payload, wrapLogger(context.log)); -} - -/** - * Sometimes we can get errors from Jira and GitHub which does not indicate a failured webhook. For example: - * Jira site is gone, we'll get 404 - * GitHub App is not installed anymore we'll get 401 - * etc. - * - * In such cases webhook processing doesn't make sense anymore and we need to silently discard these errors - */ -const UNRETRYABLE_STATUS_CODES = [401, 404, 403]; - -const RATE_LIMITING_DELAY_BUFFER_SEC = 10; -const EXPONENTIAL_BACKOFF_BASE_SEC = 60; -const EXPONENTIAL_BACKOFF_MULTIPLIER = 3; -export const pushQueueErrorHandler : ErrorHandler = async (error: JiraClientError | Octokit.HookError | RateLimitingError | Error, - context: Context) : Promise => { - - const maybeResult = maybeHandleNonFailureCase(error, context); - if (maybeResult) { - return maybeResult; - } - - const errorHandlingResult = handleFailureCase(error, context); - - if (!errorHandlingResult.retryable || context.lastAttempt ) { - context.log.error({error}, "Webhook push processing failed and won't be retried anymore"); - emitWebhookFailedMetrics("push") - } - - return errorHandlingResult; -} - -function maybeHandleNonFailureCase(error: Error, context: Context): ErrorHandlingResult | undefined { - if (error instanceof JiraClientError && - error.status && - UNRETRYABLE_STATUS_CODES.includes(error.status)) { - context.log.warn(`Received ${error.status} from Jira. Unretryable. Discarding the message`); - return {retryable: false} - } - - //If error is Octokit.HookError, then we need to check the response status - //Unfortunately we can't check if error is instance of Octokit.HookError because it is not a calss, so we'll just rely on status - //TODO Add error handling for the new GitHub client when it will be done - const maybeErrorWithStatus : any = error; - if (maybeErrorWithStatus.status && UNRETRYABLE_STATUS_CODES.includes(maybeErrorWithStatus.status)) { - context.log.warn({err: maybeErrorWithStatus}, `Received error with ${maybeErrorWithStatus.status} status. Unretryable. Discarding the message`); - return {retryable: false} - } - - return undefined; -} - -function handleFailureCase(error: Error, context: Context): ErrorHandlingResult { - if (error instanceof RateLimitingError) { - const delaySec = error.rateLimitReset + RATE_LIMITING_DELAY_BUFFER_SEC - (new Date().getTime() / 1000); - return {retryable: true, retryDelaySec: delaySec} - } - - //In case if error is unknown we should use exponential backoff - const delaySec = EXPONENTIAL_BACKOFF_BASE_SEC * Math.pow(EXPONENTIAL_BACKOFF_MULTIPLIER, context.receiveCount); - return {retryable: true, retryDelaySec: delaySec} + enhanceOctokit(githubOld); + const github = new GitHubClient(payload.installationId, context.log); + await processPush(githubOld, github, payload, wrapLogger(context.log)); } diff --git a/src/sqs/queues.ts b/src/sqs/queues.ts index 2039cbafe7..4ec0769c91 100644 --- a/src/sqs/queues.ts +++ b/src/sqs/queues.ts @@ -1,7 +1,9 @@ import envVars from "../config/env"; -import {defaultErrorHandler, SqsQueue} from "./index"; -import {BackfillMessagePayload, backfillQueueMessageHandler} from "./backfill"; -import {PushQueueMessagePayload, pushQueueMessageHandler, pushQueueErrorHandler} from "./push"; +import {SqsQueue} from "./index"; +import {BackfillMessagePayload, backfillQueueMessageHandlerFactory} from "./backfill"; +import {PushQueueMessagePayload, pushQueueMessageHandler} from "./push"; +import {jiraOctokitErrorHandler} from './error-handlers'; +import backfillQueueSupplier from "../backfill-queue-supplier"; const LONG_POLLING_INTERVAL_SEC = 3; @@ -13,8 +15,8 @@ const sqsQueues = { timeoutSec: 10*60, maxAttempts: 3 }, - backfillQueueMessageHandler, - defaultErrorHandler + backfillQueueMessageHandlerFactory(() => backfillQueueSupplier.supply()), + jiraOctokitErrorHandler ), push: new SqsQueue({ queueName: "push", @@ -22,21 +24,19 @@ const sqsQueues = { queueRegion: envVars.SQS_PUSH_QUEUE_REGION, longPollingIntervalSec: LONG_POLLING_INTERVAL_SEC, timeoutSec: 60, - maxAttempts: 5}, pushQueueMessageHandler, pushQueueErrorHandler), + maxAttempts: 5}, pushQueueMessageHandler, jiraOctokitErrorHandler), start: () => { - //do nothing - //sqsQueues.backfill.start(); + backfillQueueSupplier.setSQSQueue(sqsQueues.backfill); + sqsQueues.backfill.start(); sqsQueues.push.start(); }, stop: () => { - //do nothing - //sqsQueues.backfill.stop(); + sqsQueues.backfill.stop(); sqsQueues.push.stop(); } } - export default sqsQueues diff --git a/src/sync/discovery.ts b/src/sync/discovery.ts index 9bbe2506ab..800a8a3877 100644 --- a/src/sync/discovery.ts +++ b/src/sync/discovery.ts @@ -4,6 +4,8 @@ import enhanceOctokit from "../config/enhance-octokit"; import { Application } from "probot"; import { Repositories, SyncStatus } from "../models/subscription"; import {LoggerWithTarget} from "probot/lib/wrap-logger"; +import backfillQueueSupplier from '../backfill-queue-supplier'; +import {booleanFlag, BooleanFlags} from "../config/feature-flags"; export const DISCOVERY_LOGGER_NAME = "sync.discovery"; @@ -51,8 +53,12 @@ export const discovery = (app: Application, queues) => async (job, logger: Logge repos }); - // Create job - queues.installation.add({ installationId, jiraHost, startTime }); + if (await booleanFlag(BooleanFlags.USE_SQS_FOR_BACKFILL, false, jiraHost)) { + const backfillQueue = await backfillQueueSupplier.supply(); + await backfillQueue.schedule({installationId, jiraHost, startTime: startTime.toISOString()}, 0, logger); + } else { + await queues.installation.add({ installationId, jiraHost, startTime }); + } } catch (err) { logger.error({ job, err }, "Discovery error"); } diff --git a/src/sync/installation.ts b/src/sync/installation.ts index 0697a44bfe..0cc61f3b38 100644 --- a/src/sync/installation.ts +++ b/src/sync/installation.ts @@ -11,13 +11,14 @@ import getCommits from "./commits"; import { Application, GitHubAPI } from "probot"; import { metricSyncStatus, metricTaskStatus } from "../config/metric-names"; import Queue from "bull"; -import { booleanFlag, BooleanFlags, stringFlag, StringFlags } from "../config/feature-flags"; +import { booleanFlag, BooleanFlags, isBlocked } from "../config/feature-flags"; import { LoggerWithTarget } from "probot/lib/wrap-logger"; import { Deduplicator, DeduplicatorResult, RedisInProgressStorageWithTimeout } from "./deduplicator"; import Redis from "ioredis"; import getRedisInfo from "../config/redis-info"; import GitHubClient from "../github/client/github-client"; import {BackfillMessagePayload} from "../sqs/backfill"; +import {Hub} from "@sentry/types/dist/hub"; export const INSTALLATION_LOGGER_NAME = "sync.installation"; @@ -168,17 +169,6 @@ const updateJobStatus = async ( const getEnhancedGitHub = async (app: Application, installationId) => enhanceOctokit(await app.auth(installationId)); -const isBlocked = async (installationId: number, logger: LoggerWithTarget): Promise => { - try { - const blockedInstallationsString = await stringFlag(StringFlags.BLOCKED_INSTALLATIONS, "[]"); - const blockedInstallations: number[] = JSON.parse(blockedInstallationsString); - return blockedInstallations.includes(installationId); - } catch (e) { - logger.error(e); - return false; - } -}; - /** * Determines if an an error returned by the GitHub API means that we should retry it * with a smaller request (i.e. with fewer pages). @@ -238,7 +228,7 @@ async function doProcessInstallation(app, job, installationId: number, jiraHost: logger ); - const newGithub = new GitHubClient(installationId); + const newGithub = new GitHubClient(installationId, logger); const github = await getEnhancedGitHub(app, installationId); @@ -414,7 +404,6 @@ async function doProcessInstallation(app, job, installationId: number, jiraHost: // Export for unit testing. TODO: consider improving encapsulation by making this logic as part of Deduplicator, if needed export async function maybeScheduleNextTask( - queue: Queue.Queue, backfillQueue: BackfillQueue, jobData: BackfillMessagePayload, nextTaskDelays: Array, @@ -427,31 +416,24 @@ export async function maybeScheduleNextTask( } const delay = nextTaskDelays.shift()!; logger.info("Scheduling next job with a delay = " + delay); - if (await booleanFlag(BooleanFlags.USE_BACKFILL_QUEUE_SUPPLIER, false, jobData.jiraHost)) { - await backfillQueue.schedule(jobData, delay); - } else { - if (delay > 0) { - await queue.add(jobData, {delay}); - } else { - await queue.add(jobData); - } - } + await backfillQueue.schedule(jobData, delay, logger); } } export interface BackfillQueue { - schedule: (message: BackfillMessagePayload, delayMsecs?: number) => Promise; + schedule: (message: BackfillMessagePayload, delayMsecs?: number, logger?: LoggerWithTarget) => Promise; } -// TODO: type queues +const redis = new Redis(getRedisInfo("installations-in-progress")); + export const processInstallation = - (app: Application, queues, backfillQueueSupplier: () => Promise) => { - const inProgressStorage = new RedisInProgressStorageWithTimeout(new Redis(getRedisInfo("installations-in-progress"))); + (app: Application, backfillQueueSupplier: () => Promise) => { + const inProgressStorage = new RedisInProgressStorageWithTimeout(redis); const deduplicator = new Deduplicator( inProgressStorage, 1_000 ); - return async (job, rootLogger: LoggerWithTarget): Promise => { + return async (job: {data: BackfillMessagePayload, sentry: Hub}, rootLogger: LoggerWithTarget): Promise => { const { installationId, jiraHost } = job.data; const logger = rootLogger.child({ job }); @@ -468,7 +450,8 @@ export const processInstallation = const nextTaskDelays: Array = []; - const result = await deduplicator.executeWithDeduplication("i-" + installationId, + const result = await deduplicator.executeWithDeduplication( + "i-" + installationId + "-" + jiraHost, () => doProcessInstallation(app, job, installationId, jiraHost, logger, (delay: number) => nextTaskDelays.push(delay) )); @@ -476,18 +459,15 @@ export const processInstallation = switch (result) { case DeduplicatorResult.E_OK: logger.info("Job was executed by deduplicator"); - maybeScheduleNextTask(queues.installation, await backfillQueueSupplier(), job.data, nextTaskDelays, logger); + maybeScheduleNextTask(await backfillQueueSupplier(), job.data, nextTaskDelays, logger); break; - case DeduplicatorResult.E_NOT_SURE_TRY_AGAIN_LATER: + case DeduplicatorResult.E_NOT_SURE_TRY_AGAIN_LATER: { logger.warn("Possible duplicate job was detected, rescheduling"); - if (await booleanFlag(BooleanFlags.USE_BACKFILL_QUEUE_SUPPLIER, false, jiraHost)) { - const queue = await backfillQueueSupplier(); - await queue.schedule(job.data, 60_000); - } else { - await queues.installation.add(job.data, {delay: 60_000}); - } + const queue = await backfillQueueSupplier(); + await queue.schedule(job.data, 60_000, logger); break; - case DeduplicatorResult.E_OTHER_WORKER_DOING_THIS_JOB: + } + case DeduplicatorResult.E_OTHER_WORKER_DOING_THIS_JOB: { logger.warn("Duplicate job was detected, rescheduling"); // There could be one case where we might be losing the message even if we are sure that another worker is doing the work: // Worker A - doing a long-running task @@ -499,13 +479,10 @@ export const processInstallation = // Always rescheduling should be OK given that only one worker is working on the task right now: even if we // gather enough messages at the end of the queue, they all will be processed very quickly once the sync // is finished. - if (await booleanFlag(BooleanFlags.USE_BACKFILL_QUEUE_SUPPLIER, false, jiraHost)) { - const queue = await backfillQueueSupplier(); - await queue.schedule(job.data, Math.floor(60_000 + 60_000 * Math.random())); - } else { - await queues.installation.add(job.data, {delay: Math.floor(60_000 + 60_000 * Math.random())}); - } + const queue = await backfillQueueSupplier(); + await queue.schedule(job.data, Math.floor(60_000 + 60_000 * Math.random()), logger); break; + } } }; }; diff --git a/src/transforms/deployment.ts b/src/transforms/deployment.ts index faf736d3f2..c7934543a0 100644 --- a/src/transforms/deployment.ts +++ b/src/transforms/deployment.ts @@ -1,7 +1,91 @@ import _ from "lodash"; -import { Context } from "probot/lib/context"; import issueKeyParser from "jira-issue-key-parser"; -import { JiraDeploymentData } from "../interfaces/jira"; +import {JiraDeploymentData} from "../interfaces/jira"; +import {GitHubAPI} from "probot"; +import {WebhookPayloadDeploymentStatus} from "@octokit/webhooks"; +import {LoggerWithTarget} from "probot/lib/wrap-logger"; +import {Octokit} from "@octokit/rest"; +import {booleanFlag, BooleanFlags} from "../config/feature-flags"; +import { compareCommitsBetweenBaseAndHeadBranches } from "./util/githubApiRequests"; + +// https://docs.github.com/en/rest/reference/repos#list-deployments +async function getLastSuccessfulDeployCommitSha( + owner: string, + repoName: string, + github: GitHubAPI, + deployments: Octokit.ReposListDeploymentsResponseItem[], + logger?: LoggerWithTarget +): Promise { + + try { + for (const deployment of deployments) { + // Get each deployment status for this environment so we can have their statuses' ids + const listDeploymentStatusResponse: Octokit.Response = await github.repos.listDeploymentStatuses({ + owner: owner, + repo: repoName, + deployment_id: deployment.id, + per_page: 100 // Default is 30, max we can request is 100 + }); + + // Find the first successful one + const lastSuccessful: Octokit.ReposListDeploymentStatusesResponseItem | undefined = listDeploymentStatusResponse.data.find(deployment => deployment.state === "success"); + if (lastSuccessful !== undefined) { + return deployment.sha; + } + } + } catch (e) { + logger?.error(`Failed to get deployment statuses.`); + } + + + // If there's no successful deployment on the list of deployments that GitHub returned us (max. 100) then we'll return the last one from the array, even if it's a failed one. + return deployments[deployments.length - 1].sha; +} + +async function getCommitMessagesSinceLastSuccessfulDeployment( + owner: string, + repoName: string, + currentDeploySha: string, + currentDeployId: number, + currentDeployEnv: string, + github: GitHubAPI, + logger: LoggerWithTarget +): Promise { + + // Grab the last 10 deployments for this repo + const deployments: Octokit.Response = await github.repos.listDeployments({ + owner: owner, + repo: repoName, + environment: currentDeployEnv, + per_page: 10 // Default is 30, max we can request is 100 + }) + + // Filter per current environment and exclude itself + const filteredDeployments = deployments.data + .filter(deployment => deployment.id !== currentDeployId); + + // If this is the very first successful deployment ever, return nothing because we won't have any commit sha to compare with the current one. + if (!filteredDeployments.length) { + return undefined; + } + + const lastSuccessfullyDeployedCommit = await getLastSuccessfulDeployCommitSha(owner, repoName, github, filteredDeployments, logger); + + const compareCommitsPayload = { + owner: owner, + repo: repoName, + base: lastSuccessfullyDeployedCommit, + head: currentDeploySha + } + + const allCommitMessages = await compareCommitsBetweenBaseAndHeadBranches( + compareCommitsPayload, + github, + logger + ); + + return allCommitMessages; +} // We need to map the state of a GitHub deployment back to a valid deployment state in Jira. // https://docs.github.com/en/rest/reference/repos#list-deployments @@ -62,10 +146,32 @@ export function mapEnvironment(environment: string): string { return jiraEnv; } -export default async (context: Context): Promise => { - const { github, payload: { deployment_status, deployment } } = context; - const { data: { commit: { message } } } = await github.repos.getCommit(context.repo({ ref: deployment.sha })); - const issueKeys = issueKeyParser().parse(`${deployment.ref}\n${message}`) || []; +export default async (githubClient: GitHubAPI, payload: WebhookPayloadDeploymentStatus, jiraHost: string, logger: LoggerWithTarget): Promise => { + const deployment = payload.deployment; + const deployment_status = payload.deployment_status; + + const {data: {commit: {message}}} = await githubClient.repos.getCommit({ + owner: payload.repository.owner.login, + repo: payload.repository.name, + ref: deployment.sha + }); + + let issueKeys; + if (await booleanFlag(BooleanFlags.SUPPORT_BRANCH_AND_MERGE_WORKFLOWS_FOR_DEPLOYMENTS, false, jiraHost)) { + const allCommitsMessages = await getCommitMessagesSinceLastSuccessfulDeployment( + payload.repository.owner.login, + payload.repository.name, + deployment.sha, + deployment.id, + deployment_status.environment, + githubClient, + logger + ); + + issueKeys = issueKeyParser().parse(`${deployment.ref}\n${message}\n${allCommitsMessages}`) || []; + } else { + issueKeys = issueKeyParser().parse(`${deployment.ref}\n${message}`) || []; + } if (_.isEmpty(issueKeys)) { return undefined; @@ -73,7 +179,7 @@ export default async (context: Context): Promise const environment = mapEnvironment(deployment_status.environment); if (environment === "unmapped") { - context.log(`Unmapped environment detected for deployment. Unmapped value is ${deployment_status}. Sending it as unmapped to Jira.`); + logger?.info(`Unmapped environment detected for deployment. Unmapped value is ${deployment_status}. Sending it as unmapped to Jira.`); } return { @@ -83,14 +189,14 @@ export default async (context: Context): Promise updateSequenceNumber: deployment_status.id, issueKeys, displayName: deployment.task, - url: deployment_status.log_url || deployment_status.target_url || deployment.url, + url: deployment_status.target_url || deployment.url, description: deployment.description || deployment_status.description || deployment.task, - lastUpdated: deployment_status.updated_at, + lastUpdated: new Date(deployment_status.updated_at), state: mapState(deployment_status.state), pipeline: { id: deployment.task, displayName: deployment.task, - url: deployment_status.log_url || deployment_status.target_url || deployment.url, + url: deployment_status.target_url || deployment.url, }, environment: { id: deployment_status.environment, diff --git a/src/transforms/push.ts b/src/transforms/push.ts index 368ec1a04b..afeaaf25ab 100644 --- a/src/transforms/push.ts +++ b/src/transforms/push.ts @@ -10,9 +10,10 @@ import {JiraCommit} from "../interfaces/jira"; import _ from "lodash"; import {queues} from "../worker/queues"; import {LoggerWithTarget} from "probot/lib/wrap-logger"; -import {booleanFlag, BooleanFlags} from "../config/feature-flags"; +import {booleanFlag, BooleanFlags, isBlocked} from "../config/feature-flags"; import sqsQueues from "../sqs/queues"; import {PushQueueMessagePayload} from "../sqs/push"; +import GitHubClient from "../github/client/github-client"; // TODO: define better types for this file @@ -93,19 +94,21 @@ export async function enqueuePush( export function processPushJob(app: Application) { return async (job: Job, logger: LoggerWithTarget): Promise => { - let github; + let githubOld; try { - github = await app.auth(job.data.installationId); + githubOld = await app.auth(job.data.installationId); } catch (err) { logger.error({ err, job }, "Could not authenticate"); return; } - enhanceOctokit(github); - await processPush(github, job.data, logger); + enhanceOctokit(githubOld); + + const github = new GitHubClient(job.data.installationId, logger); + await processPush(githubOld, github, job.data, logger); }; } -export const processPush = async (github: GitHubAPI, payload, rootLogger: LoggerWithTarget) => { +export const processPush = async (githubOld: GitHubAPI, github: GitHubClient, payload, rootLogger: LoggerWithTarget) => { const { repository, repository: { owner, name: repo }, @@ -114,6 +117,11 @@ export const processPush = async (github: GitHubAPI, payload, rootLogger: Logger jiraHost, } = payload; + if (await isBlocked(installationId, rootLogger)) { + rootLogger.warn({ payload, installationId }, "blocking processing of push message because installationId is on the blocklist"); + return; + } + const webhookId = payload.webhookId || "none"; const webhookReceived = payload.webhookReceived || undefined; @@ -121,88 +129,113 @@ export const processPush = async (github: GitHubAPI, payload, rootLogger: Logger webhookId: webhookId, repoName: repo, orgName: owner.name, + installationId, webhookReceived, }); - log.info({ installationId }, "Processing push"); + log.info("Processing push"); - const subscription = await Subscription.getSingleInstallation( - jiraHost, - installationId - ); + try { + const subscription = await Subscription.getSingleInstallation( + jiraHost, + installationId + ); - if (!subscription) return; + if (!subscription) { + log.info("No subscription was found, stop processing the push"); + return; + } - const jiraClient = await getJiraClient( - subscription.jiraHost, - installationId, - log - ); - - const commits: JiraCommit[] = await Promise.all( - shas.map(async (sha): Promise => { - const { - data, - data: { commit: githubCommit }, - } = await github.repos.getCommit({ - owner: owner.login, - repo, - ref: sha.id, - }); - - const { files, author, parents, sha: commitSha, html_url } = data; - - const { author: githubCommitAuthor, message } = githubCommit; - - // Jira only accepts a max of 10 files for each commit, so don't send all of them - const filesToSend = files.slice(0, 10); - - // merge commits will have 2 or more parents, depending how many are in the sequence - const isMergeCommit = parents?.length > 1; - - return { - hash: commitSha, - message, - author: getJiraAuthor(author, githubCommitAuthor), - authorTimestamp: githubCommitAuthor.date, - displayId: commitSha.substring(0, 6), - fileCount: files.length, // Send the total count for all files - files: filesToSend.map((file) => - mapFile(file, repo, owner.name, sha.id) - ), - id: commitSha, - issueKeys: sha.issueKeys, - url: html_url, - updateSequenceId: Date.now(), - flags: isMergeCommit ? ["MERGE_COMMIT"] : undefined, - }; - }) - ); + const jiraClient = await getJiraClient( + subscription.jiraHost, + installationId, + log + ); - // Jira accepts up to 400 commits per request - // break the array up into chunks of 400 - const chunks: JiraCommit[][] = []; + const commits: JiraCommit[] = await Promise.all( + shas.map(async (sha): Promise => { + log.info("Calling GitHub to fetch commit info " + sha.id); + const useNewGithubClient = await booleanFlag(BooleanFlags.USE_NEW_GITHUB_CLIENT_FOR_PUSH, false, subscription.jiraHost); + try { + const { + data, + data: {commit: githubCommit}, + } = useNewGithubClient + ? await github.getCommit(owner.login, repo, sha.id) + : await githubOld.repos.getCommit({ + owner: owner.login, + repo, + ref: sha.id, + }); + + const {files, author, parents, sha: commitSha, html_url} = data; + + const {author: githubCommitAuthor, message} = githubCommit; + + // Jira only accepts a max of 10 files for each commit, so don't send all of them + const filesToSend = files.slice(0, 10); + + // merge commits will have 2 or more parents, depending how many are in the sequence + const isMergeCommit = parents?.length > 1; + + console.info("GitHub call succeeded"); + return { + hash: commitSha, + message, + author: getJiraAuthor(author, githubCommitAuthor), + authorTimestamp: githubCommitAuthor.date, + displayId: commitSha.substring(0, 6), + fileCount: files.length, // Send the total count for all files + files: filesToSend.map((file) => + mapFile(file, repo, owner.name, sha.id) + ), + id: commitSha, + issueKeys: sha.issueKeys, + url: html_url, + updateSequenceId: Date.now(), + flags: isMergeCommit ? ["MERGE_COMMIT"] : undefined, + } + } catch (err) { + console.warn({ err },"Failed to fetch data from GitHub"); + throw err; + } + }) + ); - while (commits.length) { - chunks.push(commits.splice(0, 400)); - } + // Jira accepts up to 400 commits per request + // break the array up into chunks of 400 + const chunks: JiraCommit[][] = []; - for (const chunk of chunks) { - const jiraPayload = { - name: repository.name, - url: repository.html_url, - id: repository.id, - commits: chunk, - updateSequenceId: Date.now(), - }; - - const jiraResponse = await jiraClient.devinfo.repository.update(jiraPayload); - - webhookReceived && emitWebhookProcessedMetrics( - webhookReceived, - "push", - log, - jiraResponse?.status - ); + while (commits.length) { + chunks.push(commits.splice(0, 400)); + } + + for (const chunk of chunks) { + const jiraPayload = { + name: repository.name, + url: repository.html_url, + id: repository.id, + commits: chunk, + updateSequenceId: Date.now(), + }; + + log.info("Sending data to Jira"); + try { + const jiraResponse = await jiraClient.devinfo.repository.update(jiraPayload); + + webhookReceived && emitWebhookProcessedMetrics( + webhookReceived, + "push", + log, + jiraResponse?.status + ); + } catch (err) { + log.warn({ err }, "Failed to send data to Jira"); + throw err; + } + } + log.info("Push has succeeded"); + } catch (err) { + log.warn({ err }, "Push has failed"); } }; diff --git a/src/transforms/util/githubApiRequests.ts b/src/transforms/util/githubApiRequests.ts new file mode 100644 index 0000000000..18144fd2e6 --- /dev/null +++ b/src/transforms/util/githubApiRequests.ts @@ -0,0 +1,31 @@ +import { GitHubAPI } from "probot"; +import { LoggerWithTarget } from "probot/lib/wrap-logger"; + +interface CompareCommitsPayload { + owner: string; + repo: string; + base: string; + head: string; +} + +// Used to compare commits for builds and deployments so we can +// obtain all issue keys referenced in commit messages. +export const compareCommitsBetweenBaseAndHeadBranches = async ( + payload: CompareCommitsPayload, + github: GitHubAPI, + logger: LoggerWithTarget +): Promise => { + try { + const commitsDiff = await github.repos.compareCommits(payload); + const allCommitMessagesFromPullRequest = commitsDiff.data?.commits + ?.map((c) => c.commit.message) + .join(" "); + + return allCommitMessagesFromPullRequest; + } catch (err) { + logger?.error( + { err, repo: payload.repo }, + "Failed to compare commits on repo." + ); + } +}; diff --git a/src/transforms/workflow.ts b/src/transforms/workflow.ts index fd1fc01535..1c94d9fbd4 100644 --- a/src/transforms/workflow.ts +++ b/src/transforms/workflow.ts @@ -1,7 +1,11 @@ import issueKeyParser from "jira-issue-key-parser"; -import { Context } from "probot/lib/context"; +import { LoggerWithTarget } from "probot/lib/wrap-logger"; import { GitHubPullRequest } from "../interfaces/github"; import { JiraBuildData, JiraPullRequest } from "../interfaces/jira"; +import { GitHubAPI } from "probot"; +import { compareCommitsBetweenBaseAndHeadBranches } from "./util/githubApiRequests"; +import { booleanFlag, BooleanFlags } from "../config/feature-flags"; +import { WorkflowPayload } from "../config/interfaces"; // We need to map the status and conclusion of a GitHub workflow back to a valid build state in Jira. // https://docs.github.com/en/rest/reference/actions#list-workflow-runs-for-a-repository @@ -33,42 +37,93 @@ function mapStatus(status: string, conclusion?: string): string { } } -function mapPullRequests(pull_requests: GitHubPullRequest[]): JiraPullRequest[] { - return pull_requests.map(pr => ( - { - commit: { - id: pr.head.sha, - repositoryUri: pr.head.repo.url, - }, - ref: { - name: pr.head.ref, - uri: `${pr.head.repo.url}/tree/${pr.head.ref}`, - }, - } - )); +function mapPullRequests( + pull_requests: GitHubPullRequest[] +): JiraPullRequest[] { + return pull_requests.map((pr) => ({ + commit: { + id: pr.head.sha, + repositoryUri: pr.head.repo.url, + }, + ref: { + name: pr.head.ref, + uri: `${pr.head.repo.url}/tree/${pr.head.ref}`, + }, + })); } -export default (context: Context): JiraBuildData | undefined => { - const { workflow_run, workflow } = context.payload; - const issueKeys = issueKeyParser().parse(`${workflow_run.head_branch}\n${workflow_run.head_commit.message}`); +export default async ( + githubClient: GitHubAPI, + payload: WorkflowPayload, + jiraHost: string, + logger: LoggerWithTarget +): Promise => { + const { workflow_run, workflow } = payload; + + const { + conclusion, + head_branch, + head_commit, + html_url, + name, + pull_requests, + repository, + run_number, + status, + updated_at, + } = workflow_run; + + let issueKeys; + + const supportBranchAndMergeWorkflowForBuildsFlagIsOn = await booleanFlag(BooleanFlags.SUPPORT_BRANCH_AND_MERGE_WORKFLOWS_FOR_BUILDS, false, jiraHost); + const workflowHasPullRequest = pull_requests.length > 0; + + if (supportBranchAndMergeWorkflowForBuildsFlagIsOn && workflowHasPullRequest) { + const { owner, name: repoName } = repository; + const { base, head } = pull_requests[0]; + + const compareCommitsPayload = { + owner: owner.login, + repo: repoName, + base: base.ref, + head: head.ref + } + + const allCommitMessages = await compareCommitsBetweenBaseAndHeadBranches( + compareCommitsPayload, + githubClient, + logger + ); + + issueKeys = issueKeyParser().parse(`${head_branch}\n${head_commit.message}\n${allCommitMessages}`) || []; + } else { + issueKeys = + issueKeyParser().parse(`${head_branch}\n${head_commit.message}`) || []; + } if (!issueKeys) { return undefined; } + const maxPullRequestReferences = mapPullRequests(pull_requests).slice(0, 5); + return { product: "GitHub Actions", - builds: [{ - schemaVersion: "1.0", - pipelineId: workflow.id, - buildNumber: workflow_run.run_number, - updateSequenceNumber: Date.now(), - displayName: workflow_run.name, - url: workflow_run.html_url, - state: mapStatus(workflow_run.status, workflow_run.conclusion), - lastUpdated: workflow_run.updated_at, - issueKeys, - references: workflow_run.pull_requests?.length ? mapPullRequests(workflow_run.pull_requests).slice(0, 5) : undefined // Optional information that links PRs. Min items: 1, Max items: 5 - }], + builds: [ + { + schemaVersion: "1.0", + pipelineId: workflow.id, + buildNumber: run_number, + updateSequenceNumber: Date.now(), + displayName: name, + url: html_url, + state: mapStatus(status, conclusion), + lastUpdated: updated_at, + issueKeys, + references: workflowHasPullRequest + ? maxPullRequestReferences + : undefined, // Optional information that links PRs. + }, + ], }; }; diff --git a/src/util/axios/common-middleware.ts b/src/util/axios/common-middleware.ts new file mode 100644 index 0000000000..ce536abd18 --- /dev/null +++ b/src/util/axios/common-middleware.ts @@ -0,0 +1,34 @@ +import url from "url"; + +/** + * Enrich the Axios Request Config with a URL object. + * TODO: non-standard and probably should be done through string interpolation + * + * @param {import("axios").AxiosRequestConfig} config - The outgoing request configuration. + * @returns {import("axios").AxiosRequestConfig} The enriched axios request config. + */ +export const urlParamsMiddleware = (config) => { + // eslint-disable-next-line prefer-const + let { query, pathname, ...rest } = url.parse(config.url, true); + config.urlParams = config.urlParams || {}; + + for (const param in config.urlParams) { + if (pathname?.includes(`:${param}`)) { + pathname = pathname.replace(`:${param}`, config.urlParams[param]); + } else { + query[param] = config.urlParams[param]; + } + } + + config.urlParams.baseUrl = config.baseURL; + + return { + ...config, + originalUrl: config.url, + url: url.format({ + ...rest, + pathname, + query + }) + }; +} diff --git a/src/util/getUrl.ts b/src/util/getUrl.ts index 7044a1c391..42df6cfa29 100644 --- a/src/util/getUrl.ts +++ b/src/util/getUrl.ts @@ -1,26 +1,9 @@ - -import Logger from "bunyan"; -import url from "url"; +import envVars from "../config/env"; export const getGitHubConfigurationUrl = ( - githubHost, - jwt, jiraHost ): string => - `https://${githubHost}/github/configuration?jwt=${jwt}&xdm_e=${jiraHost}`; + `https://${jiraHost}/plugins/servlet/ac/com.github.integration.${envVars.INSTANCE_NAME}/github-post-install-page`; export const getJiraMarketplaceUrl = (jiraHost: string): string => `${jiraHost}/plugins/servlet/ac/com.atlassian.jira.emcee/discover#!/discover/app/com.github.integration.production`; - -export const getJiraHostFromRedirectUrl = (urlOrPath: string, log: Logger): string => { - if (!urlOrPath) { - return "empty"; - } - try { - const { host, query } = url.parse(urlOrPath, true); - return (query?.xdm_e ? url.parse(query.xdm_e.toString(), false).host : host) || "unknown"; - } catch (err) { - log.error(err, "Cannot detect jiraHost from redirect URL"); - return "error"; - } -}; diff --git a/src/worker/startup.ts b/src/worker/startup.ts index 9d50b8705c..c31efb1741 100644 --- a/src/worker/startup.ts +++ b/src/worker/startup.ts @@ -16,6 +16,8 @@ import AxiosErrorEventDecorator from "../models/axios-error-event-decorator"; import SentryScopeProxy from "../models/sentry-scope-proxy"; import { getLogger } from "../config/logger"; import { v4 as uuidv4 } from 'uuid' +import backfillSupplier from '../backfill-queue-supplier'; +import { isNodeProd } from "../util/isNodeEnv"; const CONCURRENT_WORKERS = process.env.CONCURRENT_WORKERS || 1; const logger = getLogger("worker"); @@ -92,6 +94,11 @@ const setDelayOnRateLimiting = (jobHandler) => async (job: Job, logger: LoggerWi }; const sendQueueMetrics = async () => { + // Only send queue metrics in prod + if (!isNodeProd()) { + return; + } + for (const [queueName, queue] of Object.entries(queues)) { logger.info("fetching queue metrics"); @@ -123,22 +130,13 @@ export async function start() { // exposing queue metrics at a regular interval timer = setInterval(sendQueueMetrics, 60000); + backfillSupplier.setRedisQueue(queues.installation); + // Start processing queues queues.discovery.process(5, commonMiddleware(discovery(app, queues), DISCOVERY_LOGGER_NAME)); queues.installation.process( Number(CONCURRENT_WORKERS), - commonMiddleware(processInstallation(app, queues, () => { - return Promise.resolve({ - schedule: async (jobData, delayMsecs) => { - // TBD: switch to SQS with a FF - if (delayMsecs) { - await queues.installation.add(jobData, {delay: delayMsecs}); - } else { - await queues.installation.add(jobData); - } - } - }) - }), INSTALLATION_LOGGER_NAME) + commonMiddleware(processInstallation(app, () => backfillSupplier.supply()), INSTALLATION_LOGGER_NAME) ); queues.push.process( Number(CONCURRENT_WORKERS), diff --git a/static/css/github-error.css b/static/css/error.css similarity index 66% rename from static/css/github-error.css rename to static/css/error.css index 05e6a51889..ece59a7958 100644 --- a/static/css/github-error.css +++ b/static/css/error.css @@ -1,4 +1,4 @@ -.githubError__container { +.error-container { align-items: center; display: flex; flex-direction: column; @@ -6,27 +6,27 @@ justify-content: center; margin: auto; max-width: 570px; - } +} - .githubError__errorImg { +.error-errorImg { margin-bottom: 2em; width: 11.5em; - } +} - .githubError__header { +.error-header { /* Override build.css */ font-size: 1.3rem !important; - } +} - .githubError__message { +.error-message { margin: 1em auto; text-align: center; - } +} - .githubError__link { +a { color: #0052cc; - } +} - .githubError__link:hover { +a:hover { text-decoration: none; - } +} diff --git a/static/css/github-configuration-OLD.css b/static/css/github-configuration-OLD.css deleted file mode 100644 index a2e237f8ef..0000000000 --- a/static/css/github-configuration-OLD.css +++ /dev/null @@ -1,186 +0,0 @@ -.getConfiguration__logout__container { - align-items: flex-end; - display: flex; - flex-direction: column; -} - -.getConfiguration__logout__link { - cursor: pointer; - color: #0366d6; -} - -.getConfiguration__logout__name { - font-weight: bold; -} - -.getConfiguration { - margin: auto; - max-width: 740px; - height: 100vh; -} - -.getConfiguration__header { - font-weight: 500 !important; -} - -.getConfiguration__wrapper { - display: flex; - flex-direction: column; - flex-wrap: wrap; - line-height: 1; - margin: 2em auto; -} - -.getConfiguration__tableHeader { - color: #6b778c; - display: flex; - flex-direction: row; - font-size: 0.8rem; - padding: 1em 0 0; -} - -.getConfiguration__tableHeader__label { - margin-bottom: 0; -} - -.Box { - border: 0; - border-radius: 0; -} - -.githubOrgs__table__row { - border-top: 0; - line-height: 0; -} - -.githubOrgs__table__row:not(:last-child) { - padding-bottom: 0.8em; -} - -.githubOrgs__table__row, -.getConfiguration__orgContent:nth-child(2) { - display: flex; - flex-direction: row; -} - -.getConfiguration__orgContent, -.getConfiguration__orgContent__account { - align-items: center; - color: #213658; - display: flex; -} - -.getConfiguration__orgContent:last-child { - display: flex; - justify-content: flex-end; -} - -.getConfiguration__orgContent__account { - flex: 2; -} - -.getConfiguration__orgContent__avatar { - margin-right: 0.5em; - width: 25px; -} - -.getConfiguration__orgContent__repoSelection { - margin: 0; -} - -.getConfiguration__orgContent__numberOfRepos { - background-color: #f4f5f7; - border-radius: 12px; - margin: auto 0.5em; - padding: 0.7em 0.6em; -} - -.getConfiguration__orgContent__connectBtn, -.getConfiguration__orgContent__connectBtn--disabled { - border: 0; - border-radius: 3px; - color: #42526e; -} - -.getConfiguration__orgContent__connectBtn { - background-color: #f4f5f7; - padding: 1em 0.8em; -} - -.getConfiguration__orgContent__connectedMsg { - font-style: italic; - margin: 0; -} - -.getConfiguration__orgContent__connectBtn:hover, -.getConfiguration__orgContent__connectBtn:focus, -.getConfiguration__orgContent__connectBtn:active, -.getConfiguration__orgContent__connectBtn:focus:active, -.getConfiguration__orgContent__connectBtn:hover:active { - background-color: #ebecf0; -} - -.getConfiguration__loaderContainer { - display: flex; - justify-content: center; - width: 5.5em; -} - -.getConfiguration__loader { - animation: spin 2s linear infinite; - border: 2px solid #f3f3f3; - border-radius: 50%; - border-top: 2px solid #42526e; - height: 1.8em; - width: 1.8em; -} - -.getConfiguration__noOrganizations { - color: #172b4d; -} - -.getConfiguration__connectNewOrg { - color: #172b4d; - font-weight: 500; - padding: 0.8em 0; -} - -.getConfiguration__connectNewOrg__plus { - font-size: 1.1rem; - padding: 0 0.7em 0 0.3em; - transform: scale(1.5); -} - -.getConfiguration__connectNewOrg:hover, -.getConfiguration__connectNewOrg:focus, -.getConfiguration__connectNewOrg:active { - background-color: #f5f6f7; - text-decoration: none; -} - -@keyframes spin { - 0% { - transform: rotate(0deg); - } - 100% { - transform: rotate(360deg); - } -} - -/* Media Queries */ -@media (max-width: 540px) { - .getConfiguration__wrapper, - .getConfiguration__accountLink, - .getConfiguration__disclaimer { - font-size: 0.8rem; - } - - .getConfiguration__tableHeader { - padding-bottom: 0; - } - - .getConfiguration__loader { - height: 1.8em; - width: 1.8em; - } -} diff --git a/static/css/global-OLD.css b/static/css/global-OLD.css deleted file mode 100644 index d9248e57ec..0000000000 --- a/static/css/global-OLD.css +++ /dev/null @@ -1,106 +0,0 @@ -body { - overflow: hidden; -} - -p { - margin-bottom: 0; -} - -.headerImage { - margin: 2em auto 1em; - text-align: center; -} - -.headerText { - font-size: 1.5rem; - text-align: center; -} - -.jiraInstance { - color: #586069; - margin-bottom: 0; - text-align: center; -} - -.actionBtn, -.actionBtn--empty { - border: 0; - border-radius: 3px; - min-width: 5em; - padding: 0.5em 0.8em; -} - -.actionBtn { - background-color: #f4f5f7; - color: #42526e; -} - -.actionBtn--empty { - background-color: #0052cc; - color: #fff; -} - -.actionBtn:hover, -.actionBtn:focus, -.actionBtn:active, -.actionBtn:focus:active, -.actionBtn:hover:active { - background-color: #ebecf0; -} - -.tableHeader { - color: #6b778c; - display: flex; - flex-direction: row; - font-size: 0.8rem; - padding: 1em 0 0; -} - -.tableHeader__label { - margin-bottom: 0; -} - -.org__cell:not(.getConfiguration__orgContent__account) { - flex: 1; -} - -.tableHeader__label { - flex: 2; -} - -.githubOrgs__table { - border-top: 2px solid #e0e2e7; - border-bottom: 2px solid #e0e2e7; - padding: 0.8em 0; -} - -p { - margin: 0; -} - -@media (max-width: 540px) { - .getConfiguration__tableHeader { - padding-bottom: 0; - } -} - -.githubConfiguration__logout__container, -.gitHubSetup__getJira__container { - align-items: flex-end; - display: flex; - flex-direction: column; -} - -.githubConfiguration__logout__link, -.gitHubSetup__getJira__link { - cursor: pointer; - color: #0366d6; -} - -.githubConfiguration__logout__name { - font-weight: bold; -} - -.githubConfiguration__header { - font-weight: 500 !important; -} diff --git a/static/css/jira-configuration-OLD.css b/static/css/jira-configuration-OLD.css deleted file mode 100644 index 0da8fdbaa8..0000000000 --- a/static/css/jira-configuration-OLD.css +++ /dev/null @@ -1,59 +0,0 @@ -.jiraConfigurationTable { - table-layout: fixed; - width: 100%; -} - -.jiraConfigurationTable__connectionLinks > :first-child { - padding-left: 0; -} - -.jiraConfigurationTable__syncStatus { - border-radius: 5px; - font-weight: bold; - padding: 0.2em 0.3em; -} - -.jiraConfigurationTable__pending { - background-color: #DFE1E6; - color: #42526E; -} - -.jiraConfigurationTable__in-progress { - background-color: #DEEBFF; - color: #0747A6; -} - -.jiraConfigurationTable__finished { - background-color: #E3FCEF; - color: #006644; -} - -.jiraConfigurationTable__failed { - background-color: #FFEBE6; - color: #BF2600; -} - -.jiraConfigurationTable__refresh { - font-weight: bold; -} - -.jiraConfiguration__empty { - margin: auto; - max-width: 570px; - padding: 6.5em 0; - text-align: center; -} - -.jiraConfiguration__empty__header { - font-size: 1.6rem; -} - -.jiraConfiguration__empty__message { - font-size: 0.9rem; - margin: 2em auto; -} - -.jiraConfiguration__empty__image { - width: 100%; - max-width: 332px; -} diff --git a/static/js/jira-configuration.js b/static/js/jira-configuration.js index f1a679266e..00a0785c02 100644 --- a/static/js/jira-configuration.js +++ b/static/js/jira-configuration.js @@ -1,66 +1,64 @@ /* globals $, AP */ -const params = new URLSearchParams(window.location.search.substring(1)) -const appUrl = document.querySelector('meta[name=public-url]').getAttribute('content') - -$('.add-organization-link').click(function (event) { - event.preventDefault() - - const child = window.open(`${appUrl}/github/login`) - - const interval = setInterval(function () { - if (child.closed) { - clearInterval(interval) - - AP.navigator.reload() - } - }, 100) -}) - -$('.configure-connection-link').click(function (event) { - event.preventDefault() - - const installationLink = $(event.target).data('installation-link') - const child = window.open(installationLink) +const params = new URLSearchParams(window.location.search.substring(1)); +const jiraHost = params.get("xdm_e"); + +function openChildWindow(url) { + const child = window.open(url); + const interval = setInterval(function () { + if (child.closed) { + clearInterval(interval); + AP.navigator.reload(); + } + }, 100); + + return child; +} - const interval = setInterval(function () { - if (child.closed) { - clearInterval(interval) +$(".add-organization-link").click(function(event) { + event.preventDefault(); + window.AP.context.getToken(function(token) { + const child = openChildWindow("/session/github/configuration"); + child.window.jiraHost = jiraHost; + }); +}); - AP.navigator.reload() - } - }, 100) -}) +$(".configure-connection-link").click(function(event) { + event.preventDefault(); + openChildWindow($(event.target).data("installation-link")); +}); -$('.delete-connection-link').click(function (event) { - event.preventDefault() +$(".delete-connection-link").click(function(event) { + event.preventDefault(); - window.AP.context.getToken(function(token){ + window.AP.context.getToken(function(token) { $.ajax({ - type: 'DELETE', - url: `/jira/configuration?xdm_e=${encodeURIComponent(params.get('xdm_e'))}`, + type: "DELETE", + url: "/jira/configuration", data: { - installationId: $(event.target).data('installation-id'), - jwt: token + installationId: $(event.target).data("installation-id"), + jwt: token, + jiraHost: jiraHost }, - success: function (data) { - AP.navigator.reload() + success: function(data) { + AP.navigator.reload(); } }); }); -}) +}); -$('.sync-connection-link-OLD').click(function (event) { - event.preventDefault() - const installationId = $(event.target).data('installation-id') +$(".sync-connection-link-OLD").click(function(event) { + event.preventDefault(); + const installationId = $(event.target).data("installation-id"); const jiraHost = $(event.target).data("jira-host"); const csrfToken = document.getElementById("_csrf").value; - $("#restart-backfill").prop("disabled", true); - $("#restart-backfill").attr("aria-disabled", "true"); + const $el = $("#restart-backfill"); + $el.prop("disabled", true); + $el.attr("aria-disabled", "true"); - window.AP.context.getToken(function(token){ + window.AP.context.getToken(function(token) { $.ajax({ - type: 'POST', + type: "POST", url: `/jira/sync`, data: { installationId: installationId, @@ -69,28 +67,28 @@ $('.sync-connection-link-OLD').click(function (event) { jwt: token, _csrf: csrfToken }, - success: function (data) { - AP.navigator.reload() + success: function(data) { + AP.navigator.reload(); }, - error: function (error) { - console.log(error); - $("#restart-backfill").prop("disabled", false); - $("#restart-backfill").attr("aria-disabled", "false"); + error: function(error) { + $el.prop("disabled", false); + $el.attr("aria-disabled", "false"); } }); }); -}) +}); -$(".sync-connection-link").click(function (event) { +$(".sync-connection-link").click(function(event) { event.preventDefault(); const installationId = $(event.target).data("installation-id"); const jiraHost = $(event.target).data("jira-host"); const csrfToken = document.getElementById("_csrf").value; - $("#restart-backfill").prop("disabled", true); - $("#restart-backfill").attr("aria-disabled", "true"); + const $el = $("#restart-backfill"); + $el.prop("disabled", true); + $el.attr("aria-disabled", "true"); - window.AP.context.getToken(function (token) { + window.AP.context.getToken(function(token) { $.ajax({ type: "POST", url: "/jira/sync", @@ -99,86 +97,37 @@ $(".sync-connection-link").click(function (event) { jiraHost, syncType: "full", jwt: token, - _csrf: csrfToken, - }, - success: function (data) { - console.log("success") - // AP.navigator.reload(); + _csrf: csrfToken }, - error: function (error) { - console.log(error); - $("#restart-backfill").prop("disabled", false); - $("#restart-backfill").attr("aria-disabled", "false"); + success: function(data) { + AP.navigator.reload(); }, + error: function(error) { + $el.prop("disabled", false); + $el.attr("aria-disabled", "false"); + } }); }); }); -/* ***************************** */ -/* To be removed after FF tested */ -/* ***************************** */ -const retryModal = document.getElementById('sync-retry-modal') -const statusModal = document.getElementById('sync-status-modal-old') -const retryBtn = document.getElementById('sync-retry-modal-btn') -const statusBtn = document.getElementById('sync-status-modal-btn-old') -const retrySpan = document.getElementById('retry-close') -const statusSpan = document.getElementById('status-close-old') - -if (retryBtn != null) { - retryBtn.onclick = function () { - retryModal.style.display = 'block' - } -} - -if (statusBtn != null) { - statusBtn.onclick = function () { - statusModal.style.display = 'block' - } -} - -if (retrySpan != null) { - retrySpan.onclick = function () { - retryModal.style.display = 'none' - } -} - -if (statusSpan != null) { - statusSpan.onclick = function () { - statusModal.style.display = 'none' - } -} - -// When the user clicks anywhere outside of the modal, close it -window.onclick = function (event) { - if (event.target === retryModal) { - retryModal.style.display = 'none' - } - if (event.target === statusModal) { - statusModal.style.display = 'none' - } -} -/* ***************************** */ -/* To be removed after FF tested */ -/* ***************************** */ - const syncStatusBtn = document.getElementById("sync-status-modal-btn"); const syncStatusModal = document.getElementById("sync-status-modal"); const syncStatusCloseBtn = document.getElementById("status-close"); if (syncStatusBtn != null) { - syncStatusBtn.onclick = function () { + syncStatusBtn.onclick = function() { syncStatusModal.style.display = "block"; }; } if (syncStatusCloseBtn != null) { - syncStatusCloseBtn.onclick = function () { + syncStatusCloseBtn.onclick = function() { syncStatusModal.style.display = "none"; }; } // When the user clicks anywhere outside of the modal, close it -window.onclick = function (event) { +window.onclick = function(event) { if (event.target.className === "jiraConfiguration__syncRetryModalOverlay") { syncStatusModal.style.display = "none"; } diff --git a/test/fixtures/api/pull-request-multiple-commits-diff.json b/test/fixtures/api/pull-request-multiple-commits-diff.json new file mode 100644 index 0000000000..5537b4d46a --- /dev/null +++ b/test/fixtures/api/pull-request-multiple-commits-diff.json @@ -0,0 +1,145 @@ +{ + "data": { + "url": "https://api.github.com/repos/test-user/test-repo/compare/main...feature/TEST-99-a", + "html_url": "https://github.com/test-user/test-repo/compare/main...feature/TEST-99-a", + "permalink_url": "https://github.com/test-user/test-repo/compare/test-user:d46cff1...test-user:3260f2b", + "diff_url": "https://github.com/test-user/test-repo/compare/main...feature/TEST-99-a.diff", + "patch_url": "https://github.com/test-user/test-repo/compare/main...feature/TEST-99-a.patch", + "base_commit": { + "sha": "3jhqwe89rweurneifnrsjkdfnads9fsfs", + "node_id": "jweqw80e128ennasdf90sdfzdfn2093rq2erfwaefsdf", + "commit": {}, + "url": "https://api.github.com/repos/test-user/test-repo/commits/d46cff1b05d7e2367e350d4f817a7e53907764ff", + "html_url": "https://github.com/test-user/test-repo/commit/d46cff1b05d7e2367e350d4f817a7e53907764ff", + "comments_url": "https://api.github.com/repos/test-user/test-repo/commits/d46cff1b05d7e2367e350d4f817a7e53907764ff/comments", + "author": {}, + "committer": {}, + "parents": [] + }, + "merge_base_commit": { + "sha": "qwdqwer23rkfndspf89vsdnp9fivndkfv dv", + "node_id": "sjkdnfiadfnasiofnasdf90j902j3ri2qnr=", + "commit": {}, + "url": "https://api.github.com/repos/test-user/test-repo/commits/e74909439f21289650b503e20a33115eaec6e7e7", + "html_url": "https://github.com/test-user/test-repo/commit/e74909439f21289650b503e20a33115eaec6e7e7", + "comments_url": "https://api.github.com/repos/test-user/test-repo/commits/e74909439f21289650b503e20a33115eaec6e7e7/comments", + "author": {}, + "committer": {}, + "parents": [] + }, + "status": "diverged", + "ahead_by": 5, + "behind_by": 8, + "total_commits": 5, + "commits": [ + { + "sha": "2jkrnw9pidfnew89fn2903rnpfwjdfndsf", + "node_id": "jahsbduq9wnpe9128pn3euiqwnwajednajskd", + "commit": { + "author": "test-author", + "committer": {}, + "message": "TEST-117 TEST-89 edit", + "tree": {}, + "url": "https://api.github.com/repos/test-user/test-repo/git/commits/242abd12b320c75c6b5472bb535da825efabaf8f", + "comment_count": 0, + "verification": {} + }, + "url": "https://api.github.com/repos/test-user/test-repo/commits/242abd12b320c75c6b5472bb535da825efabaf8f", + "html_url": "https://github.com/test-user/test-repo/commit/242abd12b320c75c6b5472bb535da825efabaf8f", + "comments_url": "https://api.github.com/repos/test-user/test-repo/commits/242abd12b320c75c6b5472bb535da825efabaf8f/comments", + "author": { + "login": "test-user", + "id": 37155488, + "node_id": "weirweirnewiornwr89wnerw", + "avatar_url": "https://avatars.githubusercontent.com/u/37155488?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/test-user", + "html_url": "https://github.com/test-user", + "followers_url": "https://api.github.com/users/test-user/followers", + "following_url": "https://api.github.com/users/test-user/following{/other_user}", + "gists_url": "https://api.github.com/users/test-user/gists{/gist_id}", + "starred_url": "https://api.github.com/users/test-user/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/test-user/subscriptions", + "organizations_url": "https://api.github.com/users/test-user/orgs", + "repos_url": "https://api.github.com/users/test-user/repos", + "events_url": "https://api.github.com/users/test-user/events{/privacy}", + "received_events_url": "https://api.github.com/users/test-user/received_events", + "type": "User", + "site_admin": false + } + }, + { + "sha": "2jkrnw9pidfnew89fn2903rnpfwjdfndsf", + "node_id": "jahsbduq9wnpe9128pn3euiqwnwajednajskd", + "commit": { + "author": "test-author", + "committer": {}, + "message": "TEST-109", + "tree": {}, + "url": "https://api.github.com/repos/test-user/test-repo/git/commits/242abd12b320c75c6b5472bb535da825efabaf8f", + "comment_count": 0, + "verification": {} + }, + "url": "https://api.github.com/repos/test-user/test-repo/commits/242abd12b320c75c6b5472bb535da825efabaf8f", + "html_url": "https://github.com/test-user/test-repo/commit/242abd12b320c75c6b5472bb535da825efabaf8f", + "comments_url": "https://api.github.com/repos/test-user/test-repo/commits/242abd12b320c75c6b5472bb535da825efabaf8f/comments", + "author": { + "login": "test-user", + "id": 37155488, + "node_id": "weirweirnewiornwr89wnerw", + "avatar_url": "https://avatars.githubusercontent.com/u/37155488?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/test-user", + "html_url": "https://github.com/test-user", + "followers_url": "https://api.github.com/users/test-user/followers", + "following_url": "https://api.github.com/users/test-user/following{/other_user}", + "gists_url": "https://api.github.com/users/test-user/gists{/gist_id}", + "starred_url": "https://api.github.com/users/test-user/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/test-user/subscriptions", + "organizations_url": "https://api.github.com/users/test-user/orgs", + "repos_url": "https://api.github.com/users/test-user/repos", + "events_url": "https://api.github.com/users/test-user/events{/privacy}", + "received_events_url": "https://api.github.com/users/test-user/received_events", + "type": "User", + "site_admin": false + } + }, + { + "sha": "2jkrnw9pidfnew89fn2903rnpfwjdfndsf", + "node_id": "jahsbduq9wnpe9128pn3euiqwnwajednajskd", + "commit": { + "author": "test-author", + "committer": {}, + "message": "TEST-11", + "tree": {}, + "url": "https://api.github.com/repos/test-user/test-repo/git/commits/242abd12b320c75c6b5472bb535da825efabaf8f", + "comment_count": 0, + "verification": {} + }, + "url": "https://api.github.com/repos/test-user/test-repo/commits/242abd12b320c75c6b5472bb535da825efabaf8f", + "html_url": "https://github.com/test-user/test-repo/commit/242abd12b320c75c6b5472bb535da825efabaf8f", + "comments_url": "https://api.github.com/repos/test-user/test-repo/commits/242abd12b320c75c6b5472bb535da825efabaf8f/comments", + "author": { + "login": "test-user", + "id": 37155488, + "node_id": "weirweirnewiornwr89wnerw", + "avatar_url": "https://avatars.githubusercontent.com/u/37155488?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/test-user", + "html_url": "https://github.com/test-user", + "followers_url": "https://api.github.com/users/test-user/followers", + "following_url": "https://api.github.com/users/test-user/following{/other_user}", + "gists_url": "https://api.github.com/users/test-user/gists{/gist_id}", + "starred_url": "https://api.github.com/users/test-user/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/test-user/subscriptions", + "organizations_url": "https://api.github.com/users/test-user/orgs", + "repos_url": "https://api.github.com/users/test-user/repos", + "events_url": "https://api.github.com/users/test-user/events{/privacy}", + "received_events_url": "https://api.github.com/users/test-user/received_events", + "type": "User", + "site_admin": false + } + } + ] + } +} diff --git a/test/fixtures/api/pull-request-single-commit-diff.json b/test/fixtures/api/pull-request-single-commit-diff.json new file mode 100644 index 0000000000..0667e81497 --- /dev/null +++ b/test/fixtures/api/pull-request-single-commit-diff.json @@ -0,0 +1,73 @@ +{ + "data": { + "url": "https://api.github.com/repos/test-user/test-repo/compare/main...feature/TEST-99-a", + "html_url": "https://github.com/test-user/test-repo/compare/main...feature/TEST-99-a", + "permalink_url": "https://github.com/test-user/test-repo/compare/test-user:d46cff1...test-user:3260f2b", + "diff_url": "https://github.com/test-user/test-repo/compare/main...feature/TEST-99-a.diff", + "patch_url": "https://github.com/test-user/test-repo/compare/main...feature/TEST-99-a.patch", + "base_commit": { + "sha": "3jhqwe89rweurneifnrsjkdfnads9fsfs", + "node_id": "jweqw80e128ennasdf90sdfzdfn2093rq2erfwaefsdf", + "commit": {}, + "url": "https://api.github.com/repos/test-user/test-repo/commits/d46cff1b05d7e2367e350d4f817a7e53907764ff", + "html_url": "https://github.com/test-user/test-repo/commit/d46cff1b05d7e2367e350d4f817a7e53907764ff", + "comments_url": "https://api.github.com/repos/test-user/test-repo/commits/d46cff1b05d7e2367e350d4f817a7e53907764ff/comments", + "author": {}, + "committer": {}, + "parents": [] + }, + "merge_base_commit": { + "sha": "qwdqwer23rkfndspf89vsdnp9fivndkfv dv", + "node_id": "sjkdnfiadfnasiofnasdf90j902j3ri2qnr=", + "commit": {}, + "url": "https://api.github.com/repos/test-user/test-repo/commits/e74909439f21289650b503e20a33115eaec6e7e7", + "html_url": "https://github.com/test-user/test-repo/commit/e74909439f21289650b503e20a33115eaec6e7e7", + "comments_url": "https://api.github.com/repos/test-user/test-repo/commits/e74909439f21289650b503e20a33115eaec6e7e7/comments", + "author": {}, + "committer": {}, + "parents": [] + }, + "status": "diverged", + "ahead_by": 5, + "behind_by": 8, + "total_commits": 5, + "commits": [ + { + "sha": "2jkrnw9pidfnew89fn2903rnpfwjdfndsf", + "node_id": "jahsbduq9wnpe9128pn3euiqwnwajednajskd", + "commit": { + "author": "test-author", + "committer": {}, + "message": "my sole commit TEST-100", + "tree": {}, + "url": "https://api.github.com/repos/test-user/test-repo/git/commits/242abd12b320c75c6b5472bb535da825efabaf8f", + "comment_count": 0, + "verification": {} + }, + "url": "https://api.github.com/repos/test-user/test-repo/commits/242abd12b320c75c6b5472bb535da825efabaf8f", + "html_url": "https://github.com/test-user/test-repo/commit/242abd12b320c75c6b5472bb535da825efabaf8f", + "comments_url": "https://api.github.com/repos/test-user/test-repo/commits/242abd12b320c75c6b5472bb535da825efabaf8f/comments", + "author": { + "login": "test-user", + "id": 37155488, + "node_id": "weirweirnewiornwr89wnerw", + "avatar_url": "https://avatars.githubusercontent.com/u/37155488?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/test-user", + "html_url": "https://github.com/test-user", + "followers_url": "https://api.github.com/users/test-user/followers", + "following_url": "https://api.github.com/users/test-user/following{/other_user}", + "gists_url": "https://api.github.com/users/test-user/gists{/gist_id}", + "starred_url": "https://api.github.com/users/test-user/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/test-user/subscriptions", + "organizations_url": "https://api.github.com/users/test-user/orgs", + "repos_url": "https://api.github.com/users/test-user/repos", + "events_url": "https://api.github.com/users/test-user/events{/privacy}", + "received_events_url": "https://api.github.com/users/test-user/received_events", + "type": "User", + "site_admin": false + } + } + ] + } +} diff --git a/test/fixtures/deployment-basic.json b/test/fixtures/deployment_status-basic.json similarity index 98% rename from test/fixtures/deployment-basic.json rename to test/fixtures/deployment_status-basic.json index 01206094e4..e4e6624c12 100644 --- a/test/fixtures/deployment-basic.json +++ b/test/fixtures/deployment_status-basic.json @@ -4,7 +4,7 @@ "deployment_status": { "url": "test-repo-url/deployments/1234/statuses/1", "id": 123456, - "state": "pending", + "state": "success", "creator": { "login": "TestUser[bot]", "type": "Bot" diff --git a/test/fixtures/pull-request-multiple-invalid-issue-key.json b/test/fixtures/pull-request-multiple-invalid-issue-key.json new file mode 100644 index 0000000000..7faeef84bb --- /dev/null +++ b/test/fixtures/pull-request-multiple-invalid-issue-key.json @@ -0,0 +1,48 @@ +{ + "name": "pull_request", + "payload": { + "action": "opened", + "repository": { + "id": "test-repo-id", + "name": "test-repo-name", + "full_name": "example/test-repo-name", + "html_url": "test-repo-url", + "owner": { + "login": "test-repo-owner" + } + }, + "pull_request": { + "id": "test-pull-request-id", + "number": 1, + "state": "open", + "title": "[TEST-123] [TEST-222] Test pull request.", + "body": "[TEST-123] [TEST-222] body of the test pull request.", + "comments": "test-pull-request-comment-count", + "html_url": "test-pull-request-url", + "head": { + "repo": { + "html_url": "test-pull-request-head-url" + }, + "ref": "TEST-321-test-pull-request-head-ref", + "sha": "test-pull-request-sha" + }, + "base": { + "repo": { + "html_url": "test-pull-request-base-url" + }, + "ref": "test-pull-request-base-ref" + }, + "user": { + "login": "test-pull-request-user-login" + }, + "updated_at": "test-pull-request-update-time" + }, + "sender": { + "type": "User", + "login": "TestUser" + }, + "installation": { + "id": 1234 + } + } +} diff --git a/test/fixtures/workflow-basic.json b/test/fixtures/workflow-basic.json index 960c1a0ee6..8b448cd71b 100644 --- a/test/fixtures/workflow-basic.json +++ b/test/fixtures/workflow-basic.json @@ -53,7 +53,13 @@ "check_suite_id": 3099253651, "html_url": "test-repo-url", "created_at": "2021-06-28T03:53:34Z", - "updated_at": "2021-06-28T03:53:34Z" + "updated_at": "2021-06-28T03:53:34Z", + "repository": { + "owner": { + "login": "test-repo-owner" + }, + "name": "test-repo-name" + } }, "workflow": { "id": 9751894, diff --git a/test/frontend/get-jira-configuration.test.ts b/test/frontend/get-jira-configuration.test.ts index 1eb3e36729..b9a540d912 100644 --- a/test/frontend/get-jira-configuration.test.ts +++ b/test/frontend/get-jira-configuration.test.ts @@ -23,7 +23,7 @@ describe("Jira Configuration Suite", () => { subscriptions = [ { gitHubInstallationId: 15, - jiraHost: "https://test-host.jira.com", + jiraHost, destroy: jest.fn().mockResolvedValue(undefined), syncWarning: "some warning", updatedAt: "2018-04-18T15:42:13Z", @@ -51,7 +51,7 @@ describe("Jira Configuration Suite", () => { installation = { id: 19, - jiraHost: subscriptions[0].jiraHost, + jiraHost, clientKey: "abc123", enabled: true, secrets: "def234", @@ -72,7 +72,6 @@ describe("Jira Configuration Suite", () => { const mockRequest = (): any => ({ query: { xdm_e: "https://somejirasite.atlassian.net" }, - session: { jiraHost: subscriptions[0].jiraHost }, csrfToken: jest.fn().mockReturnValue({}), log: { info: jest.fn(), @@ -83,6 +82,7 @@ describe("Jira Configuration Suite", () => { const mockResponse = (): any => ({ locals: { + jiraHost, client: { apps: { getInstallation: jest.fn().mockReturnValue({ data: {} }) diff --git a/test/github/client/github-client.test.ts b/test/github/client/github-client.test.ts index d246f6aa5b..b89aeffc8b 100644 --- a/test/github/client/github-client.test.ts +++ b/test/github/client/github-client.test.ts @@ -1,55 +1,277 @@ /* eslint-disable @typescript-eslint/no-var-requires */ /* eslint-disable jest/no-standalone-expect */ +import { getLogger } from "../../../src/config/logger"; import GitHubClient from "../../../src/github/client/github-client"; +import statsd from "../../../src/config/statsd"; +import {BlockedIpError, GithubClientError, RateLimitingError} from "../../../src/github/client/errors"; +import anything = jasmine.anything; +import objectContaining = jasmine.objectContaining; describe("GitHub Client", () => { - const appTokenExpirationDate = new Date(2021, 10, 25, 0, 0); const githubInstallationId = 17979017; + let statsdHistogramSpy, statsdIncrementSpy; + beforeEach(() => { + // Lock Time + statsdHistogramSpy = jest.spyOn(statsd, "histogram"); + statsdIncrementSpy = jest.spyOn(statsd, "increment"); + }); + + afterEach(() => { + // Unlock Time + statsdHistogramSpy.mockRestore(); + }); - function givenGitHubReturnsInstallationToken(installationToken: string, expectedAppTokenInHeader?: string) { + function givenGitHubReturnsInstallationToken( + installationToken: string, + expectedAppTokenInHeader?: string + ) { githubNock .post(`/app/installations/${githubInstallationId}/access_tokens`) - .matchHeader("Authorization", expectedAppTokenInHeader ? `Bearer ${expectedAppTokenInHeader}` : /^Bearer .+$/) + .optionally() + .matchHeader( + "Authorization", + expectedAppTokenInHeader + ? `Bearer ${expectedAppTokenInHeader}` + : /^Bearer .+$/ + ) .matchHeader("Accept", "application/vnd.github.v3+json") .reply(200, { expires_at: appTokenExpirationDate.toISOString(), - token: installationToken + token: installationToken, }); } - function givenGitHubReturnsPullrequests(owner: string, repo: string, perPage: number, page: number, expectedInstallationTokenInHeader?: string) { + function givenGitHubReturnsPullrequests( + owner: string, + repo: string, + perPage: number, + page: number, + expectedInstallationTokenInHeader?: string + ) { githubNock .get(`/repos/${owner}/${repo}/pulls`) .query({ per_page: perPage, page, + installationId: /^.*$/, + }) + .matchHeader( + "Authorization", + expectedInstallationTokenInHeader + ? `Bearer ${expectedInstallationTokenInHeader}` + : /^Bearer .+$/ + ) + .matchHeader("Accept", "application/vnd.github.v3+json") + .reply(200, [ + { number: 1 }, // we don't really care about the shape of this response because it's in GitHub's hands anyways + ]); + } + + function givenGitHubReturnsCommit( + owner: string, + repo: string, + ref: string, + expectedInstallationTokenInHeader?: string + ) { + githubNock + .get(`/repos/${owner}/${repo}/commits/${ref}`) + .query({ installationId: /^.*$/ }) - .matchHeader("Authorization", expectedInstallationTokenInHeader ? `Bearer ${expectedInstallationTokenInHeader}` : /^Bearer .+$/) + .matchHeader( + "Authorization", + expectedInstallationTokenInHeader + ? `Bearer ${expectedInstallationTokenInHeader}` + : /^Bearer .+$/ + ) .matchHeader("Accept", "application/vnd.github.v3+json") .reply(200, [ - { number: 1 } // we don't really care about the shape of this response because it's in GitHub's hands anyways + { number: 1 }, // we don't really care about the shape of this response because it's in GitHub's hands anyways ]); } - it("lists pull requests", async () => { + it("lists pull requests", async () => { const owner = "owner"; const repo = "repo"; const pageSize = 5; const page = 1; givenGitHubReturnsInstallationToken("installation token"); - givenGitHubReturnsPullrequests(owner, repo, pageSize, page, "installation token"); + givenGitHubReturnsPullrequests( + owner, + repo, + pageSize, + page, + "installation token" + ); - const client = new GitHubClient(githubInstallationId); - const pullrequests = await client.getPullRequests(owner, repo, { per_page: pageSize, page }); + const client = new GitHubClient(githubInstallationId, getLogger("test")); + const pullrequests = await client.getPullRequests(owner, repo, { + per_page: pageSize, + page, + }); expect(pullrequests).toBeTruthy(); expect(githubNock.pendingMocks()).toEqual([]); + verifyMetricsSent('/repos/:owner/:repo/pulls', "200"); }); -}); + it("fetches a commit", async () => { + const owner = "owner"; + const repo = "repo"; + const sha = "84fdc9346f43f829f88fb4b1d240b1aaaa5250da"; + + givenGitHubReturnsInstallationToken("installation token"); + givenGitHubReturnsCommit( + owner, + repo, + sha, + "installation token" + ); + + const client = new GitHubClient(githubInstallationId, getLogger("test")); + const commit = await client.getCommit(owner, repo, sha); + + expect(commit).toBeTruthy(); + expect(githubNock.pendingMocks()).toEqual([]); + verifyMetricsSent('/repos/:owner/:repo/commits/:ref', "200"); + }); + + function verifyMetricsSent(path: string, status) { + expect(statsdHistogramSpy).toBeCalledWith("app.server.http.request.github", anything(), objectContaining({ + client: "axios", + method: 'GET', + path, + status + })); + } + + it("should handle rate limit error from Github when X-RateLimit-Reset not specified", async () => { + givenGitHubReturnsInstallationToken("installation token"); + githubNock.get(`/repos/owner/repo/pulls`).query({ + installationId: /^.*$/, + }).reply( + 403, { message: "API rate limit exceeded for xxx.xxx.xxx.xxx." }, + { + "X-RateLimit-Remaining": "0", + } + ); + Date.now = jest.fn(() => 1000000); + const client = new GitHubClient(githubInstallationId, getLogger("test")); + let error: any = undefined; + try { + await client.getPullRequests("owner", "repo", {}); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(RateLimitingError) + expect(error.rateLimitReset).toBe(4600) + + verifyMetricsSent('/repos/:owner/:repo/pulls', "rateLimiting"); + + }); + + it("should handle rate limit error from Github when X-RateLimit-Reset specified", async () => { + givenGitHubReturnsInstallationToken("installation token"); + githubNock.get(`/repos/owner/repo/pulls`).query({ + installationId: /^.*$/, + }).reply( + 403, [{ number: 1 }], + { + "X-RateLimit-Remaining": "0", + "X-RateLimit-Reset": "2000" + } + ); + Date.now = jest.fn(() => 1000000); + const client = new GitHubClient(githubInstallationId, getLogger("test")); + let error: any = undefined; + try { + await client.getPullRequests("owner", "repo", {}); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(RateLimitingError) + expect(error.rateLimitReset).toBe(2000) + + verifyMetricsSent('/repos/:owner/:repo/pulls', "rateLimiting"); + + }); + + it("should handle blocked IP error from Github when specified", async () => { + givenGitHubReturnsInstallationToken("installation token"); + githubNock.get(`/repos/owner/repo/pulls`).query({ + installationId: /^.*$/, + }).reply( + 403, { message: "Org has an IP allow list enabled" } + ); + Date.now = jest.fn(() => 1000000); + const client = new GitHubClient(githubInstallationId, getLogger("test")); + let error: any = undefined; + try { + await client.getPullRequests("owner", "repo", {}); + } catch (e) { + error = e; + } + expect(error).toBeInstanceOf(BlockedIpError); + expect(statsdIncrementSpy).toBeCalledWith("app.server.error.blocked-by-github-allowlist"); + verifyMetricsSent('/repos/:owner/:repo/pulls', "blockedIp"); + }); + + it("should handle rate limit properly handled regardless of the response code", async () => { + givenGitHubReturnsInstallationToken("installation token"); + githubNock.get(`/repos/owner/repo/pulls`).query({ + installationId: /^.*$/, + }).reply( + 500, [{ number: 1 }], + { + "X-RateLimit-Remaining": "0", + "X-RateLimit-Reset": "2000" + } + ); + Date.now = jest.fn(() => 1000000); + const client = new GitHubClient(githubInstallationId, getLogger("test")); + let error: any = undefined; + try { + await client.getPullRequests("owner", "repo", {}); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(RateLimitingError) + expect(error.rateLimitReset).toBe(2000) + + verifyMetricsSent('/repos/:owner/:repo/pulls', "rateLimiting"); + + }); + + it("should transform error properly on 404", async () => { + givenGitHubReturnsInstallationToken("installation token"); + githubNock.get(`/repos/owner/repo/pulls`).query({ + installationId: /^.*$/, + }).reply( + 404, [{ number: 1 }], + { + "X-RateLimit-Remaining": "0", + } + ); + Date.now = jest.fn(() => 1000000); + const client = new GitHubClient(githubInstallationId, getLogger("test")); + let error: any = undefined; + try { + await client.getPullRequests("owner", "repo", {}); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(GithubClientError) + expect(error.status).toBe(404) + + verifyMetricsSent('/repos/:owner/:repo/pulls', "404"); + }); +}); diff --git a/test/jira/multiple-jira.test.ts b/test/jira/multiple-jira.test.ts new file mode 100644 index 0000000000..1d6a216bb3 --- /dev/null +++ b/test/jira/multiple-jira.test.ts @@ -0,0 +1,132 @@ +import { jiraMatchingIssuesKeysBulkResponse, githubRequestUserLoginResponse, githubPullReviewsResponse, jiraMultipleJiraBulkResponse } from './nock-response'; +/* eslint-disable @typescript-eslint/no-var-requires */ +import { createWebhookApp } from "../utils/probot"; +import { Application } from "probot"; +import { Installation, Subscription } from "../../src/models"; +import nock from "nock"; + +describe("multiple Jira instances", () => { + let app: Application; + const gitHubInstallationId = 1234; + const jira2Host = "https://test2-atlassian-instance.net"; + const jira2Nock = nock(jira2Host); + beforeEach(async () => { + app = await createWebhookApp(); + const clientKey = "client-key"; + await Installation.create({ + clientKey, + sharedSecret: "shared-secret", + jiraHost + }); + await Subscription.create({ + gitHubInstallationId, + jiraHost, + jiraClientKey: clientKey + }); + await Installation.create({ + clientKey, + sharedSecret: "shared-secret", + jiraHost: jira2Host, + }); + await Subscription.create({ + gitHubInstallationId, + jiraHost: jira2Host, + jiraClientKey: clientKey + }); + }); + + afterEach(async () => { + await Subscription.destroy({ truncate: true }); + await Subscription.destroy({ truncate: true }); + }); + + it("should not linkify issue keys for jira instance that has matching issues", async () => { + const fixture = require("../fixtures/pull-request-multiple-invalid-issue-key.json"); + githubNock.get("/users/test-pull-request-user-login") + .times(2) + .reply(200, githubRequestUserLoginResponse); + + githubNock.get("/repos/test-repo-owner/test-repo-name/pulls/1/reviews") + .times(2) + .reply(200, githubPullReviewsResponse); + + githubNock.patch("/repos/test-repo-owner/test-repo-name/issues/1", { + body: "[TEST-123] [TEST-222] body of the test pull request.\n\n[TEST-222]: https://test2-atlassian-instance.net/browse/TEST-222", + id: "test-pull-request-id" + }).reply(200); + + jiraNock + .get("/rest/api/latest/issue/TEST-123?fields=summary") + .reply(401); + + jiraNock + .get("/rest/api/latest/issue/TEST-222?fields=summary") + .reply(401); + + jira2Nock + .get("/rest/api/latest/issue/TEST-123?fields=summary") + .reply(401); + + jira2Nock + .get("/rest/api/latest/issue/TEST-222?fields=summary") + .reply(200, { + key: "TEST-222", + fields: { + summary: "Example Issue" + } + }); + + jiraNock.post("/rest/devinfo/0.10/bulk", jiraMatchingIssuesKeysBulkResponse).reply(200); + jira2Nock.post("/rest/devinfo/0.10/bulk", jiraMatchingIssuesKeysBulkResponse).reply(200); + Date.now = jest.fn(() => 12345678); + + await expect(app.receive(fixture)).toResolve(); + }); + + it("should associate PR with to multiple jira with same issue keys", async () => { + const fixture = require("../fixtures/pull-request-basic.json"); + + githubNock.get("/users/test-pull-request-user-login") + .twice() + .reply(200, githubRequestUserLoginResponse); + + githubNock.get("/repos/test-repo-owner/test-repo-name/pulls/1/reviews") + .twice() + .reply(200, githubPullReviewsResponse); + + githubNock.patch("/repos/test-repo-owner/test-repo-name/issues/1", { + body: "[TEST-123] body of the test pull request.\n\n[TEST-123]: https://test-atlassian-instance.net/browse/TEST-123", + id: "test-pull-request-id" + }).reply(200); + + githubNock + .patch("/repos/test-repo-owner/test-repo-name/issues/1", { + body: "[TEST-123] body of the test pull request.\n\n[TEST-123]: https://test2-atlassian-instance.net/browse/TEST-123", + id: "test-pull-request-id" + }).reply(200) + + jiraNock + .get("/rest/api/latest/issue/TEST-123?fields=summary") + .reply(200, { + key: "TEST-123", + fields: { + summary: "Example Issue" + } + }); + jira2Nock + .get("/rest/api/latest/issue/TEST-123?fields=summary") + .reply(200, { + key: "TEST-123", + fields: { + summary: "Example Issue" + } + }); + + jiraNock.post("/rest/devinfo/0.10/bulk", jiraMultipleJiraBulkResponse).reply(200) + jira2Nock.post("/rest/devinfo/0.10/bulk", jiraMultipleJiraBulkResponse).reply(200); + + Date.now = jest.fn(() => 12345678); + + await expect(app.receive(fixture)).toResolve(); + }); +}); \ No newline at end of file diff --git a/test/jira/nock-response.ts b/test/jira/nock-response.ts new file mode 100644 index 0000000000..0fd24d7f0f --- /dev/null +++ b/test/jira/nock-response.ts @@ -0,0 +1,187 @@ +export const githubPullReviewsResponse = [ + { + id: 80, + node_id: "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3ODA=", + user: { + login: "test-pull-request-reviewer-login", + id: 1, + node_id: "MDQ6VXNlcjE=", + avatar_url: "test-pull-request-reviewer-avatar", + gravatar_id: "", + url: "https://api.github.com/users/reviewer", + html_url: "https://github.com/reviewer", + followers_url: "https://api.github.com/users/reviewer/followers", + following_url: "https://api.github.com/users/reviewer/following{/other_user}", + gists_url: "https://api.github.com/users/reviewer/gists{/gist_id}", + starred_url: "https://api.github.com/users/reviewer/starred{/owner}{/repo}", + subscriptions_url: "https://api.github.com/users/reviewer/subscriptions", + organizations_url: "https://api.github.com/users/reviewer/orgs", + repos_url: "https://api.github.com/users/reviewer/repos", + events_url: "https://api.github.com/users/reviewer/events{/privacy}", + received_events_url: "https://api.github.com/users/reviewer/received_events", + type: "User", + site_admin: false + }, + body: "Here is the body for the review.", + state: "APPROVED", + html_url: "https://github.com/test-repo-owner/test-repo-name/pull/1#pullrequestreview-80", + pull_request_url: "https://api.github.com/repos/test-repo-owner/test-repo-name/pulls/1", + _links: { + html: { + href: "https://github.com/test-repo-owner/test-repo-name/pull/1#pullrequestreview-80" + }, + pull_request: { + href: "https://api.github.com/repos/test-repo-owner/test-repo-name/pulls/1" + } + }, + submitted_at: "2019-11-17T17:43:43Z", + commit_id: "ecdd80bb57125d7ba9641ffaa4d7d2c19d3f3091", + author_association: "COLLABORATOR" + } +]; + +export const githubRequestUserLoginResponse = { + login: "test-pull-request-author-login", + avatar_url: "test-pull-request-author-avatar", + html_url: "test-pull-request-author-url" +} + +export const jiraMatchingIssuesKeysBulkResponse = { + preventTransitions: false, + repositories: [ + { + url: "test-pull-request-base-url", + branches: [ + { + createPullRequestUrl: "test-pull-request-head-url/pull/new/TEST-321-test-pull-request-head-ref", + lastCommit: { + author: { + avatar: "https://github.com/ghost.png", + name: "Deleted User", + email: "deleted@noreply.user.github.com", + url: "https://github.com/ghost" + }, + authorTimestamp: "test-pull-request-update-time", + displayId: "test-p", + fileCount: 0, + hash: "test-pull-request-sha", + id: "test-pull-request-sha", + issueKeys: ["TEST-123", "TEST-222", "TEST-321"], + message: "n/a", + updateSequenceId: 12345678, + url: "test-pull-request-head-url/commit/test-pull-request-sha" + }, + id: "TEST-321-test-pull-request-head-ref", + issueKeys: ["TEST-123", "TEST-222", "TEST-321"], + name: "TEST-321-test-pull-request-head-ref", + url: "test-pull-request-head-url/tree/TEST-321-test-pull-request-head-ref", + updateSequenceId: 12345678 + } + ], + pullRequests: [ + { + author: { + avatar: "test-pull-request-author-avatar", + name: "test-pull-request-author-login", + email: "test-pull-request-author-login@noreply.user.github.com", + url: "test-pull-request-author-url" + }, + commentCount: "test-pull-request-comment-count", + destinationBranch: "test-pull-request-base-url/tree/test-pull-request-base-ref", + displayId: "#1", + id: 1, + issueKeys: ["TEST-123", "TEST-222", "TEST-321"], + lastUpdate: "test-pull-request-update-time", + reviewers: [ + { + avatar: "test-pull-request-reviewer-avatar", + name: "test-pull-request-reviewer-login", + email: "test-pull-request-reviewer-login@noreply.user.github.com", + url: "https://github.com/reviewer", + approvalStatus: "APPROVED" + } + ], + sourceBranch: "TEST-321-test-pull-request-head-ref", + sourceBranchUrl: "test-pull-request-head-url/tree/TEST-321-test-pull-request-head-ref", + status: "OPEN", + timestamp: "test-pull-request-update-time", + title: "[TEST-123] [TEST-222] Test pull request.", + url: "test-pull-request-url", + updateSequenceId: 12345678 + } + ], + updateSequenceId: 12345678 + } + ], + properties: { installationId: 1234 } +}; + +export const jiraMultipleJiraBulkResponse = { + preventTransitions: false, + repositories: [ + { + url: "test-pull-request-base-url", + branches: [ + { + createPullRequestUrl: "test-pull-request-head-url/pull/new/TEST-321-test-pull-request-head-ref", + lastCommit: { + author: { + avatar: "https://github.com/ghost.png", + name: "Deleted User", + email: "deleted@noreply.user.github.com", + url: "https://github.com/ghost" + }, + authorTimestamp: "test-pull-request-update-time", + displayId: "test-p", + fileCount: 0, + hash: "test-pull-request-sha", + id: "test-pull-request-sha", + issueKeys: ["TEST-123", "TEST-321"], + message: "n/a", + updateSequenceId: 12345678, + url: "test-pull-request-head-url/commit/test-pull-request-sha" + }, + id: "TEST-321-test-pull-request-head-ref", + issueKeys: ["TEST-123", "TEST-321"], + name: "TEST-321-test-pull-request-head-ref", + url: "test-pull-request-head-url/tree/TEST-321-test-pull-request-head-ref", + updateSequenceId: 12345678 + } + ], + pullRequests: [ + { + author: { + avatar: "test-pull-request-author-avatar", + name: "test-pull-request-author-login", + email: "test-pull-request-author-login@noreply.user.github.com", + url: "test-pull-request-author-url" + }, + commentCount: "test-pull-request-comment-count", + destinationBranch: "test-pull-request-base-url/tree/test-pull-request-base-ref", + displayId: "#1", + id: 1, + issueKeys: ["TEST-123", "TEST-321"], + lastUpdate: "test-pull-request-update-time", + reviewers: [ + { + avatar: "test-pull-request-reviewer-avatar", + name: "test-pull-request-reviewer-login", + email: "test-pull-request-reviewer-login@noreply.user.github.com", + url: "https://github.com/reviewer", + approvalStatus: "APPROVED" + } + ], + sourceBranch: "TEST-321-test-pull-request-head-ref", + sourceBranchUrl: "test-pull-request-head-url/tree/TEST-321-test-pull-request-head-ref", + status: "OPEN", + timestamp: "test-pull-request-update-time", + title: "[TEST-123] Test pull request.", + url: "test-pull-request-url", + updateSequenceId: 12345678 + } + ], + updateSequenceId: 12345678 + } + ], + properties: { installationId: 1234 } +}; \ No newline at end of file diff --git a/test/safe/deployment.test.ts b/test/safe/deployment.test.ts index 0e7d004fdf..ea3ef12f6f 100644 --- a/test/safe/deployment.test.ts +++ b/test/safe/deployment.test.ts @@ -29,7 +29,7 @@ describe("Deployment Webhook", () => { describe("deployment_status", () => { it("should update the Jira issue with the linked GitHub deployment", async () => { - const fixture = require("../fixtures/deployment-basic.json"); + const fixture = require("../fixtures/deployment_status-basic.json"); const sha = fixture.payload.deployment.sha; githubNock.get(`/repos/test-repo-owner/test-repo-name/commits/${sha}`) @@ -59,8 +59,8 @@ describe("Deployment Webhook", () => { displayName: "deploy", url: "test-repo-url/commit/885bee1-commit-id-1c458/checks", description: "deploy", - lastUpdated: "2021-06-28T12:15:18Z", - state: "in_progress", + lastUpdated: "2021-06-28T12:15:18.000Z", + state: "successful", pipeline: { id: "deploy", diff --git a/test/safe/workflow.test.ts b/test/safe/workflow.test.ts index dc1fecd4e0..9270649b3a 100644 --- a/test/safe/workflow.test.ts +++ b/test/safe/workflow.test.ts @@ -28,9 +28,6 @@ describe("Workflow Webhook", () => { }); describe("workflow_run", () => { - beforeEach(() => { - jest.setTimeout(20000); - }); it("should update the Jira issue with the linked GitHub workflow_run", async () => { const fixture = require("../fixtures/workflow-basic.json"); diff --git a/test/setup/create-job.ts b/test/setup/create-job.ts index 7ca852b037..8821439c25 100644 --- a/test/setup/create-job.ts +++ b/test/setup/create-job.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ // Create a job stub with data -// TODO: add better typings +import {Hub} from "@sentry/types/dist/hub"; + export default ({ data, opts }: { data: any, opts?: any }) => { const defaultOpts = { attempts: 3, @@ -11,6 +12,8 @@ export default ({ data, opts }: { data: any, opts?: any }) => { return { data, opts: Object.assign(defaultOpts, opts || {}), - sentry: { setUser: jest.fn() } + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + sentry: { setUser: jest.fn() } as Hub }; }; diff --git a/test/unit/frontend/delete-github-subscription.test.ts b/test/unit/frontend/delete-github-subscription.test.ts index eb331c6ae7..f8bae69822 100644 --- a/test/unit/frontend/delete-github-subscription.test.ts +++ b/test/unit/frontend/delete-github-subscription.test.ts @@ -30,9 +30,6 @@ describe("POST /github/subscription", () => { installationId: gitHubInstallationId, jiraHost }, - query: { - xdm_e: jiraHost - }, session: { githubToken: "abc-token" } @@ -45,6 +42,7 @@ describe("POST /github/subscription", () => { sendStatus: jest.fn(), status: jest.fn(), locals: { + jiraHost, github: { apps: { listInstallationsForAuthenticatedUser: jest.fn().mockResolvedValue({ @@ -92,13 +90,17 @@ describe("POST /github/subscription", () => { jiraHost } }; - delete req.body[property]; - const res = { status: jest.fn(), - json: jest.fn() + json: jest.fn(), + locals: { + jiraHost + } }; + delete req.body[property]; + delete res.locals[property]; + res.status.mockReturnValue(res); await deleteSubscription(req as any, res as any); diff --git a/test/unit/frontend/delete-jira-configuration.test.ts b/test/unit/frontend/delete-jira-configuration.test.ts index 116c7e883f..25192a2c26 100644 --- a/test/unit/frontend/delete-jira-configuration.test.ts +++ b/test/unit/frontend/delete-jira-configuration.test.ts @@ -1,5 +1,4 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import nock from "nock"; import { Installation, Subscription } from "../../../src/models"; import { mocked } from "ts-jest/utils"; import deleteJiraConfiguration from "../../../src/frontend/delete-jira-configuration"; @@ -14,13 +13,13 @@ describe("DELETE /jira/configuration", () => { beforeEach(async () => { subscription = { githubInstallationId: 15, - jiraHost: "https://test-host.jira.com", + jiraHost, destroy: jest.fn().mockResolvedValue(undefined) }; installation = { id: 19, - jiraHost: subscription.jiraHost, + jiraHost, clientKey: "abc123", enabled: true, secrets: "def234", @@ -33,7 +32,7 @@ describe("DELETE /jira/configuration", () => { }); it("Delete Jira Configuration", async () => { - nock(subscription.jiraHost) + jiraNock .delete("/rest/devinfo/0.10/bulkByProperties") .query({ installationId: subscription.githubInstallationId }) .reply(200, "OK"); @@ -41,16 +40,13 @@ describe("DELETE /jira/configuration", () => { // TODO: use supertest for this const req = { log: getLogger("request"), - body: { installationId: subscription.githubInstallationId }, - query: { - xdm_e: subscription.jiraHost - }, - session: { + body: { + installationId: subscription.githubInstallationId, jiraHost: subscription.jiraHost } }; - const res = { sendStatus: jest.fn(), locals: { installation } }; + const res = { sendStatus: jest.fn(), locals: { installation, jiraHost } }; await deleteJiraConfiguration(req as any, res as any); expect(subscription.destroy).toHaveBeenCalled(); expect(res.sendStatus).toHaveBeenCalledWith(204); diff --git a/test/unit/frontend/verify-jira-middleware.test.ts b/test/unit/frontend/verify-jira-middleware.test.ts index 631cbc0017..611610b210 100644 --- a/test/unit/frontend/verify-jira-middleware.test.ts +++ b/test/unit/frontend/verify-jira-middleware.test.ts @@ -13,12 +13,13 @@ describe("#verifyJiraMiddleware", () => { let res; let next; let installation; - let subscription; const testSharedSecret = "test-secret"; beforeEach(async () => { res = { - locals: {}, + locals: { + jiraHost + }, status: jest.fn(), json: jest.fn() }; @@ -27,15 +28,9 @@ describe("#verifyJiraMiddleware", () => { res.status.mockReturnValue(res) res.json.mockReturnValue(res) - subscription = { - githubInstallationId: 15, - jiraHost: "https://test-host.jira.com", - destroy: jest.fn().mockResolvedValue(undefined) - }; - installation = { id: 19, - jiraHost: subscription.jiraHost, + jiraHost, clientKey: "abc123", enabled: true, secrets: "def234", @@ -56,7 +51,7 @@ describe("#verifyJiraMiddleware", () => { jwt: jwtValue }, session: { - jiraHost: subscription.jiraHost, + // jiraHost, jwt: jwtValue }, addLogFields: jest.fn(), @@ -69,9 +64,6 @@ describe("#verifyJiraMiddleware", () => { body: { jiraHost, }, - session: { - jiraHost: subscription.jiraHost - }, query: { xdm_e: jiraHost, }, @@ -93,7 +85,6 @@ describe("#verifyJiraMiddleware", () => { jwt: jwtValue }, session: { - jiraHost: subscription.jiraHost, jwt: jwtValue }, addLogFields: jest.fn(), @@ -137,7 +128,7 @@ describe("#verifyJiraMiddleware", () => { await verifyJiraContextJwtTokenMiddleware(req, res, next); expect(addLogFieldsSpy).toHaveBeenCalledWith({ - jiraHost: installation.jiraHost, + jiraHost, jiraClientKey: "abc12***" // should be the shortened key }); }); @@ -154,9 +145,6 @@ describe("#verifyJiraMiddleware", () => { body: { jiraHost, }, - session: { - jiraHost: subscription.jiraHost - }, query: { xdm_e: jiraHost, jwt: encodedJwt diff --git a/test/unit/models/axios-error-event-decorator.test.ts b/test/unit/models/axios-error-event-decorator.test.ts index 9296b4c4e1..89675eed93 100644 --- a/test/unit/models/axios-error-event-decorator.test.ts +++ b/test/unit/models/axios-error-event-decorator.test.ts @@ -35,14 +35,13 @@ describe("AxiosErrorDecorator", () => { it("adds request data", () => { const decoratedEvent = AxiosErrorDecorator.decorate(event, hint); - expect(decoratedEvent.extra.request).toEqual({ + expect(decoratedEvent.extra.request).toMatchObject({ method: "GET", path: "/foo/bar", host: "www.example.com", headers: { accept: "application/json, text/plain, */*", host: "www.example.com", - "user-agent": "axios/0.21.1" } }); }); diff --git a/test/unit/sqs/backfill.test.ts b/test/unit/sqs/backfill.test.ts new file mode 100644 index 0000000000..21775d6544 --- /dev/null +++ b/test/unit/sqs/backfill.test.ts @@ -0,0 +1,54 @@ + +import {processInstallation} from "../../../src/sync/installation"; +import {mocked} from "ts-jest/utils"; +import {BackfillMessagePayload, backfillQueueMessageHandlerFactory} from "../../../src/sqs/backfill"; +import {Context} from "../../../src/sqs"; +import {getLogger} from "../../../src/config/logger"; +import * as Sentry from "@sentry/node"; + +jest.mock("../../../src/sync/installation"); + +jest.mock("@sentry/node"); + +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +mocked(Sentry.getCurrentHub).mockImplementation(() => ({ + getClient: jest.fn() +})); + +const sentryCaptureExceptionMock = jest.fn(); + +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +mocked(Sentry.Hub).mockImplementation(() => ({ + configureScope: jest.fn(), + setTag: jest.fn(), + setExtra: jest.fn(), + captureException: sentryCaptureExceptionMock, +} as Sentry.Hub)) + +describe('backfill', () => { + const BACKFILL_MESSAGE_CONTEXT: Context = { + payload: { + installationId: 123, + jiraHost: "https://test.atlassian.net" + }, + message: { + }, + log: getLogger('test'), + receiveCount: 1, + lastAttempt: false + } + + afterEach(() => { + sentryCaptureExceptionMock.mockReset(); + }); + + test('sentry captures exception', async () => { + const mockedProcessor = jest.fn(); + mocked(processInstallation).mockReturnValue(mockedProcessor); + mockedProcessor.mockRejectedValue(new Error("something went horribly wrong")); + await backfillQueueMessageHandlerFactory(jest.fn())(BACKFILL_MESSAGE_CONTEXT).catch(e => console.warn(e)); + expect(sentryCaptureExceptionMock).toBeCalled(); + }); +}); diff --git a/test/unit/sqs/push.test.ts b/test/unit/sqs/error-handlers.test.ts similarity index 65% rename from test/unit/sqs/push.test.ts rename to test/unit/sqs/error-handlers.test.ts index c27c2f9d51..f4c45559fc 100644 --- a/test/unit/sqs/push.test.ts +++ b/test/unit/sqs/error-handlers.test.ts @@ -1,13 +1,15 @@ import statsd from "../../../src/config/statsd"; -import {pushQueueErrorHandler, PushQueueMessagePayload} from "../../../src/sqs/push"; +import {jiraOctokitErrorHandler} from "../../../src/sqs/error-handlers"; import {Context} from "../../../src/sqs/index"; import {getLogger} from "../../../src/config/logger"; import {JiraClientError} from "../../../src/jira/client/axios"; -import {RateLimitingError} from "../../../src/config/enhance-octokit"; +import {RateLimitingError as OldRateLimitingError} from "../../../src/config/enhance-octokit"; import {Octokit} from "probot"; +import {RateLimitingError} from "../../../src/github/client/errors"; +import {AxiosError} from "axios"; -describe("Push Queue Error Handler", () => { +describe("jiraOktokitErrorHandler", () => { let statsdIncrementSpy = jest.spyOn(statsd, "histogram"); @@ -37,7 +39,7 @@ describe("Push Queue Error Handler", () => { webhookId: "string" }; - const createContext = (receiveCount: number, lastAttempt: boolean): Context => + const createContext = (receiveCount: number, lastAttempt: boolean): Context => ({ receiveCount, lastAttempt, log: getLogger("test"), message: {}, payload: mockPayload }) @@ -45,7 +47,7 @@ describe("Push Queue Error Handler", () => { it("Returns normal retry when error is unknown", async () => { - const result = await pushQueueErrorHandler(new Error(), createContext(1, false)); + const result = await jiraOctokitErrorHandler(new Error(), createContext(1, false)); expect(result.retryable).toBe(true) expect(result.retryDelaySec).toBe(3*60) @@ -54,7 +56,7 @@ describe("Push Queue Error Handler", () => { it("Exponential backoff works", async () => { - const result = await pushQueueErrorHandler(new Error(), createContext(3, false)); + const result = await jiraOctokitErrorHandler(new Error(), createContext(3, false)); expect(result.retryable).toBe(true) expect(result.retryDelaySec).toBe(27*60) @@ -63,7 +65,7 @@ describe("Push Queue Error Handler", () => { it("Sends metrics when it was last attempt", async () => { - await pushQueueErrorHandler(new Error(), createContext(1, true)); + await jiraOctokitErrorHandler(new Error(), createContext(1, true)); expect(statsdIncrementSpy).toBeCalledTimes(1); }); @@ -78,34 +80,45 @@ describe("Push Queue Error Handler", () => { it("Unretryable and no failure metric on Jira 401", async () => { - const result = await pushQueueErrorHandler(getJiraClientError(401), createContext(1, true)); + const result = await jiraOctokitErrorHandler(getJiraClientError(401), createContext(1, true)); expect(result.retryable).toBe(false) expect(statsdIncrementSpy).toBeCalledTimes(0); }); it("Unretryable and no failure metric on Jira 403", async () => { - const result = await pushQueueErrorHandler(getJiraClientError(403), createContext(1, true)); + const result = await jiraOctokitErrorHandler(getJiraClientError(403), createContext(1, true)); expect(result.retryable).toBe(false) expect(statsdIncrementSpy).toBeCalledTimes(0); }); it("Unretryable and no failure metric on Jira 404", async () => { - const result = await pushQueueErrorHandler(getJiraClientError(404), createContext(1, true)); + const result = await jiraOctokitErrorHandler(getJiraClientError(404), createContext(1, true)); expect(result.retryable).toBe(false) expect(statsdIncrementSpy).toBeCalledTimes(0); }); it("Retryable and no failure metric sent on Jira 500", async () => { - const result = await pushQueueErrorHandler(getJiraClientError(500), createContext(1, true)); + const result = await jiraOctokitErrorHandler(getJiraClientError(500), createContext(1, true)); expect(result.retryable).toBe(true) expect(statsdIncrementSpy).toBeCalledTimes(1); }); + it("Retryable with proper delay on Rate Limiting (old)", async () => { + const result = await jiraOctokitErrorHandler(new OldRateLimitingError(Math.floor(new Date("2020-01-01").getTime()/1000) + 100), createContext(1, false)); + expect(result.retryable).toBe(true) + //Make sure delay is equal to recommended delay + 10 seconds + expect(result.retryDelaySec).toBe(110) + expect(statsdIncrementSpy).toBeCalledTimes(0); + }); + it("Retryable with proper delay on Rate Limiting", async () => { - const result = await pushQueueErrorHandler(new RateLimitingError(Math.floor(new Date("2020-01-01").getTime()/1000) + 100), createContext(1, false)); + const result = await jiraOctokitErrorHandler(new RateLimitingError( + Math.floor(new Date("2020-01-01").getTime()/1000) + 100, + 0, {} as AxiosError + ), createContext(1, false)); expect(result.retryable).toBe(true) //Make sure delay is equal to recommended delay + 10 seconds expect(result.retryDelaySec).toBe(110) @@ -116,7 +129,7 @@ describe("Push Queue Error Handler", () => { const error : Octokit.HookError = {...new Error("Err"), status: 401, headers: {}} - const result = await pushQueueErrorHandler(error, createContext(1, true)); + const result = await jiraOctokitErrorHandler(error, createContext(1, true)); expect(result.retryable).toBe(false) expect(statsdIncrementSpy).toBeCalledTimes(0); }); @@ -125,7 +138,7 @@ describe("Push Queue Error Handler", () => { const error : Octokit.HookError = {...new Error("Err"), status: 500, headers: {}} - const result = await pushQueueErrorHandler(error, createContext(1, true)); + const result = await jiraOctokitErrorHandler(error, createContext(1, true)); expect(result.retryable).toBe(true) expect(statsdIncrementSpy).toBeCalledTimes(1); }); diff --git a/test/unit/sync/branch.test.ts b/test/unit/sync/branch.test.ts index c03225f7fc..41b78e052d 100644 --- a/test/unit/sync/branch.test.ts +++ b/test/unit/sync/branch.test.ts @@ -96,6 +96,11 @@ describe.skip("sync/branches", () => { .reply(200, emptyNodesFixture); } + const backfillQueue = { + schedule: jest.fn() + }; + const queueSupplier = () => Promise.resolve(backfillQueue); + beforeEach(async () => { const repoSyncStatus = { installationId: installationId, @@ -130,6 +135,10 @@ describe.skip("sync/branches", () => { app = await createWebhookApp(); }); + afterEach(() => { + backfillQueue.schedule.mockReset(); + }) + it("should sync to Jira when branch refs have jira references", async () => { const job = createJob({ data: { installationId, jiraHost }, @@ -144,13 +153,8 @@ describe.skip("sync/branches", () => { ) .reply(200); - const queues = { - installation: { - add: jest.fn() - } - }; - await expect(processInstallation(app, queues, jest.fn())(job, getLogger('test'))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger('test'))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts.delay); }); it("should send data if issue keys are only present in commits", async () => { @@ -169,13 +173,8 @@ describe.skip("sync/branches", () => { ) .reply(200); - const queues = { - installation: { - add: jest.fn() - } - }; - await expect(processInstallation(app, queues, jest.fn())(job, getLogger('test'))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger('test'))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts.delay); }); it("should send data if issue keys are only present in an associatd PR title", async () => { @@ -228,13 +227,8 @@ describe.skip("sync/branches", () => { }) .reply(200); - const queues = { - installation: { - add: jest.fn() - } - }; - await expect(processInstallation(app, queues, jest.fn())(job, getLogger('test'))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger('test'))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts.delay); }); it("should not call Jira if no issue keys are found", async () => { @@ -244,17 +238,11 @@ describe.skip("sync/branches", () => { }); nockBranchRequest(branchNoIssueKeys); - const queues = { - installation: { - add: jest.fn() - } - }; - const interceptor = jiraNock.post(/.*/); const scope = interceptor.reply(200); - await expect(processInstallation(app, queues, jest.fn())(job, getLogger('test'))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger('test'))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts.delay); expect(scope).not.toBeDone(); nock.removeInterceptor(interceptor); }); diff --git a/test/unit/sync/commits.test.ts b/test/unit/sync/commits.test.ts index 57c20fa959..db15c075f8 100644 --- a/test/unit/sync/commits.test.ts +++ b/test/unit/sync/commits.test.ts @@ -22,6 +22,11 @@ describe.skip("sync/commits", () => { const defaultBranchNullFixture = require("../../fixtures/api/graphql/default-branch-null.json"); const commitsNoKeys = require("../../fixtures/api/graphql/commit-nodes-no-keys.json"); + const backfillQueue = { + schedule: jest.fn() + }; + const queueSupplier = () => Promise.resolve(backfillQueue); + beforeEach(async () => { // TODO: move this into utils to easily construct mock data const repoSyncStatus = { @@ -59,6 +64,10 @@ describe.skip("sync/commits", () => { app = createApplication(); }); + afterEach(() => { + backfillQueue.schedule.mockReset(); + }); + it("should sync to Jira when Commit Nodes have jira references", async () => { const job = createJob({ data: { installationId, jiraHost }, @@ -105,13 +114,8 @@ describe.skip("sync/commits", () => { } }).reply(200); - const queues = { - installation: { - add: jest.fn() - } - }; - await expect(processInstallation(app, queues, jest.fn())(job, getLogger('test'))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger('test'))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts.delay); }); it("should send Jira all commits that have Issue Keys", async () => { @@ -194,13 +198,8 @@ describe.skip("sync/commits", () => { } }).reply(200); - const queues = { - installation: { - add: jest.fn() - } - }; - await expect(processInstallation(app, queues, jest.fn())(job, getLogger('test'))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger('test'))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts); }); it("should default to master branch if defaultBranchRef is null", async () => { @@ -249,13 +248,8 @@ describe.skip("sync/commits", () => { } }).reply(200); - const queues = { - installation: { - add: jest.fn() - } - }; - await expect(processInstallation(app, queues, jest.fn())(job, getLogger('test'))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger('test'))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts); }); it("should not call Jira if no issue keys are present", async () => { @@ -275,13 +269,8 @@ describe.skip("sync/commits", () => { const interceptor = jiraNock.post(/.*/); const scope = interceptor.reply(200); - const queues = { - installation: { - add: jest.fn() - } - }; - await expect(processInstallation(app, queues, jest.fn())(job, getLogger('test'))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger('test'))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts); expect(scope).not.toBeDone(); nock.removeInterceptor(interceptor); }); @@ -300,13 +289,8 @@ describe.skip("sync/commits", () => { const interceptor = jiraNock.post(/.*/); const scope = interceptor.reply(200); - const queues = { - installation: { - add: jest.fn() - } - }; - await expect(processInstallation(app, queues, jest.fn())(job, getLogger('test'))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger('test'))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts); expect(scope).not.toBeDone(); nock.removeInterceptor(interceptor); }); diff --git a/test/unit/sync/installation.test.ts b/test/unit/sync/installation.test.ts index 270502e201..f3271b1dec 100644 --- a/test/unit/sync/installation.test.ts +++ b/test/unit/sync/installation.test.ts @@ -1,5 +1,4 @@ import { - BackfillQueue, isRetryableWithSmallerRequest, maybeScheduleNextTask, processInstallation @@ -8,21 +7,12 @@ import { import {DeduplicatorResult} from "../../../src/sync/deduplicator"; import '../../../src/config/feature-flags'; -import {BooleanFlags} from "../../../src/config/feature-flags"; import {Application} from "probot"; import {getLogger} from "../../../src/config/logger"; +import createJob from "../../setup/create-job"; -import Queue from "bull"; - -let mockedBooleanFeatureFlags = {}; -jest.mock('../../../src/config/feature-flags', () => { - return { - ...jest.requireActual('../../../src/config/feature-flags'), - booleanFlag: (key) => Promise.resolve(mockedBooleanFeatureFlags[key]) - }; -}); - +const TEST_LOGGER = getLogger('test'); const mockedExecuteWithDeduplication = jest.fn(); jest.mock('../../../src/sync/deduplicator', () => { @@ -38,7 +28,9 @@ jest.mock("../../../src/models"); describe("sync/installation", () => { - const backfillQueue: BackfillQueue = { + const JOB_DATA = {installationId: 1, jiraHost: "http://foo"}; + + const backfillQueue = { schedule: jest.fn() } @@ -46,11 +38,9 @@ describe("sync/installation", () => { beforeEach(() => { mockedExecuteWithDeduplication.mockReset(); - mockedBooleanFeatureFlags[BooleanFlags.USE_BACKFILL_QUEUE_SUPPLIER] = true; }); afterEach(() => { - mockedBooleanFeatureFlags = {}; (backfillQueue.schedule as jest.Mock).mockReset(); }); @@ -97,70 +87,46 @@ describe("sync/installation", () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore const app: Application = jest.fn() as Application; - const queues = { - installation: { - add: jest.fn() - } - }; - const job = { - data: { - installationId: 1, - jiraHost: 'https://blah.atlassian.com' - }, - sentry: { - setUser: jest.fn() - } - }; - const logger = getLogger('test'); + + const job = createJob({ + data: JOB_DATA + }); test('should process the installation with deduplication', async () => { - await processInstallation(app, queues, () => Promise.resolve(backfillQueue))(job, logger); + await processInstallation(app, () => Promise.resolve(backfillQueue))(job, TEST_LOGGER); expect(mockedExecuteWithDeduplication.mock.calls.length).toBe(1); }); test('should reschedule the job if deduplicator is unsure', async () => { mockedExecuteWithDeduplication.mockResolvedValue(DeduplicatorResult.E_NOT_SURE_TRY_AGAIN_LATER); - await processInstallation(app, queues, () => Promise.resolve(backfillQueue))(job, logger); - expect(backfillQueueSchedule.mock.calls).toEqual([[job.data, 60_000]]); + await processInstallation(app, () => Promise.resolve(backfillQueue))(job, TEST_LOGGER); + expect(backfillQueueSchedule.mock.calls).toHaveLength(1); + expect(backfillQueueSchedule.mock.calls[0][0]).toEqual(JOB_DATA); + expect(backfillQueueSchedule.mock.calls[0][1]).toEqual(60_000); + expect(backfillQueueSchedule.mock.calls[0][2].warn).toBeDefined(); }); test('should also reschedule the job if deduplicator is sure', async () => { mockedExecuteWithDeduplication.mockResolvedValue(DeduplicatorResult.E_OTHER_WORKER_DOING_THIS_JOB); - await processInstallation(app, queues, () => Promise.resolve(backfillQueue))(job, logger); + await processInstallation(app, () => Promise.resolve(backfillQueue))(job, TEST_LOGGER); expect(backfillQueueSchedule.mock.calls.length).toEqual(1); }); }); describe('maybeScheduleNextTask', () => { test('does nothing if there is no next task', () => { - const queue = { - add: jest.fn() - }; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - maybeScheduleNextTask(queue as Queue.Queue, backfillQueue, {foo: 'bar'}, [], getLogger('test')); + maybeScheduleNextTask(backfillQueue, JOB_DATA, [], TEST_LOGGER); expect(backfillQueueSchedule.mock.calls).toHaveLength(0); }); test('when multiple tasks, picks the one with the highest delay', async () => { - const queue = { - add: jest.fn() - }; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - - await maybeScheduleNextTask(queue as Queue.Queue, backfillQueue, {foo: 'bar'}, [30, 60, 0], getLogger('test')); - expect(backfillQueueSchedule.mock.calls).toEqual([[{foo: 'bar'}, 60]]); + await maybeScheduleNextTask(backfillQueue, JOB_DATA, [30, 60, 0], TEST_LOGGER); + expect(backfillQueueSchedule.mock.calls).toEqual([[JOB_DATA, 60, TEST_LOGGER]]); }); test('not passing delay to queue when not provided', async () => { - const queue = { - add: jest.fn() - }; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - await maybeScheduleNextTask(queue as Queue.Queue, backfillQueue, {foo: 'bar'}, [0], getLogger('test')); - expect(backfillQueueSchedule.mock.calls).toEqual([[{foo: 'bar'}, 0]]); + await maybeScheduleNextTask(backfillQueue, JOB_DATA, [0], TEST_LOGGER); + expect(backfillQueueSchedule.mock.calls).toEqual([[JOB_DATA, 0, TEST_LOGGER]]); }); }); }); diff --git a/test/unit/sync/pull-requests.test.ts b/test/unit/sync/pull-requests.test.ts index ea55d5674d..e570d3b631 100644 --- a/test/unit/sync/pull-requests.test.ts +++ b/test/unit/sync/pull-requests.test.ts @@ -15,7 +15,11 @@ jest.mock("../../../src/models"); describe.skip("sync/pull-request", () => { const installationId = 1234; let app: Application; - let queues; + + const backfillQueue = { + schedule: jest.fn() + }; + const queueSupplier = () => Promise.resolve(backfillQueue); beforeEach(async () => { jest.setTimeout(10000); @@ -39,14 +43,9 @@ describe.skip("sync/pull-request", () => { } }; - queues = { - installation: { - add: jest.fn() - }, - pullRequests: { - add: jest.fn() - } - }; + afterEach(() => { + backfillQueue.schedule.mockReset(); + }) Date.now = jest.fn(() => 12345678); @@ -115,7 +114,7 @@ describe.skip("sync/pull-request", () => { properties: { installationId: 1234 } }).reply(200); - await expect(processInstallation(app, queues, jest.fn())(job, getLogger("test"))).toResolve(); + await expect(processInstallation(app, queueSupplier)(job, getLogger("test"))).toResolve(); }); }); @@ -128,8 +127,7 @@ describe.skip("sync/pull-request", () => { const interceptor = jiraNock.post(/.*/); const scope = interceptor.reply(200); - await expect(processInstallation(app, queues, jest.fn())(job, getLogger("test"))).toResolve(); - expect(queues.pullRequests.add).not.toHaveBeenCalled(); + await expect(processInstallation(app, queueSupplier)(job, getLogger("test"))).toResolve(); expect(scope).not.toBeDone(); nock.removeInterceptor(interceptor); }); @@ -147,8 +145,8 @@ describe.skip("sync/pull-request", () => { const interceptor = jiraNock.post(/.*/); const scope = interceptor.reply(200); - await expect(processInstallation(app, queues, jest.fn())(job, getLogger("test"))).toResolve(); - expect(queues.installation.add).toHaveBeenCalledWith(job.data, job.opts); + await expect(processInstallation(app, queueSupplier)(job, getLogger("test"))).toResolve(); + expect(backfillQueue.schedule).toHaveBeenCalledWith(job.data, job.opts.delay); expect(scope).not.toBeDone(); nock.removeInterceptor(interceptor); }); diff --git a/test/unit/transforms/deployments.test.ts b/test/unit/transforms/deployments.test.ts index 00ea438475..a31b96938d 100644 --- a/test/unit/transforms/deployments.test.ts +++ b/test/unit/transforms/deployments.test.ts @@ -1,5 +1,13 @@ -import _ from "lodash"; +/* eslint-disable @typescript-eslint/no-var-requires */ +// eslint-disable-next-line import/no-duplicates import { mapEnvironment } from "../../../src/transforms/deployment"; +import transformDeployment from "../../../src/transforms/deployment"; +import {getLogger} from "../../../src/config/logger"; +import {GitHubAPI} from "probot"; +import { when } from "jest-when"; +import { booleanFlag, BooleanFlags } from "../../../src/config/feature-flags"; + +jest.mock("../../../src/config/feature-flags"); describe("deployment environment mapping", () => { test("classifies known environments correctly", () => { @@ -67,3 +75,99 @@ describe("deployment environment mapping", () => { expect(mapEnvironment("製造")).toBe("unmapped"); }); }); + +describe("transform GitHub webhook payload to Jira payload", () => { + + const deployment_status = require("../../fixtures/deployment_status-basic.json"); + const owner = deployment_status.payload.repository.owner.login; + const repo = deployment_status.payload.repository.name; + + it("supports branch and merge workflows - FF TRUE", async () => { + // Mocking all GitHub API Calls + // Get commit + githubNock.get(`/repos/${owner}/${repo}/commits/${deployment_status.payload.deployment.sha}`) + .reply(200, { + ...owner, + commit: { + message: "testing" + } + }); + + // List deployments + githubNock.get(`/repos/${owner}/${repo}/deployments?environment=Production&per_page=10`) + .reply(200, + [ + { + id: 1, + environment: "Production", + sha: "6e87a40179eb7ecf5094b9c8d690db727472d5bc" + } + ] + ); + + // List deployments statuses + githubNock.get(`/repos/${owner}/${repo}/deployments/1/statuses?per_page=100`) + .reply(200, [ + { + id: 1, + state: "pending" + }, + { + id: 2, + state: "success" + } + ]); + + // Compare commits + githubNock.get(`/repos/${owner}/${repo}/compare/6e87a40179eb7ecf5094b9c8d690db727472d5bc...${deployment_status.payload.deployment.sha}`) + .reply(200,{ + commits: [ + { + commit: { + message: "ABC-1" + } + }, + { + commit: { + message: "ABC-2" + } + } + ]} + ); + + + when(booleanFlag).calledWith( + BooleanFlags.SUPPORT_BRANCH_AND_MERGE_WORKFLOWS_FOR_DEPLOYMENTS, + expect.anything(), + expect.anything() + ).mockResolvedValue(true); + + const jiraPayload = await transformDeployment(GitHubAPI(), deployment_status.payload, "testing.atlassian.net", getLogger("deploymentLogger")) + + expect(jiraPayload).toMatchObject({ + deployments: [{ + schemaVersion: "1.0", + deploymentSequenceNumber: 1234, + updateSequenceNumber: 123456, + issueKeys: ["ABC-1", "ABC-2"], + displayName: "deploy", + url: "test-repo-url/commit/885bee1-commit-id-1c458/checks", + description: "deploy", + lastUpdated: new Date("2021-06-28T12:15:18.000Z"), + state: "successful", + pipeline: { + id: "deploy", + displayName: "deploy", + url: "test-repo-url/commit/885bee1-commit-id-1c458/checks", + }, + environment: { + id: "Production", + displayName: "Production", + type: "production", + }, + }] + }) + }) + + // TODO add test if FF is false +}); diff --git a/test/unit/transforms/util/githubApiRequests.test.ts b/test/unit/transforms/util/githubApiRequests.test.ts new file mode 100644 index 0000000000..fc325f8e66 --- /dev/null +++ b/test/unit/transforms/util/githubApiRequests.test.ts @@ -0,0 +1,88 @@ +/* eslint-disable @typescript-eslint/no-var-requires */ +import GitHubAPI from "../../../../src/config/github-api"; +import { compareCommitsBetweenBaseAndHeadBranches } from "../../../../src/transforms/util/githubApiRequests"; +import { getLogger } from "../../../../src/config/logger"; + +describe("GitHub API Request Suite", () => { + describe("compareCommitsBetweenBaseAndHeadBranches", () => { + it("should return message from multiple commits containing multiple issue keys", async () => { + const workflowRunPayload = Object.assign( + {}, + require("../../../fixtures/workflow-basic.json") + ); + + const { pull_requests, repository } = + workflowRunPayload.payload.workflow_run; + + const payload = { + owner: repository.owner.login, + repo: repository.name, + base: pull_requests[0].base.ref, + head: pull_requests[0].head.ref, + }; + + const pullRequestCommits = Object.assign( + {}, + require("../../../fixtures/api/pull-request-multiple-commits-diff.json") + ); + + const { data } = pullRequestCommits; + + githubNock + .get( + `/repos/${payload.owner}/${payload.repo}/compare/${payload.base}...${payload.head}` + ) + .reply(200, { + ...data, + }); + + const bob = await compareCommitsBetweenBaseAndHeadBranches( + payload, + GitHubAPI(), + getLogger("test") + ); + + expect(bob).toEqual("TEST-117 TEST-89 edit TEST-109 TEST-11"); + }); + + it("should return message with multiple issue keys for a single commit", async () => { + const workflowRunPayload = Object.assign( + {}, + require("../../../fixtures/workflow-basic.json") + ); + + const { pull_requests, repository } = + workflowRunPayload.payload.workflow_run; + + const payload = { + owner: repository.owner.login, + repo: repository.name, + base: pull_requests[0].base.ref, + head: pull_requests[0].head.ref, + }; + + const pullRequestCommits = Object.assign( + {}, + require("../../../fixtures/api/pull-request-single-commit-diff.json") + ); + + const data = pullRequestCommits.data; + + githubNock + .get( + `/repos/${payload.owner}/${payload.repo}/compare/${payload.base}...${payload.head}` + ) + .reply(200, { + ...data, + }); + + const bob = await compareCommitsBetweenBaseAndHeadBranches( + payload, + GitHubAPI(), + getLogger("test") + ); + + expect(bob).toEqual("my sole commit TEST-100"); + }); + }); +}); diff --git a/test/unit/util/getUrl.test.ts b/test/unit/util/getUrl.test.ts deleted file mode 100644 index 0e9b7ac5f8..0000000000 --- a/test/unit/util/getUrl.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -import {getJiraHostFromRedirectUrl} from "../../../src/util/getUrl"; -import {getLogger} from "../../../src/config/logger"; - -describe("getJiraHostFromRedirectUrl", () => { - test("extracts jiraHost from xdm_e", () => { - const host = getJiraHostFromRedirectUrl("/github/configuration?jwt=eyo&xdm_e=https%3A%2F%2Fbgvozdev.atlassian.net", getLogger("test")) - expect(host).toBe("bgvozdev.atlassian.net"); - }); - - test("extracts jiraHost from host", () => { - const host = getJiraHostFromRedirectUrl("https://bgvozdev.atlassian.net/whatever", getLogger("test")) - expect(host).toBe("bgvozdev.atlassian.net"); - }); - - test("xdm_e has priority", () => { - const host = getJiraHostFromRedirectUrl("https://bgvozdev.atlassian.net/whatever?xdm_e=https%3A%2F%2Fblah.atlassian.net", getLogger("test")) - expect(host).toBe("blah.atlassian.net"); - }); - - test("returns unknown when neither host nor xdm_e were provided", () => { - const host = getJiraHostFromRedirectUrl("/whatever", getLogger("test")) - expect(host).toBe("unknown"); - }); - - test("returns unknown when cannot parse", ()=> { - const host = getJiraHostFromRedirectUrl("yadayada", getLogger("test")); - expect(host).toBe("unknown"); - }); - - test("returns empty on empty", ()=> { - const host = getJiraHostFromRedirectUrl("", getLogger("test")); - expect(host).toBe("empty"); - }); -}); diff --git a/views/github-error.hbs b/views/error.hbs similarity index 70% rename from views/github-error.hbs rename to views/error.hbs index 9cb2fee47a..b39814cbe4 100644 --- a/views/github-error.hbs +++ b/views/error.hbs @@ -4,35 +4,33 @@ {{title}} - + -
+
Error -

Looks like something went wrong!

+

Looks like something went wrong!

{{#if message}} -

{{message}}

+

{{{message}}}

{{/if}} -

+

We track these errors automatically, but if the problem persists feel free to contact us. In the meantime, try adding an organization again in Jira.

Learn how
diff --git a/views/github-configuration-OLD.hbs b/views/github-configuration-OLD.hbs deleted file mode 100644 index bfb54a0a32..0000000000 --- a/views/github-configuration-OLD.hbs +++ /dev/null @@ -1,138 +0,0 @@ - - - - - - {{title}} - - - - - - - - - - - -
- -

Logout

-
- -
-
- Jira and GitHub logos -
- -

Connect a - GitHub organization to your Jira site

-

{{jiraHost}}

- -
-
-
-

Unconnected organizations - with Jira app

-

Repository access

-
- -
- {{#each installations}} -
- {{! Organizations }} - - - {{! Repository access }} -
-

{{repoAccessType repository_selection}}

-

{{numberOfRepos}}

- {{#if admin}} - - Edit - - {{/if}} -
- - {{! Connect }} -
- {{#if admin}} - {{#if (isNotConnected syncStatus)}} - - {{else if (inProgressOrPendingSync syncStatus)}} -
-
-
- {{else}} -

- {{connectedStatus syncStatus}} -

- {{/if}} - {{/if}} -
- -
- {{/each}} - - {{#unless installations}} -

No GitHub - organizations with Jira installed.

- {{/unless}} -
- - + - Install Jira on a new organization - -
-
- -
- - - - diff --git a/views/github-configuration.hbs b/views/github-configuration.hbs index 9ce4289606..2b7f36d7e3 100644 --- a/views/github-configuration.hbs +++ b/views/github-configuration.hbs @@ -69,7 +69,7 @@ Github organization avatar {{account.login}} diff --git a/views/github-error-OLD.hbs b/views/github-error-OLD.hbs deleted file mode 100644 index 139f7d5ff3..0000000000 --- a/views/github-error-OLD.hbs +++ /dev/null @@ -1,22 +0,0 @@ - - - - - - - {{title}} - - - - - -
-

Looks like something went wrong!

-
- {{#if message}} -

{{message}}

- {{/if}} -

We track these errors automatically, but if the problem persists feel free to contact us. In the meantime, try adding an organization again in Jira.

-
- - diff --git a/views/github-setup-OLD.hbs b/views/github-setup-OLD.hbs deleted file mode 100644 index 358c68a430..0000000000 --- a/views/github-setup-OLD.hbs +++ /dev/null @@ -1,43 +0,0 @@ - - - - - - - - Setup - - - - - -
-

Jira Cloud configuration

-

What's your Jira Cloud Site?

- -
- -
- - - - -
-

- -

- {{#if error}} -
- {{error}} -
- {{/if}} -
-
- - - diff --git a/views/jira-configuration-OLD.hbs b/views/jira-configuration-OLD.hbs deleted file mode 100644 index 763c80bb6c..0000000000 --- a/views/jira-configuration-OLD.hbs +++ /dev/null @@ -1,181 +0,0 @@ - - - - - - - - {{title}} - - - - - - - -
-
-
-

GitHub configuration

-
- - {{#if hasConnections}} - - {{/if}} - -
- {{#if hasConnections}} - - - - - - - - - - - - -
-
- × -

IN PROGRESS - The sync has started and is still in progress for this account. New data may not immediately be displayed in Jira.

-

FAILED - There was a problem syncing data from your account. If there were temporary technical issues, a normal resync will pick up from where it left off and continue with the sync. - If it continues to return to FAILED state after re-trying, a full resync may be necessary.

-

PENDING - The sync has been queued, but is not actively syncing data from GitHub.

-

COMPLETE - The sync has finished. Information from selected repositories will be shown in Jira's development information panel.

-
-
- - - - -
-
- × -

Normal - Retry if the sync failed and you haven't changed the configuration on GitHub. - This will not attempt to rediscover the GitHub repositories in your installation.

-

Full - Rediscover your GitHub repositories and perform another full sync. - Use this is you've changed the configuration on GitHub or would otherwise like to rescan all repositories.

-
-
- - - - - {{#each connections}} - - {{!-- Organization --}} - - - {{!-- No header --}} - - - {{!-- Added --}} - - - {{!-- Repos Synced --}} - - - - - - - - - {{/each}} - {{#each failedConnections}} - - - - - - {{/each}} - -
OrganizationAddedRepos SyncedSync Status ℹ️Last Sync UpdateRetry ℹ️
{{ account.login }} - - Configure - - - {{ installedAt.relative }} - - - {{#ifAllReposSynced numberOfSyncedRepos totalNumberOfRepos}} - {{ totalNumberOfRepos }} - {{else}} - {{ numberOfSyncedRepos }} / {{ totalNumberOfRepos }} - {{/ifAllReposSynced}} - {{#if isGlobalInstall }} - (All) - {{else}} - (Selected) - {{/if}} - - {{#if syncWarning}} - - {{ syncStatus }} * - - {{else}} - {{ syncStatus }} - {{/if}} - - - {{ subscriptionUpdatedAt.relative }} - - - - - -
- #{{ id }} - {{#if deleted}} - This installation seems to be removed from GitHub but not from the database - {{else}} - There was an error getting the information of this installation - {{/if}} - - - {{ installedAt.relative }} - - - -
- {{else}} -
- Connecting GitHub and Jira -

Connect GitHub to Jira Software

-

Choose a GitHub organization to connect and start including issue keys in branches, commit messages or pull requests to see development insights in Jira.

- -
- {{/if}} -
- - {{#if hasConnections}} -

Metadata for commits, branches, and pull requests that use the Smart Commit syntax - will be synced to Jira and appear in the Development Information panel - of the relevant issue.

- -

To view your Organization’s updated sync status, please refresh this page.

- {{/if}} -
- -
- - - - - - diff --git a/views/maintenance.hbs b/views/maintenance.hbs index d787f8091c..a8af6e943d 100644 --- a/views/maintenance.hbs +++ b/views/maintenance.hbs @@ -8,7 +8,7 @@ {{title}} - + + + +

{{title}}

+ + + + + + +