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

modules/nebula: add nebula overlay network #1285

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

siriobalmelli
Copy link
Contributor

Added on a similar pattern as netbird; this is in production on our systems locally.

@siriobalmelli siriobalmelli force-pushed the sb/nebula branch 2 times, most recently from 7c2c315 to 4809ed4 Compare February 5, 2025 09:42
Comment on lines +144 to +148
tun.device = mkOption {
type = types.nullOr types.str;
default = null;
description = "Name of the tun device. Defaults to nebula.\${networkName}.";
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
tun.device = mkOption {
type = types.nullOr types.str;
default = null;
description = "Name of the tun device. Defaults to nebula.\${networkName}.";
};
tun.device = mkOption {
type = types.str;
default = "nebula.\${networkName}";
description = "Name of the TUN device.";
};

You'll probably also need to set defaultText

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure this is a good idea.

It appears that on Darwin the tunneling interface currently must be utun[0-9]+.

The parameter is not required, so I don't see an advantage to setting a default.

See https://nebula.defined.net/docs/config/tun/#tundev

Comment on lines 249 to 251
script = ''
exec ${cfg.package}/bin/nebula -config ${configFile}
'';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
script = ''
exec ${cfg.package}/bin/nebula -config ${configFile}
'';
command = "${cfg.package}/bin/nebula -config ${configFile}";

type = types.attrsOf (
types.submodule {
options = {
enable = mkOption {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
enable = mkOption {
enable = mkEnableOption {

Can you replace all the booleans that default to false and just enable logic with mkEnableOption?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implementing this change yields errors like:

error: cannot coerce a set to a string: { type = «thunk»; description = "Whether this node is a lighthouse."; }

I've reverted this for now, as it seems to be a stylistic rather than operational point - and I am using this in prod.

If there are in fact compelling reasons for this change, which I'm not tracking with, please help point me in the right direction 🙏

@siriobalmelli siriobalmelli marked this pull request as draft March 6, 2025 10:21
@siriobalmelli
Copy link
Contributor Author

Apologies for the delay, I was on leave.

Thanks for the review and suggestions, added @Enzime as co-author

@siriobalmelli siriobalmelli force-pushed the sb/nebula branch 2 times, most recently from 91c39d5 to e62583f Compare March 6, 2025 12:27
@siriobalmelli siriobalmelli marked this pull request as ready for review March 6, 2025 15:07
@siriobalmelli siriobalmelli requested a review from Enzime March 6, 2025 15:07
Signed-off-by: Sirio Balmelli <[email protected]>
Co-authored-by: Michael Hoang <[email protected]>
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

Successfully merging this pull request may close these issues.

3 participants