diff --git a/api/profilerecording/v1alpha1/profilerecording_types.go b/api/profilerecording/v1alpha1/profilerecording_types.go index e8d1219380..85fe2b419a 100644 --- a/api/profilerecording/v1alpha1/profilerecording_types.go +++ b/api/profilerecording/v1alpha1/profilerecording_types.go @@ -21,6 +21,7 @@ import ( "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilrand "k8s.io/apimachinery/pkg/util/rand" "sigs.k8s.io/security-profiles-operator/internal/pkg/config" ) @@ -79,17 +80,12 @@ type ProfileRecording struct { Status ProfileRecordingStatus `json:"status,omitempty"` } -func (pr *ProfileRecording) CtrAnnotation(replica, ctrName string) (key, value string, err error) { - ctrReplicaName := ctrName - if replica != "" { - ctrReplicaName += replica - } - +func (pr *ProfileRecording) CtrAnnotation(ctrName string) (key, value string, err error) { switch pr.Spec.Kind { case ProfileRecordingKindSeccompProfile: - return pr.ctrAnnotationSeccomp(ctrReplicaName, ctrName) + return pr.ctrAnnotationSeccomp(ctrName) case ProfileRecordingKindSelinuxProfile: - return pr.ctrAnnotationSelinux(ctrReplicaName, ctrName) + return pr.ctrAnnotationSelinux(ctrName) } return "", "", fmt.Errorf( @@ -105,28 +101,26 @@ func (pr *ProfileRecording) IsKindSupported() bool { return false } -func (pr *ProfileRecording) ctrAnnotationSeccomp(ctrReplicaName, ctrName string) (key, value string, err error) { +func (pr *ProfileRecording) ctrAnnotationValue(ctrName string) string { + const nonceSize = 5 + + return fmt.Sprintf( + "%s_%s_%s_%d", + pr.GetName(), + ctrName, + utilrand.String(nonceSize), + time.Now().Unix(), + ) +} + +func (pr *ProfileRecording) ctrAnnotationSeccomp(ctrName string) (key, value string, err error) { var annotationPrefix string switch pr.Spec.Recorder { case ProfileRecorderLogs: annotationPrefix = config.SeccompProfileRecordLogsAnnotationKey - value = fmt.Sprintf( - "%s-%s-%d", - pr.GetName(), - ctrReplicaName, - time.Now().Unix(), - ) - case ProfileRecorderBpf: annotationPrefix = config.SeccompProfileRecordBpfAnnotationKey - value = fmt.Sprintf( - "%s-%s-%d", - pr.GetName(), - ctrReplicaName, - time.Now().Unix(), - ) - default: return "", "", fmt.Errorf( "invalid recorder: %s", pr.Spec.Recorder, @@ -134,22 +128,16 @@ func (pr *ProfileRecording) ctrAnnotationSeccomp(ctrReplicaName, ctrName string) } key = annotationPrefix + ctrName + value = pr.ctrAnnotationValue(ctrName) return key, value, err } -func (pr *ProfileRecording) ctrAnnotationSelinux(ctrReplicaName, ctrName string) (key, value string, err error) { +func (pr *ProfileRecording) ctrAnnotationSelinux(ctrName string) (key, value string, err error) { var annotationPrefix string switch pr.Spec.Recorder { case ProfileRecorderLogs: annotationPrefix = config.SelinuxProfileRecordLogsAnnotationKey - value = fmt.Sprintf( - "%s-%s-%d", - pr.GetName(), - ctrReplicaName, - time.Now().Unix(), - ) - case ProfileRecorderBpf: default: return "", "", fmt.Errorf( @@ -157,6 +145,7 @@ func (pr *ProfileRecording) ctrAnnotationSelinux(ctrReplicaName, ctrName string) ) } + value = pr.ctrAnnotationValue(ctrName) key = annotationPrefix + ctrName return } diff --git a/internal/pkg/daemon/profilerecorder/impl.go b/internal/pkg/daemon/profilerecorder/impl.go index 44e44ec56a..090c08e963 100644 --- a/internal/pkg/daemon/profilerecorder/impl.go +++ b/internal/pkg/daemon/profilerecorder/impl.go @@ -18,6 +18,7 @@ package profilerecorder import ( "context" + "github.com/containers/common/pkg/seccomp" "github.com/crossplane/crossplane-runtime/pkg/resource" "google.golang.org/grpc" diff --git a/internal/pkg/daemon/profilerecorder/profilerecorder.go b/internal/pkg/daemon/profilerecorder/profilerecorder.go index e798a70e0c..88b4d3cfdf 100644 --- a/internal/pkg/daemon/profilerecorder/profilerecorder.go +++ b/internal/pkg/daemon/profilerecorder/profilerecorder.go @@ -90,6 +90,7 @@ type profileToCollect struct { } type podToWatch struct { + baseName types.NamespacedName recorder profilerecording1alpha1.ProfileRecorder profiles []profileToCollect } @@ -257,9 +258,16 @@ func (r *RecorderReconciler) Reconcile(_ context.Context, req reconcile.Request) logger.Info("Recording profile", "kind", prf.kind, "name", prf.name, "pod", req.NamespacedName.String()) } + // for pods managed by a replicated controller, let's store the replicated + // name so that we can later strip the suffix from the fully-generated pod name + baseName := req.NamespacedName + if pod.GenerateName != "" { + baseName.Name = pod.GenerateName + } + r.podsToWatch.Store( req.NamespacedName.String(), - podToWatch{recorder, profiles}, + podToWatch{baseName, recorder, profiles}, ) r.record.Event(pod, event.Normal(reasonProfileRecording, "Recording profiles")) } @@ -322,9 +330,9 @@ func (r *RecorderReconciler) stopBpfRecorder() error { } func (r *RecorderReconciler) collectProfile( - ctx context.Context, name types.NamespacedName, + ctx context.Context, podName types.NamespacedName, ) error { - n := name.String() + n := podName.String() value, ok := r.podsToWatch.Load(n) if !ok { @@ -336,9 +344,15 @@ func (r *RecorderReconciler) collectProfile( return errors.New("type assert pod to watch") } + replicaSuffix := "" + if podToWatch.baseName.Name != podName.Name && strings.HasPrefix(podName.Name, podToWatch.baseName.Name) { + // this is a replica, we need to strip the suffix from the pod name + replicaSuffix = strings.TrimPrefix(podName.Name, podToWatch.baseName.Name) + } + if podToWatch.recorder == profilerecording1alpha1.ProfileRecorderLogs { if err := r.collectLogProfiles( - ctx, name, podToWatch.profiles, + ctx, replicaSuffix, podName, podToWatch.profiles, ); err != nil { return fmt.Errorf("collect log profile: %w", err) } @@ -346,7 +360,7 @@ func (r *RecorderReconciler) collectProfile( if podToWatch.recorder == profilerecording1alpha1.ProfileRecorderBpf { if err := r.collectBpfProfiles( - ctx, name, podToWatch.profiles, + ctx, replicaSuffix, podName, podToWatch.profiles, ); err != nil { return fmt.Errorf("collect bpf profile: %w", err) } @@ -358,7 +372,8 @@ func (r *RecorderReconciler) collectProfile( func (r *RecorderReconciler) collectLogProfiles( ctx context.Context, - name types.NamespacedName, + replicaSuffix string, + podName types.NamespacedName, profiles []profileToCollect, ) error { r.log.Info("Checking checking if enricher is enabled") @@ -381,19 +396,18 @@ func (r *RecorderReconciler) collectLogProfiles( enricherClient := enricherapi.NewEnricherClient(conn) for _, prf := range profiles { - // Remove the timestamp - profileName, err := extractProfileName(prf.name) + profileNamespacedName, err := profileIDToName(replicaSuffix, podName.Namespace, prf.name) if err != nil { - return fmt.Errorf("extract profile name: %w", err) + return fmt.Errorf("converting profile id to name: %w", err) } - r.log.Info("Collecting profile", "name", profileName, "kind", prf.kind) + r.log.Info("Collecting profile", "name", profileNamespacedName, "kind", prf.kind) switch prf.kind { case profilerecording1alpha1.ProfileRecordingKindSeccompProfile: - err = r.collectLogSeccompProfile(ctx, enricherClient, profileName, name.Namespace, prf.name) + err = r.collectLogSeccompProfile(ctx, enricherClient, profileNamespacedName, prf.name) case profilerecording1alpha1.ProfileRecordingKindSelinuxProfile: - err = r.collectLogSelinuxProfile(ctx, enricherClient, profileName, name.Namespace, prf.name) + err = r.collectLogSelinuxProfile(ctx, enricherClient, profileNamespacedName, prf.name) default: err = fmt.Errorf("unrecognized kind %s", prf.kind) } @@ -409,8 +423,7 @@ func (r *RecorderReconciler) collectLogProfiles( func (r *RecorderReconciler) collectLogSeccompProfile( ctx context.Context, enricherClient enricherapi.EnricherClient, - profileName string, - namespace string, + profileNamespacedName types.NamespacedName, profileID string, ) error { // Retrieve the syscalls for the recording @@ -436,8 +449,8 @@ func (r *RecorderReconciler) collectLogSeccompProfile( profile := &seccompprofileapi.SeccompProfile{ ObjectMeta: metav1.ObjectMeta{ - Name: profileName, - Namespace: namespace, + Name: profileNamespacedName.Name, + Namespace: profileNamespacedName.Namespace, }, Spec: profileSpec, } @@ -454,7 +467,7 @@ func (r *RecorderReconciler) collectLogSeccompProfile( return fmt.Errorf("create seccompProfile resource: %w", err) } - r.log.Info("Created/updated profile", "action", res, "name", profileName) + r.log.Info("Created/updated profile", "action", res, "name", profileNamespacedName.Name) r.record.Event( profile, event.Normal(reasonProfileCreated, "seccomp profile created"), @@ -471,8 +484,7 @@ func (r *RecorderReconciler) collectLogSeccompProfile( func (r *RecorderReconciler) collectLogSelinuxProfile( ctx context.Context, enricherClient enricherapi.EnricherClient, - profileName string, - namespace string, + profileNamespacedName types.NamespacedName, profileID string, ) error { // Retrieve the syscalls for the recording @@ -493,8 +505,8 @@ func (r *RecorderReconciler) collectLogSelinuxProfile( profile := &selxv1alpha2.SelinuxProfile{ ObjectMeta: metav1.ObjectMeta{ - Name: profileName, - Namespace: namespace, + Name: profileNamespacedName.Name, + Namespace: profileNamespacedName.Namespace, }, Spec: selinuxProfileSpec, } @@ -518,7 +530,7 @@ func (r *RecorderReconciler) collectLogSelinuxProfile( r.record.Event(profile, event.Warning(reasonProfileCreationFailed, err)) return fmt.Errorf("create selinuxprofile resource: %w", err) } - r.log.Info("Created/updated selinux profile", "action", res, "name", profileName) + r.log.Info("Created/updated selinux profile", "action", res, "name", profileNamespacedName) r.record.Event( profile, event.Normal(reasonProfileCreated, "selinuxprofile profile created"), @@ -526,7 +538,7 @@ func (r *RecorderReconciler) collectLogSelinuxProfile( // Reset the selinuxprofile for further recordings if err := r.ResetAvcs(ctx, enricherClient, request); err != nil { - return fmt.Errorf("reset selinuxprofile for profile %s: %w", profileName, err) + return fmt.Errorf("reset selinuxprofile for profile %s: %w", profileNamespacedName, err) } return nil @@ -552,7 +564,8 @@ func (r *RecorderReconciler) formatSelinuxProfile( func (r *RecorderReconciler) collectBpfProfiles( ctx context.Context, - name types.NamespacedName, + replicaSuffix string, + podName types.NamespacedName, profiles []profileToCollect, ) error { recorderClient, cancel, err := r.getBpfRecorderClient() @@ -593,15 +606,15 @@ func (r *RecorderReconciler) collectBpfProfiles( } // Remove the timestamp - profileName, err := extractProfileName(profile.name) + profileNamespacedName, err := profileIDToName(replicaSuffix, podName.Namespace, profile.name) if err != nil { - return fmt.Errorf("extract profile name: %w", err) + return fmt.Errorf("converting profile id to name: %w", err) } profile := &seccompprofileapi.SeccompProfile{ ObjectMeta: metav1.ObjectMeta{ - Name: profileName, - Namespace: name.Namespace, + Name: profileNamespacedName.Name, + Namespace: profileNamespacedName.Namespace, }, Spec: profileSpec, } @@ -618,7 +631,7 @@ func (r *RecorderReconciler) collectBpfProfiles( return fmt.Errorf("create seccompProfile resource: %w", err) } - r.log.Info("Created/updated profile", "action", res, "name", profileName) + r.log.Info("Created/updated profile", "action", res, "name", profileNamespacedName) r.record.Event( profile, event.Normal(reasonProfileCreated, "seccomp profile created"), @@ -632,12 +645,49 @@ func (r *RecorderReconciler) collectBpfProfiles( return nil } -func extractProfileName(s string) (string, error) { - lastIndex := strings.LastIndex(s, "-") - if lastIndex == -1 { - return "", fmt.Errorf("malformed profile path: %s", s) +type parsedAnnotation struct { + profileName string + cntName string + nonce string + timestamp string +} + +func parseProfileAnnotation(annotation string) (*parsedAnnotation, error) { + const expectedParts = 4 + + parts := strings.Split(annotation, "_") + if len(parts) != expectedParts { + return nil, + fmt.Errorf("invalid annotation: %s, expected %d parts got %d", annotation, expectedParts, len(parts)) } - return s[:lastIndex], nil + + return &parsedAnnotation{ + profileName: parts[0], + cntName: parts[1], + nonce: parts[2], // unused for now, but we might need it in the future + timestamp: parts[3], // unused for now, but let's keep it for future use + }, nil +} + +func createProfileName(cntName, replicaSuffix, namespace, profileName string) types.NamespacedName { + name := fmt.Sprintf("%s-%s", profileName, cntName) + if replicaSuffix != "" { + name = fmt.Sprintf("%s-%s", name, replicaSuffix) + } + + return types.NamespacedName{ + Name: name, + Namespace: namespace, + } +} + +func profileIDToName(replicaSuffix, namespace, profileID string) (types.NamespacedName, error) { + parsedProfileName, err := parseProfileAnnotation(profileID) + if err != nil { + return types.NamespacedName{}, fmt.Errorf("parse profile raw annotation: %w", err) + } + + return createProfileName(parsedProfileName.cntName, replicaSuffix, namespace, parsedProfileName.profileName), nil } // parseLogAnnotations parses the provided annotations and extracts the diff --git a/internal/pkg/daemon/profilerecorder/profilerecorder_test.go b/internal/pkg/daemon/profilerecorder/profilerecorder_test.go index 17c12e4a4f..362a5dd9e8 100644 --- a/internal/pkg/daemon/profilerecorder/profilerecorder_test.go +++ b/internal/pkg/daemon/profilerecorder/profilerecorder_test.go @@ -194,7 +194,7 @@ func TestReconcile(t *testing.T) { }, { // BPF success collect prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_4bbwm_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderBpf, profiles: []profileToCollect{ @@ -242,7 +242,7 @@ func TestReconcile(t *testing.T) { { //nolint:dupl // test duplicates are fine // BPF GoArchToSeccompArch fails prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderBpf, profiles: []profileToCollect{ @@ -281,7 +281,7 @@ func TestReconcile(t *testing.T) { { //nolint:dupl // test duplicates are fine // BPF CreateOrUpdate fails prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderBpf, profiles: []profileToCollect{ @@ -320,7 +320,7 @@ func TestReconcile(t *testing.T) { { //nolint:dupl // test duplicates are fine // BPF DialBpfRecorder fails prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderBpf, profiles: []profileToCollect{ @@ -351,7 +351,7 @@ func TestReconcile(t *testing.T) { }, { // BPF DialBpfRecorder fails on StopBpfRecorder prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderBpf, profiles: []profileToCollect{ @@ -427,7 +427,7 @@ func TestReconcile(t *testing.T) { }, { // BPF SyscallsForProfile returns not found prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderBpf, profiles: []profileToCollect{ @@ -461,7 +461,7 @@ func TestReconcile(t *testing.T) { { //nolint:dupl // test duplicates are fine // BPF SyscallsForProfile fails prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderBpf, profiles: []profileToCollect{ @@ -658,7 +658,7 @@ func TestReconcile(t *testing.T) { }, { // logs seccomp success collect prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_4bbwm_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -702,7 +702,7 @@ func TestReconcile(t *testing.T) { }, { // logs seccomp failed ResetSyscalls prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -738,7 +738,7 @@ func TestReconcile(t *testing.T) { { //nolint:dupl // test duplicates are fine // logs seccomp failed CreateOrUpdate prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -774,7 +774,7 @@ func TestReconcile(t *testing.T) { { //nolint:dupl // test duplicates are fine // logs seccomp failed GoArchToSeccompArch prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -810,7 +810,7 @@ func TestReconcile(t *testing.T) { { //nolint:dupl // test duplicates are fine // logs seccomp failed Syscalls prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -842,7 +842,7 @@ func TestReconcile(t *testing.T) { }, { // logs seccomp failed unknown kind prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -906,7 +906,7 @@ func TestReconcile(t *testing.T) { { //nolint:dupl // test duplicates are fine // logs seccomp DialEnricher fails prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -937,7 +937,7 @@ func TestReconcile(t *testing.T) { }, { // logs seccomp EnableLogEnricher false prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -967,7 +967,7 @@ func TestReconcile(t *testing.T) { }, { // logs seccomp failed GetSPOD prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -995,7 +995,7 @@ func TestReconcile(t *testing.T) { }, { // logs selinux success collect prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_4bbwm_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -1044,7 +1044,7 @@ func TestReconcile(t *testing.T) { }, { // logs selinux failed ResetAvcs prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -1076,7 +1076,7 @@ func TestReconcile(t *testing.T) { }, { // logs selinux failed CreateOrUpdate prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -1108,7 +1108,7 @@ func TestReconcile(t *testing.T) { }, { // logs selinux failed formatSelinuxProfile prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ @@ -1145,7 +1145,7 @@ func TestReconcile(t *testing.T) { { //nolint:dupl // test duplicates are fine // logs selinux failed Avcs prepare: func(sut *RecorderReconciler, mock *profilerecorderfakes.FakeImpl) { - profileName := fmt.Sprintf("profile-%d", time.Now().Unix()) + profileName := fmt.Sprintf("profile_replica-123_%d", time.Now().Unix()) value := podToWatch{ recorder: recordingapi.ProfileRecorderLogs, profiles: []profileToCollect{ diff --git a/internal/pkg/webhooks/recording/recording.go b/internal/pkg/webhooks/recording/recording.go index 6317c46b85..679eba46aa 100644 --- a/internal/pkg/webhooks/recording/recording.go +++ b/internal/pkg/webhooks/recording/recording.go @@ -22,8 +22,6 @@ import ( "errors" "fmt" "net/http" - "strings" - "sync" "github.com/go-logr/logr" admissionv1 "k8s.io/api/admission/v1" @@ -46,8 +44,7 @@ const finalizer = "active-seccomp-profile-recording-lock" type podSeccompRecorder struct { impl - log logr.Logger - replicas sync.Map + log logr.Logger } func RegisterWebhook(server *webhook.Server, c client.Client) { @@ -189,16 +186,10 @@ func (p *podSeccompRecorder) updatePod( } } - // Handle replicas by tracking them - replica, err := p.getReplica(pod, profileRecording) - if err != nil { - return false, err - } - for i := range ctrs { ctr := ctrs[i] - key, value, err := profileRecording.CtrAnnotation(replica, ctr.Name) + key, value, err := profileRecording.CtrAnnotation(ctr.Name) if err != nil { return false, err } @@ -290,46 +281,6 @@ func (p *podSeccompRecorder) updateSelinuxSecurityContext( ctr.SecurityContext.SELinuxOptions.Type = config.SelinuxPermissiveProfile } -func replicaKey(recordingName, podName string) string { - return fmt.Sprintf("%s/%s", recordingName, podName) -} - -func (p *podSeccompRecorder) cleanupReplicas(recordingName, podName string) { - rKey := replicaKey(recordingName, podName) - - p.replicas.Range(func(key, _ interface{}) bool { - keyString, ok := key.(string) - if !ok { - return false - } - if strings.HasPrefix(rKey, keyString) { - p.replicas.Delete(key) - return false - } - return true - }) -} - -func (p *podSeccompRecorder) getReplica( - pod *corev1.Pod, - profileRecording *profilerecordingv1alpha1.ProfileRecording, -) (string, error) { - replica := "" - if pod.Name == "" && pod.GenerateName != "" { - rKey := replicaKey(profileRecording.Name, pod.GenerateName) - - v, _ := p.replicas.LoadOrStore(rKey, uint(0)) - replica = fmt.Sprintf("-%d", v) - vUint, ok := v.(uint) - if !ok { - return "", errors.New("replicas value is not an uint") - } - p.replicas.Store(rKey, vUint+1) - } - - return replica, nil -} - func (p *podSeccompRecorder) setRecordingReferences( ctx context.Context, op admissionv1.Operation, @@ -401,7 +352,6 @@ func (p *podSeccompRecorder) setFinalizers( } else { if controllerutil.ContainsFinalizer(profileRecording, finalizer) { controllerutil.RemoveFinalizer(profileRecording, finalizer) - p.cleanupReplicas(profileRecording.Name, podName) } } diff --git a/test/tc_bpf_recorder_test.go b/test/tc_bpf_recorder_test.go index c5120d7daa..35faf5eb5c 100644 --- a/test/tc_bpf_recorder_test.go +++ b/test/tc_bpf_recorder_test.go @@ -92,11 +92,14 @@ func (e *e2e) testCaseBpfRecorderStaticPod() { e.kubectl("delete", "sp", resourceName) metrics := e.getSpodMetrics() + // we don't use resource name here, because the metrics are tracked by the annotation name which contains + // underscores instead of dashes + metricName := recordingName + "_nginx" e.Regexp(fmt.Sprintf(`(?m)security_profiles_operator_seccomp_profile_bpf_total{`+ `mount_namespace=".*",`+ `node=".*",`+ - `profile="%s-.*"} \d+`, - resourceName, + `profile="%s_.*"} \d+`, + metricName, ), metrics) } @@ -170,9 +173,12 @@ spec: e.retryGet("deploy", deployName) e.waitFor("condition=available", "deploy", deployName) + suffixes := e.getPodSuffixesByLabel("app=alpine") + e.Len(suffixes, 2) + since := time.Now() - const profileName0 = recordingName + "-nginx-0" - const profileName1 = recordingName + "-nginx-1" + profileName0 := recordingName + "-nginx-" + suffixes[0] + profileName1 := recordingName + "-nginx-" + suffixes[1] e.waitForBpfRecorderLogs(since, profileName0, profileName1) e.kubectl("delete", "deploy", deployName) diff --git a/test/tc_profilerecordings_test.go b/test/tc_profilerecordings_test.go index f7e50d35d4..a04372db23 100644 --- a/test/tc_profilerecordings_test.go +++ b/test/tc_profilerecordings_test.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "regexp" + "strings" "time" spoutil "sigs.k8s.io/security-profiles-operator/internal/pkg/util" @@ -322,17 +323,17 @@ func (e *e2e) profileRecordingDeployment( e.waitForEnricherLogs(since, waitConditions...) } + suffixes := e.getPodSuffixesByLabel("app=alpine") e.kubectl("delete", "deploy", deployName) - const profileName0 = recordingName + "-nginx-0" - const profileName1 = recordingName + "-nginx-1" - profile0 := e.retryGetSeccompProfile(profileName0) - profile1 := e.retryGetSeccompProfile(profileName1) - e.Contains(profile0, "listen") - e.Contains(profile1, "listen") + for _, sfx := range suffixes { + recordedProfileName := recordingName + "-nginx-" + sfx + profile := e.retryGetSeccompProfile(recordedProfileName) + e.Contains(profile, "listen") + e.kubectl("delete", "sp", recordedProfileName) + } e.kubectl("delete", "-f", recording) - e.kubectl("delete", "sp", profileName0, profileName1) } func (e *e2e) profileRecordingSelinuxDeployment( @@ -346,24 +347,24 @@ func (e *e2e) profileRecordingSelinuxDeployment( e.waitForEnricherLogs(since, waitConditions...) } + suffixes := e.getPodSuffixesByLabel("app=alpine") e.kubectl("delete", "deploy", deployName) - const profileName0 = selinuxRecordingName + "-nginx-0" - const profileName1 = selinuxRecordingName + "-nginx-1" - - profile0result := e.retryGetSelinuxJsonpath("{.spec.allow.http_cache_port_t.tcp_socket}", profileName0) - e.Contains(profile0result, "name_bind") + fmt.Println(e.kubectl("get", "sp")) - profile1result := e.retryGetSelinuxJsonpath("{.spec.allow.http_cache_port_t.tcp_socket}", profileName1) - e.Contains(profile1result, "name_bind") + for _, sfx := range suffixes { + recordedProfileName := selinuxRecordingName + "-nginx-" + sfx + profileResult := e.retryGetSelinuxJsonpath("{.spec.allow.http_cache_port_t.tcp_socket}", recordedProfileName) + e.Contains(profileResult, "name_bind") + e.kubectl("delete", "selinuxprofile", recordedProfileName) + } e.kubectl("delete", "-f", recording) - e.kubectl("delete", "selinuxprofile", profileName0, profileName1) } -func (e *e2e) createRecordingTestDeployment() (since time.Time, podName string) { +func (e *e2e) createRecordingTestDeployment() (since time.Time, deployName string) { e.logf("Creating test deployment") - podName = "my-deployment" + deployName = "my-deployment" e.setupRecordingSa(e.getCurrentContextNamespace(defaultNamespace)) @@ -405,12 +406,11 @@ spec: since = time.Now() e.kubectl("create", "-f", testFile.Name()) - const deployName = "my-deployment" e.retryGet("deploy", deployName) e.waitFor("condition=available", "deploy", deployName) e.Nil(os.Remove(testFile.Name())) - return since, podName + return since, deployName } func (e *e2e) retryGetProfile(kind string, args ...string) string { @@ -538,11 +538,12 @@ func (e *e2e) profileRecordingScaleDeployment( // wait for the pods to be ready as per the readinessProbe e.kubectl("rollout", "status", "deploy", deployName) + suffixes := e.getPodSuffixesByLabel("app=alpine") e.kubectl("delete", "deploy", deployName) // check the expected number of policies was created - for i := 0; i < 5; i++ { - recordedProfileName := fmt.Sprintf("%s-nginx-%d", recordingName, i) + for _, sfx := range suffixes { + recordedProfileName := recordingName + "-nginx-" + sfx profile := e.retryGetSeccompProfile(recordedProfileName) e.Contains(profile, "listen") e.kubectl("delete", "sp", recordedProfileName) @@ -550,3 +551,15 @@ func (e *e2e) profileRecordingScaleDeployment( e.kubectl("delete", "-f", recording) } + +func (e *e2e) getPodSuffixesByLabel(label string) []string { //nolint:unparam // it's better to keep the param around + suffixes := make([]string, 0) + podNamesString := e.kubectl("get", "pods", "-l", label, "-o", "jsonpath={.items[*].metadata.name}") + podNames := strings.Fields(podNamesString) + for _, podName := range podNames { + suffixIdx := strings.LastIndex(podName, "-") + suffixes = append(suffixes, podName[suffixIdx+1:]) + } + + return suffixes +}