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

reload service registration configuration on SIGHUP #17598

Conversation

kevinschoonover
Copy link
Contributor

If the consul token is ever rotated in a service configuration on disk, vault will never pick up the changes and will be unable to perform future service registrations.

image

This change reloads the configuration for service registrations whenever vault receives SIGHUP for consul by recreating the consul client since consul doesn't have a clear API for editting an agent's ACL token.

You can test these changes locally by:

  1. creating the following configuration
    # service_registration.hcl
    service_registration "consul" {
      token = "test"
    }
    
  2. running consul and vault in dev mode
    VAULT_LOG_LEVEL=debug ~vault server -dev -config service_registration.hcl
    consul agent -dev
    
  3. editing the service_registration.hcl file
  4. sending HUP to vault kill -HUP $(pidof vault)

@kevinschoonover kevinschoonover force-pushed the kschoon/reload-service-registration branch from 4858d8f to bf741bf Compare July 22, 2023 03:17
@gbolo
Copy link

gbolo commented Aug 28, 2023

can we get some traction on this? Vault should have the ability to reload service_registration without having to restart and unseal

@kevinschoonover
Copy link
Contributor Author

@VioletHynes - was hoping to take advantage of your offer while the iron is hot. Not sure if this applied to only the consul-template repo.

@VioletHynes
Copy link
Contributor

Applies here too! I'll try and get some eyes on it, thank you!

Copy link
Contributor

@marcboudreau marcboudreau left a comment

Choose a reason for hiding this comment

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

LGTM

@marcboudreau
Copy link
Contributor

I'm just looking into a panic that was thrown from one of the tests.

Copy link
Contributor

@marcboudreau marcboudreau left a comment

Choose a reason for hiding this comment

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

Sorry, that I didn't pick this up before, but one of the tests showed that ServiceRegistration can be nil, which leads to a panic.

command/server.go Outdated Show resolved Hide resolved
@marcboudreau marcboudreau merged commit c0ea7b1 into hashicorp:main May 9, 2024
66 of 67 checks passed
@marcboudreau
Copy link
Contributor

Thanks for your submission @kevinschoonover! ❤️

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.

None yet

4 participants