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

Is clusterresourcebinding.name suitable by kind and name #3627

Open
weilaaa opened this issue Jun 2, 2023 · 8 comments
Open

Is clusterresourcebinding.name suitable by kind and name #3627

weilaaa opened this issue Jun 2, 2023 · 8 comments
Labels
kind/question Indicates an issue that is a support question.
Milestone

Comments

@weilaaa
Copy link
Member

weilaaa commented Jun 2, 2023

I have two different resources, the Kind is the same, but Group and Version are not. Those as bellow:

clusters.cluster.example1.com/v1 named cluster.a
clusters.cluster.example2.com/v1beta1 named cluster.a

Now, I noticed clusterresourcebinding.name is generated by kind and name which means for clusters.cluster.example1.com/v1 and clusters.cluster.example2.com/v1beta1, the name of their clusterresourcebinding is the same, both are cluster.a-cluster. This seems to be problematic.

// GenerateBindingName will generate binding name by kind and name
func GenerateBindingName(kind, name string) string {
	// The name of resources, like 'Role'/'ClusterRole'/'RoleBinding'/'ClusterRoleBinding',
	// may contain symbols(like ':') that are not allowed by CRD resources which require the
	// name can be used as a DNS subdomain name. So, we need to replace it.
	// These resources may also allow for other characters(like '&','$') that are not allowed
	// by CRD resources, we only handle the most common ones now for performance concerns.
	// For more information about the DNS subdomain name, please refer to
	// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names.
	if strings.Contains(name, ":") {
		name = strings.ReplaceAll(name, ":", ".")
	}

	return strings.ToLower(name + "-" + kind)
}
@weilaaa weilaaa added the kind/question Indicates an issue that is a support question. label Jun 2, 2023
@weilaaa
Copy link
Member Author

weilaaa commented Jun 2, 2023

@RainbowMango cc

@chaunceyjiang
Copy link
Member

Yes, This is a known issue. refer to #2969

@RainbowMango
Copy link
Member

Hi @weilaaa glad to see you are back.
Yes, it's a known issue and also a tricky one. We don't have a better solution for it yet. :(

@weilaaa
Copy link
Member Author

weilaaa commented Jun 2, 2023

I read the pr #2969 and all guys comments , IMHO, I don't like RB name with two format exist, the one is kind-name, the other is kind-name-rand.String(5), which makes user confused, even though RB is not user-face api. K8s used CollisionCount but it kept final name as name-hash.

@weilaaa
Copy link
Member Author

weilaaa commented Jun 2, 2023

I ask is there a way to migrate the existing RB data to the new RB named by name-hash without affecting the user's use.

For example, we clone the corresponding new RB based on the existing RB, and mark the existing RB as deprecated. At this time, the newly added RB will use the new name-hash name format. Just in case, we can also record CollisionCount in the annotation to prevent hash collisions. Besides that, GenerateBindingName does only one thing, simply generating the name is like K8s generating the bash suffix.

@RainbowMango RainbowMango added this to the v1.7 milestone Jun 4, 2023
@RainbowMango
Copy link
Member

I ask is there a way to migrate the existing RB data to the new RB named by name-hash without affecting the user's use.

Yes, this is also the thing that concerns me the most. I expect a solution that could transform the legacy RB without perception.

For example, we clone the corresponding new RB based on the existing RB, and mark the existing RB as deprecated. At this time, the newly added RB will use the new name-hash name format.

Sounds good to me. Speaking of the naming rule of RB, the naming of "Work" also has similar issues. I analyzed the issue and draft some ideas at this document, you can take a look.

This is a very challenging task, and now is also a very good time to solve this problem(Now is the beginning of the v1.7 version cycle.) I'll appreciate it if you want to lead the effort.

@weilaaa
Copy link
Member Author

weilaaa commented Jun 6, 2023

Thanks for the chance. I'll do some investigation first to check out the impact and possible solutions.

@RainbowMango
Copy link
Member

I'm trying to figure out which PR/Issue should be included in the coming v1.7 release which is planned at the end of this month.
I guess we don't have enough time for this feature, so I'm moving this to v1.8.

FYI, We are working on this, the relevant topic was talked at the community meeting, and see #4000 for the progress.

@RainbowMango RainbowMango modified the milestones: v1.7, v1.8 Aug 25, 2023
@RainbowMango RainbowMango modified the milestones: v1.8, v1.9 Jan 9, 2024
@RainbowMango RainbowMango modified the milestones: v1.9, v1.10 Feb 29, 2024
@RainbowMango RainbowMango modified the milestones: v1.10, v1.11 May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/question Indicates an issue that is a support question.
Projects
None yet
Development

No branches or pull requests

3 participants