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

Add validation of ProvisioningRequest's Parameters #7511

Open
PBundyra opened this issue Nov 20, 2024 · 8 comments · May be fixed by #7547
Open

Add validation of ProvisioningRequest's Parameters #7511

PBundyra opened this issue Nov 20, 2024 · 8 comments · May be fixed by #7547
Assignees
Labels
area/cluster-autoscaler kind/feature Categorizes issue or PR as related to a new feature.

Comments

@PBundyra
Copy link
Contributor

Which component are you using?:
cluster-autoscaler

Is your feature request designed to solve a problem? If so describe the problem this feature should solve.:

Currently if a user sets invalid key or values of ProvisioningRequest's Parameter they get no information that the Parameters is invalid.

Describe the solution you'd like.:
Validate keys and values of supported ProvisioningRequest's Parameters

Describe any alternative solutions you've considered.:
Not validate parameters and rely on the user to provide correct keys and values

Additional context.:

#7496 (comment)

@PBundyra PBundyra added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 20, 2024
@PBundyra
Copy link
Contributor Author

cc @aleksandra-malinowska

@PBundyra
Copy link
Contributor Author

/area cluster-autoscaler

@omerap12
Copy link
Member

I can help with that.
/assign

@omerap12
Copy link
Member

omerap12 commented Dec 1, 2024

I added CEL validation in the CRD to validate parameters for check-capacity.autoscaling.x-k8s.io provisioning class, what do you think?

@omerap12
Copy link
Member

omerap12 commented Dec 1, 2024

@PBundyra
Copy link
Contributor Author

PBundyra commented Dec 2, 2024

@omerap12
Copy link
Member

omerap12 commented Dec 2, 2024

Looks great, thanks @omerap12

If looks valid we can remove those lines: https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/provisioningrequest/checkcapacity/provisioningclass.go#L155-#L157

Agreed

Thanks, so I will have another commit to clean the previous approach

@omerap12
Copy link
Member

omerap12 commented Dec 2, 2024

@PBundyra Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cluster-autoscaler kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants