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

Export isClusterScoped #95

Closed
wants to merge 1 commit into from
Closed

Export isClusterScoped #95

wants to merge 1 commit into from

Conversation

aliok
Copy link
Contributor

@aliok aliok commented Jan 18, 2023

No description provided.

Copy link
Collaborator

@jcrossley3 jcrossley3 left a comment

Choose a reason for hiding this comment

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

Hi Ali!

@@ -99,7 +99,7 @@ func InjectOwner(owner Owner) Transformer {
}
}

func isClusterScoped(kind string) bool {
func IsClusterScoped(kind string) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As you can tell from the TODO, I always considered this function a kind of placeholder, until I (or someone, hint, hint!) could discover The Right Way. I'm happy to export it, but I wish it wasn't so "hard-codey". 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Brought this up to #controller-runtime Slack channel. link

I guess we can simply use the Discovery API (https://pkg.go.dev/k8s.io/client-go/discovery) to query the APIResource by GVK.

However, there's a performance aspect to think about: every call to isClusterScoped shouldn't go and actually hit the API server. I asked in that Slack thread about a cache+informer+watcher mechanism for APIResources.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jcrossley3 kubernetes-sigs/controller-runtime#2136

That is a new PR that exposes a functionality IsAPINamespaced. Not sure if that can be used as-is, since it requires a scheme and a restMapper.

Copy link
Contributor Author

@aliok aliok Jan 20, 2023

Choose a reason for hiding this comment

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

Anyway, I think we can close this PR and leave isClusterScoped internal. I will use the IsApiNamespaced from kubernetes-sigs/controller-runtime#2136 in my usecase.

@aliok aliok closed this Jan 20, 2023
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