Skip to content

Restores the tar command to its original state#21

Merged
LucilleH merged 3 commits intomainfrom
lucille--check-cache-owner
Sep 22, 2023
Merged

Restores the tar command to its original state#21
LucilleH merged 3 commits intomainfrom
lucille--check-cache-owner

Conversation

@LucilleH
Copy link
Copy Markdown
Contributor

@LucilleH LucilleH commented Sep 20, 2023 •
edited
Loading

What

This PR tries to reduce side effects by:

  • Restores the tar command to its original state
  • Narrow the ~/.cache directory to just nix and devbox

Why

We are using chmod u+s "$(command -v tar)" to setuid on tar (This is needed because we are caching /nix/ and other directories outside of ~, which is owned by root)

This has a problem where it changes the parent directory of ~/.cache/nix to be owned by root instead of runner.

I thought #22 had fixed it, but we ran into similar issues when we use a go cache on top of the devbox cache.

Caveat

Cache enabled on self hosted Linux may stop working

@LucilleH LucilleH force-pushed the lucille--check-cache-owner branch 2 times, most recently from 4477bdb to 9d8041c Compare September 20, 2023 23:59
@LucilleH LucilleH mentioned this pull request Sep 21, 2023
Merged
github-merge-queue bot pushed a commit that referenced this pull request Sep 21, 2023
@LucilleH
Revert "Narrow the cache of xdg directory" (#22)
c1d7077 
Reverts #20

This results in the `~/.cache` directory gets created under `root`
instead of `runner`. Reverting fix this problem.

Verified the fix with
#21
@LucilleH LucilleH force-pushed the lucille--check-cache-owner branch 12 times, most recently from ab5a017 to efe662b Compare September 21, 2023 23:46
@LucilleH LucilleH marked this pull request as ready for review September 21, 2023 23:46
@LucilleH LucilleH changed the title check if local cache folder ownership has changed Override tar command on Linux to execute with sudo Sep 21, 2023
@LucilleH LucilleH force-pushed the lucille--check-cache-owner branch 4 times, most recently from 9e45375 to 3112d09 Compare September 21, 2023 23:56
@LucilleH LucilleH force-pushed the lucille--check-cache-owner branch 2 times, most recently from 53ea73b to 3d65e73 Compare September 22, 2023 00:18
@LucilleH LucilleH marked this pull request as draft September 22, 2023 00:39
@LucilleH LucilleH force-pushed the lucille--check-cache-owner branch 2 times, most recently from c3234f7 to d2b7cc5 Compare September 22, 2023 00:48
@LucilleH LucilleH force-pushed the lucille--check-cache-owner branch from d2b7cc5 to 5feb572 Compare September 22, 2023 00:50
@LucilleH LucilleH marked this pull request as ready for review September 22, 2023 00:55
@LucilleH LucilleH requested a review from gcurtis September 22, 2023 00:55
@LucilleH
Copy link
Copy Markdown
Contributor Author

LucilleH commented Sep 22, 2023

@gcurtis verified with https://github.com/jetpack-io/axiom/actions/runs/6268714473/job/17024088430?pr=3399

@LucilleH LucilleH changed the title Override tar command on Linux to execute with sudo Restores the tar command to its original state Sep 22, 2023
gcurtis
gcurtis approved these changes Sep 22, 2023
View reviewed changes
Copy link
Copy Markdown

@gcurtis gcurtis left a comment

Choose a reason for hiding this comment

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

Thanks!

@LucilleH LucilleH added this pull request to the merge queue Sep 22, 2023
Merged via the queue into main with commit f013611 Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gcurtis gcurtis gcurtis approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@LucilleH @gcurtis