Open
Conversation
- Split various functionality out of controller.go - Add reporting.go file to controller package containing logic related to reporting deployments to the Artifact Metadata API - Add workload package with encapsulated logic for resolving workload identity for pods.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the controller by extracting reporting logic into a dedicated file and introducing a new internal/workload package to encapsulate pod-to-workload resolution and workload liveness checks, reducing the size and responsibility of controller.go.
Changes:
- Split deployment record reporting logic out of
controller.gointointernal/controller/reporting.go(+ new unit tests). - Added
internal/workloadpackage (with tests) to centralize supported owner detection, workload identity resolution, and “is workload still active” checks. - Updated controller startup/sync flow to use
SharedInformerFactory.Start()andWaitForCacheSync().
Show a summary per file
| File | Description |
|---|---|
| internal/controller/controller.go | Simplifies Controller by storing the informer factory + delegating workload logic to internal/workload; starts/syncs informers via the factory. |
| internal/controller/reporting.go | New home for pod event processing + posting deployment records to the Artifact Metadata API. |
| internal/controller/reporting_test.go | New unit tests for unknown-artifact caching behavior. |
| internal/controller/controller_test.go | Updates test controller setup to satisfy new workloadResolver dependency and informer-factory sync behavior. |
| internal/controller/controller_integration_test.go | Updates cache-sync waiting to use the controller’s informer factory. |
| internal/workload/workload.go | New package for resolving workload identity and checking workload existence via informer listers. |
| internal/workload/workload_test.go | New tests for supported-owner checks, name extraction helpers, and workload resolution. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
This PR splits up some of the functionality in
controller.go, both to simplify theControllerstruct and to reduce the amount of code incontroller.go:reporting.gofile tocontrollerpackage containing logic related to reporting deployments to the Artifact Metadata APIworkloadpackage with encapsulated logic for getting pod workload info (resolving parent workload, deployment name, etc).