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

Apply SELinux label to allocate directory of docker driver #377

Merged
merged 1 commit into from
Nov 5, 2015
Merged

Apply SELinux label to allocate directory of docker driver #377

merged 1 commit into from
Nov 5, 2015

Conversation

nak3
Copy link
Contributor

@nak3 nak3 commented Nov 3, 2015

No description provided.

@nak3
Copy link
Contributor Author

nak3 commented Nov 3, 2015

Without this fix, driver test (i.e. $ go test -cover -timeout=500s github.com/hashicorp/nomad/client/drive) also fails on SELinux enabled environment.

@@ -108,8 +108,8 @@ func (d *DockerDriver) containerBinds(alloc *allocdir.AllocDir, task *structs.Ta
}

return []string{
fmt.Sprintf("%s:%s", shared, allocdir.SharedAllocName),
fmt.Sprintf("%s:%s", local, allocdir.TaskLocal),
fmt.Sprintf("%s:%s:rw,Z", shared, allocdir.SharedAllocName),
Copy link
Contributor

Choose a reason for hiding this comment

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

The shared alloc is shared across containers so it should be z (lowercase)

Copy link
Contributor

Choose a reason for hiding this comment

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

Please put a comment on why these are needed above

@nak3
Copy link
Contributor Author

nak3 commented Nov 4, 2015

Thanks, I updated.

// "z" and "Z" option is to allocate directory with SELinux label.
fmt.Sprintf("%s:%s:rw,z", shared, allocdir.SharedAllocName),
// capital "Z" will label with Multi-Category Security (MCS) labels
fmt.Sprintf("%s:%s:rw,Z", local, allocdir.TaskLocal),
Copy link
Contributor

Choose a reason for hiding this comment

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

I pulled your branch and you need to add one more thing for it to work.

Can you change %s:%s to %s:/%s on both of them. It needs to be an absolute path with that flag.

@nak3
Copy link
Contributor Author

nak3 commented Nov 5, 2015

Thanks again. I updated. The test scripts passed both (w/ or w/o /%s), so it may need test case.

dadgar added a commit that referenced this pull request Nov 5, 2015
Apply SELinux label to allocate directory of docker driver
@dadgar dadgar merged commit cb811dd into hashicorp:master Nov 5, 2015
@github-actions
Copy link

github-actions bot commented May 6, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants