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

Clean up propagated parameter/workspace validation #6647

Closed
lbernick opened this issue May 11, 2023 · 3 comments · Fixed by #6684
Closed

Clean up propagated parameter/workspace validation #6647

lbernick opened this issue May 11, 2023 · 3 comments · Fixed by #6684
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@lbernick
Copy link
Member

Currently, some validation logic for parameters and workspaces can be done in the admission webhook, but due to param/workspace propagation, some of it must happen in the reconciler. Currently, we get around this by skipping some forms of validation if sentinel values are injected into the context. Instead, we should split validation into parts that must happen in the reconciler, and parts that can happen in the webhook, and call validation logic in the appropriate place.

This work blocks #6616.

@lbernick lbernick added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label May 11, 2023
@lbernick lbernick self-assigned this May 11, 2023
@chitrangpatel
Copy link
Contributor

/assign

@pritidesai
Copy link
Member

Thanks @lbernick, one question. My knowledge is outdated at this point in terms of validation. But at some point in the past, we had discussed about validating specifications once instead of validating them in every reconcile cycle. Have we thought about exploring that route?

#4562

@lbernick
Copy link
Member Author

Thanks @pritidesai, I think this issue isn't intended to have any functional changes, it's just intended to make the codebase more readable and unblock #6616. I'm trying to make it easier to tell what validation is happening at various points by getting rid of the sentinel values that indicate which forms of validation to do.

We could definitely explore validating only once per reconcile loop-- I think #6616 or #6645 is the best place for that conversation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants