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

feat: Allow running container as non-root UID/GID for ownership issues (docker) #433

Merged
merged 29 commits into from
Sep 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
38f2a48
Add read permissions to tmpfile in wrapper hook
tofupup Sep 1, 2022
4f3d238
Update docs for running docker with user
tofupup Sep 1, 2022
cd766e1
Add initial docker entrypoint script
tofupup Sep 2, 2022
c3a6529
Add entrypoint user creation and permission checks
tofupup Sep 2, 2022
78fdb40
Add entrypoint.sh to trigger github image test
tofupup Sep 2, 2022
f7871ca
Add entrypoint container structure tests
tofupup Sep 2, 2022
1ac51f8
Fix whitespace in entrypoint.sh
tofupup Sep 2, 2022
64ccfa1
Update docker documentation in README
tofupup Sep 2, 2022
9266a93
Clean up pip cache from skeleton files
tofupup Sep 2, 2022
0b4d72a
Add su-exec container structure test
tofupup Sep 3, 2022
281c53e
Use /root for new user homedir skeleton
tofupup Sep 3, 2022
dc3f996
Merge remote-tracking branch 'upstream/master' into fix-wrapper-permi…
tofupup Sep 4, 2022
a019428
Clean up Dockerfile apk installs
tofupup Sep 5, 2022
0988930
Clean up root su-exec
tofupup Sep 5, 2022
5ad0a7d
Clean up variable references
tofupup Sep 5, 2022
367f0a4
Create function for error reporting
tofupup Sep 5, 2022
ed40055
Remove extraneous braces from variables
tofupup Sep 5, 2022
6b3f6a9
Add check to insure container is running as root
tofupup Sep 5, 2022
12d8526
Correct initial UID check
tofupup Sep 6, 2022
c5e4d01
Split long gid error into multiline
tofupup Sep 6, 2022
070b8a2
Split long uid failure error message
tofupup Sep 6, 2022
ea6f65f
Split long id -G error message
tofupup Sep 6, 2022
87541f4
Split long addgroup error message
tofupup Sep 6, 2022
ab15d72
Fix bad pattern match for group info
tofupup Sep 6, 2022
527a521
Separate error message array from string
tofupup Sep 6, 2022
8df4f20
Fix entrypoint container test with new error msg
tofupup Sep 6, 2022
44263d0
Pin su-exec more strictly
MaxymVlasov Sep 7, 2022
c16e2f4
Reorder docs and fix minor issues
MaxymVlasov Sep 7, 2022
4064ba1
Update README.md
antonbabenko Sep 7, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
*
!.dockerignore
!Dockerfile
!tools/entrypoint.sh
13 changes: 13 additions & 0 deletions .github/.container-structure-test-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,19 @@ commandTests:
args: [ "version" ]
expectedOutput: [ "([0-9]+\\.){2}[0-9]+\\n$" ]

- name: "entrypoint.sh"
envVars:
- key: "USERID"
value: "1000:1000"
command: "/entrypoint.sh"
args: [ "-V" ]
expectedError: ["^ERROR: uid:gid 1000:1000 lacks permissions to //\\n$"]
exitCode: 1

- name: "su-exec"
command: "su-exec"
expectedOutput: ["^Usage: su-exec user-spec command \\[args\\]\\n$"]

fileExistenceTests:
- name: 'terrascan init'
path: '/root/.terrascan/pkg/policies/opa/rego/github/github_repository/privateRepoEnabled.rego'
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/build-image-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ jobs:
with:
files: |
Dockerfile
.dockerignore
tools/entrypoint.sh

- name: Build if Dockerfile changed
if: steps.changed-files-specific.outputs.any_changed == 'true'
Expand Down
11 changes: 7 additions & 4 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ WORKDIR /bin_dir

RUN apk add --no-cache \
# Builder deps
curl=~7 \
unzip=~6 && \
curl=~7 && \
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
# Upgrade pip for be able get latest Checkov
python3 -m pip install --no-cache-dir --upgrade pip

Expand Down Expand Up @@ -177,7 +176,9 @@ RUN apk add --no-cache \
bash=~5 \
# pre-commit-hooks deps: https://github.com/pre-commit/pre-commit-hooks
musl-dev=~1 \
gcc=~10
gcc=~10 \
# entrypoint wrapper deps
su-exec=~0.2

# Copy tools
COPY --from=builder \
Expand All @@ -203,9 +204,11 @@ RUN if [ "$(grep -o '^terraform-docs SKIPPED$' /usr/bin/tools_versions_info)" =
# unsafe repository ('/lint' is owned by someone else)
git config --global --add safe.directory /lint

COPY tools/entrypoint.sh /entrypoint.sh

ENV PRE_COMMIT_COLOR=${PRE_COMMIT_COLOR:-always}

ENV INFRACOST_API_KEY=${INFRACOST_API_KEY:-}
ENV INFRACOST_SKIP_UPDATE_CHECK=${INFRACOST_SKIP_UPDATE_CHECK:-false}

ENTRYPOINT [ "pre-commit" ]
ENTRYPOINT [ "/entrypoint.sh" ]
37 changes: 35 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ If you are using `pre-commit-terraform` already or want to support its developme
* [terraform_wrapper_module_for_each](#terraform_wrapper_module_for_each)
* [terrascan](#terrascan)
* [tfupdate](#tfupdate)
* [Docker Usage: File Permissions](#docker-usage-file-permissions)
* [Authors](#authors)
* [License](#license)
* [Additional information for users from Russia and Belarus](#additional-information-for-users-from-russia-and-belarus)
Expand Down Expand Up @@ -227,16 +228,18 @@ pre-commit run -a

Or, using Docker ([available tags](https://github.com/antonbabenko/pre-commit-terraform/pkgs/container/pre-commit-terraform/versions)):

**NOTE:** This command uses your user id and group id for the docker container to use to access the local files. If the files are owned by another user, update the `USERID` environment variable. See [File Permissions section](#docker-usage-file-permissions) for more information.

```bash
TAG=latest
docker run -v $(pwd):/lint -w /lint ghcr.io/antonbabenko/pre-commit-terraform:$TAG run -a
docker run -e "USERID=$(id -u):$(id -g)" -v $(pwd):/lint -w /lint ghcr.io/antonbabenko/pre-commit-terraform:$TAG run -a
```

Execute this command to list the versions of the tools in Docker:

```bash
TAG=latest
docker run --entrypoint cat ghcr.io/antonbabenko/pre-commit-terraform:$TAG /usr/bin/tools_versions_info
docker run --rm --entrypoint cat ghcr.io/antonbabenko/pre-commit-terraform:$TAG /usr/bin/tools_versions_info
```

## Available Hooks
Expand Down Expand Up @@ -735,6 +738,16 @@ Sample configuration:
- --args=--verbose # Verbose output
```

**If you use hook inside Docker:**
The `terraform_wrapper_module_for_each` hook attempts to determine the module's short name to be inserted into the generated `README.md` files for the `source` URLs. Since the container uses a bind mount at a static location, it can cause this short name to be incorrect.
If the generated name is incorrect, set them by providing the `module-repo-shortname` option to the hook:

```yaml
- id: terraform_wrapper_module_for_each
args:
- '--args=--module-repo-shortname=ec2-instance'
```

### terrascan

1. `terrascan` supports custom arguments so you can pass supported flags like `--non-recursive` and `--policy-type` to disable recursive inspection and set the policy type respectively:
Expand Down Expand Up @@ -779,6 +792,26 @@ Sample configuration:
Check [`tfupdate` usage instructions](https://github.com/minamijoyo/tfupdate#usage) for other available options and usage examples.
No need to pass `--recursive .` as it is added automatically.

## Docker Usage: File Permissions

A mismatch between the Docker container's user and the local repository file ownership can cause permission issues in the repository where `pre-commit` is run. The container runs as the `root` user by default, and uses a `tools/entrypoint.sh` script to assume a user ID and group ID if specified by the environment variable `USERID`.

The [recommended command](#4-run) to run the Docker container is:

```bash
TAG=latest
docker run -e "USERID=$(id -u):$(id -g)" -v $(pwd):/lint -w /lint ghcr.io/antonbabenko/pre-commit-terraform:$TAG run -a
```

which uses your current session's user ID and group ID to set the variable in the run command. Without this setting, you may find files and directories owned by `root` in your local repository.

If the local repository is using a different user or group for permissions, you can modify the `USERID` to the user ID and group ID needed. **Do not use the username or groupname in the environment variable, as it has no meaning in the container.** You can get the current directory's owner user ID and group ID from the 3rd (user) and 4th (group) columns in `ls` output:

```bash
$ ls -aldn .
drwxr-xr-x 9 1000 1000 4096 Sep 1 16:23 .
```

## Authors

This repository is managed by [Anton Babenko](https://github.com/antonbabenko) with help from these awesome contributors:
Expand Down
3 changes: 3 additions & 0 deletions hooks/terraform_wrapper_module_for_each.sh
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,9 @@ function create_tmp_file_tf {
mv "$tmp_file" "$tmp_file.tf"
tmp_file_tf="$tmp_file.tf"

# mktemp creates with no group/other read permissions
chmod a+r "$tmp_file_tf"

echo "$CONTENT_MAIN_TF" > "$tmp_file_tf"
}

Expand Down
82 changes: 82 additions & 0 deletions tools/entrypoint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
#!/usr/bin/env bash
#exit on error
set -e

readonly USERBASE="run"
readonly BASHPATH="/bin/bash"
readonly HOMEPATH="/home"

function echo_error_and_exit {
echo -e "ERROR: " "$@" >&2
exit 1
}

# make sure entrypoint is running as root
if [[ $(id -u) -ne 0 ]]; then
echo_error_and_exit "Container must run as root. Use environment variable USERID to set user.\n" \
"Example: \"TAG=latest && " \
"docker run -e USERID=$(id -u):$(id -g) -v $(pwd):/lint -w /lint ghcr.io/antonbabenko/pre-commit-terraform:$TAG run -a\""
fi

# make sure USERID makes sense as UID:GID
# it looks like the alpine distro limits UID and GID to 256000, but
# could be more, so we accept any valid integers
USERID=${USERID:-"0:0"}
if [[ ! $USERID =~ ^[0-9]+:[0-9]+$ ]]; then
echo_error_and_exit "USERID environment variable invalid, format is userid:groupid. Received: \"$USERID\""
fi

# separate uid and gid
uid=${USERID%%:*}
gid=${USERID##*:}

# if requested UID:GID is root, go ahead and run without other processing
[[ $USERID == "0:0" ]] && exec su-exec "$USERID" pre-commit "$@"

# make sure workdir and some files are readable/writable by the provided UID/GID
# combo, otherwise will have errors when processing hooks
wdir="$(pwd)"
if ! su-exec "$USERID" "$BASHPATH" -c "test -w $wdir && test -r $wdir"; then
echo_error_and_exit "uid:gid $USERID lacks permissions to $wdir/"
fi
wdirgitindex="$wdir/.git/index"
if ! su-exec "$USERID" "$BASHPATH" -c "test -w $wdirgitindex && test -r $wdirgitindex"; then
echo_error_and_exit "uid:gid $USERID cannot write to $wdirgitindex"
fi

# check if group by this GID already exists, if so get the name since adduser
# only accepts names
if groupinfo="$(getent group "$gid")"; then
groupname="${groupinfo%%:*}"
else
# create group in advance in case GID is different than UID
groupname="$USERBASE$gid"
if ! err="$(addgroup -g "$gid" "$groupname" 2>&1)"; then
echo_error_and_exit "failed to create gid \"$gid\" with name \"$groupname\"\ncommand output: \"$err\""
fi
fi

# check if user by this UID already exists, if so get the name since id
# only accepts names
if userinfo="$(getent passwd "$uid")"; then
username="${userinfo%%:*}"
else
username="$USERBASE$uid"
if ! err="$(adduser -h "$HOMEPATH$username" -s "$BASHPATH" -G "$groupname" -D -u "$uid" -k "$HOME" "$username" 2>&1)"; then
yermulnik marked this conversation as resolved.
Show resolved Hide resolved
echo_error_and_exit "failed to create uid \"$uid\" with name \"$username\" and group \"$groupname\"\ncommand output: \"$err\""
fi
fi

# it's possible it was not in the group specified, add it
if ! idgroupinfo="$(id -G "$username" 2>&1)"; then
echo_error_and_exit "failed to get group list for username \"$username\"\ncommand output: \"$idgroupinfo\""
fi
if [[ ! " $idgroupinfo " =~ [[:blank:]]${gid}[[:blank:]] ]]; then
if ! err="$(addgroup "$username" "$groupname" 2>&1)"; then
echo_error_and_exit "failed to add user \"$username\" to group \"$groupname\"\ncommand output: \"$err\""
fi
fi

# user and group of specified UID/GID should exist now, and user should be
# a member of group, so execute pre-commit
exec su-exec "$USERID" pre-commit "$@"