SELinux labels in batch changes #570

martin-sucha · 2021-07-21T08:57:04Z

On Fedora 34 I get an error like the following (with src-cli 3.30.0):

   run: echo Hello World | tee -a $(find -name README.md)
   container: alpine:3
   
   standard error:
   	/bin/sh: can't open '/tmp/tmp.IbdkiA': Permission denied

when running the hello world batch change. SELinux blocks the Docker bind mount.

src-cli uses Docker arguments like --mount type=bind,source=/tmp/205206724,target=/tmp/tmp.MLPLgP,ro for mounting. If I replace them with /tmp/205206724:/tmp/tmp.MLPLgP:ro,Z then the mount succeeds. I have replaced those occurrences in my local copy of src-cli and now it works.

However, we need to be careful with using the Z option as it modifies the SELinux labels on the host, see https://docs.docker.com/storage/bind-mounts/#configure-the-selinux-label

If all the files that src-cli mounts are temporary files then it should probably be okay to use it.

I have not tried to run rootless docker yet, so I don't know if that would fix the issue.

In any case even if it is decided not add the Z flag to src-cli, the error message could be better.

What do you think about it?

The text was updated successfully, but these errors were encountered:

mrnugget · 2021-07-21T09:21:01Z

Thanks for reporting! Have you tried the volume-based workspaces? If you have -workspace volume to src batch [preview|apply] then Docker volumes are created and attached to the containers instead of mounting a directory on the host into the container.

Agree on the error message. In order to fix that, though, we'll probably need to do dry run to check. I'll bring this up in our team sync.

martin-sucha · 2021-07-21T09:29:35Z

Nice! I didn't know about -workspace volume option. I checked https://docs.sourcegraph.com/batch_changes/references/troubleshooting and tried searching for SELinux but I didn't find the option that way.

src batch preview -workspace volume -f hello-world.yaml works for me, thanks!

mrnugget · 2021-07-21T09:32:13Z

Fantastic! And also agree on the troubleshooting guide: this one should be in there. I think until now we've wanted to see how well -workspace volume works in customer environments before we advertise it further.

mrnugget · 2021-07-21T09:49:37Z

Adding it to the troubleshooting page here: https://github.com/sourcegraph/sourcegraph/pull/23068

martin-sucha · 2021-07-21T10:29:27Z

Thanks! I don't know if you want to keep this issue open (for tracking change to the error message that src-cli prints) or not. The doc update works for me, so feel free to close issue when needed.

martin-sucha · 2021-07-21T11:31:08Z

Actually, it looks like the volume command only works after I run my modified version (that changes the SELinux labels on host). I see sh: can't open '/run.sh': Permission denied when I try to run a preview with -workspace volume on a new yaml file.

mrnugget · 2021-07-21T11:37:11Z

Interesting! So what exactly did you modify? Adding the ,Z to the bind command and that in updated the SELinux on the host and thus gave it permission?

Does using a different temp dir work? You can use the -tmp flag to set it to another directory. Default on Linux is what Go's os.TempDir returns: https://pkg.go.dev/os#TempDir And that in turn is based on $TMP.

martin-sucha · 2021-07-21T12:37:01Z

Interesting! So what exactly did you modify? Adding the ,Z to the bind command and that in updated the SELinux on the host and thus gave it permission?

Yes, exactly. This is the change I made:

diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go
index fe3fdc1..db6376f 100644
--- a/internal/batches/executor/run_steps.go
+++ b/internal/batches/executor/run_steps.go
@@ -281,11 +281,11 @@ func executeSingleStep(
 		"--init",
 		"--cidfile", cidFile,
 		"--workdir", scriptWorkDir,
-		"--mount", fmt.Sprintf("type=bind,source=%s,target=%s,ro", runScriptFile, containerTemp),
+		"-v", fmt.Sprintf("%s:%s:ro,Z", runScriptFile, containerTemp),
 	}, workspaceOpts...)
 
 	for target, source := range filesToMount {
-		args = append(args, "--mount", fmt.Sprintf("type=bind,source=%s,target=%s,ro", source.Name(), target))
+		args = append(args, "-v", fmt.Sprintf("%s:%s:ro,Z", source.Name(), target))
 	}
 
 	for k, v := range env {
diff --git a/internal/batches/workspace/bind_workspace.go b/internal/batches/workspace/bind_workspace.go
index f7281c0..4d70776 100644
--- a/internal/batches/workspace/bind_workspace.go
+++ b/internal/batches/workspace/bind_workspace.go
@@ -117,8 +117,8 @@ func (w *dockerBindWorkspace) Close(ctx context.Context) error {
 
 func (w *dockerBindWorkspace) DockerRunOpts(ctx context.Context, target string) ([]string, error) {
 	return []string{
-		"--mount",
-		fmt.Sprintf("type=bind,source=%s,target=%s", w.dir, target),
+		"-v",
+		fmt.Sprintf("%s:%s:Z", w.dir, target),
 	}, nil
 }

martin-sucha · 2021-07-21T12:48:05Z

Changing the temporary directory with -tmp does not seem to have any effect after it succeeded for some repo. Perhaps the data is cached somewhere, so src-cli skips some steps?

When I try to run the change against a git repository that I haven't used yet, I get the error even with -workspace volume if I don't run the patched version first.

mrnugget · 2021-07-21T15:51:01Z

Yeah, src-cli caches results heavily in order to make iterating faster.

So, just confirming: if you run src batch [apply|preview] with

-tmp set to a directory that you have access to
-clear-cache to skip the cache
-workspace volume

it still produces the error?

martin-sucha · 2021-07-22T12:08:42Z

Yeah, src-cli caches results heavily in order to make iterating faster.

So, just confirming: if you run src batch [apply|preview] with

-tmp set to a directory that you have access to

-clear-cache to skip the cache

-workspace volume

it still produces the error?

Yes. With -clear-cache the error is always reproducible with src-cli version 3.30.0.

~/P/large-scale-changes> src version
Current version: 3.30.0
Recommended version: 3.30.0 or later
~/P/large-scale-changes> ~/OpenSource/src-cli/src version
Current version: dev
Recommended version: 3.30.0 or later

Initial workspace creation with released src-cli fails.

Command-line log

~/P/large-scale-changes> src batch preview -tmp $HOME/Projects/large-scale-changes/test-temp3 -clear-cache -workspace volume -f hello-world3.yaml
✅ Parsing batch spec
✅ Resolving namesapce
✅ Preparing container images  ██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████
✅ Set workspace type
✅ Resolved 1 repositories
✅ Found 1 workspaces with steps to execute
✅ Found 0 cached changeset specs; 1 task needs to be executed
⠼  Executing... (0/1, 0 errored)  
│                                                                                                                                                            
└── github.com/kiwicom/kiwi-platform-py  Initializing workspace                                                                                            1s

❌ Error:
   github.com/kiwicom/kiwi-platform-py:
   creating workspace: preparing local git repo: preparing workspace: Docker output:
   
   sh: can't open '/run.sh': Permission denied
   
   
   : exit status 2
   Log: /home/martin/Projects/large-scale-changes/test-temp3/changeset-github.com-kiwicom-kiwi-platform-py-15a6c3f41680b941834e1142429ac12de9131a06.024493812.log
   
   

💡 The troubleshooting documentation can help to narrow down the cause of the errors:
   https://docs.sourcegraph.com/batch_changes/references/troubleshooting

Then if I run the patched version (without -workspace volume since I haven't patched that code path), it updates the SELinux labels and the command succeeds.

Command-line log

~/P/large-scale-changes> ~/OpenSource/src-cli/src batch preview -tmp $HOME/Projects/large-scale-changes/test-temp3 -clear-cache -f hello-world3.yaml
✅ Parsing batch spec
✅ Resolving namesapce
✅ Preparing container images  ██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████
✅ Set workspace type
✅ Resolved 1 repositories
✅ Found 1 workspaces with steps to execute
✅ Found 0 cached changeset specs; 1 task needs to be executed
✅ Executing... (1/1, 0 errored)  ███████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████
✅ Creating batch spec on Sourcegraph

✅ To preview or apply the batch spec, go to:
   https://sourcegraph.gitlab-gcp.skypicker.com/users/martin.sucha/batch-changes/apply/QmF0Y2hTcGVjOiI2S1NOSmh1ZjEydSI=

Then if I run the released version, SELinux labels are still present on the cached version and even the upstream command succeeds.

Command-line log

~/P/large-scale-changes> src batch preview -tmp $HOME/Projects/large-scale-changes/test-temp3 -workspace volume -f hello-world3.yaml
✅ Parsing batch spec
✅ Resolving namesapce
✅ Preparing container images  ██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████
✅ Set workspace type
✅ Resolved 1 repositories
✅ Found 1 workspaces with steps to execute
✅ Found 0 cached changeset specs; no tasks need to be executed
✅ Creating batch spec on Sourcegraph

✅ To preview or apply the batch spec, go to:
   https://sourcegraph.gitlab-gcp.skypicker.com/users/martin.sucha/batch-changes/apply/QmF0Y2hTcGVjOiI2cFZQNXgzVWxXYyI=

Then if I run with -clear-cache, the released version fails again.

Command-line log

~/P/large-scale-changes> src batch preview -tmp $HOME/Projects/large-scale-changes/test-temp3 -clear-cache -workspace volume -f hello-world3.yaml
✅ Parsing batch spec
✅ Resolving namesapce
✅ Preparing container images  ██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████
✅ Set workspace type
✅ Resolved 1 repositories
✅ Found 1 workspaces with steps to execute
✅ Found 0 cached changeset specs; 1 task needs to be executed
⠸  Executing... (0/1, 0 errored)  
│                                                                                                                                                            
└── github.com/kiwicom/kiwi-platform-py  Initializing workspace                                                                                            1s

❌ Error:
   github.com/kiwicom/kiwi-platform-py:
   creating workspace: preparing local git repo: preparing workspace: Docker output:
   
   sh: can't open '/run.sh': Permission denied
   
   
   : exit status 2
   Log: /home/martin/Projects/large-scale-changes/test-temp3/changeset-github.com-kiwicom-kiwi-platform-py-15a6c3f41680b941834e1142429ac12de9131a06.455430908.log
   
   

💡 The troubleshooting documentation can help to narrow down the cause of the errors:
   https://docs.sourcegraph.com/batch_changes/references/troubleshooting

mrnugget · 2021-07-22T14:16:27Z

Thanks so much! We'll look into this as soon as we can. In the meantime it sounds like you're at least unblocked from trying it out - even though it involved building your own version 😓

malomarrec · 2021-08-04T11:46:02Z

We discussed this issue and we are going to backlog it for now, given that we have big milestones for this quarter that we need to focus on, and given there is a workaround (thanks for that!). We're keeping it on our radar, though, thanks for reporting.

I updated the documentation to make that clear.

mrnugget mentioned this issue Jul 21, 2021

Add workspace volume mode to batch changes troubleshooting doc sourcegraph/sourcegraph-public-snapshot#23068

Merged

mrnugget added the bug Something isn't working label Jul 22, 2021

chrispine added the estimate/3d label Aug 3, 2021

malomarrec self-assigned this Aug 3, 2021

chrispine added the team/code-search label Oct 1, 2021

Piszmog removed the estimate/3d label Oct 6, 2022

courier-new unassigned malomarrec Jun 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SELinux labels in batch changes #570

SELinux labels in batch changes #570

martin-sucha commented Jul 21, 2021 •

edited

Loading

mrnugget commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

mrnugget commented Jul 21, 2021

mrnugget commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

mrnugget commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

mrnugget commented Jul 21, 2021

martin-sucha commented Jul 22, 2021

mrnugget commented Jul 22, 2021

malomarrec commented Aug 4, 2021

SELinux labels in batch changes #570

SELinux labels in batch changes #570

Comments

martin-sucha commented Jul 21, 2021 • edited Loading

mrnugget commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

mrnugget commented Jul 21, 2021

mrnugget commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

mrnugget commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

martin-sucha commented Jul 21, 2021

mrnugget commented Jul 21, 2021

martin-sucha commented Jul 22, 2021

mrnugget commented Jul 22, 2021

malomarrec commented Aug 4, 2021

martin-sucha commented Jul 21, 2021 •

edited

Loading