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

Add userid to ark alias #6

Merged
merged 1 commit into from
Aug 4, 2017
Merged

Conversation

carsonoid
Copy link
Contributor

Since the ark container runs as a non-privileged user by default there are cases where it may not be able to read some user's config files if the permissions are more restrictive. Running the ark as the active user will make sure that the config file can be used in all cases.

@heptibot
Copy link

heptibot commented Aug 4, 2017

Can one of the admins verify this patch?

@ncdc
Copy link
Contributor

ncdc commented Aug 4, 2017

@carsonoid thanks for your pull request! This is a good idea 😄. Would you mind signing your commit as described here?

README.md Outdated
@@ -75,7 +75,7 @@ kubectl get deployments --namespace=nginx-example
Finally, create an alias for the Ark client's Docker executable. (Make sure that your `KUBECONFIG` environment variable is pointing at the proper config first). This will save a lot of future typing:

```
alias ark='docker run --rm -v $(dirname $KUBECONFIG):/kubeconfig -e KUBECONFIG=/kubeconfig/$(basename $KUBECONFIG) gcr.io/heptio-images/ark:latest'
alias ark='docker run --rm -u$(id -u) -v $(dirname $KUBECONFIG):/kubeconfig -e KUBECONFIG=/kubeconfig/$(basename $KUBECONFIG) gcr.io/heptio-images/ark:latest'
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could you put a space here for consistency (e.g. -u $(id -u))?
otherwise looks good!

Copy link
Contributor

Choose a reason for hiding this comment

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

oh also one more thing: it's a bit of duplication, but do you mind also fixing the alias command here: https://github.com/heptio/ark/blob/master/docs/cli-reference/README.md

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks!

Since the ark container runs as a non-privileged user by default there are cases where it may not be able to read some user's config files if the permissions are more restrictive. Running the ark as the active user will make sure that the config file can be used in all cases.

Signed-off-by: Carson Anderson <ca@carsonoid.net>
@carsonoid
Copy link
Contributor Author

Updated with all requested changes

@ncdc
Copy link
Contributor

ncdc commented Aug 4, 2017

Thanks @carsonoid!

@ncdc
Copy link
Contributor

ncdc commented Aug 4, 2017

test this please

@ncdc ncdc merged commit f7f6e16 into vmware-tanzu:master Aug 4, 2017
dymurray added a commit to dymurray/velero that referenced this pull request Nov 8, 2019
use experimental --skip-unchanged/--delete restic options
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.

4 participants