-
Notifications
You must be signed in to change notification settings - Fork 235
Pass registered resource names to config.Validate
#423
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
Pass registered resource names to config.Validate
#423
Conversation
5046b61 to
28d5f3c
Compare
| if err := ackCfg.Validate(); err != nil { | ||
| managerFactories := svcresource.GetManagerFactories() | ||
| resourceKinds := make([]string, 0, len(managerFactories)) | ||
| for _, mf := range managerFactories { |
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.
Maybe we should just pass the entire GroupKind, so that different downstream validation can use whichever naming and casing it needs
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.
Agreed. Changed to GroupKinds
config.Validateconfig.Validate
28d5f3c to
6ef3d4f
Compare
0753eca to
af878bc
Compare
|
/unhold |
|
/retest |
|
/test s3-controller-test |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/test all |
templates/cmd/controller/main.go.tpl
Outdated
|
|
||
| if err := ackCfg.Validate(); err != nil { | ||
| managerFactories := svcresource.GetManagerFactories() | ||
| resourceGroupKinds := make([]schema.GroupVersionKind, 0, len(managerFactories)) |
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.
nit: These are now GVKs
Completes aws-controllers-k8s/runtime#117 This patch adds a tiny modification to the controllers `main.go` that passes the resource names to `ackCfg.Validate` method. Needs a new runtime release. /hold Signed-off-by: Amine Hilaly <[email protected]>
af878bc to
3a51a86
Compare
jaypipes
left a comment
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.
👍 thanks Amine!
|
/retest |
jaypipes
left a comment
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.
👍
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: A-Hilaly, jaypipes, RedbackThomson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[fixes https://github.com/aws-controllers-k8s/community/issues/1647]
Completes aws-controllers-k8s/runtime#117
This patch adds a tiny modification to the controllers
main.gothatpasses the resource names to
ackCfg.Validatemethod.Needs a new runtime release.
/hold
Signed-off-by: Amine Hilaly [email protected]
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.