From bc7c91d78e84c5597e8e02d6ff8604ee835b8da0 Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Mon, 12 May 2025 15:57:58 +0200 Subject: [PATCH 01/14] fix: better management of relative paths - Improve management of relative paths, calculating them once. - Set outdir globally and reuse. - Reduce code duplication (still lots to improve) - Move flags processing to helper func when present - Improve UX on some error messages, help messages and debug messages - Normalize words in help messages Fixes #508 --- cmd/apply.go | 115 ++++++++++-------- cmd/create/config.go | 27 ++-- cmd/create/pki.go | 2 +- cmd/delete/cluster.go | 113 +++++++++-------- cmd/diff.go | 75 +++++------- cmd/download/dependencies.go | 33 +++-- cmd/dump/template.go | 104 ++++++---------- cmd/get.go | 2 +- cmd/get/kubeconfig.go | 61 ++++------ cmd/get/upgrade-paths.go | 47 +++---- cmd/renew/certificates.go | 44 +++---- cmd/root.go | 106 +++++++++------- cmd/validate/config.go | 23 ++-- cmd/validate/dependencies.go | 26 ++-- .../kfd/v1alpha2/common/create/plugins.go | 2 +- .../kfd/v1alpha2/common/create/preupgrade.go | 2 +- internal/cluster/phase.go | 2 +- internal/distribution/iac.go | 2 +- internal/git/flag.go | 43 +++++++ internal/git/protocols.go | 25 +++- pkg/dependencies/download.go | 2 +- pkg/distribution/download.go | 1 + 22 files changed, 438 insertions(+), 419 deletions(-) create mode 100644 internal/git/flag.go diff --git a/cmd/apply.go b/cmd/apply.go index 7583d4ceb..f00185255 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -79,6 +79,18 @@ func NewApplyCmd() *cobra.Command { applyCmd := &cobra.Command{ Use: "apply", Short: "Apply the configuration to create, update, or upgrade a battle-tested SIGHUP Distribution cluster", + Example: ` Apply the configuration file and all the phases using the default configuration file: + furyctl apply + + Apply a custom configuration file: + furyctl apply --config mycluster.yaml + + Apply a single phase, for example the distribution phase: + furyctl apply --phase distribution + + Apply all the phases, and repeat the distribution phase afterwards: + furyctl apply --post-apply-phases distribution +`, PreRun: func(cmd *cobra.Command, _ []string) { cmdEvent = analytics.NewCommandEvent(cobrax.GetFullname(cmd)) @@ -93,48 +105,18 @@ func NewApplyCmd() *cobra.Command { tracker.Flush() // Get flags. - flags, err := getCreateClusterCmdFlags() - if err != nil { - return err - } - - outDir := flags.Outdir - - // Get home dir. - logrus.Debug("Getting Home Directory Path...") - - homeDir, err := os.UserHomeDir() + flags, err := getApplyCmdFlags() if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) - return fmt.Errorf("error while getting user home directory: %w", err) - } - - if outDir == "" { - outDir = homeDir - } - - if flags.BinPath == "" { - flags.BinPath = filepath.Join(outDir, ".furyctl", "bin") + return err } if flags.DryRun { logrus.Info("Dry run mode enabled, no changes will be applied") } - absDistroPatchesLocation := flags.DistroPatchesLocation - - if absDistroPatchesLocation != "" { - absDistroPatchesLocation, err = filepath.Abs(flags.DistroPatchesLocation) - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting absolute path of distro patches location: %w", err) - } - } - var distrodl *dist.Downloader // Init first half of collaborators. @@ -143,9 +125,9 @@ func NewApplyCmd() *cobra.Command { depsvl := dependencies.NewValidator(executor, flags.BinPath, flags.FuryctlPath, flags.VpnAutoConnect) if flags.DistroLocation == "" { - distrodl = dist.NewCachingDownloader(client, outDir, flags.GitProtocol, absDistroPatchesLocation) + distrodl = dist.NewCachingDownloader(client, flags.Outdir, flags.GitProtocol, flags.DistroPatchesLocation) } else { - distrodl = dist.NewDownloader(client, flags.GitProtocol, absDistroPatchesLocation) + distrodl = dist.NewDownloader(client, flags.GitProtocol, flags.DistroPatchesLocation) } // Init packages. @@ -212,10 +194,10 @@ func NewApplyCmd() *cobra.Command { } defer lockFileHandler.Remove() //nolint:errcheck // ignore error - basePath := filepath.Join(outDir, ".furyctl", res.MinimalConf.Metadata.Name) + basePath := filepath.Join(flags.Outdir, ".furyctl", res.MinimalConf.Metadata.Name) // Init second half of collaborators. - depsdl := dependencies.NewCachingDownloader(client, outDir, basePath, flags.BinPath, flags.GitProtocol) + depsdl := dependencies.NewCachingDownloader(client, flags.Outdir, basePath, flags.BinPath, flags.GitProtocol) // Validate the furyctl.yaml file. logrus.Info("Validating configuration file...") @@ -235,6 +217,8 @@ func NewApplyCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, errs) } + } else { + logrus.Info("Skipping dependencies download") } // Validate the dependencies, unless explicitly told to skip it. @@ -246,19 +230,13 @@ func NewApplyCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } - } - - absFuryctlPath, err := filepath.Abs(flags.FuryctlPath) - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while initializing cluster creation: %w", err) + } else { + logrus.Info("Skipping dependencies validation") } // Define cluster creation paths. paths := cluster.CreatorPaths{ - ConfigPath: absFuryctlPath, + ConfigPath: flags.FuryctlPath, WorkDir: basePath, DistroPath: res.RepoPath, BinPath: flags.BinPath, @@ -308,7 +286,7 @@ func NewApplyCmd() *cobra.Command { }, } - setupCreateClusterCmdFlags(applyCmd) + setupApplyCmdFlags(applyCmd) return applyCmd } @@ -322,9 +300,38 @@ func getSkipsClusterCmdFlags() ClusterSkipsCmdFlags { } } -func getCreateClusterCmdFlags() (ClusterCmdFlags, error) { +func getApplyCmdFlags() (ClusterCmdFlags, error) { + var err error + skips := getSkipsClusterCmdFlags() + // The binPath path must be calculated here because when we launch the tools + // we sometimes change the working directory where the binary is launched + // breaking the relative path. + binPath := viper.GetString("bin-path") + if binPath == "" { + // The outdir flag is already calculated in the root command, so we can use it here. + binPath = filepath.Join(viper.GetString("outdir"), ".furyctl", "bin") + } else { + binPath, err = filepath.Abs(binPath) + if err != nil { + return ClusterCmdFlags{}, fmt.Errorf("error while getting absolute path for bin folder: %w", err) + } + } + + distroPatchesLocation := viper.GetString("distro-patches") + if distroPatchesLocation != "" { + distroPatchesLocation, err = filepath.Abs(viper.GetString("distro-patches")) + if err != nil { + return ClusterCmdFlags{}, fmt.Errorf("error while getting absolute path of distro patches location: %w", err) + } + } + + furyctlPath, err := filepath.Abs(viper.GetString("config")) + if err != nil { + return ClusterCmdFlags{}, fmt.Errorf("error while calculating furyctl configuration file absolute path: %w", err) + } + phase := viper.GetString("phase") if err := cluster.CheckPhase(phase); err != nil { @@ -382,11 +389,11 @@ func getCreateClusterCmdFlags() (ClusterCmdFlags, error) { return ClusterCmdFlags{ Debug: viper.GetBool("debug"), - FuryctlPath: viper.GetString("config"), + FuryctlPath: furyctlPath, DistroLocation: viper.GetString("distro-location"), Phase: phase, StartFrom: startFrom, - BinPath: viper.GetString("bin-path"), + BinPath: binPath, VpnAutoConnect: vpnAutoConnect, DryRun: viper.GetBool("dry-run"), NoTTY: viper.GetBool("no-tty"), @@ -400,7 +407,7 @@ func getCreateClusterCmdFlags() (ClusterCmdFlags, error) { Upgrade: upgrade, UpgradePathLocation: viper.GetString("upgrade-path-location"), UpgradeNode: upgradeNode, - DistroPatchesLocation: viper.GetString("distro-patches"), + DistroPatchesLocation: distroPatchesLocation, ClusterSkipsCmdFlags: skips, PostApplyPhases: postApplyPhases, }, nil @@ -416,7 +423,7 @@ func validatePostApplyPhasesFlag(phases []string) error { return nil } -func setupCreateClusterCmdFlags(cmd *cobra.Command) { +func setupApplyCmdFlags(cmd *cobra.Command) { cmd.Flags().StringP( "config", "c", @@ -458,7 +465,7 @@ func setupCreateClusterCmdFlags(cmd *cobra.Command) { "Location where to download schemas, defaults, and the distribution manifests from. "+ "It can either be a local path (eg: /path/to/distribution) or "+ "a remote URL (eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) cmd.Flags().String( @@ -469,14 +476,14 @@ func setupCreateClusterCmdFlags(cmd *cobra.Command) { "a remote URL (eg: git::git@github.com:your-org/distro-patches?depth=1&ref=BRANCH_NAME). "+ "Any format supported by hashicorp/go-getter can be used."+ " Patches within this location must be in a folder named after the distribution version (eg: v1.29.0) and "+ - "must have the same structure as the distribution's repository.", + "must have the same structure as the distribution's repository", ) cmd.Flags().StringP( "bin-path", "b", "", - "Path to the folder where all the dependencies' binaries are installed", + "Path to the folder where all the dependencies' binaries are downloaded", ) cmd.Flags().Bool( diff --git a/cmd/create/config.go b/cmd/create/config.go index 89810d593..5cbdddd77 100644 --- a/cmd/create/config.go +++ b/cmd/create/config.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -85,18 +86,6 @@ func NewConfigCmd() *cobra.Command { return fmt.Errorf("%w: %w", ErrParsingFlag, err) } - homeDir, err := os.UserHomeDir() - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting user home directory: %w", err) - } - - if outDir == "" { - outDir = homeDir - } - minimalConf := distroconf.Furyctl{ APIVersion: apiVersion, Kind: kind, @@ -218,17 +207,21 @@ func NewConfigCmd() *cobra.Command { "distro-location", "", "", - "Base URL used to download schemas, defaults and the distribution manifest. "+ + "Location where to download schemas, defaults, and the distribution manifests from. "+ "It can either be a local path(eg: /path/to/distribution) or "+ "a remote URL(eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME)."+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) configCmd.Flags().String( "distro-patches", "", - "Location where to download distribution's user-made patches from. "+ - "Any format supported by hashicorp/go-getter can be used.", + "Location where the distribution's user-made patches can be downloaded from. "+ + "This can be either a local path (eg: /path/to/distro-patches) or "+ + "a remote URL (eg: git::git@github.com:your-org/distro-patches?depth=1&ref=BRANCH_NAME). "+ + "Any format supported by hashicorp/go-getter can be used."+ + " Patches within this location must be in a folder named after the distribution version (eg: v1.29.0) and "+ + "must have the same structure as the distribution's repository", ) configCmd.Flags().StringP( @@ -242,7 +235,7 @@ func NewConfigCmd() *cobra.Command { "kind", "k", "", - "Type of cluster to create (eg: EKSCluster, KFDDistribution, OnPremises)", + "Type of cluster to create. Options are "+strings.Join(distribution.ConfigKinds(), ", "), ) if err := configCmd.RegisterFlagCompletionFunc("kind", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { diff --git a/cmd/create/pki.go b/cmd/create/pki.go index 6ce5eca21..cddf065dc 100644 --- a/cmd/create/pki.go +++ b/cmd/create/pki.go @@ -130,7 +130,7 @@ You can limit the creation of the PKI to just etcd or just Kubernetes using the "path", "p", "pki", - "path where to save the created PKI files. One subfolder will be created for the control plane files and another one for the etcd files.", + "path where to save the created PKI files. One subfolder will be created for the control plane files and another one for the etcd files", ) pkiCmd.Flags().BoolP( diff --git a/cmd/delete/cluster.go b/cmd/delete/cluster.go index f916b735c..af382fbfd 100644 --- a/cmd/delete/cluster.go +++ b/cmd/delete/cluster.go @@ -11,6 +11,7 @@ import ( "os" "os/signal" "path/filepath" + "strings" "syscall" "github.com/sirupsen/logrus" @@ -77,48 +78,21 @@ func NewClusterCmd() *cobra.Command { // Get flags. flags, err := getDeleteClusterCmdFlags() - if err != nil { - return err - } - - // Init paths. - - outDir := flags.Outdir - - logrus.Debug("Getting Home Directory Path...") - - homeDir, err := os.UserHomeDir() if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) - return fmt.Errorf("error while getting user home directory: %w", err) + return err } - if outDir == "" { - outDir = homeDir - } + // Init paths. - if flags.BinPath == "" { - flags.BinPath = filepath.Join(outDir, ".furyctl", "bin") - } + outDir := flags.Outdir if flags.DryRun { logrus.Info("Dry run mode enabled, no changes will be applied") } - absDistroPatchesLocation := flags.DistroPatchesLocation - - if absDistroPatchesLocation != "" { - absDistroPatchesLocation, err = filepath.Abs(flags.DistroPatchesLocation) - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting absolute path of distro patches location: %w", err) - } - } - var distrodl *dist.Downloader // Init first half of collaborators. @@ -127,9 +101,9 @@ func NewClusterCmd() *cobra.Command { depsvl := dependencies.NewValidator(executor, flags.BinPath, flags.FuryctlPath, flags.VpnAutoConnect) if flags.DistroLocation == "" { - distrodl = dist.NewCachingDownloader(client, outDir, flags.GitProtocol, absDistroPatchesLocation) + distrodl = dist.NewCachingDownloader(client, outDir, flags.GitProtocol, flags.DistroPatchesLocation) } else { - distrodl = dist.NewDownloader(client, flags.GitProtocol, absDistroPatchesLocation) + distrodl = dist.NewDownloader(client, flags.GitProtocol, flags.DistroPatchesLocation) } execx.NoTTY = flags.NoTTY @@ -220,6 +194,8 @@ func NewClusterCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, errs) } + } else { + logrus.Info("Skipping dependencies download") } // Validate the dependencies, unless explicitly told to skip it. @@ -231,19 +207,13 @@ func NewClusterCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } - } - - absFuryctlPath, err := filepath.Abs(flags.FuryctlPath) - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while initializing cluster creation: %w", err) + } else { + logrus.Info("Skipping dependencies validation") } // Define cluster deletion paths. paths := cluster.DeleterPaths{ - ConfigPath: absFuryctlPath, + ConfigPath: flags.FuryctlPath, WorkDir: basePath, BinPath: flags.BinPath, DistroPath: res.RepoPath, @@ -277,7 +247,7 @@ func NewClusterCmd() *cobra.Command { return fmt.Errorf("error while printing to stdout: %w", err) } - _, err = fmt.Println("Are you sure you want to continue? Only 'yes' will be accepted to confirm.") + _, err = fmt.Println("\nAre you sure you want to continue? Only 'yes' will be accepted to confirm.") if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) @@ -326,37 +296,36 @@ func NewClusterCmd() *cobra.Command { "Location where to download schemas, defaults and the distribution manifests from. "+ "It can either be a local path (eg: /path/to/distribution) or "+ "a remote URL (eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) clusterCmd.Flags().String( "distro-patches", "", - "Location where to download distribution's user-made patches from. "+ - "Any format supported by hashicorp/go-getter can be used.", + "Location where the distribution's user-made patches can be downloaded from. "+ + "This can be either a local path (eg: /path/to/distro-patches) or "+ + "a remote URL (eg: git::git@github.com:your-org/distro-patches?depth=1&ref=BRANCH_NAME). "+ + "Any format supported by hashicorp/go-getter can be used."+ + " Patches within this location must be in a folder named after the distribution version (eg: v1.29.0) and "+ + "must have the same structure as the distribution's repository", ) clusterCmd.Flags().StringP( "bin-path", "b", "", - "Path to the folder where all the dependencies' binaries are installed", + "Path to the folder where all the dependencies' binaries are downloaded", ) clusterCmd.Flags().StringP( "phase", "p", "", - "Limit execution to the specified phase. Options are: infrastructure, kubernetes, distribution", + "Limit execution to a specific phase. Options are: "+strings.Join(cluster.MainPhases(), ", "), ) if err := clusterCmd.RegisterFlagCompletionFunc("phase", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return []string{ - cluster.OperationPhaseInfrastructure, - cluster.OperationPhaseKubernetes, - cluster.OperationPhaseDistribution, - }, - cobra.ShellCompDirectiveDefault + return cluster.MainPhases(), cobra.ShellCompDirectiveDefault }); err != nil { logrus.Fatalf("error while registering flag completion: %v", err) } @@ -402,10 +371,38 @@ func NewClusterCmd() *cobra.Command { } func getDeleteClusterCmdFlags() (ClusterCmdFlags, error) { - phase := viper.GetString("phase") + var err error + + // The binPath path must be calculated here because when we launch the tools + // we sometimes change the working directory where the binary is launched + // breaking the relative path. + binPath := viper.GetString("bin-path") + if binPath == "" { + // The outdir flag is already calculated in the root command, so we can use it here. + binPath = filepath.Join(viper.GetString("outdir"), ".furyctl", "bin") + } else { + binPath, err = filepath.Abs(binPath) + if err != nil { + return ClusterCmdFlags{}, fmt.Errorf("error while getting absolute path for bin folder: %w", err) + } + } - err := cluster.CheckPhase(phase) + distroPatchesLocation := viper.GetString("distro-patches") + if distroPatchesLocation != "" { + distroPatchesLocation, err = filepath.Abs(viper.GetString("distro-patches")) + if err != nil { + return ClusterCmdFlags{}, fmt.Errorf("error while getting absolute path of distro patches location: %w", err) + } + } + + furyctlPath, err := filepath.Abs(viper.GetString("config")) if err != nil { + return ClusterCmdFlags{}, fmt.Errorf("error while initializing cluster creation: %w", err) + } + + phase := viper.GetString("phase") + err = cluster.CheckPhase(phase) + if err != nil { //nolint: wsl // conflicts with gofumpt return ClusterCmdFlags{}, fmt.Errorf("%w: %s: %s", ErrParsingFlag, "phase", err.Error()) } @@ -429,10 +426,10 @@ func getDeleteClusterCmdFlags() (ClusterCmdFlags, error) { return ClusterCmdFlags{ Debug: viper.GetBool("debug"), - FuryctlPath: viper.GetString("config"), + FuryctlPath: furyctlPath, DistroLocation: viper.GetString("distro-location"), Phase: phase, - BinPath: viper.GetString("bin-path"), + BinPath: binPath, SkipVpn: skipVpn, VpnAutoConnect: vpnAutoConnect, DryRun: viper.GetBool("dry-run"), @@ -442,6 +439,6 @@ func getDeleteClusterCmdFlags() (ClusterCmdFlags, error) { Outdir: viper.GetString("outdir"), SkipDepsDownload: viper.GetBool("skip-deps-download"), SkipDepsValidation: viper.GetBool("skip-deps-validation"), - DistroPatchesLocation: viper.GetString("distro-patches"), + DistroPatchesLocation: distroPatchesLocation, }, nil } diff --git a/cmd/diff.go b/cmd/diff.go index a5317b998..4d690525b 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -6,7 +6,6 @@ package cmd import ( "fmt" - "os" "path/filepath" "strings" @@ -62,61 +61,30 @@ func NewDiffCmd() *cobra.Command { defer tracker.Flush() flags, err := getDiffCommandFlags() - if err != nil { - return err - } - - execx.Debug = flags.Debug - - logrus.Debug("Getting Home Directory Path...") - outDir := flags.Outdir - - homeDir, err := os.UserHomeDir() if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) - return fmt.Errorf("error while getting user home directory: %w", err) - } - - if outDir == "" { - outDir = homeDir - } - - if flags.BinPath == "" { - flags.BinPath = filepath.Join(outDir, ".furyctl", "bin") + return err } - absDistroPatchesLocation := flags.DistroPatchesLocation - - if absDistroPatchesLocation != "" { - absDistroPatchesLocation, err = filepath.Abs(flags.DistroPatchesLocation) - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting absolute path of distro patches location: %w", err) - } - } + execx.Debug = flags.Debug client := netx.NewGoGetterClient() - distrodl := dist.NewDownloader(client, flags.GitProtocol, absDistroPatchesLocation) + distrodl := dist.NewDownloader(client, flags.GitProtocol, flags.DistroPatchesLocation) if flags.DistroLocation == "" { - distrodl = dist.NewCachingDownloader(client, outDir, flags.GitProtocol, absDistroPatchesLocation) + distrodl = dist.NewCachingDownloader(client, flags.Outdir, flags.GitProtocol, flags.DistroPatchesLocation) } logrus.Info("Downloading distribution...") res, err := distrodl.Download(flags.DistroLocation, flags.FuryctlPath) if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - return fmt.Errorf("error while downloading distribution: %w", err) } - basePath := filepath.Join(outDir, ".furyctl", res.MinimalConf.Metadata.Name) + basePath := filepath.Join(flags.Outdir, ".furyctl", res.MinimalConf.Metadata.Name) stateStore := state.NewStore( res.RepoPath, @@ -203,21 +171,25 @@ func NewDiffCmd() *cobra.Command { "Location where to download schemas, defaults and the distribution manifests from. "+ "It can either be a local path (eg: /path/to/distribution) or "+ "a remote URL (eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) diffCmd.Flags().String( "distro-patches", "", - "Location where to download distribution's user-made patches from. "+ - "Any format supported by hashicorp/go-getter can be used.", + "Location where the distribution's user-made patches can be downloaded from. "+ + "This can be either a local path (eg: /path/to/distro-patches) or "+ + "a remote URL (eg: git::git@github.com:your-org/distro-patches?depth=1&ref=BRANCH_NAME). "+ + "Any format supported by hashicorp/go-getter can be used."+ + " Patches within this location must be in a folder named after the distribution version (eg: v1.29.0) and "+ + "must have the same structure as the distribution's repository", ) diffCmd.Flags().StringP( "bin-path", "b", "", - "Path to the folder where all the dependencies' binaries are installed", + "Path to the folder where all the dependencies' binaries are downloaded", ) diffCmd.Flags().StringP( @@ -311,6 +283,23 @@ func getDiffs(diffChecker diffs.Checker, phasePath string) (diff.Changelog, erro } func getDiffCommandFlags() (DiffCommandFlags, error) { + var err error + + binPath := viper.GetString("bin-path") + if binPath == "" { + binPath = filepath.Join(viper.GetString("outdir"), ".furyctl", "bin") + } else { + binPath, err = filepath.Abs(binPath) + if err != nil { + return DiffCommandFlags{}, fmt.Errorf("error while getting absolute path for bin folder: %w", err) + } + } + + distroPatchesLocation, err := filepath.Abs(viper.GetString("distro-patches")) + if err != nil { + return DiffCommandFlags{}, fmt.Errorf("error while getting absolute path of distro patches location: %w", err) + } + phase := viper.GetString("phase") if err := cluster.CheckPhase(phase); err != nil { return DiffCommandFlags{}, fmt.Errorf("%w: %s: %s", ErrParsingFlag, "phase", err.Error()) @@ -330,9 +319,9 @@ func getDiffCommandFlags() (DiffCommandFlags, error) { Phase: phase, NoTTY: viper.GetBool("no-tty"), GitProtocol: typedGitProtocol, - BinPath: viper.GetString("bin-path"), + BinPath: binPath, Outdir: viper.GetString("outdir"), UpgradePathLocation: viper.GetString("upgrade-path-location"), - DistroPatchesLocation: viper.GetString("distro-patches"), + DistroPatchesLocation: distroPatchesLocation, }, nil } diff --git a/cmd/download/dependencies.go b/cmd/download/dependencies.go index c276e52c8..81af39ff2 100644 --- a/cmd/download/dependencies.go +++ b/cmd/download/dependencies.go @@ -7,7 +7,6 @@ package download import ( "errors" "fmt" - "os" "path/filepath" "github.com/sirupsen/logrus" @@ -56,27 +55,23 @@ func NewDependenciesCmd() *cobra.Command { binPath := viper.GetString("bin-path") typedGitProtocol, err := git.NewProtocol(gitProtocol) - if err != nil { - return fmt.Errorf("%w: %w", ErrParsingFlag, err) - } - - // Init paths. - logrus.Debug("Getting Home Directory Path...") - - homeDir, err := os.UserHomeDir() if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) - return fmt.Errorf("error while getting user home directory: %w", err) - } - - if outDir == "" { - outDir = homeDir + return fmt.Errorf("%w: %w", ErrParsingFlag, err) } if binPath == "" { binPath = filepath.Join(outDir, ".furyctl", "bin") + } else { + binPath, err = filepath.Abs(binPath) + if err != nil { + cmdEvent.AddErrorMessage(err) + tracker.Track(cmdEvent) + + return fmt.Errorf("error while getting absolute path for bin path: %w", err) + } } absDistroPatchesLocation := distroPatchesLocation @@ -179,14 +174,18 @@ func NewDependenciesCmd() *cobra.Command { "Location where to download schemas, defaults and the distribution manifests from. "+ "It can either be a local path (eg: /path/to/distribution) or "+ "a remote URL (eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) dependenciesCmd.Flags().String( "distro-patches", "", - "Location where to download distribution's user-made patches from. "+ - "Any format supported by hashicorp/go-getter can be used.", + "Location where the distribution's user-made patches can be downloaded from. "+ + "This can be either a local path (eg: /path/to/distro-patches) or "+ + "a remote URL (eg: git::git@github.com:your-org/distro-patches?depth=1&ref=BRANCH_NAME). "+ + "Any format supported by hashicorp/go-getter can be used."+ + " Patches within this location must be in a folder named after the distribution version (eg: v1.29.0) and "+ + "must have the same structure as the distribution's repository", ) return dependenciesCmd diff --git a/cmd/dump/template.go b/cmd/dump/template.go index b0b85b058..7502091ba 100644 --- a/cmd/dump/template.go +++ b/cmd/dump/template.go @@ -7,7 +7,6 @@ package dump import ( "errors" "fmt" - "os" "path/filepath" "github.com/sirupsen/logrus" @@ -68,56 +67,23 @@ The command will dump into a 'distribution' folder in the working directory all tracker.Flush() flags, err := getDumpTemplateCmdFlags() - if err != nil { - return err - } - - absFuryctlPath, err := filepath.Abs(flags.FuryctlPath) if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) - return fmt.Errorf("error: %w", err) - } - - outDir := flags.Outdir - - if outDir == "" { - // Get home dir and use that as outdir if it's not set. - logrus.Debug("Getting Home Directory Path...") - - homeDir, err := os.UserHomeDir() - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting user home directory: %w", err) - } - outDir = homeDir - } - - absDistroPatchesLocation := flags.DistroPatchesLocation - - if absDistroPatchesLocation != "" { - absDistroPatchesLocation, err = filepath.Abs(flags.DistroPatchesLocation) - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting absolute path of distro patches location: %w", err) - } + return err } var distrodl *dist.Downloader client := netx.NewGoGetterClient() executor := execx.NewStdExecutor() - depsvl := dependencies.NewValidator(executor, "", absFuryctlPath, false) + depsvl := dependencies.NewValidator(executor, "", flags.FuryctlPath, false) if flags.DistroLocation == "" { - distrodl = dist.NewCachingDownloader(client, outDir, flags.GitProtocol, absDistroPatchesLocation) + distrodl = dist.NewCachingDownloader(client, flags.Outdir, flags.GitProtocol, flags.DistroPatchesLocation) } else { - distrodl = dist.NewDownloader(client, flags.GitProtocol, absDistroPatchesLocation) + distrodl = dist.NewDownloader(client, flags.GitProtocol, flags.DistroPatchesLocation) } if err := depsvl.ValidateBaseReqs(); err != nil { @@ -128,7 +94,7 @@ The command will dump into a 'distribution' folder in the working directory all } logrus.Info("Downloading distribution...") - res, err := distrodl.Download(flags.DistroLocation, absFuryctlPath) + res, err := distrodl.Download(flags.DistroLocation, flags.FuryctlPath) if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) @@ -144,39 +110,26 @@ The command will dump into a 'distribution' folder in the working directory all if !flags.SkipValidation { logrus.Info("Validating configuration file...") - if err := config.Validate(absFuryctlPath, res.RepoPath); err != nil { + if err := config.Validate(flags.FuryctlPath, res.RepoPath); err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) return fmt.Errorf("error while validating configuration file: %w", err) } + } else { + logrus.Info("Skipping configuration file validation") } - furyctlFile, err := yamlx.FromFileV2[map[any]any](absFuryctlPath) + furyctlFile, err := yamlx.FromFileV2[map[any]any](flags.FuryctlPath) if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) - return fmt.Errorf("%s - %w", absFuryctlPath, err) + return fmt.Errorf("%s - %w", flags.FuryctlPath, err) } // Note: this is already the right working directory because it is updated in the root command. - currentDir, err := os.Getwd() - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting current directory: %w", err) - } - - dumpDir := filepath.Join(currentDir, "distribution") - absDumpDir, err := filepath.Abs(dumpDir) - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting absolute path of dump directory: %w", err) - } + dumpDir := filepath.Join(viper.GetString("workdir"), "distribution") logrus.Info("Rendering distribution manifests...") @@ -185,7 +138,7 @@ The command will dump into a 'distribution' folder in the working directory all res.RepoPath, res.MinimalConf.Kind, dumpDir, - absFuryctlPath, + flags.FuryctlPath, flags.NoOverwrite, flags.DryRun, ) @@ -201,13 +154,13 @@ The command will dump into a 'distribution' folder in the working directory all tracker.Track(cmdEvent) if errors.Is(err, template.ErrTargetIsNotEmpty) { - return fmt.Errorf("%w: \"%s\"", ErrTargetIsNotEmpty, absDumpDir) + return fmt.Errorf("%w: \"%s\"", ErrTargetIsNotEmpty, dumpDir) } return fmt.Errorf("error while generating distribution manifests: %w", err) } - logrus.Info("Distribution manifests successfully dumped to ", absDumpDir) + logrus.Info("Distribution manifests successfully dumped to ", dumpDir) cmdEvent.AddSuccessMessage("Distribution manifests dumped successfully") tracker.Track(cmdEvent) @@ -219,7 +172,7 @@ The command will dump into a 'distribution' folder in the working directory all templateCmd.Flags().Bool( "dry-run", false, - "Furyctl will try its best to generate the manifests despite the errors", + "furyctl will try its best to generate the manifests despite the errors", ) templateCmd.Flags().Bool( @@ -235,7 +188,7 @@ The command will dump into a 'distribution' folder in the working directory all "Location where to download schemas, defaults and the distribution manifest from. "+ "It can either be a local path(eg: /path/to/distribution) or "+ "a remote URL(eg: git::git@github.com:sighupio/distribution?ref=BRANCH_NAME&depth=1). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) templateCmd.Flags().StringP( @@ -254,8 +207,12 @@ The command will dump into a 'distribution' folder in the working directory all templateCmd.Flags().String( "distro-patches", "", - "Location where to download distribution's user-made patches from. "+ - "Any format supported by hashicorp/go-getter can be used.", + "Location where the distribution's user-made patches can be downloaded from. "+ + "This can be either a local path (eg: /path/to/distro-patches) or "+ + "a remote URL (eg: git::git@github.com:your-org/distro-patches?depth=1&ref=BRANCH_NAME). "+ + "Any format supported by hashicorp/go-getter can be used."+ + " Patches within this location must be in a folder named after the distribution version (eg: v1.29.0) and "+ + "must have the same structure as the distribution's repository", ) return templateCmd @@ -269,14 +226,27 @@ func getDumpTemplateCmdFlags() (TemplateCmdFlags, error) { return TemplateCmdFlags{}, fmt.Errorf("%w: %w", ErrParsingFlag, err) } + furyctlPath, err := filepath.Abs(viper.GetString("config")) + if err != nil { + return TemplateCmdFlags{}, fmt.Errorf("error: %w", err) + } + + distroPatchesLocation := viper.GetString("distro-patches") + if distroPatchesLocation != "" { + distroPatchesLocation, err = filepath.Abs(distroPatchesLocation) + if err != nil { + return TemplateCmdFlags{}, fmt.Errorf("error while getting absolute path of distro patches location: %w", err) + } + } + return TemplateCmdFlags{ DryRun: viper.GetBool("dry-run"), NoOverwrite: viper.GetBool("no-overwrite"), SkipValidation: viper.GetBool("skip-validation"), Outdir: viper.GetString("outdir"), DistroLocation: viper.GetString("distro-location"), - FuryctlPath: viper.GetString("config"), + FuryctlPath: furyctlPath, GitProtocol: typedGitProtocol, - DistroPatchesLocation: viper.GetString("distro-patches"), + DistroPatchesLocation: distroPatchesLocation, }, nil } diff --git a/cmd/get.go b/cmd/get.go index 3d94e1dd2..07ed34e3e 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -13,7 +13,7 @@ import ( func NewGetCmd() *cobra.Command { getCmd := &cobra.Command{ Use: "get", - Short: "Get the kubeconfig, available upgrade paths for a cluster or compatible versions to use between SD, providers, furyctl.", + Short: "Get the kubeconfig, available upgrade paths for a cluster or compatible versions to use between SD, providers, furyctl", } getCmd.AddCommand(get.NewKubeconfigCmd()) diff --git a/cmd/get/kubeconfig.go b/cmd/get/kubeconfig.go index 0abba933d..a663a1a23 100644 --- a/cmd/get/kubeconfig.go +++ b/cmd/get/kubeconfig.go @@ -7,7 +7,6 @@ package get import ( "errors" "fmt" - "os" "path" "path/filepath" @@ -46,32 +45,23 @@ func NewKubeconfigCmd() *cobra.Command { } }, RunE: func(_ *cobra.Command, _ []string) error { + var err error ctn := app.GetContainerInstance() tracker := ctn.Tracker() tracker.Flush() // Get flags. - debug := viper.GetBool("debug") binPath := viper.GetString("bin-path") - furyctlPath := viper.GetString("config") - outDir := viper.GetString("outdir") + currentDir := viper.GetString("workdir") + debug := viper.GetBool("debug") distroLocation := viper.GetString("distro-location") + furyctlPath := viper.GetString("config") gitProtocol := viper.GetString("git-protocol") + outDir := viper.GetString("outdir") skipDepsDownload := viper.GetBool("skip-deps-download") skipDepsValidation := viper.GetBool("skip-deps-validation") - // Get Current dir. - logrus.Debug("Getting current directory path...") - - currentDir, err := os.Getwd() - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting current directory: %w", err) - } - // Get absolute path to the config file. furyctlPath, err = filepath.Abs(furyctlPath) if err != nil { @@ -81,27 +71,26 @@ func NewKubeconfigCmd() *cobra.Command { return fmt.Errorf("error while getting config directory: %w", err) } - // Get home dir. - logrus.Debug("Getting Home directory path...") + if binPath == "" { + binPath = path.Join(outDir, ".furyctl", "bin") + } else { + binPath, err = filepath.Abs(binPath) + if err != nil { + cmdEvent.AddErrorMessage(err) + tracker.Track(cmdEvent) + + return fmt.Errorf("error while getting absolute path for bin folder: %w", err) + } + } - homeDir, err := os.UserHomeDir() + typedGitProtocol, err := git.NewProtocol(gitProtocol) if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) - return fmt.Errorf("error while getting user home directory: %w", err) - } - - if outDir == "" { - outDir = homeDir + return fmt.Errorf("%w: %w", ErrParsingFlag, err) } - if binPath == "" { - binPath = path.Join(outDir, ".furyctl", "bin") - } - - parsedGitProtocol := (git.Protocol)(gitProtocol) - // Init packages. execx.Debug = debug @@ -114,9 +103,9 @@ func NewKubeconfigCmd() *cobra.Command { client := netx.NewGoGetterClient() if distroLocation == "" { - distrodl = dist.NewCachingDownloader(client, outDir, parsedGitProtocol, "") + distrodl = dist.NewCachingDownloader(client, outDir, typedGitProtocol, "") } else { - distrodl = dist.NewDownloader(client, parsedGitProtocol, "") + distrodl = dist.NewDownloader(client, typedGitProtocol, "") } // Validate base requirements. @@ -141,7 +130,7 @@ func NewKubeconfigCmd() *cobra.Command { basePath := path.Join(outDir, ".furyctl", res.MinimalConf.Metadata.Name) // Init second half of collaborators. - depsdl := dependencies.NewCachingDownloader(client, outDir, basePath, binPath, parsedGitProtocol) + depsdl := dependencies.NewCachingDownloader(client, outDir, basePath, binPath, typedGitProtocol) // Validate the furyctl.yaml file. logrus.Info("Validating configuration file...") @@ -161,6 +150,8 @@ func NewKubeconfigCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, err) } + } else { + logrus.Info("Skipping dependencies download") } // Validate the dependencies, unless explicitly told to skip it. @@ -172,6 +163,8 @@ func NewKubeconfigCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } + } else { + logrus.Info("Skipping dependencies validation") } getter, err := cluster.NewKubeconfigGetter(res.MinimalConf, res.DistroManifest, res.RepoPath, furyctlPath, currentDir) @@ -202,7 +195,7 @@ func NewKubeconfigCmd() *cobra.Command { "bin-path", "b", "", - "Path to the folder where all the dependencies' binaries are installed", + "Path to the folder where all the dependencies' binaries are downloaded", ) kubeconfigCmd.Flags().StringP( @@ -219,7 +212,7 @@ func NewKubeconfigCmd() *cobra.Command { "Location where to download schemas, defaults and the distribution manifests from. "+ "It can either be a local path (eg: /path/to/distribution) or "+ "a remote URL (eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) kubeconfigCmd.Flags().Bool( diff --git a/cmd/get/upgrade-paths.go b/cmd/get/upgrade-paths.go index c30a42c05..04e109d27 100644 --- a/cmd/get/upgrade-paths.go +++ b/cmd/get/upgrade-paths.go @@ -7,7 +7,6 @@ package get import ( "fmt" "io/fs" - "os" "path" "path/filepath" "strings" @@ -79,27 +78,26 @@ func NewUpgradePathsCmd() *cobra.Command { return fmt.Errorf("error while getting config directory: %w", err) } - // Get home dir. - logrus.Debug("Getting Home directory path...") + if binPath == "" { + binPath = path.Join(outDir, ".furyctl", "bin") + } else { + binPath, err = filepath.Abs(binPath) + if err != nil { + cmdEvent.AddErrorMessage(err) + tracker.Track(cmdEvent) - homeDir, err := os.UserHomeDir() + return fmt.Errorf("error while getting absolute path for bin folder: %w", err) + } + } + + typedGitProtocol, err := git.NewProtocol(gitProtocol) if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) - return fmt.Errorf("error while getting user home directory: %w", err) + return fmt.Errorf("%w: %w", ErrParsingFlag, err) } - if outDir == "" { - outDir = homeDir - } - - if binPath == "" { - binPath = path.Join(outDir, ".furyctl", "bin") - } - - parsedGitProtocol := (git.Protocol)(gitProtocol) - // Init packages. execx.Debug = debug @@ -125,9 +123,9 @@ func NewUpgradePathsCmd() *cobra.Command { client := netx.NewGoGetterClient() if distroLocation == "" { - distrodl = dist.NewCachingDownloader(client, outDir, parsedGitProtocol, "") + distrodl = dist.NewCachingDownloader(client, outDir, typedGitProtocol, "") } else { - distrodl = dist.NewDownloader(client, parsedGitProtocol, "") + distrodl = dist.NewDownloader(client, typedGitProtocol, "") } // Validate base requirements. @@ -152,7 +150,7 @@ func NewUpgradePathsCmd() *cobra.Command { basePath := path.Join(outDir, ".furyctl", res.MinimalConf.Metadata.Name) // Init second half of collaborators. - depsdl := dependencies.NewCachingDownloader(client, homeDir, basePath, binPath, parsedGitProtocol) + depsdl := dependencies.NewCachingDownloader(client, outDir, basePath, binPath, typedGitProtocol) // Validate the furyctl.yaml file. logrus.Info("Validating configuration file...") @@ -172,6 +170,8 @@ func NewUpgradePathsCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, err) } + } else { + logrus.Info("Skipping dependencies download") } // Validate the dependencies, unless explicitly told to skip it. @@ -183,6 +183,8 @@ func NewUpgradePathsCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } + } else { + logrus.Info("Skipping dependencies validation") } logrus.Debugf("either kind or fromVersion is not specified, reading them from the configuration file in path %s.", furyctlPath) @@ -203,7 +205,7 @@ func NewUpgradePathsCmd() *cobra.Command { } } - // We don't need the starting v in the version. Drop it if the user passes it. + // We don't need the "v" prefix in the version. Drop it if the user passes it. fromVersion, _ = strings.CutPrefix(fromVersion, "v") // Validate the kind. We don't need the normalised kind because we are checking against the folder names. @@ -213,6 +215,7 @@ func NewUpgradePathsCmd() *cobra.Command { return fmt.Errorf("error while validating kind: %w", err) } + globPattern := fmt.Sprintf("%s/%s/%s-*", "upgrades", strings.ToLower(kind), fromVersion) availablePaths, err := fs.Glob(configs.Tpl, globPattern) logrus.Debug("found folders: ", availablePaths) @@ -271,7 +274,7 @@ func NewUpgradePathsCmd() *cobra.Command { "Location where to download schemas, defaults and the distribution manifests from. "+ "It can either be a local path (eg: /path/to/distribution) or "+ "a remote URL (eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) upgradePathsCmd.Flags().Bool( @@ -289,14 +292,14 @@ func NewUpgradePathsCmd() *cobra.Command { upgradePathsCmd.Flags().String( "from", "", - "Show upgrade paths for the version specified (eg. 1.29.2) instead of the distribution version in the configuration file.", + "Show upgrade paths for the version specified (eg. 1.29.2) instead of the distribution version in the configuration file", ) upgradePathsCmd.Flags().StringP( "kind", "k", "", - "Show upgrade paths for the kind of cluster specified (eg: EKSCluster, KFDDistribution, OnPremises) instead of the kind defined in the configuration file.", + "Show upgrade paths for the kind of cluster specified instead of the kind defined in the configuration file. Options are: "+strings.Join(distribution.ConfigKinds(), ", "), ) if err := upgradePathsCmd.RegisterFlagCompletionFunc("kind", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { diff --git a/cmd/renew/certificates.go b/cmd/renew/certificates.go index 11529f4f7..349ff9b67 100644 --- a/cmd/renew/certificates.go +++ b/cmd/renew/certificates.go @@ -7,7 +7,6 @@ package renew import ( "errors" "fmt" - "os" "path" "path/filepath" @@ -68,27 +67,26 @@ func NewCertificatesCmd() *cobra.Command { return fmt.Errorf("error while getting config directory: %w", err) } - // Get home dir. - logrus.Debug("Getting Home directory path...") + if binPath == "" { + binPath = path.Join(outDir, ".furyctl", "bin") + } else { + binPath, err = filepath.Abs(binPath) + if err != nil { + cmdEvent.AddErrorMessage(err) + tracker.Track(cmdEvent) - homeDir, err := os.UserHomeDir() + return fmt.Errorf("error while getting absolute path for bin folder: %w", err) + } + } + + typedGitProtocol, err := git.NewProtocol(gitProtocol) if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) - return fmt.Errorf("error while getting user home directory: %w", err) + return fmt.Errorf("%w", err) } - if outDir == "" { - outDir = homeDir - } - - if binPath == "" { - binPath = path.Join(outDir, ".furyctl", "bin") - } - - parsedGitProtocol := (git.Protocol)(gitProtocol) - // Init packages. execx.Debug = debug @@ -101,9 +99,9 @@ func NewCertificatesCmd() *cobra.Command { client := netx.NewGoGetterClient() if distroLocation == "" { - distrodl = dist.NewCachingDownloader(client, outDir, parsedGitProtocol, "") + distrodl = dist.NewCachingDownloader(client, outDir, typedGitProtocol, "") } else { - distrodl = dist.NewDownloader(client, parsedGitProtocol, "") + distrodl = dist.NewDownloader(client, typedGitProtocol, "") } // Validate base requirements. @@ -128,7 +126,7 @@ func NewCertificatesCmd() *cobra.Command { basePath := path.Join(outDir, ".furyctl", res.MinimalConf.Metadata.Name) // Init second half of collaborators. - depsdl := dependencies.NewCachingDownloader(client, outDir, basePath, binPath, parsedGitProtocol) + depsdl := dependencies.NewCachingDownloader(client, outDir, basePath, binPath, typedGitProtocol) // Validate the furyctl.yaml file. logrus.Info("Validating configuration file...") @@ -148,6 +146,8 @@ func NewCertificatesCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, err) } + } else { + logrus.Info("Skipping dependencies download") } // Validate the dependencies, unless explicitly told to skip it. @@ -159,6 +159,8 @@ func NewCertificatesCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } + } else { + logrus.Info("Skipping dependencies validation") } renewer, err := cluster.NewCertificatesRenewer(res.MinimalConf, res.DistroManifest, res.RepoPath, furyctlPath) @@ -189,7 +191,7 @@ func NewCertificatesCmd() *cobra.Command { "bin-path", "b", "", - "Path to the folder where all the dependencies' binaries are installed", + "Path to the folder where all the dependencies' binaries are downloaded", ) certificatesCmd.Flags().StringP( @@ -206,13 +208,13 @@ func NewCertificatesCmd() *cobra.Command { "Location where to download schemas, defaults and the distribution manifests from. "+ "It can either be a local path (eg: /path/to/distribution) or "+ "a remote URL (eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) certificatesCmd.Flags().Bool( "skip-deps-download", false, - "Skip downloading the binaries", + "Skip downloading the distribution modules, installers and binaries", ) certificatesCmd.Flags().Bool( diff --git a/cmd/root.go b/cmd/root.go index 917d55d18..3f4527092 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -19,19 +19,21 @@ import ( "github.com/spf13/viper" "github.com/sighupio/furyctl/internal/app" + "github.com/sighupio/furyctl/internal/git" execx "github.com/sighupio/furyctl/internal/x/exec" iox "github.com/sighupio/furyctl/internal/x/io" logrusx "github.com/sighupio/furyctl/internal/x/logrus" ) type rootConfig struct { - Spinner *spinner.Spinner Debug bool DisableAnalytics bool DisableTty bool - Workdir string - Outdir string + GitProtocol git.Protocol Log string + Outdir string + Spinner *spinner.Spinner + Workdir string } type RootCommand struct { @@ -62,9 +64,14 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP ctn := app.GetContainerInstance() + // Configure analytics. tracker := ctn.Tracker() + if viper.GetBool("disable-analytics") { + tracker.Disable() + } defer tracker.Flush() + // Tab-autocompletion. if cmd.Name() == "__complete" { oldPreRunFunc := cmd.PreRun @@ -77,20 +84,47 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP } } - // Configure the spinner. - cflag := viper.GetBool("no-tty") + // Change working directory (if it is specified) as first thing so all the following paths are relative + // to this new working directory. + if workdir := viper.GetString("workdir"); workdir != "" { + // Get absolute path of workdir. + absWorkdir, err := filepath.Abs(workdir) + if err != nil { + logrus.Fatalf("Error getting absolute path of workdir: %v", err) + } - outDir := viper.GetString("outdir") + if err := os.Chdir(absWorkdir); err != nil { + logrus.Fatalf("Could not change directory: %v", err) + } - homeDir, err := os.UserHomeDir() - if err != nil { - logrus.Fatalf("error while getting user home directory: %v", err) + // We need to defer this log because we don't have logging configured yet. + defer logrus.Debugf("Changed working directory to %s", absWorkdir) + // Update the workdir in viper to the absolute path in case it is accessed from somewhere else. + viper.Set("workdir", absWorkdir) } + // Calculate the right outDir. + outDir := viper.GetString("outdir") if outDir == "" { + homeDir, err := os.UserHomeDir() + if err != nil { + logrus.Fatalf("error while getting user's home directory: %v", err) + } + outDir = homeDir } + // The outdir path must be an absolute path, relative paths can be messy because the current directory + // can change during execution, for example when rendering the templates. + outDir, err = filepath.Abs(outDir) + if err != nil { + logrus.Fatalf("error while getting absolute path for outdir: %v", err) + } + // We need to defer this log because we don't have logging configured yet. + defer logrus.Debugf("Outdir is set to %s", outDir) + viper.Set("outdir", outDir) + // Configure the logging options. Set the logging target (stdout or a file). + // Note that we can't move this upper because we depend on calculating the workdir and outdir first. logPath := viper.GetString("log") if logPath != "stdout" { if logPath == "" { @@ -114,36 +148,17 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP execx.LogFile = logFile } - // Set log level. + // Configure logging level and format. + cflag := viper.GetBool("no-tty") dflag := viper.GetBool("debug") logrusx.InitLog(logFile, dflag, cflag) - logrus.Debugf("logging to: %s", logPath) - - // Configure analytics. - aflag := viper.GetBool("disable-analytics") - if aflag { - tracker.Disable() - } - - // Change working directory if it is specified. - if workdir := viper.GetString("workdir"); workdir != "" { - // Get absolute path of workdir. - absWorkdir, err := filepath.Abs(workdir) - if err != nil { - logrus.Fatalf("Error getting absolute path of workdir: %v", err) - } - - if err := os.Chdir(absWorkdir); err != nil { - logrus.Fatalf("Could not change directory: %v", err) - } - - logrus.Debugf("Changed working directory to %s", absWorkdir) - } + logrus.Debugf("Writing logs to %s", logPath) + // Deprected flags. https := viper.GetBool("https") if !https { - logrus.Warn("The --https flag is deprecated, if you want to use SSH protocol to download repositories use --git-protocol ssh") + logrus.Warn("The --https flag is deprecated and https is the default, if you want to use SSH protocol to download repositories use --git-protocol ssh") } }, }, @@ -157,7 +172,7 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP "debug", "D", false, - "Enables furyctl debug output. This will greatly increase the verbosity. Notice that you can always access the debug output in the log file.", + "Enables furyctl debug output. This will greatly increase the verbosity. Notice that you can always access the debug output and even more logs in the log file", ) rootCmd.PersistentFlags().BoolVarP( @@ -181,7 +196,7 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP "workdir", "w", "", - "Switch to a different working directory before executing the given subcommand", + "Switch to a different working directory before executing the given subcommand. NOTE: this will affect all the paths passed the command, including other flags like outdir and log, for example", ) rootCmd.PersistentFlags().StringVarP( @@ -189,7 +204,7 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP "outdir", "o", "", - "Path where to create the data directory (.furyctl). Default is the user's home.", + "Path where to create the \".furyctl\" data directory. Default is the user's home", ) rootCmd.PersistentFlags().StringVarP( @@ -197,23 +212,28 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP "log", "l", "", - "Path to the log file or set to 'stdout' to log to standard output. Default is '/.furyctl/furyctl.-.log'", + "Path to a file or folder where to write logs to. Set to 'stdout' write to standard output. Target path will be created if it does not exists. Default is '/.furyctl/furyctl.-.log'", ) - rootCmd.PersistentFlags().StringP( + rootCmd.PersistentFlags().VarP( + &git.ProtocolFlag{Protocol: git.ProtocolHTTPS}, "git-protocol", "g", - "https", - "Download repositories using the given protocol (options: https, ssh). Use when SSH traffic is being blocked or when SSH "+ - "client has not been configured\nset the GITHUB_TOKEN environment variable with your token to use "+ - "authentication while downloading, for example for private repositories", + "Download repositories using the given protocol. Use when SSH traffic is being blocked or when SSH "+ + "client has not been configured\nset the GITHUB_TOKEN environment variable with your "+ + "token to use authentication while downloading, for example for private repositories. "+ + "\nOptions are: "+strings.Join(git.ProtocolsS(), ", ")+"", ) + if err := rootCmd.RegisterFlagCompletionFunc("git-protocol", git.ProtocolFlagCompletion); err != nil { + logrus.Fatalf("error while registering flag completion: %v", err) + } + rootCmd.PersistentFlags().BoolP( "https", "H", true, - "DEPRECATED: by default furyctl uses https protocol to download repositories", + "DEPRECATED: by default furyctl uses https protocol to download repositories. See --git-protocol flag", ) if err := viper.BindPFlags(rootCmd.PersistentFlags()); err != nil { diff --git a/cmd/validate/config.go b/cmd/validate/config.go index 800b42e10..51b11c489 100644 --- a/cmd/validate/config.go +++ b/cmd/validate/config.go @@ -7,7 +7,6 @@ package validate import ( "errors" "fmt" - "os" "path/filepath" "github.com/sirupsen/logrus" @@ -60,18 +59,6 @@ func NewConfigCmd() *cobra.Command { return fmt.Errorf("%w: %w", ErrParsingFlag, err) } - homeDir, err := os.UserHomeDir() - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting user home directory: %w", err) - } - - if outDir == "" { - outDir = homeDir - } - absDistroPatchesLocation := distroPatchesLocation if absDistroPatchesLocation != "" { @@ -153,14 +140,18 @@ func NewConfigCmd() *cobra.Command { "Location where to download schemas, defaults and the distribution manifests from. "+ "It can either be a local path (eg: /path/to/distribution) or "+ "a remote URL (eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) configCmd.Flags().String( "distro-patches", "", - "Location where to download distribution's user-made patches from. "+ - "Any format supported by hashicorp/go-getter can be used.", + "Location where the distribution's user-made patches can be downloaded from. "+ + "This can be either a local path (eg: /path/to/distro-patches) or "+ + "a remote URL (eg: git::git@github.com:your-org/distro-patches?depth=1&ref=BRANCH_NAME). "+ + "Any format supported by hashicorp/go-getter can be used."+ + " Patches within this location must be in a folder named after the distribution version (eg: v1.29.0) and "+ + "must have the same structure as the distribution's repository", ) return configCmd diff --git a/cmd/validate/dependencies.go b/cmd/validate/dependencies.go index 4aeeb4d8f..1b422b1b2 100644 --- a/cmd/validate/dependencies.go +++ b/cmd/validate/dependencies.go @@ -7,7 +7,6 @@ package validate import ( "errors" "fmt" - "os" "path/filepath" "github.com/sirupsen/logrus" @@ -55,21 +54,6 @@ func NewDependenciesCmd() *cobra.Command { binPath := viper.GetString("bin-path") gitProtocol := viper.GetString("git-protocol") - // Init paths. - logrus.Debug("Getting Home Directory Path...") - - homeDir, err := os.UserHomeDir() - if err != nil { - cmdEvent.AddErrorMessage(err) - tracker.Track(cmdEvent) - - return fmt.Errorf("error while getting user home directory: %w", err) - } - - if outDir == "" { - outDir = homeDir - } - typedGitProtocol, err := git.NewProtocol(gitProtocol) if err != nil { return fmt.Errorf("%w: %w", ErrParsingFlag, err) @@ -198,14 +182,18 @@ func NewDependenciesCmd() *cobra.Command { "Location where to download schemas, defaults and the distribution manifests from. "+ "It can either be a local path (eg: /path/to/distribution) or "+ "a remote URL (eg: git::git@github.com:sighupio/distribution?depth=1&ref=BRANCH_NAME). "+ - "Any format supported by hashicorp/go-getter can be used.", + "Any format supported by hashicorp/go-getter can be used", ) dependenciesCmd.Flags().String( "distro-patches", "", - "Location where to download distribution's user-made patches from. "+ - "Any format supported by hashicorp/go-getter can be used.", + "Location where the distribution's user-made patches can be downloaded from. "+ + "This can be either a local path (eg: /path/to/distro-patches) or "+ + "a remote URL (eg: git::git@github.com:your-org/distro-patches?depth=1&ref=BRANCH_NAME). "+ + "Any format supported by hashicorp/go-getter can be used."+ + " Patches within this location must be in a folder named after the distribution version (eg: v1.29.0) and "+ + "must have the same structure as the distribution's repository", ) return dependenciesCmd diff --git a/internal/apis/kfd/v1alpha2/common/create/plugins.go b/internal/apis/kfd/v1alpha2/common/create/plugins.go index d44ac8b80..169555e8b 100644 --- a/internal/apis/kfd/v1alpha2/common/create/plugins.go +++ b/internal/apis/kfd/v1alpha2/common/create/plugins.go @@ -111,7 +111,7 @@ func (p *Plugins) Exec() error { confPath := filepath.Join(outDirPath1, "config.yaml") - logrus.Debugf("config path = %s", confPath) + logrus.Debugf("Plugins configuration file path %s", confPath) if err = os.WriteFile(confPath, outYaml, iox.FullRWPermAccess); err != nil { return fmt.Errorf("error writing config file: %w", err) diff --git a/internal/apis/kfd/v1alpha2/common/create/preupgrade.go b/internal/apis/kfd/v1alpha2/common/create/preupgrade.go index 7c181fbda..03caefc0b 100644 --- a/internal/apis/kfd/v1alpha2/common/create/preupgrade.go +++ b/internal/apis/kfd/v1alpha2/common/create/preupgrade.go @@ -135,7 +135,7 @@ func (p *PreUpgrade) Exec() error { confPath := filepath.Join(outDirPath1, "config.yaml") - logrus.Debugf("config path = %s", confPath) + logrus.Debugf("Preupgrade configuration file path %s", confPath) if err = os.WriteFile(confPath, outYaml, iox.FullRWPermAccess); err != nil { return fmt.Errorf("error writing config file: %w", err) diff --git a/internal/cluster/phase.go b/internal/cluster/phase.go index 12e99a476..977931f21 100644 --- a/internal/cluster/phase.go +++ b/internal/cluster/phase.go @@ -278,7 +278,7 @@ func (*OperationPhase) CopyFromTemplate( confPath := filepath.Join(outDirPath, "config.yaml") - logrus.Debugf("config path = %s", confPath) + logrus.Debugf("%s configuration file path %s", prefix, confPath) if err = os.WriteFile(confPath, outYaml, iox.FullRWPermAccess); err != nil { return fmt.Errorf("error writing config file: %w", err) diff --git a/internal/distribution/iac.go b/internal/distribution/iac.go index 4745ea9b9..49252aa2c 100644 --- a/internal/distribution/iac.go +++ b/internal/distribution/iac.go @@ -133,7 +133,7 @@ func (m *IACBuilder) Build() error { confPath := filepath.Join(outDirPath, "config.yaml") - logrus.Debugf("config path = %s", confPath) + logrus.Debugf("IaC configuration file path %s", confPath) if err = os.WriteFile(confPath, outYaml, iox.FullRWPermAccess); err != nil { return fmt.Errorf("error writing config file: %w", err) diff --git a/internal/git/flag.go b/internal/git/flag.go new file mode 100644 index 000000000..d7e423f73 --- /dev/null +++ b/internal/git/flag.go @@ -0,0 +1,43 @@ +// Copyright (c) 2017-present SIGHUP s.r.l All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package git + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" +) + +type ProtocolFlag struct { + Protocol Protocol +} + +func (p *ProtocolFlag) String() string { + return string(p.Protocol) +} + +func (p *ProtocolFlag) Set(value string) error { + protocol, err := NewProtocol(value) + if err != nil { + return fmt.Errorf("%w: \"%s\". Supported protocols are %s", + ErrUnsupportedGitProtocol, + value, + strings.Join(ProtocolsS(), ", "), + ) + } + + p.Protocol = protocol + + return nil +} + +func (p *ProtocolFlag) Type() string { //nolint:revive // I know p is not used + return "git-protocol" +} + +func ProtocolFlagCompletion(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { + return ProtocolsS(), cobra.ShellCompDirectiveDefault +} diff --git a/internal/git/protocols.go b/internal/git/protocols.go index 5dd398f12..37482760a 100644 --- a/internal/git/protocols.go +++ b/internal/git/protocols.go @@ -7,6 +7,7 @@ package git import ( "errors" "fmt" + "strings" ) var ErrUnsupportedGitProtocol = errors.New("unsupported git protocol") @@ -20,7 +21,11 @@ func NewProtocol(protocol string) (Protocol, error) { return ProtocolHTTPS, nil } - return "", fmt.Errorf("%w: %s", ErrUnsupportedGitProtocol, protocol) + return "", fmt.Errorf("%w: %s. Supported protocols are %s", + ErrUnsupportedGitProtocol, + protocol, + strings.Join(ProtocolsS(), ", "), + ) } type Protocol string @@ -29,3 +34,21 @@ const ( ProtocolSSH = Protocol("ssh") ProtocolHTTPS = Protocol("https") ) + +// Protocols returns a slice of Protocols that are supported. +func Protocols() []Protocol { + return []Protocol{ + ProtocolSSH, + ProtocolHTTPS, + } +} + +// ProtocolsS returns a slice of Strings representation of the Protocols that are supported. +func ProtocolsS() []string { + protocols := []string{} + for _, p := range Protocols() { + protocols = append(protocols, string(p)) + } + + return protocols +} diff --git a/pkg/dependencies/download.go b/pkg/dependencies/download.go index a0a6d141b..f5f78f293 100644 --- a/pkg/dependencies/download.go +++ b/pkg/dependencies/download.go @@ -75,7 +75,7 @@ func (dd *Downloader) DownloadAll(kfd config.KFD) ([]error, []string) { vendorFolder := filepath.Join(dd.basePath, "vendor") - logrus.Debug("Cleaning vendor folder") + logrus.Debug("Cleaning vendor folder ", vendorFolder) if err := iox.CheckDirIsEmpty(vendorFolder); err != nil { if err := os.RemoveAll(vendorFolder); err != nil { diff --git a/pkg/distribution/download.go b/pkg/distribution/download.go index ff26a8d02..b2316e3d9 100644 --- a/pkg/distribution/download.go +++ b/pkg/distribution/download.go @@ -142,6 +142,7 @@ func (d *Downloader) DoDownload( src := url dst := filepath.Join(baseDst, "data") + logrus.Debugf("Downloading distribution from %s to %s", src, dst) if err := d.client.Download(src, dst); err != nil { if errors.Is(err, netx.ErrDownloadOptionsExhausted) { From 60a756473887744e300c6071055fc82015d8fbc1 Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Mon, 12 May 2025 18:15:23 +0200 Subject: [PATCH 02/14] fix: various fixes - Fix logic on some commands now that we have outdir always set to an absolute path - Fix some UX messages --- cmd/apply.go | 21 +++++++------------ cmd/create/pki.go | 17 +++++++++++---- cmd/delete/cluster.go | 4 ++-- cmd/diff.go | 12 ++++++++--- cmd/get/kubeconfig.go | 4 ++-- cmd/get/upgrade-paths.go | 12 +++++------ cmd/renew/certificates.go | 4 ++-- cmd/root.go | 2 +- cmd/validate/dependencies.go | 19 ++++++++++++++--- .../kfd/v1alpha2/common/create/preupgrade.go | 3 ++- internal/cluster/util.go | 2 +- pkg/distribution/download.go | 14 +++++++++++-- test/e2e/furyctl_test.go | 2 +- 13 files changed, 75 insertions(+), 41 deletions(-) diff --git a/cmd/apply.go b/cmd/apply.go index f00185255..2f2541c79 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -79,17 +79,10 @@ func NewApplyCmd() *cobra.Command { applyCmd := &cobra.Command{ Use: "apply", Short: "Apply the configuration to create, update, or upgrade a battle-tested SIGHUP Distribution cluster", - Example: ` Apply the configuration file and all the phases using the default configuration file: - furyctl apply - - Apply a custom configuration file: - furyctl apply --config mycluster.yaml - - Apply a single phase, for example the distribution phase: - furyctl apply --phase distribution - - Apply all the phases, and repeat the distribution phase afterwards: - furyctl apply --post-apply-phases distribution + Example: ` furyctl apply Apply all the configuration to the cluster using the default configuration file name + furyctl apply --config mycluster.yaml Apply a custom configuration file + furyctl apply --phase distribution Apply a single phase, for example the distribution phase + furyctl apply --post-apply-phases distribution Apply all the phases, and repeat the distribution phase afterwards `, PreRun: func(cmd *cobra.Command, _ []string) { cmdEvent = analytics.NewCommandEvent(cobrax.GetFullname(cmd)) @@ -119,6 +112,8 @@ func NewApplyCmd() *cobra.Command { var distrodl *dist.Downloader + logrus.Debugf("Using configuration file from path %s", flags.FuryctlPath) + // Init first half of collaborators. client := netx.NewGoGetterClient() executor := execx.NewStdExecutor() @@ -218,7 +213,7 @@ func NewApplyCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, errs) } } else { - logrus.Info("Skipping dependencies download") + logrus.Info("Dependencies download skipped") } // Validate the dependencies, unless explicitly told to skip it. @@ -231,7 +226,7 @@ func NewApplyCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } } else { - logrus.Info("Skipping dependencies validation") + logrus.Info("Dependencies validation skipped") } // Define cluster creation paths. diff --git a/cmd/create/pki.go b/cmd/create/pki.go index cddf065dc..cb1760f96 100644 --- a/cmd/create/pki.go +++ b/cmd/create/pki.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "net" + "path/filepath" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -107,11 +108,18 @@ You can limit the creation of the PKI to just etcd or just Kubernetes using the tracker := ctn.Tracker() defer tracker.Flush() - // Get flags - // maybe we could get this path from the furyctl.yaml file. - pkiPath := viper.GetString("path") + // Get flags. etcd := viper.GetBool("etcd") controlplane := viper.GetBool("controlplane") + // Maybe we could get this path from the furyctl.yaml file in the future. + pkiPath := viper.GetString("path") + pkiPath, err := filepath.Abs(pkiPath) + if err != nil { + tracker.Track(cmdEvent) + cmdEvent.AddErrorMessage(err) + + return fmt.Errorf("error while getting absolute path for PKI folder path: %w", err) + } if err := NewPki(etcd, controlplane, pkiPath); err != nil { cmdEvent.AddErrorMessage(err) @@ -119,8 +127,9 @@ You can limit the creation of the PKI to just etcd or just Kubernetes using the return fmt.Errorf("PKI creation failed with error: %w", err) } - cmdEvent.AddSuccessMessage("PKI files successfully created at:" + pkiPath) + cmdEvent.AddSuccessMessage("PKI files successfully created at" + pkiPath) tracker.Track(cmdEvent) + logrus.Infof("PKI files successfully created at %s", pkiPath) return nil }, diff --git a/cmd/delete/cluster.go b/cmd/delete/cluster.go index af382fbfd..15a441157 100644 --- a/cmd/delete/cluster.go +++ b/cmd/delete/cluster.go @@ -195,7 +195,7 @@ func NewClusterCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, errs) } } else { - logrus.Info("Skipping dependencies download") + logrus.Info("Dependencies download skipped") } // Validate the dependencies, unless explicitly told to skip it. @@ -208,7 +208,7 @@ func NewClusterCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } } else { - logrus.Info("Skipping dependencies validation") + logrus.Info("Dependencies validation skipped") } // Define cluster deletion paths. diff --git a/cmd/diff.go b/cmd/diff.go index 4d690525b..d7a86bcaf 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -81,6 +81,9 @@ func NewDiffCmd() *cobra.Command { logrus.Info("Downloading distribution...") res, err := distrodl.Download(flags.DistroLocation, flags.FuryctlPath) if err != nil { + cmdEvent.AddErrorMessage(err) + tracker.Track(cmdEvent) + return fmt.Errorf("error while downloading distribution: %w", err) } @@ -295,9 +298,12 @@ func getDiffCommandFlags() (DiffCommandFlags, error) { } } - distroPatchesLocation, err := filepath.Abs(viper.GetString("distro-patches")) - if err != nil { - return DiffCommandFlags{}, fmt.Errorf("error while getting absolute path of distro patches location: %w", err) + distroPatchesLocation := viper.GetString("distro-patches") + if distroPatchesLocation != "" { + distroPatchesLocation, err = filepath.Abs(distroPatchesLocation) + if err != nil { + return DiffCommandFlags{}, fmt.Errorf("error while getting absolute path of distro patches location: %w", err) + } } phase := viper.GetString("phase") diff --git a/cmd/get/kubeconfig.go b/cmd/get/kubeconfig.go index a663a1a23..1c1bad500 100644 --- a/cmd/get/kubeconfig.go +++ b/cmd/get/kubeconfig.go @@ -151,7 +151,7 @@ func NewKubeconfigCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, err) } } else { - logrus.Info("Skipping dependencies download") + logrus.Info("Dependencies download skipped") } // Validate the dependencies, unless explicitly told to skip it. @@ -164,7 +164,7 @@ func NewKubeconfigCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } } else { - logrus.Info("Skipping dependencies validation") + logrus.Info("Dependencies validation skipped") } getter, err := cluster.NewKubeconfigGetter(res.MinimalConf, res.DistroManifest, res.RepoPath, furyctlPath, currentDir) diff --git a/cmd/get/upgrade-paths.go b/cmd/get/upgrade-paths.go index 04e109d27..7548ee0a1 100644 --- a/cmd/get/upgrade-paths.go +++ b/cmd/get/upgrade-paths.go @@ -38,10 +38,10 @@ func NewUpgradePathsCmd() *cobra.Command { Use: "upgrade-paths", Short: "Get available upgrade paths for the kind and version defined in the configuration file or a custom one.", Long: `Get available upgrade paths for the kind and version defined in the configuration file or a custom one. If the "--from" or "--kind" parameters are specified, the command will give the upgrade path for those instead.`, - Example: `- furyctl get upgrade-paths will show the available upgrade paths for the kind and distribution version defined in the configuration file (furyctl.yaml by default) -- furyctl get upgrade-paths --from vX.Y.Z will show the available upgrade paths for the kind defined in the configuration file but for the version X.Y.Z instead. -- furyctl get upgrade-paths --kind OnPremises will show the available upgrade paths for the version defined in the configuration file but for the OnPremises kind, even if the cluster is an EKSCluster, for example. -- furyctl get upgrade-paths --kind OnPremises --from X.Y.X will show the available upgrade paths for the version X.Y.Z of the OnPremises kind, without reading the configuration file. + Example: ` furyctl get upgrade-paths shows the available upgrade paths for the kind and distribution version defined in the configuration file (furyctl.yaml by default) + furyctl get upgrade-paths --from vX.Y.Z shows the available upgrade paths for the kind defined in the configuration file but for the version X.Y.Z instead. + furyctl get upgrade-paths --kind OnPremises shows the available upgrade paths for the version defined in the configuration file but for the OnPremises kind, even if the cluster is an EKSCluster, for example. + furyctl get upgrade-paths --kind OnPremises --from X.Y.X shows the available upgrade paths for the version X.Y.Z of the OnPremises kind, without reading the configuration file. `, PreRun: func(cmd *cobra.Command, _ []string) { cmdEvent = analytics.NewCommandEvent(cobrax.GetFullname(cmd)) @@ -171,7 +171,7 @@ func NewUpgradePathsCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, err) } } else { - logrus.Info("Skipping dependencies download") + logrus.Info("Dependencies download skipped") } // Validate the dependencies, unless explicitly told to skip it. @@ -184,7 +184,7 @@ func NewUpgradePathsCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } } else { - logrus.Info("Skipping dependencies validation") + logrus.Info("Dependencies validation skipped") } logrus.Debugf("either kind or fromVersion is not specified, reading them from the configuration file in path %s.", furyctlPath) diff --git a/cmd/renew/certificates.go b/cmd/renew/certificates.go index 349ff9b67..1db8138dd 100644 --- a/cmd/renew/certificates.go +++ b/cmd/renew/certificates.go @@ -147,7 +147,7 @@ func NewCertificatesCmd() *cobra.Command { return fmt.Errorf("%w: %v", ErrDownloadDependenciesFailed, err) } } else { - logrus.Info("Skipping dependencies download") + logrus.Info("Dependencies download skipped") } // Validate the dependencies, unless explicitly told to skip it. @@ -160,7 +160,7 @@ func NewCertificatesCmd() *cobra.Command { return fmt.Errorf("error while validating dependencies: %w", err) } } else { - logrus.Info("Skipping dependencies validation") + logrus.Info("Dependencies validation skipped") } renewer, err := cluster.NewCertificatesRenewer(res.MinimalConf, res.DistroManifest, res.RepoPath, furyctlPath) diff --git a/cmd/root.go b/cmd/root.go index 3f4527092..bde86e4b2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -196,7 +196,7 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP "workdir", "w", "", - "Switch to a different working directory before executing the given subcommand. NOTE: this will affect all the paths passed the command, including other flags like outdir and log, for example", + "Switch to a different working directory before executing the given subcommand. NOTE: this will affect all the paths passed, including other flags like outdir and log, for example", ) rootCmd.PersistentFlags().StringVarP( diff --git a/cmd/validate/dependencies.go b/cmd/validate/dependencies.go index 1b422b1b2..ad2235ff1 100644 --- a/cmd/validate/dependencies.go +++ b/cmd/validate/dependencies.go @@ -41,6 +41,7 @@ func NewDependenciesCmd() *cobra.Command { } }, RunE: func(_ *cobra.Command, _ []string) error { + var err error ctn := app.GetContainerInstance() tracker := ctn.Tracker() @@ -51,9 +52,21 @@ func NewDependenciesCmd() *cobra.Command { distroPatchesLocation := viper.GetString("distro-patches") outDir := viper.GetString("outdir") + binPath := viper.GetString("bin-path") - gitProtocol := viper.GetString("git-protocol") + if binPath == "" { + binPath = filepath.Join(outDir, ".furyctl", "bin") + } else { + binPath, err = filepath.Abs(binPath) + if err != nil { + cmdEvent.AddErrorMessage(err) + tracker.Track(cmdEvent) + return fmt.Errorf("error while getting absolute path for bin folder: %w", err) + } + } + + gitProtocol := viper.GetString("git-protocol") typedGitProtocol, err := git.NewProtocol(gitProtocol) if err != nil { return fmt.Errorf("%w: %w", ErrParsingFlag, err) @@ -145,8 +158,8 @@ func NewDependenciesCmd() *cobra.Command { tracker.Track(cmdEvent) logrus.Info( - "You can use the 'furyctl download dependencies' command to download most dependencies, " + - "and a package manager such as 'asdf' to install the remaining ones.", + "You can use the `furyctl download dependencies` command to download most dependencies, " + + "and a package manager such as `asdf` or `mise` to install the remaining ones.", ) return ErrDependencies diff --git a/internal/apis/kfd/v1alpha2/common/create/preupgrade.go b/internal/apis/kfd/v1alpha2/common/create/preupgrade.go index 03caefc0b..1eaec78a7 100644 --- a/internal/apis/kfd/v1alpha2/common/create/preupgrade.go +++ b/internal/apis/kfd/v1alpha2/common/create/preupgrade.go @@ -236,7 +236,8 @@ func (p *PreUpgrade) Exec() error { // }. if !cluster.IsForceEnabledForFeature(p.forceFlag, cluster.ForceFeatureUpgrades) { - if _, err := fmt.Println("Are you sure you want to continue? Only 'yes' will be accepted to confirm."); err != nil { + _, err := fmt.Println("\nAre you sure you want to continue? Only 'yes' will be accepted to confirm.") + if err != nil { return fmt.Errorf("error writing to stdout: %w", err) } diff --git a/internal/cluster/util.go b/internal/cluster/util.go index dc9109219..4370b00ab 100644 --- a/internal/cluster/util.go +++ b/internal/cluster/util.go @@ -36,7 +36,7 @@ func AskConfirmation(force bool) (bool, error) { return false, fmt.Errorf("error while printing to stdout: %w", err) } - if _, err := fmt.Println("Are you sure you want to continue? Only 'yes' will be accepted to confirm."); err != nil { + if _, err := fmt.Println("\nAre you sure you want to continue? Only 'yes' will be accepted to confirm."); err != nil { return false, fmt.Errorf("error while printing to stdout: %w", err) } diff --git a/pkg/distribution/download.go b/pkg/distribution/download.go index b2316e3d9..890984ee3 100644 --- a/pkg/distribution/download.go +++ b/pkg/distribution/download.go @@ -40,7 +40,7 @@ var ( ErrWriteFile = errors.New("error writing file") ErrYamlMarshalFile = errors.New("error marshaling yaml file") ErrYamlUnmarshalFile = errors.New("error unmarshaling yaml file") - ErrUnsupportedVersion = errors.New("unsupported KFD version") + ErrUnsupportedVersion = errors.New("unsupported SD version") ) type DownloadResult struct { @@ -147,10 +147,20 @@ func (d *Downloader) DoDownload( if err := d.client.Download(src, dst); err != nil { if errors.Is(err, netx.ErrDownloadOptionsExhausted) { if distroLocation == "" { + msg := "try another version from the official repository" + if !strings.HasPrefix(minimalConf.Spec.DistributionVersion, "v") { + msg = fmt.Sprintf( + "versions usually have the `v` prefix as in `v%s`, you may want to try adding the prefix or %s", + minimalConf.Spec.DistributionVersion, + msg, + ) + } + return DownloadResult{}, fmt.Errorf("%w: seems like the specified version "+ - "%s does not exist, try another version from the official repository", + "%s does not exist, %v", ErrUnsupportedVersion, minimalConf.Spec.DistributionVersion, + msg, ) } diff --git a/test/e2e/furyctl_test.go b/test/e2e/furyctl_test.go index 40a7545d4..6d3d772c9 100644 --- a/test/e2e/furyctl_test.go +++ b/test/e2e/furyctl_test.go @@ -171,7 +171,7 @@ var ( out, err := FuryctlValidateConfig("../data/e2e/validate/config/nodistro") Expect(err).To(HaveOccurred()) - Expect(out).To(ContainSubstring("unsupported KFD version")) + Expect(out).To(ContainSubstring("unsupported SD version")) }) It("should report an error when config validation fails", func() { From cc84ba8eeb5a0279e8988f7d6f173b0f8343d015 Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Mon, 12 May 2025 18:55:02 +0200 Subject: [PATCH 03/14] chore(get/supported-versions): standardise output --- cmd/get/supported-versions.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/get/supported-versions.go b/cmd/get/supported-versions.go index 7e8fc858e..aa7485b58 100644 --- a/cmd/get/supported-versions.go +++ b/cmd/get/supported-versions.go @@ -33,8 +33,8 @@ func NewSupportedVersionsCmd() *cobra.Command { Use: "supported-versions", Short: "List of currently supported SD versions and their compatibility with this version of furyctl for each kind.", Long: "List of currently supported SD versions and their compatibility with this version of furyctl for each kind. If the `--kind` parameter is specified, the command will only provide information about the selected provider.", - Example: ` - furyctl get supported-versions will list the currently supported SD versions and their compatibility with this version of furyctl for each kind. - - furyctl get supported-versions --kind OnPremises will list the currently supported SD versions and their compatibility with this version of furyctl but for the OnPremises kind. + Example: ` furyctl get supported-versions lists the currently supported SD versions and their compatibility with this version of furyctl for all kinds. + furyctl get supported-versions --kind OnPremises lists the currently supported SD versions and their compatibility with this version of furyctl but for the OnPremises kind. `, PreRun: func(cmd *cobra.Command, _ []string) { cmdEvent = analytics.NewCommandEvent(cobrax.GetFullname(cmd)) From 031411b9d8500d5fbd32bbf4ee81cb19bada9b77 Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Mon, 12 May 2025 18:59:28 +0200 Subject: [PATCH 04/14] fix(delete/cluster): command does not support plugins phase --- cmd/delete/cluster.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/cmd/delete/cluster.go b/cmd/delete/cluster.go index 15a441157..6411d24b8 100644 --- a/cmd/delete/cluster.go +++ b/cmd/delete/cluster.go @@ -321,11 +321,19 @@ func NewClusterCmd() *cobra.Command { "phase", "p", "", - "Limit execution to a specific phase. Options are: "+strings.Join(cluster.MainPhases(), ", "), + "Limit execution to a specific phase. Options are: "+strings.Join([]string{ + cluster.OperationPhaseInfrastructure, + cluster.OperationPhaseKubernetes, + cluster.OperationPhaseDistribution, + }, ", "), ) if err := clusterCmd.RegisterFlagCompletionFunc("phase", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return cluster.MainPhases(), cobra.ShellCompDirectiveDefault + return []string{ + cluster.OperationPhaseInfrastructure, + cluster.OperationPhaseKubernetes, + cluster.OperationPhaseDistribution, + }, cobra.ShellCompDirectiveDefault }); err != nil { logrus.Fatalf("error while registering flag completion: %v", err) } From b74c60dd79f6eaac2a82745abc70a3018efc4c52 Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Tue, 13 May 2025 14:28:22 +0200 Subject: [PATCH 05/14] chore(ux): add a new line so make the warning more readable --- cmd/delete/cluster.go | 4 ++-- internal/cluster/util.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/delete/cluster.go b/cmd/delete/cluster.go index 6411d24b8..38cfd9f62 100644 --- a/cmd/delete/cluster.go +++ b/cmd/delete/cluster.go @@ -239,7 +239,7 @@ func NewClusterCmd() *cobra.Command { } if !flags.Force { - _, err = fmt.Println("WARNING: You are about to delete a cluster. This action is irreversible.") + _, err = fmt.Println("\nWARNING: You are about to delete a cluster. This action is irreversible.") if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) @@ -247,7 +247,7 @@ func NewClusterCmd() *cobra.Command { return fmt.Errorf("error while printing to stdout: %w", err) } - _, err = fmt.Println("\nAre you sure you want to continue? Only 'yes' will be accepted to confirm.") + _, err = fmt.Println("Are you sure you want to continue? Only 'yes' will be accepted to confirm.") if err != nil { cmdEvent.AddErrorMessage(err) tracker.Track(cmdEvent) diff --git a/internal/cluster/util.go b/internal/cluster/util.go index 4370b00ab..b957ee455 100644 --- a/internal/cluster/util.go +++ b/internal/cluster/util.go @@ -32,11 +32,11 @@ func IsForceEnabledForFeature(force []string, feature string) bool { //nolint:revive // force bool needs to be here func AskConfirmation(force bool) (bool, error) { if !force { - if _, err := fmt.Println("WARNING: You are about to apply changes to the cluster configuration."); err != nil { + if _, err := fmt.Println("\nWARNING: You are about to apply changes to the cluster configuration."); err != nil { return false, fmt.Errorf("error while printing to stdout: %w", err) } - if _, err := fmt.Println("\nAre you sure you want to continue? Only 'yes' will be accepted to confirm."); err != nil { + if _, err := fmt.Println("Are you sure you want to continue? Only 'yes' will be accepted to confirm."); err != nil { return false, fmt.Errorf("error while printing to stdout: %w", err) } From 49627b0b065828e36dcaf033b3acf6cc465ddb42 Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Thu, 15 May 2025 11:29:29 +0200 Subject: [PATCH 06/14] chore(ux/parser): put env var name between quotes Put var name between qoutes in the error message to make evident if there is a space in the env var name, for exmaple. --- internal/parser/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/parser/config.go b/internal/parser/config.go index 77f76c8cd..8444e4233 100644 --- a/internal/parser/config.go +++ b/internal/parser/config.go @@ -54,7 +54,7 @@ func (p *ConfigParser) ParseDynamicValue(val any) (string, error) { case Env: envVar, exists := os.LookupEnv(sourceValue) if !exists { - return "", fmt.Errorf("%w: %s is empty", ErrCannotParseDynamicValue, sourceValue) + return "", fmt.Errorf("%w: \"%s\" is empty", ErrCannotParseDynamicValue, sourceValue) } envVar = strings.TrimRight(envVar, "\n") From 549504935040372dd4330752213c5f0d4e65726b Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Thu, 15 May 2025 12:45:56 +0200 Subject: [PATCH 07/14] chore: improve log message --- internal/apis/kfd/v1alpha2/ekscluster/create/preflight.go | 2 +- internal/apis/kfd/v1alpha2/kfddistribution/create/preflight.go | 2 +- internal/apis/kfd/v1alpha2/onpremises/create/preflight.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/apis/kfd/v1alpha2/ekscluster/create/preflight.go b/internal/apis/kfd/v1alpha2/ekscluster/create/preflight.go index d5db8e3b7..d4c1bb787 100644 --- a/internal/apis/kfd/v1alpha2/ekscluster/create/preflight.go +++ b/internal/apis/kfd/v1alpha2/ekscluster/create/preflight.go @@ -246,7 +246,7 @@ func (p *PreFlight) Exec(renderedConfig map[string]any) (*Status, error) { diffChecker.DiffToString(d), ) - logrus.Info("Cluster configuration has changed, checking for immutable violations...") + logrus.Info("Cluster configuration has changed, checking for immutability violations...") if err := p.CheckImmutablesDiffs(d, diffChecker); err != nil { return status, fmt.Errorf("error checking state diffs: %w", err) diff --git a/internal/apis/kfd/v1alpha2/kfddistribution/create/preflight.go b/internal/apis/kfd/v1alpha2/kfddistribution/create/preflight.go index 02d2a7ec0..128aacc60 100644 --- a/internal/apis/kfd/v1alpha2/kfddistribution/create/preflight.go +++ b/internal/apis/kfd/v1alpha2/kfddistribution/create/preflight.go @@ -168,7 +168,7 @@ func (p *PreFlight) Exec(renderedConfig map[string]any) (*Status, error) { diffChecker.DiffToString(d), ) - logrus.Info("Cluster configuration has changed, checking for immutable violations...") + logrus.Info("Cluster configuration has changed, checking for immutability violations...") if err := p.CheckStateDiffs(d, diffChecker); err != nil { return status, fmt.Errorf("error checking state diffs: %w", err) diff --git a/internal/apis/kfd/v1alpha2/onpremises/create/preflight.go b/internal/apis/kfd/v1alpha2/onpremises/create/preflight.go index cf952472d..232881e19 100644 --- a/internal/apis/kfd/v1alpha2/onpremises/create/preflight.go +++ b/internal/apis/kfd/v1alpha2/onpremises/create/preflight.go @@ -189,7 +189,7 @@ func (p *PreFlight) Exec(renderedConfig map[string]any) (*Status, error) { diffChecker.DiffToString(d), ) - logrus.Info("Cluster configuration has changed, checking for immutable violations...") + logrus.Info("Cluster configuration has changed, checking for immutability violations...") if err := p.CheckStateDiffs(d, diffChecker); err != nil { return status, fmt.Errorf("error checking state diffs: %w", err) From ea469c096ca3af4c067e6535d19180abefb7774c Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Fri, 16 May 2025 11:37:47 +0200 Subject: [PATCH 08/14] ux: improve warning on unsafe changes mention that the changes are not potentially safe, the user already knows they are applying new configuration. --- internal/cluster/util.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/cluster/util.go b/internal/cluster/util.go index b957ee455..238bcac63 100644 --- a/internal/cluster/util.go +++ b/internal/cluster/util.go @@ -32,7 +32,8 @@ func IsForceEnabledForFeature(force []string, feature string) bool { //nolint:revive // force bool needs to be here func AskConfirmation(force bool) (bool, error) { if !force { - if _, err := fmt.Println("\nWARNING: You are about to apply changes to the cluster configuration."); err != nil { + if _, err := fmt.Println("\nWARNING: You are about to apply changes to the cluster configuration " + + "that could potentially produce data loss or service disruption."); err != nil { return false, fmt.Errorf("error while printing to stdout: %w", err) } From a9be1dfd929ed14a0f91a16044ed6a5e7cfad273 Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Fri, 30 May 2025 18:58:53 +0200 Subject: [PATCH 09/14] fix: apply suggestions from code review define `err` variable in func output instead of standalone Co-authored-by: Manuel Romei --- cmd/apply.go | 3 +-- cmd/delete/cluster.go | 3 +-- cmd/get/kubeconfig.go | 3 +-- cmd/root.go | 2 +- cmd/validate/dependencies.go | 3 +-- 5 files changed, 5 insertions(+), 9 deletions(-) diff --git a/cmd/apply.go b/cmd/apply.go index 2f2541c79..1e1df5369 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -295,8 +295,7 @@ func getSkipsClusterCmdFlags() ClusterSkipsCmdFlags { } } -func getApplyCmdFlags() (ClusterCmdFlags, error) { - var err error +func getApplyCmdFlags() (_ ClusterCmdFlags, err error) { skips := getSkipsClusterCmdFlags() diff --git a/cmd/delete/cluster.go b/cmd/delete/cluster.go index 38cfd9f62..9efa3063e 100644 --- a/cmd/delete/cluster.go +++ b/cmd/delete/cluster.go @@ -378,8 +378,7 @@ func NewClusterCmd() *cobra.Command { return clusterCmd } -func getDeleteClusterCmdFlags() (ClusterCmdFlags, error) { - var err error +func getDeleteClusterCmdFlags() (_ ClusterCmdFlags, err error) { // The binPath path must be calculated here because when we launch the tools // we sometimes change the working directory where the binary is launched diff --git a/cmd/get/kubeconfig.go b/cmd/get/kubeconfig.go index 1c1bad500..bd85f7012 100644 --- a/cmd/get/kubeconfig.go +++ b/cmd/get/kubeconfig.go @@ -44,8 +44,7 @@ func NewKubeconfigCmd() *cobra.Command { logrus.Fatalf("error while binding flags: %v", err) } }, - RunE: func(_ *cobra.Command, _ []string) error { - var err error + RunE: func(_ *cobra.Command, _ []string) (err error) { ctn := app.GetContainerInstance() tracker := ctn.Tracker() diff --git a/cmd/root.go b/cmd/root.go index bde86e4b2..eacf2bcf3 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -155,7 +155,7 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP logrus.Debugf("Writing logs to %s", logPath) - // Deprected flags. + // Deprecated flags. https := viper.GetBool("https") if !https { logrus.Warn("The --https flag is deprecated and https is the default, if you want to use SSH protocol to download repositories use --git-protocol ssh") diff --git a/cmd/validate/dependencies.go b/cmd/validate/dependencies.go index ad2235ff1..968e5415d 100644 --- a/cmd/validate/dependencies.go +++ b/cmd/validate/dependencies.go @@ -40,8 +40,7 @@ func NewDependenciesCmd() *cobra.Command { logrus.Fatalf("error while binding flags: %v", err) } }, - RunE: func(_ *cobra.Command, _ []string) error { - var err error + RunE: func(_ *cobra.Command, _ []string) (err error) { ctn := app.GetContainerInstance() tracker := ctn.Tracker() From f7f9508dc9ed062ea9626a13416e7940dfe75fc8 Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Fri, 30 May 2025 19:32:29 +0200 Subject: [PATCH 10/14] chore: improve flags help messages --- cmd/root.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index eacf2bcf3..6bc9ab06e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -172,7 +172,7 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP "debug", "D", false, - "Enables furyctl debug output. This will greatly increase the verbosity. Notice that you can always access the debug output and even more logs in the log file", + "Enables furyctl debug output. This will greatly increase the verbosity. Debug logs and additional logs are also always written to the log file. See the --log flag.", ) rootCmd.PersistentFlags().BoolVarP( @@ -204,7 +204,7 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP "outdir", "o", "", - "Path where to create the \".furyctl\" data directory. Default is the user's home", + "Path where to create the \".furyctl\" data directory. Default is the user's home. Path is relative to --workdir", ) rootCmd.PersistentFlags().StringVarP( @@ -212,7 +212,7 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP "log", "l", "", - "Path to a file or folder where to write logs to. Set to 'stdout' write to standard output. Target path will be created if it does not exists. Default is '/.furyctl/furyctl.-.log'", + "Path to a file or folder where to write logs to. Set to 'stdout' write to standard output. Target path will be created if it does not exists. Path is relative to --workdir. Default is '/.furyctl/furyctl.-.log'", ) rootCmd.PersistentFlags().VarP( From d4ed233043998146a913a7cd4377a5cf9b7744dc Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Tue, 3 Jun 2025 10:01:05 +0200 Subject: [PATCH 11/14] Revert "fix: apply suggestions from code review" This reverts commit a9be1dfd929ed14a0f91a16044ed6a5e7cfad273. The change did not pass the linting checks. --- cmd/apply.go | 3 ++- cmd/delete/cluster.go | 3 ++- cmd/get/kubeconfig.go | 3 ++- cmd/root.go | 2 +- cmd/validate/dependencies.go | 3 ++- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cmd/apply.go b/cmd/apply.go index 1e1df5369..2f2541c79 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -295,7 +295,8 @@ func getSkipsClusterCmdFlags() ClusterSkipsCmdFlags { } } -func getApplyCmdFlags() (_ ClusterCmdFlags, err error) { +func getApplyCmdFlags() (ClusterCmdFlags, error) { + var err error skips := getSkipsClusterCmdFlags() diff --git a/cmd/delete/cluster.go b/cmd/delete/cluster.go index 9efa3063e..38cfd9f62 100644 --- a/cmd/delete/cluster.go +++ b/cmd/delete/cluster.go @@ -378,7 +378,8 @@ func NewClusterCmd() *cobra.Command { return clusterCmd } -func getDeleteClusterCmdFlags() (_ ClusterCmdFlags, err error) { +func getDeleteClusterCmdFlags() (ClusterCmdFlags, error) { + var err error // The binPath path must be calculated here because when we launch the tools // we sometimes change the working directory where the binary is launched diff --git a/cmd/get/kubeconfig.go b/cmd/get/kubeconfig.go index bd85f7012..1c1bad500 100644 --- a/cmd/get/kubeconfig.go +++ b/cmd/get/kubeconfig.go @@ -44,7 +44,8 @@ func NewKubeconfigCmd() *cobra.Command { logrus.Fatalf("error while binding flags: %v", err) } }, - RunE: func(_ *cobra.Command, _ []string) (err error) { + RunE: func(_ *cobra.Command, _ []string) error { + var err error ctn := app.GetContainerInstance() tracker := ctn.Tracker() diff --git a/cmd/root.go b/cmd/root.go index 6bc9ab06e..c4af14e94 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -155,7 +155,7 @@ furyctl is a command line interface tool to manage the full lifecycle of SIGHUP logrus.Debugf("Writing logs to %s", logPath) - // Deprecated flags. + // Deprected flags. https := viper.GetBool("https") if !https { logrus.Warn("The --https flag is deprecated and https is the default, if you want to use SSH protocol to download repositories use --git-protocol ssh") diff --git a/cmd/validate/dependencies.go b/cmd/validate/dependencies.go index 968e5415d..ad2235ff1 100644 --- a/cmd/validate/dependencies.go +++ b/cmd/validate/dependencies.go @@ -40,7 +40,8 @@ func NewDependenciesCmd() *cobra.Command { logrus.Fatalf("error while binding flags: %v", err) } }, - RunE: func(_ *cobra.Command, _ []string) (err error) { + RunE: func(_ *cobra.Command, _ []string) error { + var err error ctn := app.GetContainerInstance() tracker := ctn.Tracker() From 42ae75318734410b24a533f8aea34a346dc70697 Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Tue, 3 Jun 2025 10:17:21 +0200 Subject: [PATCH 12/14] chore: more idiomatic code Co-authored-by: Manuel Romei --- cmd/delete/cluster.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/delete/cluster.go b/cmd/delete/cluster.go index 38cfd9f62..269372a10 100644 --- a/cmd/delete/cluster.go +++ b/cmd/delete/cluster.go @@ -409,8 +409,7 @@ func getDeleteClusterCmdFlags() (ClusterCmdFlags, error) { } phase := viper.GetString("phase") - err = cluster.CheckPhase(phase) - if err != nil { //nolint: wsl // conflicts with gofumpt + if err = cluster.CheckPhase(phase); err != nil { return ClusterCmdFlags{}, fmt.Errorf("%w: %s: %s", ErrParsingFlag, "phase", err.Error()) } From d5b9f77fba52e9cff6f8b0cacca2e78567df66cb Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Tue, 3 Jun 2025 12:51:49 +0200 Subject: [PATCH 13/14] fix: check for empty --config flag --- cmd/apply.go | 10 ++++++++-- cmd/delete/cluster.go | 10 ++++++++-- cmd/dump/template.go | 10 ++++++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/cmd/apply.go b/cmd/apply.go index 2f2541c79..46a22d6e6 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -322,9 +322,15 @@ func getApplyCmdFlags() (ClusterCmdFlags, error) { } } - furyctlPath, err := filepath.Abs(viper.GetString("config")) + furyctlPath := viper.GetString("config") + + if furyctlPath == "" { + return ClusterCmdFlags{}, fmt.Errorf("%w --config: cannot be an empty string", ErrParsingFlag) + } + + furyctlPath, err = filepath.Abs(furyctlPath) if err != nil { - return ClusterCmdFlags{}, fmt.Errorf("error while calculating furyctl configuration file absolute path: %w", err) + return ClusterCmdFlags{}, fmt.Errorf("error while getting configuration file absolute path: %w", err) } phase := viper.GetString("phase") diff --git a/cmd/delete/cluster.go b/cmd/delete/cluster.go index 269372a10..d4ee34e3c 100644 --- a/cmd/delete/cluster.go +++ b/cmd/delete/cluster.go @@ -403,9 +403,15 @@ func getDeleteClusterCmdFlags() (ClusterCmdFlags, error) { } } - furyctlPath, err := filepath.Abs(viper.GetString("config")) + furyctlPath := viper.GetString("config") + + if furyctlPath == "" { + return ClusterCmdFlags{}, fmt.Errorf("%w --config: cannot be an empty string", ErrParsingFlag) + } + + furyctlPath, err = filepath.Abs(furyctlPath) if err != nil { - return ClusterCmdFlags{}, fmt.Errorf("error while initializing cluster creation: %w", err) + return ClusterCmdFlags{}, fmt.Errorf("error while getting configuration file absolute path: %w", err) } phase := viper.GetString("phase") diff --git a/cmd/dump/template.go b/cmd/dump/template.go index 7502091ba..3c0c71f33 100644 --- a/cmd/dump/template.go +++ b/cmd/dump/template.go @@ -226,9 +226,15 @@ func getDumpTemplateCmdFlags() (TemplateCmdFlags, error) { return TemplateCmdFlags{}, fmt.Errorf("%w: %w", ErrParsingFlag, err) } - furyctlPath, err := filepath.Abs(viper.GetString("config")) + furyctlPath := viper.GetString("config") + + if furyctlPath == "" { + return TemplateCmdFlags{}, fmt.Errorf("%w --config: cannot be an empty string", ErrParsingFlag) + } + + furyctlPath, err = filepath.Abs(furyctlPath) if err != nil { - return TemplateCmdFlags{}, fmt.Errorf("error: %w", err) + return TemplateCmdFlags{}, fmt.Errorf("error while getting configuration file absolute path: %w", err) } distroPatchesLocation := viper.GetString("distro-patches") From e1214b2d910201258ea39ffd3142c44f757c284d Mon Sep 17 00:00:00 2001 From: Ramiro Algozino Date: Tue, 3 Jun 2025 12:53:52 +0200 Subject: [PATCH 14/14] fix: error out when both phase and post-apply-phase are set Error out when phase and post-apply-phases are set, post-apply-phases will be ignored when phase is set. We should probably support both flags simultaneously in the future, but now at least we error out instead of ignoring the post-apply-phases flag. Relates: #602 --- cmd/apply.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/apply.go b/cmd/apply.go index 46a22d6e6..a6bc823cd 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -384,6 +384,10 @@ func getApplyCmdFlags() (ClusterCmdFlags, error) { postApplyPhases := viper.GetStringSlice("post-apply-phases") + if phase != cluster.OperationPhaseAll && len(postApplyPhases) > 0 { + return ClusterCmdFlags{}, fmt.Errorf("%w: phase and post-apply-phases cannot be used at the same time", ErrParsingFlag) + } + if err := validatePostApplyPhasesFlag(postApplyPhases); err != nil { return ClusterCmdFlags{}, fmt.Errorf("%w: %s %w", ErrParsingFlag, "post-apply-phases", err) }