chore(api): rename CertManagement webhook fields to generic names#3386
chore(api): rename CertManagement webhook fields to generic names#3386tariq-hasan wants to merge 1 commit intokubeflow:masterfrom
Conversation
Signed-off-by: tariq-hasan <mmtariquehsn@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
🎉 Welcome to the Kubeflow Trainer! 🎉 Thanks for opening your first PR! We're happy to have you as part of our community 🚀 Here's what happens next:
Join the community:
Feel free to ask questions in the comments if you need any help or clarification! |
There was a problem hiding this comment.
Pull request overview
This PR renames the CertManagement configuration fields from webhook-specific names (WebhookServiceName and WebhookSecretName) to generic names (ServiceName and SecretName) throughout the codebase, addressing issue #3327. This change is necessary because the TLS certificate is used not only by admission webhooks but also by the metrics server and status server.
Changes:
- Renamed struct fields and JSON tags in the CertManagement configuration type to use generic names
- Updated all usages of these fields across the codebase including the cert utility, trainjobstatus plugin, and main controller manager
- Updated documentation comments to clarify that the certificate serves multiple components
- Updated all configuration files, templates, and test fixtures to use the new field names
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/apis/config/v1alpha1/configuration_types.go | Updated CertManagement struct fields and documentation to use generic names |
| pkg/util/cert/cert.go | Updated Config struct to use the new field names |
| cmd/trainer-controller-manager/main.go | Updated references to use new field names when calling cert.ManageCerts |
| pkg/runtime/framework/plugins/trainjobstatus/trainjobstatus.go | Updated usage and removed obsolete TODO comment |
| pkg/apis/config/v1alpha1/defaults.go | Updated default value setting to use new field names |
| pkg/config/config_test.go | Updated all test fixtures and configuration examples |
| pkg/runtime/framework/plugins/trainjobstatus/trainjobstatus_test.go | Updated test configurations |
| manifests/base/manager/controller_manager_config.yaml | Updated manifest YAML |
| charts/kubeflow-trainer/values.yaml | Updated Helm values |
| charts/kubeflow-trainer/templates/manager/configmap.yaml | Updated Helm template |
| charts/kubeflow-trainer/tests/manager/configmap_test.yaml | Updated Helm test assertions |
| charts/kubeflow-trainer/README.md | Updated auto-generated documentation |
What this PR does / why we need it:
Renames the
CertManagementconfiguration fields from webhook-specific names to generic names, since the TLS certificate is used by the metrics server and the status server in addition to webhooks.Which issue(s) this PR fixes (optional, in
Fixes #<issue number>, #<issue number>, ...format, will close the issue(s) when PR gets merged):Fixes #3327
Checklist: