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

[Question] Why not merge SelectorSyncSet and SyncSet into one CRD? #601

Closed
gyliu513 opened this issue Oct 16, 2019 · 6 comments
Closed

[Question] Why not merge SelectorSyncSet and SyncSet into one CRD? #601

gyliu513 opened this issue Oct 16, 2019 · 6 comments

Comments

@gyliu513
Copy link
Contributor

SyncSet can use cluster name while SelectorSyncSet can use cluster labels, why not merge those two CRD into one?

@dgoodwin
Copy link
Contributor

This was our original plan but I believe the determining factor was namespaced vs global. We use namespaces as a small layer of isolation for clusters as those namespaces contain account credentials and certificates. In the case of SyncSet we're tying to specific clusters and thus it made sense to have them be a namespaced CRD. At the time of writing they could contain secrets (we since have a way to break those out), but in general for per cluster content we felt it was best to isolate those into the namespace with the cluster(s) they relate to.

SelectorSyncSet on the other hand we wanted to be a global resource as they typically span many or all clusters in all namespaces. We could do that with some kind of implicit rule like they live in the Hive namespace, but at the time we discussed it felt best to separate them given the differences in their scope.

@dgoodwin
Copy link
Contributor

cc @csrwng @twiest in case I'm misremembering any of that.

@csrwng
Copy link
Contributor

csrwng commented Oct 16, 2019

No, that's correct. We wanted SyncSets to be a namespaced resource because it's associated with a cluster deployment in a specific namespace. SelectorSyncSets can apply to cluster deployments across many namespaces therefore they are a cluster-scoped CRD

@gyliu513
Copy link
Contributor Author

gyliu513 commented Oct 16, 2019

Thanks @dgoodwin and @csrwng , but I think the SelectorSyncSet should be able to cover the case of SyncSet?

@dgoodwin
Copy link
Contributor

It could but it would expose more information globally than we wanted to. SyncSets transfer per cluster certificates, identity providers, lists of dedicated admin usernames, etc. Pushing this up to a global SelectorSyncSet CRD complicates RBAC and potentially exposes us to reveal more information to someone than we wanted to. The distinction between the two offers better flexibility for RBAC in a multi-tenant use of Hive, and possibly better security as well.

@gyliu513
Copy link
Contributor Author

@dgoodwin this make sense, thanks!

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

No branches or pull requests

3 participants