Fixing the Audience in the load testing apis | Adding the retries for patch test | Securty issue to update the shell commands (#131)

* Update README.md

* Refactor cloud token scopes to use TaskParameters

* Add token scopes for Azure management

* Simplify token retrieval in AuthenticatorService

Refactor getTokenAPI to simplify command arguments.

* Update AzCliUtility.js

* Refactor httpClientRetries for improved readability

* Refactor getTaskParameters and update subscription logic

* Refactor token scopes to use enums

* Refactor token scopes in TaskParameters interface

* Simplify token retrieval in getTokenAPI

Refactor getTokenAPI to simplify command arguments.

* Refactor execAz function to accept token scope

* Refactor httpClientRetries for improved error handling

* Refactor subscription parameter retrieval methods

* Refactor test stubs for AzCliUtility calls

* Increase timeout for deleteFileAPI test

* Add conditional for installing dependencies on main branch

* correcting UTs.

* fixing installation step.

* updating redme.

* printing the platform

* account show is not working in the cmd.

* changed back to true.

* changing js too.

* change.

* updating function name and removing console.

Author: mohitpavan

.github/workflows/pr_check_load_test.yml

@@ -64,6 +64,7 @@ jobs:
           node-version: '20'
 
       - name: Installing dependencies and building latest changes
+        if: github.ref == 'refs/heads/main' || (github.event_name == 'pull_request' && github.base_ref == 'main')
         run: |
           npm install --include=dev -f
           npm ci

README.md

@@ -54,7 +54,13 @@ For using any credentials like Azure Service Principal in your workflow, add the
       with:
         creds: ${{ secrets.AZURE_CREDENTIALS }}
     ```
-    
+
+## Breaking changes
+
+All minor versions under `1.1` (e.g., `1.1.x`) will stop working due to a breaking change.
+To ensure continued compatibility, we recommend switching to the `v1` tag, which we will continue to support.
+We are also bumping the minor version to `1.2.1`. If you prefer to pin to a specific version, please update your workflows to use `1.2.1`.
+
 ## Azure Load Testing Action
 
 This section describes the Azure Load Testing GitHub action. You can use this action by referencing `azure/load-testing@v1` action in your workflow. The action runs on Windows, Linux, and Mac runners.

lib/Constants/EnvironmentConstants.js

@@ -1,14 +1,15 @@
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
 exports.AzureUSGovernmentCloud = exports.AzurePublicCloud = void 0;
+const TaskParameters_1 = require("../models/TaskParameters");
 exports.AzurePublicCloud = {
     cloudName: "AzureCloud",
-    armTokenScope: "https://management.core.windows.net",
-    dataPlaneTokenScope: "https://loadtest.azure-dev.com",
+    armTokenScope: TaskParameters_1.armPublicTokenScope,
+    dataPlaneTokenScope: TaskParameters_1.publicTokenScope,
     armEndpoint: "https://management.azure.com",
 };
 exports.AzureUSGovernmentCloud = {
     cloudName: "AzureUSGovernment",
-    armTokenScope: "https://management.usgovcloudapi.net",
-    dataPlaneTokenScope: "https://cnt-prod.loadtesting.azure.us",
+    armTokenScope: TaskParameters_1.armUsGovernmentTokenScope,
+    dataPlaneTokenScope: TaskParameters_1.usGovernmentTokenScope,
 };

lib/Utils/AzCliUtility.js

@@ -9,13 +9,27 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
     });
 };
 Object.defineProperty(exports, "__esModule", { value: true });
-exports.execAz = void 0;
+exports.getDPTokens = getDPTokens;
+exports.getAccounts = getAccounts;
 const child_process_1 = require("child_process");
+function getDPTokens(tokenScope) {
+    return __awaiter(this, void 0, void 0, function* () {
+        const cmdArguments = ["account", "get-access-token", "--resource"];
+        cmdArguments.push(tokenScope);
+        return execAz(cmdArguments);
+    });
+}
+function getAccounts(accountType) {
+    return __awaiter(this, void 0, void 0, function* () {
+        const cmdArguments = accountType === 'Subscription' ? ["account", "show"] : ["cloud", "show"];
+        return execAz(cmdArguments);
+    });
+}
 function execAz(cmdArguments) {
     return __awaiter(this, void 0, void 0, function* () {
         const azCmd = process.platform === "win32" ? "az.cmd" : "az";
         return new Promise((resolve, reject) => {
-            (0, child_process_1.execFile)(azCmd, [...cmdArguments, "--out", "json"], { encoding: "utf8", shell: true }, (error, stdout) => {
+            (0, child_process_1.execFile)(azCmd, [...cmdArguments, "--out", "json"], { encoding: "utf8", shell: process.platform === "win32" }, (error, stdout) => {
                 if (error) {
                     return reject(error);
                 }
@@ -31,4 +45,3 @@ function execAz(cmdArguments) {
         });
     });
 }
-exports.execAz = execAz;

lib/Utils/FetchUtils.js

@@ -47,11 +47,13 @@ const methodEnumToString = {
     [UtilModels_1.FetchCallType.patch]: "patch"
 };
 // (note mohit): shift to the enum later.
-function httpClientRetries(urlSuffix_1, header_1, method_1) {
-    return __awaiter(this, arguments, void 0, function* (urlSuffix, header, method, retries = 1, data, isUploadCall = true, log = true) {
+function httpClientRetries(urlSuffix, header, method, retries = 1, data, isUploadCall = true, log = true) {
+    return __awaiter(this, void 0, void 0, function* () {
         let httpResponse;
+        const retrriableCodes = [408, 429, 500, 502, 503, 504]; // 408 - Request Timeout, 429 - Too Many Requests, 500 - Internal Server Error, 502 - Bad Gateway, 503 - Service Unavailable, 504 - Gateway Timeout
+        let backOffTimeForRetry = 5; // seconds
+        let correlationId = `gh-actions-${(0, CommonUtils_1.getUniqueId)()}`;
         try {
-            let correlationId = `gh-actions-${(0, CommonUtils_1.getUniqueId)()}`;
             header[UtilModels_1.correlationHeader] = correlationId; // even if we put console.debug its printing along with the logs, so lets just go ahead with the differentiation with azdo, so we can search the timeframe for azdo in correlationid and resource filter.
             if (method == UtilModels_1.FetchCallType.get) {
                 httpResponse = yield httpClient.get(urlSuffix, header);
@@ -79,22 +81,26 @@ function httpClientRetries(urlSuffix_1, header_1, method_1) {
             if (httpResponse.message.statusCode != undefined && httpResponse.message.statusCode >= 300) {
                 CoreUtils.debug(`correlation id : ${correlationId}`);
             }
-            if (httpResponse.message.statusCode != undefined && [408, 429, 502, 503, 504].includes(httpResponse.message.statusCode)) {
+            if (httpResponse.message.statusCode != undefined && retrriableCodes.includes(httpResponse.message.statusCode)) {
+                if (method == UtilModels_1.FetchCallType.patch) {
+                    backOffTimeForRetry += 60; // extra 60 seconds for patch, basically this happens when the service didnot handle some of the external service dependencies, and the external can take time to recover.
+                }
                 let err = yield (0, CommonUtils_1.getResultObj)(httpResponse);
                 throw { message: (err && err.error && err.error.message) ? err.error.message : (0, CommonUtils_1.errorCorrection)(httpResponse) }; // throwing as message to catch it as err.message
             }
             return httpResponse;
         }
         catch (err) {
             if (retries) {
-                let sleeptime = (5 - retries) * 1000 + Math.floor(Math.random() * 5001);
+                let sleeptime = backOffTimeForRetry * 1000;
                 if (log) {
                     console.log(`Failed to connect to ${urlSuffix} due to ${err.message}, retrying in ${sleeptime / 1000} seconds`);
                 }
                 yield (0, CommonUtils_1.sleep)(sleeptime);
                 return yield httpClientRetries(urlSuffix, header, method, retries - 1, data);
             }
             else {
+                console.log(err, "\ncorrelationId:" + correlationId);
                 throw new Error(`Operation did not succeed after 3 retries. Pipeline failed with error : ${err.message}`);
             }
         }

lib/Utils/TaskParametersUtil.js

@@ -15,13 +15,23 @@ var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (
 }) : function(o, v) {
     o["default"] = v;
 });
-var __importStar = (this && this.__importStar) || function (mod) {
-    if (mod && mod.__esModule) return mod;
-    var result = {};
-    if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k);
-    __setModuleDefault(result, mod);
-    return result;
-};
+var __importStar = (this && this.__importStar) || (function () {
+    var ownKeys = function(o) {
+        ownKeys = Object.getOwnPropertyNames || function (o) {
+            var ar = [];
+            for (var k in o) if (Object.prototype.hasOwnProperty.call(o, k)) ar[ar.length] = k;
+            return ar;
+        };
+        return ownKeys(o);
+    };
+    return function (mod) {
+        if (mod && mod.__esModule) return mod;
+        var result = {};
+        if (mod != null) for (var k = ownKeys(mod), i = 0; i < k.length; i++) if (k[i] !== "default") __createBinding(result, mod, k[i]);
+        __setModuleDefault(result, mod);
+        return result;
+    };
+})();
 var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
     function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); }
     return new (P || (P = Promise))(function (resolve, reject) {
@@ -73,8 +83,7 @@ class TaskParametersUtil {
     static setSubscriptionParameters(taskParameters) {
         return __awaiter(this, void 0, void 0, function* () {
             try {
-                const cmdArguments = ["account", "show"];
-                var result = yield AzCliUtility.execAz(cmdArguments);
+                var result = yield AzCliUtility.getAccounts('Subscription');
                 taskParameters.subscriptionId = result.id;
                 taskParameters.subscriptionName = result.name;
             }
@@ -88,8 +97,7 @@ class TaskParametersUtil {
     static setEndpointAndScopeParameters(taskParameters) {
         return __awaiter(this, void 0, void 0, function* () {
             try {
-                const cmdArguments = ["cloud", "show"];
-                var result = yield AzCliUtility.execAz(cmdArguments);
+                var result = yield AzCliUtility.getAccounts('Cloud');
                 let env = result ? result.name : null;
                 taskParameters.environment = env !== null && env !== void 0 ? env : EnvironmentConstants.AzurePublicCloud.cloudName;
                 let endpointUrl = (result && result.endpoints) ? result.endpoints.resourceManager : null;

lib/models/TaskParameters.js

@@ -1,2 +1,7 @@
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
+exports.armUsGovernmentTokenScope = exports.armPublicTokenScope = exports.usGovernmentTokenScope = exports.publicTokenScope = void 0;
+exports.publicTokenScope = "https://cnt-prod.loadtesting.azure.com";
+exports.usGovernmentTokenScope = "https://cnt-prod.loadtesting.azure.us";
+exports.armPublicTokenScope = "https://management.core.windows.net";
+exports.armUsGovernmentTokenScope = "https://management.usgovcloudapi.net";

lib/services/AuthenticatorService.js

@@ -15,13 +15,23 @@ var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (
 }) : function(o, v) {
     o["default"] = v;
 });
-var __importStar = (this && this.__importStar) || function (mod) {
-    if (mod && mod.__esModule) return mod;
-    var result = {};
-    if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k);
-    __setModuleDefault(result, mod);
-    return result;
-};
+var __importStar = (this && this.__importStar) || (function () {
+    var ownKeys = function(o) {
+        ownKeys = Object.getOwnPropertyNames || function (o) {
+            var ar = [];
+            for (var k in o) if (Object.prototype.hasOwnProperty.call(o, k)) ar[ar.length] = k;
+            return ar;
+        };
+        return ownKeys(o);
+    };
+    return function (mod) {
+        if (mod && mod.__esModule) return mod;
+        var result = {};
+        if (mod != null) for (var k = ownKeys(mod), i = 0; i < k.length; i++) if (k[i] !== "default") __createBinding(result, mod, k[i]);
+        __setModuleDefault(result, mod);
+        return result;
+    };
+})();
 var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
     function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); }
     return new (P || (P = Promise))(function (resolve, reject) {
@@ -71,9 +81,7 @@ class AuthenticatorService {
         return __awaiter(this, void 0, void 0, function* () {
             let tokenScopeDecoded = scope == UtilModels_1.TokenScope.Dataplane ? this.taskParameters.dataPlaneTokenScope : this.taskParameters.armTokenScope;
             try {
-                const cmdArguments = ["account", "get-access-token", "--resource"];
-                cmdArguments.push(tokenScopeDecoded);
-                let result = yield AzCliUtility.execAz(cmdArguments);
+                let result = yield AzCliUtility.getDPTokens(tokenScopeDecoded);
                 let token = result.accessToken;
                 scope == UtilModels_1.TokenScope.ControlPlane ? this.controlPlaneToken = token : this.dataPlanetoken = token;
                 return token;

src/Constants/EnvironmentConstants.ts

@@ -1,19 +1,21 @@
+import { ControlPlaneTokenScope, DataPlaneTokenScope, publicTokenScope, usGovernmentTokenScope, armUsGovernmentTokenScope, armPublicTokenScope } from "../models/TaskParameters";
+
 type EnvironmentSettings = {
     cloudName: string;
-    armTokenScope: string;
-    dataPlaneTokenScope: string;
+    armTokenScope: ControlPlaneTokenScope;
+    dataPlaneTokenScope: DataPlaneTokenScope;
     armEndpoint?: string;
 }
 
 export const AzurePublicCloud: EnvironmentSettings = {
     cloudName: "AzureCloud",
-    armTokenScope: "https://management.core.windows.net",
-    dataPlaneTokenScope: "https://loadtest.azure-dev.com",
+    armTokenScope: armPublicTokenScope,
+    dataPlaneTokenScope: publicTokenScope,
     armEndpoint: "https://management.azure.com",
 }
 
 export const AzureUSGovernmentCloud: EnvironmentSettings = {
     cloudName: "AzureUSGovernment",
-    armTokenScope: "https://management.usgovcloudapi.net",
-    dataPlaneTokenScope: "https://cnt-prod.loadtesting.azure.us",
-}
+    armTokenScope: armUsGovernmentTokenScope,
+    dataPlaneTokenScope: usGovernmentTokenScope,
+}

src/Utils/AzCliUtility.ts

@@ -1,9 +1,21 @@
 import { execFile } from "child_process";
+import { AccountType, ControlPlaneTokenScope, DataPlaneTokenScope } from "../models/TaskParameters";
 
-export async function execAz(cmdArguments: string[]): Promise<any> {
+export async function getDPTokens(tokenScope: ControlPlaneTokenScope| DataPlaneTokenScope): Promise<any> {
+    const cmdArguments = ["account", "get-access-token", "--resource"];
+    cmdArguments.push(tokenScope);
+    return execAz(cmdArguments);
+}
+
+export async function getAccounts(accountType: AccountType): Promise<any> {
+    const cmdArguments = accountType === 'Subscription' ? ["account", "show"] : ["cloud", "show"];
+    return execAz(cmdArguments);
+}
+
+async function execAz(cmdArguments: string[]): Promise<any> {
     const azCmd = process.platform === "win32" ? "az.cmd" : "az";
     return new Promise<any>((resolve, reject) => {
-        execFile(azCmd, [...cmdArguments, "--out", "json"], { encoding: "utf8", shell : true }, (error:any, stdout:any) => {
+        execFile(azCmd, [...cmdArguments, "--out", "json"], { encoding: "utf8", shell : process.platform === "win32" }, (error:any, stdout:any) => {
             if (error) {
                 return reject(error);
             }
@@ -17,4 +29,4 @@ export async function execAz(cmdArguments: string[]): Promise<any> {
             }
         });
     });
-}
+}