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

tar needs a wrapper to add --no-same-owner when executing in an airlock context #365

Open
jeremymv2 opened this issue Apr 10, 2018 · 26 comments

Comments

@jeremymv2
Copy link
Contributor

jeremymv2 commented Apr 10, 2018

A simple tar extraction during pkg build fails on Builder and works locally.

Originally we had this:
tar -xzf lpeg-${lpeg_version}.tar.gz

Then we changed to this
tar -xzf lpeg-${lpeg_version}.tar.gz --owner=$(id -u) --group=$(id -g)

Regardless, the above two commands result in this error only on Builder:

--2018-04-10 00:12:53--  http://www.inf.puc-rio.br/~roberto/lpeg/lpeg-0.12.tar.gz
Resolving www.inf.puc-rio.br... 139.82.16.199
Connecting to www.inf.puc-rio.br|139.82.16.199|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 66649 (65K) [application/x-gzip]
Saving to: 'lpeg-0.12.tar.gz'

lpeg-0.12.tar.gz      0%[                    ]       0  --.-KB/s               
lpeg-0.12.tar.gz     33%[=====>              ]  21.79K   100KB/s               
lpeg-0.12.tar.gz     76%[==============>     ]  50.07K   115KB/s               
lpeg-0.12.tar.gz    100%[===================>]  65.09K   150KB/s    in 0.4s    
2018-04-10 00:12:54 (150 KB/s) - 'lpeg-0.12.tar.gz' saved [66649/66649]
tar: lpeg-0.12/makefile: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/HISTORY: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/test.lua: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/re.lua: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpeg.html: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/re.html: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpeg-128.gif: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lptypes.h: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpcap.h: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpcap.c: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpcode.h: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpcode.c: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpprint.h: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpprint.c: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lptree.h: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lptree.c: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpvm.h: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12/lpvm.c: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: lpeg-0.12: Cannot change ownership to uid 1000, gid 1000: Invalid argument
tar: Exiting with failure status due to previous errors
   openresty-noroot: Build time: 1m48s
   openresty-noroot: Exiting on error

Aha! Link: https://chef.aha.io/features/APPDL-124

@jeremymv2
Copy link
Contributor Author

Add this one to the list with the exact same problem: https://bldr.habitat.sh/#/pkgs/chef-server/oc_id/builds/959414338453241856

@tashimi
Copy link

tashimi commented Apr 18, 2018

From @irvingpop: On Builder, Plans that use tar fail with Cannot change ownership issues (ex: https://bldr.habitat.sh/#/pkgs/chef-server/oc_id/builds/963044754598322176 )

@baumanj baumanj self-assigned this Apr 18, 2018
@baumanj
Copy link
Contributor

baumanj commented Apr 19, 2018

I'm taking a look at this, will have more updates tomorrow

@irvingpop
Copy link

awesome @baumanj ! I invited you to the chef-server origin so you can poke around at the build logs

@rsertelon
Copy link
Contributor

We've got core/opam having this issue IIRC

@baumanj
Copy link
Contributor

baumanj commented Apr 19, 2018

That's very strange, but useful know, @rsertelon. Thanks for pointing it out!

@baumanj
Copy link
Contributor

baumanj commented Apr 19, 2018

The short version is that tar tries to take the ownership information from the archive when running as euid 0 (root). In the context of builder, the studio process (and tar) see itself as root, so it triggers that behavior and fails because there is no user:group of 1000:1000.

The reason --owner and --group didn't fix this issue is that they're only for setting these attributes on files when adding them to the archive. See man tar:

       --owner=NAME[:UID]
              Force NAME as owner for added files.  If UID is not supplied, NAME can be either a user name or numeric UID.  In this case the missing part (UID or name) will be inferred from the cur‐
              rent host's user database.

Instead, the fix is adding:

       --no-same-owner
              Extract files as yourself (default for ordinary users).

So, the command

tar -xzf lpeg-${lpeg_version}.tar.gz --no-same-owner

Should do the trick.

More details at habitat-sh/habitat#3972 and habitat-sh/core-plans#913. Fixing core/opam will be a little more involved since its tar call is not directly in the plan, so I'm keeping this issue open to track that.

Let me know if this fix doesn't resolve things for you, @irvingpop.

@rsertelon
Copy link
Contributor

@baumanj thanks! Although, for opam, don't worry, there's an issue for that in core-plans. I'll take care of it at some point. still hesitating between patching the tar commands or building hab packages for the deps that are being downloaded by the build :)

@baumanj
Copy link
Contributor

baumanj commented Apr 19, 2018

Thanks, @rsertelon! I commented on habitat-sh/core-plans#1294, but I want to keep this one open while we discuss if we want to try something to fix this in the general case. This seems the right place as opposed to core-plans to talk about that.

@baumanj
Copy link
Contributor

baumanj commented Apr 19, 2018

The general problem is that when tar is run with euid 0, it attempts to chown the extracted files according to the ownership defined by the archive. This won't work inside airlock on builder since the user isn't really root, but tar thinks it is. habitat-sh/habitat#3972 fixes the issue by adding --no-same-owner to disable this chowning behavior for the unpack_file helper which do_default_unpack uses.

However, there may be other uses of tar that don't go through the unpack_file helper, and may exist outside the plan file altogether. The issue with core/opam is one example. In that case, the tar command occurs in a Makefile for a sub-component.

While correcting individual plans in the core-plans repo to use unpack_file seems reasonable, dealing with all the potential ways tar could be called in the build processes orchestrated through Makefiles or other build systems seems less practical. We can't even search for potential cases because the places they occur may exist only in files that are downloaded during the course of the build.

It seems like to reliably avoid this error, we need to find a way to prevent tar from trying to do the chown when running in airlock. One way we could achieve this is by creating a wrapper for tar under /hab/bin that added --no-same-owner if it wasn't already present. Maybe there are other, better ways.

What do you think, @fnichol ?

@jeremymv2
Copy link
Contributor Author

jeremymv2 commented Apr 20, 2018

@baumanj I'll try --no-same-owner and see what happens.
Two ideas that come to mind when thinking about this scenario:

  • making changes which require a merge-to-master are an expensive way to investigate and apply fixes for Builder builds and overall would be a lot nicer if we could build from a branch
  • somehow making --no-same-owner a default of the Builder environment will make future traveler's experiences more delightful by avoiding this unique-to-Builder constraint altogether 😃 Or perhaps a tar wrapper simply outputs a WARNing indicating that the extract might fail since --no-same-owner isn't set.

@baumanj
Copy link
Contributor

baumanj commented Apr 20, 2018

I think this would work if we created a /hab/bin/tar wrapper upon studio creation similar to what we do for build:

#!/bin/bash

add_no_same_owner=true

for arg; do
    if [ "$arg" == "--no-same-owner" ]; then
        add_no_same_owner=false
        break
    fi
done

if $add_no_same_owner; then
    set -- --no-same-owner "$@"
fi

echo exec hab pkg exec core/tar "$@"

@baumanj
Copy link
Contributor

baumanj commented Apr 20, 2018

@jeremymv2: I agree that we should address the tar issue proactively rather than patching it in all the places that it occurs. I had written the above snippet yesterday, but forgot to actually post it. Thanks for the reminder.

As for doing builder builds on non-master branches, I'll let that discussion play out in the filed issue, but I think there are a couple things you could do in the meantime that may reduce your pain:

  1. Local studio builds should generate exactly the same .harts that builder does. You can do those on whatever git branch you like and then they can be uploaded to builder manually with hab pkg upload if you want to share them with others through the origin. The downside being that unless that code is committed to master, it will be replaced by a subsequent builder build.
  2. You can fork the git repo and connect the fork's plan on a separate origin. Then you can develop on master of the fork to get the normal builder behavior and merge back to the repo you forked from when done.

jeremymv2 added a commit to jeremymv2/chef-server that referenced this issue Apr 21, 2018
…s like builder

Attempts to fix Builder builds currently failing because they occur in a privilege restricted environment and cannot change owner or group based on the archive metadata during a tar extract.

This should introduce a new behavior to the tar command where owner/group archive metadata is ignored.

Additionally, it adds `core/libxml2` and `core/libxslt` to the build deps of `oc_id` so that we can then pass custom paths during the `nokogiri` gem build.

Otherwise, when we bundle install, `nokogiri` fails to build because of a `tar` extraction failure of libxml like [this](https://bldr.habitat.sh/#/pkgs/chef-server/oc_id/builds/959414338453241856)

Further Discussion on the `tar` extraction issue: habitat-sh/builder#365

Signed-off-by: Jeremy J. Miller <jm@chef.io>
@rsertelon
Copy link
Contributor

rsertelon commented Apr 24, 2018

core/jetty fails with the same error: https://bldr.habitat.sh/#/pkgs/core/jetty/builds/967456856196702208

@baumanj
Copy link
Contributor

baumanj commented Apr 30, 2018

@jeremymv2: did this resolve your issue?

I want to repurpose this issue to the more general underlying cause, but before I do I want to ensure you're unblocked.

@jeremymv2
Copy link
Contributor Author

@baumanj Where I had declared a tar extraction in plan.sh - I just quickly added --no-same-owner and it fixed the issue.

However, I have another pkg build failing because there is a tar extract buried under a make - which I can't control.

I'm not currently blocked by this though since I've migrated the pkg builds to expeditor until this gets sorted out.

@baumanj
Copy link
Contributor

baumanj commented May 2, 2018

The tar buried under a make is exactly the sort of situation I think this issue should resolve. Thanks, Jeremy.

@baumanj baumanj changed the title tar: Cannot change ownership to uid 1000, gid 1000: Invalid argument tar needs a wrapper to add --no-same-owner when executing in an airlock context May 2, 2018
@baumanj baumanj removed their assignment May 2, 2018
@rsertelon
Copy link
Contributor

@baumanj is there any simple way for me to provide a PR with the script you wrote above? :) I think it would be really cool to finally fix this ^^

@baumanj
Copy link
Contributor

baumanj commented Nov 7, 2018

I would really like to get @fnichol's input on the approach since airlock and the guts of the studio are his magical realm. I'll remind him to in the next day or two if he misses this comment.

Is there anything new where this is tripping you up that makes a good case for fixing this now?

@rsertelon
Copy link
Contributor

Still the same blocked things in core plans actually. They've been there for a while now.

@fnichol
Copy link
Collaborator

fnichol commented Nov 8, 2018

We currently run all unpack_file() calls (used by do_unpack()) with a --no-same-owner in the default build program and I went with this approach as it would be the default behavior for non-root users. There is a --same-owner flag for tar but I'm not sure what the equivalent of running tar with --no-same-owner --same-owner would be as root and non-root. I'm personally nervous about in effect changing the default behavior of tar for all Habitat consuming software as this feels close to a defaults-changing upstream patch. That may be the tack we'll have to take in the end, however there might be some alternatives to explore, remembering that we're solving a use case of (I'm assuming) some less-than-trivial build invocation (so far I've only seen third party Plans in the core origin as examples) which we tend to override/alter/patch when we need to bend that software to the requirements of the Habitat build and link process (using sed, patch, the "sed-patch", altering command invocation with environment variables. Perhaps some of these ideas below are also worth considering?

  • Provide an alternate wrapper, such as bin/tar-no-same-owner which could be used when such behavior is desired.
  • Provide the same alternate wrapper, but in a location in the core/tar package not presently on $PATH like alt/bin/tar which could be added to a command's $PATH before running like env PATH="$(pkg_path_for core/tar)/alt/bin:$PATH" make.
  • Provide an alternate package which depends on core/tar and wraps bin/tar, for example a package like core/tar-no-same-owner.

@stale
Copy link

stale bot commented Apr 2, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

@swaroopbalan
Copy link

swaroopbalan commented May 22, 2020

this issue is not resolved for me
im getting an error

tar: --no-same-owner: Cannot open: No such file or directory
tar: Error is not recoverable: exiting now
scripts/Makefile.package:87: recipe for target 'intdeb-pkg' failed
make[4]: *** [intdeb-pkg] Error 2
Makefile:1430: recipe for target 'intdeb-pkg' failed

code change was
(cd $srctree; tar -c -f - -T -) < "$objtree/debian/hdrsrcfiles" | (cd $destdir; tar -xf --no-same-owner -)

@stale
Copy link

stale bot commented May 23, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

@stale
Copy link

stale bot commented Jun 11, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

@stale
Copy link

stale bot commented Sep 18, 2023

This issue has been automatically closed after being stale for 400 days. We still value your input and contribution. Please re-open the issue if desired and leave a comment with details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants