-
Notifications
You must be signed in to change notification settings - Fork 596
api: Remove proto-style getter functions #12746
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: main
Are you sure you want to change the base?
api: Remove proto-style getter functions #12746
Conversation
This removes the legacy getter functions defined in the api/ directory. These pattern is no longer relevant now that we've migrated away from proto-based APIs in favor of kubebuilder. Signed-off-by: timflannagan <[email protected]>
fa552f7 to
17fc7f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the codebase to remove a large number of getter methods (e.g., GetRegistry(), GetEnabled(), etc.) from API types and replaces their usage with direct field access. This simplification removes over 600 lines of boilerplate code while adding nil checks where necessary to maintain safety.
- Removed all
Get*()methods from API types inapi/v1alpha1package - Updated all call sites to use direct field access with proper nil checks
- Added early nil returns in helper functions to prevent nil pointer dereferences
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| api/v1alpha1/kube_types.go | Removed getter methods for Image, Service, ServiceAccount, Pod, and GracefulShutdownSpec types |
| api/v1alpha1/gateway_parameters_types.go | Removed getter methods for all GatewayParameters-related types including KubernetesProxyConfig, ProxyDeployment, containers, and AI extension types |
| pkg/deployer/values_helpers.go | Updated to use direct field access with nil checks; added early returns in GetServiceValues, GetServiceAccountValues, and enhanced tracing conversion logic |
| pkg/deployer/merge.go | Updated all merge functions to use direct field access instead of getter methods |
| pkg/deployer/gateway_parameters.go | Added conditional nil checks when accessing nested fields, replaced slice creation pattern with conditional appends |
| internal/kgateway/deployer/gateway_parameters.go | Added nil checks before accessing Kube field and wrapped field accesses in conditional blocks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| istioContainerConfig := istioConfig.GetIstioProxyContainer() | ||
| aiExtensionConfig := kubeProxyConfig.GetAiExtension() | ||
| if aiExtensionConfig != nil && aiExtensionConfig.GetEnabled() != nil && *aiExtensionConfig.GetEnabled() { | ||
| deployConfig := kubeProxyConfig.Deployment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably need a nil check for kubeProxyConfig first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm getting dizzy reading through this implementation. Such an eye sore in it's current form. Going to take a stab at cleaning this up and address any potential NPEs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part 1 of the cleanup: #12747.
Description
This removes the legacy getter functions defined in the api/ directory. These pattern is no longer relevant now that we've migrated away from proto-based APIs in favor of kubebuilder.
Fixes #12227.
Change Type
/kind cleanup
Changelog
Additional Notes