Repository: kubernetes-sigs/security-profiles-operator
Pull request #1290 (2022-11-10 15:24:50)

Add command argument to the daemon which allows to disable profile recorder controller
<!--  Thanks for sending a pull request!  Here are some tips for you:

- If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/sig-release/release.md#issuepr-kind-label
- If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
- If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

#### What type of PR is this?

<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind design

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
-->
/kind feature

#### What this PR does / why we need it:

This PR adds an command argument to the daemon which allows to disable the profile recorder controller when neither bpf-recorder or log-enricher are enabled. 

#### 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
-->

#### Does this PR have test?

<!--
If tests aren't applicable just write N/A.
-->

#### Special notes for your reviewer:

#### 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
Add a command argument to the daemon which allows to disable the profile recorder controller.
```


k8s-ci-robot: @ccojocar: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our [release note process](https://git.k8s.io/community/contributors/guide/release-notes.md) to remove it.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
ccojocar: /assign @saschagrunert 
ccojocar: > Would you also agree with enabling or disabling the controller with an environment variable? I'm thinking this way because of OLM installs where it's very easy to pass an environment variable to the operator through a Subscription object.

I'll add an environment variable in addition. What do you think?
ccojocar: > Also two nits - I think this should be a /kind feature and the switch (and the reason for introducing it) should be mentioned in installation-usage.md. Maybe if you think that the other switches should be mentioned as well, we could have a cleanup ticked that would document them all in one go, but currently our docs are not bad and I'd like to keep everything documented nicely.

I'll do this and also try to add a test.
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/1290#" title="Author self-approved">ccojocar</a>*
**Once this PR has been reviewed and has the lgtm label**, please ask for approval from saschagrunert by writing `/assign @saschagrunert` in a comment. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

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).

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

- **[OWNERS](https://github.com/kubernetes-sigs/security-profiles-operator/blob/main/OWNERS)**

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":["saschagrunert"]} -->
codecov-commenter: # [Codecov](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/1290?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#1290](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/pull/1290?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (a34675d) into [main](https://codecov.io/gh/kubernetes-sigs/security-profiles-operator/commit/868709e5dd3cc61e07f19002705d17a380d938a5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (868709e) will **decrease** coverage by `0.03%`.
> The diff coverage is `0.00%`.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1290      +/-   ##
==========================================
- Coverage   43.92%   43.88%   -0.04%     
==========================================
  Files          49       49              
  Lines        5514     5519       +5     
==========================================
  Hits         2422     2422              
- Misses       2972     2977       +5     
  Partials      120      120              
```

</details>
k8s-ci-robot: @ccojocar: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-security-profiles-operator-test-e2e | 6612e6e870fa85e418e00dec36e727efdda431d4 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/1290/pull-security-profiles-operator-test-e2e/1591073029754982400) | true | `/test pull-security-profiles-operator-test-e2e`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=security-profiles-operator&pr=1290). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Accojocar). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/security-profiles-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @ccojocar: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-security-profiles-operator-test-e2e | e4d7cdb5a2aaed0fb078d2d80e3e3c769f781558 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/1290/pull-security-profiles-operator-test-e2e/1591103094979039232) | true | `/test pull-security-profiles-operator-test-e2e`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=security-profiles-operator&pr=1290). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Accojocar). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/security-profiles-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @ccojocar: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-security-profiles-operator-test-e2e | 239a97207bb66dbb5ef9b3a126297756912f2803 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/1290/pull-security-profiles-operator-test-e2e/1592069638458445824) | true | `/test pull-security-profiles-operator-test-e2e`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=security-profiles-operator&pr=1290). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Accojocar). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/security-profiles-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
ccojocar: /test all
k8s-ci-robot: @ccojocar: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-security-profiles-operator-test-e2e | 4c6103525d96b2bec09a591d6b14d26087ab10ad | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/1290/pull-security-profiles-operator-test-e2e/1592107867203178496) | true | `/test pull-security-profiles-operator-test-e2e`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=security-profiles-operator&pr=1290). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Accojocar). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/security-profiles-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @ccojocar: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-security-profiles-operator-test-e2e | 769e20afcacca93e24f993e74422b05884ce53c0 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/1290/pull-security-profiles-operator-test-e2e/1592155382921302016) | true | `/test pull-security-profiles-operator-test-e2e`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=security-profiles-operator&pr=1290). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Accojocar). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/security-profiles-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @ccojocar: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-security-profiles-operator-test-e2e | 42542a558672c4d23258fa5cac1ef0350777df4c | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_security-profiles-operator/1290/pull-security-profiles-operator-test-e2e/1592178897208741888) | true | `/test pull-security-profiles-operator-test-e2e`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=security-profiles-operator&pr=1290). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Accojocar). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/security-profiles-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
ccojocar: /retest
saschagrunert: /lgtm

Patch:
diff --git a/Makefile b/Makefile
index 5c1273832..3abaa9046 100644
--- a/Makefile
+++ b/Makefile
@@ -468,9 +468,15 @@ OLM_EXAMPLES := \
 
 .PHONY: bundle
 bundle: operator-sdk deployments ## Generate bundle manifests and metadata, then validate generated files.
+ifeq ($(shell uname -s), Darwin)
+	sed -i '' "s/\(olm.skipRange: '>=.*\)<.*'/\1<$(VERSION)'/" deploy/base/clusterserviceversion.yaml
+	sed -i '' "s/\(\"name\": \"security-profiles-operator.v\).*\"/\1$(VERSION)\"/" deploy/catalog-preamble.json
+	sed -i '' "s/\(\"skipRange\": \">=.*\)<.*\"/\1<$(VERSION)\"/" deploy/catalog-preamble.json
+else
 	sed -i "s/\(olm.skipRange: '>=.*\)<.*'/\1<$(VERSION)'/" deploy/base/clusterserviceversion.yaml
 	sed -i "s/\(\"name\": \"security-profiles-operator.v\).*\"/\1$(VERSION)\"/" deploy/catalog-preamble.json
 	sed -i "s/\(\"skipRange\": \">=.*\)<.*\"/\1<$(VERSION)\"/" deploy/catalog-preamble.json
+endif
 	cat $(OLM_EXAMPLES) $(BUNDLE_OPERATOR_MANIFEST) deploy/base/clusterserviceversion.yaml | $(OPERATOR_SDK) generate bundle -q --overwrite --version $(VERSION) $(BUNDLE_METADATA_OPTS)
 	git restore deploy/base/clusterserviceversion.yaml
 	mkdir -p ./bundle/tests/scorecard
diff --git a/cmd/security-profiles-operator/main.go b/cmd/security-profiles-operator/main.go
index 6b4f65072..6e94bc693 100644
--- a/cmd/security-profiles-operator/main.go
+++ b/cmd/security-profiles-operator/main.go
@@ -66,6 +66,7 @@ import (
 
 const (
 	jsonFlag           string = "json"
+	recordingFlag      string = "with-recording"
 	selinuxFlag        string = "with-selinux"
 	apparmorFlag       string = "with-apparmor"
 	webhookFlag        string = "webhook"
@@ -151,6 +152,12 @@ func main() {
 					Usage: "Listen for AppArmor API resources",
 					Value: false,
 				},
+				&cli.BoolFlag{
+					Name:    recordingFlag,
+					Usage:   "Listen for ProfileRecording API recources",
+					Value:   false,
+					EnvVars: []string{config.EnableRecordingEnvKey},
+				},
 			},
 		},
 		&cli.Command{
@@ -379,7 +386,10 @@ func setControllerOptionsForNamespaces(opts *ctrl.Options) {
 func getEnabledControllers(ctx *cli.Context) []controller.Controller {
 	controllers := []controller.Controller{
 		seccompprofile.NewController(),
-		profilerecorder.NewController(),
+	}
+
+	if ctx.Bool(recordingFlag) {
+		controllers = append(controllers, profilerecorder.NewController())
 	}
 
 	if ctx.Bool(selinuxFlag) {
diff --git a/installation-usage.md b/installation-usage.md
index 776f79ac7..9a1b22a3d 100644
--- a/installation-usage.md
+++ b/installation-usage.md
@@ -25,6 +25,7 @@
     - [Log enricher based recording](#log-enricher-based-recording)
     - [eBPF based recording](#ebpf-based-recording)
     - [Merging per-container profile instances](#merging-per-container-profile-instances)
+    - [Disable profile recording](#disable-profile-recording)
 - [Create a SELinux Profile](#create-a-selinux-profile)
   - [Apply a SELinux profile to a pod](#apply-a-selinux-profile-to-a-pod)
   - [Make a SELinux profile permissive](#make-a-selinux-profile-permissive)
@@ -768,6 +769,16 @@ including the `mknod` syscall:
   - mknod
 ```
 
+#### Disable profile recording
+
+Profile recorder controller along with the corresponding sidecar container is disabled
+when neither `enableBpfRecorder` nor `enableLogEnricher` is set in the SPOD configuration, and
+automatically enabled when either one of them is on.
+
+Also, when running the daemon in standalone mode is possible to switch on the profile recorder
+controller by providing the `with-recording` command line argument or setting the `ENABLE_RECORDING`
+environment variable.
+
 ## Create a SELinux Profile
 
 There are two kinds that can be used to define a SELinux profile - `SelinuxProfile` and `RawSelinuxProfile`.
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index d1ab38c83..2f0127869 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -70,6 +70,9 @@ const (
 	// EnableLogEnricherEnvKey is the environment variable key for enabling the log enricher.
 	EnableLogEnricherEnvKey = "ENABLE_LOG_ENRICHER"
 
+	// EnableRecordingEnvKey is the environment variable key to enabeling profile recording.
+	EnableRecordingEnvKey = "ENABLE_RECORDING"
+
 	// VerboseLevel is the increased verbosity log level.
 	VerboseLevel = 1
 
diff --git a/internal/pkg/manager/spod/spod_controller.go b/internal/pkg/manager/spod/spod_controller.go
index b459f954a..467787ddc 100644
--- a/internal/pkg/manager/spod/spod_controller.go
+++ b/internal/pkg/manager/spod/spod_controller.go
@@ -499,6 +499,8 @@ func (r *ReconcileSPOd) getConfiguredSPOd(
 	useCustomHostProc := cfg.Spec.HostProcVolumePath != bindata.DefaultHostProcPath
 	volume, mount := bindata.CustomHostProcVolume(cfg.Spec.HostProcVolumePath)
 
+	// Disable profile recording controller by default
+	enableRecording := false
 	if cfg.Spec.EnableLogEnricher || cfg.Spec.EnableBpfRecorder {
 		if useCustomHostProc {
 			templateSpec.Volumes = append(templateSpec.Volumes, volume)
@@ -507,7 +509,13 @@ func (r *ReconcileSPOd) getConfiguredSPOd(
 		// HostPID is required for the log-enricher and bpf recorder
 		// and is used to access cgroup files to map Process IDs to Pod IDs
 		templateSpec.HostPID = true
+
+		// Enable profile recording controller which is disabled by default
+		enableRecording = true
 	}
+	templateSpec.Containers[bindata.ContainerIDDaemon].Args = append(
+		templateSpec.Containers[bindata.ContainerIDDaemon].Args,
+		fmt.Sprintf("--with-recording=%t", enableRecording))
 
 	// Log enricher parameters. The spod setting takes precedence over the env var.
 	enableLogEnricherEnv, err := strconv.ParseBool(os.Getenv(config.EnableLogEnricherEnvKey))


Test patch:
diff --git a/test/e2e_test.go b/test/e2e_test.go
index 36ef01da0..498abc50b 100644
--- a/test/e2e_test.go
+++ b/test/e2e_test.go
@@ -124,6 +124,10 @@ func (e *e2e) TestSecurityProfilesOperator() {
 			"SPOD: Change webhook config",
 			e.testCaseWebhookOptionsChange,
 		},
+		{
+			"SPOD: Enable profile recorder",
+			e.testCaseSPODEnableProfileRecorder,
+		},
 	}
 	for _, testCase := range testCases {
 		tc := testCase
diff --git a/test/tc_enable_profile_recorder_test.go b/test/tc_enable_profile_recorder_test.go
new file mode 100644
index 000000000..230f15588
--- /dev/null
+++ b/test/tc_enable_profile_recorder_test.go
@@ -0,0 +1,25 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e_test
+
+func (e *e2e) testCaseSPODEnableProfileRecorder(nodes []string) {
+	e.enableLogEnricherInSpod()
+
+	e.logf("assert profile recorder is enabled in the spod DS when log enricher is enabled")
+	profileRecorderEnabledInSPODDS := e.kubectlOperatorNS("get", "ds", "spod", "-o", "yaml")
+	e.Contains(profileRecorderEnabledInSPODDS, "--with-recording=true")
+}