Use rsync to copy pack contents by cognifloyd · Pull Request #415 · StackStorm/stackstorm-k8s · GitHub
Skip to content

Use rsync to copy pack contents - #415

Open
cognifloyd wants to merge 4 commits into
masterfrom
rsync
Open

cognifloyd wants to merge 4 commits into
masterfrom
rsync

Conversation

@cognifloyd

Copy link
Copy Markdown
Member

Closes #245

@cognifloyd cognifloyd added this to the v1.1.0 milestone Apr 12, 2024
@cognifloyd
cognifloyd requested a review from a team April 12, 2024 00:40
@cognifloyd cognifloyd self-assigned this Apr 12, 2024
@pull-request-size pull-request-size Bot added the size/M PR that changes 30-99 lines. Good size to review. label Apr 12, 2024

@cognifloyd cognifloyd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Either we do something like this, or we make the command configurable via values.

Maybe .utility.rsync = "rsync -rlp". That would mean the user would have to update this if they needed to switch to "cp". Or we keep both, but allow specifying the args.

utility:
  rsync: "rsync -a"
  cp: "cp -aR"
utility:
  rsync: "rsync -rlE"
  cp: "cp -dR --preserve=mode"

The gotcha with making it configurable is that people could forget to include recursive, symlinks, and preserving execute bits.

Comment thread templates/_helpers.tpl Outdated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

-a is equivalent to -rlptgoD: https://www.mankier.com/1/rsync#--archive

  • -r: copy directories recursively
  • -l: recreate symlinks in destination
  • -p: permissions
  • -t: modification times
  • -g: copy gid (group id) by name
  • -o: copy oid (owner id) by name
  • -D: devices (character and block) and specials (sockets, fifos, etc)

Of those, I'm confident we need -rlp or -rlE (where -E is short for --executability, a subset of -p). Depending on which user is running this, we might also want -og. The other parts of -a are probably not necessary.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I'll push a commit that switches to -rlptD based on rsync...Stealthii:stackstorm-k8s:rsync

Comment thread templates/_helpers.tpl Outdated
rsync -a /opt/stackstorm/packs/. /opt/stackstorm/packs-shared &&
rsync -a /opt/stackstorm/virtualenvs/. /opt/stackstorm/virtualenvs-shared;
else
/bin/cp -aR /opt/stackstorm/packs/. /opt/stackstorm/packs-shared &&

@cognifloyd cognifloyd Apr 12, 2024 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

-a is equivalent to -dR --preserve=all:

  • -d: is equivalent to -P --preserve=links
  • -P: no dereference / never follow symbolic links in SOURCE
  • -R: copy directories recursively
  • --preserve=all: preserve file attributes
    • mode
    • ownership for user and group
    • timestamps
    • links for hard links
    • context for security context
    • xattr for extended attributes

Of these we need -dR --preserve=mode (we need mode to maintain the execute bits of action files). Depending on which user is copying the file, we might also need --preserve=ownership.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I'll push a commit that switches to -RP --preserve=mode,timestamps,links,xattr based on rsync...Stealthii:stackstorm-k8s:rsync

@disastercurl

Copy link
Copy Markdown

In an attempt to solve the ownership issue that blocks #400, we should only preserve the file attributes that matter for this purpose.

I've updated a fork of this branch with proposed changes that once verified, could be brought into this PR.

rsync...Stealthii:stackstorm-k8s:rsync

cognifloyd and others added 3 commits May 9, 2024 16:39
This change aims to preserve file attributes when copying via rsync or
cp, without ownership to enable environments where containers are ran
without root privileges.

Co-authored-by: Daniel Porter <dporter@proofpoint.com>
@cognifloyd
cognifloyd marked this pull request as ready for review May 9, 2024 22:47
@cognifloyd
cognifloyd requested a review from winem December 11, 2024 16:25
@u1735067

u1735067 commented Mar 2, 2026

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature size/M PR that changes 30-99 lines. Good size to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make packs/virtualenv file copy more robust (use rsync w/ fallback to cp)

3 participants