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

Track include and TLS certs with Reloader #164

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

samuelattwood
Copy link
Member

  • Adds logic to recursively find any includes and certs in the NATS config and add them to the file watcher
  • Dependency updates

if len(fileSet) == 0 {
nconfig.ConfigFiles = []string{"/etc/nats-config/gnatsd.conf"}
nconfig.WatchedFiles = []string{"/etc/nats-config/gnatsd.conf"}
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is old code, but is there an opportunity to update this to /etc/nats-config/nats.conf?

Copy link
Member Author

Choose a reason for hiding this comment

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

I only left it in the event that someone is still using that default, but that's probably pretty unlikely at this point so we can change it if no one has concerns.

pkg/natsreloader/natsreloader.go Show resolved Hide resolved
@caleblloyd
Copy link
Contributor

caleblloyd commented Jan 25, 2024

While we are on the subject of regex, I guess we should techncially unit test that the regex handles all of these cases. Can you think of any others?

This is just for the include, but we'd need something similar for TLS certs too

include nats.conf
include nats.conf;    // semicolon terminated
include "nats.conf"   // quoted
include "nats.conf";  // quoted and semicolon terminated
include 'nats.conf'   // are single quotes valid in nats conf?  if so also need this
include 'nats.conf';  // are single quotes valid in nats conf?  if so also need this
include $NATS;        // ignore, this is a variable.  let's not worry about variable interpolation
include "$NATS.conf"  // don't ignore, this is a string not a variable

@samuelattwood
Copy link
Member Author

Golang regex doesn't support negative lookaheads, but I updated the code to handle includes with $ and updated the test to verify these cases

Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

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

LGTM

@samuelattwood samuelattwood merged commit e7edc6d into main Jan 26, 2024
3 checks passed
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