Skip to content

Use containers/common for seccomp handling#2569

Closed
saschagrunert wants to merge 1 commit into
opencontainers:masterfrom
saschagrunert:seccomp
Closed

Use containers/common for seccomp handling#2569
saschagrunert wants to merge 1 commit into
opencontainers:masterfrom
saschagrunert:seccomp

Conversation

@saschagrunert
Copy link
Copy Markdown
Contributor

This change introduces the new dependency containers/common, which
mainly conains the logic from libcontainer/seccomp. We're also now able
to completely remove the internal Seccomp type and only rely on the
runtime-spec. The only visible API is now internal only (see
internal/seccomp).

Fixes #2565

PTAL @rhatdan @mrunalp @AkihiroSuda @kolyshkin

@saschagrunert
Copy link
Copy Markdown
Contributor Author

cc @cyphar

This change introduces the new dependency `containers/common`, which
mainly conains the logic from libcontainer/seccomp. We're also now able
to completely remove the internal `Seccomp` type and only rely on the
runtime-spec. The only visible API is now internal only (see
`internal/seccomp`).

Signed-off-by: Sascha Grunert <sgrunert@suse.com>
Comment thread go.sum
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure why so much changes in here... have you tried go mod tidy?

Copy link
Copy Markdown
Contributor Author

@saschagrunert saschagrunert Sep 9, 2020

Choose a reason for hiding this comment

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

Yes, go mod tidy does not change anything inside go.sum.

@kolyshkin
Copy link
Copy Markdown
Contributor

LGTM overall, and kudos for using internal/!

@cyphar
Copy link
Copy Markdown
Member

cyphar commented Feb 4, 2021

This would probably need a rewrite given #2750.

@cyphar
Copy link
Copy Markdown
Member

cyphar commented Feb 4, 2021

I'm also slightly concerned about this PR because it's changing the serialisation format of the Config structure -- which is stored as part of the state (and thus this will cause problems with runc upgrades -- we made this mistake before with initProcessTime).

@saschagrunert
Copy link
Copy Markdown
Contributor Author

I'm also slightly concerned about this PR because it's changing the serialisation format of the Config structure -- which is stored as part of the state (and thus this will cause problems with runc upgrades -- we made this mistake before with initProcessTime).

Ah yeah so I guess we cannot remove the internal types in an easy way. I'm closing for now and will revisit later.

@saschagrunert saschagrunert deleted the seccomp branch February 4, 2021 08:55
@cyphar
Copy link
Copy Markdown
Member

cyphar commented Feb 4, 2021

Yeah in the long-term we should work on removing the really ugly libcontainer serialisation but unfortunately for now it is needed and changing it has caused some /lovely/ bugs in the past.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make seccomp code re-usable

3 participants