-
Notifications
You must be signed in to change notification settings - Fork 177
WIP: ROX-34074: Fix components init func #19966
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 17 commits
79d020c
ce0bf8d
99f16b5
391316f
6834046
29b615f
2cf4237
2af5b79
eb3f1e0
bdfdc77
2287e20
0449ea6
8b5a49f
eca4ef0
d3de83e
987c76a
82a6968
c78f077
93f7842
db6b9d5
f1a6ada
6744213
9b30c56
f24eb46
668c7ef
0a704c0
b84cb73
3b1086c
530b499
f487fd3
4e8afdd
f8083d4
4c0dbf7
998006c
3ab5d12
5dfcdc9
26ed417
e9f029f
4eb3d12
51425e9
441a472
d62a670
a7c0386
0d397c9
0cb5d2a
6fceb99
dd31084
aeee0e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package app | ||
|
|
||
| import ( | ||
| "github.com/stackrox/rox/pkg/logging" | ||
| "github.com/stackrox/rox/pkg/memlimit" | ||
| "github.com/stackrox/rox/pkg/premain" | ||
| "github.com/stackrox/rox/pkg/profiling" | ||
| ) | ||
|
|
||
| var ( | ||
| log = logging.LoggerForModule() | ||
| ) | ||
|
|
||
| // Run is the main entry point for the central application. | ||
| // Performs early initialization and component-specific setup before | ||
| // main.CentralRun() starts the actual central service logic. | ||
| func Run() { | ||
| profiling.SetComponentLabel() | ||
| memlimit.SetMemoryLimit() | ||
| premain.StartMain() | ||
|
|
||
| initMetrics() | ||
| initCompliance() | ||
| initGraphQL() | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package app | ||
|
|
||
| import ( | ||
| "github.com/stackrox/rox/central/metrics" | ||
| ) | ||
|
|
||
| func initMetrics() { | ||
| metrics.Init() | ||
| } | ||
|
|
||
| // initCompliance registers all compliance checks. | ||
| func initCompliance() { | ||
| // Import side-effect: pkg/compliance/checks registers all standard checks via init() | ||
| // We consolidate that registration here by calling the registration function explicitly | ||
| // This is handled by importing central/compliance/checks/remote which calls MustRegisterChecks | ||
|
|
||
| // The actual registration is done via the package import | ||
| // Future work: refactor compliance/checks to use explicit registration | ||
| } | ||
|
|
||
| // initGraphQL registers all GraphQL type loaders. | ||
| func initGraphQL() { | ||
| // GraphQL loaders registration | ||
| // Each loader registers itself via RegisterTypeFactory in their init() functions | ||
|
|
||
| // Similar to compliance checks, this requires refactoring the loader registration | ||
| // to be explicit rather than init()-based | ||
| // Stub for now - full migration in separate PR | ||
|
sourcery-ai[bot] marked this conversation as resolved.
Outdated
|
||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These helpers do not remove the busybox init cost yet. Both functions are stubs, and the comments explicitly say the real registration still happens through package-import side effects. In the consolidated binary, those 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -233,6 +233,7 @@ import ( | |||||||||||||||||
| pkgVersion "github.com/stackrox/rox/pkg/version" | ||||||||||||||||||
|
|
||||||||||||||||||
| // BusyBox-style consolidation - import app packages | ||||||||||||||||||
| app "github.com/stackrox/rox/central/app" | ||||||||||||||||||
| complianceapp "github.com/stackrox/rox/compliance/cmd/compliance/app" | ||||||||||||||||||
| roxagentapp "github.com/stackrox/rox/compliance/virtualmachines/roxagent/app" | ||||||||||||||||||
| configcontrollerapp "github.com/stackrox/rox/config-controller/app" | ||||||||||||||||||
|
|
@@ -281,7 +282,9 @@ func runSafeMode() { | |||||||||||||||||
| log.Info("Central terminated") | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| func centralRun() { | ||||||||||||||||||
| // CentralRun is the main central application logic. | ||||||||||||||||||
| // Exported temporarily to allow central/app package to call it. | ||||||||||||||||||
| func CentralRun() { | ||||||||||||||||||
| defer utils.IgnoreError(log.InnerLogger.Sync) | ||||||||||||||||||
|
|
||||||||||||||||||
| premain.StartMain() | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (bug_risk): Central mode now runs premain and initialization twice and default mode misses app-level init In the Please refactor so there is a single, consistent initialization path—for example, by having all |
||||||||||||||||||
|
|
@@ -1086,7 +1089,8 @@ func main() { | |||||||||||||||||
|
|
||||||||||||||||||
| switch binaryName { | ||||||||||||||||||
| case "central": | ||||||||||||||||||
| centralRun() | ||||||||||||||||||
| app.Run() | ||||||||||||||||||
| CentralRun() | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (bug_risk): Avoid calling premain.StartMain (and other early-init) twice for the central binary For the "central" case, both |
||||||||||||||||||
| case "migrator": | ||||||||||||||||||
| migratorapp.Run() | ||||||||||||||||||
| case "compliance": | ||||||||||||||||||
|
|
@@ -1106,6 +1110,6 @@ func main() { | |||||||||||||||||
| default: | ||||||||||||||||||
| // Default to central if called with unknown name | ||||||||||||||||||
| log.Warnf("Unknown binary name %q, defaulting to central mode", binaryName) | ||||||||||||||||||
| centralRun() | ||||||||||||||||||
| CentralRun() | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Default-to-central path skips the new central init flow. The 🔧 Proposed fix default:
// Default to central if called with unknown name
log.Warnf("Unknown binary name %q, defaulting to central mode", binaryName)
+ app.Run()
CentralRun()
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package app | ||
|
|
||
| // initMetrics registers config-controller-specific prometheus metrics. | ||
| // Currently config-controller has minimal metrics, so this is a stub for future use. | ||
| func initMetrics() { | ||
| // No metrics to register yet - config-controller has very light init overhead | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # Conditional Initialization in Busybox Binary | ||
|
|
||
| ## Overview | ||
|
|
||
| StackRox uses a busybox-style consolidated binary where multiple components (central, sensor, admission-control, config-controller, etc.) are built into a single binary and dispatched via os.Args[0]. | ||
|
|
||
| ## Problem | ||
|
|
||
| Go's package init() functions run when packages are imported, regardless of which component will actually execute. This caused all 535 init() functions to run for every component, leading to: | ||
|
|
||
| - Memory overhead (~166 MB for sensor, ~64 MB for central) | ||
| - OOMKills under race detector (config-controller: 7 restarts, admission-control: 6-7 restarts) | ||
|
|
||
| ## Solution | ||
|
|
||
| Move initialization from package-level init() to explicit component-specific functions called from app.Run(). | ||
|
|
||
| ### Architecture | ||
|
|
||
| ``` | ||
| central/main.go (dispatcher) | ||
| ↓ os.Args[0] check | ||
| component/app/app.go | ||
| ↓ Run() | ||
| component/app/init.go | ||
| ↓ initMetrics(), initCompliance(), etc. | ||
| ``` | ||
|
|
||
| ### Migration Status | ||
|
|
||
| **Completed:** | ||
| - ✅ central/metrics (37 metrics) | ||
| - ✅ sensor/common/metrics (42 metrics) | ||
| - ✅ sensor/admission-control (6 metrics) | ||
|
|
||
| **Stubs (future work):** | ||
| - ⏳ compliance checks (109 files) | ||
| - ⏳ GraphQL loaders (15 files) | ||
|
|
||
| **Not migrated:** | ||
| - Shared infrastructure (logging, feature flags, env settings) | ||
| - Simple/negligible init() functions | ||
|
|
||
| ## Usage | ||
|
|
||
| When adding new component-specific initialization: | ||
|
|
||
| 1. Add function to `component/app/init.go` | ||
| 2. Call from `component/app/app.go Run()` function | ||
| 3. Avoid package-level init() for component-specific code | ||
|
|
||
| ## Testing | ||
|
|
||
| Verify no cross-component init() execution: | ||
|
|
||
| ```bash | ||
| # Check import chains | ||
| go list -f '{{.Deps}}' ./sensor/kubernetes | grep "central/" | ||
|
|
||
| # Should return no central/* packages (pkg/* is okay) | ||
| ``` | ||
|
|
||
| ## References | ||
|
|
||
| - Design spec: docs/superpowers/specs/2026-04-13-conditional-init-design.md | ||
| - Implementation plan: docs/superpowers/plans/2026-04-13-conditional-init.md | ||
| - Verification report: docs/init-migration-verification.md | ||
| - ROX-33958: BusyBox binary consolidation OOMKill fixes |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| # Compliance Check Init Migration | ||
|
|
||
| ## Current State | ||
| - 109 compliance check files have init() functions | ||
| - Each calls framework.MustRegisterChecks() individually | ||
| - Registration happens via package init when imported | ||
|
|
||
| ## Migration Strategy | ||
| Phase 3.2 establishes initCompliance() stub in central/app/init.go | ||
|
|
||
| Future work (separate PR): | ||
| 1. Refactor pkg/compliance/checks to export registration functions | ||
| 2. Call those functions from central/app/init.go initCompliance() | ||
| 3. Remove init() from all 109 check files | ||
|
|
||
| ## Files affected: | ||
|
|
||
| ### central/compliance/checks/ | ||
| - remote/all.go | ||
| - nist80053/check_cm_11/check.go | ||
| - nist80053/check_cm_7/check.go | ||
| - nist80053/check_sa_10/check.go | ||
| - nist80053/check_ra_3/check.go | ||
| - nist80053/check_ac_14/check.go | ||
| - nist80053/check_cm_3/check.go | ||
| - nist80053/check_cm_2/check.go | ||
| - nist80053/check_si_4/check.go | ||
| - nist80053/check_ir_4_5/check.go | ||
| - nist80053/check_cm_8/check.go | ||
| - nist80053/check_sc_6/check.go | ||
| - nist80053/check_ir_5/check.go | ||
| - nist80053/check_cm_6/check.go | ||
| - nist80053/check_ir_6_1/check.go | ||
| - nist80053/check_ca_9/check.go | ||
| - nist80053/check_cm_5/check.go | ||
| - nist80053/check_sc_7/check.go | ||
| - nist80053/check_ra_5/check.go | ||
| - nist80053/check_si_3_8/check.go | ||
| - nist80053/check_si_2_2/check.go | ||
| - pcidss32/check811/check.go | ||
| - pcidss32/check12/check.go | ||
| - pcidss32/check24/check.go | ||
| - pcidss32/check71/check.go | ||
| - pcidss32/check62/check.go | ||
| - pcidss32/check85/check.go | ||
| - pcidss32/check22/check.go | ||
| - pcidss32/check362/check.go | ||
| - pcidss32/check712/check.go | ||
| - pcidss32/check112/check.go | ||
| - pcidss32/check225/check.go | ||
| - pcidss32/check134/check.go | ||
| - pcidss32/check722/check.go | ||
| - pcidss32/check723/check.go | ||
| - pcidss32/check21/check.go | ||
| - pcidss32/check23/check.go | ||
| - pcidss32/check135/check.go | ||
| - pcidss32/check132/check.go | ||
| - pcidss32/check61/check.go | ||
| - pcidss32/check1121/check.go | ||
| - pcidss32/check114/check.go | ||
| - pcidss32/check121/check.go | ||
| - pcidss32/check656/check.go | ||
| - pcidss32/check713/check.go | ||
| - nist800-190/check435/check.go | ||
| - nist800-190/check411/check.go | ||
| - nist800-190/check412/check.go | ||
| - nist800-190/check433/check.go | ||
| - nist800-190/check442/check.go | ||
| - nist800-190/check414/check.go | ||
| - nist800-190/check444/check.go | ||
| - nist800-190/check432/check.go | ||
| - nist800-190/check422/check.go | ||
| - nist800-190/check455/check.go | ||
| - nist800-190/check451/check.go | ||
| - nist800-190/check431/check.go | ||
| - nist800-190/check443/check.go | ||
| - hipaa_164/check312e/check.go | ||
| - hipaa_164/check312c/check.go | ||
| - hipaa_164/check308a7iie/check.go | ||
| - hipaa_164/check306e/check.go | ||
| - hipaa_164/check312e1/check.go | ||
| - hipaa_164/check316b2iii/check.go | ||
| - hipaa_164/check308a1i/check.go | ||
| - hipaa_164/check308a1iia/check.go | ||
| - hipaa_164/check308a5iib/check.go | ||
| - hipaa_164/check308a1iib/check.go | ||
| - hipaa_164/check310a1/check.go | ||
| - hipaa_164/check308a6ii/check.go | ||
| - hipaa_164/check308a3iib/check.go | ||
| - hipaa_164/check308a4iib/check.go | ||
| - hipaa_164/check310a1a/check.go | ||
| - hipaa_164/check308a3iia/check.go | ||
| - hipaa_164/check314a2ic/check.go | ||
| - hipaa_164/check308a4/check.go | ||
| - hipaa_164/check310d/check.go | ||
|
|
||
| ### pkg/compliance/checks/ | ||
| - nist80053/check_ac_14/check.go | ||
| - nist80053/check_ac_3_7/check.go | ||
| - nist80053/check_cm_5/check.go | ||
| - nist80053/check_ac_24/check.go | ||
| - pcidss32/check811/check.go | ||
| - pcidss32/check71/check.go | ||
| - pcidss32/check85/check.go | ||
| - pcidss32/check362/check.go | ||
| - pcidss32/check712/check.go | ||
| - pcidss32/check722/check.go | ||
| - pcidss32/check723/check.go | ||
| - pcidss32/check713/check.go | ||
| - nist800-190/check421/check.go | ||
| - nist800-190/check432/check.go | ||
| - nist800-190/check431/check.go | ||
| - hipaa_164/check312e1/check.go | ||
| - hipaa_164/check308a3iib/check.go | ||
| - hipaa_164/check308a4/check.go | ||
| - kubernetes/master_scheduler.go | ||
| - kubernetes/policies_network_cni.go | ||
| - kubernetes/policies_secrets_management.go | ||
| - kubernetes/kubelet_command.go | ||
| - kubernetes/worker_node_config.go | ||
| - kubernetes/policies_admission_control.go | ||
| - kubernetes/control_plane_config.go | ||
| - kubernetes/master_api_server.go | ||
| - kubernetes/policies_rbac.go | ||
| - kubernetes/policies_pod_security.go | ||
| - kubernetes/master_config.go | ||
| - kubernetes/etcd.go | ||
| - kubernetes/master_controller_manager.go | ||
| - kubernetes/policies_general.go |
Uh oh!
There was an error while loading. Please reload this page.