Skip to content
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

chore: removing webhook validation in favor of kubebuilder CRD validation, and removing old SIG Gallery Logic #10

Conversation

Bryce-Soghigian
Copy link
Collaborator

@Bryce-Soghigian Bryce-Soghigian commented Nov 7, 2023

Fixes #

Description
We removed ImageID from the AKSNodeClass API, and plan to add it back in the future, so i want to reduce/remove all validation in the webhooks, so removed here, and added Kubebuilder Validation instead for when we do turn it back on.

How was this change tested?
Temporarily added back in ImageID validated it validated our requests, and removed it again.

  • make presubmit

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note

Replaced ImageID Validation webhook with CEL Validation

Copy link
Collaborator Author

@Bryce-Soghigian Bryce-Soghigian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/test

@Bryce-Soghigian Bryce-Soghigian changed the title chore: removing webhook validation in favor of cel validation, and removing old SIG Gallery Logic chore: removing webhook validation in favor of kubebuilder CRD validation, and removing old SIG Gallery Logic Nov 7, 2023
@Bryce-Soghigian
Copy link
Collaborator Author

Bryce-Soghigian commented Nov 7, 2023

Seems CI doesnt work from Fork Yet, makes sense, will reopen from branch, or can wait for it to be in place

Copy link
Collaborator Author

@Bryce-Soghigian Bryce-Soghigian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/test

Copy link
Collaborator Author

@Bryce-Soghigian Bryce-Soghigian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/test

@Bryce-Soghigian
Copy link
Collaborator Author

Seems the be lowest priority, we can close and reopen later if its needed

@Bryce-Soghigian Bryce-Soghigian deleted the bsoghigian/removing-remaining-webhook branch January 13, 2024 08:31
@Bryce-Soghigian Bryce-Soghigian restored the bsoghigian/removing-remaining-webhook branch February 20, 2024 02:15
@coveralls
Copy link

Pull Request Test Coverage Report for Build 7967579848

Details

  • -1 of 3 (66.67%) changed or added relevant lines in 2 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.05%) to 97.597%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/apis/v1alpha2/aksnodeclass_validation.go 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
pkg/apis/v1alpha2/aksnodeclass_validation.go 3 0.0%
Totals Coverage Status
Change from base Build 7874902941: 0.05%
Covered Lines: 35737
Relevant Lines: 36617

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants