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

Default pki.disconnect_invalid to true and make it reloadable #859

Merged
merged 3 commits into from
Nov 13, 2023

Conversation

nbrownus
Copy link
Collaborator

@nbrownus nbrownus commented May 4, 2023

With #838 merged there is a path to refresh certificates and rekey tunnels without having to restart the nebula process. It would be a good practice to flip this to true.

@nbrownus nbrownus added this to the v1.7.0 milestone May 4, 2023
Copy link
Collaborator

@jasikpark jasikpark left a comment

Choose a reason for hiding this comment

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

would be nice! i don't understand the code, but it looks right?

@nbrownus nbrownus force-pushed the default-disconnect-invalid branch from 6e6e895 to 8cdb7d7 Compare May 4, 2023 22:08
@nbrownus nbrownus modified the milestones: v1.7.0, v1.8.0 May 4, 2023
@@ -170,7 +169,6 @@ func NewInterface(ctx context.Context, c *InterfaceConfig) (*Interface, error) {
writers: make([]*udp.Conn, c.routines),
readers: make([]io.ReadWriteCloser, c.routines),
caPool: c.caPool,
disconnectInvalid: c.disconnectInvalid,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need ifce.disconnectInvalid.Store(c.disconnectInvalid) below?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like no, because we call ifce.reloadDisconnectInvalid(c) after calling NewInterface from main.go.

This is contrast to ifce.certState which is handled internally in this function.

I think this approach is a little awkward because the caller may not be aware they need to initialize this value.

@nbrownus nbrownus merged commit 3356e03 into master Nov 13, 2023
7 checks passed
@nbrownus nbrownus deleted the default-disconnect-invalid branch November 13, 2023 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants