Manage contributor add failure - rolebinding unique name issue

[lalithvaka]

/kind bug

What steps did you take and what happened:
Manage contributor screen. Adding a contributor creates a role binding / service role binding with name including userID's first character by ignoring the digits and not able to add other users with similar numbers.
our Org uses UserIDs as a1234@xx.org, a2345@xx.org.

What did you expect to happen:
rolebinding and service role bindings names to use digits too

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

• Kubeflow version: (version number can be found at the bottom left corner of the Kubeflow dashboard):1.0.2
• kfctl version: (use kfctl version):
• Kubernetes platform: (e.g. minikube)
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

[issue-label-bot]

Issue-Label Bot is automatically applying the labels:

Label	Probability
kind/bug	0.95

Please mark this comment with 👍 or 👎 to give our bot feedback!
Links: app homepage, dashboard and code for this bot.

[yanniszark]

This is the code that decides the name of RoleBindings/ServiceRoleBindings:

kubeflow/components/access-management/kfam/bindings.go

Lines 56 to 72 in 8f8e348

	//getBindingName returns bindingName, which is combination of user kind, username, RoleRef kind, RoleRef name.
	func getBindingName(binding *Binding) (string, error) {
	nameRaw := strings.ToLower(
	strings.Join([]string{
	binding.User.Kind,
	url.QueryEscape(binding.User.Name),
	binding.RoleRef.Kind,
	binding.RoleRef.Name,
	},"-"),
	)
	// Only keep lower case letters, replace other with -
	reg, err := regexp.Compile("[^a-z]+")
	if err != nil {
	return "", err
	}
	return reg.ReplaceAllString(nameRaw, "-"), nil
	}

We should probably amend that logic.
I'm not sure what priority to put on this.
We should decide our plans for the contributor UI first, because if we are moving towards a GitOps approach, the contributor UI becomes less valuable.
On the other hand, this should be an easy fix if we can decide on what naming rule to use.

/area multiuser
cc @jlewi @kunmingg

[lalithvaka]

Thank you @yanniszark , I was able to fix this code(with my limited go knowledge) to support numbers in the email address. This changes email address to i397401@zq.msds.kp.org to i397401-zq-msds-kp-org which is being used in the role binding names.

//getBindingName returns bindingName, which is combination of user kind, username, RoleRef kind, RoleRef name.
func getBindingName(binding *Binding) (string, error) {
	// Only keep lower case letters, replace other with -
	reg, err := regexp.Compile("[^a-z0-9]+")
	if err != nil {
		return "", err
	}
	nameRaw := strings.ToLower(
		strings.Join([]string{
			binding.User.Kind,
			url.QueryEscape(reg.ReplaceAllString(binding.User.Name, "-")),
			binding.RoleRef.Kind,
			binding.RoleRef.Name,
		}, "-"),
	)

	return reg.ReplaceAllString(nameRaw, "-"), nil
}

[lalithvaka]

@yanniszark submitted a pull request - #4979

/area multiuser
cc @jlewi @kunmingg