Configure the default local seccomp profile according to the runtime

[ccojocar]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Currently the spod fails to start on cri-o runtime because unlike containerd or docker, the cri-o runtime expects the local seccomp profile to be prefixed with 'localhost'.

See for more details https://github.com/cri-o/cri-o/blob/1e6fd9c520d03d47835d1d4c3209e0f77c38f542/internal/config/seccomp/seccomp.go#L240

Which issue(s) this PR fixes:

Does this PR have test?

yes

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Configure the default local seccomp profile according to the runtime (e.g. cri-o expects the profile to be prefixed with `localhost`).

[k8s-ci-robot]

Hi @ccojocar. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[JAORMX]

/ok-to-test

[ccojocar]

/test all

[ccojocar]

/test build

[k8s-ci-robot]

@ccojocar: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

• /test pull-security-profiles-operator-build
• /test pull-security-profiles-operator-build-image
• /test pull-security-profiles-operator-test-e2e
• /test pull-security-profiles-operator-test-unit
• /test pull-security-profiles-operator-verify

Use /test all to run all jobs.

In response to this:

/test build

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.

[ccojocar]

/test pull-security-profiles-operator-build

[JAORMX]

/retest

[codecov-commenter]

Codecov Report

Merging #1255 (bcc11f8) into main (e87e953) will increase coverage by 0.00%.
The diff coverage is 42.85%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1255   +/-   ##
=======================================
  Coverage   44.80%   44.80%           
=======================================
  Files          48       48           
  Lines        5406     5432   +26     
=======================================
+ Hits         2422     2434   +12     
- Misses       2864     2876   +12     
- Partials      120      122    +2     

[ccojocar]

/test pull-security-profiles-operator-test-e2e

[ccojocar]

/test all

[ccojocar]

/test all

[ccojocar]

/test all

[ccojocar]

/test all

[ccojocar]

/test all

[ccojocar]

@saschagrunert All checks finally passed after some reworking. Please could you have a look? Thanks

[saschagrunert]

Currently the spod fails to start on cri-o runtime because unlike containerd or docker, the cri-o runtime expects the local seccomp profile to be prefixed with 'localhost'.

I do not understand this. We test with CRI-O in the CI, so what is the error case?

[ccojocar]

It seems to work on the CI because everything is copied into the root folder but we hit this place in our cluster when running on CRI-O: https://github.com/cri-o/cri-o/blob/1e6fd9c520d03d47835d1d4c3209e0f77c38f542/internal/config/seccomp/seccomp.go#L240

	// Load local seccomp profiles including their availability validation
	if !strings.HasPrefix(profilePath, k8sV1.SeccompLocalhostProfileNamePrefix) {
		return fmt.Errorf("unknown seccomp profile path: %q", profilePath)
	}

As far as I can understand, it expects that the profile is loaded from localhost/....

[saschagrunert]

https://github.com/cri-o/cri-o/blob/1e6fd9c520d03d47835d1d4c3209e0f77c38f542/internal/config/seccomp/seccomp.go#L240

This is the code path for the annotation based seccomp profile, but we're using the fields here, right? So we should land there: https://github.com/cri-o/cri-o/blob/1e6fd9c520d03d47835d1d4c3209e0f77c38f542/internal/config/seccomp/seccomp.go#L296-L302

[ccojocar]

I see but I don't get why in our case we hit the setupFromPath at least from what I see in the error message.

I was wondering because the fix here has no effect on CI cluster. The profile had to be copied under localhost in order to get it working on CI.

On the setupFromPath case the prefix gets stripped off after the check, right?
https://github.com/cri-o/cri-o/blob/1e6fd9c520d03d47835d1d4c3209e0f77c38f542/internal/config/seccomp/seccomp.go#L244

So essential, it doesn't need to be copy under localhost folder, just the localhost prefix needs to be part of the path in the security context.

[ccojocar]

It might be because the Kubernetes version in that cluster is 1.22 and the Field based seccomp profiles are available in the newer version.

[ccojocar]

@saschagrunert if we want to support older version of kubernetes, we will need to prefix the path in the security context. Right? Do you know in which version the switch was made to field base?

[saschagrunert]

Hm, we introduced the fields in 1.19 with seccomp going GA. Do we also use CRI-O 1.22 with k8s 1.22?

[ccojocar]

Hm, we introduced the fields in 1.19 with seccomp going GA. Do we also use CRI-O 1.22 with k8s 1.22?

yeah, we use v1.22.5.

[ccojocar]

We will eventually upgrade, but for now we need to support this.

[ccojocar]

I should revert the changes in non-root enabler which should not copy the profile under localhost but we still need to add the localhost prefix into the security context but also check the Kubernetes version. I am thinking that for newer version this is not required.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ccojocar, saschagrunert

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [saschagrunert]

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

[ccojocar]

@saschagrunert Thanks for approving but I think this is not the final fix. As mentioned above we don't need to copy under localhost folder because the prefix is stripped out.

[saschagrunert]

/test pull-security-profiles-operator-test-e2e