seccomp.json moved to @seccomp/containers-golang

[lsm5]

Signed-off-by: Lokesh Mandvekar lsm5@fedoraproject.org

@baude @mheon @rhatdan

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lsm5
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: umohnani8

If they are not already assigned, you can assign the PR to them by writing /assign @umohnani8 in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[lsm5]

/assign @umohnani8

[vrothberg]

Can you elaborate on why this change is needed? I feel rather strongly against this change for a multitude of reasons:

• Removing the file here will certainly break package builds of at least openSUSE and SUSE. Those packages would then be forced to add yet another source to the package just to fetch the seccomp.json.

• Why do we need to rely on an github.com/containers external project for such an important config?

• The two seccomp.json files are different.

• Compiling libpod from scratch is becoming rediculously hard. I don't think it's user or even developer friendly to force them to fetch configs and files from 3+ projects inside and outside of github.com/containers.

If this change is really desired, I really want to know why and request to document this somewhere (and to highlight it in the changelogs for the next release).

[lsm5]

* Removing the file here will certainly break package builds of at least openSUSE and SUSE. Those packages would then be forced to add yet another source to the package just to fetch the seccomp.json.

Ack, Fedora includes that file in the dist-git source itself. I'd really like 1 canonical location for that file, wherever that may be.

* Why do we need to rely on an github.com/containers external project for such an important config?

@rhatdan can we move @seccomp/containers-golang to @containers?

* The two seccomp.json files are different.

Ack. They're being installed to the same location so maybe we should work on making these consistent?

* Compiling libpod from scratch is becoming rediculously hard. I don't think it's user or even developer friendly to force them to fetch configs and files from 3+ projects inside and outside of github.com/containers.

Ack, see question above about moving the repo here.

If this change is really desired, I really want to know why and request to document this somewhere (and to highlight it in the changelogs for the next release).

Will do, I can add this if we decide to go ahead with this.

[rhatdan]

Is there a way to handle this via github? Subpackages? I agree with both of you @lsm5 and @giuseppe,

[lsm5]

Is there a way to handle this via github? Subpackages? I agree with both of you @lsm5 and @giuseppe,

I guess you meant @vrothberg :) . How about vendoring like we did for registries.conf in skopeo?

[vrothberg]

I am still seeking for an answer to why this change is needed :^)

It's already very complex to know which packages/dependencies in which version are used/required where. The packaging efforts for Debian keep revealing more and more issues and I am worried that this change will increase the complexity.

To be fair, it's not a huge thing but I would rather go in the opposite direction and make the projects more self-contained. Especially the configs are important. Imagine the seccomp.json config has a bug. We first had to update the go-seccomp version, then revendor into the tools. How do we handle issues for stable versions where upstream keeps diverging? Do we need to patch a config in the vendor folder? If upstream developers are already fighting the dependencies, it is even harder for downstream packagers. It really doesn't feel right.

I am sorry to hijack this PR, @lsm5. My concerns are more of a general nature then being specific to the proposed change.

[lsm5]

@rhatdan @mrunalp so I see that seccomp.json is being installed to /usr/share/containers in libpod's Makefile and to /etc/crio/ in cri-o's Makefile. If those are the final locations for both, then I'm fine with closing this PR.

[rhatdan]

Well I would prefer to have them all share the same location. I don't like the fact that CRI-O has this in a different location. The more the configurations are shared the easier it is for an admin to manage.

[vrothberg]

Well I would prefer to have them all share the same location. I don't like the fact that CRI-O has this in a different location. The more the configurations are shared the easier it is for an admin to manage.

I don't see how this change is achieving it. libpod is already required as a package source now for containers-common (or libcontainers-common on openSUSE) because of the oci-hooks manpage and the seccomp.json. CRI-O could just use the seccomp.conf from libpod.

[mheon]

I'd really like to deprecate CRI-O's /etc/crio location - it should default to the same paths Podman and Buildah do, so config changes affect the whole system.

@vrothberg While I agree that this causes packaging concerns, config files like this don't really belong to any one project. In your Seccomp profile bug case, right now, we have to fix it in at least two locations (CRI-O and Podman repos). I think that's just as bad for packaging complexity.

I don't know is containers-seccomp is the best place for the config - maybe we should have a github.com/containers/configs to store the default mounts.conf, storage.conf, etc files, and then submodule that into our projects?

[vrothberg]

@vrothberg While I agree that this causes packaging concerns, config files like this don't really belong to any one project. In your Seccomp profile bug case, right now, we have to fix it in at least two locations (CRI-O and Podman repos). I think that's just as bad for packaging complexity.

The config file has been in libpod since I know it. Removing it breaks downstream packages. Currently, CRI-O ships it's own seccomp.json but there are plans to change that. Executing the current plan means work for everybody, upstream and downstream. I don't think that's necessary.

[rhatdan]

Can we move this to containers common and then vendor it into podman and cri-o.

[rhatdan]

@lsm5 Any update on this?

[lsm5]

@lsm5 Any update on this?

I guess we gotta have everyone agree on a single seccomp.json before we can proceed with this.

[lsm5]

Also, if we're moving to go mod on this, I don't know how that'd affect this file if it's in seccomp/containers-golang.

[rhatdan]

@vrothberg @lsm5 Where do we stand with this one?

[vrothberg]

As @lsm5 mentioned, as soon as we move to use go modules, we cannot use such files from the ./vendor directory anymore. I am okay with any solution that does not rely on ./vendor but we need to be verbose in the changelogs to avoid breaking packages for other distros.

[rh-atomic-bot]

☔ The latest upstream changes (presumably #3278) made this pull request unmergeable. Please resolve the merge conflicts.

[openshift-ci-robot]

@lsm5: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vrothberg]

@lsm5, can you rebase?
LGTM

[vrothberg]

Needs a rebase

[vrothberg]

Ping @lsm5

[baude]

@lsm5 is this needed anymore?

[github-actions]

This pull request had no activity for 30 days. In the absence of activity or the "do-not-close" label, the pull request will be automatically closed within 7 days.

[rhatdan]

@lsm5 Whats the scoop on this one?