Arbitrary user ID is comming $HOME #86

jpopelka · 2020-11-23T18:36:46Z

(hopefully) Fixes #85

Trying in 2 terminals

$ docker build && docker run -ti --rm -u 123456 usercont/sandcastle
$ docker ps && docker exec -ti -u 123456 <container name> npm install bower
+ bower@1.8.8
added 1 package from 1 contributor and audited 1 package in 3.44s

TomasTomecek

sadly all the nested tests are failing :/ are you working for you locally?

TomasTomecek · 2020-11-24T14:33:43Z

Dockerfile

+    HOME=/home/sandcastle
+
+# npm needs to access ~/.npm and ~/.config
+RUN mkdir --mode 0776 ${HOME}


this should ideally be owned by sandcastle user

there's no such user at the time of building the image

softwarefactory-project-zuul · 2020-11-24T15:57:19Z

Build failed.

pre-commit : SUCCESS in 1m 56s
sandcastle-tests : FAILURE in 11m 55s

jpopelka · 2020-11-24T16:14:48Z

sadly all the nested tests are failing :/ are you working for you locally?

nope and moreover, they fail from some different cause locally
(ERROR: not found: /home/sandcastle/tests/e2e/test_ironman.py::test_exec)

TomasTomecek · 2020-11-25T11:28:47Z

sadly all the nested tests are failing :/ are you working for you locally?

nope and moreover, they fail from some different cause locally
(ERROR: not found: /home/sandcastle/tests/e2e/test_ironman.py::test_exec)

so you probably need to update this function: https://github.com/packit/sandcastle/blob/master/tests/conftest.py#L70

Fixes packit#85

TomasTomecek · 2020-12-11T13:36:55Z

here are my changes https://github.com/TomasTomecek/sandcastle/tree/kodink-ist-hardt

it made a test case pass for me locally:

tests/e2e/test_ironman.py::test_from_pod[test_exec-None] PASSED

if you read the code of k8s client and how they load configuration, you may start thinking something's really iffy - that magic no longer works in 3.9 this commit explicitly passes client.Configuration() to config.load_* so that kubernetes doesn't need to overwrite types or w/e there is also code here to make this work with minishift - not that it worked for me, I ended up doing `eval $(minishft docker-env)` and make test-image-build to get the test image into the VM - couldn't access minishift's registry good luck Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

lachmanfrantisek

Looks good to me, but haven't tested.

csomh · 2020-12-11T14:04:21Z

sandcastle/api.py

+            raise SandcastleException(
+                f"We loaded the kube config but can't access any cluster. "
+                "We have this configutation:\n"
+                f"Auth: {configuration.auth_settings()}\n"


What kind of information does this expose? Is there anything sensitive, like API keys or passwords?

Good point.
Yes, it contains API key, but we raise this exception only if not configuration.api_key in which case the auth_settings() returns nothing valuable -> so we don't actually need it :-)
I'm removing it, thanks.

I added this code because the assert config.api_key was not very clear what's wrong; this code gives at least some context what's wrong

it's true though that content of those 2 method calls is empty/nonsense-ish, so +1 removing those

- auth_settings() doesn't return anything valuable if api_key is empty - get_host_settings() is not implemented in k8s==8.0.0

TomasTomecek

LGTM, tests are green, glad it's working again

softwarefactory-project-zuul · 2020-12-11T16:51:32Z

Build succeeded.

pre-commit : SUCCESS in 3m 06s
sandcastle-tests : SUCCESS in 13m 43s

softwarefactory-project-zuul · 2020-12-11T17:44:10Z

Build succeeded (gate pipeline).

pre-commit : SUCCESS in 1m 50s

In packit#86 we started using the nss wrapper to run commands as users who actually have passwd entries - that's awesome though for .exec() this wasn't utilized b/c setup_env_in_openshift.sh persists only within a shell session - exec creates new sessions so if we want exec's to be run by "real" users, we need to source the setup_env_in_openshift.sh script - there is a test case which verifies this works and we have write access to $HOME Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

In packit#86 we started using the nss wrapper to run commands as users who actually have passwd entries - that's awesome though for .exec() this wasn't utilized b/c setup_env_in_openshift.sh persists only within a shell session - exec creates new sessions so if we want exec's to be run by "real" users, we need to source the setup_env_in_openshift.sh script - there is a test case which verifies this works and we have write access to $HOME Co-authored-by: Hunor Csomortáni <csomortani.hunor@gmail.com> Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

jpopelka requested a review from TomasTomecek November 23, 2020 18:41

This comment has been minimized.

Sign in to view

TomasTomecek reviewed Nov 24, 2020

View reviewed changes

jpopelka force-pushed the npm-home branch 6 times, most recently from 5d0f1a1 to e773a2a Compare November 24, 2020 15:44

jpopelka added 2 commits December 10, 2020 14:21

[Dockerfile] FROM fedora:31 -> docker.io/usercont/base

55fe0df

Arbitrary user ID is comming $HOME

4703b96

Fixes packit#85

jpopelka force-pushed the npm-home branch from e773a2a to 4703b96 Compare December 10, 2020 13:22

This comment has been minimized.

Sign in to view

lachmanfrantisek approved these changes Dec 11, 2020

View reviewed changes

csomh reviewed Dec 11, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

Amend previous commit

73e5bdc

- auth_settings() doesn't return anything valuable if api_key is empty - get_host_settings() is not implemented in k8s==8.0.0

TomasTomecek approved these changes Dec 11, 2020

View reviewed changes

jpopelka force-pushed the npm-home branch from 0da11d1 to 73e5bdc Compare December 11, 2020 16:37

jpopelka added mergeit Run Zuul's gate pipeline and merge the PR and removed mergeit Run Zuul's gate pipeline and merge the PR labels Dec 11, 2020

jpopelka added the mergeit Run Zuul's gate pipeline and merge the PR label Dec 11, 2020

softwarefactory-project-zuul bot merged commit 54ba93c into packit:master Dec 11, 2020

jpopelka deleted the npm-home branch December 11, 2020 17:50

TomasTomecek mentioned this pull request Jan 15, 2021

prepare sandcastle for running multiple commands in a single sandbox #93

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arbitrary user ID is comming $HOME #86

Arbitrary user ID is comming $HOME #86

jpopelka commented Nov 23, 2020 •

edited

Loading

This comment has been minimized.

TomasTomecek left a comment

TomasTomecek Nov 24, 2020

jpopelka Nov 24, 2020

softwarefactory-project-zuul bot commented Nov 24, 2020

jpopelka commented Nov 24, 2020

TomasTomecek commented Nov 25, 2020

This comment has been minimized.

TomasTomecek commented Dec 11, 2020

This comment has been minimized.

lachmanfrantisek left a comment

csomh Dec 11, 2020

jpopelka Dec 11, 2020

TomasTomecek Dec 11, 2020 •

edited

Loading

This comment has been minimized.

TomasTomecek left a comment

softwarefactory-project-zuul bot commented Dec 11, 2020

softwarefactory-project-zuul bot commented Dec 11, 2020

Arbitrary user ID is comming $HOME #86

Arbitrary user ID is comming $HOME #86

Conversation

jpopelka commented Nov 23, 2020 • edited Loading

This comment has been minimized.

TomasTomecek left a comment

Choose a reason for hiding this comment

TomasTomecek Nov 24, 2020

Choose a reason for hiding this comment

jpopelka Nov 24, 2020

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Nov 24, 2020

jpopelka commented Nov 24, 2020

TomasTomecek commented Nov 25, 2020

This comment has been minimized.

TomasTomecek commented Dec 11, 2020

This comment has been minimized.

lachmanfrantisek left a comment

Choose a reason for hiding this comment

csomh Dec 11, 2020

Choose a reason for hiding this comment

jpopelka Dec 11, 2020

Choose a reason for hiding this comment

TomasTomecek Dec 11, 2020 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

TomasTomecek left a comment

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Dec 11, 2020

softwarefactory-project-zuul bot commented Dec 11, 2020

jpopelka commented Nov 23, 2020 •

edited

Loading

TomasTomecek Dec 11, 2020 •

edited

Loading