Skip to content

ROX-33683: Add tailored profiles central capability#19518

Merged
guzalv merged 5 commits intomasterfrom
master-base/gualvare/ROX-33683-tp-capability
Mar 20, 2026
Merged

ROX-33683: Add tailored profiles central capability#19518
guzalv merged 5 commits intomasterfrom
master-base/gualvare/ROX-33683-tp-capability

Conversation

@guzalv
Copy link
Contributor

@guzalv guzalv commented Mar 20, 2026

Description

A Tailored Profiles (TP) central capability is needed to handle scenarios where a Sensor that supports TPs connects to a Central that doesn’t. Because TPs use the same proto message as regular profiles (adding a kind=TAILORED_PROFILE field), old Centrals would ignore that field and store the TP as a regular profile, allowing users to schedule scans using those TPs.

Those scans would fail to be created in Sensor because the Kind of the profiles they include would be wrong. Even if that were fixed in Sensor by figuring out the kind by reaching out to the k8s API, the resulting ComplianceCheckResults and associated reporting would be broken due to referring to TPs and (possibly) custom rules.

This PR adds a new capability (ComplianceV2TailoredProfiles) and modifies logic so that Sensor only processes Compliance V2 tailored profiles if Central exposes the capability.

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

CI

@openshift-ci
Copy link

openshift-ci bot commented Mar 20, 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 1 issue, and left some high level feedback:

  • The new early return in TailoredProfileDispatcher.ProcessEvent based on ComplianceV2TailoredProfiles means we now drop all tailored profile events (including deletes) when Central doesn't advertise this capability; please double‑check this aligns with expected downgrade/compat behavior and that we don't need to treat delete actions differently.
  • In the tests you call centralcaps.Set(...) but never reset the capability state, which can leak configuration across tests in this package; consider using t.Cleanup or a helper to restore the previous capabilities after each test.
  • In service_impl.go you append centralsensor.ComplianceV2TailoredProfiles.String() while other capabilities are appended directly as CentralCapability values; for consistency and to avoid type surprises, consider aligning this with how the other capabilities are added.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new early return in `TailoredProfileDispatcher.ProcessEvent` based on `ComplianceV2TailoredProfiles` means we now drop all tailored profile events (including deletes) when Central doesn't advertise this capability; please double‑check this aligns with expected downgrade/compat behavior and that we don't need to treat delete actions differently.
- In the tests you call `centralcaps.Set(...)` but never reset the capability state, which can leak configuration across tests in this package; consider using `t.Cleanup` or a helper to restore the previous capabilities after each test.
- In `service_impl.go` you append `centralsensor.ComplianceV2TailoredProfiles.String()` while other capabilities are appended directly as `CentralCapability` values; for consistency and to avoid type surprises, consider aligning this with how the other capabilities are added.

## Individual Comments

### Comment 1
<location path="central/sensor/service/service_impl.go" line_range="141" />
<code_context>
 		capabilities = append(capabilities, centralsensor.SendDeduperStateOnReconnect)
 		if features.ComplianceEnhancements.Enabled() {
 			capabilities = append(capabilities, centralsensor.ComplianceV2Integrations)
+			capabilities = append(capabilities, centralsensor.ComplianceV2TailoredProfiles.String())
 		}
 		if features.ComplianceRemediationV2.Enabled() {
</code_context>
<issue_to_address>
**issue (bug_risk):** Use the capability value consistently instead of calling `.String()` here

Other capabilities in this block (e.g. `ComplianceV2Integrations`) are appended directly, and `CentralCapability` is already a `string` type alias. Calling `.String()` here is unnecessary and may not compile if that method doesn’t exist. This should match the others, e.g.:

```go
capabilities = append(capabilities, centralsensor.ComplianceV2TailoredProfiles)
```
</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.

@rhacs-bot
Copy link
Contributor

rhacs-bot commented Mar 20, 2026

Images are ready for the commit at 50a8511.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-395-g50a85117da.

@guzalv
Copy link
Contributor Author

guzalv commented Mar 20, 2026

  • The new early return in TailoredProfileDispatcher.ProcessEvent based on ComplianceV2TailoredProfiles means we now drop all tailored profile events (including deletes) when Central doesn't advertise this capability; please double‑check this aligns with expected downgrade/compat behavior and that we don't need to treat delete actions differently.

No need to keep delete for downgrade purposes. CO events are not deduped, and central reconciles after sync. So on downgrade no TPs will be sent, and central will delete those that previously existed.

@guzalv guzalv force-pushed the master-base/gualvare/ROX-33683-tp-capability branch from 312b0b6 to 7f6b4c8 Compare March 20, 2026 14:03
@guzalv guzalv marked this pull request as ready for review March 20, 2026 14:13
@guzalv guzalv requested a review from a team as a code owner March 20, 2026 14:13
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 left some high level feedback:

  • In TestProcessEvent_V2EventHasTailoredProfileKind, the assertion that the V2 message is at index 1 couples the test to the ordering of ForwardMessages; consider locating the V2 message by checking which entry has GetComplianceOperatorProfileV2() != nil to make the test more robust to future changes.
  • The conditional in ProcessEvent now checks centralcaps.Has(ComplianceV2Integrations) and centralcaps.Has(ComplianceV2TailoredProfiles) separately; if this pattern is expected to recur, consider introducing a small helper (e.g., hasComplianceV2TailoredProfilesSupport()) to centralize the logic and make the intent clearer.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `TestProcessEvent_V2EventHasTailoredProfileKind`, the assertion that the V2 message is at index 1 couples the test to the ordering of `ForwardMessages`; consider locating the V2 message by checking which entry has `GetComplianceOperatorProfileV2() != nil` to make the test more robust to future changes.
- The conditional in `ProcessEvent` now checks `centralcaps.Has(ComplianceV2Integrations)` and `centralcaps.Has(ComplianceV2TailoredProfiles)` separately; if this pattern is expected to recur, consider introducing a small helper (e.g., `hasComplianceV2TailoredProfilesSupport()`) to centralize the logic and make the intent clearer.

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.

Co-authored-by: Mladen Todorovic <mtodor@gmail.com>
@guzalv
Copy link
Contributor Author

guzalv commented Mar 20, 2026

/test ocp-4-21-compliance-e2e-tests

@codecov
Copy link

codecov bot commented Mar 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.24%. Comparing base (dc8c1ec) to head (50a8511).
⚠️ Report is 10 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19518      +/-   ##
==========================================
- Coverage   49.27%   49.24%   -0.03%     
==========================================
  Files        2726     2727       +1     
  Lines      205628   205765     +137     
==========================================
+ Hits       101314   101329      +15     
- Misses      96777    96899     +122     
  Partials     7537     7537              
Flag Coverage Δ
go-unit-tests 49.24% <100.00%> (-0.03%) ⬇️

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.

@guzalv guzalv merged commit 7b8c18d into master Mar 20, 2026
112 of 114 checks passed
@guzalv guzalv deleted the master-base/gualvare/ROX-33683-tp-capability branch March 20, 2026 21:30
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.

3 participants