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

Pull #1 of Sylabs PRs in preparation for v3.8.1 #6094

Closed
wants to merge 15 commits into from

Conversation

kmuriki
Copy link
Collaborator

@kmuriki kmuriki commented Aug 3, 2021

This PR handles issues #6080 #6078 #6077 #6064 #6063 and #6062

Adam Hughes and others added 15 commits July 22, 2021 13:49
Rather than include config.h through CGO_CFLAGS in mconfig, include
directly in relevant source. This allows tools such as code editors and
linters that are not aware of makeit the ability to build/parse the
file.
The github.com/satori/go.uuid package is no longer maintained. We cannot
fully replace this yet because the github.com/sylabs/sif API requires
it. For now, replace all other uses of github.com/satori/go.uuid with
github.com/google/uuid.

Conflicts:
	e2e/imgbuild/regressions.go
	e2e/instance/instance.go
	e2e/oci/oci.go
	pkg/util/crypt/crypt_dev.go
Not a file used or understood by GitHub. It's intended for humans, so
should be somewhere more noticable :-)

Fixes apptainer#127
Change to sandbox to keep restrictive permissions was made in
Singularity 3.5. It's now appropriate to revise the message so it
doesn't point to a discussion thread, and is just a plain warning.

Fixes apptainer#85

Conflicts:
	internal/pkg/build/sources/oci_unpack.go
Conflicts:
	.circleci/config.yml
	INSTALL.md
In a multi-stage build `%copy`, if the source for the copy line is a
symlink we should dereference it and copy the target. Note that if we
are copying a directory we shouldn't dereference any symlinks
within the directory, only the directory itself.

Previously we were doing a straight `cp -frH` to implement this, but
it does not handle the case where a symlink to be copied is an
absolute symlink. The absolute symlink's target is correct in the
context of the stage's rootfs, but we are copying in the host context.

To fix this we manually dereference the top level src of a `%copy` line,
in a manner that will resolve absolute symlinks relative to the src
stage's rootfs correctly.

This PR also:

 - Splits host->rootfs copy, and stage->stage copy functions.
 - Rewrites the tests for host->rootfs and stage->stage copy
   functions to cover more cases including top-level and nested source
   files, dirs, and absolute + relative symlinks.
 - Uses securejoin to restrict resolution of paths to being inside of a
   rootfs where this is appropriate. This is for least-surprise rather
   than security, as the hostfs is still modifiable in `%setup` etc.

Fixes apptainer#159
In a multi-stage build `%copy`, if the source for the copy line is a
symlink we should dereference it and copy the target. Note that if we
are copying a directory we shouldn't dereference any symlinks
within the directory, only the directory itself.

Previously we were doing a straight `cp -frH` to implement this, but
it does not handle the case where a symlink to be copied is an
absolute symlink. The absolute symlink's target is correct in the
context of the stage's rootfs, but we are copying in the host context.

To fix this we manually dereference the top level src of a `%copy` line,
in a manner that will resolve absolute symlinks relative to the src
stage's rootfs correctly.

This PR also:

 - Splits host->rootfs copy, and stage->stage copy functions.
 - Rewrites the tests for host->rootfs and stage->stage copy
   functions to cover more cases including top-level and nested source
   files, dirs, and absolute + relative symlinks.
 - Uses securejoin to restrict resolution of paths to being inside of a
   rootfs where this is appropriate. This is for least-surprise rather
   than security, as the hostfs is still modifiable in `%setup` etc.

Fixes apptainer#159

Conflicts:
	go.mod
Conflicts:
	CHANGELOG.md
Conflicts:
	e2e/imgbuild/regressions.go
	e2e/instance/instance.go
	e2e/instance/regressions.go
	e2e/oci/oci.go
	internal/app/singularity/plugin_compile_linux.go
	internal/pkg/build/assemblers/sif.go
	pkg/image/sif_test.go
	pkg/util/crypt/crypt_dev.go
@kmuriki kmuriki requested review from DrDaveD and ikaneshiro August 3, 2021 13:25
@DrDaveD DrDaveD added this to the 3.8.1 milestone Aug 3, 2021
@DrDaveD DrDaveD assigned DrDaveD and kmuriki and unassigned DrDaveD Aug 3, 2021
e2e/instance/instance.go Outdated Show resolved Hide resolved
@ikaneshiro
Copy link
Contributor

@kmuriki It looks like the e2e tests are failing because sylabs/singularity@78f8778 has not been pulled in

@DrDaveD
Copy link
Collaborator

DrDaveD commented Aug 3, 2021

@kmuriki It looks like the e2e tests are failing because sylabs/singularity@78f8778 has not been pulled in

Yeah and many of the other ci tests are failing with the same error.

@@ -309,7 +318,11 @@ func (c *ctx) applyCgroupsInstance(t *testing.T) {
}

// pick up a random name
<<<<<<< HEAD
Copy link
Collaborator

@DrDaveD DrDaveD Aug 3, 2021

Choose a reason for hiding this comment

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

I still see <<<< merge text here, and in regressions.go

@kmuriki kmuriki closed this Aug 4, 2021
@kmuriki
Copy link
Collaborator Author

kmuriki commented Aug 4, 2021

Let me try pulling these PRs one at a time and resolve all these conflicts.

@kmuriki kmuriki deleted the sylabs-prs branch August 5, 2021 04:00
@DrDaveD DrDaveD mentioned this pull request Aug 11, 2021
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