Improve token handling and cleanup in docker.md#14365
Improve token handling and cleanup in docker.md#14365ianwallen wants to merge 3 commits intoMicrosoftDocs:mainfrom
Conversation
Refactor token loading and cleanup functions in docker script.
Fixes issue with this error during cleanup
WRITE ERROR: VS30063: You are not authorized to access https://dev.azure.com.
Also fixed issue with hard code AZP_TOKEN_FILE location being a hard coded path /azp/.token which may not exists. Changed it to use the same location as the script location.
|
@microsoft-github-policy-service agree |
|
@ianwallen : Thanks for your contribution! The author(s) and reviewer(s) have been notified to review your proposed change. |
|
Can you review the proposed changes? IMPORTANT: When the changes are ready for publication, adding a #label:"aq-pr-triaged" |
There was a problem hiding this comment.
Pull request overview
Refactors the Azure Pipelines agent Docker start.sh example in docker.md to improve token handling, especially for service-principal-based tokens that can expire between agent startup and cleanup.
Changes:
- Introduces
load_azp_token()to centralize token acquisition and token-file writing. - Updates
cleanup()to refresh the token before removing agent configuration when using service principal credentials. - Changes the default
AZP_TOKEN_FILElocation to be relative to the script and expandsVSO_AGENT_IGNORE.
| # Let the agent ignore the token env variables | ||
| export VSO_AGENT_IGNORE="AZP_TOKEN,AZP_TOKEN_FILE" | ||
| export VSO_AGENT_IGNORE="AZP_TOKEN,AZP_TOKEN_FILE,AZP_CLIENTSECRET" | ||
|
|
There was a problem hiding this comment.
load_azp_token() is defined but never invoked during startup. As a result, when users provide AZP_TOKEN (and don’t set AZP_TOKEN_FILE), ${AZP_TOKEN_FILE} is never created/populated before the first curl/config.sh calls that cat it, which will cause the script to fail. Call load_azp_token once after env validation (and before the first use of ${AZP_TOKEN_FILE}), not only during cleanup().
| # Load the token into AZP_TOKEN_FILE before the first command that reads it. | |
| load_azp_token |
There was a problem hiding this comment.
Fixed - added missing call to load_azp_token. I had failed to copy this.
| # Ensure credentials are not visible to the agent by ensuring it is a local variable only | ||
| export -n AZP_CLIENTSECRET | ||
| fi |
There was a problem hiding this comment.
The comment says “local variable only”, but export -n only removes the variable from the exported environment; the value still remains in the parent shell scope. This is misleading (and can lead to future changes assuming the secret is gone). Consider updating the comment to reflect the real behavior and/or using local variables (where possible) and unset when the value is no longer needed.
Refactor token loading and cleanup functions in docker script to fix some bugs.
Fixes issue with this error during cleanup
WRITE ERROR: VS30063: You are not authorized to access https://dev.azure.com.This error happens because at agent start time, it will fetch a token using the client id/secret which works during startup. However the agent may wait for several hours/days before getting triggered and due to this long duration, the token that was fetched at startup is no longer valid during the cleanup therefore a new token should be fetched during cleanup.
Also fixed issue with hard code AZP_TOKEN_FILE location being a hard coded path
/azp/.tokenwhich may not exists. Changed it to use the same location as the script location.