CRDs: allow patch merge for ScaledObject's and ScaledJob's triggers#7553
CRDs: allow patch merge for ScaledObject's and ScaledJob's triggers#7553paulolieuthier wants to merge 2 commits intokedacore:mainfrom
Conversation
Signed-off-by: Paulo Lieuthier <[email protected]>
Signed-off-by: Paulo Lieuthier <[email protected]>
|
Thank you for your contribution! 🙏 Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected. While you are waiting, make sure to:
Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient. Learn more about our contribution guide. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Pull request overview
Enables Kustomize/strategic-merge patching of spec.triggers for KEDA ScaledObject and ScaledJob CRDs by making the triggers list mergeable.
Changes:
- Add
x-kubernetes-patch-strategy: mergetospec.triggersinScaledObjectCRD schema. - Add
x-kubernetes-patch-strategy: mergetospec.triggersinScaledJobCRD schema. - Set
x-kubernetes-patch-merge-key: namefor both triggers lists.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| config/crd/bases/keda.sh_scaledobjects.yaml | Marks spec.triggers as mergeable using name as the merge key. |
| config/crd/bases/keda.sh_scaledjobs.yaml | Marks spec.triggers as mergeable using name as the merge key. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| x-kubernetes-patch-merge-key: name | ||
| x-kubernetes-patch-strategy: merge |
There was a problem hiding this comment.
triggers is being made an associative list keyed by name, but ScaleTriggers.name is optional and (per schema) can be an empty string. With x-kubernetes-patch-strategy: merge, unnamed/empty-name triggers will all share the same merge key, which can cause Kustomize/strategic-merge patches to unexpectedly overwrite or drop triggers. Consider tightening the schema for triggers[].name (eg minLength + clear description that it must be set uniquely when using overlays), and/or using the structural-schema list markers (x-kubernetes-list-type: map + x-kubernetes-list-map-keys: ["name"]) to make the intent explicit.
| x-kubernetes-patch-merge-key: name | ||
| x-kubernetes-patch-strategy: merge |
There was a problem hiding this comment.
triggers is being made an associative list keyed by name, but ScaleTriggers.name is optional and (per schema) can be an empty string. With x-kubernetes-patch-strategy: merge, unnamed/empty-name triggers will all share the same merge key, which can cause Kustomize/strategic-merge patches to unexpectedly overwrite or drop triggers. Consider tightening the schema for triggers[].name (eg minLength + clear description that it must be set uniquely when using overlays), and/or using the structural-schema list markers (x-kubernetes-list-type: map + x-kubernetes-list-map-keys: ["name"]) to make the intent explicit.
| x-kubernetes-patch-merge-key: name | |
| x-kubernetes-patch-strategy: merge | |
| x-kubernetes-list-type: atomic |
This patch makes it possible to use Kustomize to define triggers across Kustomize overlays.
Checklist
make generate-scalers-schemahas been run to update any outdated generated files