Skip to content

Commit dd91ed3

Browse files
committed
cli-plugins/manager: refactor for easier debugging
Extract the code inside the loop to a closure, so that we can more easily set up debug-logging. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent 0501cf8 commit dd91ed3

1 file changed

Lines changed: 38 additions & 17 deletions

File tree

cli-plugins/manager/hooks.go

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ package manager
66
import (
77
"context"
88
"encoding/json"
9+
"errors"
10+
"fmt"
11+
"strconv"
912
"strings"
1013

1114
"github.com/docker/cli/cli-plugins/hooks"
@@ -66,47 +69,65 @@ func invokeAndCollectHooks(ctx context.Context, cfg *configfile.ConfigFile, root
6669

6770
pluginDirs := getPluginDirs(cfg)
6871
nextSteps := make([]string, 0, len(pluginsCfg))
69-
for pluginName, pluginCfg := range pluginsCfg {
70-
match, ok := pluginMatch(pluginCfg, subCmdStr, cmdErrorMessage)
71-
if !ok {
72-
continue
72+
73+
tryInvokeHook := func(pluginName string, pluginCfg map[string]string) (messages []string, ok bool, err error) {
74+
match, matched := pluginMatch(pluginCfg, subCmdStr, cmdErrorMessage)
75+
if !matched {
76+
return nil, false, nil
7377
}
7478

7579
p, err := getPlugin(pluginName, pluginDirs, rootCmd)
7680
if err != nil {
77-
continue
81+
return nil, false, err
7882
}
7983

80-
hookReturn, err := p.RunHook(ctx, HookPluginData{
84+
resp, err := p.RunHook(ctx, HookPluginData{
8185
RootCmd: match,
8286
Flags: flags,
8387
CommandError: cmdErrorMessage,
8488
})
8589
if err != nil {
86-
// skip misbehaving plugins, but don't halt execution
87-
continue
90+
return nil, false, err
8891
}
8992

90-
var hookMessageData hooks.HookMessage
91-
err = json.Unmarshal(hookReturn, &hookMessageData)
92-
if err != nil {
93-
continue
93+
var message hooks.HookMessage
94+
if err := json.Unmarshal(resp, &message); err != nil {
95+
return nil, false, fmt.Errorf("failed to unmarshal hook response (%q): %w", string(resp), err)
9496
}
9597

9698
// currently the only hook type
97-
if hookMessageData.Type != hooks.NextSteps {
98-
continue
99+
if message.Type != hooks.NextSteps {
100+
return nil, false, errors.New("unexpected hook response type: " + strconv.Itoa(int(message.Type)))
99101
}
100102

101-
processedHook, err := hooks.ParseTemplate(hookMessageData.Template, subCmd)
103+
messages, err = hooks.ParseTemplate(message.Template, subCmd)
102104
if err != nil {
105+
return nil, false, err
106+
}
107+
108+
return messages, true, nil
109+
}
110+
111+
for pluginName, pluginCfg := range pluginsCfg {
112+
messages, ok, err := tryInvokeHook(pluginName, pluginCfg)
113+
if err != nil {
114+
// skip misbehaving plugins, but don't halt execution
115+
logrus.WithFields(logrus.Fields{
116+
"error": err,
117+
"plugin": pluginName,
118+
}).Debug("Plugin hook invocation failed")
119+
continue
120+
}
121+
if !ok {
103122
continue
104123
}
105124

106125
var appended bool
107-
nextSteps, appended = appendNextSteps(nextSteps, processedHook)
126+
nextSteps, appended = appendNextSteps(nextSteps, messages)
108127
if !appended {
109-
logrus.Debugf("Plugin %s responded with an empty hook message %q. Ignoring.", pluginName, string(hookReturn))
128+
logrus.WithFields(logrus.Fields{
129+
"plugin": pluginName,
130+
}).Debug("Plugin responded with an empty hook message; ignoring")
110131
}
111132
}
112133
return nextSteps

0 commit comments

Comments
 (0)