-
Notifications
You must be signed in to change notification settings - Fork 583
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
Update AWSMachine webhook validate logic on update to be consistent #3728
Update AWSMachine webhook validate logic on update to be consistent #3728
Conversation
Hi @cnmcavoy. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Need to address #3109 (comment) first. |
/ok-to-test |
2701f7f
to
3c098fc
Compare
@cnmcavoy is this still WIP? Would you be able to continue with this PR? |
I have an open question here: #3109 (comment) If we don't want to address, this can be unmarked as WIP and merged. |
3c699be
to
2f7a99f
Compare
I added logic to the AWSMachineTemplate so that the validation behavior is consistent with the existing on create behavior of AWSMachine's as well. |
2f7a99f
to
1037077
Compare
/test pull-cluster-api-provider-aws-e2e-blocking |
@cnmcavoy I think the update part is still not done right, as discussed in the original issue? |
…ith validation on create
…istent with AWSMachine webhook validation on create
1037077
to
e23b67d
Compare
@Ankitasw I realize there description of this change is lacking and the issue it fixes is rather brief as well. Here's a summary of the changes in the PR: The existing AWSMachine webhook validates
The existing AWSMachine webhook validates
My change addresses the missing validation of the security groups Additionally, the existing AWSMachineTemplate webhook validates that
The existing AWSMachineTemplate webhook validates
My change addresses all of the missing validation from the AWSMachineTemplate
There shouldn't be any changes to the API contract in this PR, only enforcement of the existing contract. Is there anything that is incorrect or I left out in this? Please let me know if so. |
/test pull-cluster-api-provider-aws-e2e-blocking |
/easycla |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Ankitasw The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Update AWSMachine webhook validate logic on update to be consistent with validation on creation.
What type of PR is this?
/kind bug
What this PR does / why we need it:
onUpdate
for the AWSMachine webhook.onCreate
validation to be consistent with the AWSMachineonCreate
validation.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #3109 & #3105
Special notes for your reviewer:
Checklist:
Release note: