From 890663df00cef2f8e22c648b0f06292f022d7cbe Mon Sep 17 00:00:00 2001 From: Balaga Gayatri Date: Wed, 27 Apr 2022 13:52:12 +0530 Subject: [PATCH 1/9] added handling warning --- lib/main.js | 15 ++++++++++----- src/main.ts | 15 ++++++++++----- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/main.js b/lib/main.js index 42ab22d5b..8bf04697a 100644 --- a/lib/main.js +++ b/lib/main.js @@ -40,22 +40,27 @@ function main() { return __awaiter(this, void 0, void 0, function* () { try { //Options for error handling - let commandStdErr = false; + // let commandStdErr = false; const loginOptions = { silent: true, ignoreReturnCode: true, - failOnStdErr: true, + // failOnStdErr: true, listeners: { stderr: (data) => { let error = data.toString(); + let isWarning = error.toLowerCase().startsWith('warning'); + // logging WARNING + if (isWarning) { + core.warning(error); + } //removing the keyword 'ERROR' to avoid duplicates while throwing error if (error.toLowerCase().startsWith('error')) { error = error.slice(5); } // printing error - if (error && error.trim().length !== 0) { - commandStdErr = true; - core.error(error); + if (error && error.trim().length !== 0 && !isWarning) { + // commandStdErr = true; + throw error; } } } diff --git a/src/main.ts b/src/main.ts index 616b3ddc1..66a5a49bf 100644 --- a/src/main.ts +++ b/src/main.ts @@ -12,22 +12,27 @@ var azPSHostEnv = !!process.env.AZUREPS_HOST_ENVIRONMENT ? `${process.env.AZUREP async function main() { try { //Options for error handling - let commandStdErr = false; + // let commandStdErr = false; const loginOptions: ExecOptions = { silent: true, ignoreReturnCode: true, - failOnStdErr: true, + // failOnStdErr: true, listeners: { stderr: (data: Buffer) => { let error = data.toString(); + let isWarning = error.toLowerCase().startsWith('warning') + // logging WARNING + if(isWarning) { + core.warning(error); + } //removing the keyword 'ERROR' to avoid duplicates while throwing error if (error.toLowerCase().startsWith('error')) { error = error.slice(5); } // printing error - if (error && error.trim().length !== 0) { - commandStdErr = true; - core.error(error); + if (error && error.trim().length !== 0 && !isWarning) { + // commandStdErr = true; + throw error; } } } From c86e11950c44ce6d608151739b69d6989ec0c8fd Mon Sep 17 00:00:00 2001 From: Balaga Gayatri Date: Wed, 27 Apr 2022 15:25:15 +0530 Subject: [PATCH 2/9] changes --- lib/main.js | 2 +- src/main.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/main.js b/lib/main.js index 8bf04697a..8f0f040f3 100644 --- a/lib/main.js +++ b/lib/main.js @@ -60,7 +60,7 @@ function main() { // printing error if (error && error.trim().length !== 0 && !isWarning) { // commandStdErr = true; - throw error; + core.setFailed(error); } } } diff --git a/src/main.ts b/src/main.ts index 66a5a49bf..4b536dbd6 100644 --- a/src/main.ts +++ b/src/main.ts @@ -32,7 +32,7 @@ async function main() { // printing error if (error && error.trim().length !== 0 && !isWarning) { // commandStdErr = true; - throw error; + core.setFailed(error); } } } From 571a1dba5495344bf2680443c5d2d03009229779 Mon Sep 17 00:00:00 2001 From: Balaga Gayatri Date: Wed, 27 Apr 2022 15:31:11 +0530 Subject: [PATCH 3/9] changes --- lib/main.js | 2 +- src/main.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/main.js b/lib/main.js index 8f0f040f3..a39231049 100644 --- a/lib/main.js +++ b/lib/main.js @@ -60,7 +60,7 @@ function main() { // printing error if (error && error.trim().length !== 0 && !isWarning) { // commandStdErr = true; - core.setFailed(error); + core.error(error); } } } diff --git a/src/main.ts b/src/main.ts index 4b536dbd6..20add3b61 100644 --- a/src/main.ts +++ b/src/main.ts @@ -32,7 +32,7 @@ async function main() { // printing error if (error && error.trim().length !== 0 && !isWarning) { // commandStdErr = true; - core.setFailed(error); + core.error(error); } } } From 11a73ff53bf15eb7d8f8b5cfb201cc4f10c2ee38 Mon Sep 17 00:00:00 2001 From: Balaga Gayatri Date: Wed, 27 Apr 2022 15:37:06 +0530 Subject: [PATCH 4/9] ignorereturn code disabled --- lib/main.js | 2 +- src/main.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/main.js b/lib/main.js index a39231049..25080af85 100644 --- a/lib/main.js +++ b/lib/main.js @@ -43,7 +43,7 @@ function main() { // let commandStdErr = false; const loginOptions = { silent: true, - ignoreReturnCode: true, + // ignoreReturnCode: true, // failOnStdErr: true, listeners: { stderr: (data) => { diff --git a/src/main.ts b/src/main.ts index 20add3b61..3a1b2e633 100644 --- a/src/main.ts +++ b/src/main.ts @@ -15,7 +15,7 @@ async function main() { // let commandStdErr = false; const loginOptions: ExecOptions = { silent: true, - ignoreReturnCode: true, + // ignoreReturnCode: true, // failOnStdErr: true, listeners: { stderr: (data: Buffer) => { From d8a0fd9c9d8f308212ae3b16cfd2284dafd12154 Mon Sep 17 00:00:00 2001 From: Balaga Gayatri Date: Thu, 28 Apr 2022 13:03:35 +0530 Subject: [PATCH 5/9] setting failed for error --- lib/main.js | 2 +- src/main.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/main.js b/lib/main.js index 25080af85..3262d93c5 100644 --- a/lib/main.js +++ b/lib/main.js @@ -60,7 +60,7 @@ function main() { // printing error if (error && error.trim().length !== 0 && !isWarning) { // commandStdErr = true; - core.error(error); + core.setFailed(error); } } } diff --git a/src/main.ts b/src/main.ts index 3a1b2e633..870bab064 100644 --- a/src/main.ts +++ b/src/main.ts @@ -32,7 +32,7 @@ async function main() { // printing error if (error && error.trim().length !== 0 && !isWarning) { // commandStdErr = true; - core.error(error); + core.setFailed(error); } } } From 0c87ae202a07db9097fe10ae2352c0e77fdd5f24 Mon Sep 17 00:00:00 2001 From: Balaga Gayatri Date: Mon, 2 May 2022 12:08:03 +0530 Subject: [PATCH 6/9] silencing warnings --- lib/main.js | 20 +++++++------------- src/main.ts | 20 +++++++------------- 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/lib/main.js b/lib/main.js index 3262d93c5..ed4e42984 100644 --- a/lib/main.js +++ b/lib/main.js @@ -40,26 +40,19 @@ function main() { return __awaiter(this, void 0, void 0, function* () { try { //Options for error handling - // let commandStdErr = false; const loginOptions = { silent: true, - // ignoreReturnCode: true, - // failOnStdErr: true, listeners: { stderr: (data) => { let error = data.toString(); let isWarning = error.toLowerCase().startsWith('warning'); - // logging WARNING - if (isWarning) { - core.warning(error); - } - //removing the keyword 'ERROR' to avoid duplicates while throwing error - if (error.toLowerCase().startsWith('error')) { - error = error.slice(5); - } - // printing error + let isError = error.toLowerCase().startsWith('error'); + // printing ERROR if (error && error.trim().length !== 0 && !isWarning) { - // commandStdErr = true; + if (isError) { + //removing the keyword 'ERROR' to avoid duplicates while throwing error + error = error.slice(5); + } core.setFailed(error); } } @@ -195,6 +188,7 @@ function main() { commonArgs = commonArgs.concat("--federated-token", federatedToken); } else { + console.log("Note: Azure/login action also supports OIDC login mechanism. Refer https://github.com/azure/login#configure-a-service-principal-with-a-federated-credential-to-use-oidc-based-authentication for more details."); commonArgs = commonArgs.concat("-p", servicePrincipalKey); } yield executeAzCliCommand(`login`, true, loginOptions, commonArgs); diff --git a/src/main.ts b/src/main.ts index 870bab064..5a0094bb7 100644 --- a/src/main.ts +++ b/src/main.ts @@ -12,26 +12,19 @@ var azPSHostEnv = !!process.env.AZUREPS_HOST_ENVIRONMENT ? `${process.env.AZUREP async function main() { try { //Options for error handling - // let commandStdErr = false; const loginOptions: ExecOptions = { silent: true, - // ignoreReturnCode: true, - // failOnStdErr: true, listeners: { stderr: (data: Buffer) => { let error = data.toString(); let isWarning = error.toLowerCase().startsWith('warning') - // logging WARNING - if(isWarning) { - core.warning(error); - } - //removing the keyword 'ERROR' to avoid duplicates while throwing error - if (error.toLowerCase().startsWith('error')) { - error = error.slice(5); - } - // printing error + let isError = error.toLowerCase().startsWith('error') + // printing ERROR if (error && error.trim().length !== 0 && !isWarning) { - // commandStdErr = true; + if(isError) { + //removing the keyword 'ERROR' to avoid duplicates while throwing error + error = error.slice(5); + } core.setFailed(error); } } @@ -179,6 +172,7 @@ async function main() { commonArgs = commonArgs.concat("--federated-token", federatedToken); } else { + console.log("Note: Azure/login action also supports OIDC login mechanism. Refer https://github.com/azure/login#configure-a-service-principal-with-a-federated-credential-to-use-oidc-based-authentication for more details.") commonArgs = commonArgs.concat("-p", servicePrincipalKey); } await executeAzCliCommand(`login`, true, loginOptions, commonArgs); From 53733dac92655991d39393057ec884294aaaac47 Mon Sep 17 00:00:00 2001 From: Balaga Gayatri Date: Mon, 2 May 2022 12:30:58 +0530 Subject: [PATCH 7/9] :nit picks --- src/main.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.ts b/src/main.ts index 5a0094bb7..866cb3f37 100644 --- a/src/main.ts +++ b/src/main.ts @@ -17,8 +17,8 @@ async function main() { listeners: { stderr: (data: Buffer) => { let error = data.toString(); - let isWarning = error.toLowerCase().startsWith('warning') - let isError = error.toLowerCase().startsWith('error') + let isWarning = error.toLowerCase().startsWith('warning'); + let isError = error.toLowerCase().startsWith('error'); // printing ERROR if (error && error.trim().length !== 0 && !isWarning) { if(isError) { From 034eb62ad0060204bd58c72a172da7da133db570 Mon Sep 17 00:00:00 2001 From: Balaga Gayatri Date: Mon, 2 May 2022 15:32:24 +0530 Subject: [PATCH 8/9] Minor doc changes --- README.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b50cda692..80adc3504 100644 --- a/README.md +++ b/README.md @@ -207,11 +207,13 @@ Follow the steps to configure Azure Service Principal with a secret: # The command should output a JSON object similar to this: + { "clientId": "", - "clientSecret": "", + "clientSecret": "", "subscriptionId": "", "tenantId": "", + "resourceManagerEndpointUrl": "" (...) } @@ -219,6 +221,10 @@ Follow the steps to configure Azure Service Principal with a secret: * Now in the workflow file in your branch: `.github/workflows/workflow.yml` replace the secret in Azure login action with your secret (Refer to the example above) * Note: The above `az ad sp create-for-rbac` command will give you the `--sdk-auth` deprecation warning. As we are working with CLI for this deprecation process, we strongly recommend users to use this `--sdk-auth` flag as the result dictionary output changes and not accepted by login action if `--sdk-auth` is not used. +### Manually creating the Credentials object + +If you already created and assigned a Service Principal in Azure you can manually create the .json object above by finding the `clientId` and `clientSecret` on the Service Principal, and your `subscriptionId` and `tenantId` of the subscription and tenant respectively. The `resourceManagerEndpointUrl` will be `https://management.azure.com/` if you are using the public Azure cloud. + ### Configure a service principal with a Federated Credential to use OIDC based authentication: @@ -234,7 +240,7 @@ You can add federated credentials in the Azure portal or with the Microsoft Grap 7. For **Entity type**, select **Environment**, **Branch**, **Pull request**, or **Tag** and specify the value, based on how you have configured the trigger for your GitHub workflow. For a more detailed overview, see [GitHub OIDC guidance]( https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect#defining-[…]dc-claims). 8. Add a **Name** for the federated credential. 9. Click **Add** to configure the federated credential. -10. Make sure the above created application has the `contributor` access to the provided subscription. +10. Make sure the above created application has the `contributor` access to the provided subscription. Visit [role-based-access-control](https://docs.microsoft.com/en-us/azure/role-based-access-control/role-assignments-portal?tabs=current#prerequisites) for more details. For a more detailed overview, see more guidance around [Azure Federated Credentials](https://docs.microsoft.com/en-us/azure/active-directory/develop/workload-identity-federation-create-trust-github). From c514f3334434c1e08391924ef331624659cd1e2d Mon Sep 17 00:00:00 2001 From: Balaga Gayatri Date: Mon, 2 May 2022 19:16:05 +0530 Subject: [PATCH 9/9] Implementing NIT suggestions --- lib/main.js | 8 ++++---- src/main.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/main.js b/lib/main.js index ed4e42984..c2ed27af9 100644 --- a/lib/main.js +++ b/lib/main.js @@ -45,11 +45,11 @@ function main() { listeners: { stderr: (data) => { let error = data.toString(); - let isWarning = error.toLowerCase().startsWith('warning'); - let isError = error.toLowerCase().startsWith('error'); + let startsWithWarning = error.toLowerCase().startsWith('warning'); + let startsWithError = error.toLowerCase().startsWith('error'); // printing ERROR - if (error && error.trim().length !== 0 && !isWarning) { - if (isError) { + if (error && error.trim().length !== 0 && !startsWithWarning) { + if (startsWithError) { //removing the keyword 'ERROR' to avoid duplicates while throwing error error = error.slice(5); } diff --git a/src/main.ts b/src/main.ts index 866cb3f37..49473977c 100644 --- a/src/main.ts +++ b/src/main.ts @@ -17,11 +17,11 @@ async function main() { listeners: { stderr: (data: Buffer) => { let error = data.toString(); - let isWarning = error.toLowerCase().startsWith('warning'); - let isError = error.toLowerCase().startsWith('error'); + let startsWithWarning = error.toLowerCase().startsWith('warning'); + let startsWithError = error.toLowerCase().startsWith('error'); // printing ERROR - if (error && error.trim().length !== 0 && !isWarning) { - if(isError) { + if (error && error.trim().length !== 0 && !startsWithWarning) { + if(startsWithError) { //removing the keyword 'ERROR' to avoid duplicates while throwing error error = error.slice(5); }