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

Fix handling of user set on the task level #193

Merged
merged 4 commits into from
Apr 22, 2024
Merged

Fix handling of user set on the task level #193

merged 4 commits into from
Apr 22, 2024

Conversation

umputun
Copy link
Owner

@umputun umputun commented Apr 22, 2024

fix #192

Currently support of user set on the task level is partial and confusing. The user is set on the playbook level, but forced to the top-level (playbook's) user if not defined. This is not a good idea, as this way we can't properly use it because it doesn't handle inventory-level user. In fact, the code didn't even use it to pick the user for remote session, but rather used it for SPOT_REMOTE_USER resolution.

The change eliminates setting the default task.User on the config level,
and moves this responsibility to the runner. Now the runner explicitly
checks the presence of task.User and passes it in.

In addition, runTaskOnHost sets the passed user into activeTask
(a copy of the task used later on). This was done to be able to know what
the actual user is. Currently, the only use is in setting SPOT_REMOTE_USER,
however, it can also be used in any place that needs to know the active remote user.
@coveralls
Copy link

coveralls commented Apr 22, 2024

Pull Request Test Coverage Report for Build 8789060733

Details

  • 8 of 9 (88.89%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 83.937%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/runner/runner.go 8 9 88.89%
Totals Coverage Status
Change from base Build 8757053566: 0.03%
Covered Lines: 2712
Relevant Lines: 3231

💛 - Coveralls

Copy link

cloudflare-pages bot commented Apr 22, 2024

Deploying spot-site with  Cloudflare Pages  Cloudflare Pages

Latest commit: ac11d63
Status: ✅  Deploy successful!
Preview URL: https://d02ffa87.spot-7ki.pages.dev
Branch Preview URL: https://task-user-fix.spot-7ki.pages.dev

View logs

@umputun umputun merged commit ad58ee5 into master Apr 22, 2024
4 checks passed
@umputun umputun deleted the task-user-fix branch April 22, 2024 18:36
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.

Task level user setting looks not working, docs mistake
2 participants