From 9d968ab357cffb716b469a97e8a27adf95dadb3a Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Thu, 9 Oct 2025 15:41:35 +0200 Subject: [PATCH] CLI: Refactor "dry-run" and "yes" command flags to use helper functions Signed-off-by: Michael Mayer --- internal/commands/cleanup.go | 7 +--- internal/commands/cluster_nodes_mod.go | 23 +++++++++-- internal/commands/cluster_nodes_remove.go | 50 +++++++++++++++++------ internal/commands/cluster_nodes_rotate.go | 44 ++++++++++++++++---- internal/commands/cluster_register.go | 2 +- internal/commands/flags.go | 13 ++++++ internal/commands/purge.go | 7 +--- internal/commands/vision_run.go | 14 ++++++- 8 files changed, 124 insertions(+), 36 deletions(-) diff --git a/internal/commands/cleanup.go b/internal/commands/cleanup.go index a2f1d6a61..03387e1fe 100644 --- a/internal/commands/cleanup.go +++ b/internal/commands/cleanup.go @@ -20,10 +20,7 @@ var CleanUpCommand = &cli.Command{ } var cleanUpFlags = []cli.Flag{ - &cli.BoolFlag{ - Name: "dry", - Usage: "performs a dry run that doesn't actually remove anything", - }, + DryRunFlag("performs a dry run that doesn't actually remove anything"), } // cleanUpAction removes orphaned index entries, sidecar and thumbnail files. @@ -49,7 +46,7 @@ func cleanUpAction(ctx *cli.Context) error { w := get.CleanUp() opt := photoprism.CleanUpOptions{ - Dry: ctx.Bool("dry"), + Dry: ctx.Bool("dry-run"), } // Start cleanup worker. diff --git a/internal/commands/cluster_nodes_mod.go b/internal/commands/cluster_nodes_mod.go index ff1c8c63c..6959bcac8 100644 --- a/internal/commands/cluster_nodes_mod.go +++ b/internal/commands/cluster_nodes_mod.go @@ -24,9 +24,12 @@ var ClusterNodesModCommand = &cli.Command{ Name: "mod", Usage: "Updates node properties", ArgsUsage: "", - Flags: []cli.Flag{nodesModRoleFlag, nodesModInternal, nodesModLabel, &cli.BoolFlag{Name: "yes", Aliases: []string{"y"}, Usage: "runs the command non-interactively"}}, - Hidden: true, // Required for cluster-management only. - Action: clusterNodesModAction, + Flags: []cli.Flag{nodesModRoleFlag, nodesModInternal, nodesModLabel, + DryRunFlag("preview updates without modifying the registry"), + YesFlag(), + }, + Hidden: true, // Required for cluster-management only. + Action: clusterNodesModAction, } func clusterNodesModAction(ctx *cli.Context) error { @@ -62,11 +65,15 @@ func clusterNodesModAction(ctx *cli.Context) error { return cli.Exit(fmt.Errorf("node not found"), 3) } + changes := make([]string, 0, 4) + if v := ctx.String("role"); v != "" { n.Role = clean.TypeLowerDash(v) + changes = append(changes, fmt.Sprintf("role=%s", clean.Log(n.Role))) } if v := ctx.String("advertise-url"); v != "" { n.AdvertiseUrl = v + changes = append(changes, fmt.Sprintf("advertise-url=%s", clean.Log(n.AdvertiseUrl))) } if labels := ctx.StringSlice("label"); len(labels) > 0 { if n.Labels == nil { @@ -77,6 +84,16 @@ func clusterNodesModAction(ctx *cli.Context) error { n.Labels[k] = v } } + changes = append(changes, fmt.Sprintf("labels+=%s", clean.Log(strings.Join(labels, ",")))) + } + + if ctx.Bool("dry-run") { + if len(changes) == 0 { + log.Infof("dry-run: no updates to apply for node %s", clean.LogQuote(n.Name)) + } else { + log.Infof("dry-run: would update node %s (%s)", clean.LogQuote(n.Name), strings.Join(changes, ", ")) + } + return nil } confirmed := RunNonInteractively(ctx.Bool("yes")) diff --git a/internal/commands/cluster_nodes_remove.go b/internal/commands/cluster_nodes_remove.go index 4ff582c70..e6279c118 100644 --- a/internal/commands/cluster_nodes_remove.go +++ b/internal/commands/cluster_nodes_remove.go @@ -20,9 +20,10 @@ var ClusterNodesRemoveCommand = &cli.Command{ Usage: "Deletes a node from the registry", ArgsUsage: "", Flags: []cli.Flag{ - &cli.BoolFlag{Name: "yes", Aliases: []string{"y"}, Usage: "runs the command non-interactively"}, + YesFlag(), &cli.BoolFlag{Name: "all-ids", Usage: "delete all records that share the same UUID (admin cleanup)"}, &cli.BoolFlag{Name: "drop-db", Aliases: []string{"d"}, Usage: "drop the node’s provisioned database and user after registry deletion"}, + DryRunFlag("preview deletion without modifying the registry or database"), }, Hidden: true, // Required for cluster-management only. Action: clusterNodesRemoveAction, @@ -66,33 +67,54 @@ func clusterNodesRemoveAction(ctx *cli.Context) error { uuid := node.UUID - confirmed := RunNonInteractively(ctx.Bool("yes")) - if !confirmed { - prompt := promptui.Prompt{Label: fmt.Sprintf("Delete node %s?", clean.Log(uuid)), IsConfirm: true} - if _, err := prompt.Run(); err != nil { - log.Infof("node %s was not deleted", clean.Log(uuid)) - return nil - } - } - dropDB := ctx.Bool("drop-db") dbName, dbUser := "", "" + if dropDB && node.Database != nil { dbName = node.Database.Name dbUser = node.Database.User } + if ctx.Bool("dry-run") { + log.Infof("dry-run: would delete node %s (uuid=%s, clientId=%s)", clean.LogQuote(node.Name), clean.Log(uuid), clean.Log(node.ClientID)) + + if ctx.Bool("all-ids") { + log.Infof("dry-run: would remove all registry entries that share uuid %s", clean.Log(uuid)) + } + + if dropDB { + if dbName == "" && dbUser == "" { + log.Infof("dry-run: --drop-db requested but no database credentials are recorded for node %s", clean.LogQuote(node.Name)) + } else { + log.Infof("dry-run: would drop database %s and user %s", clean.Log(dbName), clean.Log(dbUser)) + } + } + + return nil + } + + if !RunNonInteractively(ctx.Bool("yes")) { + prompt := promptui.Prompt{Label: fmt.Sprintf("Delete node %s?", clean.Log(uuid)), IsConfirm: true} + if _, err = prompt.Run(); err != nil { + log.Infof("node %s was not deleted", clean.Log(uuid)) + return nil + } + } + if ctx.Bool("all-ids") { - if err := r.DeleteAllByUUID(uuid); err != nil { + if err = r.DeleteAllByUUID(uuid); err != nil { return cli.Exit(err, 1) } - } else if err := r.Delete(uuid); err != nil { + } else if err = r.Delete(uuid); err != nil { return cli.Exit(err, 1) } + loggedDeletion := false + if dropDB { if dbName == "" && dbUser == "" { log.Infof("node %s has been deleted (no database credentials recorded)", clean.Log(uuid)) + loggedDeletion = true } else { dropCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -103,7 +125,9 @@ func clusterNodesRemoveAction(ctx *cli.Context) error { } } - log.Infof("node %s has been deleted", clean.Log(uuid)) + if !loggedDeletion { + log.Infof("node %s has been deleted", clean.Log(uuid)) + } return nil }) diff --git a/internal/commands/cluster_nodes_rotate.go b/internal/commands/cluster_nodes_rotate.go index b90b71d5e..89970559d 100644 --- a/internal/commands/cluster_nodes_rotate.go +++ b/internal/commands/cluster_nodes_rotate.go @@ -13,6 +13,7 @@ import ( "github.com/photoprism/photoprism/internal/service/cluster" reg "github.com/photoprism/photoprism/internal/service/cluster/registry" "github.com/photoprism/photoprism/pkg/clean" + "github.com/photoprism/photoprism/pkg/txt" "github.com/photoprism/photoprism/pkg/txt/report" ) @@ -28,8 +29,11 @@ var ClusterNodesRotateCommand = &cli.Command{ Name: "rotate", Usage: "Rotates a node's DB and/or secret via Portal (HTTP)", ArgsUsage: "", - Flags: append([]cli.Flag{rotateDatabaseFlag, rotateSecretFlag, &cli.BoolFlag{Name: "yes", Aliases: []string{"y"}, Usage: "runs the command non-interactively"}, rotatePortalURL, rotatePortalTok}, report.CliFlags...), - Action: clusterNodesRotateAction, + Flags: append([]cli.Flag{rotateDatabaseFlag, rotateSecretFlag, + DryRunFlag("preview rotation without contacting the Portal"), + YesFlag(), + rotatePortalURL, rotatePortalTok}, report.CliFlags...), + Action: clusterNodesRotateAction, } func clusterNodesRotateAction(ctx *cli.Context) error { @@ -64,9 +68,6 @@ func clusterNodesRotateAction(ctx *cli.Context) error { if portalURL == "" { portalURL = os.Getenv(config.EnvVar("portal-url")) } - if portalURL == "" { - return cli.Exit(fmt.Errorf("portal URL is required (use --portal-url or set portal-url)"), 2) - } token := ctx.String("join-token") if token == "" { token = conf.JoinToken() @@ -74,14 +75,41 @@ func clusterNodesRotateAction(ctx *cli.Context) error { if token == "" { token = os.Getenv(config.EnvVar("join-token")) } - if token == "" { - return cli.Exit(fmt.Errorf("portal token is required (use --join-token or set join-token)"), 2) - } // Default: rotate DB only if no flag given (safer default) rotateDatabase := ctx.Bool("database") || (!ctx.IsSet("database") && !ctx.IsSet("secret")) rotateSecret := ctx.Bool("secret") + if ctx.Bool("dry-run") { + target := clean.LogQuote(name) + if target == "" { + target = "(unnamed node)" + } + var what []string + if rotateDatabase { + what = append(what, "database credentials") + } + if rotateSecret { + what = append(what, "node secret") + } + if len(what) == 0 { + what = append(what, "no resources (no rotation flags set)") + } + if portalURL == "" { + log.Infof("dry-run: would rotate %s for %s (portal URL not set)", txt.JoinAnd(what), target) + } else { + log.Infof("dry-run: would rotate %s for %s via %s", txt.JoinAnd(what), target, clean.Log(portalURL)) + } + return nil + } + + if portalURL == "" { + return cli.Exit(fmt.Errorf("portal URL is required (use --portal-url or set portal-url)"), 2) + } + if token == "" { + return cli.Exit(fmt.Errorf("portal token is required (use --join-token or set join-token)"), 2) + } + confirmed := RunNonInteractively(ctx.Bool("yes")) if !confirmed { var what string diff --git a/internal/commands/cluster_register.go b/internal/commands/cluster_register.go index 2722b6091..ed15c89fd 100644 --- a/internal/commands/cluster_register.go +++ b/internal/commands/cluster_register.go @@ -37,7 +37,7 @@ var ( regPortalTok = &cli.StringFlag{Name: "join-token", Usage: "Portal access `TOKEN` (defaults to config)"} regWriteConf = &cli.BoolFlag{Name: "write-config", Usage: "persists returned secrets and DB settings to local config"} regForceFlag = &cli.BoolFlag{Name: "force", Aliases: []string{"f"}, Usage: "confirm actions that may overwrite/replace local data (e.g., --write-config)"} - regDryRun = &cli.BoolFlag{Name: "dry-run", Usage: "print derived values and payload without performing registration"} + regDryRun = DryRunFlag("print derived values and payload without performing registration") ) // ClusterRegisterCommand registers a node with the Portal via HTTP. diff --git a/internal/commands/flags.go b/internal/commands/flags.go index 77b066d5d..28e08bfe6 100644 --- a/internal/commands/flags.go +++ b/internal/commands/flags.go @@ -14,6 +14,19 @@ func JsonFlag() *cli.BoolFlag { return &cli.BoolFlag{Name: "json", Aliases: []string{"j"}, Usage: "print machine-readable JSON"} } +// DryRunFlag returns the shared CLI flag definition for dry runs across commands. +func DryRunFlag(usage string) *cli.BoolFlag { + if usage == "" { + usage = "performs a dry run without making any destructive changes" + } + return &cli.BoolFlag{Name: "dry-run", Aliases: []string{"dry"}, Usage: usage} +} + +// YesFlag returns the shared CLI flag definition for non-interactive runs across commands. +func YesFlag() *cli.BoolFlag { + return &cli.BoolFlag{Name: "yes", Aliases: []string{"y"}, Usage: "runs the command non-interactively"} +} + // PicturesCountFlag returns a shared flag definition limiting how many pictures a batch operation processes. // Usage: commands from the vision or import tooling that need to cap result size per invocation. func PicturesCountFlag() *cli.IntFlag { diff --git a/internal/commands/purge.go b/internal/commands/purge.go index 4662843e9..971fcc06b 100644 --- a/internal/commands/purge.go +++ b/internal/commands/purge.go @@ -28,10 +28,7 @@ var purgeFlags = []cli.Flag{ Name: "hard", Usage: "permanently removes data from the index", }, - &cli.BoolFlag{ - Name: "dry", - Usage: "performs a dry run that doesn't actually remove anything", - }, + DryRunFlag("performs a dry run that doesn't actually remove anything"), } // purgeAction removes missing files from search results @@ -67,7 +64,7 @@ func purgeAction(ctx *cli.Context) error { opt := photoprism.PurgeOptions{ Path: subPath, - Dry: ctx.Bool("dry"), + Dry: ctx.Bool("dry-run"), Hard: ctx.Bool("hard"), Force: true, } diff --git a/internal/commands/vision_run.go b/internal/commands/vision_run.go index 6aaf9a391..e69732e0b 100644 --- a/internal/commands/vision_run.go +++ b/internal/commands/vision_run.go @@ -30,6 +30,7 @@ var VisionRunCommand = &cli.Command{ Aliases: []string{"f"}, Usage: "replaces existing data if the model supports it and the source priority is equal or higher", }, + DryRunFlag("preview the run without executing any models"), }, Action: visionRunAction, } @@ -45,10 +46,21 @@ func visionRunAction(ctx *cli.Context) error { return cli.Exit(err.Error(), 1) } + models := vision.ParseModelTypes(ctx.String("models")) + + if ctx.Bool("dry-run") { + modelList := strings.Join(models, ",") + if modelList == "" { + modelList = "(none)" + } + log.Infof("dry-run: vision run would execute models [%s] with filter=%q (count=%d, source=%s, force=%v)", modelList, filter, ctx.Int("count"), string(source), ctx.Bool("force")) + return nil + } + return worker.Start( filter, ctx.Int("count"), - vision.ParseModelTypes(ctx.String("models")), + models, string(source), ctx.Bool("force"), vision.RunManual,