ROX-32452: implement token generation service for the OCP plugin #18545

rhybrillou · 2026-01-19T14:57:40Z

Description

PR stack:

ROX-32452: create token generation service for the OCP plugin #18456
- refactor: create common issuer source for token generation services #18543
  - refactor: use new issuer source for rox token services #18544
    - ROX-32452: implement token generation service for the OCP plugin #18545 👈

User-facing documentation

CHANGELOG.md is updated OR update is not needed
documentation PR is created and is linked above OR is not needed

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

How I validated my change

change me!

rhacs-bot · 2026-01-19T15:32:04Z

Images are ready for the commit at 103c730.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.10.x-847-g103c730796.

codecov · 2026-01-19T16:39:31Z

Codecov Report

❌ Patch coverage is 87.66234% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.15%. Comparing base (ad2b83d) to head (103c730).

Files with missing lines	Patch %	Lines
central/auth/ocpplugin/service/singleton.go	43.75%	9 Missing ⚠️
central/auth/ocpplugin/service/service_impl.go	83.78%	6 Missing ⚠️
central/auth/ocpplugin/service/role_manager.go	96.03%	4 Missing ⚠️

Additional details and impacted files

@@                              Coverage Diff                              @@
##           master-yann/plug-common-tokenbased-source   #18545      +/-   ##
=============================================================================
+ Coverage                                      49.12%   49.15%   +0.02%     
=============================================================================
  Files                                           2649     2652       +3     
  Lines                                         198837   198991     +154     
=============================================================================
+ Hits                                           97672    97805     +133     
- Misses                                         93731    93751      +20     
- Partials                                        7434     7435       +1

Flag	Coverage Δ
go-unit-tests	`49.15% <87.66%> (+0.02%)`	⬆️

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.

parametalol · 2026-01-20T09:54:25Z

central/auth/internaltokens/service/role_manager.go

Any particular reason to put this service under central/auth? Other services are under central.

There is already a service layer for the m2m service under auth, and the service here offers something different.
While most services are indeed in direct subdirectories of central, this is not always the case. Out of currently 71 service directories in central, 15 of them are not in direct subdirectories of central. The exceptions include administration sub-services for admin events and usage, compliance manager, compliance v2, cves for cluster and node, declarative configuration health and a few more.

stehessel · 2026-01-20T08:07:48Z

central/auth/ocpplugin/service/singleton.go

+const (
+	id = "https://stackrox.io/jwt-sources#ocp-rox-tokens"
+	//#nosec G101 -- This constant is only there as a source type name, not as a credential.
+	ocpToken = "ocp-plugin-token"


I would probably avoid mentioning the ocp plugin too explicitly here. Sure that is the end use case, but from Central's point of view, it is serving an internal token service. What Sensor then does with these tokens is out of scope for Central. For example, what if we expand the feature to plain k8s plugins? It would be awkward to have ocp everywhere just because it was the first end consumer of the api.

stehessel · 2026-01-20T08:09:11Z

central/auth/ocpplugin/service/service.go

+	"github.com/stackrox/rox/pkg/grpc"
+)
+
+// Service provides the interface to the microservice that serves tokens for the OCP plugin.


Microservice?

This bad pattern was propagated from the m2m service. I'll fix that.

stehessel · 2026-01-20T08:10:23Z

central/auth/ocpplugin/service/service_impl.go

+const (
+	permissionSetNameFormat = "Generated permission set for %s"
+	accessScopeNameFormat   = "Generated access scope for %s"
+	roleNameFormat          = "Generated role for PermissionSet %s and AccessScope %s"


Suggested change

roleNameFormat = "Generated role for PermissionSet %s and AccessScope %s"

roleNameFormat = "Generated role for permission set %s and access scope %s"

Or change the formatting in the other strings to be consistent about the case.

stehessel · 2026-01-20T08:12:18Z

central/auth/ocpplugin/service/service_impl.go

+		return nil, errors.Wrap(err, "getting expiration time")
+	}
+	claimName := fmt.Sprintf(
+		"Generated claims for role %s expiring at %s",


Why is this string not extracted like the others?

stehessel · 2026-01-20T08:13:04Z

central/auth/ocpplugin/service/service_impl.go

+	return response, nil
+}
+
+// region helpers


Suggested change

// region helpers

Not helpful imo.

stehessel · 2026-01-20T08:14:46Z

central/auth/ocpplugin/service/service_impl.go

+) (time.Time, error) {
+	duration, err := protocompat.DurationFromProto(req.GetValidFor())
+	if err != nil {
+		return time.Time{}, errors.Wrap(err, "converting requested token validity duration")


Shouldn't this one be errox.InvalidArgs as well?

stehessel · 2026-01-20T08:19:38Z

central/auth/internaltokens/service/role_manager.go

+	ctx context.Context,
+	req *central.GenerateTokenForPermissionsAndScopeRequest,
+) (string, error) {
+	// TODO: Consider pruning the generated permission sets after some idle time.


We should certainly prune the dynamic resources, especially the access scopes and roles. Would the permission sets, access scopes and roles be visible in the UI similar to user created resources?

With the current implementation, the dynamically created roles, permission sets and access scopes are standard ones, and will be accessible though APU and UI like other user-created resources.

Do you think we can make them inaccessible? I'm a bit worried that these dynamically created resources will flood the UI.

rhybrillou · 2026-01-19T16:10:06Z

central/auth/internaltokens/service/service_impl.go

+		return nil, err
+	}
+	response := &central.GenerateTokenForPermissionsAndScopeResponse{
+		Token: tokenInfo.Token,


From @parametalol - comment lines 58 to 73 on central/auth/ocpplugin/service/service_impl.go in the first commit (reference at time of writing: 3755769#diff-892fd289d36f704f4f0565732fe8df62e7726ca57d0f1afe2c367aca1b2cadfcR73 )

The expiry could be assed via the WithExpiry option

issuedAt should be taken from the tokenInfo
I'm not even sure that Roles, IssuedAt and ExpiresAt are needed in the response

The call to s.issuer.Issue now uses the WithExpiry option to set the expiration in the issued token.
The response message is now reduced to its minimal form: a single string containing the issued token.

rhybrillou · 2026-01-19T16:18:18Z

central/auth/internaltokens/service/role_manager.go

+	permissionSetID, err := rm.createPermissionSet(ctx, req)
+	if err != nil {
+		return "", errors.Wrap(err, "creating permission set for role")
+	}
+	accessScopeID, err := rm.createAccessScope(ctx, req)
+	if err != nil {
+		return "", errors.Wrap(err, "creating access scope for role")
+	}


From @parametalol comment lines 96 and 97 on central/auth/ocpplugin/service/service_impl.go in the first commit (reference at time of writing: 3755769#diff-892fd289d36f704f4f0565732fe8df62e7726ca57d0f1afe2c367aca1b2cadfcR97)
If we need only IDs, maybe we could optimize the getters now and in the future: (change suggestion not ported over).

The API was updated to return the identifier string for access scopes and permission sets (role name for roles).
The functions were renamed to reflect the fact that they are not simple getters on objects, but rather change the underlying database.

rhybrillou · 2026-01-19T16:23:33Z

central/auth/internaltokens/service/role_manager.go

+}
+
+var (
+	generatedObjectTraits = &storage.Traits{Origin: storage.Traits_IMPERATIVE}


From @parametalol comment line 105 on central/auth/ocpplugin/service/service_impl.go in the first commit (reference at time of writing: 3755769#diff-892fd289d36f704f4f0565732fe8df62e7726ca57d0f1afe2c367aca1b2cadfcR105)
Would it make sense to add Traits_DYNAMIC for visibility or other needs?

That is a good idea. Not implemented in the current PR.
It would be a candidate for a follow-up PR, as it might touch multiple locations in the code, most likely

stackrox/pkg/declarativeconfig/context.go

Lines 38 to 47 in 387c138

// CanModifyResource returns whether context holder is allowed to modify resource.

func CanModifyResource(ctx context.Context, resource ResourceWithTraits) bool {

if ctx.Value(originCheckerKey{}) == allowOnlyDeclarativeOperations {

return IsDeclarativeOrigin(resource)

}

if ctx.Value(originCheckerKey{}) == allowModifyDeclarativeOrImperative {

return IsDeclarativeOrigin(resource) || resource.GetTraits().GetOrigin() == storage.Traits_IMPERATIVE

}

return resource.GetTraits().GetOrigin() == storage.Traits_IMPERATIVE

}

and maybe some others.
It would likely require to add primitives like IsImperativeOrigin and IsDynamicOrigin and use them in traits validation conditions.

…ce in central

…tokens

openshift-ci · 2026-01-20T20:07:23Z

@rhybrillou: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/gke-qa-e2e-tests	`be1282e`	link	false	`/test gke-qa-e2e-tests`
ci/prow/gke-upgrade-tests	`be1282e`	link	false	`/test gke-upgrade-tests`
ci/prow/gke-nongroovy-e2e-tests	`be1282e`	link	true	`/test gke-nongroovy-e2e-tests`
ci/prow/gke-ui-e2e-tests	`be1282e`	link	true	`/test gke-ui-e2e-tests`
ci/prow/ocp-4-12-nongroovy-e2e-tests	`be1282e`	link	false	`/test ocp-4-12-nongroovy-e2e-tests`
ci/prow/ocp-4-20-ui-e2e-tests	`be1282e`	link	false	`/test ocp-4-20-ui-e2e-tests`
ci/prow/ocp-4-20-nongroovy-e2e-tests	`be1282e`	link	false	`/test ocp-4-20-nongroovy-e2e-tests`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

rhybrillou requested a review from a team as a code owner January 19, 2026 14:57

github-actions bot added the area/central label Jan 19, 2026

This was referenced Jan 19, 2026

ROX-32452: create token generation service for the OCP plugin #18456

Open

refactor: create common issuer source for token generation services #18543

Open

refactor: use new issuer source for rox token services #18544

Open

rhybrillou force-pushed the master-yann/plug-common-tokenbased-source branch from 934ec64 to 0a84d21 Compare January 19, 2026 16:02

rhybrillou force-pushed the master-yann/implement-plugin-token-service branch from 9256d36 to 5752516 Compare January 19, 2026 16:02

parametalol reviewed Jan 20, 2026

View reviewed changes

stehessel reviewed Jan 20, 2026

View reviewed changes

rhybrillou commented Jan 20, 2026

View reviewed changes

rhybrillou force-pushed the master-yann/plug-common-tokenbased-source branch from 0a84d21 to d16f7a5 Compare January 20, 2026 10:51

rhybrillou force-pushed the master-yann/implement-plugin-token-service branch from 5752516 to a432044 Compare January 20, 2026 10:51

rhybrillou added 3 commits January 20, 2026 14:17

ROX-32452: implement token generation service for the OCP plugin

cd1de4b

Extract role creation to a dedicated object + register internal servi…

b11f7d7

…ce in central

Adjust proto references after token service move to central.v1

e9d5b90

rhybrillou force-pushed the master-yann/plug-common-tokenbased-source branch from d16f7a5 to ad2b83d Compare January 20, 2026 13:18

rhybrillou force-pushed the master-yann/implement-plugin-token-service branch from 6f844ae to e9d5b90 Compare January 20, 2026 13:18

rhybrillou added 2 commits January 20, 2026 15:10

address review comments

103c730

disable gosec on ID const + rename service from ocpplugin to internal…

be1282e

…tokens

	roleNameFormat = "Generated role for PermissionSet %s and AccessScope %s"
	roleNameFormat = "Generated role for permission set %s and access scope %s"

	// CanModifyResource returns whether context holder is allowed to modify resource.
	func CanModifyResource(ctx context.Context, resource ResourceWithTraits) bool {
	if ctx.Value(originCheckerKey{}) == allowOnlyDeclarativeOperations {
	return IsDeclarativeOrigin(resource)
	}
	if ctx.Value(originCheckerKey{}) == allowModifyDeclarativeOrImperative {
	return IsDeclarativeOrigin(resource) \|\| resource.GetTraits().GetOrigin() == storage.Traits_IMPERATIVE
	}
	return resource.GetTraits().GetOrigin() == storage.Traits_IMPERATIVE
	}

ROX-32452: implement token generation service for the OCP plugin #18545

Are you sure you want to change the base?

ROX-32452: implement token generation service for the OCP plugin #18545

Uh oh!

Conversation

rhybrillou commented Jan 19, 2026 • edited by parametalol Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

User-facing documentation

Testing and quality

Automated testing

How I validated my change

Uh oh!

rhacs-bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rhybrillou commented Jan 19, 2026 •

edited by parametalol

Loading

rhacs-bot commented Jan 19, 2026 •

edited

Loading

codecov bot commented Jan 19, 2026 •

edited

Loading