Skip to content

ROX-33477: Add support for tailored profiles to CO scan setting binding creation#19316

Draft
guzalv wants to merge 24 commits intomaster-base/gualvare/ROX-31229-tailored-profiles-trackingfrom
master-base/gualvare/ROX-33477-update-ssb-logic-to-handle-tp
Draft

ROX-33477: Add support for tailored profiles to CO scan setting binding creation#19316
guzalv wants to merge 24 commits intomaster-base/gualvare/ROX-31229-tailored-profiles-trackingfrom
master-base/gualvare/ROX-33477-update-ssb-logic-to-handle-tp

Conversation

@guzalv
Copy link
Contributor

@guzalv guzalv commented Mar 6, 2026

Determine whether profile is tailored by calling k8s api from sensor

Revert "Determine whether profile is tailored by calling k8s api from sensor"

Let's look at a different approach.

This reverts commit 0ed1a0a.

Add protos to pass profile kind from central to sensor

Use correct profile kind when creating scanSettingBinding

Description

change me!

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

change me!

guzalv added 24 commits March 4, 2026 11:41
I had to do:

  docker run --platform linux/amd64 --rm -v "$(pwd):/src" -w /src quay.io/stackrox-io/apollo-ci:stackrox-test-0.5.2 bash -c "rm -rf .gotools && go mod tidy && make proto-generated-srcs"
Extract this to separate PR?
… sensor"

Let's look at a different approach.

This reverts commit 0ed1a0a.
@openshift-ci
Copy link

openshift-ci bot commented Mar 6, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 3 issues, and left some high level feedback:

  • The log.Infof calls inside the profile loops in convertCentralRequestToScanSettingBinding and updateScanSettingBindingFromCentralRequest will emit a log per profile on every apply/update, which may be noisy in large clusters; consider reducing to debug level or aggregating into a single log statement.
  • The logic that builds profileRefs for scan setting bindings is duplicated between convertCentralRequestToScanSettingBinding and updateScanSettingBindingFromCentralRequest; consider extracting a shared helper to keep the legacy/new-path behavior consistent and easier to maintain.
  • In profileKindToString, all unknown or future ComplianceOperatorProfileKind values silently fall back to Profile.Kind; consider handling unexpected enum values explicitly (e.g., via a switch with a default log or error) to make misconfigurations easier to detect.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `log.Infof` calls inside the profile loops in `convertCentralRequestToScanSettingBinding` and `updateScanSettingBindingFromCentralRequest` will emit a log per profile on every apply/update, which may be noisy in large clusters; consider reducing to debug level or aggregating into a single log statement.
- The logic that builds `profileRefs` for scan setting bindings is duplicated between `convertCentralRequestToScanSettingBinding` and `updateScanSettingBindingFromCentralRequest`; consider extracting a shared helper to keep the legacy/new-path behavior consistent and easier to maintain.
- In `profileKindToString`, all unknown or future `ComplianceOperatorProfileKind` values silently fall back to `Profile.Kind`; consider handling unexpected enum values explicitly (e.g., via a `switch` with a default log or error) to make misconfigurations easier to detect.

## Individual Comments

### Comment 1
<location path="sensor/kubernetes/complianceoperator/utils.go" line_range="84-86" />
<code_context>
-		})
+	if refs := request.GetProfileRefs(); len(refs) > 0 {
+		// New Central: kind is provided directly — no k8s API call needed.
+		for _, ref := range refs {
+			kind := profileKindToString(ref.GetKind())
+			log.Infof("Profile %q in namespace %q: kind %q provided by Central", ref.GetName(), namespace, kind)
+			profileRefs = append(profileRefs, v1alpha1.NamedObjectReference{
+				Name:     ref.GetName(),
</code_context>
<issue_to_address>
**suggestion (performance):** Profile-level Info logging may be noisy at scale

For larger clusters, logging at Info for every profile on each scan could overwhelm logs. If this is mainly for debugging or initial verification, consider using Debug level, a verbosity flag, or some rate limiting to reduce log noise.

```suggestion
			kind := profileKindToString(ref.GetKind())
			log.Debugf("Profile %q in namespace %q: kind %q provided by Central", ref.GetName(), namespace, kind)
			profileRefs = append(profileRefs, v1alpha1.NamedObjectReference{
```
</issue_to_address>

### Comment 2
<location path="central/complianceoperator/v2/compliancemanager/manager_impl.go" line_range="290-292" />
<code_context>

+	// Build profile refs from the already-fetched profiles so Sensor can use the correct CO kind
+	// (Profile vs TailoredProfile) without needing to probe the k8s API.
+	profileRefs := make([]*central.ApplyComplianceScanConfigRequest_ProfileReference, 0, len(returnedProfiles))
+	for _, p := range returnedProfiles {
+		profileRefs = append(profileRefs, &central.ApplyComplianceScanConfigRequest_ProfileReference{
+			Name: p.GetName(),
+			Kind: central.ComplianceOperatorProfileV2_ComplianceOperatorProfileKind(p.GetComplianceOperatorKind()),
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Relying on a cast between two enums can be brittle if they diverge

This line relies on `central.ComplianceOperatorProfileV2_ComplianceOperatorProfileKind(p.GetComplianceOperatorKind())` preserving matching numeric values between the datastore and proto enums. If either enum changes independently, the wrong kind could be sent. Prefer an explicit mapping (e.g., a small switch) to make the dependency and failure modes explicit.

Suggested implementation:

```golang
	// Build profile refs from the already-fetched profiles so Sensor can use the correct CO kind
	// (Profile vs TailoredProfile) without needing to probe the k8s API.
	profileRefs := make([]*central.ApplyComplianceScanConfigRequest_ProfileReference, 0, len(returnedProfiles))
	for _, p := range returnedProfiles {
		var kind central.ComplianceOperatorProfileV2_ComplianceOperatorProfileKind

		switch p.GetComplianceOperatorKind() {
		case storage.ComplianceOperatorProfileV2_PROFILE:
			kind = central.ComplianceOperatorProfileV2_PROFILE
		case storage.ComplianceOperatorProfileV2_TAILORED_PROFILE:
			kind = central.ComplianceOperatorProfileV2_TAILORED_PROFILE
		default:
			// Make dependency and failure mode explicit if enums diverge or an unexpected
			// value is stored. Log and skip this profile rather than sending an incorrect kind.
			log.Errorf("Unknown compliance operator profile kind %v for profile %q; skipping profile in scan configuration %q",
				p.GetComplianceOperatorKind(), p.GetName(), scanRequest.GetScanConfigName())
			continue
		}

		profileRefs = append(profileRefs, &central.ApplyComplianceScanConfigRequest_ProfileReference{
			Name: p.GetName(),
			Kind: kind,
		})
	}

```

1. This change assumes that:
   - The datastore enum type used by `p.GetComplianceOperatorKind()` is `storage.ComplianceOperatorProfileV2_ComplianceOperatorProfileKind`.
   - The datastore enum values are `storage.ComplianceOperatorProfileV2_PROFILE` and `storage.ComplianceOperatorProfileV2_TAILORED_PROFILE`.
   - The central proto enum values are `central.ComplianceOperatorProfileV2_PROFILE` and `central.ComplianceOperatorProfileV2_TAILORED_PROFILE`.
   If the actual enum names differ, adjust the `switch` cases and assigned central constants accordingly.
2. Ensure that the `storage` package is imported in this file (it typically is in this codebase). If it is not, add:
   `storage "github.com/stackrox/rox/generated/storage"`
   to the imports block.
</issue_to_address>

### Comment 3
<location path="sensor/kubernetes/complianceoperator/utils.go" line_range="79" />
<code_context>

 func convertCentralRequestToScanSettingBinding(namespace string, request *central.ApplyComplianceScanConfigRequest_BaseScanSettings, _ string) runtime.Object {
 	profileRefs := make([]v1alpha1.NamedObjectReference, 0, len(request.GetProfiles()))
-	for _, profile := range request.GetProfiles() {
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the duplicated profile reference branching and construction logic into a shared helper function used by both callers to simplify and unify the code.

You can reduce the added complexity by extracting the duplicated branching + construction into a small helper that both call sites use.

For example:

```go
func buildProfileRefs(
    request *central.ApplyComplianceScanConfigRequest_BaseScanSettings,
    namespace string,
    action string, // e.g. "create" or "update"
) []v1alpha1.NamedObjectReference {
    refs := make([]v1alpha1.NamedObjectReference, 0, len(request.GetProfiles()))

    if profileRefs := request.GetProfileRefs(); len(profileRefs) > 0 {
        // New Central: kind is provided directly — no k8s API call needed.
        for _, ref := range profileRefs {
            kind := profileKindToString(ref.GetKind())
            log.Infof("Profile %q in namespace %q: kind %q provided by Central (%s)", ref.GetName(), namespace, kind, action)
            refs = append(refs, v1alpha1.NamedObjectReference{
                Name:     ref.GetName(),
                Kind:     kind,
                APIGroup: complianceoperator.GetGroupVersion().String(),
            })
        }
        return refs
    }

    // Legacy: old Central without profile_refs — default to Profile kind.
    for _, profile := range request.GetProfiles() {
        refs = append(refs, v1alpha1.NamedObjectReference{
            Name:     profile,
            Kind:     complianceoperator.Profile.Kind,
            APIGroup: complianceoperator.GetGroupVersion().String(),
        })
    }

    return refs
}
```

Then both callers become simpler and consistent:

```go
func convertCentralRequestToScanSettingBinding(namespace string, request *central.ApplyComplianceScanConfigRequest_BaseScanSettings, _ string) runtime.Object {
    profileRefs := buildProfileRefs(request, namespace, "create")

    return &v1alpha1.ScanSettingBinding{
        TypeMeta: v1.TypeMeta{
            Kind:       complianceoperator.ScanSettingBinding.Kind,
            APIVersion: complianceoperator.GetGroupVersion().String(),
        },
        ObjectMeta: v1.ObjectMeta{
            Name:        request.GetScanName(),
            Namespace:   namespace,
            Labels:      stackroxLabels,
            Annotations: map[string]string{},
        },
        Profiles: profileRefs,
        ScanSettingsName: request.GetScanSettingsName(),
    }
}
```

```go
func updateScanSettingBindingFromCentralRequest(scanSettingBinding *v1alpha1.ScanSettingBinding, request *central.ApplyComplianceScanConfigRequest_BaseScanSettings) *v1alpha1.ScanSettingBinding {
    ns := scanSettingBinding.GetNamespace()
    scanSettingBinding.Profiles = buildProfileRefs(request, ns, "update")
    // TODO:  Update additional fields as ACS capability expands
    return scanSettingBinding
}
```

This keeps all current behavior, logging, and backward compatibility, while centralizing the branching and `NamedObjectReference` construction in one place.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +84 to +86
kind := profileKindToString(ref.GetKind())
log.Infof("Profile %q in namespace %q: kind %q provided by Central", ref.GetName(), namespace, kind)
profileRefs = append(profileRefs, v1alpha1.NamedObjectReference{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (performance): Profile-level Info logging may be noisy at scale

For larger clusters, logging at Info for every profile on each scan could overwhelm logs. If this is mainly for debugging or initial verification, consider using Debug level, a verbosity flag, or some rate limiting to reduce log noise.

Suggested change
kind := profileKindToString(ref.GetKind())
log.Infof("Profile %q in namespace %q: kind %q provided by Central", ref.GetName(), namespace, kind)
profileRefs = append(profileRefs, v1alpha1.NamedObjectReference{
kind := profileKindToString(ref.GetKind())
log.Debugf("Profile %q in namespace %q: kind %q provided by Central", ref.GetName(), namespace, kind)
profileRefs = append(profileRefs, v1alpha1.NamedObjectReference{

Comment on lines +290 to +292
profileRefs := make([]*central.ApplyComplianceScanConfigRequest_ProfileReference, 0, len(returnedProfiles))
for _, p := range returnedProfiles {
profileRefs = append(profileRefs, &central.ApplyComplianceScanConfigRequest_ProfileReference{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Relying on a cast between two enums can be brittle if they diverge

This line relies on central.ComplianceOperatorProfileV2_ComplianceOperatorProfileKind(p.GetComplianceOperatorKind()) preserving matching numeric values between the datastore and proto enums. If either enum changes independently, the wrong kind could be sent. Prefer an explicit mapping (e.g., a small switch) to make the dependency and failure modes explicit.

Suggested implementation:

	// Build profile refs from the already-fetched profiles so Sensor can use the correct CO kind
	// (Profile vs TailoredProfile) without needing to probe the k8s API.
	profileRefs := make([]*central.ApplyComplianceScanConfigRequest_ProfileReference, 0, len(returnedProfiles))
	for _, p := range returnedProfiles {
		var kind central.ComplianceOperatorProfileV2_ComplianceOperatorProfileKind

		switch p.GetComplianceOperatorKind() {
		case storage.ComplianceOperatorProfileV2_PROFILE:
			kind = central.ComplianceOperatorProfileV2_PROFILE
		case storage.ComplianceOperatorProfileV2_TAILORED_PROFILE:
			kind = central.ComplianceOperatorProfileV2_TAILORED_PROFILE
		default:
			// Make dependency and failure mode explicit if enums diverge or an unexpected
			// value is stored. Log and skip this profile rather than sending an incorrect kind.
			log.Errorf("Unknown compliance operator profile kind %v for profile %q; skipping profile in scan configuration %q",
				p.GetComplianceOperatorKind(), p.GetName(), scanRequest.GetScanConfigName())
			continue
		}

		profileRefs = append(profileRefs, &central.ApplyComplianceScanConfigRequest_ProfileReference{
			Name: p.GetName(),
			Kind: kind,
		})
	}
  1. This change assumes that:
    • The datastore enum type used by p.GetComplianceOperatorKind() is storage.ComplianceOperatorProfileV2_ComplianceOperatorProfileKind.
    • The datastore enum values are storage.ComplianceOperatorProfileV2_PROFILE and storage.ComplianceOperatorProfileV2_TAILORED_PROFILE.
    • The central proto enum values are central.ComplianceOperatorProfileV2_PROFILE and central.ComplianceOperatorProfileV2_TAILORED_PROFILE.
      If the actual enum names differ, adjust the switch cases and assigned central constants accordingly.
  2. Ensure that the storage package is imported in this file (it typically is in this codebase). If it is not, add:
    storage "github.com/stackrox/rox/generated/storage"
    to the imports block.

@@ -70,12 +78,26 @@ func convertCentralRequestToScanSetting(namespace string, request *central.Apply

func convertCentralRequestToScanSettingBinding(namespace string, request *central.ApplyComplianceScanConfigRequest_BaseScanSettings, _ string) runtime.Object {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the duplicated profile reference branching and construction logic into a shared helper function used by both callers to simplify and unify the code.

You can reduce the added complexity by extracting the duplicated branching + construction into a small helper that both call sites use.

For example:

func buildProfileRefs(
    request *central.ApplyComplianceScanConfigRequest_BaseScanSettings,
    namespace string,
    action string, // e.g. "create" or "update"
) []v1alpha1.NamedObjectReference {
    refs := make([]v1alpha1.NamedObjectReference, 0, len(request.GetProfiles()))

    if profileRefs := request.GetProfileRefs(); len(profileRefs) > 0 {
        // New Central: kind is provided directly — no k8s API call needed.
        for _, ref := range profileRefs {
            kind := profileKindToString(ref.GetKind())
            log.Infof("Profile %q in namespace %q: kind %q provided by Central (%s)", ref.GetName(), namespace, kind, action)
            refs = append(refs, v1alpha1.NamedObjectReference{
                Name:     ref.GetName(),
                Kind:     kind,
                APIGroup: complianceoperator.GetGroupVersion().String(),
            })
        }
        return refs
    }

    // Legacy: old Central without profile_refs — default to Profile kind.
    for _, profile := range request.GetProfiles() {
        refs = append(refs, v1alpha1.NamedObjectReference{
            Name:     profile,
            Kind:     complianceoperator.Profile.Kind,
            APIGroup: complianceoperator.GetGroupVersion().String(),
        })
    }

    return refs
}

Then both callers become simpler and consistent:

func convertCentralRequestToScanSettingBinding(namespace string, request *central.ApplyComplianceScanConfigRequest_BaseScanSettings, _ string) runtime.Object {
    profileRefs := buildProfileRefs(request, namespace, "create")

    return &v1alpha1.ScanSettingBinding{
        TypeMeta: v1.TypeMeta{
            Kind:       complianceoperator.ScanSettingBinding.Kind,
            APIVersion: complianceoperator.GetGroupVersion().String(),
        },
        ObjectMeta: v1.ObjectMeta{
            Name:        request.GetScanName(),
            Namespace:   namespace,
            Labels:      stackroxLabels,
            Annotations: map[string]string{},
        },
        Profiles: profileRefs,
        ScanSettingsName: request.GetScanSettingsName(),
    }
}
func updateScanSettingBindingFromCentralRequest(scanSettingBinding *v1alpha1.ScanSettingBinding, request *central.ApplyComplianceScanConfigRequest_BaseScanSettings) *v1alpha1.ScanSettingBinding {
    ns := scanSettingBinding.GetNamespace()
    scanSettingBinding.Profiles = buildProfileRefs(request, ns, "update")
    // TODO:  Update additional fields as ACS capability expands
    return scanSettingBinding
}

This keeps all current behavior, logging, and backward compatibility, while centralizing the branching and NamedObjectReference construction in one place.

@rhacs-bot
Copy link
Contributor

Images are ready for the commit at f590332.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-258-gf5903320e0.

@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 52.63158% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.58%. Comparing base (7475a09) to head (f590332).

Files with missing lines Patch % Lines
sensor/kubernetes/complianceoperator/utils.go 41.30% 25 Missing and 2 partials ⚠️
Additional details and impacted files
@@                                      Coverage Diff                                      @@
##           master-base/gualvare/ROX-31229-tailored-profiles-tracking   #19316      +/-   ##
=============================================================================================
- Coverage                                                      49.59%   49.58%   -0.01%     
=============================================================================================
  Files                                                           2681     2681              
  Lines                                                         202308   202349      +41     
=============================================================================================
+ Hits                                                          100336   100343       +7     
- Misses                                                         94495    94525      +30     
- Partials                                                        7477     7481       +4     
Flag Coverage Δ
go-unit-tests 49.58% <52.63%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants