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

allow for custom domain in k8s-nameserver #3

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Conversation

thirdeyenick
Copy link

This allows to have a custom magicDNS domain for the k8s-nameserver.

@@ -75,6 +75,10 @@ type DNSConfigSpec struct {
// Tailscale Ingresses. The operator will always deploy this nameserver
// when a DNSConfig is applied.
Nameserver *Nameserver `json:"nameserver"`
// Domain is the domain for which DNS entries will be resolved.
// Defaults to ts.net if empty.
Copy link

Choose a reason for hiding this comment

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

The flag in the nameserver defaults to ts.net but here it does not so this comment is not super accurate. Maybe a kubebuilder default here would make sense?

Copy link
Author

Choose a reason for hiding this comment

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

I was thinking about this, but I didn't see any kubebuilder default annotation used in any other type file. That is why I went without. I could change the comment to

If left empty, the default of the k8s-nameserver will be used.

?

Copy link
Author

Choose a reason for hiding this comment

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

Furthermore the domain is currently defined by the control-server and by the k8s-nameserver, but the k8s-operator is quite domain agnostic. The "ts.net" domain is only used in test files.

Copy link

@ctrox ctrox Dec 3, 2024

Choose a reason for hiding this comment

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

If left empty, the default of the k8s-nameserver will be used.

Fine with me

Copy link
Author

Choose a reason for hiding this comment

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

I changed it.

@coveralls
Copy link

coveralls commented Dec 3, 2024

Pull Request Test Coverage Report for Build 12138963348

Details

  • 2 of 9 (22.22%) changed or added relevant lines in 2 files are covered.
  • 145 unchanged lines in 22 files lost coverage.
  • Overall coverage decreased (-0.09%) to 51.308%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/k8s-nameserver/main.go 0 3 0.0%
cmd/k8s-operator/nameserver.go 2 6 33.33%
Files with Coverage Reduction New Missed Lines %
cmd/sniproxy/server.go 1 44.02%
tsnet/tsnet.go 1 72.39%
wgengine/userspace.go 1 67.15%
k8s-operator/sessionrecording/ws/message.go 2 91.8%
types/persist/persist.go 2 88.46%
control/controlclient/direct.go 2 62.48%
net/tstun/wrap.go 2 78.87%
portlist/poller.go 2 83.78%
control/controlclient/auto.go 3 76.48%
portlist/portlist.go 3 87.8%
Totals Coverage Status
Change from base Build 12054923117: -0.09%
Covered Lines: 55874
Relevant Lines: 108899

💛 - Coveralls

This allows for a custom domain in the k8s-nameserver application. It can be set via an argument to the binary and will default to ts.net if not set.
@thirdeyenick thirdeyenick merged commit 36903c2 into main Dec 3, 2024
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants