Move GetLoggingConfig from cmd/broker to pkg/utils#9015
Move GetLoggingConfig from cmd/broker to pkg/utils#9015Ankitsinghsisodya wants to merge 1 commit intoknative:mainfrom
Conversation
The GetLoggingConfig helper lived in package broker (cmd/broker/config.go) even though it is generic and was being imported by non-broker commands (jobsink, auth_proxy) just to read the logging ConfigMap. Move it into pkg/utils alongside the other small cross-cutting helpers, update all call sites, and delete the now-empty cmd/broker package. Fixes knative#8712
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ankitsinghsisodya The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Ankitsinghsisodya. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
There was a problem hiding this comment.
Pull request overview
Refactors logging configuration access by relocating GetLoggingConfig out of the cmd/broker package and into pkg/utils, so multiple binaries can reuse it without importing a cmd/* package.
Changes:
- Moved
GetLoggingConfigintopkg/utilsand updated its doc comment for clarity. - Updated the affected binaries (broker filter/ingress, jobsink, auth_proxy) to call
utils.GetLoggingConfig. - Removed the now-unused
knative.dev/eventing/cmd/brokerimport usage at the call sites.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/utils/logging.go | Hosts GetLoggingConfig in pkg/utils (shared helper location). |
| cmd/jobsink/main.go | Switches logging config lookup to utils.GetLoggingConfig. |
| cmd/broker/ingress/main.go | Switches logging config lookup to utils.GetLoggingConfig and adds the utils import. |
| cmd/broker/filter/main.go | Switches logging config lookup to utils.GetLoggingConfig and adds the utils import. |
| cmd/auth_proxy/main.go | Switches logging setup to use utils.GetLoggingConfig and adds the utils import. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
GetLoggingConfigout of packagebroker(cmd/broker/config.go) and intopkg/utils(pkg/utils/logging.go), where the other small cross-cutting helpers already live.cmd/broker/filter,cmd/broker/ingress,cmd/jobsink,cmd/auth_proxy) to importpkg/utilsinstead ofcmd/broker.cmd/jobsinkandcmd/auth_proxywere importingcmd/brokersolely for this helper.cmd/brokerpackage.No behavior change — this is a pure code-organization refactor.
Fixes #8712
Test plan
go build ./...go vet ./cmd/... ./pkg/utils/...go test ./cmd/... ./pkg/utils/...grepconfirms no remaining references tocmdbrokerorknative.dev/eventing/cmd/broker"anywhere in the repo