Repository: kubernetes-sigs/security-profiles-operator
Pull request #256 (2021-02-02 14:37:22)

Add profile recording 'kind' field

#### What type of PR is this?

/kind cleanup
/kind feature

#### What this PR does / why we need it:
Target of this field is to support additional profile types in the
future. We now also re-structure the project a bit to reflect which
parts are intended to be generic (binding/recording) and specific to a
certain technology (seccomp/selinux/apparmor).
#### Which issue(s) this PR fixes:

<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.

Fixes #

or

None
-->
Refers to #46 
#### Does this PR have test?

<!--
If tests aren't applicable just write N/A.
-->
None
#### Special notes for your reviewer:
None
#### Does this PR introduce a user-facing change?

<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".

For more information on release notes see: https://git.k8s.io/community/contributors/guide/release-notes.md
-->

```release-note
None
```


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/256#" title="Author self-approved">saschagrunert</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fsecurity-profiles-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/security-profiles-operator/blob/master/OWNERS)~~ [saschagrunert]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
JAORMX: /lgtm

And thanks for the cleanup on the seccomp profile controller package!

Patch:
diff --git a/api/profilerecording/v1alpha1/profilebinding_types.go b/api/profilerecording/v1alpha1/profilebinding_types.go
index a0e120906..f211245b7 100644
--- a/api/profilerecording/v1alpha1/profilebinding_types.go
+++ b/api/profilerecording/v1alpha1/profilebinding_types.go
@@ -22,6 +22,10 @@ import (
 
 // ProfileRecordingSpec defines the desired state of ProfileRecording.
 type ProfileRecordingSpec struct {
+	// Kind of object to be recorded.
+	// +kubebuilder:validation:Enum=SeccompProfile
+	Kind string `json:"kind"`
+
 	// PodSelector selects the pods to record. This field follows standard
 	// label selector semantics. An empty podSelector matches all pods in this
 	// namespace.
diff --git a/cmd/security-profiles-operator/main.go b/cmd/security-profiles-operator/main.go
index 8b670c14b..162ad8baf 100644
--- a/cmd/security-profiles-operator/main.go
+++ b/cmd/security-profiles-operator/main.go
@@ -35,8 +35,8 @@ import (
 	seccompprofilev1alpha1 "sigs.k8s.io/security-profiles-operator/api/seccompprofile/v1alpha1"
 	selinuxpolicyv1alpha1 "sigs.k8s.io/security-profiles-operator/api/selinuxpolicy/v1alpha1"
 	"sigs.k8s.io/security-profiles-operator/internal/pkg/config"
-	"sigs.k8s.io/security-profiles-operator/internal/pkg/controllers/profile"
 	"sigs.k8s.io/security-profiles-operator/internal/pkg/controllers/profilerecorder"
+	"sigs.k8s.io/security-profiles-operator/internal/pkg/controllers/seccompprofile"
 	"sigs.k8s.io/security-profiles-operator/internal/pkg/controllers/selinuxpolicy"
 	"sigs.k8s.io/security-profiles-operator/internal/pkg/controllers/spod"
 	"sigs.k8s.io/security-profiles-operator/internal/pkg/version"
@@ -237,11 +237,11 @@ func getEnabledControllers(ctx *cli.Context) []*controllerSettings {
 	enabledSettings = append(enabledSettings,
 		&controllerSettings{
 			name:          "seccomp-spod",
-			setupFn:       profile.Setup,
+			setupFn:       seccompprofile.Setup,
 			schemaBuilder: seccompprofilev1alpha1.SchemeBuilder,
 		},
 		&controllerSettings{
-			name:          "seccomp-recorder-spod",
+			name:          "recorder-spod",
 			setupFn:       profilerecorder.Setup,
 			schemaBuilder: seccompprofilev1alpha1.SchemeBuilder,
 		},
diff --git a/deploy/base/webhook/crd-recording.yaml b/deploy/base/webhook/crd-recording.yaml
index 2a691dae2..312460559 100644
--- a/deploy/base/webhook/crd-recording.yaml
+++ b/deploy/base/webhook/crd-recording.yaml
@@ -32,6 +32,11 @@ spec:
           spec:
             description: ProfileRecordingSpec defines the desired state of ProfileRecording.
             properties:
+              kind:
+                description: Kind of object to be recorded.
+                enum:
+                - SeccompProfile
+                type: string
               podSelector:
                 description: PodSelector selects the pods to record. This field follows standard label selector semantics. An empty podSelector matches all pods in this namespace.
                 properties:
@@ -63,6 +68,7 @@ spec:
                     type: object
                 type: object
             required:
+            - kind
             - podSelector
             type: object
           status:
diff --git a/deploy/webhook.yaml b/deploy/webhook.yaml
index 7c3b760e1..854462550 100644
--- a/deploy/webhook.yaml
+++ b/deploy/webhook.yaml
@@ -108,6 +108,11 @@ spec:
           spec:
             description: ProfileRecordingSpec defines the desired state of ProfileRecording.
             properties:
+              kind:
+                description: Kind of object to be recorded.
+                enum:
+                - SeccompProfile
+                type: string
               podSelector:
                 description: PodSelector selects the pods to record. This field follows standard label selector semantics. An empty podSelector matches all pods in this namespace.
                 properties:
@@ -139,6 +144,7 @@ spec:
                     type: object
                 type: object
             required:
+            - kind
             - podSelector
             type: object
           status:
diff --git a/examples/profilerecording.yaml b/examples/profilerecording.yaml
index 9096ee9dc..5dc009414 100644
--- a/examples/profilerecording.yaml
+++ b/examples/profilerecording.yaml
@@ -6,6 +6,7 @@ metadata:
   # after reconciliation.
   name: test-recording
 spec:
+  kind: SeccompProfile
   podSelector:
     matchLabels:
       app: alpine
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index 7c0b83177..aa29d626c 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -47,10 +47,10 @@ const (
 	SeccompProfileRecordAnnotationKey = "io.containers.trace-syscall"
 )
 
-// SeccompProfileRecordOutputPath is the path where the OCI seccomp BPF
-// hook saves the profiles. Those profiles are going to be reconciled into
-// the seccomp profile CRD and have a limited lifetime.
-var SeccompProfileRecordOutputPath = filepath.Join(os.TempDir(), "seccomp-profile-recordings")
+// ProfileRecordingOutputPath is the path where the recorded profiles will be
+// stored. Those profiles are going to be reconciled into native CRDs and
+// therefore have a limited lifetime.
+var ProfileRecordingOutputPath = filepath.Join(os.TempDir(), "security-profiles-operator-recordings")
 
 // Controller configMap keys.
 const (
diff --git a/internal/pkg/controllers/profilerecorder/profilerecorder.go b/internal/pkg/controllers/profilerecorder/profilerecorder.go
index 9b28f2d06..e611c6587 100644
--- a/internal/pkg/controllers/profilerecorder/profilerecorder.go
+++ b/internal/pkg/controllers/profilerecorder/profilerecorder.go
@@ -172,7 +172,7 @@ func (r *RecorderReconciler) Reconcile(req reconcile.Request) (reconcile.Result,
 			r.record.Event(pod, event.Warning(reasonAnnotationParsing, err))
 			return reconcile.Result{}, nil
 		}
-		if !strings.HasPrefix(outputFile, config.SeccompProfileRecordOutputPath) {
+		if !strings.HasPrefix(outputFile, config.ProfileRecordingOutputPath) {
 			logger.Info("Ignoring Profile outside standard output path")
 		}
 
diff --git a/internal/pkg/controllers/profile/profile.go b/internal/pkg/controllers/seccompprofile/seccompprofile.go
similarity index 99%
rename from internal/pkg/controllers/profile/profile.go
rename to internal/pkg/controllers/seccompprofile/seccompprofile.go
index d504c9943..058b66ac0 100644
--- a/internal/pkg/controllers/profile/profile.go
+++ b/internal/pkg/controllers/seccompprofile/seccompprofile.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package profile
+package seccompprofile
 
 import (
 	"context"
diff --git a/internal/pkg/controllers/spod/bindata/spod.go b/internal/pkg/controllers/spod/bindata/spod.go
index b3dc87022..ad1644d11 100644
--- a/internal/pkg/controllers/spod/bindata/spod.go
+++ b/internal/pkg/controllers/spod/bindata/spod.go
@@ -202,8 +202,8 @@ var Manifest = &appsv1.DaemonSet{
 								MountPath: SelinuxdSocketDir,
 							},
 							{
-								Name:      "host-seccomp-hook-output-volume",
-								MountPath: config.SeccompProfileRecordOutputPath,
+								Name:      "profile-recording-output-volume",
+								MountPath: config.ProfileRecordingOutputPath,
 							},
 						},
 						SecurityContext: &v1.SecurityContext{
@@ -380,10 +380,10 @@ var Manifest = &appsv1.DaemonSet{
 						},
 					},
 					{
-						Name: "host-seccomp-hook-output-volume",
+						Name: "profile-recording-output-volume",
 						VolumeSource: v1.VolumeSource{
 							HostPath: &v1.HostPathVolumeSource{
-								Path: config.SeccompProfileRecordOutputPath,
+								Path: config.ProfileRecordingOutputPath,
 								Type: &hostPathDirectoryOrCreate,
 							},
 						},
diff --git a/internal/pkg/webhooks/binding/webhook.go b/internal/pkg/webhooks/binding/binding.go
similarity index 100%
rename from internal/pkg/webhooks/binding/webhook.go
rename to internal/pkg/webhooks/binding/binding.go
diff --git a/internal/pkg/webhooks/binding/webhook_test.go b/internal/pkg/webhooks/binding/binding_test.go
similarity index 100%
rename from internal/pkg/webhooks/binding/webhook_test.go
rename to internal/pkg/webhooks/binding/binding_test.go
diff --git a/internal/pkg/webhooks/recording/webhook.go b/internal/pkg/webhooks/recording/recording.go
similarity index 97%
rename from internal/pkg/webhooks/recording/webhook.go
rename to internal/pkg/webhooks/recording/recording.go
index 64edf12b4..c787e4b68 100644
--- a/internal/pkg/webhooks/recording/webhook.go
+++ b/internal/pkg/webhooks/recording/recording.go
@@ -83,6 +83,13 @@ func (p *podSeccompRecorder) Handle(
 	items := profileRecordings.Items
 
 	for i := range items {
+		if items[i].Spec.Kind != "SeccompProfile" {
+			log.Info(fmt.Sprintf(
+				"recording kind %s not supported", items[i].Spec.Kind,
+			))
+			continue
+		}
+
 		selector, err := metav1.LabelSelectorAsSelector(
 			&items[i].Spec.PodSelector,
 		)
@@ -132,7 +139,7 @@ func (p *podSeccompRecorder) addAnnotation(
 
 	targetValue := fmt.Sprintf(
 		"of:%s/%s-%d.json",
-		config.SeccompProfileRecordOutputPath,
+		config.ProfileRecordingOutputPath,
 		profileRecording.GetName(),
 		time.Now().Unix(),
 	)


Test patch:
diff --git a/internal/pkg/controllers/profile/profile_test.go b/internal/pkg/controllers/seccompprofile/seccompprofile_test.go
similarity index 99%
rename from internal/pkg/controllers/profile/profile_test.go
rename to internal/pkg/controllers/seccompprofile/seccompprofile_test.go
index f17e4531b..81d158a63 100644
--- a/internal/pkg/controllers/profile/profile_test.go
+++ b/internal/pkg/controllers/seccompprofile/seccompprofile_test.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package profile
+package seccompprofile
 
 import (
 	"io/ioutil"
diff --git a/test/tc_default_profiles_test.go b/test/tc_default_profiles_test.go
index 9aa791484..909d321e0 100644
--- a/test/tc_default_profiles_test.go
+++ b/test/tc_default_profiles_test.go
@@ -24,7 +24,7 @@ import (
 
 	"sigs.k8s.io/security-profiles-operator/api/seccompprofile/v1alpha1"
 	"sigs.k8s.io/security-profiles-operator/internal/pkg/config"
-	"sigs.k8s.io/security-profiles-operator/internal/pkg/controllers/profile"
+	"sigs.k8s.io/security-profiles-operator/internal/pkg/controllers/seccompprofile"
 )
 
 func (e *e2e) testCaseDefaultAndExampleProfiles(nodes []string) {
@@ -86,13 +86,13 @@ func (e *e2e) verifyBaseProfileContent(node string, cm *v1.ConfigMap) {
 func (e *e2e) verifyCRDProfileContent(node string, sp *v1alpha1.SeccompProfile) {
 	e.logf("Verifying %s profile on node %s", sp.Name, node)
 	name := sp.Name
-	profilePath, err := profile.GetProfilePath(name, sp.ObjectMeta.Namespace, sp.Spec.TargetWorkload)
+	profilePath, err := seccompprofile.GetProfilePath(name, sp.ObjectMeta.Namespace, sp.Spec.TargetWorkload)
 	e.Nil(err)
 	catOutput := e.execNode(node, "cat", profilePath)
-	output := profile.OutputProfile{}
+	output := seccompprofile.OutputProfile{}
 	err = json.Unmarshal([]byte(catOutput), &output)
 	e.Nil(err)
-	expected := profile.OutputProfile{}
+	expected := seccompprofile.OutputProfile{}
 	spec, err := json.Marshal(sp.Spec)
 	e.Nil(err)
 	err = json.Unmarshal(spec, &expected)
diff --git a/test/tc_delete_profiles_test.go b/test/tc_delete_profiles_test.go
index 823f88510..db74ff815 100644
--- a/test/tc_delete_profiles_test.go
+++ b/test/tc_delete_profiles_test.go
@@ -20,7 +20,7 @@ import (
 	"fmt"
 	"time"
 
-	"sigs.k8s.io/security-profiles-operator/internal/pkg/controllers/profile"
+	"sigs.k8s.io/security-profiles-operator/internal/pkg/controllers/seccompprofile"
 )
 
 func (e *e2e) testCaseDeleteProfiles(nodes []string) {
@@ -100,7 +100,7 @@ spec:
 
 	namespace := e.getCurrentContextNamespace(defaultNamespace)
 	sp := e.getSeccompProfile(deleteProfileName, namespace)
-	path, err := profile.GetProfilePath(deleteProfileName, sp.ObjectMeta.Namespace, sp.Spec.TargetWorkload)
+	path, err := seccompprofile.GetProfilePath(deleteProfileName, sp.ObjectMeta.Namespace, sp.Spec.TargetWorkload)
 	e.Nil(err)
 
 	e.logf("Verifying profile exists")