Skip to content

Commit 2b70dbf

Browse files
mmetcmmetc
authored
cscli hub/items: always show action plan; fix --interactive in pipes (#3451)
1 parent 45624c6 commit 2b70dbf

16 files changed

+196
-79
lines changed

‎cmd/crowdsec-cli/clihub/hub.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,15 @@ func (cli *cliHub) update(ctx context.Context, withContent bool) error {
118118

119119
indexProvider := require.HubDownloader(ctx, cli.cfg())
120120

121-
if err := hub.Update(ctx, indexProvider, withContent); err != nil {
121+
updated, err := hub.Update(ctx, indexProvider, withContent)
122+
if err != nil {
122123
return fmt.Errorf("failed to update hub: %w", err)
123124
}
124125

126+
if !updated && (log.StandardLogger().Level >= log.InfoLevel) {
127+
fmt.Println("Nothing to do, the hub index is up to date.")
128+
}
129+
125130
if err := hub.Load(); err != nil {
126131
return fmt.Errorf("failed to load hub: %w", err)
127132
}
@@ -187,9 +192,10 @@ func (cli *cliHub) upgrade(ctx context.Context, interactive bool, dryRun bool, f
187192
return err
188193
}
189194

190-
verbose := (cfg.Cscli.Output == "raw")
195+
showPlan := (log.StandardLogger().Level >= log.InfoLevel)
196+
verbosePlan := (cfg.Cscli.Output == "raw")
191197

192-
if err := plan.Execute(ctx, interactive, dryRun, verbose); err != nil {
198+
if err := plan.Execute(ctx, interactive, dryRun, showPlan, verbosePlan); err != nil {
193199
return err
194200
}
195201

‎cmd/crowdsec-cli/cliitem/cmdinstall.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,10 @@ func (cli cliItem) install(ctx context.Context, args []string, interactive bool,
7878
}
7979
}
8080

81-
verbose := (cfg.Cscli.Output == "raw")
81+
showPlan := (log.StandardLogger().Level >= log.InfoLevel)
82+
verbosePlan := (cfg.Cscli.Output == "raw")
8283

83-
if err := plan.Execute(ctx, interactive, dryRun, verbose); err != nil {
84+
if err := plan.Execute(ctx, interactive, dryRun, showPlan, verbosePlan); err != nil {
8485
if !ignoreError {
8586
return err
8687
}

‎cmd/crowdsec-cli/cliitem/cmdremove.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,10 @@ func (cli cliItem) remove(ctx context.Context, args []string, interactive bool,
9898
return err
9999
}
100100

101-
verbose := (cfg.Cscli.Output == "raw")
101+
showPlan := (log.StandardLogger().Level >= log.InfoLevel)
102+
verbosePlan := (cfg.Cscli.Output == "raw")
102103

103-
if err := plan.Execute(ctx, interactive, dryRun, verbose); err != nil {
104+
if err := plan.Execute(ctx, interactive, dryRun, showPlan, verbosePlan); err != nil {
104105
return err
105106
}
106107

‎cmd/crowdsec-cli/cliitem/cmdupgrade.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,10 @@ func (cli cliItem) upgrade(ctx context.Context, args []string, interactive bool,
6060
return err
6161
}
6262

63-
verbose := (cfg.Cscli.Output == "raw")
63+
showPlan := (log.StandardLogger().Level >= log.InfoLevel)
64+
verbosePlan := (cfg.Cscli.Output == "raw")
6465

65-
if err := plan.Execute(ctx, interactive, dryRun, verbose); err != nil {
66+
if err := plan.Execute(ctx, interactive, dryRun, showPlan, verbosePlan); err != nil {
6667
return err
6768
}
6869

‎cmd/crowdsec-cli/clisetup/setup.go

+10-9
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ func (cli *cliSetup) newDetectCmd() *cobra.Command {
9595

9696
func (cli *cliSetup) newInstallHubCmd() *cobra.Command {
9797
var (
98-
yes bool
99-
dryRun bool
98+
interactive bool
99+
dryRun bool
100100
)
101101

102102
cmd := &cobra.Command{
@@ -105,14 +105,14 @@ func (cli *cliSetup) newInstallHubCmd() *cobra.Command {
105105
Args: cobra.ExactArgs(1),
106106
DisableAutoGenTag: true,
107107
RunE: func(cmd *cobra.Command, args []string) error {
108-
return cli.install(cmd.Context(), yes, dryRun, args[0])
108+
return cli.install(cmd.Context(), interactive, dryRun, args[0])
109109
},
110110
}
111111

112112
flags := cmd.Flags()
113-
flags.BoolVarP(&yes, "yes", "y", false, "confirm execution without prompt")
113+
flags.BoolVarP(&interactive, "interactive", "i", false, "Ask for confirmation before proceeding")
114114
flags.BoolVar(&dryRun, "dry-run", false, "don't install anything; print out what would have been")
115-
cmd.MarkFlagsMutuallyExclusive("yes", "dry-run")
115+
cmd.MarkFlagsMutuallyExclusive("interactive", "dry-run")
116116

117117
return cmd
118118
}
@@ -281,7 +281,7 @@ func (cli *cliSetup) dataSources(fromFile string, toDir string) error {
281281
return nil
282282
}
283283

284-
func (cli *cliSetup) install(ctx context.Context, yes bool, dryRun bool, fromFile string) error {
284+
func (cli *cliSetup) install(ctx context.Context, interactive bool, dryRun bool, fromFile string) error {
285285
input, err := os.ReadFile(fromFile)
286286
if err != nil {
287287
return fmt.Errorf("while reading file %s: %w", fromFile, err)
@@ -294,11 +294,12 @@ func (cli *cliSetup) install(ctx context.Context, yes bool, dryRun bool, fromFil
294294
return err
295295
}
296296

297-
verbose := (cfg.Cscli.Output == "raw")
298-
299297
contentProvider := require.HubDownloader(ctx, cfg)
300298

301-
return setup.InstallHubItems(ctx, hub, contentProvider, input, yes, dryRun, verbose)
299+
showPlan := (log.StandardLogger().Level >= log.InfoLevel)
300+
verbosePlan := (cfg.Cscli.Output == "raw")
301+
302+
return setup.InstallHubItems(ctx, hub, contentProvider, input, interactive, dryRun, showPlan, verbosePlan)
302303
}
303304

304305
func (cli *cliSetup) validate(fromFile string) error {

‎pkg/cwhub/cwhub_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"testing"
1111

1212
log "github.com/sirupsen/logrus"
13+
"github.com/stretchr/testify/assert"
1314
"github.com/stretchr/testify/require"
1415

1516
"github.com/crowdsecurity/crowdsec/pkg/csconfig"
@@ -61,8 +62,9 @@ func testHubOld(t *testing.T, update bool) *Hub {
6162
URLTemplate: mockURLTemplate,
6263
}
6364

64-
err = hub.Update(ctx, indexProvider, false)
65+
updated, err := hub.Update(ctx, indexProvider, false)
6566
require.NoError(t, err)
67+
assert.True(t, updated)
6668
}
6769

6870
err = hub.Load()

‎pkg/cwhub/download.go

+1
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ func addURLParam(rawURL string, param string, value string) (string, error) {
6262
// FetchIndex downloads the index from the hub and writes it to the filesystem.
6363
// It uses a temporary file to avoid partial downloads, and won't overwrite the original
6464
// if it has not changed.
65+
// Return true if the file has been updated, false if already up to date.
6566
func (d *Downloader) FetchIndex(ctx context.Context, destPath string, withContent bool, logger *logrus.Logger) (bool, error) {
6667
url, err := d.urlTo(".index.json")
6768
if err != nil {

‎pkg/cwhub/hub.go

+3-14
Original file line numberDiff line numberDiff line change
@@ -153,24 +153,13 @@ var ErrUpdateAfterSync = errors.New("cannot update hub index after load/sync")
153153

154154
// Update downloads the latest version of the index and writes it to disk if it changed.
155155
// It cannot be called after Load() unless the index was completely empty.
156-
func (h *Hub) Update(ctx context.Context, indexProvider IndexProvider, withContent bool) error {
156+
func (h *Hub) Update(ctx context.Context, indexProvider IndexProvider, withContent bool) (bool, error) {
157157
if len(h.items) > 0 {
158158
// if this happens, it's a bug.
159-
return ErrUpdateAfterSync
159+
return false, ErrUpdateAfterSync
160160
}
161161

162-
downloaded, err := indexProvider.FetchIndex(ctx, h.local.HubIndexFile, withContent, h.logger)
163-
if err != nil {
164-
return err
165-
}
166-
167-
if !downloaded {
168-
// use logger and the message will be silenced in the cron job
169-
// (no mail if nothing happened)
170-
h.logger.Info("Nothing to do, the hub index is up to date.")
171-
}
172-
173-
return nil
162+
return indexProvider.FetchIndex(ctx, h.local.HubIndexFile, withContent, h.logger)
174163
}
175164

176165
// addItem adds an item to the hub. It silently replaces an existing item with the same type and name.

‎pkg/cwhub/hub_test.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,9 @@ func TestHubUpdate(t *testing.T) {
130130
URLTemplate: mockServer.URL + "/%s/%s",
131131
}
132132

133-
err = hub.Update(ctx, downloader, true)
133+
updated, err := hub.Update(ctx, downloader, true)
134134
require.NoError(t, err)
135+
assert.True(t, updated)
135136

136137
err = hub.Load()
137138
require.NoError(t, err)
@@ -151,8 +152,9 @@ func TestHubUpdateInvalidTemplate(t *testing.T) {
151152
URLTemplate: "x",
152153
}
153154

154-
err = hub.Update(ctx, downloader, true)
155+
updated, err := hub.Update(ctx, downloader, true)
155156
cstest.RequireErrorMessage(t, err, "failed to build hub index request: invalid URL template 'x'")
157+
assert.False(t, updated)
156158
}
157159

158160
func TestHubUpdateCannotWrite(t *testing.T) {
@@ -194,8 +196,9 @@ func TestHubUpdateCannotWrite(t *testing.T) {
194196

195197
hub.local.HubIndexFile = "/proc/foo/bar/baz/.index.json"
196198

197-
err = hub.Update(ctx, downloader, true)
199+
updated, err := hub.Update(ctx, downloader, true)
198200
cstest.RequireErrorContains(t, err, "failed to create temporary download file for /proc/foo/bar/baz/.index.json")
201+
assert.False(t, updated)
199202
}
200203

201204
func TestHubUpdateAfterLoad(t *testing.T) {
@@ -252,6 +255,7 @@ func TestHubUpdateAfterLoad(t *testing.T) {
252255
URLTemplate: mockServer.URL + "/%s/%s",
253256
}
254257

255-
err = hub.Update(ctx, downloader, true)
258+
updated, err := hub.Update(ctx, downloader, true)
256259
require.ErrorIs(t, err, ErrUpdateAfterSync)
260+
assert.False(t, updated)
257261
}

‎pkg/hubops/plan.go

+22-17
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"github.com/AlecAivazis/survey/v2"
1313
"github.com/fatih/color"
14-
isatty "github.com/mattn/go-isatty"
1514

1615
"github.com/crowdsecurity/go-cs-lib/slicetools"
1716

@@ -192,11 +191,6 @@ func (p *ActionPlan) compactDescription() string {
192191
}
193192

194193
func (p *ActionPlan) Confirm(verbose bool) (bool, error) {
195-
// user provided an --interactive flag, but we go with the defaults if it's not a tty
196-
if !isatty.IsTerminal(os.Stdout.Fd()) && !isatty.IsCygwinTerminal(os.Stdout.Fd()) {
197-
return true, nil
198-
}
199-
200194
fmt.Println("The following actions will be performed:\n" + p.Description(verbose))
201195

202196
var answer bool
@@ -206,9 +200,15 @@ func (p *ActionPlan) Confirm(verbose bool) (bool, error) {
206200
Default: true,
207201
}
208202

203+
tty, err := os.OpenFile("/dev/tty", os.O_RDWR, 0)
204+
if err != nil {
205+
return prompt.Default, nil
206+
}
207+
defer tty.Close()
208+
209209
// in case of EOF, it's likely stdin has been closed in a script or package manager,
210210
// we can't do anything but go with the default
211-
if err := survey.AskOne(prompt, &answer); err != nil {
211+
if err := survey.AskOne(prompt, &answer, survey.WithStdio(tty, tty, tty)); err != nil {
212212
if errors.Is(err, io.EOF) {
213213
return prompt.Default, nil
214214
}
@@ -221,22 +221,18 @@ func (p *ActionPlan) Confirm(verbose bool) (bool, error) {
221221
return answer, nil
222222
}
223223

224-
func (p *ActionPlan) Execute(ctx context.Context, interactive bool, dryRun bool, verbose bool) error {
224+
func (p *ActionPlan) Execute(ctx context.Context, interactive bool, dryRun bool, alwaysShowPlan bool, verbosePlan bool) error {
225+
// interactive: show action plan, ask for confirm
226+
// dry-run: show action plan, no prompt, no action
227+
// alwaysShowPlan: print plan even if interactive and dry-run are false
228+
// verbosePlan: plan summary is displaying each step in order
225229
if len(p.commands) == 0 {
226-
// XXX: show skipped commands, warnings?
227230
fmt.Println("Nothing to do.")
228231
return nil
229232
}
230233

231-
if dryRun {
232-
fmt.Println("Action plan:\n" + p.Description(verbose))
233-
fmt.Println("Dry run, no action taken.")
234-
235-
return nil
236-
}
237-
238234
if interactive {
239-
answer, err := p.Confirm(verbose)
235+
answer, err := p.Confirm(verbosePlan)
240236
if err != nil {
241237
return err
242238
}
@@ -245,6 +241,15 @@ func (p *ActionPlan) Execute(ctx context.Context, interactive bool, dryRun bool,
245241
fmt.Println("Operation canceled.")
246242
return nil
247243
}
244+
} else {
245+
if dryRun || alwaysShowPlan {
246+
fmt.Println("Action plan:\n" + p.Description(verbosePlan))
247+
}
248+
249+
if dryRun {
250+
fmt.Println("Dry run, no action taken.")
251+
return nil
252+
}
248253
}
249254

250255
for _, c := range p.commands {

‎pkg/setup/install.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func decodeSetup(input []byte, fancyErrors bool) (Setup, error) {
4848
}
4949

5050
// InstallHubItems installs the objects recommended in a setup file.
51-
func InstallHubItems(ctx context.Context, hub *cwhub.Hub, contentProvider cwhub.ContentProvider, input []byte, yes, dryRun, verbose bool) error {
51+
func InstallHubItems(ctx context.Context, hub *cwhub.Hub, contentProvider cwhub.ContentProvider, input []byte, interactive, dryRun, showPlan, verbosePlan bool) error {
5252
setupEnvelope, err := decodeSetup(input, false)
5353
if err != nil {
5454
return err
@@ -134,7 +134,7 @@ func InstallHubItems(ctx context.Context, hub *cwhub.Hub, contentProvider cwhub.
134134
}
135135
}
136136

137-
return plan.Execute(ctx, yes, dryRun, verbose)
137+
return plan.Execute(ctx, interactive, dryRun, showPlan, verbosePlan)
138138
}
139139

140140
// marshalAcquisDocuments creates the monolithic file, or itemized files (if a directory is provided) with the acquisition documents.

‎test/bats/20_hub.bats

+20-2
Original file line numberDiff line numberDiff line change
@@ -109,17 +109,33 @@ teardown() {
109109
rune -0 cscli hub update
110110
assert_output "Downloading $INDEX_PATH"
111111
rune -0 cscli hub update
112+
assert_output "Nothing to do, the hub index is up to date."
113+
114+
# hub update must honor the --error flag to be silent in noop cron jobs
115+
rune -0 cscli hub update --error
112116
refute_output
113-
assert_stderr 'level=info msg="Nothing to do, the hub index is up to date."'
117+
refute_stderr
114118
}
115119

116120
@test "cscli hub upgrade (up to date)" {
117121
rune -0 cscli hub upgrade
118-
refute_output
122+
assert_output - <<-EOT
123+
Action plan:
124+
🔄 check & update data files
125+
EOT
119126

120127
rune -0 cscli parsers install crowdsecurity/syslog-logs
121128
rune -0 cscli hub upgrade --force
129+
assert_output - <<-EOT
130+
Action plan:
131+
🔄 check & update data files
132+
EOT
133+
134+
# hub upgrade must honor the --error flag to be silent in noop cron jobs
135+
rune -0 cscli hub upgrade --error
122136
refute_output
137+
refute_stderr
138+
123139
skip "todo: data files are re-downloaded with --force"
124140
}
125141

@@ -129,6 +145,8 @@ teardown() {
129145
rune -0 cscli hub upgrade
130146
assert_output - <<-EOT
131147
collections:foo.yaml - not downloading local item
148+
Action plan:
149+
🔄 check & update data files
132150
EOT
133151
}
134152

0 commit comments

Comments
 (0)