diff --git a/.github/shared/package.json b/.github/shared/package.json index 5c94d416ddfb..b20ab5106766 100644 --- a/.github/shared/package.json +++ b/.github/shared/package.json @@ -19,6 +19,7 @@ "./spec-model": "./src/spec-model.js", "./swagger": "./src/swagger.js", "./tag": "./src/tag.js", + "./simple-git": "./src/simple-git.js", "./test/examples": "./test/examples.js" }, "bin": { diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index 433ca87ca7ef..cab93e7e2199 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -190,6 +190,18 @@ export function example(file) { ); } +/** + * @param {string} file + * @returns {boolean} + */ +export function typespec(file) { + return ( + typeof file === "string" && + (file.toLowerCase().endsWith(".tsp") || file.toLowerCase().endsWith("tspconfig.yaml")) && + specification(file) + ); +} + /** * @param {string} [file] * @returns {boolean} diff --git a/.github/shared/src/error-reporting.js b/.github/shared/src/error-reporting.js index 71a71a9eab9d..5eab9575c988 100644 --- a/.github/shared/src/error-reporting.js +++ b/.github/shared/src/error-reporting.js @@ -19,6 +19,28 @@ export function setSummary(content) { fs.writeFileSync(summaryFile, content); } +/** + * Set the output for a Github Actions step. The output is written to the GITHUB_OUTPUT environment variable. + * This is used to pass data between steps in a workflow. + * + * To access this output later, leverage: ${{ steps..outputs. }}. + * + * This function is the equivalent of using `core.setOutput(name, value)` in a GitHub Action, without the package dependency. + * @param {string} name - The name of the output variable. + * @param {string} value - The value to set for the output variable. + * @returns {void} + */ +export function setOutput(name, value) { + if (!process.env.GITHUB_OUTPUT) { + console.log(`GITHUB_OUTPUT is not set. Skipping ${name} update with value '${value}.'`); + return; + } + + if (process.env.GITHUB_OUTPUT) { + fs.appendFileSync(process.env.GITHUB_OUTPUT, `${name}=${value}\n`); + } +} + /** * This function is used to ask the github agent to annotate a file in a github PR with an error message. * @param {string} repoPath diff --git a/.github/shared/src/simple-git.js b/.github/shared/src/simple-git.js new file mode 100644 index 000000000000..cdf0d7bf5c9f --- /dev/null +++ b/.github/shared/src/simple-git.js @@ -0,0 +1,13 @@ +import { simpleGit } from "simple-git"; +import { resolve } from "path"; + +/** + * + * @param {string} inputPath + * @returns {Promise} + */ +export async function getRootFolder(inputPath) { + // expecting users to handle the case where inputPath is not a git repo + const gitRoot = await simpleGit(inputPath).revparse("--show-toplevel"); + return resolve(gitRoot.trim()); +} diff --git a/.github/shared/test/changed-files.test.js b/.github/shared/test/changed-files.test.js index c4908df76df2..7ecf778a0740 100644 --- a/.github/shared/test/changed-files.test.js +++ b/.github/shared/test/changed-files.test.js @@ -20,6 +20,7 @@ import { scenario, specification, swagger, + typespec, } from "../src/changed-files.js"; import { debugLogger } from "../src/logger.js"; @@ -48,6 +49,7 @@ describe("changedFiles", () => { "README.MD", "specification/contosowidgetmanager/data-plane/readme.md", "specification/contosowidgetmanager/Contoso.Management/main.tsp", + "specification/contosowidgetmanager/Contoso.Management/tspconfig.yaml", "specification/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", "specification/contosowidgetmanager/resource-manager/readme.md", "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", @@ -82,6 +84,7 @@ describe("changedFiles", () => { const expected = [ "specification/contosowidgetmanager/data-plane/readme.md", "specification/contosowidgetmanager/Contoso.Management/main.tsp", + "specification/contosowidgetmanager/Contoso.Management/tspconfig.yaml", "specification/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", "specification/contosowidgetmanager/resource-manager/readme.md", "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", @@ -92,6 +95,14 @@ describe("changedFiles", () => { expect(files.filter(specification)).toEqual(expected); }); + it("filter:typespec", () => { + const expected = [ + "specification/contosowidgetmanager/Contoso.Management/main.tsp", + "specification/contosowidgetmanager/Contoso.Management/tspconfig.yaml", + ]; + expect(files.filter(typespec)).toEqual(expected); + }); + it("filter:data-plane", () => { const expected = ["specification/contosowidgetmanager/data-plane/readme.md"]; diff --git a/.github/shared/test/error-reporting.test.js b/.github/shared/test/error-reporting.test.js index 107e2141de71..b89728184734 100644 --- a/.github/shared/test/error-reporting.test.js +++ b/.github/shared/test/error-reporting.test.js @@ -1,7 +1,7 @@ // @ts-check import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { setSummary, annotateFileError } from "../src/error-reporting.js"; +import { setSummary, annotateFileError, setOutput } from "../src/error-reporting.js"; import fs from "fs/promises"; describe("ErrorReporting", () => { @@ -12,13 +12,21 @@ describe("ErrorReporting", () => { // ensure that on test runs GITHUB_STEP_SUMMARY is not set in my current env by default // this gives us a clean slate for each test delete process.env.GITHUB_STEP_SUMMARY; + delete process.env.GITHUB_OUTPUT; }); afterEach(() => { logSpy.mockRestore(); }); - it("should warn when GITHUB_STEP_SUMMARY is unset", () => { + it("should warn when calling setOutput when GITHUB_OUTPUT is unset", () => { + setOutput("test", "value"); + expect(logSpy).toHaveBeenCalledWith( + "GITHUB_OUTPUT is not set. Skipping test update with value 'value.'", + ); + }); + + it("should warn when calling setSummary when GITHUB_STEP_SUMMARY is unset", () => { setSummary("hello"); expect(logSpy).toHaveBeenCalledWith("GITHUB_STEP_SUMMARY is not set. Skipping summary update."); }); @@ -42,6 +50,22 @@ describe("ErrorReporting", () => { expect(content).toBe("# Title"); }); + it("should write an output when GITHUB_OUTPUT is set", async () => { + process.env.GITHUB_OUTPUT = `${__dirname}/tmp-output.txt`; + + setOutput("test", "value"); + + expect(logSpy).not.toHaveBeenCalledWith( + "GITHUB_OUTPUT is not set. Skipping test update with value 'value.'", + ); + + const content = await fs.readFile(process.env.GITHUB_OUTPUT, "utf-8"); + expect(content).toBe("test=value\n"); + + // cleanup after the test so nothing is left behind + await fs.unlink(process.env.GITHUB_OUTPUT); + }); + it("should emit a GitHub-style error annotation", () => { annotateFileError("src/foo.js", "Something broke", 42, 7); expect(logSpy).toHaveBeenCalledWith("::error file=src/foo.js,line=42,col=7::Something broke"); diff --git a/.github/shared/test/simple-git.test.js b/.github/shared/test/simple-git.test.js new file mode 100644 index 000000000000..3fbd308430a7 --- /dev/null +++ b/.github/shared/test/simple-git.test.js @@ -0,0 +1,20 @@ +import path from "path"; +import os from "os"; +import { mkdtemp, rm } from "fs/promises"; +import { getRootFolder } from "../src/simple-git.js"; +import { describe, expect, it } from "vitest"; + +describe("getRootFolder", () => { + it("resolves to repo root from a nested folder", async () => { + const testDir = __dirname; + const calculatedRoot = await getRootFolder(testDir); + const expectedRoot = path.resolve(path.join(__dirname, "..", "..", "..")); + expect(calculatedRoot).toBe(expectedRoot); + }); + + it("throws when directory is not a git repository", async () => { + const tempDir = await mkdtemp(path.join(os.tmpdir(), "non-git-")); + await expect(getRootFolder(tempDir)).rejects.toThrow(); + await rm(tempDir, { recursive: true, force: true }); + }); +}); diff --git a/.github/workflows/src/summarize-checks/label-rules.js b/.github/workflows/src/summarize-checks/labelling.js similarity index 60% rename from .github/workflows/src/summarize-checks/label-rules.js rename to .github/workflows/src/summarize-checks/labelling.js index a21205c8eed2..aab8a2203a38 100644 --- a/.github/workflows/src/summarize-checks/label-rules.js +++ b/.github/workflows/src/summarize-checks/labelling.js @@ -1,8 +1,7 @@ /* - This file determines what labels are required for a given PR. It uses this context to obtain two key - pieces of information: - 1. The labels that are currently present on the PR. - 2. The target branch of the PR. + This file covers two areas of enforcement: + 1. It calculates what set of label rules has been violated by the current PR, for the purposes of updating next steps to merge. + 2. It calculates what set of labels should be added or removed from the PR. */ import { @@ -13,6 +12,56 @@ import { wrapInArmReviewMessage, } from "./tsgs.js"; +// #region typedefs +/** + * The LabelContext is used by the updateLabels() to determine which labels to add or remove to the PR. + * + * The "present" set represents the set of labels that are currently present on the PR and should be populated + * ONCE at the beginning of the summarize-checks action script. + * + * The "toAdd" set is the set of labels to be added to the PR at the end of invocation of updateLabels(). + * This is to be done by calling GitHub Octokit API to add the labels. + * + * The "toRemove" set is analogous to "toAdd" set, but instead it is the set of labels to be removed. + * + * The general pattern used in the code to populate "toAdd" or "toRemove" sets to be ready for + * Octokit invocation is as follows: + * + * - the summary() function passes the context through its invocation chain. + * - given function responsible for given label, like e.g. for label "ARMReview", + * creates a new instance of Label: const armReviewLabel = new Label("ARMReview", labelContext.present) + * - the function then processes the label to determine if armReviewLabel.shouldBePresent is to be set to true or false. + * - the function at the end of its invocation calls armReviewLabel.applyStateChanges(labelContext.toAdd, labelContext.toRemove) + * to update the sets. + * - the function may optionally return { armReviewLabel.shouldBePresent } to allow the caller to pass this value + * further to downstream business logic that depends on it. + * - at the end of invocation summary() calls Octokit passing it as input labelContext.toAdd and labelContext.toRemove. + */ +/** + * @typedef {Object} LabelContext + * @property {Set} present - The current set of labels + * @property {Set} toAdd - The set of labels to add + * @property {Set} toRemove - The set of labels to remove + */ + +/** + * @typedef {Object} ImpactAssessment + * @property {boolean} resourceManagerRequired - Whether a resource manager review is required. + * @property {boolean} suppressionReviewRequired - Whether a suppression review is required. + * @property {boolean} versioningReviewRequired - Whether a versioning policy review is required. + * @property {boolean} breakingChangeReviewRequired - Whether a breaking change review is required. + * @property {boolean} isNewApiVersion - Whether this PR introduces a new API version. + * @property {boolean} rpaasExceptionRequired - Whether an RPaaS exception is required. + * @property {boolean} rpaasRpNotInPrivateRepo - Whether the RPaaS RP is not present in the private repo. + * @property {boolean} rpaasChange - Whether this PR includes RPaaS changes. + * @property {boolean} newRP - Whether this PR introduces a new resource provider. + * @property {boolean} rpaasRPMissing - Whether the RPaaS RP label is missing. + * @property {boolean} typeSpecChanged - Whether a TypeSpec file has changed. + * @property {boolean} isDraft - Whether the PR is a draft. + * @property {LabelContext} labelContext - The context containing present, to-add, and to-remove labels. + * @property {string} targetBranch - The name of the target branch for the PR. + */ + /** * This file is the single source of truth for the labels used by the SDK generation tooling * in the Azure/azure-rest-api-specs and Azure/azure-rest-api-specs-pr repositories. @@ -23,6 +72,230 @@ import { * - https://microsoftapc-my.sharepoint.com/:w:/g/personal/raychen_microsoft_com/EbOAA9SkhQhGlgxtf7mc0kUB-25bFue0EFbXKXS3TFLTQA */ +/** + * RequiredLabelRule: + * IF ((any of the anyPrerequisiteLabels is present + * OR all of the allPrerequisiteLabels are present) + * AND none of the allPrerequisiteAbsentLabels is present) + * THEN any of the anyRequiredLabels is required. + * + * IF any of the anyRequiredLabels is required + * THEN display the troubleshootingGuide in "Next Steps to Merge" comment. + * + * @typedef {Object} RequiredLabelRule + * @property {number} precedence - If multiple RequiredLabelRules are violated, the one with lowest + * precedence should be displayed to the user first. If multiple rules have + * the same precedence, and one of them should be displayed, + * then all of them should be displayed. + * + * Note this independent of the CheckMetadata.precedence. That is, + * if there are failing checks, and failing required label rules, + * both of them will be shown, both taking appropriate lowest precedence. + * @property {string[]} [branches] - Branches, in format "repo/branch", e.g. "azure-rest-api-specs/main", + * to which this required label rule applies. + * + * To be exact: + * - If there is at least one branch defined, and the evaluated PR is + * not targeting any of the branches defined, then the rule is not applicable (it implicitly passes). + * - If "branches" is empty or undefined, then the rule applies to all branches in all repos. + * @property {string[]} [anyPrerequisiteLabels] - If any of anyPrerequisiteLabels is present, the requiredLabel is required. + * This condition is ORed with allPrerequisiteLabels. + * + * If the anyRequiredLabels collection is empty or undefined, anyPrerequisiteLabels must have exactly one entry. + * @property {string[]} [allPrerequisiteLabels] - If all of allPrerequisiteLabels are present, the requiredLabel is required. + * This condition is ORed with anyPrerequisiteLabels. + * + * If the anyRequiredLabels collection is empty or undefined, allPrerequisiteLabels must be empty or undefined. + * @property {string[]} [allPrerequisiteAbsentLabels] - If any of the allPrerequisiteAbsentLabels is present, + * the requiredLabel is not required. + * @property {string[]} anyRequiredLabels - If any of the labels in anyRequiredLabels is present, + * then the rule prerequisites, as expressed by anyPrerequisiteLabels and allPrerequisiteLabels, are met. + * Conversely, if none of anyRequiredLabels are present, then the rule is violated. + * + * For the purposes of determining which label to display as required in the 'automated merging requirements met' check and + * 'Next Steps to Merge" comments, the first label in anyRequiredLabels is used. + * The assumption here is that anyRequiredLabels[0] is the 'current' label, + * while the remaining required label options are 'legacy' labels. + * + * If given required label string ends with an asterisk, it is treated as a prefix substring match. + * For example, requiredLabel of 'Foo-Approved-*' means that any label with prefix 'Foo-Approved-' will satisfy the rule. + * For example, 'Foo-Approved-Bar' or 'Foo-Approved-Qux', but not 'Foo-Approved' nor 'Foo-Approved-'. + * + * If anyRequiredLabels is an empty array, then if the rule is violated, there is no way to meet the rule prerequisites. + * @property {string} troubleshootingGuide - The doc to display to the user if the required label is required, but missing. + */ + +/** + * This file is the single source of truth for types used by the OpenAPI specification breaking change checks + * in the Azure/azure-rest-api-specs and Azure/azure-rest-api-specs-pr repositories. + * + * For additional context, see: + * + * - "Deep-dive into breaking changes on spec PRs" + * https://aka.ms/azsdk/pr-brch-deep + * + * - "[Breaking Change][PR Workflow] Use more granular labels for Breaking Changes approvals" + * https://github.com/Azure/azure-sdk-tools/issues/6374 + */ +/** + * @typedef {"SameVersion" | "CrossVersion"} BreakingChangesCheckType + * @typedef {"VersioningReviewRequired" | "BreakingChangeReviewRequired"} ReviewRequiredLabel + * @typedef {"Versioning-Approved-*" | "BreakingChange-Approved-*"} ReviewApprovalPrefixLabel + * @typedef {"Versioning-Approved-Benign" | "Versioning-Approved-BugFix" | "Versioning-Approved-PrivatePreview" | "Versioning-Approved-BranchPolicyException" | "Versioning-Approved-Previously" | "Versioning-Approved-Retired"} ValidVersioningApproval + * @typedef {"BreakingChange-Approved-Benign" | "BreakingChange-Approved-BugFix" | "BreakingChange-Approved-UserImpact" | "BreakingChange-Approved-BranchPolicyException" | "BreakingChange-Approved-Previously" | "BreakingChange-Approved-Security"} ValidBreakingChangeApproval + * @typedef {ReviewRequiredLabel | ReviewApprovalPrefixLabel | ValidBreakingChangeApproval | ValidVersioningApproval} SpecsBreakingChangesLabel + */ +/** + * @typedef {Object} BreakingChangesCheckConfig + * @property {ReviewRequiredLabel} reviewRequiredLabel + * @property {ReviewApprovalPrefixLabel} approvalPrefixLabel + * @property {(ValidVersioningApproval | ValidBreakingChangeApproval)[]} approvalLabels + * @property {string} [deprecatedReviewRequiredLabel] + */ + +/** + * @typedef {"SwaggerFile" | "TypeSpecFile" | "ExampleFile" | "ReadmeFile"} FileTypes + */ + +/** + * @typedef {"Addition" | "Deletion" | "Update"} ChangeTypes + */ + +/** + * @typedef {Object} PRChange + * @property {FileTypes} fileType + * @property {ChangeTypes} changeType + * @property {string} filePath + * @property {any} [additionalInfo] + */ + +/** + * @typedef {Object} ChangeHandler + * @property {function(PRChange): void} [SwaggerFile] + * @property {function(PRChange): void} [TypeSpecFile] + * @property {function(PRChange): void} [ExampleFile] + * @property {function(PRChange): void} [ReadmeFile] + */ + +/** + * @typedef {"resource-manager" | "data-plane"} PRType + */ + +/** + * Represents a GitHub label. + * Currently used in the context of processing + * labels related to the review workflow of PRs submitted to + * Azure/azure-rest-api-specs and Azure/azure-rest-api-specs-pr repositories. + * This processing happens in the prSummary.ts / summary() function. + * + * See also: https://aka.ms/SpecPRReviewARMInvariants + */ +// todo: inject `core` for access to logging +export class Label { + /** + * @param {string} name + * @param {Set} [presentLabels] + */ + constructor(name, presentLabels) { + /** @type {string} */ + this.name = name; + + /** + * Is the label currently present on the pull request? + * This is determined at the time of construction of this object. + * @type {boolean | undefined} + */ + this.present = presentLabels?.has(this.name) ?? undefined; + + /** + * Should this label be present on the pull request? + * Must be defined before applyStateChange is called. + * Not set at the construction time to facilitate determining desired presence + * of multiple labels in single code block, without intermixing it with + * label construction logic. + * @type {boolean | undefined} + */ + this.shouldBePresent = undefined; + } + + /** + * If the label should be added, add its name to labelsToAdd. + * If the label should be removed, add its name to labelsToRemove. + * Otherwise, do nothing. + * + * Precondition: this.shouldBePresent has been defined. + * @param {Set} labelsToAdd + * @param {Set} labelsToRemove + */ + applyStateChange(labelsToAdd, labelsToRemove) { + if (this.shouldBePresent === undefined) { + console.warn( + "ASSERTION VIOLATION! " + + `Cannot applyStateChange for label '${this.name}' ` + + "as its desired presence hasn't been defined. Returning early.", + ); + throw new Error( + `Label '${this.name}' has not been properly initialized with shouldBePresent before being applied.`, + ); + } + + if (!this.present && this.shouldBePresent) { + if (!labelsToAdd.has(this.name)) { + console.log( + `Label.applyStateChange: '${this.name}' was not present and should be present. Scheduling addition.`, + ); + labelsToAdd.add(this.name); + } else { + console.log( + `Label.applyStateChange: '${this.name}' was not present and should be present. It is already scheduled for addition.`, + ); + } + } else if (this.present && !this.shouldBePresent) { + if (!labelsToRemove.has(this.name)) { + console.log( + `Label.applyStateChange: '${this.name}' was present and should not be present. Scheduling removal.`, + ); + labelsToRemove.add(this.name); + } else { + console.log( + `Label.applyStateChange: '${this.name}' was present and should not be present. It is already scheduled for removal.`, + ); + } + } else if (this.present === this.shouldBePresent) { + console.log( + `Label.applyStateChange: '${this.name}' is ${this.present ? "present" : "not present"}. This is the desired state.`, + ); + } else { + console.warn( + "ASSERTION VIOLATION! " + + `Label.applyStateChange: '${this.name}' is ${this.present ? "present" : "not present"} while it should be ${this.shouldBePresent ? "present" : "not present"}. ` + + `At this point of execution this should not happen.`, + ); + } + } + + /** + * @param {string} label + * @returns {boolean} + */ + isEqualToOrPrefixOf(label) { + return this.name.endsWith("*") ? label.startsWith(this.name.slice(0, -1)) : this.name === label; + } + + /** + * @returns {string} + */ + logString() { + return ( + `Label: name: ${this.name}, ` + + `present: ${this.present}, ` + + `shouldBePresent: ${this.shouldBePresent}. ` + ); + } +} + +// #endregion typedefs +// #region constants export const sdkLabels = { "azure-cli-extensions": { breakingChange: undefined, @@ -99,38 +372,10 @@ export const sdkLabels = { }, }; -/** - * This file is the single source of truth for types used by the OpenAPI specification breaking change checks - * in the Azure/azure-rest-api-specs and Azure/azure-rest-api-specs-pr repositories. - * - * For additional context, see: - * - * - "Deep-dive into breaking changes on spec PRs" - * https://aka.ms/azsdk/pr-brch-deep - * - * - "[Breaking Change][PR Workflow] Use more granular labels for Breaking Changes approvals" - * https://github.com/Azure/azure-sdk-tools/issues/6374 - */ -/** - * @typedef {"SameVersion" | "CrossVersion"} BreakingChangesCheckType - * @typedef {"VersioningReviewRequired" | "BreakingChangeReviewRequired"} ReviewRequiredLabel - * @typedef {"Versioning-Approved-*" | "BreakingChange-Approved-*"} ReviewApprovalPrefixLabel - * @typedef {"Versioning-Approved-Benign" | "Versioning-Approved-BugFix" | "Versioning-Approved-PrivatePreview" | "Versioning-Approved-BranchPolicyException" | "Versioning-Approved-Previously" | "Versioning-Approved-Retired"} ValidVersioningApproval - * @typedef {"BreakingChange-Approved-Benign" | "BreakingChange-Approved-BugFix" | "BreakingChange-Approved-UserImpact" | "BreakingChange-Approved-BranchPolicyException" | "BreakingChange-Approved-Previously" | "BreakingChange-Approved-Security"} ValidBreakingChangeApproval - * @typedef {ReviewRequiredLabel | ReviewApprovalPrefixLabel | ValidBreakingChangeApproval | ValidVersioningApproval} SpecsBreakingChangesLabel - */ -/** - * @typedef {Object} BreakingChangesCheckConfig - * @property {ReviewRequiredLabel} reviewRequiredLabel - * @property {ReviewApprovalPrefixLabel} approvalPrefixLabel - * @property {(ValidVersioningApproval | ValidBreakingChangeApproval)[]} approvalLabels - * @property {string} [deprecatedReviewRequiredLabel] - */ - /** * @type {Record} */ -// todo: pull this from eng/tools/openapi-diff-runner/src/types/breaking-change.ts +// todo: pull values from eng/tools/openapi-diff-runner/src/types/breaking-change.ts export const breakingChangesCheckType = { SameVersion: { reviewRequiredLabel: "VersioningReviewRequired", @@ -158,6 +403,355 @@ export const breakingChangesCheckType = { }, }; +// #endregion constants +// #region Required Labels + +/** + * @param {LabelContext} context + * @param {string[]} existingLabels + * @returns {void} + */ +export function processArmReviewLabels(context, existingLabels) { + // the important part about how this will work depends how the users use it + // EG: if they add the "ARMSignedOff" label, we will remove the "ARMChangesRequested" and "WaitForARMFeedback" labels. + // if they add the "ARMChangesRequested" label, we will remove the "WaitForARMFeedback" label. + // if they remove the "ARMChangesRequested" label, we will add the "WaitForARMFeedback" label. + // so if the user or ARM team actually unlabels `ARMChangesRequested`, then we're actually ok + // if we are signed off, we should remove the "ARMChangesRequested" and "WaitForARMFeedback" labels + if (containsAll(existingLabels, ["ARMSignedOff"])) { + if (existingLabels.includes("ARMChangesRequested")) { + context.toRemove.add("ARMChangesRequested"); + } + if (existingLabels.includes("WaitForARMFeedback")) { + context.toRemove.add("WaitForARMFeedback"); + } + } + // if there are ARM changes requested, we should remove the "WaitForARMFeedback" label as the presence indicates that ARM has reviewed + else if ( + containsAll(existingLabels, ["ARMChangesRequested"]) && + containsNone(existingLabels, ["ARMSignedOff"]) + ) { + if (existingLabels.includes("WaitForARMFeedback")) { + context.toRemove.add("WaitForARMFeedback"); + } + } + // finally, if ARMChangesRequested are not present, and we've gotten here by lac;k of signoff, we should add the "WaitForARMFeedback" label + else if (containsNone(existingLabels, ["ARMChangesRequested"])) { + if (!existingLabels.includes("WaitForARMFeedback")) { + context.toAdd.add("WaitForARMFeedback"); + } + } +} + +/** + * + * @param {any[]} arr + * @param {any[]} values + * @returns + */ +function containsAll(arr, values) { + return values.every((value) => arr.includes(value)); +} + +/** + * + * @param {any[]} arr + * @param {any[]} values + * @returns + */ +function containsNone(arr, values) { + return values.every((value) => !arr.includes(value)); +} + +/** +This function determines which labels of the ARM review should +be applied to given PR. It adds and removes the labels as appropriate. It used to be called +ProcessARMReview, but was renamed to processImpactAssessment to better reflect its purpose given that is +merely evaluating the impact assessment of the PR and returning a final set of labels to be applied/removed +from the PR. + +This function does the following, **among other things**: + +- Adds the "ARMReview" label if all of the following conditions hold: + - The processed PR "isReleaseBranch" or "isShiftLeftPRWithRPSaaSDev" + - The PR is not a draft, as determined by "isDraftPR" + - The PR is labelled with "resource-manager" label, meaning it pertains + to ARM, as previously determined by the "isManagementPR" function, + called from the "getPRType" function. + +- Calls the "processARMReviewWorkflowLabels" function if "ARMReview" label applies. +*/ +// todo: refactor to take context: PRContext as input instead of IValidatorContext. +// All downstream usage appears to be using "context.contextConfig() as PRContext". +/** + * @param {string} targetBranch + * @param {LabelContext} labelContext + * @param {boolean} resourceManagerLabelShouldBePresent + * @param {boolean} versioningReviewRequiredLabelShouldBePresent + * @param {boolean} breakingChangeReviewRequiredLabelShouldBePresent + * @param {boolean} ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent + * @param {boolean} rpaasExceptionLabelShouldBePresent + * @param {boolean} ciRpaasRPNotInPrivateRepoLabelShouldBePresent + * @param {boolean} isNewApiVersion + * @param {boolean} isDraft + * @returns {Promise<{armReviewLabelShouldBePresent: boolean}>} + */ +export async function processImpactAssessment( + targetBranch, + labelContext, + resourceManagerLabelShouldBePresent, + versioningReviewRequiredLabelShouldBePresent, + breakingChangeReviewRequiredLabelShouldBePresent, + ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent, + rpaasExceptionLabelShouldBePresent, + ciRpaasRPNotInPrivateRepoLabelShouldBePresent, + isNewApiVersion, + isDraft, +) { + console.log("ENTER definition processARMReview"); + + const armReviewLabel = new Label("ARMReview", labelContext.present); + // By default this label should not be present. We may determine later in this function that it should be present after all. + armReviewLabel.shouldBePresent = false; + + const newApiVersionLabel = new Label("new-api-version", labelContext.present); + // By default this label should not be present. We may determine later in this function that it should be present after all. + newApiVersionLabel.shouldBePresent = false; + + const branch = targetBranch; + const isReleaseBranchVal = isReleaseBranch(branch); + + // we used to also calculate if the branch name was from ShiftLeft, in which case we would or that + // with isReleaseBranchVal to see if it's in scope of ARM review. ShiftLeft is not supported anymore, + // so we only check if the branch is a release branch. + // const prTitle = await getPrTitle(owner, repo, prNumber) + // const isShiftLeftPRWithRPSaaSDevVal = isShiftLeftPRWithRPSaaSDev(prTitle, branch) + const isBranchInScopeOfSpecReview = isReleaseBranchVal; // || isShiftLeftPRWithRPSaaSDevVal + + // 'specReviewApplies' means that either ARM or data-plane review applies. Downstream logic + // determines which kind of review exactly we need. + let specReviewApplies = !isDraft && isBranchInScopeOfSpecReview; + if (specReviewApplies) { + if (isNewApiVersion) { + // Note that in case of data-plane PRs, the addition of this label will result + // in API stewardship board review being required. + // See requiredLabelsRules.ts. + newApiVersionLabel.shouldBePresent = true; + } + + armReviewLabel.shouldBePresent = resourceManagerLabelShouldBePresent; + await processARMReviewWorkflowLabels( + labelContext, + armReviewLabel.shouldBePresent, + versioningReviewRequiredLabelShouldBePresent, + breakingChangeReviewRequiredLabelShouldBePresent, + ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent, + rpaasExceptionLabelShouldBePresent, + ciRpaasRPNotInPrivateRepoLabelShouldBePresent, + ); + } + + newApiVersionLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + armReviewLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + + console.log( + `RETURN definition processARMReview. ` + + `isReleaseBranch: ${isReleaseBranchVal}, ` + + `isBranchInScopeOfArmReview: ${isBranchInScopeOfSpecReview}, ` + + `isNewApiVersion: ${isNewApiVersion}, ` + + `isDraft: ${isDraft}, ` + + `newApiVersionLabel.shouldBePresent: ${newApiVersionLabel.shouldBePresent}, ` + + `armReviewLabel.shouldBePresent: ${armReviewLabel.shouldBePresent}.`, + ); + + return { armReviewLabelShouldBePresent: armReviewLabel.shouldBePresent }; +} + +/** + * @param {string} branchName + * @returns {boolean} + */ +function isReleaseBranch(branchName) { + const branchRegex = [/main/, /RPSaaSMaster/, /release*/, /ARMCoreRPDev/]; + return branchRegex.some((b) => b.test(branchName)); +} + +/** +CODESYNC: +- requiredLabelsRules.ts / requiredLabelsRules +- https://github.com/Azure/azure-rest-api-specs/blob/main/.github/comment.yml + +This function determines which label from the ARM review workflow labels +should be present on the PR. It adds and removes the labels as appropriate. + +In other words, this function captures the +ARM review workflow label processing logic. + +To be exact, this function executes if and only if the PR in question +has been determined to have the "ARMReview" label, denoting given PR +is in scope for ARM review. + +The implementation of this function is the source of truth specifying the +desired behavior. + +To understand this implementation, the most important constraint to keep in mind +is that if "ARMReview" label is present, then exactly one of the following +labels must be present: + +- NotReadyForARMReview +- WaitForARMFeedback +- ARMChangesRequested +- ARMSignedOff + +Note that another important place in this codebase where ARM review workflow +labels are being removed or added to a PR is pipelineBotOnPRLabelEvent.ts. +*/ +/** + * @param {LabelContext} labelContext + * @param {boolean} armReviewLabelShouldBePresent + * @param {boolean} versioningReviewRequiredLabelShouldBePresent + * @param {boolean} breakingChangeReviewRequiredLabelShouldBePresent + * @param {boolean} ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent + * @param {boolean} rpaasExceptionLabelShouldBePresent + * @param {boolean} ciRpaasRPNotInPrivateRepoLabelShouldBePresent + * @returns {Promise} + */ +async function processARMReviewWorkflowLabels( + labelContext, + armReviewLabelShouldBePresent, + versioningReviewRequiredLabelShouldBePresent, + breakingChangeReviewRequiredLabelShouldBePresent, + ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent, + rpaasExceptionLabelShouldBePresent, + ciRpaasRPNotInPrivateRepoLabelShouldBePresent, +) { + console.log("ENTER definition processARMReviewWorkflowLabels"); + + const notReadyForArmReviewLabel = new Label("NotReadyForARMReview", labelContext.present); + + const waitForArmFeedbackLabel = new Label("WaitForARMFeedback", labelContext.present); + + const armChangesRequestedLabel = new Label("ARMChangesRequested", labelContext.present); + + const armSignedOffLabel = new Label("ARMSignedOff", labelContext.present); + + const blockedOnVersioningPolicy = getBlockedOnVersioningPolicy( + labelContext, + breakingChangeReviewRequiredLabelShouldBePresent, + versioningReviewRequiredLabelShouldBePresent, + ); + + const blockedOnRpaas = getBlockedOnRpaas( + ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent, + rpaasExceptionLabelShouldBePresent, + ciRpaasRPNotInPrivateRepoLabelShouldBePresent, + ); + + const blocked = blockedOnVersioningPolicy || blockedOnRpaas; + + // If given PR is in scope of ARM review and it is blocked for any reason, + // the "NotReadyForARMReview" label should be present, to the exclusion + // of all other ARM review workflow labels. + notReadyForArmReviewLabel.shouldBePresent = armReviewLabelShouldBePresent && blocked; + + // If given PR is in scope of ARM review and the review is not blocked, + // then "ARMSignedOff" label should remain present on the PR if it was + // already present. This means that labels "ARMChangesRequested" + // and "WaitForARMFeedback" are invalid and will be removed by automation + // in presence of "ARMSignedOff". + armSignedOffLabel.shouldBePresent = + armReviewLabelShouldBePresent && !blocked && armSignedOffLabel.present; + + // If given PR is in scope of ARM review and the review is not blocked and + // not signed-off, then the label "ARMChangesRequested" should remain present + // if it was already present. This means that labels "WaitForARMFeedback" + // is invalid and will be removed by automation in presence of + // "WaitForARMFeedback". + armChangesRequestedLabel.shouldBePresent = + armReviewLabelShouldBePresent && + !blocked && + !armSignedOffLabel.shouldBePresent && + armChangesRequestedLabel.present; + + // If given PR is in scope of ARM review and the review is not blocked and + // not signed-off, and ARM reviewer didn't request any changes, + // then the label "WaitForARMFeedback" should be present on the PR, whether + // it was present before or not. + waitForArmFeedbackLabel.shouldBePresent = + armReviewLabelShouldBePresent && + !blocked && + !armSignedOffLabel.shouldBePresent && + !armChangesRequestedLabel.shouldBePresent && + (waitForArmFeedbackLabel.present || true); + + const exactlyOneArmReviewWorkflowLabelShouldBePresent = + Number(notReadyForArmReviewLabel.shouldBePresent) + + Number(armSignedOffLabel.shouldBePresent) + + Number(armChangesRequestedLabel.shouldBePresent) + + Number(waitForArmFeedbackLabel.shouldBePresent) === + 1 || !armReviewLabelShouldBePresent; + + if (!exactlyOneArmReviewWorkflowLabelShouldBePresent) { + console.warn("ASSERTION VIOLATION! exactlyOneArmReviewWorkflowLabelShouldBePresent is false"); + } + + notReadyForArmReviewLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + armSignedOffLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + armChangesRequestedLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + waitForArmFeedbackLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + + console.log( + `RETURN definition processARMReviewWorkflowLabels. ` + + `presentLabels: ${[...labelContext.present].join(",")}, ` + + `blockedOnVersioningPolicy: ${blockedOnVersioningPolicy}. ` + + `blockedOnRpaas: ${blockedOnRpaas}. ` + + `exactlyOneArmReviewWorkflowLabelShouldBePresent: ${exactlyOneArmReviewWorkflowLabelShouldBePresent}. `, + ); + return; +} + +/** + * @param {LabelContext} labelContext + * @param {boolean} breakingChangeReviewRequiredLabelShouldBePresent + * @param {boolean} versioningReviewRequiredLabelShouldBePresent + * @returns {boolean} + */ +function getBlockedOnVersioningPolicy( + labelContext, + breakingChangeReviewRequiredLabelShouldBePresent, + versioningReviewRequiredLabelShouldBePresent, +) { + const pendingVersioningReview = + versioningReviewRequiredLabelShouldBePresent && + !anyApprovalLabelPresent("SameVersion", [...labelContext.present]); + + const pendingBreakingChangeReview = + breakingChangeReviewRequiredLabelShouldBePresent && + !anyApprovalLabelPresent("CrossVersion", [...labelContext.present]); + + const blockedOnVersioningPolicy = pendingVersioningReview || pendingBreakingChangeReview; + return blockedOnVersioningPolicy; +} + +/** + * @param {boolean} ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent + * @param {boolean} rpaasExceptionLabelShouldBePresent + * @param {boolean} ciRpaasRPNotInPrivateRepoLabelShouldBePresent + * @returns {boolean} + */ +function getBlockedOnRpaas( + ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent, + rpaasExceptionLabelShouldBePresent, + ciRpaasRPNotInPrivateRepoLabelShouldBePresent, +) { + return ( + (ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent && !rpaasExceptionLabelShouldBePresent) || + ciRpaasRPNotInPrivateRepoLabelShouldBePresent + ); +} + +// #endregion +// #region LabelRules /** * @param {BreakingChangesCheckType} approvalType * @param {string[]} labels @@ -169,59 +763,6 @@ export function anyApprovalLabelPresent(approvalType, labels) { return anyLabelMatches(labelsToMatchAgainst, labels); } -/** - * RequiredLabelRule: - * IF ((any of the anyPrerequisiteLabels is present - * OR all of the allPrerequisiteLabels are present) - * AND none of the allPrerequisiteAbsentLabels is present) - * THEN any of the anyRequiredLabels is required. - * - * IF any of the anyRequiredLabels is required - * THEN display the troubleshootingGuide in "Next Steps to Merge" comment. - * - * @typedef {Object} RequiredLabelRule - * @property {number} precedence - If multiple RequiredLabelRules are violated, the one with lowest - * precedence should be displayed to the user first. If multiple rules have - * the same precedence, and one of them should be displayed, - * then all of them should be displayed. - * - * Note this independent of the CheckMetadata.precedence. That is, - * if there are failing checks, and failing required label rules, - * both of them will be shown, both taking appropriate lowest precedence. - * @property {string[]} [branches] - Branches, in format "repo/branch", e.g. "azure-rest-api-specs/main", - * to which this required label rule applies. - * - * To be exact: - * - If there is at least one branch defined, and the evaluated PR is - * not targeting any of the branches defined, then the rule is not applicable (it implicitly passes). - * - If "branches" is empty or undefined, then the rule applies to all branches in all repos. - * @property {string[]} [anyPrerequisiteLabels] - If any of anyPrerequisiteLabels is present, the requiredLabel is required. - * This condition is ORed with allPrerequisiteLabels. - * - * If the anyRequiredLabels collection is empty or undefined, anyPrerequisiteLabels must have exactly one entry. - * @property {string[]} [allPrerequisiteLabels] - If all of allPrerequisiteLabels are present, the requiredLabel is required. - * This condition is ORed with anyPrerequisiteLabels. - * - * If the anyRequiredLabels collection is empty or undefined, allPrerequisiteLabels must be empty or undefined. - * @property {string[]} [allPrerequisiteAbsentLabels] - If any of the allPrerequisiteAbsentLabels is present, - * the requiredLabel is not required. - * @property {string[]} anyRequiredLabels - If any of the labels in anyRequiredLabels is present, - * then the rule prerequisites, as expressed by anyPrerequisiteLabels and allPrerequisiteLabels, are met. - * Conversely, if none of anyRequiredLabels are present, then the rule is violated. - * - * For the purposes of determining which label to display as required in the 'automated merging requirements met' check and - * 'Next Steps to Merge" comments, the first label in anyRequiredLabels is used. - * The assumption here is that anyRequiredLabels[0] is the 'current' label, - * while the remaining required label options are 'legacy' labels. - * - * If given required label string ends with an asterisk, it is treated as a prefix substring match. - * For example, requiredLabel of 'Foo-Approved-*' means that any label with prefix 'Foo-Approved-' will satisfy the rule. - * For example, 'Foo-Approved-Bar' or 'Foo-Approved-Qux', but not 'Foo-Approved' nor 'Foo-Approved-'. - * - * If anyRequiredLabels is an empty array, then if the rule is violated, there is no way to meet the rule prerequisites. - * @property {string} troubleshootingGuide - The doc to display to the user if the required label is required, but missing. - */ - /** * @param {RequiredLabelRule} rule * @returns {boolean} @@ -772,3 +1313,4 @@ export function isEqualToOrPrefixOf(inputstring, label) { ? label.startsWith(inputstring.slice(0, -1)) : inputstring === label; } +// #endregion diff --git a/.github/workflows/src/summarize-checks/summarize-checks.js b/.github/workflows/src/summarize-checks/summarize-checks.js index 07b0051c67eb..3504f81aa507 100644 --- a/.github/workflows/src/summarize-checks/summarize-checks.js +++ b/.github/workflows/src/summarize-checks/summarize-checks.js @@ -23,7 +23,14 @@ import { extractInputs } from "../context.js"; // eslint-disable-next-line no-unused-vars import { commentOrUpdate } from "../comment.js"; import { PER_PAGE_MAX } from "../github.js"; -import { verRevApproval, brChRevApproval, getViolatedRequiredLabelsRules } from "./label-rules.js"; +import { execFile } from "../../../shared/src/exec.js"; +import { + verRevApproval, + brChRevApproval, + getViolatedRequiredLabelsRules, + processArmReviewLabels, + processImpactAssessment, +} from "./labelling.js"; import { brchTsg, @@ -35,6 +42,10 @@ import { typeSpecRequirementDataPlaneTsg, } from "./tsgs.js"; +import fs from "fs/promises"; +import os from "os"; +import path from "path"; + /** * @typedef {Object} CheckMetadata * @property {number} precedence @@ -52,7 +63,67 @@ import { */ /** - * @typedef {import("./label-rules.js").RequiredLabelRule} RequiredLabelRule + * @typedef {Object} WorkflowRunArtifact + * @property {string} name + * @property {number} id + * @property {string} url + * @property {string} archive_download_url + */ + +/** + * @typedef {Object} WorkflowRunInfo + * @property {string} name + * @property {number} id + * @property {number} databaseId + * @property {string} url + * @property {number} workflowId + * @property {string} status + * @property {string} conclusion + * @property {string} createdAt + * @property {string} updatedAt + */ + +/** + * @typedef {Object} GraphQLCheckRun + * @property {string} name + * @property {string} status + * @property {string} conclusion + * @property {boolean} isRequired + */ + +/** + * @typedef {Object} GraphQLCheckSuite + * @property {GraphQLCheckRun[]} nodes + */ + +/** + * @typedef {Object} GraphQLCheckSuites + * @property {GraphQLCheckSuite[]} nodes + */ + +/** + * @typedef {Object} GraphQLCommit + * @property {GraphQLCheckSuites} checkSuites + */ + +/** + * @typedef {Object} GraphQLResource + * @property {GraphQLCheckSuites} checkSuites + */ + +/** + * @typedef {Object} GraphQLResponse + * @property {GraphQLResource} resource + * @property {Object} rateLimit + * @property {number} rateLimit.limit + * @property {number} rateLimit.cost + * @property {number} rateLimit.used + * @property {number} rateLimit.remaining + * @property {string} rateLimit.resetAt + */ + +/** + * @typedef {import("./labelling.js").RequiredLabelRule} RequiredLabelRule */ // Placing these configuration items here until we decide another way to pull them in. @@ -209,7 +280,6 @@ const EXCLUDED_CHECK_NAMES = []; * @returns {Promise} */ export default async function summarizeChecks({ github, context, core }) { - logGitHubRateLimitInfo(github, core); let { owner, repo, issue_number, head_sha } = await extractInputs(github, context, core); const targetBranch = context.payload.pull_request?.base?.ref; core.info(`PR target branch: ${targetBranch}`); @@ -254,43 +324,16 @@ export async function summarizeChecksImpl( `Handling ${event_name} event for PR #${issue_number} in ${owner}/${repo} with targeted branch ${targetBranch}`, ); + // retrieve latest labels state const labels = await github.paginate(github.rest.issues.listLabelsOnIssue, { - owner: owner, - repo: repo, + owner, + repo, issue_number: issue_number, per_page: PER_PAGE_MAX, }); - /** @type {string[]} */ - let labelNames = labels.map((/** @type {{ name: string; }} */ label) => label.name); - - // handle our label trigger first, we may bail out early if it's a label action we're reacting to - // this also implies that if a label action is performed before any workflows complete, we shouldn't - // accidentally update the next steps to merge with the results of the workflows that haven't completed yet. - if (event_name in ["labeled", "unlabeled"]) { - // if anything goes wrong with label actions, the invocation will end within handleLabeledEvent due to localized error handling - const [labelsToAdd, labelsToRemove] = await handleLabeledEvent( - github, - context, - core, - owner, - repo, - issue_number, - event_name, - labelNames, - ); - - // adjust labelNames based on labelsToAdd/labelsToRemove - labelNames = labelNames.filter((name) => !labelsToRemove.includes(name)); - for (const label of labelsToAdd) { - if (!labelNames.includes(label)) { - labelNames.push(label); - } - } - } - - /** @type {[CheckRunData[], CheckRunData[]]} */ - const [requiredCheckRuns, fyiCheckRuns] = await getCheckRunTuple( + /** @type {[CheckRunData[], CheckRunData[], import("./labelling.js").ImpactAssessment | undefined]} */ + const [requiredCheckRuns, fyiCheckRuns, impactAssessment] = await getCheckRunTuple( github, core, owner, @@ -300,6 +343,43 @@ export async function summarizeChecksImpl( EXCLUDED_CHECK_NAMES, ); + /** @type {string[]} */ + let labelNames = labels.map((/** @type {{ name: string; }} */ label) => label.name); + + // todo: how important is it that we know if we're in draft? I don't want to have to pull the PR details unless we actually need to + // do we need to pull this from the PR? if triggered from a workflow_run we won't have payload.pullrequest populated + let labelContext = await updateLabels(labelNames, impactAssessment); + + for (const label of labelContext.toRemove) { + core.info(`Removing label: ${label} from ${owner}/${repo}#${issue_number}.`); + // await github.rest.issues.removeLabel({ + // owner: owner, + // repo: repo, + // issue_number: issue_number, + // name: label, + // }); + } + + if (labelContext.toAdd.size > 0) { + core.info( + `Adding labels: ${Array.from(labelContext.toAdd).join(", ")} to ${owner}/${repo}#${issue_number}.`, + ); + // await github.rest.issues.addLabels({ + // owner: owner, + // repo: repo, + // issue_number: issue_number, + // labels: Array.from(labelsToAdd), + // }); + } + + // adjust labelNames based on labelsToAdd/labelsToRemove + labelNames = labelNames.filter((name) => !labelContext.toRemove.has(name)); + for (const label of labelContext.toAdd) { + if (!labelNames.includes(label)) { + labelNames.push(label); + } + } + const commentBody = await createNextStepsComment( core, repo, @@ -323,21 +403,6 @@ export async function summarizeChecksImpl( // ) } -/** - * @param {import('@actions/github-script').AsyncFunctionArguments['github']} github - * @param {import('@actions/github-script').AsyncFunctionArguments['core']} core - * @returns {Promise} - */ -export async function logGitHubRateLimitInfo(github, core) { - try { - const { data: rateLimit } = await github.rest.rateLimit.get(); - const { data: user } = await github.rest.users.getAuthenticated(); - core.info(`GitHub RateLimit Info for user ${user.login}: ${JSON.stringify(rateLimit)}`); - } catch (e) { - core.error(`GitHub RateLimit Info: error emitting. Exception: ${e}`); - } -} - /** * A GraphQL query to GitHub API that returns all check runs for given commit, with "isRequired" field for given PR. * @@ -377,6 +442,13 @@ function getGraphQLQuery(owner, repo, sha, prNumber) { ... on Commit { checkSuites(first: 20) { nodes { + workflowRun { + id + databaseId + workflow { + name + } + } checkRuns(first: 30) { nodes { name @@ -403,79 +475,73 @@ function getGraphQLQuery(owner, repo, sha, prNumber) { // #endregion // #region label update /** - * @param {import('@actions/github-script').AsyncFunctionArguments['github']} github - * @param {import('@actions/github').context } context - * @param {typeof import("@actions/core")} core - * @param {string} owner - * @param {string} repo - * @param {number} issue_number - * @param {string} event_name - * @param {string[]} known_labels - * @returns {Promise<[string[], string[]]>} + * @param {Set} labelsToAdd + * @param {Set} labelsToRemove */ -// @ts-ignore: 'github' is currently unused but will be used after necessary changes -export async function handleLabeledEvent( - github, - context, - core, - owner, - repo, - issue_number, - event_name, - known_labels, -) { - // logic for this event is based on code directly ripped from pipelinebot: - // private/openapi-kebab/src/bots/pipeline/pipelineBotOnPRLabelEvent.ts - // todo: further enhance with labelling actions from `PR Summary` check. - const changedLabel = context.payload.label?.name; - const labelsToAdd = new Set(); - const labelsToRemove = new Set(); - - if (event_name === "labeled") { - if (changedLabel == "ARMChangesRequested") { - if (known_labels.indexOf("WaitForARMFeedback") !== -1) { - labelsToRemove.add("WaitForARMFeedback"); - } - } - if (changedLabel == "ARMSignedOff") { - if (known_labels.indexOf("WaitForARMFeedback") !== -1) { - labelsToRemove.add("WaitForARMFeedback"); - } - if (known_labels.indexOf("ARMChangesRequested") !== -1) { - labelsToRemove.add("ARMChangesRequested"); - } - } +function warnIfLabelSetsIntersect(labelsToAdd, labelsToRemove) { + const intersection = Array.from(labelsToAdd).filter((label) => labelsToRemove.has(label)); + if (intersection.length > 0) { + console.warn( + "ASSERTION VIOLATION! The intersection of labelsToRemove and labelsToAdd is non-empty! " + + `labelsToAdd: [${[...labelsToAdd].join(", ")}]. ` + + `labelsToRemove: [${[...labelsToRemove].join(", ")}]. ` + + `intersection: [${intersection.join(", ")}].`, + ); + } +} - for (const label of labelsToRemove) { - core.info(`Removing label: ${label} from ${owner}/${repo}#${issue_number}.`); - // await github.rest.issues.removeLabel({ - // owner: owner, - // repo: repo, - // issue_number: issue_number, - // name: label, - // }); - } - } else if (event_name === "unlabeled") { - if (changedLabel == "ARMChangesRequested") { - if (known_labels.indexOf("WaitForARMFeedback") !== -1) { - labelsToAdd.add("WaitForARMFeedback"); - } - } +// * @param {string} eventName +// * @param {string | undefined } changedLabel +/** + * @param {string[]} existingLabels + * @param {import("./labelling.js").ImpactAssessment | undefined} impactAssessment + * @returns {import("./labelling.js").LabelContext} + */ +export function updateLabels(existingLabels, impactAssessment) { + // logic for this function originally present in: + // - private/openapi-kebab/src/bots/pipeline/pipelineBotOnPRLabelEvent.ts + // - public/rest-api-specs-scripts/src/prSummary.ts + // it has since been simplified and moved here to handle all label addition and subtraction given a PR context + + /** @type {import("./labelling.js").LabelContext} */ + const labelContext = { + present: new Set(existingLabels), + toAdd: new Set(), + toRemove: new Set(), + }; + + if (impactAssessment) { + console.log(`Downloaded impact assessment: ${JSON.stringify(impactAssessment)}`); + // Merge impact assessment labels into the main labelContext + impactAssessment.labelContext.toAdd.forEach((label) => { + labelContext.toAdd.add(label); + }); + impactAssessment.labelContext.toRemove.forEach((label) => { + labelContext.toRemove.add(label); + }); + } - if (labelsToAdd.size > 0) { - core.info( - `Adding labels: ${Array.from(labelsToAdd).join(", ")} to ${owner}/${repo}#${issue_number}.`, - ); - // await github.rest.issues.addLabels({ - // owner: owner, - // repo: repo, - // issue_number: issue_number, - // labels: Array.from(labelsToAdd), - // }); - } + // this is the only labelling that was part of original pipelinebot logic + processArmReviewLabels(labelContext, existingLabels); + + if (impactAssessment) { + // will further update the label context if necessary + processImpactAssessment( + impactAssessment.targetBranch, + labelContext, + impactAssessment.resourceManagerRequired, + impactAssessment.versioningReviewRequired, + impactAssessment.breakingChangeReviewRequired, + impactAssessment.rpaasRPMissing, + impactAssessment.rpaasExceptionRequired, + impactAssessment.rpaasRpNotInPrivateRepo, + impactAssessment.isNewApiVersion, + impactAssessment.isDraft, + ); } - return [Array.from(labelsToAdd), Array.from(labelsToRemove)]; + warnIfLabelSetsIntersect(labelContext.toAdd, labelContext.toRemove); + return labelContext; } // #endregion @@ -488,7 +554,7 @@ export async function handleLabeledEvent( * @param {string} head_sha - The commit SHA to check. * @param {number} prNumber - The pull request number. * @param {string[]} excludedCheckNames - * @returns {Promise<[CheckRunData[], CheckRunData[]]>} + * @returns {Promise<[CheckRunData[], CheckRunData[], import("./labelling.js").ImpactAssessment | undefined]>} */ export async function getCheckRunTuple( github, @@ -506,14 +572,35 @@ export async function getCheckRunTuple( /** @type {CheckRunData[]} */ let fyiCheckRuns = []; + /** @type {number | undefined} */ + let impactAssessmentWorkflowRun = undefined; + + /** @type { import("./labelling.js").ImpactAssessment | undefined } */ + let impactAssessment = undefined; + const response = await github.graphql(getGraphQLQuery(owner, repo, head_sha, prNumber)); core.info(`GraphQL Rate Limit Information: ${JSON.stringify(response.rateLimit)}`); - [reqCheckRuns, fyiCheckRuns] = extractRunsFromGraphQLResponse(response); + [reqCheckRuns, fyiCheckRuns, impactAssessmentWorkflowRun] = + extractRunsFromGraphQLResponse(response); + + if (impactAssessmentWorkflowRun) { + core.info( + `Impact Assessment Workflow Run ID is present: ${impactAssessmentWorkflowRun}. Downloading job summary artifact`, + ); + impactAssessment = await getImpactAssessment( + github, + core, + owner, + repo, + impactAssessmentWorkflowRun, + ); + } core.info( `RequiredCheckRuns: ${JSON.stringify(reqCheckRuns)}, ` + - `FyiCheckRuns: ${JSON.stringify(fyiCheckRuns)}`, + `FyiCheckRuns: ${JSON.stringify(fyiCheckRuns)}, ` + + `ImpactAssessment: ${JSON.stringify(impactAssessment)}`, ); const filteredReqCheckRuns = reqCheckRuns.filter( /** @@ -528,7 +615,7 @@ export async function getCheckRunTuple( (checkRun) => !excludedCheckNames.includes(checkRun.name), ); - return [filteredReqCheckRuns, filteredFyiCheckRuns]; + return [filteredReqCheckRuns, filteredFyiCheckRuns, impactAssessment]; } /** @@ -554,7 +641,7 @@ export function checkRunIsSuccessful(checkRun) { /** * @param {any} response - GraphQL response data - * @returns {[CheckRunData[], CheckRunData[]]} + * @returns {[CheckRunData[], CheckRunData[], number | undefined]} */ function extractRunsFromGraphQLResponse(response) { /** @type {CheckRunData[]} */ @@ -562,11 +649,14 @@ function extractRunsFromGraphQLResponse(response) { /** @type {CheckRunData[]} */ const fyiCheckRuns = []; + /** @type {number | undefined} */ + let impactAssessmentWorkflowRun = undefined; + // Define the automated merging requirements check name if (response.resource?.checkSuites?.nodes) { response.resource.checkSuites.nodes.forEach( - /** @param {{ checkRuns?: { nodes?: any[] } }} checkSuiteNode */ + /** @param {{ workflowRun?: WorkflowRunInfo, checkRuns?: { nodes?: any[] } }} checkSuiteNode */ (checkSuiteNode) => { if (checkSuiteNode.checkRuns?.nodes) { checkSuiteNode.checkRuns.nodes.forEach((checkRunNode) => { @@ -591,7 +681,7 @@ function extractRunsFromGraphQLResponse(response) { // Note the "else" here. It means that: // A GH check will be bucketed into "failing FYI check run" if: // - It is failing - // - AND is is NOT marked as 'required' in GitHub branch policy + // - AND it is is NOT marked as 'required' in GitHub branch policy // - AND it is marked as 'FYI' in this file's FYI_CHECK_NAMES array else if (FYI_CHECK_NAMES.includes(checkRunNode.name)) { fyiCheckRuns.push({ @@ -606,7 +696,29 @@ function extractRunsFromGraphQLResponse(response) { }, ); } - return [reqCheckRuns, fyiCheckRuns]; + + // extract the ImpactAssessment check run if it is completed and successful + if (response.resource?.checkSuites?.nodes) { + response.resource.checkSuites.nodes.forEach( + /** @param {{ workflowRun?: WorkflowRunInfo, checkRuns?: { nodes?: any[] } }} checkSuiteNode */ + (checkSuiteNode) => { + if (checkSuiteNode.checkRuns?.nodes) { + checkSuiteNode.checkRuns.nodes.forEach((checkRunNode) => { + if ( + checkRunNode.name === "[TEST-IGNORE] Summarize PR Impact" && + checkRunNode.status?.toLowerCase() === "completed" && + checkRunNode.conclusion?.toLowerCase() === "success" + ) { + // Assign numeric databaseId, not the string node ID + impactAssessmentWorkflowRun = checkSuiteNode.workflowRun?.databaseId; + } + }); + } + }, + ); + } + + return [reqCheckRuns, fyiCheckRuns, impactAssessmentWorkflowRun]; } // #endregion // #region next steps @@ -825,3 +937,64 @@ function buildViolatedLabelRulesNextStepsText(violatedRequiredLabelsRules) { return violatedReqLabelsNextStepsText; } // #endregion + +// #region artifact downloading +/** + * Downloads the job-summary artifact for a given workflow run. + * @param {import('@actions/github-script').AsyncFunctionArguments['github']} github + * @param {typeof import("@actions/core")} core + * @param {string} owner + * @param {string} repo + * @param {number} runId - The workflow run databaseId + * @returns {Promise} The parsed job summary data + */ +export async function getImpactAssessment(github, core, owner, repo, runId) { + try { + // List artifacts for provided workflow run + const artifacts = await github.rest.actions.listWorkflowRunArtifacts({ + owner, + repo, + run_id: runId, + }); + + // Find the job-summary artifact + const jobSummaryArtifact = artifacts.data.artifacts.find( + (artifact) => artifact.name === "job-summary", + ); + + if (!jobSummaryArtifact) { + core.info("No job-summary artifact found"); + return undefined; + } + + // Download the artifact as a zip archive + const download = await github.rest.actions.downloadArtifact({ + owner, + repo, + artifact_id: jobSummaryArtifact.id, + archive_format: "zip", + }); + + core.info(`Successfully downloaded job-summary artifact ID: ${jobSummaryArtifact.id}`); + + // Write zip buffer to temp file and extract JSON + const tmpZip = path.join(process.env.RUNNER_TEMP || os.tmpdir(), `job-summary-${runId}.zip`); + // Convert ArrayBuffer to Buffer + // Convert ArrayBuffer (download.data) to Node Buffer + const arrayBuffer = /** @type {ArrayBuffer} */ (download.data); + const zipBuffer = Buffer.from(new Uint8Array(arrayBuffer)); + await fs.writeFile(tmpZip, zipBuffer); + // Extract JSON content from zip archive + const { stdout: jsonContent } = await execFile("unzip", ["-p", tmpZip]); + await fs.unlink(tmpZip); + + /** @type {import("./labelling.js").ImpactAssessment} */ + // todo: we need to zod this to ensure the structure is correct, however we do not have zod installed at time of run + const impact = JSON.parse(jsonContent); + return impact; + } catch (/** @type {any} */ error) { + core.error(`Failed to download job summary artifact: ${error.message}`); + return undefined; + } +} +// #endregion diff --git a/.github/workflows/summarize-checks.yaml b/.github/workflows/summarize-checks.yaml index 1cffd04eb3cf..4e39bd8fb5a4 100644 --- a/.github/workflows/summarize-checks.yaml +++ b/.github/workflows/summarize-checks.yaml @@ -6,6 +6,7 @@ on: workflows: - "\\[TEST-IGNORE\\] Swagger SemanticValidation - Set Status" - "\\[TEST-IGNORE\\] Swagger ModelValidation - Set Status" + - "\\[TEST-IGNORE\\] Summarize PR Impact" - "Swagger Avocado - Set Status" - "Swagger LintDiff - Set Status" - "SDK Validation Status" @@ -24,7 +25,7 @@ permissions: contents: read # for actions/checkout checks: read # for octokit.rest.checks.listForRef statuses: read # for octokit.rest.repos.getCombinedStatusForRef - issues: write # to post comments via the Issues API I'm not certain if I need this one or pull-requests only. we'll find out! + issues: write # to post comments via the Issues API pull-requests: write # to post comments via the Pull Requests API jobs: @@ -47,6 +48,8 @@ jobs: sparse-checkout: | .github + # todo: add the npm install ci here for the zod usage in summarize-checks (not currently there) + - id: summarize-checks name: Summarize Checks uses: actions/github-script@v7 diff --git a/.github/workflows/summarize-impact.yaml b/.github/workflows/summarize-impact.yaml new file mode 100644 index 000000000000..b00d70dedc49 --- /dev/null +++ b/.github/workflows/summarize-impact.yaml @@ -0,0 +1,75 @@ +name: "[TEST-IGNORE] Summarize PR Impact" + +on: pull_request + +permissions: + contents: read + pull-requests: read + +jobs: + impact: + name: "[TEST-IGNORE] Summarize PR Impact" + runs-on: ubuntu-24.04 + + steps: + - name: Checkout 'after' state + uses: actions/checkout@v4 + with: + fetch-depth: 2 + path: after + + - name: Checkout 'before' state + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.base.sha }} + path: before + + - name: Setup Node and install deps + uses: ./after/.github/actions/setup-node-install-deps + with: + working-directory: after + + - name: Get PR labels + id: get-labels + working-directory: after + run: | + labels=$(gh pr view ${{ github.event.pull_request.number }} --json labels --jq '[.labels[].name] | join(",")') + echo "labels=$labels" >> "$GITHUB_OUTPUT" + env: + GH_TOKEN: ${{ github.token }} + + - name: Run Summarize Impact + id: summarize-impact + working-directory: after + run: | + npm exec --no -- summarize-impact --sourceDirectory . --targetDirectory ../before \ + --number "${{ github.event.pull_request.number }}" \ + --sourceBranch "$HEAD_REF" \ + --targetBranch "${{ github.event.pull_request.base.ref }}" \ + --sha "${{ github.event.pull_request.head.sha }}" \ + --repo "${{ github.repository }}" \ + --owner "${{ github.repository_owner }}" \ + --labels "${{ steps.get-labels.outputs.labels }}" \ + --isDraft ${{ github.event.pull_request.draft }} + env: + # We absolutely need to avoid OOM errors due to certain inherited types from openapi-alps + NODE_OPTIONS: "--max-old-space-size=8192" + HEAD_REF: ${{ github.event.pull_request.head.ref }} + + - name: Set job-summary artifact + if: ${{ always() && steps.summarize-impact.outputs.summary }} + uses: actions/upload-artifact@v4 + with: + name: job-summary + path: ${{ steps.summarize-impact.outputs.summary }} + # If the file doesn't exist, just don't add the artifact + if-no-files-found: ignore + + - if: | + always() && + github.event.pull_request.number > 0 + name: Upload artifact with issue number + uses: ./after/.github/actions/add-empty-artifact + with: + name: "issue-number" + value: "${{ github.event.pull_request.number }}" diff --git a/.github/workflows/test/summarize-checks.test.js b/.github/workflows/test/summarize-checks.test.js index dee9ce4f8379..6d3208b26423 100644 --- a/.github/workflows/test/summarize-checks.test.js +++ b/.github/workflows/test/summarize-checks.test.js @@ -2,13 +2,14 @@ import { describe, expect, it } from "vitest"; import { createNextStepsComment, summarizeChecksImpl, + updateLabels, } from "../src/summarize-checks/summarize-checks.js"; import { createMockCore } from "./mocks.js"; import { Octokit } from "@octokit/rest"; const mockCore = createMockCore(); -describe("summarizeChecksImpl", () => { +describe("Summarize Checks Tests", () => { describe("next steps comment rendering", () => { it("Should generate summary for a mockdata PR scenario", async () => { const repo = "azure-rest-api-specs"; @@ -511,4 +512,74 @@ describe("summarizeChecksImpl", () => { 600000, ); }); + + describe("label add and remove", () => { + const testCases = [ + { + existingLabels: ["WaitForARMFeedback", "ARMChangesRequested", "other-label"], + expectedLabelsToAdd: [], + expectedLabelsToRemove: ["WaitForARMFeedback"], + }, + { + existingLabels: ["other-label", "ARMChangesRequested"], + expectedLabelsToAdd: [], + expectedLabelsToRemove: [], + }, + { + existingLabels: [ + "WaitForARMFeedback", + "ARMSignedOff", + "ARMChangesRequested", + "other-label", + ], + expectedLabelsToAdd: [], + expectedLabelsToRemove: ["WaitForARMFeedback", "ARMChangesRequested"], + }, + { + existingLabels: ["WaitForARMFeedback", "ARMSignedOff", "other-label"], + expectedLabelsToAdd: [], + expectedLabelsToRemove: ["WaitForARMFeedback"], + }, + { + existingLabels: ["ARMChangesRequested", "ARMSignedOff", "other-label"], + expectedLabelsToAdd: [], + expectedLabelsToRemove: ["ARMChangesRequested"], + }, + { + existingLabels: ["other-label", "ARMSignedOff"], + expectedLabelsToAdd: [], + expectedLabelsToRemove: [], + }, + { + existingLabels: ["WaitForARMFeedback", "other-label"], + expectedLabelsToAdd: [], + expectedLabelsToRemove: [], + }, + { + existingLabels: ["other-label"], + expectedLabelsToAdd: ["WaitForARMFeedback"], + expectedLabelsToRemove: [], + }, + { + existingLabels: ["WaitForARMFeedback", "ARMChangesRequested"], + expectedLabelsToAdd: [], + expectedLabelsToRemove: ["WaitForARMFeedback"], + }, + { + existingLabels: ["WaitForARMFeedback", "ARMChangesRequested"], + expectedLabelsToAdd: [], + expectedLabelsToRemove: ["WaitForARMFeedback"], + }, + ]; + + it.each(testCases)( + "$description", + async ({ existingLabels, expectedLabelsToAdd, expectedLabelsToRemove }) => { + const labelContext = await updateLabels(existingLabels, undefined); + + expect([...labelContext.toAdd].sort()).toEqual(expectedLabelsToAdd.sort()); + expect([...labelContext.toRemove].sort()).toEqual(expectedLabelsToRemove.sort()); + }, + ); + }); }); diff --git a/eng/tools/package.json b/eng/tools/package.json index 981da6b3bb5e..e7bceb3ca520 100644 --- a/eng/tools/package.json +++ b/eng/tools/package.json @@ -10,7 +10,8 @@ "@azure-tools/tsp-client-tests": "file:tsp-client-tests", "@azure-tools/typespec-migration-validation": "file:typespec-migration-validation", "@azure-tools/typespec-requirement": "file:typespec-requirement", - "@azure-tools/typespec-validation": "file:typespec-validation" + "@azure-tools/typespec-validation": "file:typespec-validation", + "@azure-tools/summarize-impact": "file:summarize-impact" }, "scripts": { "build": "tsc --build", diff --git a/eng/tools/summarize-impact/README.md b/eng/tools/summarize-impact/README.md new file mode 100644 index 000000000000..674eddddae5e --- /dev/null +++ b/eng/tools/summarize-impact/README.md @@ -0,0 +1,24 @@ +# `Summarize Impact` + +This tool models the PR and produces an artifact that is consumed by the `summarize-checks` check. + +The schema of the artifact looks like: + +```typescript +export type ImpactAssessment = { + prType: string[]; + resourceManagerRequired: boolean; + suppressionReviewRequired: boolean; + versioningReviewRequired: boolean; + breakingChangeReviewRequired: boolean; + isNewApiVersion: boolean; + rpaasExceptionRequired: boolean; + rpaasRpNotInPrivateRepo: boolean; + rpaasChange: boolean; + newRP: boolean; + rpaasRPMissing: boolean; + typeSpecChanged: boolean; + isDraft: boolean; + labelContext: LabelContext; +}; +``` diff --git a/eng/tools/summarize-impact/cmd/summarize-impact.js b/eng/tools/summarize-impact/cmd/summarize-impact.js new file mode 100755 index 000000000000..abdd7016b6cf --- /dev/null +++ b/eng/tools/summarize-impact/cmd/summarize-impact.js @@ -0,0 +1,5 @@ +#!/usr/bin/env node + +import { main } from "../dist/src/cli.js"; + +await main(); diff --git a/eng/tools/summarize-impact/package.json b/eng/tools/summarize-impact/package.json new file mode 100644 index 000000000000..83e013aa8257 --- /dev/null +++ b/eng/tools/summarize-impact/package.json @@ -0,0 +1,40 @@ +{ + "name": "@azure-tools/summarize-impact", + "private": true, + "type": "module", + "main": "dist/src/main.js", + "bin": { + "summarize-impact": "cmd/summarize-impact.js" + }, + "scripts": { + "build": "tsc --build", + "format": "prettier . --ignore-path ../.prettierignore --write", + "format:check": "prettier . --ignore-path ../.prettierignore --check", + "format:check:ci": "prettier . --ignore-path ../.prettierignore --check --log-level debug", + "test": "vitest --run", + "test:ci": "vitest run --coverage --reporter=verbose" + }, + "dependencies": { + "@azure-tools/openapi-tools-common": "^1.2.2", + "@azure-tools/specs-shared": "file:../../../.github/shared", + "@azure/openapi-markdown": "0.9.4", + "@ts-common/commonmark-to-markdown": "^2.0.2", + "commonmark": "^0.29.0", + "glob": "^11.0.3", + "js-yaml": "^4.1.0", + "lodash": "^4.17.20", + "simple-git": "^3.27.0" + }, + "devDependencies": { + "@types/commonmark": "^0.27.4", + "@types/glob": "^8.1.0", + "@types/lodash": "^4.14.161", + "@types/node": "^20.0.0", + "prettier": "~3.5.3", + "typescript": "~5.8.2", + "vitest": "^3.0.7" + }, + "engines": { + "node": ">=20.0.0" + } +} diff --git a/eng/tools/summarize-impact/src/ImpactAssessment.ts b/eng/tools/summarize-impact/src/ImpactAssessment.ts new file mode 100644 index 000000000000..c5693229f8bb --- /dev/null +++ b/eng/tools/summarize-impact/src/ImpactAssessment.ts @@ -0,0 +1,18 @@ +import { LabelContext } from "./labelling-types.js"; + +export type ImpactAssessment = { + resourceManagerRequired: boolean; + suppressionReviewRequired: boolean; + versioningReviewRequired: boolean; + breakingChangeReviewRequired: boolean; + isNewApiVersion: boolean; + rpaasExceptionRequired: boolean; + rpaasRpNotInPrivateRepo: boolean; + rpaasChange: boolean; + newRP: boolean; + rpaasRPMissing: boolean; + typeSpecChanged: boolean; + isDraft: boolean; + labelContext: LabelContext; + targetBranch: string; +}; diff --git a/eng/tools/summarize-impact/src/PRContext.ts b/eng/tools/summarize-impact/src/PRContext.ts new file mode 100644 index 000000000000..1038892c9724 --- /dev/null +++ b/eng/tools/summarize-impact/src/PRContext.ts @@ -0,0 +1,340 @@ +import { join, dirname } from "path"; +import * as fs from "fs"; + +import { parseMarkdown } from "@azure-tools/openapi-tools-common"; +import * as amd from "@azure/openapi-markdown"; + +import { SpecModel } from "@azure-tools/specs-shared/spec-model"; +import { Readme } from "@azure-tools/specs-shared/readme"; +import { + swagger, + typespec, + example, + readme, + specification, +} from "@azure-tools/specs-shared/changed-files"; + +import { LabelContext } from "./labelling-types.js"; +import { DiffResult, ReadmeTag, TagDiff, TagConfigDiff } from "./diff-types.js"; + +export type FileListInfo = { + additions: string[]; + modifications: string[]; + deletions: string[]; + renames: { from: string; to: string }[]; + total: number; +}; + +export type PRContextOptions = { + fileList?: FileListInfo; + sourceBranch: string; + targetBranch: string; + sha: string; + repo: string; + owner: string; + prNumber: string; + isDraft: boolean; +}; + +export class PRContext { + // we are starting with checking out before and after to different directories + // sourceDirectory corresponds to "after". EG the PR context is the "after" state of the PR. + // The "before" state is the git root directory without the changes aka targetDirectory + sourceDirectory: string; + targetDirectory: string; + sourceSpecModel?: SpecModel; + targetSpecModel?: SpecModel; + fileList?: FileListInfo; + sourceBranch: string; + targetBranch: string; + sha: string; + repo: string; + owner: string; + prNumber: string; + isDraft: boolean; + labelContext: LabelContext; + + constructor( + sourceDirectory: string, + targetDirectory: string, + labelContext: LabelContext, + options: PRContextOptions, + ) { + this.sourceDirectory = sourceDirectory; + this.targetDirectory = targetDirectory; + this.sourceSpecModel = new SpecModel(sourceDirectory); + this.targetSpecModel = new SpecModel(targetDirectory); + this.labelContext = labelContext; + this.sourceBranch = options.sourceBranch; + this.targetBranch = options.targetBranch; + this.sha = options.sha; + this.repo = options.repo; + this.prNumber = options.prNumber; + this.fileList = options.fileList; + this.isDraft = options.isDraft; + this.owner = options.owner; + } + + getChangedFiles(): string[] { + if (!this.fileList) { + return []; + } + + const changedFiles: string[] = (this.fileList.additions || []) + .concat(this.fileList.modifications || []) + .concat(this.fileList.deletions || []) + .concat(this.fileList.renames.map((r) => r.to) || []); + return changedFiles; + } + + // todo get rid of async here not necessary + // todo store the results + getTypeSpecDiffs(): DiffResult { + if (!this.fileList) { + return { + additions: [], + deletions: [], + changes: [], + }; + } + + const additions = this.fileList.additions.filter((file) => typespec(file)); + const deletions = this.fileList.deletions.filter((file) => typespec(file)); + const changes = this.fileList.modifications.filter((file) => typespec(file)); + + return { + additions, + deletions, + changes, + }; + } + + getSwaggerDiffs(): DiffResult { + if (!this.fileList) { + return { + additions: [], + deletions: [], + changes: [], + }; + } + + const additions = this.fileList.additions.filter((file) => swagger(file)); + const deletions = this.fileList.deletions.filter((file) => swagger(file)); + const changes = this.fileList.modifications.filter((file) => swagger(file)); + + return { + additions, + deletions, + changes, + }; + } + + getExampleDiffs(): DiffResult { + if (!this.fileList) { + return { + additions: [], + deletions: [], + changes: [], + }; + } + + const additions = this.fileList.additions.filter((file) => example(file)); + const deletions = this.fileList.deletions.filter((file) => example(file)); + const changes = this.fileList.modifications.filter((file) => example(file)); + + return { + additions, + deletions, + changes, + }; + } + + async getTagsFromReadme(readmePath: string): Promise { + const tags = await new Readme(readmePath).getTags(); + return [...tags.values()].map((tag) => tag.name); + } + + async getPossibleParentConfigurations(): Promise { + console.log("ENTER definition getPossibleParentConfigurations"); + const changedFiles = await this.getChangedFiles(); + console.log(`Detect changes in the PR:\n${JSON.stringify(changedFiles, null, 2)}`); + const readmes = changedFiles.filter((f) => readme(f)); + + const visitedFolder = new Set(); + changedFiles + .filter((f) => [".md", ".json", ".yaml", ".yml"].some((p) => f.endsWith(p))) + .forEach((f) => { + let dir = dirname(f); + if (visitedFolder.has(dir)) { + return; + } + while (specification(dir)) { + if (visitedFolder.has(dir)) { + break; + } + visitedFolder.add(dir); + const possibleReadme = join(dir, "readme.md"); + if (fs.existsSync(possibleReadme)) { + if (!readmes.includes(possibleReadme)) { + readmes.push(possibleReadme); + } + break; + } + dir = dirname(dir); + } + }); + console.log("RETURN definition getPossibleParentConfigurations"); + return readmes; + } + + getAllTags(readMeContent: string): string[] { + // todo: we should refactor this to use the spec model, but I haven't had the the time to explicitly + // diff what oad does does here, so I'm leaving it alone for now until I can build some strong unit tests around this. + const cmd = parseMarkdown(readMeContent); + const allTags = new amd.ReadMeManipulator( + { error: (_msg: string) => {} }, + new amd.ReadMeBuilder(), + ).getAllTags(cmd); + return [...allTags]; + } + + async getInputFiles(readMeContent: string, tag: string) { + // todo: we should refactor this to use spec model, but I haven't had time to isolate exactly what + // openapi-markdown is doing here, so I'm just going to use the same logic for now + const cmd = parseMarkdown(readMeContent); + return amd.getInputFilesForTag(cmd.markDown, tag); + } + + async getChangingTags(): Promise { + // we are retrieving all the readme changes, no matter if they're additions, deletions, etc + // Additionally, we're also retrieving all the readme files that may be affected by the changes in the PR, which means + // climbing up the directory tree until we find a readme.md file if necessary. + const allAffectedReadmes: string[] = await this.getPossibleParentConfigurations(); + console.log(`all affected readme are:`); + console.log(JSON.stringify(allAffectedReadmes, null, 2)); + const Diffs: TagDiff[] = []; + for (const readme of allAffectedReadmes) { + const oldReadme = join(this.targetDirectory, readme); + const newReadme = join(this.sourceDirectory, readme); + // As the readme may be not existing , need to check if it's existing. + // we are checking target and source individually, because the readme may not exist in the target branch + const oldTags: string[] = fs.existsSync(oldReadme) + ? [...(await new Readme(oldReadme).getTags()).keys()] + : []; + const newTags: string[] = fs.existsSync(newReadme) + ? [...(await new Readme(newReadme).getTags()).keys()] + : []; + const intersect = oldTags + .filter((t) => newTags.includes(t)) + .filter((tag) => tag !== "all-api-versions"); + const insertions = newTags.filter((t) => !oldTags.includes(t)); + const deletions = oldTags.filter((t) => !newTags.includes(t)); + const differences: TagConfigDiff[] = []; + + // todo: we need to ensure we get ALL effected swaggers by their relationships, not just the swagger files that are directly changed in the PR. + // right now I'm just going to filter to the ones that are directly changed in the PR. + // this is a temporary solution, we need to ensure we get all of the swagger files + // that are affected by the changes in the readme. SpecModel will be useful for this + // we want to get all of the swagger files that are affected by the changes in the readme. + // we can do that using the specmodel + // const allAffectedInputFiles = await this.getRealAffectedSwagger(readme) + // talk to Mike and ask him how we could get all affected swagger files from a readme path. + // I want to say that readme(readme).specModel.getAffectedSwaggerFiles will work? + const allAffectedInputFiles = await (await this.getChangedFiles()).filter((f) => swagger(f)); + console.log(`all affected swagger files in ${readme} are:`); + console.log(JSON.stringify(allAffectedInputFiles, null, 2)); + const getChangedInputFiles = async (tag: string) => { + const readmeContent = await fs.promises.readFile(newReadme, "utf-8"); + const inputFiles = await this.getInputFiles(readmeContent, tag); + if (inputFiles) { + const changedInputFiles = (inputFiles as string[]).filter((f) => + allAffectedInputFiles.some((a) => a.endsWith(f)), + ); + return changedInputFiles; + } + return []; + }; + const changes: string[] = []; + for (const tag of intersect) { + const tagDiff: TagConfigDiff = { name: tag }; + const changedInputFiles = await getChangedInputFiles(tag); + if (changedInputFiles.length) { + console.log("found changed input files under tag:" + tag); + tagDiff.changedInputFiles = changedInputFiles; + changes.push(tag); + } + } + Diffs.push({ readme, insertions, deletions, changes, differences }); + } + return Diffs; + } + + // this function is based upon LocalDirContext.getReadmeDiffs() and CommonPRContext.getReadmeDiffs() which are + // very different from each other (because why not). This implementation is based MOSTLY on CommonPRContext + async getReadmeDiffs(): Promise> { + // this gets all of the readme diffs + // const readmeAdditions = this.fileList?.additions.filter(file => readme(file)) || []; + // const readmeChanges = this.fileList?.modifications.filter(file => readme(file)) + // const readmeDeletions = this.fileList?.deletions.filter(file => readme(file)); + //const changedFiles: DiffFileResult | undefined = await this.localPRContext?.getChangingFiles(); + + const changedFiles = await this.fileList; + const tagDiffs = (await this.getChangingTags()) || []; + + const readmeTagDiffs = tagDiffs + ?.filter((tagDiff: TagDiff) => readme(tagDiff.readme)) + .map((tagDiff: TagDiff) => { + return { + readme: tagDiff.readme, + tags: { + changes: tagDiff.changes, + deletions: tagDiff.deletions, + additions: tagDiff.insertions, + }, + } as ReadmeTag; + }); + + const readmeTagDiffsInAddedReadmeFiles: ReadmeTag[] = readmeTagDiffs.filter( + (readmeTag: ReadmeTag): boolean => + Boolean(changedFiles?.additions.includes(readmeTag.readme)), + ); + const readmeTagDiffsInDeletedReadmeFiles: ReadmeTag[] = readmeTagDiffs.filter( + (readmeTag: ReadmeTag): boolean => + Boolean(changedFiles?.deletions.includes(readmeTag.readme)), + ); + const readmeTagDiffsInChangedReadmeFiles: ReadmeTag[] = readmeTagDiffs.filter( + (readmeTag: ReadmeTag): boolean => { + // The README file that contains the API version tags that have been diffed in given readmeTag (i.e. readmeTag.tags) + // has been modified. + const readmeModified = changedFiles?.modifications.includes(readmeTag.readme); + + // The README was not modified, added or deleted; just the specs belonging to the API version tags in the README were modified. + // In such case we assume the README is 'changed' in the sense the specs belonging to the API version tags in the README were modified. + // The README file itself wasn't modified. + // We assume here the README is present in the repository - otherwise it would show up as 'deleted' or would not + // appear as readmeTag.readme in any of the readmeTagDiffs. + const readmeUnchanged = + !changedFiles?.additions.includes(readmeTag.readme) && + !changedFiles?.deletions.includes(readmeTag.readme); + + return readmeModified || readmeUnchanged; + }, + ); + + const result = { + additions: readmeTagDiffsInAddedReadmeFiles, + deletions: readmeTagDiffsInDeletedReadmeFiles, + changes: readmeTagDiffsInChangedReadmeFiles, + }; + + console.log( + `RETURN definition CommonPRContext.getReadmeDiffs. ` + + `changedFiles: ${JSON.stringify(changedFiles)}, ` + + `tagDiffs: ${JSON.stringify(tagDiffs)}, ` + + `readmeTagDiffs: ${JSON.stringify(readmeTagDiffs)}, ` + + `this.readmeDiffs: ${JSON.stringify(result)}.`, + ); + + return result; + } +} diff --git a/eng/tools/summarize-impact/src/cli.ts b/eng/tools/summarize-impact/src/cli.ts new file mode 100644 index 000000000000..75641b3b6093 --- /dev/null +++ b/eng/tools/summarize-impact/src/cli.ts @@ -0,0 +1,139 @@ +#!/usr/bin/env node + +import { evaluateImpact } from "./impact.js"; +import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; +import { setOutput } from "@azure-tools/specs-shared/error-reporting"; + +import { resolve, join } from "path"; +import fs from "fs"; +import { parseArgs, ParseArgsConfig } from "node:util"; +import { LabelContext } from "./labelling-types.js"; +import { PRContext } from "./PRContext.js"; +import { getRootFolder } from "@azure-tools/specs-shared/simple-git"; + +export async function getRoot(inputPath: string): Promise { + try { + const gitRoot = await getRootFolder(inputPath); + return resolve(gitRoot.trim()); + } catch (error) { + console.error( + `Error: Unable to determine the root folder of the git repository.`, + `Please ensure you are running this command within a git repository OR providing a targeted directory that is within a git repo.`, + ); + process.exit(1); + } +} + +export async function main() { + const config: ParseArgsConfig = { + options: { + // the target branch checked out + targetDirectory: { + type: "string", + multiple: false, + }, + // the pr + sourceDirectory: { + type: "string", + multiple: false, + default: process.cwd(), + }, + fileList: { + type: "string", + short: "f", + multiple: false, + default: undefined, + }, + number: { + type: "string", + short: "n", + multiple: false, + }, + sourceBranch: { + type: "string", + multiple: false, + }, + targetBranch: { + type: "string", + multiple: false, + }, + sha: { + type: "string", + short: "s", + multiple: false, + }, + repo: { + type: "string", + short: "r", + multiple: false, + }, + owner: { + type: "string", + short: "o", + multiple: false, + }, + labels: { + type: "string", + short: "l", + multiple: false, + }, + isDraft: { + type: "boolean", + multiple: false, + }, + }, + allowPositionals: true, + }; + + const { values: opts } = parseArgs(config); + + // todo: refactor these opts + const sourceDirectory = opts.sourceDirectory as string; + const targetDirectory = opts.targetDirectory as string; + const sourceGitRoot = await getRoot(sourceDirectory); + const targetGitRoot = await getRoot(targetDirectory); + const fileList = await getChangedFilesStatuses({ cwd: sourceGitRoot }); + const sha = opts.sha as string; + const sourceBranch = opts.sourceBranch as string; + const targetBranch = opts.targetBranch as string; + const repo = opts.repo as string; + const owner = opts.owner as string; + const prNumber = opts.number as string; + const existingLabels = (opts.labels as string).split(",").map((l) => l.trim()); + const isDraft = opts.prStaisDrafttus as boolean; + + const labelContext: LabelContext = { + present: new Set(existingLabels), + toAdd: new Set(), + toRemove: new Set(), + }; + + const prContext = new PRContext(sourceGitRoot, targetGitRoot, labelContext, { + sha, + sourceBranch, + targetBranch, + repo, + prNumber, + owner, + fileList, + isDraft, + }); + + let impact = await evaluateImpact(prContext, labelContext); + + // sets by default are not serializable, so we need to convert them to arrays + // before we can write them to the output file. + function setReplacer(_key: string, value: any) { + if (value instanceof Set) { + return [...value]; + } + return value; + } + + console.log("Evaluated impact: ", JSON.stringify(impact, setReplacer, 2)); + + // Write to a temp file that can get picked up later. + const summaryFile = join(process.cwd(), "summary.json"); + fs.writeFileSync(summaryFile, JSON.stringify(impact, setReplacer, 2)); + setOutput("summary", summaryFile); +} diff --git a/eng/tools/summarize-impact/src/diff-types.ts b/eng/tools/summarize-impact/src/diff-types.ts new file mode 100644 index 000000000000..c34f851a873c --- /dev/null +++ b/eng/tools/summarize-impact/src/diff-types.ts @@ -0,0 +1,40 @@ +export type FileTypes = "SwaggerFile" | "TypeSpecFile" | "ExampleFile" | "ReadmeFile"; +export type ChangeTypes = "Addition" | "Deletion" | "Update"; + +export type PRChange = { + fileType: FileTypes; + changeType: ChangeTypes; + filePath: string; + additionalInfo?: any; +}; + +export type ReadmeTag = { + readme: string; + tags: DiffResult; +}; + +export type TagConfigDiff = { + name: string; + oldConfig?: any; + newConfig?: any; + difference?: any; + changedInputFiles?: string[]; +}; + +export type TagDiff = { + readme: string; + changes: string[]; + insertions: string[]; + deletions: string[]; + differences?: TagConfigDiff[]; +}; + +export type ChangeHandler = { + [key in FileTypes]?: (event: PRChange) => void | Promise; +}; + +export type DiffResult = { + additions?: T[]; + deletions?: T[]; + changes?: T[]; +}; diff --git a/eng/tools/summarize-impact/src/impact.ts b/eng/tools/summarize-impact/src/impact.ts new file mode 100644 index 000000000000..757521c8b227 --- /dev/null +++ b/eng/tools/summarize-impact/src/impact.ts @@ -0,0 +1,919 @@ +#!/usr/bin/env node + +import * as fs from "fs"; +import { glob } from "glob"; +import * as path from "path"; + +import * as commonmark from "commonmark"; +import yaml from "js-yaml"; +import * as _ from "lodash"; + +import { breakingChangesCheckType } from "@azure-tools/specs-shared/breaking-change"; + +import { + DiffResult, + ReadmeTag, + FileTypes, + ChangeTypes, + PRChange, + ChangeHandler, +} from "./diff-types.js"; + +import { PRType, Label, LabelContext } from "./labelling-types.js"; + +import { ImpactAssessment } from "./ImpactAssessment.js"; +import { PRContext } from "./PRContext.js"; + +import { Readme } from "@azure-tools/specs-shared/readme"; + +export const breakingChangeLabelVarName = "breakingChangeVar"; +export const crossVersionBreakingChangeLabelVarName = "crossVersionBreakingChangeVar"; + +// todo: we need to populate this so that we can tell if it's a new APIVersion down stream +export async function isNewApiVersion(context: PRContext): Promise { + const handlers: ChangeHandler[] = []; + let isAddingNewApiVersion = false; + const apiVersionSet = new Set(); + + const rpFolders = new Set(); + + const createSwaggerFileHandler = () => { + return (e: PRChange) => { + if (e.changeType === "Addition") { + const apiVersion = getApiVersionFromSwaggerFile(e.filePath); + if (apiVersion) { + apiVersionSet.add(apiVersion); + } + const rpFolder = getRPFolderFromSwaggerFile(e.filePath); + if (rpFolder !== undefined) { + rpFolders.add(rpFolder); + } + console.log(`apiVersion: ${apiVersion}, rpFolder: ${rpFolder}`); + } else if (e.changeType === "Update") { + const rpFolder = getRPFolderFromSwaggerFile(e.filePath); + if (rpFolder !== undefined) { + rpFolders.add(rpFolder); + } + } + }; + }; + + handlers.push({ SwaggerFile: createSwaggerFileHandler() }); + await processPrChanges(context, handlers); + + console.log(`rpFolders: ${Array.from(rpFolders).join(",")}`); + + const firstRPFolder = Array.from(rpFolders)[0]; + + console.log(`apiVersion: ${Array.from(apiVersionSet).join(",")}`); + + if (firstRPFolder === undefined) { + console.log("RP folder not found."); + return false; + } + + const targetBranchRPFolder = path.resolve(context.targetDirectory, firstRPFolder); + + console.log(`targetBranchRPFolder: ${targetBranchRPFolder}`); + + const existingApiVersions = getAllApiVersionFromRPFolder(targetBranchRPFolder); + + console.log(`existingApiVersions: ${existingApiVersions.join(",")}`); + + for (const apiVersion of apiVersionSet) { + if (!existingApiVersions.includes(apiVersion)) { + console.log(`The apiVersion ${apiVersion} is added. and not found in existing ApiVersions`); + isAddingNewApiVersion = true; + } + } + return isAddingNewApiVersion; +} + +export async function evaluateImpact( + context: PRContext, + labelContext: LabelContext, +): Promise { + const typeSpecLabelShouldBePresent = await processTypeSpec(context, labelContext); + + // examine changed files. if changedpaths includes data-plane, add "data-plane" + // same for "resource-manager". We care about whether resourcemanager will be present for a later check + const { resourceManagerLabelShouldBePresent } = await processPRType(context, labelContext); + + // Has to be run in a PR context. Uses addition and update to understand + // if the suppressions have been changed. If they have, suppressionReviewRequired must be added + // as a label + const suppressionRequired = await processSuppression(context, labelContext); + console.log(`suppressionRequired: ${suppressionRequired}`); + + // Calculates whether or not BreakingChangeReviewRequired and VersioningReviewRequired labels should be present + let versioningReviewRequiredLabelShouldBePresent: boolean = false; + let breakingChangeReviewRequiredLabelShouldBePresent: boolean = false; + try { + ({ + versioningReviewRequiredLabelShouldBePresent, + breakingChangeReviewRequiredLabelShouldBePresent, + } = await processBreakingChangeLabels(context, labelContext)); + } catch (error) { + console.error("Error processing breaking change labels:", error); + } + + // needs to examine "after" context to understand if a readme that was changed is RPaaS or not + const { rpaasLabelShouldBePresent } = await processRPaaS(context, labelContext); + + // Has to be in PR context. Uses the addition to understand if the newRPNamespace label should be present. + const { newRPNamespaceLabelShouldBePresent } = await processNewRPNamespace( + context, + labelContext, + resourceManagerLabelShouldBePresent, + ); + + // doesn't necessarily need to be in the PR context. + // Uses the previous outputs that DID need to be a PR context, but otherwise only examines targetBranch and those + // output labels. + const { ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent, rpaasExceptionLabelShouldBePresent } = + await processNewRpNamespaceWithoutRpaasLabel( + context, + labelContext, + resourceManagerLabelShouldBePresent, + newRPNamespaceLabelShouldBePresent, + rpaasLabelShouldBePresent, + ); + + // examines the additions. if the changetype is addition, then it will add the ciRpaasRPNotInPrivateRepo label + const { ciRpaasRPNotInPrivateRepoLabelShouldBePresent } = + await processRpaasRpNotInPrivateRepoLabel( + context, + labelContext, + resourceManagerLabelShouldBePresent, + rpaasLabelShouldBePresent, + ); + + const newApiVersion = await isNewApiVersion(context); + + return { + suppressionReviewRequired: labelContext.toAdd.has("suppressionsReviewRequired"), + versioningReviewRequired: versioningReviewRequiredLabelShouldBePresent, + breakingChangeReviewRequired: breakingChangeReviewRequiredLabelShouldBePresent, + rpaasChange: rpaasLabelShouldBePresent, + newRP: newRPNamespaceLabelShouldBePresent, + rpaasRPMissing: ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent, + rpaasRpNotInPrivateRepo: ciRpaasRPNotInPrivateRepoLabelShouldBePresent, + resourceManagerRequired: resourceManagerLabelShouldBePresent, + rpaasExceptionRequired: rpaasExceptionLabelShouldBePresent, + typeSpecChanged: typeSpecLabelShouldBePresent, + isNewApiVersion: newApiVersion, + isDraft: context.isDraft, + labelContext: labelContext, + targetBranch: context.targetBranch, + }; +} + +export function isManagementPR(filePaths: string[]): boolean { + return filePaths.some((it) => it.includes("resource-manager")); +} + +export function isDataPlanePR(filePaths: string[]): boolean { + return filePaths.some((it) => it.includes("data-plane")); +} + +export function getAllApiVersionFromRPFolder(rpFolder: string): string[] { + const allSwaggerFilesFromRPFolder = glob.sync(`${rpFolder}/**/*.json`); + console.log(`allSwaggerFilesFromRPFolder: ${allSwaggerFilesFromRPFolder}`); + + const apiVersions: Set = new Set(); + for (const it of allSwaggerFilesFromRPFolder) { + if (!it.includes("examples")) { + const apiVersion = getApiVersionFromSwaggerFile(it); + console.log(`Get api version from swagger file: ${it}, apiVersion: ${apiVersion}`); + if (apiVersion) { + apiVersions.add(apiVersion); + } + } + } + return [...apiVersions]; +} + +export function getApiVersionFromSwaggerFile(swaggerFile: string): string | undefined { + const swagger = fs.readFileSync(swaggerFile).toString(); + const swaggerObject = JSON.parse(swagger); + if (swaggerObject["info"] && swaggerObject["info"]["version"]) { + return swaggerObject["info"]["version"]; + } + return undefined; +} + +export function getRPFolderFromSwaggerFile(swaggerFile: string): string | undefined { + const resourceProvider = getResourceProviderFromFilePath(swaggerFile); + + if (resourceProvider === undefined) { + return undefined; + } + + const lastIdx = swaggerFile.lastIndexOf(resourceProvider!); + return swaggerFile.substring(0, lastIdx + resourceProvider!.length); +} + +export const getResourceProviderFromFilePath = (filePath: string): string | undefined => { + // Example filePath: + // specification/purview/data-plane/Azure.Analytics.Purview.Workflow/preview/2023-10-01-preview/purviewWorkflow.json + // Match: + // /Azure.Analytics.Purview.Workflow/ + // Note: + // The regex matches a directory name in the path of form: /Foo.Bar.Baz/, where: + // - The directory name must have at least one period. If it would match 0 periods, + // then in the example path it would match to "/purview/" instead. + // - The directory name can have one or more periods. + // The "Foo.Bar.Baz" example given above has two periods. + const regex = /\/([a-z0-9]+(\.[a-z0-9]+)+)\//i; + const match = filePath.match(regex); + if (match && match.length > 0) { + return match[1]; + } + // Second matching attempt to cover scenarios in which: + // - the resource provider is for data-plane + // - and it has no dots in the name. + // + // Example filePath: + // specification/communication/data-plane/Sms/preview/2024-02-05-preview/communicationServicesSms.json + // Match: + // /Sms/ + // + // For details see: https://github.com/Azure/azure-sdk-tools/issues/7552 + const regexForDirImmediatelyAfterDataPlane = /\/data-plane\/([a-z0-9]+)\//i; + const match2 = filePath.match(regexForDirImmediatelyAfterDataPlane); + if (match2 && match2.length > 0) { + return match2[1]; + } + + return undefined; +}; + +// todo: download the labels from the breaking change check run +// right now this logic is coded to parse the ADO build variables that are set +// by the breakingchange run in an earlier job. +async function processBreakingChangeLabels( + prContext: PRContext, + labelContext: LabelContext, +): Promise<{ + versioningReviewRequiredLabelShouldBePresent: boolean; + breakingChangeReviewRequiredLabelShouldBePresent: boolean; +}> { + console.log("ENTER definition processBreakingChangeLabels"); + + // Debug the breakingChangesCheckType import + console.log("breakingChangesCheckType:", breakingChangesCheckType); + console.log("breakingChangesCheckType.SameVersion:", breakingChangesCheckType?.SameVersion); + console.log("breakingChangesCheckType.CrossVersion:", breakingChangesCheckType?.CrossVersion); + + const prTargetsProductionBranch: boolean = checkPrTargetsProductionBranch(prContext); + const breakingChangesLabelsFromOad = getBreakingChangesLabelsFromOad(); + + const versioningReviewRequiredLabel = new Label( + breakingChangesCheckType.SameVersion.reviewRequiredLabel, + labelContext.present, + ); + + const breakingChangeReviewRequiredLabel = new Label( + breakingChangesCheckType.CrossVersion.reviewRequiredLabel, + labelContext.present, + ); + + const versioningApprovalLabels = breakingChangesCheckType.SameVersion.approvalLabels.map( + (label: string) => new Label(label, labelContext.present), + ); + const breakingChangeApprovalLabels = breakingChangesCheckType.CrossVersion.approvalLabels.map( + (label: string) => new Label(label, labelContext.present), + ); + + // ----- Breaking changes label processing logic ----- + // These lines implement the set of rules determining which of the breaking change + // labels should be present on given PR. + // The doc describing breaking change review rules can be found here: + // https://aka.ms/brch-dev + + // ❗ IMPORTANT ❗: this MUST be set BEFORE versioningReviewRequiredLabel.shouldBePresent + // due to logical dependency + breakingChangeReviewRequiredLabel.shouldBePresent = + // "BreakingChangeReviewRequired" label should be present if + // 1. The PR targets a production branch + prTargetsProductionBranch && + // 2. AND given OAD run determined it is still applicable. + breakingChangesLabelsFromOad.includes(breakingChangeReviewRequiredLabel.name); + + // ❗ IMPORTANT ❗: this MUST be set AFTER breakingChangeReviewRequiredLabel.shouldBePresent + // due to logical dependency + versioningReviewRequiredLabel.shouldBePresent = + // "VersioningReviewRequired" label should be unconditionally removed if + // the label "BreakingChangeReviewRequired" should be present. + !breakingChangeReviewRequiredLabel.shouldBePresent && + // AND "VersioningReviewRequired" label should be present if + // 1. The PR targets a production branch + prTargetsProductionBranch && + // 2. AND given OAD run determined it is still applicable. + breakingChangesLabelsFromOad.includes(versioningReviewRequiredLabel.name); + + versioningApprovalLabels.forEach((approvalLabel) => { + approvalLabel.shouldBePresent = + approvalLabel.present && versioningReviewRequiredLabel.shouldBePresent; + }); + + breakingChangeApprovalLabels.forEach((approvalLabel) => { + approvalLabel.shouldBePresent = + approvalLabel.present && breakingChangeReviewRequiredLabel.shouldBePresent; + }); + + // ---------- + + applyLabelsStateChanges(labelContext.toAdd, labelContext.toRemove); + + logDiagInfo(); + + return { + versioningReviewRequiredLabelShouldBePresent: versioningReviewRequiredLabel.shouldBePresent, + breakingChangeReviewRequiredLabelShouldBePresent: + breakingChangeReviewRequiredLabel.shouldBePresent, + }; + + /** + * Get labels denoting required breaking change or versioning review. + * The labels are read from ADO pipeline environment variables. + * These variables are set to appropriate value by BreakingChangesRuleManager.addBreakingChangeLabels(). + * Read that function's comment for details. + */ + function getBreakingChangesLabelsFromOad(): string[] { + const oadLabelsEnvVars = [ + breakingChangeLabelVarName.toUpperCase(), + crossVersionBreakingChangeLabelVarName.toUpperCase(), + ]; + let breakingChangeLabelsFromOad: string[] = _.uniq( + oadLabelsEnvVars + .map((labelSenderEnvVar) => process.env[labelSenderEnvVar]) + .filter((envVarValue) => envVarValue !== undefined) + .map((envVarValue) => envVarValue?.split(",") || []) + .reduce( + (accumulatedLabels, currentEnvVarLabels) => accumulatedLabels.concat(currentEnvVarLabels), + [], + ), + ).filter((label) => label); + return breakingChangeLabelsFromOad; + } + + function applyLabelsStateChanges(labelsToAdd: Set, labelsToRemove: Set) { + breakingChangeReviewRequiredLabel.applyStateChange(labelsToAdd, labelsToRemove); + versioningReviewRequiredLabel.applyStateChange(labelsToAdd, labelsToRemove); + versioningApprovalLabels.forEach((approvalLabel) => { + approvalLabel.applyStateChange(labelsToAdd, labelsToRemove); + }); + breakingChangeApprovalLabels.forEach((approvalLabel) => { + approvalLabel.applyStateChange(labelsToAdd, labelsToRemove); + }); + } + + function logDiagInfo() { + if ( + !prTargetsProductionBranch && + (breakingChangesLabelsFromOad.includes(breakingChangeReviewRequiredLabel.name) || + breakingChangesLabelsFromOad.includes(versioningReviewRequiredLabel.name)) + ) { + // We are using this log as a metric to track and measure impact of the work on improving "breaking changes" tooling. Log statement added around 11/29/2023. + // See: https://github.com/Azure/azure-sdk-tools/issues/7223#issuecomment-1839830834 + // Note it duplicates the label "shouldBePresent" ruleset logic. + console.log( + `processBreakingChangeLabels: PR: ${`https://github.com/${prContext.owner}/${prContext.repo}/pull/${prContext.prNumber}`}, targetBranch: ${prContext.targetBranch}. ` + + `The addition of 'BreakingChangesReviewRequired' or 'VersioningReviewRequired' labels has been prevented ` + + `because we checked that the PR is not targeting a production branch.`, + ); + } + + console.log( + `processBreakingChangeLabels returned. ` + + `prTargetsProductionBranch: ${prTargetsProductionBranch}, ` + + breakingChangeReviewRequiredLabel.logString() + + versioningReviewRequiredLabel.logString(), + ); + } +} + +/** + * The "production" branches are defined at https://aka.ms/azsdk/pr-brch-deep + */ +function checkPrTargetsProductionBranch(prContext: PRContext): boolean { + const targetsPublicProductionBranch = + prContext.repo.includes("azure-rest-api-specs") && + prContext.repo !== "azure-rest-api-specs-pr" && + prContext.targetBranch === "main"; + + const targetsPrivateProductionBranch = + prContext.repo === "azure-rest-api-specs-pr" && prContext.targetBranch === "RPSaaSMaster"; + + return targetsPublicProductionBranch || targetsPrivateProductionBranch; +} + +async function processTypeSpec(ctx: PRContext, labelContext: LabelContext): Promise { + console.log("ENTER definition processTypeSpec"); + const typeSpecLabel = new Label("TypeSpec", labelContext.present); + // By default this label should not be present. We may determine later in this function that it should be present after all. + typeSpecLabel.shouldBePresent = false; + const handlers: ChangeHandler[] = []; + const typeSpecFileHandler = () => { + return (_: PRChange) => { + // Note: this code will be executed if the PR has a diff on a TypeSpec file, + // as defined in public/swagger-validation-common/src/context.ts/defaultFilePatterns/typespec + typeSpecLabel.shouldBePresent = true; + }; + }; + const swaggerFileHandler = () => { + return (prChange: PRChange) => { + if (prChange.changeType !== "Deletion" && isSwaggerGeneratedByTypeSpec(prChange.filePath)) { + typeSpecLabel.shouldBePresent = true; + } + }; + }; + handlers.push({ + TypeSpecFile: typeSpecFileHandler(), + SwaggerFile: swaggerFileHandler(), + }); + await processPrChanges(ctx, handlers); + typeSpecLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + console.log("RETURN definition processTypeSpec"); + + return typeSpecLabel.shouldBePresent; +} + +function isSwaggerGeneratedByTypeSpec(swaggerFilePath: string): boolean { + try { + return !!JSON.parse(fs.readFileSync(swaggerFilePath).toString())?.info["x-typespec-generated"]; + } catch { + return false; + } +} + +export async function processPrChanges(ctx: PRContext, Handlers: ChangeHandler[]) { + console.log("ENTER definition processPrChanges"); + const prChanges = await getPRChanges(ctx); + prChanges.forEach((prChange) => { + Handlers.forEach((handler) => { + if (prChange.fileType in handler) { + handler?.[prChange.fileType]?.(prChange); + } + }); + }); + console.log("RETURN definition processPrChanges"); +} + +export async function processPRChangesAsync(ctx: PRContext, Handlers: ChangeHandler[]) { + console.log("ENTER definition processPRChangesAsync"); + const prChanges = await getPRChanges(ctx); + + for (const prChange of prChanges) { + for (const handler of Handlers) { + if (prChange.fileType in handler) { + await handler?.[prChange.fileType]?.(prChange); + } + } + } + + console.log("RETURN definition processPRChangesAsync"); +} + +export async function getPRChanges(ctx: PRContext): Promise { + console.log("ENTER definition getPRChanges"); + const results: PRChange[] = []; + + function newChange( + fileType: FileTypes, + changeType: ChangeTypes, + filePath?: string, + additionalInfo?: any, + ) { + if (filePath) { + results.push({ + filePath, + fileType, + changeType, + additionalInfo, + }); + } + } + + function newChanges(fileType: FileTypes, changeType: ChangeTypes, files?: string[]) { + if (files) { + files.forEach((filePath) => newChange(fileType, changeType, filePath)); + } + } + + function genChanges(type: FileTypes, diffs: DiffResult) { + newChanges(type, "Addition", diffs.additions); + newChanges(type, "Deletion", diffs.deletions); + newChanges(type, "Update", diffs.changes); + } + + function genReadmeChanges(readmeDiffs?: DiffResult) { + if (readmeDiffs) { + readmeDiffs.additions?.forEach((d) => newChange("ReadmeFile", "Addition", d.readme, d.tags)); + readmeDiffs.changes?.forEach((d) => newChange("ReadmeFile", "Update", d.readme, d.tags)); + readmeDiffs.deletions?.forEach((d) => newChange("ReadmeFile", "Deletion", d.readme, d.tags)); + } + } + + genChanges("SwaggerFile", ctx.getSwaggerDiffs()); + genChanges("TypeSpecFile", ctx.getTypeSpecDiffs()); + genChanges("ExampleFile", ctx.getExampleDiffs()); + genReadmeChanges(await ctx.getReadmeDiffs()); + + console.log("RETURN definition getPRChanges"); + return results; +} + +// related pending work: +// If a PR has both data-plane and resource-manager labels, it should fail with appropriate messaging #8144 +// https://github.com/Azure/azure-sdk-tools/issues/8144 +async function processPRType( + context: PRContext, + labelContext: LabelContext, +): Promise<{ resourceManagerLabelShouldBePresent: boolean }> { + console.log("ENTER definition processPRType"); + const types: PRType[] = await getPRType(context); + + const resourceManagerLabelShouldBePresent = processPRTypeLabel( + "resource-manager", + types, + labelContext, + ); + processPRTypeLabel("data-plane", types, labelContext); + + console.log("RETURN definition processPRType"); + return { resourceManagerLabelShouldBePresent }; +} + +async function getPRType(context: PRContext): Promise { + console.log("ENTER definition getPRType"); + const prChanges: PRChange[] = await getPRChanges(context); + + logPRChanges(prChanges); + + const changedFilePaths: string[] = prChanges.map((it) => it.filePath); + + const prTypes: PRType[] = []; + if (changedFilePaths.length > 0) { + if (isDataPlanePR(changedFilePaths)) { + prTypes.push("data-plane"); + } + if (isManagementPR(changedFilePaths)) { + prTypes.push("resource-manager"); + } + } + console.log("RETURN definition getPRType"); + return prTypes; + + function logPRChanges(prChanges: PRChange[]) { + console.log(`PR changes table (count: ${prChanges.length}):`); + console.log("LEGEND: changeType | fileType | filePath"); + prChanges + .map((it) => `${it.changeType} | ${it.fileType} | ${it.filePath}`) + .forEach((it) => console.log(it)); + console.log("END of PR changes table"); + + console.log("PR changes with additionalInfo:"); + prChanges.filter((it) => it.additionalInfo != null).forEach((it) => console.log(it)); + console.log("END of PR changes with additionalInfo"); + } +} + +function processPRTypeLabel( + labelName: PRType, + types: PRType[], + labelContext: LabelContext, +): boolean { + const label = new Label(labelName, labelContext.present); + label.shouldBePresent = types.includes(labelName); + + label.applyStateChange(labelContext.toAdd, labelContext.toRemove); + return label.shouldBePresent; +} + +async function processSuppression(context: PRContext, labelContext: LabelContext) { + console.log("ENTER definition processSuppression"); + + const suppressionReviewRequiredLabel = new Label( + "SuppressionReviewRequired", + labelContext.present, + ); + // By default this label should not be present. We may determine later in this function that it should be present after all. + suppressionReviewRequiredLabel.shouldBePresent = false; + const handlers: ChangeHandler[] = []; + + const createReadmeFileHandler = () => { + return (e: PRChange) => { + if ( + (e.changeType === "Addition" && getSuppressions(e.filePath).length) || + (e.changeType === "Update" && + diffSuppression( + path.resolve(context.targetDirectory, e.filePath), + path.resolve(context.sourceDirectory, e.filePath), + ).length) + ) { + suppressionReviewRequiredLabel.shouldBePresent = true; + } + }; + }; + handlers.push({ + ReadmeFile: createReadmeFileHandler(), + }); + await processPrChanges(context, handlers); + + suppressionReviewRequiredLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + console.log("RETURN definition processSuppression"); + + return suppressionReviewRequiredLabel.shouldBePresent; +} + +function getSuppressions(readmePath: string) { + const walkToNode = ( + walker: commonmark.NodeWalker, + cb: (node: commonmark.Node) => boolean, + ): commonmark.Node | undefined => { + let event = walker.next(); + + while (event) { + const curNode = event.node; + if (cb(curNode)) { + return curNode; + } + event = walker.next(); + } + return undefined; + }; + const getAllCodeBlockNodes = (startNode: commonmark.Node) => { + const walker = startNode.walker(); + const result = []; + while (true) { + const a = walkToNode(walker, (n) => n.type === "code_block"); + if (!a) { + break; + } + result.push(a); + } + return result; + }; + let suppressionResult: any[] = []; + try { + const readme = fs.readFileSync(readmePath).toString(); + const codeBlocks = getAllCodeBlockNodes(new commonmark.Parser().parse(readme)); + for (const block of codeBlocks) { + if (block.literal) { + try { + const blockObject = yaml.load(block.literal) as any; + const directives = blockObject?.["directive"]; + if (directives && Array.isArray(directives)) { + suppressionResult = suppressionResult.concat(directives.filter((s) => s.suppress)); + } + const suppressions = blockObject?.["suppressions"]; + if (suppressions && Array.isArray(suppressions)) { + suppressionResult = suppressionResult.concat(suppressions); + } + } catch (e) {} + } + } + } catch (e) {} + return suppressionResult; +} + +export function diffSuppression(readmeBefore: string, readmeAfter: string) { + const beforeSuppressions = getSuppressions(readmeBefore); + const afterSuppressions = getSuppressions(readmeAfter); + const newSuppressions = []; + for (const suppression of afterSuppressions) { + const properties = ["suppress", "from", "where", "code", "reason"]; + if ( + -1 === + beforeSuppressions.findIndex((s) => properties.every((p) => _.isEqual(s[p], suppression[p]))) + ) { + newSuppressions.push(suppression); + } + } + return newSuppressions; +} + +async function processRPaaS( + context: PRContext, + labelContext: LabelContext, +): Promise<{ rpaasLabelShouldBePresent: boolean }> { + console.log("ENTER definition processRPaaS"); + const rpaasLabel = new Label("RPaaS", labelContext.present); + // By default this label should not be present. We may determine later in this function that it should be present after all. + rpaasLabel.shouldBePresent = false; + + const handlers: ChangeHandler[] = []; + const createReadmeFileHandler = () => { + return async (e: PRChange) => { + if ( + e.changeType !== "Deletion" && + (await isRPSaaS(path.join(context.sourceDirectory, e.filePath))) + ) { + rpaasLabel.shouldBePresent = true; + } + }; + }; + handlers.push({ + ReadmeFile: createReadmeFileHandler(), + }); + await processPRChangesAsync(context, handlers); + + rpaasLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + console.log("RETURN definition processRPaaS"); + return { rpaasLabelShouldBePresent: rpaasLabel.shouldBePresent }; +} + +async function isRPSaaS(readmeFilePath: string) { + const config: any = await new Readme(readmeFilePath).getGlobalConfig(); + return config["openapi-subtype"] === "rpaas" || config["openapi-subtype"] === "providerHub"; +} + +async function processNewRPNamespace( + context: PRContext, + labelContext: LabelContext, + resourceManagerLabelShouldBePresent: boolean, +): Promise<{ newRPNamespaceLabelShouldBePresent: boolean }> { + console.log("ENTER definition processNewRPNamespace"); + const newRPNamespaceLabel = new Label("new-rp-namespace", labelContext.present); + // By default this label should not be present. We may determine later in this function that it should be present after all. + newRPNamespaceLabel.shouldBePresent = false; + const handlers: ChangeHandler[] = []; + + let skip = false; + + const targetBranch = context.targetBranch; + if (targetBranch !== "main" && targetBranch !== "ARMCoreRPDev") { + console.log(`Not main or ARMCoreRPDev branch, skip new RP namespace check`); + skip = true; + } + + if (!skip && !resourceManagerLabelShouldBePresent) { + console.log(`Not resource-manager, skip new RP namespace check`); + skip = true; + } + + if (!skip) { + const createSwaggerFileHandler = () => { + return (e: PRChange) => { + if (e.changeType === "Addition") { + const rpFolder = getRPFolderFromSwaggerFile(path.dirname(e.filePath)); + console.log(`Processing newRPNameSpace rpFolder: ${rpFolder}`); + if (rpFolder !== undefined) { + const rpFolderFullPath = path.resolve(context.targetDirectory, rpFolder); + if (!fs.existsSync(rpFolderFullPath)) { + console.log(`Adding newRPNameSpace rpFolder: ${rpFolder}`); + newRPNamespaceLabel.shouldBePresent = true; + } + } + } + }; + }; + + handlers.push({ SwaggerFile: createSwaggerFileHandler() }); + await processPrChanges(context, handlers); + } + + newRPNamespaceLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + console.log("RETURN definition processNewRPNamespace"); + return { newRPNamespaceLabelShouldBePresent: newRPNamespaceLabel.shouldBePresent }; +} + +// CODESYNC: +// - see entries for related labels in https://github.com/Azure/azure-rest-api-specs/blob/main/.github/comment.yml +// - requiredLabelsRules.ts / requiredLabelsRules +async function processNewRpNamespaceWithoutRpaasLabel( + context: PRContext, + labelContext: LabelContext, + resourceManagerLabelShouldBePresent: boolean, + newRPNamespaceLabelShouldBePresent: boolean, + rpaasLabelShouldBePresent: boolean, +): Promise<{ + ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent: boolean; + rpaasExceptionLabelShouldBePresent: boolean; +}> { + console.log("ENTER definition processNewRpNamespaceWithoutRpaasLabel"); + const ciNewRPNamespaceWithoutRpaaSLabel = new Label( + "CI-NewRPNamespaceWithoutRPaaS", + labelContext.present, + ); + // By default this label should not be present. We may determine later in this function that it should be present after all. + ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent = false; + + const rpaasExceptionLabel = new Label("RPaaSException", labelContext.present); + + let skip = false; + + if (!resourceManagerLabelShouldBePresent) { + console.log(`Not resource-manager, skip checking for 'CI-NewRPNamespaceWithoutRPaaS'`); + skip = true; + } + + if (!skip) { + const branch = context.targetBranch; + if (branch === "RPSaaSDev" || branch === "RPSaaSMaster") { + console.log( + `PR is targeting '${branch}' branch. Skip checking for 'CI-NewRPNamespaceWithoutRPaaS'`, + ); + skip = true; + } + } + + if (!skip && newRPNamespaceLabelShouldBePresent && !rpaasLabelShouldBePresent) { + console.log( + "Adding new RP namespace, but not RPaaS. Label 'CI-NewRPNamespaceWithoutRPaaS' should be present.", + ); + ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent = true; + } + + rpaasExceptionLabel.shouldBePresent = + rpaasExceptionLabel.present && ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent; + + ciNewRPNamespaceWithoutRpaaSLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + rpaasExceptionLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + console.log("RETURN definition processNewRpNamespaceWithoutRpaasLabel"); + + return { + ciNewRPNamespaceWithoutRpaaSLabelShouldBePresent: + ciNewRPNamespaceWithoutRpaaSLabel.shouldBePresent, + rpaasExceptionLabelShouldBePresent: rpaasExceptionLabel.shouldBePresent as boolean, + }; +} + +// CODESYNC: see entries for related labels in https://github.com/Azure/azure-rest-api-specs/blob/main/.github/comment.yml +async function processRpaasRpNotInPrivateRepoLabel( + context: PRContext, + labelContext: LabelContext, + resourceManagerLabelShouldBePresent: boolean, + rpaasLabelShouldBePresent: boolean, +): Promise<{ ciRpaasRPNotInPrivateRepoLabelShouldBePresent: boolean }> { + console.log("ENTER definition processRpaasRpNotInPrivateRepoLabel"); + const ciRpaasRPNotInPrivateRepoLabel = new Label( + "CI-RpaaSRPNotInPrivateRepo", + labelContext.present, + ); + // By default this label should not be present. We may determine later in this function that it should be present after all. + ciRpaasRPNotInPrivateRepoLabel.shouldBePresent = false; + + let skip = false; + + if (!resourceManagerLabelShouldBePresent) { + console.log(`Not resource-manager, skip RPaaSRPNotInPrivateRepo check`); + skip = true; + } + + if (!skip) { + const targetBranch = context.targetBranch; + if (targetBranch !== "main" || !rpaasLabelShouldBePresent) { + console.log("Not main branch or not RPaaS PR, skip block PR when RPaaS RP not onboard"); + skip = true; + } + } + + // todo: retrieve the list and populate this value properly. + // if (!skip) { + // // this is a request to get the list of RPaaS folders from azure-rest-api-specs-pr -> RPSaasMaster branch -> dump specification folder + // // names + // const rpaasRPFolderList = await getRPaaSFolderList(); + // const rpFolderNames: string[] = rpaasRPFolderList.map((f) => f.name); + + // console.log(`RPaaS RP folder list: ${rpFolderNames}`); + + // const handlers: ChangeHandler[] = []; + + // const processPrChange = () => { + // return (e: PRChange) => { + // if (e.changeType === "Addition") { + // const rpFolderName = getRPRootFolderName(e.filePath); + // console.log( + // `Processing processRpaasRpNotInPrivateRepoLabel rpFolderName: ${rpFolderName}` + // ); + + // if (rpFolderName === undefined) { + // console.log(`RP folder is undefined for changed file path '${e.filePath}'.`); + // return; + // } + + // if (!rpFolderNames.includes(rpFolderName)) { + // console.log( + // `This RP is RPSaaS RP but could not find rpFolderName: ${rpFolderName} in RPFolderNames: ${rpFolderNames}. ` + // + `Label 'CI-RpaaSRPNotInPrivateRepo' should be present.` + // ); + // ciRpaasRPNotInPrivateRepoLabel.shouldBePresent = true; + // } + // } + // }; + // }; + + // handlers.push({ SwaggerFile: processPrChange() }); + // await processPrChanges(context, handlers); + // } + + ciRpaasRPNotInPrivateRepoLabel.applyStateChange(labelContext.toAdd, labelContext.toRemove); + console.log("RETURN definition processRpaasRpNotInPrivateRepoLabel"); + + return { + ciRpaasRPNotInPrivateRepoLabelShouldBePresent: ciRpaasRPNotInPrivateRepoLabel.shouldBePresent, + }; +} diff --git a/eng/tools/summarize-impact/src/labelling-types.ts b/eng/tools/summarize-impact/src/labelling-types.ts new file mode 100644 index 000000000000..fbd3bccfb616 --- /dev/null +++ b/eng/tools/summarize-impact/src/labelling-types.ts @@ -0,0 +1,123 @@ +export type PRType = "resource-manager" | "data-plane"; + +/** + * The LabelContext is used by prSummary.ts / summary() and downstream invocations. + * + * The "present" set represents the set of labels that are currently present on the PR + * processed by given invocation of summary(). It is obtained via GitHub Octokit API at the beginning + * of summary(). + * + * The "toAdd" set is the set of labels to be added to the PR at the end of invocation of summary(). + * This is to be done by calling GitHub Octokit API to add the labels. + * + * The "toRemove" set is analogous to "toAdd" set, but instead it is the set of labels to be removed. + * + * The general pattern used in the code to populate "toAdd" or "toRemove" sets to be ready for + * Octokit invocation is as follows: + * + * - the summary() function passes the context through its invocation chain. + * - given function responsible for given label, like e.g. for label "ARMReview", + * creates a new instance of Label: const armReviewLabel = new Label("ARMReview", labelContext.present) + * - the function then processes the label to determine if armReviewLabel.shouldBePresent is to be set to true or false. + * - the function at the end of its invocation calls armReviewLabel.applyStateChanges(labelContext.toAdd, labelContext.toRemove) + * to update the sets. + * - the function may optionally return { armReviewLabel.shouldBePresent } to allow the caller to pass this value + * further to downstream business logic that depends on it. + * - at the end of invocation summary() calls Octokit passing it as input labelContext.toAdd and labelContext.toRemove. + * + * todo: this type is duplicated in JSDoc over in summarize-checks.js + */ +export type LabelContext = { + present: Set; + toAdd: Set; + toRemove: Set; +}; + +export class Label { + name: string; + + /** Is the label currently present on the pull request? + * + * This is determined at the time of construction of this object. + */ + present?: boolean; + + /** Should this label be present on the pull request? + * + * Must be defined before applyStateChange is called. + * + * Not set at the construction time to facilitate determining desired presence + * of multiple labels in single code block, without intermixing it with + * label construction logic. + */ + shouldBePresent: boolean | undefined = undefined; + + constructor(name: string, presentLabels?: Set) { + this.name = name; + this.present = presentLabels?.has(this.name) ?? undefined; + } + + /** + * If the label should be added, add its name to labelsToAdd. + * If the label should be removed, add its name to labelsToRemove. + * Otherwise, do nothing. + * + * Precondition: this.shouldBePresent has been defined. + */ + applyStateChange(labelsToAdd: Set, labelsToRemove: Set): void { + if (this.shouldBePresent === undefined) { + console.warn( + "ASSERTION VIOLATION! " + + `Cannot applyStateChange for label '${this.name}' ` + + "as its desired presence hasn't been defined. Returning early.", + ); + return; + } + + if (!this.present && this.shouldBePresent) { + if (!labelsToAdd.has(this.name)) { + console.log( + `Label.applyStateChange: '${this.name}' was not present and should be present. Scheduling addition.`, + ); + labelsToAdd.add(this.name); + } else { + console.log( + `Label.applyStateChange: '${this.name}' was not present and should be present. It is already scheduled for addition.`, + ); + } + } else if (this.present && !this.shouldBePresent) { + if (!labelsToRemove.has(this.name)) { + console.log( + `Label.applyStateChange: '${this.name}' was present and should not be present. Scheduling removal.`, + ); + labelsToRemove.add(this.name); + } else { + console.log( + `Label.applyStateChange: '${this.name}' was present and should not be present. It is already scheduled for removal.`, + ); + } + } else if (this.present === this.shouldBePresent) { + console.log( + `Label.applyStateChange: '${this.name}' is ${this.present ? "present" : "not present"}. This is the desired state.`, + ); + } else { + console.warn( + "ASSERTION VIOLATION! " + + `Label.applyStateChange: '${this.name}' is ${this.present ? "present" : "not present"} while it should be ${this.shouldBePresent ? "present" : "not present"}. ` + + `At this point of execution this should not happen.`, + ); + } + } + + isEqualToOrPrefixOf(label: string): boolean { + return this.name.endsWith("*") ? label.startsWith(this.name.slice(0, -1)) : this.name === label; + } + + logString(): string { + return ( + `Label: name: ${this.name}, ` + + `present: ${this.present}, ` + + `shouldBePresent: ${this.shouldBePresent}. ` + ); + } +} diff --git a/eng/tools/summarize-impact/src/types.ts b/eng/tools/summarize-impact/src/types.ts new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/eng/tools/summarize-impact/test/cli.test.ts b/eng/tools/summarize-impact/test/cli.test.ts new file mode 100644 index 000000000000..9e1fa1f18853 --- /dev/null +++ b/eng/tools/summarize-impact/test/cli.test.ts @@ -0,0 +1,107 @@ +import { describe, it, expect } from "vitest"; //vi + +import path from "path"; + +import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; +import { PRContext } from "../src/PRContext.js"; +import { LabelContext } from "../src/labelling-types.js"; +import { evaluateImpact } from "../src/impact.js"; + +describe("Check Changes", () => { + it.skipIf(!process.env.GITHUB_TOKEN || !process.env.INTEGRATION_TEST)( + "Integration test 35346", + async () => { + const targetDirectory = path.join("/home/semick/repo/rest-s/35346", "before"); + const sourceDirectory = path.join("/home/semick/repo/rest-s/35346", "after"); + + // Change to source directory and save original + const originalCwd = process.cwd(); + process.chdir(sourceDirectory); + + try { + const changedFileDetails = await getChangedFilesStatuses({ + cwd: sourceDirectory, + baseCommitish: "origin/main", + }); + const labelContext: LabelContext = { + present: new Set(), + toAdd: new Set(), + toRemove: new Set(), + }; + + const prContext = new PRContext(sourceDirectory, targetDirectory, labelContext, { + sha: "ad7c74cb27d2cf3ba83996aaea36b07caa4d16c8", + sourceBranch: "dev/nandiniy/DTLTypeSpec", + targetBranch: "main", + repo: "azure-rest-api-specs", + prNumber: "35346", + owner: "Azure", + fileList: changedFileDetails, + isDraft: false, + }); + + const result = await evaluateImpact(prContext, labelContext); + + expect(result).toBeDefined(); + expect(result.typeSpecChanged).toBeTruthy(); + expect(result.labelContext.toAdd.has("resource-manager")).toBeTruthy(); + expect(result.labelContext.toAdd.has("SuppressionReviewRequired")).toBeTruthy(); + expect(changedFileDetails).toBeDefined(); + expect(changedFileDetails.total).toEqual(293); + } finally { + // Restore original directory + process.chdir(originalCwd); + } + }, + 60000000, + ); + + it.skipIf(!process.env.GITHUB_TOKEN || !process.env.INTEGRATION_TEST)( + "Integration test 35982", + async () => { + const targetDirectory = path.join("/home/semick/repo/rest-s/35982", "before"); + const sourceDirectory = path.join("/home/semick/repo/rest-s/35982", "after"); + + // Change to source directory and save original + const originalCwd = process.cwd(); + process.chdir(sourceDirectory); + + try { + const changedFileDetails = await getChangedFilesStatuses({ + cwd: sourceDirectory, + baseCommitish: "origin/main", + }); + const labelContext: LabelContext = { + present: new Set(), + toAdd: new Set(), + toRemove: new Set(), + }; + + const prContext = new PRContext(sourceDirectory, targetDirectory, labelContext, { + sha: "2bd8350d465081401a0f4f03e633eca41f0991de", + sourceBranch: "features/users/deepika/cosmos-connectors-confluent", + targetBranch: "main", + repo: "azure-rest-api-specs", + prNumber: "35982", + owner: "Azure", + fileList: changedFileDetails, + isDraft: false, + }); + + const result = await evaluateImpact(prContext, labelContext); + expect(result.isNewApiVersion).toBeTruthy(); + expect(result.labelContext.toAdd.has("TypeSpec")).toBeTruthy(); + expect(result.labelContext.toAdd.has("resource-manager")).toBeTruthy(); + expect(result.isNewApiVersion).toBeTruthy(); + expect(result.labelContext.toAdd.has("ARMReview")).toBeTruthy(); + expect(result.labelContext.toAdd.has("RPaaS")).toBeTruthy(); + expect(result.labelContext.toAdd.has("WaitForARMFeedback")).toBeTruthy(); + expect(result).toBeDefined(); + } finally { + // Restore original directory + process.chdir(originalCwd); + } + }, + 60000000, + ); +}); diff --git a/eng/tools/summarize-impact/tsconfig.json b/eng/tools/summarize-impact/tsconfig.json new file mode 100644 index 000000000000..5f48d4c6a5b5 --- /dev/null +++ b/eng/tools/summarize-impact/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "outDir": "./dist", + "rootDir": ".", + "allowJs": true, + }, + "include": ["*.ts", "src/**/*.ts", "test/**/*.ts"], +} diff --git a/eng/tools/tsconfig.json b/eng/tools/tsconfig.json index ac5fe54c268f..ba7486de2512 100644 --- a/eng/tools/tsconfig.json +++ b/eng/tools/tsconfig.json @@ -41,5 +41,8 @@ { "path": "./typespec-validation", }, + { + "path": "./summarize-impact", + }, ], } diff --git a/package-lock.json b/package-lock.json index e49406c5ec48..c686bd969fd4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -92,6 +92,7 @@ "@azure-tools/openapi-diff-runner": "file:openapi-diff-runner", "@azure-tools/sdk-suppressions": "file:sdk-suppressions", "@azure-tools/spec-gen-sdk-runner": "file:spec-gen-sdk-runner", + "@azure-tools/summarize-impact": "file:summarize-impact", "@azure-tools/suppressions": "file:suppressions", "@azure-tools/tsp-client-tests": "file:tsp-client-tests", "@azure-tools/typespec-migration-validation": "file:typespec-migration-validation", @@ -204,6 +205,46 @@ "dev": true, "license": "MIT" }, + "eng/tools/node_modules/glob": { + "version": "11.0.3", + "resolved": "https://registry.npmjs.org/glob/-/glob-11.0.3.tgz", + "integrity": "sha512-2Nim7dha1KVkaiF4q6Dj+ngPPMdfvLJEOpZk/jKiUAkqKebpGAWQXAq9z1xu9HKu5lWfqw/FASuccEjyznjPaA==", + "dev": true, + "license": "ISC", + "dependencies": { + "foreground-child": "^3.3.1", + "jackspeak": "^4.1.1", + "minimatch": "^10.0.3", + "minipass": "^7.1.2", + "package-json-from-dist": "^1.0.0", + "path-scurry": "^2.0.0" + }, + "bin": { + "glob": "dist/esm/bin.mjs" + }, + "engines": { + "node": "20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "eng/tools/node_modules/jackspeak": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/jackspeak/-/jackspeak-4.1.1.tgz", + "integrity": "sha512-zptv57P3GpL+O0I7VdMJNBZCu+BPHVQUk55Ft8/QCJjTVxrnJHuVuX/0Bl2A6/+2oyR/ZMEuFKwmzqqZ/U5nPQ==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "@isaacs/cliui": "^8.0.2" + }, + "engines": { + "node": "20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "eng/tools/node_modules/json-schema-traverse": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz", @@ -211,6 +252,16 @@ "dev": true, "license": "MIT" }, + "eng/tools/node_modules/lru-cache": { + "version": "11.1.0", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-11.1.0.tgz", + "integrity": "sha512-QIXZUBJUx+2zHUdQujWejBkcD9+cs94tLn0+YL8UrCh+D5sCXZ4c7LaEH48pNwRY3MLDgqUFyhlCyjJPf1WP0A==", + "dev": true, + "license": "ISC", + "engines": { + "node": "20 || >=22" + } + }, "eng/tools/node_modules/marked": { "version": "16.0.0", "resolved": "https://registry.npmjs.org/marked/-/marked-16.0.0.tgz", @@ -240,6 +291,23 @@ "url": "https://github.com/sponsors/isaacs" } }, + "eng/tools/node_modules/path-scurry": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/path-scurry/-/path-scurry-2.0.0.tgz", + "integrity": "sha512-ypGJsmGtdXUOeM5u93TyeIEfEhM6s+ljAhrk5vAvSx8uyY/02OvrZnA0YNGUrPXfpJMgI1ODd3nwz8Npx4O4cg==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "lru-cache": "^11.0.0", + "minipass": "^7.1.2" + }, + "engines": { + "node": "20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "eng/tools/node_modules/string-width": { "version": "7.2.0", "resolved": "https://registry.npmjs.org/string-width/-/string-width-7.2.0.tgz", @@ -420,6 +488,36 @@ "node": ">=20.0.0" } }, + "eng/tools/summarize-impact": { + "name": "@azure-tools/summarize-impact", + "dev": true, + "dependencies": { + "@azure-tools/openapi-tools-common": "^1.2.2", + "@azure-tools/specs-shared": "file:../../../.github/shared", + "@azure/openapi-markdown": "0.9.4", + "@ts-common/commonmark-to-markdown": "^2.0.2", + "commonmark": "^0.29.0", + "glob": "^11.0.3", + "js-yaml": "^4.1.0", + "lodash": "^4.17.20", + "simple-git": "^3.27.0" + }, + "bin": { + "summarize-impact": "cmd/summarize-impact.js" + }, + "devDependencies": { + "@types/commonmark": "^0.27.4", + "@types/glob": "^8.1.0", + "@types/lodash": "^4.14.161", + "@types/node": "^20.0.0", + "prettier": "~3.5.3", + "typescript": "~5.8.2", + "vitest": "^3.0.7" + }, + "engines": { + "node": ">=20.0.0" + } + }, "eng/tools/suppressions": { "name": "@azure-tools/suppressions", "dev": true, @@ -913,6 +1011,10 @@ "resolved": ".github/shared", "link": true }, + "node_modules/@azure-tools/summarize-impact": { + "resolved": "eng/tools/summarize-impact", + "link": true + }, "node_modules/@azure-tools/suppressions": { "resolved": "eng/tools/suppressions", "link": true @@ -4629,6 +4731,17 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/glob": { + "version": "8.1.0", + "resolved": "https://registry.npmjs.org/@types/glob/-/glob-8.1.0.tgz", + "integrity": "sha512-IO+MJPVhoqz+28h1qLAcBEH2+xHMK6MTyHJc7MTnnYb6wsoLR29POVGJ7LycmVXIqyy/4/2ShP5sUwTXuOwb/w==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/minimatch": "^5.1.2", + "@types/node": "*" + } + }, "node_modules/@types/js-yaml": { "version": "4.0.9", "resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-4.0.9.tgz", @@ -4657,6 +4770,13 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/minimatch": { + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-5.1.2.tgz", + "integrity": "sha512-K0VQKziLUWkVKiRVrx4a40iPaxTUefQmjtkQofBkYRcoaaL/8rhwDWww9qWbrgicNOgnpIsMxyNIUM4+n6dUIA==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/ms": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/@types/ms/-/ms-2.1.0.tgz",