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

Make pre and post action cleanup (az account clear) optional for better performance on ephemeral runners #426

Closed
maskati opened this issue Mar 11, 2024 · 4 comments · Fixed by #484
Assignees
Labels
product enhancement New feature or request

Comments

@maskati
Copy link

maskati commented Mar 11, 2024

The pre and post action cleanup is somewhat time consuming, especially for otherwise short actions. It is also unnecessary in some contexts, for example when running on GitHub hosted runners or ephemeral self-hosted runners. I agree that the current is a good secure by default implementation, but there should ideally be an opt-out for use cases where it is known that the action will be running in a clean environment with no logged in context, allowing the user of the action to optimize for performance.

@maskati maskati added the need-to-triage Requires investigation label Mar 11, 2024
@maskati maskati changed the title Make pre-action cleanup (az account clear) optional for better performance on ephemeral runners Make pre and post action cleanup (az account clear) optional for better performance on ephemeral runners Mar 11, 2024
@MoChilia MoChilia added product enhancement New feature or request and removed need-to-triage Requires investigation labels Mar 12, 2024
@MoChilia
Copy link
Member

Hi @maskati, this makes sense. Performing pre/post cleanup for Azure/login on runners that are ephemeral for only one job is unnecessary. We should consider how to bypass such scenarios to save time for users.

@kardiojack
Copy link

Just wanted to provide support for this improvement. The clearing of credentials is taking over a minute for some of my actions. 60-90 seconds is not a huge deal in isolation, but many actions would take 10-20 seconds total if not for the Az Login pre and post slowdown...and these times add up for a busy repo with a merge queue and many actions run per day.

@vrdse
Copy link

vrdse commented Sep 9, 2024

In addition to the performance, the pre login cleanup also throws a warning and is skipped, if pwsh is not yet installed. Thus, I second the request. Though, parameter documentation should be clear about the security implications for non-ephemeral runners.

@YanaXu
Copy link
Collaborator

YanaXu commented Sep 18, 2024

Hi @maskati & @Storm-BE & @olafurnielsen & @vrdse & @JosiahSiegel& @lanni-energinet & @kardiojack & @baksetercx ,

we've released a new version v2.2.0. In this version, we removed the pre step and added 2 env variables to enable/disable pre or post cleanup.
v2 is aligned with v2.2.0 now.
Please try the new version. You can also refer to Enable/Disable the cleanup steps.

Just FYI, the time cost is mainly for login context initialization. After we remove pre step, the main step may need more time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product enhancement New feature or request
Projects
None yet
5 participants