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

Invalid leader election ID is generated #3790

Closed
yaa110 opened this issue Feb 19, 2024 · 9 comments · Fixed by #3876
Closed

Invalid leader election ID is generated #3790

yaa110 opened this issue Feb 19, 2024 · 9 comments · Fixed by #3876
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.

Comments

@yaa110
Copy link

yaa110 commented Feb 19, 2024

What broke? What's expected?

generated LeaderElectionID ends with a period (.) which raises the following error:

E0219 20:35:10.387636       1 leaderelection.go:336] error initially creating leader election record: Lease.coordination.k8s.io "be3caa60." is invalid: metadata.name: Invalid value: "be3caa60.": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')

Reproducing this issue

No response

KubeBuilder (CLI) Version

Version: main.version{KubeBuilderVersion:"3.13.0", KubernetesVendor:"1.27.1", GitCommit:"c8a7cc58eeb56586c019cf8845dad37286d077ff", BuildDate:"2023-11-02T20:43:44Z", GoOs:"linux", GoArch:"amd64"}

PROJECT version

3

Plugin versions

- go.kubebuilder.io/v4

Other versions

go version go1.22.0 linux/amd64
kubectl Client Version: v1.29.2

Extra Labels

No response

@yaa110 yaa110 added the kind/bug Categorizes issue or PR as related to a bug. label Feb 19, 2024
@camilamacedo86
Copy link
Member

Hi @yaa110,

See that the value is generated here: https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/plugins/golang/v4/scaffolds/internal/templates/main.go#L282

LeaderElectionID: "{{ hashFNV .Repo }}.{{ .Domain }}",

So, can you please share with us what values do you use to to create the project?
How we can reproduce it so that it can help us to fix it?

@camilamacedo86 camilamacedo86 added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Feb 26, 2024
@camilamacedo86
Copy link
Member

camilamacedo86 commented Feb 26, 2024

This one seems to be a good first issue.
What we need to do here:

@camilamacedo86 camilamacedo86 added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Feb 26, 2024
@Bharadwajshivam28
Copy link

Hey @camilamacedo86 I can take a look on this issue

@Bharadwajshivam28
Copy link

I need some steps to get started as I am new contributor

@yaa110
Copy link
Author

yaa110 commented Feb 26, 2024

Thank you @camilamacedo86. According to the source code you shared, the issue is connected to the empty domain value I set for the init command:

kubebuilder init --domain "" --repo github.com/blah/blah --project-name mykind
kubebuilder create api --group domain.com --version v1 --kind MyKind

It would be nice to also take this situation into consideration.

@camilamacedo86
Copy link
Member

camilamacedo86 commented Feb 26, 2024

@yaa110

Great job !!! thank you for check this one

kubebuilder init --domain "" --repo github.com/blah/blah --project-name mykind

So, the root cause is to empty the domain.
Why should we allow that? Why did you create the project with an empty domain?
Therefore, just adding a validation to ensure it fails should be enough.
I do not know if other options will properly work with an empty domain.

@yaa110

Would you like to push a PR so we add validation and do not allow this scenario?
WDYT?

@yaa110
Copy link
Author

yaa110 commented Feb 26, 2024

@camilamacedo86 I intended to use an API group of domain.com. Therefore, if I had set the domain, I would have expected to create the following resource sample, which was deemed unacceptable (observe the value of the apiVersion field):

using:

kubebuilder init --domain "domain.com" --repo github.com/blah/blah --project-name mykind
kubebuilder create api --group domain.com --version v1 --kind MyKind

generates:

apiVersion: domain.com.domain.com/v1
kind: MyKind
spec: {}

however, using aforementioned commands (with empty domain), generates:

apiVersion: domain.com/v1
kind: MyKind
spec: {}

Is it a bad idea to leave the domain blank? Does it cause issues beyond the leader election ID? Wouldn't it be better to allow the domain to be empty and change the templating code accordingly to support it?

{{ if .Domain }}
LeaderElectionID:       "{{ hashFNV .Repo }}.{{ .Domain }}",
{{ else }}
LeaderElectionID:       "{{ hashFNV .Repo }}",
{{ end }}

@uos-ljtian
Copy link
Contributor

Maybe I can check and see if I need to submit a PR

@camilamacedo86
Copy link
Member

camilamacedo86 commented Apr 6, 2024

Hi @yaa110,

The "domain" and "group" serve different purposes in structuring and naming resources. A "domain" usually refers to the base DNS domain name used for all resources, serving as a namespace to group them under a common name, preventing conflicts across different projects or organizations.

A "group", on the other hand, is a subdivision within the domain that categorizes related kinds of resources, allowing for clearer organization and resource management within the domain.

Example if your base domain is example.com, you might define a resource in the ships group. The full API Group for a resource named Sloop would be ships.example.com.

So, the domain should be filled with the domain of your project.

Using empty domains in Kubernetes could bring issues like:

  • Naming Conflicts: Easier creation of resources that might conflict with others, potentially causing overwrites or interference.
  • Identification Problems: Complications in identifying or categorizing resources, affecting management and deployments.
  • Integration Challenges: Potential difficulties in integrating with tools or systems that expect domain-based naming conventions.

Therefore, I thought in not allow empty domains. However, others might either done the same.
In this way, the second option approach might be as you suggested use only the repo instead of the repo + domain seems OK too

I am OK within as well if you want to push this change.`

replace: "{{ hashFNV .Repo }}.{{ .Domain }}",

with: LeaderElectionID: "{{ hashFNV .Repo }}", (ONLY when the domain is not informed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants