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.

Sign up for GitHub

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

Jump to bottom

Fix #4076 - Agent configured Watches now work with HTTPS only agents #4184

Merged
merged 3 commits into from
Jun 1, 2018
Merged

Fix #4076 - Agent configured Watches now work with HTTPS only agents #4184

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e0e239
Allow passing in a config to the watch plan to use when creating the …
mkeeler May 31, 2018
53fbe2b
Update unit tests to reflect change to func signature
mkeeler May 31, 2018
1fbe828
Add RunWithConfig and put Run signature back to normal
mkeeler Jun 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,14 +646,19 @@ func (a *Agent) reloadWatches(cfg *config.RuntimeConfig) error {

// Determine the primary http(s) endpoint.
var netaddr net.Addr
https := false
if len(cfg.HTTPAddrs) > 0 {
netaddr = cfg.HTTPAddrs[0]
} else {
netaddr = cfg.HTTPSAddrs[0]
https = true
}
addr := netaddr.String()
if netaddr.Network() == "unix" {
addr = "unix://" + addr
https = false
} else if https {
addr = "https://" + addr
}

// Fire off a goroutine for each new watch plan.
Expand All @@ -669,7 +674,19 @@ func (a *Agent) reloadWatches(cfg *config.RuntimeConfig) error {
wp.Handler = makeHTTPWatchHandler(a.LogOutput, httpConfig)
}
wp.LogOutput = a.LogOutput
if err := wp.Run(addr); err != nil {

config := api.DefaultConfig()
if https {
if a.config.CAPath != "" {
config.TLSConfig.CAPath = a.config.CAPath
}
if a.config.CAFile != "" {
config.TLSConfig.CAFile = a.config.CAFile
}
config.TLSConfig.Address = addr
}

if err := wp.Run(addr, config); err != nil {
a.logger.Printf("[ERR] agent: Failed to run watch: %v", err)
}
}(wp)
Expand Down
20 changes: 20 additions & 0 deletions agent/agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2206,3 +2206,23 @@ func TestAgent_reloadWatches(t *testing.T) {
t.Fatalf("bad: %s", err)
}
}

func TestAgent_reloadWatchesHTTPS(t *testing.T) {
t.Parallel()
a := TestAgent{Name: t.Name(), UseTLS: true}
a.Start()
defer a.Shutdown()

// Normal watch with http addr set, should succeed
newConf := *a.config
newConf.Watches = []map[string]interface{}{
{
"type": "key",
"key": "asdf",
"args": []interface{}{"ls"},
},
}
if err := a.reloadWatches(&newConf); err != nil {
t.Fatalf("bad: %s", err)
}
}
2 changes: 1 addition & 1 deletion command/watch/watch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func (c *cmd) Run(args []string) int {
}()

// Run the watch
if err := wp.Run(c.http.Addr()); err != nil {
if err := wp.Run(c.http.Addr(), nil); err != nil {
c.UI.Error(fmt.Sprintf("Error querying Consul agent: %s", err))
return 1
}
Expand Down
18 changes: 9 additions & 9 deletions watch/funcs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestKeyWatch(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()
if err := plan.Run(a.HTTPAddr()); err != nil {
if err := plan.Run(a.HTTPAddr(), nil); err != nil {
t.Fatalf("err: %v", err)
}
}()
Expand Down Expand Up @@ -118,7 +118,7 @@ func TestKeyWatch_With_PrefixDelete(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()
if err := plan.Run(a.HTTPAddr()); err != nil {
if err := plan.Run(a.HTTPAddr(), nil); err != nil {
t.Fatalf("err: %v", err)
}
}()
Expand Down Expand Up @@ -171,7 +171,7 @@ func TestKeyPrefixWatch(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()
if err := plan.Run(a.HTTPAddr()); err != nil {
if err := plan.Run(a.HTTPAddr(), nil); err != nil {
t.Fatalf("err: %v", err)
}
}()
Expand Down Expand Up @@ -225,7 +225,7 @@ func TestServicesWatch(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()
if err := plan.Run(a.HTTPAddr()); err != nil {
if err := plan.Run(a.HTTPAddr(), nil); err != nil {
t.Fatalf("err: %v", err)
}
}()
Expand Down Expand Up @@ -276,7 +276,7 @@ func TestNodesWatch(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()
if err := plan.Run(a.HTTPAddr()); err != nil {
if err := plan.Run(a.HTTPAddr(), nil); err != nil {
t.Fatalf("err: %v", err)
}
}()
Expand Down Expand Up @@ -332,7 +332,7 @@ func TestServiceWatch(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()
if err := plan.Run(a.HTTPAddr()); err != nil {
if err := plan.Run(a.HTTPAddr(), nil); err != nil {
t.Fatalf("err: %v", err)
}
}()
Expand Down Expand Up @@ -393,7 +393,7 @@ func TestChecksWatch_State(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()
if err := plan.Run(a.HTTPAddr()); err != nil {
if err := plan.Run(a.HTTPAddr(), nil); err != nil {
t.Fatalf("err: %v", err)
}
}()
Expand Down Expand Up @@ -459,7 +459,7 @@ func TestChecksWatch_Service(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()
if err := plan.Run(a.HTTPAddr()); err != nil {
if err := plan.Run(a.HTTPAddr(), nil); err != nil {
t.Fatalf("err: %v", err)
}
}()
Expand Down Expand Up @@ -510,7 +510,7 @@ func TestEventWatch(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()
if err := plan.Run(a.HTTPAddr()); err != nil {
if err := plan.Run(a.HTTPAddr(), nil); err != nil {
t.Fatalf("err: %v", err)
}
}()
Expand Down
6 changes: 4 additions & 2 deletions watch/plan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ const (
)

// Run is used to run a watch plan
func (p *Plan) Run(address string) error {
func (p *Plan) Run(address string, conf *consulapi.Config) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worth keeping this signature the same and moving to a RunWithConfig method so we don't break tools that import this package. It is pretty small, though - what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats probably best. Nothing in our code does but I guess the api can be imported by other tools.

// Setup the client
p.address = address
conf := consulapi.DefaultConfig()
if conf == nil {
conf = consulapi.DefaultConfig()
}
conf.Address = address
conf.Datacenter = p.Datacenter
conf.Token = p.Token
Expand Down
2 changes: 1 addition & 1 deletion watch/plan_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestRun_Stop(t *testing.T) {

errCh := make(chan error, 1)
go func() {
errCh <- plan.Run("127.0.0.1:8500")
errCh <- plan.Run("127.0.0.1:8500", nil)
}()

select {
Expand Down