refactor(config)!: extract config interfaces to pkg/api/config #566
+204
−142
Conversation
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
manusa
force-pushed
the
refactor/decouple-config
branch
from
d8451c1 to
aeb75d7
Compare
manusa
changed the title
manusa
requested review from
Cali0707 and
nader-ziada
and removed request for
Cali0707
nader-ziada
reviewed
Dec 10, 2025
Collaborator
nader-ziada
left a comment
nader-ziada
left a comment
There was a problem hiding this comment.
I like the idea of creating the interfaces for other packages to use, so there is no dependency on pkg/config
added some small suggestions
Decouples the pkg/kubernetes module from pkg/config by introducing interfaces for configuration access. This allows pkg/kubernetes to depend on abstractions rather than the concrete StaticConfig type. Changes: - Add AuthProvider, ClusterProvider, DeniedResourcesProvider, and ExtendedProvider interfaces in pkg/api/config - Move cluster provider strategy constants to pkg/api/config - Move GroupVersionKind struct to pkg/api/config - Update pkg/kubernetes to use the new interfaces - StaticConfig now implements the Config interface Signed-off-by: Marc Nuri <[email protected]>
manusa
force-pushed
the
refactor/decouple-config
branch
from
aeb75d7 to
522795c
Compare
Collaborator
|
thanks @manusa /lgtm |
Member
Author
|
@Cali0707 note that this will break downstream again :( |
Collaborator
there is already another breaking change from the drop-in reload PR that is not synced downstream yet |
Collaborator
|
Yep I will do another sync later, will aim to do one per week |
Member
Author
🙌 I'll try to get more involved in the process. I want to check before the end of the week to see if we can better reorganize upstream so that the syncing process could be streamlined even more. |
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
At the moment, the config and kubernetes modules are tightly coupled.
While reviewing the changes in #559, the current state proves to be a problem since workarounds are required to be able to easily unmarshal the configuration.
Ideally, configuration unmarshalling and parsing should be part of the
pkg/configpackage, while the api package should focus just on defining the structs and interfaces.This PR attempts to decouple kubernetes from the config module by providing config API interfaces.
kubernetes->api->config->...I'm not proud of how it looks like, but I think that it's a step in the right direction.
Suggestions for further improvements are welcome ;)
Decouples the pkg/kubernetes module from pkg/config by introducing interfaces for configuration access. This allows pkg/kubernetes to depend on abstractions rather than the concrete StaticConfig type.
Changes: