Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set user agent for Azure/CLI action #122

Merged
merged 6 commits into from
Nov 27, 2023
Merged

Conversation

MoChilia
Copy link
Member

@MoChilia MoChilia commented Nov 8, 2023

This pr will configure the user agent for Azure/CLI action, allowing us to label the user agent as GITHUBACTIONS/AzureCLIAction for ARM. This will help us identify the CLI commands being executed within the Azure/CLI action.

@MoChilia MoChilia requested review from jiasli and evelyn-ys November 8, 2023 03:47
dist/index.js Outdated
Copy link
Member

@jiasli jiasli Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to update this file when committing to master branch? BTW, how is it generated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dist/index.js is the executable file of the action which is generated by running ncc build -C -m src/entrypoint.ts
If we don't update it in this pr, it will be auto generated by bot running in this workflow with a new commit when we push the changes to the master branch.

src/main.ts Outdated

export async function main() {
let usrAgentRepo = `${process.env.GITHUB_REPOSITORY}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is GITHUB_REPOSITORY sensitive? Can we log this in the telemetry?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jacekey23 @dcaro Could you help us to confirm this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jacekey23 @dcaro I have encoded the user repo with hash. Is it appropriate to store the information of user repo with hash?

@jiasli
Copy link
Member

jiasli commented Nov 22, 2023

Could you give an example of what the final User-Agent looks like?

@MoChilia
Copy link
Member Author

MoChilia commented Nov 22, 2023

Could you give an example of what the final User-Agent looks like?

If there is no AZURE_HTTP_USER_AGENT set before in the environment, we will set AZURE_HTTP_USER_AGENT like

GITHUBACTIONS/AzureCLIAction_3065f147ad3329c9480c20a58c1fc01e8a5b354d9233e27605224fb717484b2f

If AZURE_HTTP_USER_AGENT exists, it will look like

{my_azure_http_user_agent}+GITHUBACTIONS/AzureCLIAction_3065f147ad3329c9480c20a58c1fc01e8a5b354d9233e27605224fb717484b2f

And when running azure cli commands, azure cli core appends AZURE_HTTP_USER_AGENT set by azure/cli action to the User-Agent. See https://github.com/Azure/azure-cli/blob/b6ebd6fac6342c1112c2bb03927509d39263cdee/src/azure-cli-core/azure/cli/core/util.py#L898-L900.

Finally, we will see the user agent record in ARM like

AZURECLI/2.53.1 (DOCKER) azsdk-python-azure-mgmt-web/7.0.0 Python/3.10.13 (Linux-6.2.0-1015-azure-x86_64-with) GITHUBACTIONS/AzureCLIAction_3065f147ad3329c9480c20a58c1fc01e8a5b354d9233e27605224fb717484b2f

And if AZURE_HTTP_USER_AGENT has been set before, it will look like

AZURECLI/2.53.1 (DOCKER) azsdk-python-azure-mgmt-web/7.0.0 Python/3.10.13 (Linux-6.2.0-1015-azure-x86_64-with) {my_azure_http_user_agent}+GITHUBACTIONS/AzureCLIAction_3065f147ad3329c9480c20a58c1fc01e8a5b354d9233e27605224fb717484b2f

The plus sign in ${prefix}+ is literal, this pattern follows azure/login action: https://github.com/Azure/login/blob/e3b217c21b731d452b8d09e42a53fbbc6014b83c/src/main.ts#L13 and azure/powershell action: https://github.com/Azure/powershell/blob/cf0065bbca339333985d4281b0f249261221cd34/src/main.ts#L19.

@MoChilia MoChilia merged commit a4cc595 into Azure:master Nov 27, 2023
@MoChilia MoChilia deleted the setuseragent branch November 27, 2023 08:39
@@ -10,7 +10,7 @@ const cpExec = util.promisify(require('child_process').exec);
import { createScriptFile, TEMP_DIRECTORY, NullOutstreamStringWritable, deleteFile, getCurrentTime, checkIfEnvironmentVariableIsOmitted } from './utils';

const START_SCRIPT_EXECUTION_MARKER: string = "Starting script execution via docker image mcr.microsoft.com/azure-cli:";
const AZ_CLI_VERSION_DEFAULT_VALUE = 'agentazcliversion';
const AZ_CLI_VERSION_DEFAULT_VALUE = 'agentazcliversion'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the semicolon removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this line is not related this pr, I don't want to include this line in comparison to lead confusion.

@@ -83,6 +90,8 @@ export async function main() {
await deleteFile(scriptFilePath);
console.log("cleaning up container...");
await executeDockerCommand(["rm", "--force", CONTAINER_NAME], true);
// Reset AZURE_HTTP_USER_AGENT
core.exportVariable('AZURE_HTTP_USER_AGENT', prefix);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to L14, if AZURE_HTTP_USER_AGENT is not set, prefix will be set to an empty string "". This line will make this action modify AZURE_HTTP_USER_AGENT from unset to an empty string.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Will also consider this for login and powershell action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants