-
Notifications
You must be signed in to change notification settings - Fork 73
Jailer support #57
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
Jailer support #57
Conversation
f202987
to
d4fb2ec
Compare
Updated the commits to include the "signed-off" line |
Thanks for this. I'll take a look as soon as I've got some time. Seems like I also need to spend some time with the buildkite configuration, too. |
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.
This looks good to me. My only concern is about the go.sum reference to the firecracker-go-sdk module. I'll try to investigate that a little deeper; please feel free to do so yourself, if you're able. I'd like to confirm that we have the right value in that file before merging.
github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= | ||
github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= | ||
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= | ||
github.com/firecracker-microvm/firecracker-go-sdk v0.21.0 h1:41W/zyL3S33ZK/pNFfVrW38Y2YpNE6ZFArQOHL27V2I= |
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.
There seems to be something wrong with the firecracker-go-sdk served by proxy.golang.org and/or sum.golang.org, and I'm running in to firecracker-microvm/firecracker-go-sdk#224 and/or firecracker-microvm/firecracker-go-sdk#235 when trying to build your changes. I can obviously work around this while reviewing, but I'm not sure this checksum (h1.41W/...
) is the correct one.
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.
Hmm, I ran 'go mod tidy', thats what caused the go.sum changes.
We could just revert the go.sum file to the previous state?
I executed go mod tidy on macOS, even though I can't build the project for darwin,
due to one of its dependencies:
$ go install
go build github.com/containernetworking/plugins/pkg/ns: build constraints exclude all Go files in /Users/username/go/pkg/mod/github.com/containernetworking/[email protected]/pkg/ns
Running go mod tidy again gives me the following output:
$ git status
On branch jailer-support
Your branch is up to date with 'origin/jailer-support'.
nothing to commit, working tree clean
$ go mod tidy
go: downloading github.com/firecracker-microvm/firecracker-go-sdk v0.21.0
go: downloading github.com/go-openapi/strfmt v0.19.5
go: downloading github.com/stretchr/testify v1.5.1
go: downloading github.com/go-openapi/errors v0.19.3
go: downloading github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a
go: downloading go.mongodb.org/mongo-driver v1.4.0
go: downloading github.com/go-openapi/runtime v0.19.11
go: downloading github.com/go-openapi/swag v0.19.7
go: downloading github.com/hashicorp/go-multierror v1.1.0
go: downloading github.com/containernetworking/cni v0.8.0
go: downloading github.com/containerd/fifo v0.0.0-20200410184934-f15a3290365b
go: downloading github.com/sparrc/go-ping v0.0.0-20190613174326-4e5b6552494c
go: downloading gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127
go: downloading github.com/containernetworking/plugins v0.8.7-0.20200805193842-bd589992fbe0
go: downloading github.com/mailru/easyjson v0.7.2
go: downloading github.com/hashicorp/errwrap v1.0.0
go: downloading github.com/go-openapi/validate v0.19.6
go: downloading github.com/onsi/gomega v1.10.1
go: downloading github.com/onsi/ginkgo v1.14.0
go: downloading github.com/josharian/intern v1.0.0
go: downloading github.com/go-openapi/analysis v0.19.5
go: downloading github.com/kr/text v0.1.0
go: downloading github.com/go-openapi/loads v0.19.4
go: downloading github.com/go-openapi/spec v0.19.9
go: downloading github.com/nxadm/tail v1.4.4
go: downloading github.com/vishvananda/netlink v1.1.0
go: downloading github.com/go-openapi/jsonpointer v0.19.3
go: downloading github.com/fsnotify/fsnotify v1.4.9
go: downloading github.com/go-openapi/jsonreference v0.19.4
go: downloading github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae
go: downloading github.com/PuerkitoBio/purell v1.1.1
go: downloading github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578
go: downloading github.com/tidwall/pretty v1.0.1
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.
We could just revert the go.sum file to the previous state?
When firecracker-microvm/firecracker-go-sdk#266 gets merged, we can update to version 0.22. I think that's the better way to go. Let's wait for that and see if the new version resolves the proxy/hash issue.
// getSocketPath provides a randomized socket path by building a unique fielname | ||
// and searching for the existance of directories {$HOME, os.TempDir()} and returning | ||
// getSocketPath provides a randomized socket path by building a unique filename | ||
// and searching for the existence of directories {$HOME, os.TempDir()} and returning |
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.
👍
I've added a couple new commits on top of yours to introduce SDK 0.22.0 support. It looks like at least |
Signed-off-by: Philipp Mieden <[email protected]>
Signed-off-by: Noah Meyerhans <[email protected]>
Signed-off-by: Noah Meyerhans <[email protected]>
9ec8335
to
2939d8d
Compare
Done, squashed all my commits into a single one #f1c8ba0. |
Signed-off-by: Noah Meyerhans <[email protected]>
f3f344a
to
3ad1b9f
Compare
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.
Thanks for contributing this useful feature.
Jailer Support
This pull request adds support to use the jailer for securing firecracker vms launched with firectl.
It extends the flag set with the following flags, following the firecracker flag naming of the firecracker and jailer binaries:
It makes use of the official Go SDK to invoke the jailer and is configured entirely via commandline flags.
The new command line interface looks like this:
I made a small change to the API to keep the configuration simple and ease debugging: firecracker-microvm/firecracker-go-sdk#255
However, this pull request does not depend on it and uses the latest version v0.21.0 to accomplish its task.
cc @ppartarr
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.