-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix(core): Bundling in selinux-enabled OSes #9445
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Hi @corollari From the Docker doc
What are the risks here (in the context of a CDK app)? And do we need the |
According to this source (not official, but I managed to find a few other discussions where people other say the same) this change should only affect systems where SELinux is enforced (systems where it's currently broken) and as long as we stick to only mounting selected directories (like what we are doing right now) there shouldn't be any problems. I also went ahead and asked this question directly on the r/fedora subreddit in order to get the opinions of people more knowledgeable than me on the topic, feel free to skim through the responses there.
If we don't mount |
@eladb opinion on this? |
@@ -112,7 +112,7 @@ export class BundlingDockerImage { | |||
...options.user | |||
? ['-u', options.user] | |||
: [], | |||
...flatten(volumes.map(v => ['-v', `${v.hostPath}:${v.containerPath}:${v.consistency ?? DockerVolumeConsistency.DELEGATED}`])), | |||
...flatten(volumes.map(v => ['-v', `${v.hostPath}:${v.containerPath}:z,${v.consistency ?? DockerVolumeConsistency.DELEGATED}`])), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we only add this if the OS is selinux?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment
Closing for now. Please reopen when ready to follow up |
TLDR: In some linux distributions such as Fedora, the docker container that is used for bundling assets runs into fatal problems due to the selinux configuration
Reproduction steps
@aws-cdk/aws-lambda
bundling system (see below for some example code)cdk synth
Example code:
Problem
Running
cdk synth
produces the following error:So this seems to be a permissions problem, but when you check the permissions of the folders mounted on the root of the docker container, you get:
Given that the code executed inside docker is being run under userdid 1000, this is quite puzzling, as there shouldn't be any permissions problem with a user that is accessing a directory owned by himself. Furthermore all attempts to change the permissions of the
asset-input
directory or modify anything about it just fail.Luckily, after some digging I found the real root of the problem, which is the selinux configuration interfering with the volume mounted inside the docker container.
Solution
The volume just needs to be mounted with the
z
flag, which modifies the selinux labels of the directory.Note that I haven't tested this solution with other OSes such as windows.
... and here goes a 10-hour debugging session that resulted in a 2-character change 😆
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license