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 list by index to ClusterClass webhook #5719

Closed
killianmuldoon opened this issue Nov 23, 2021 · 2 comments · Fixed by #5810
Closed

Add list by index to ClusterClass webhook #5719

killianmuldoon opened this issue Nov 23, 2021 · 2 comments · Fixed by #5810
Labels
area/clusterclass Issues or PRs related to clusterclass
Milestone

Comments

@killianmuldoon
Copy link
Contributor

The ClusterClass webhook in PR #5717 lists all clusters and then cycles through them to find those using the clusterClass being validated. Instead the ClusterClass webhook should use index.ClusterClassNameField to pull the correct clusters directly.

This workflow is currently blocked as it requires envtest to test the indices. fakeClient does not populate indices like index.ClusterClassNameField. The index currently can't be tested as there is a circular dependency between webhooks and envtest.

The index should be added once the circular dependency with webhooks has been removed.

c/f #5717 (comment)

@sbueringer
Copy link
Member

sbueringer commented Nov 23, 2021

/area topology
I'll open the issue to remove the circular dependency ASAP
(I think we need to do this anyway to enable more realistic unit tests for the variable defaulting/validation)

@fabriziopandini
Copy link
Member

/milestone v1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clusterclass Issues or PRs related to clusterclass
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants