From 9c3532c1d859914684c284162add8a4d1b8312d5 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Fri, 28 Jun 2024 11:18:42 +0200 Subject: [PATCH] refactor: remove all use of message.Fatal --- src/cmd/dev.go | 17 ++++++++++--- src/cmd/initialize.go | 5 +++- src/cmd/package.go | 35 +++++++++++++++++++++------ src/cmd/tools/zarf.go | 5 +++- src/internal/agent/http/server.go | 7 +++--- src/internal/agent/start.go | 6 ++++- src/internal/packager/helm/repo.go | 8 +++--- src/pkg/cluster/state.go | 12 +++++++-- src/pkg/cluster/state_test.go | 4 ++- src/pkg/message/message.go | 19 --------------- src/pkg/message/spinner.go | 15 ------------ src/pkg/packager/common.go | 18 -------------- src/pkg/packager/deploy.go | 2 +- src/pkg/packager/deploy_test.go | 3 ++- src/pkg/pki/pki.go | 39 +++++++++++++++--------------- 15 files changed, 97 insertions(+), 98 deletions(-) diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 7560b43651..f6fad80cbe 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -50,7 +50,10 @@ var devDeployCmd = &cobra.Command{ pkgConfig.PkgOpts.SetVariables = helpers.TransformAndMergeMap( v.GetStringMapString(common.VPkgDeploySet), pkgConfig.PkgOpts.SetVariables, strings.ToUpper) - pkgClient := packager.NewOrDie(&pkgConfig) + pkgClient, err := packager.New(&pkgConfig) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() if err := pkgClient.DevDeploy(cmd.Context()); err != nil { @@ -72,10 +75,13 @@ var devGenerateCmd = &cobra.Command{ pkgConfig.CreateOpts.BaseDir = "." pkgConfig.FindImagesOpts.RepoHelmChartPath = pkgConfig.GenerateOpts.GitPath - pkgClient := packager.NewOrDie(&pkgConfig) + pkgClient, err := packager.New(&pkgConfig) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() - err := pkgClient.Generate() + err = pkgClient.Generate() if err != nil { return err } @@ -217,7 +223,10 @@ var devFindImagesCmd = &cobra.Command{ v.GetStringMapString(common.VPkgCreateSet), pkgConfig.CreateOpts.SetVariables, strings.ToUpper) pkgConfig.PkgOpts.SetVariables = helpers.TransformAndMergeMap( v.GetStringMapString(common.VPkgDeploySet), pkgConfig.PkgOpts.SetVariables, strings.ToUpper) - pkgClient := packager.NewOrDie(&pkgConfig) + pkgClient, err := packager.New(&pkgConfig) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() if _, err := pkgClient.FindImages(cmd.Context()); err != nil { diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index 5a680b7836..2262454866 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -63,7 +63,10 @@ var initCmd = &cobra.Command{ pkgConfig.PkgOpts.SetVariables = helpers.TransformAndMergeMap( v.GetStringMapString(common.VPkgDeploySet), pkgConfig.PkgOpts.SetVariables, strings.ToUpper) - pkgClient := packager.NewOrDie(&pkgConfig, packager.WithSource(src)) + pkgClient, err := packager.New(&pkgConfig, packager.WithSource(src)) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() err = pkgClient.Deploy(cmd.Context()) diff --git a/src/cmd/package.go b/src/cmd/package.go index b5b28cff41..fa939e10bb 100644 --- a/src/cmd/package.go +++ b/src/cmd/package.go @@ -54,7 +54,10 @@ var packageCreateCmd = &cobra.Command{ pkgConfig.CreateOpts.SetVariables = helpers.TransformAndMergeMap( v.GetStringMapString(common.VPkgCreateSet), pkgConfig.CreateOpts.SetVariables, strings.ToUpper) - pkgClient := packager.NewOrDie(&pkgConfig) + pkgClient, err := packager.New(&pkgConfig) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() if err := pkgClient.Create(cmd.Context()); err != nil { @@ -81,7 +84,10 @@ var packageDeployCmd = &cobra.Command{ pkgConfig.PkgOpts.SetVariables = helpers.TransformAndMergeMap( v.GetStringMapString(common.VPkgDeploySet), pkgConfig.PkgOpts.SetVariables, strings.ToUpper) - pkgClient := packager.NewOrDie(&pkgConfig) + pkgClient, err := packager.New(&pkgConfig) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() ctx := cmd.Context() @@ -106,7 +112,10 @@ var packageMirrorCmd = &cobra.Command{ return err } pkgConfig.PkgOpts.PackageSource = packageSource - pkgClient := packager.NewOrDie(&pkgConfig) + pkgClient, err := packager.New(&pkgConfig) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() if err := pkgClient.Mirror(cmd.Context()); err != nil { return fmt.Errorf("failed to mirror package: %w", err) @@ -131,7 +140,10 @@ var packageInspectCmd = &cobra.Command{ if err != nil { return err } - pkgClient := packager.NewOrDie(&pkgConfig, packager.WithSource(src)) + pkgClient, err := packager.New(&pkgConfig, packager.WithSource(src)) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() if err := pkgClient.Inspect(cmd.Context()); err != nil { return fmt.Errorf("failed to inspect package: %w", err) @@ -200,7 +212,10 @@ var packageRemoveCmd = &cobra.Command{ if err != nil { return err } - pkgClient := packager.NewOrDie(&pkgConfig, packager.WithSource(src)) + pkgClient, err := packager.New(&pkgConfig, packager.WithSource(src)) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() if err := pkgClient.Remove(cmd.Context()); err != nil { return fmt.Errorf("unable to remove the package with an error of: %w", err) @@ -238,7 +253,10 @@ var packagePublishCmd = &cobra.Command{ pkgConfig.PublishOpts.PackageDestination = ref.String() - pkgClient := packager.NewOrDie(&pkgConfig) + pkgClient, err := packager.New(&pkgConfig) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() if err := pkgClient.Publish(cmd.Context()); err != nil { @@ -255,7 +273,10 @@ var packagePullCmd = &cobra.Command{ Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { pkgConfig.PkgOpts.PackageSource = args[0] - pkgClient := packager.NewOrDie(&pkgConfig) + pkgClient, err := packager.New(&pkgConfig) + if err != nil { + return err + } defer pkgClient.ClearTempPaths() if err := pkgClient.Pull(cmd.Context()); err != nil { return fmt.Errorf("failed to pull package: %w", err) diff --git a/src/cmd/tools/zarf.go b/src/cmd/tools/zarf.go index eafa9cbb47..f911b2ec40 100644 --- a/src/cmd/tools/zarf.go +++ b/src/cmd/tools/zarf.go @@ -233,7 +233,10 @@ var generatePKICmd = &cobra.Command{ Short: lang.CmdToolsGenPkiShort, Args: cobra.ExactArgs(1), RunE: func(_ *cobra.Command, args []string) error { - pki := pki.GeneratePKI(args[0], subAltNames...) + pki, err := pki.GeneratePKI(args[0], subAltNames...) + if err != nil { + return err + } if err := os.WriteFile("tls.ca", pki.CA, helpers.ReadAllWriteUser); err != nil { return err } diff --git a/src/internal/agent/http/server.go b/src/internal/agent/http/server.go index 9bcff6a431..62fb72c59d 100644 --- a/src/internal/agent/http/server.go +++ b/src/internal/agent/http/server.go @@ -18,12 +18,12 @@ import ( ) // NewAdmissionServer creates a http.Server for the mutating webhook admission handler. -func NewAdmissionServer(ctx context.Context, port string) *http.Server { +func NewAdmissionServer(ctx context.Context, port string) (*http.Server, error) { message.Debugf("http.NewAdmissionServer(%s)", port) c, err := cluster.NewCluster() if err != nil { - message.Fatalf(err, err.Error()) + return nil, err } // Routers @@ -46,11 +46,12 @@ func NewAdmissionServer(ctx context.Context, port string) *http.Server { mux.Handle("/mutate/argocd-repository", admissionHandler.Serve(argocdRepositoryMutation)) mux.Handle("/metrics", promhttp.Handler()) - return &http.Server{ + srv := &http.Server{ Addr: fmt.Sprintf(":%s", port), Handler: mux, ReadHeaderTimeout: 5 * time.Second, // Set ReadHeaderTimeout to avoid Slowloris attacks } + return srv, nil } // NewProxyServer creates and returns an http proxy server. diff --git a/src/internal/agent/start.go b/src/internal/agent/start.go index 83172d2e95..f110ce2b52 100644 --- a/src/internal/agent/start.go +++ b/src/internal/agent/start.go @@ -30,7 +30,11 @@ const ( // StartWebhook launches the Zarf agent mutating webhook in the cluster. func StartWebhook(ctx context.Context) error { message.Debug("agent.StartWebhook()") - return startServer(ctx, agentHttp.NewAdmissionServer(ctx, httpPort)) + srv, err := agentHttp.NewAdmissionServer(ctx, httpPort) + if err != nil { + return err + } + return startServer(ctx, srv) } // StartHTTPProxy launches the zarf agent proxy in the cluster. diff --git a/src/internal/packager/helm/repo.go b/src/internal/packager/helm/repo.go index 245625c787..6bc19c8e50 100644 --- a/src/internal/packager/helm/repo.go +++ b/src/internal/packager/helm/repo.go @@ -82,7 +82,7 @@ func (h *Helm) PackageChartFromLocalFiles(cosignKeyPath string) error { var saved string temp := filepath.Join(h.chartPath, "temp") if _, ok := cl.(loader.DirLoader); ok { - err = h.buildChartDependencies(spinner) + err = h.buildChartDependencies() if err != nil { return fmt.Errorf("unable to build dependencies for the chart: %w", err) } @@ -157,7 +157,7 @@ func (h *Helm) DownloadPublishedChart(cosignKeyPath string) error { if registry.IsOCI(h.chart.URL) { regClient, err = registry.NewClient(registry.ClientOptEnableCache(true)) if err != nil { - spinner.Fatalf(err, "Unable to create a new registry client") + return fmt.Errorf("unable to create the new registry client: %w", err) } chartURL = h.chart.URL // Explicitly set the pull version for OCI @@ -279,11 +279,11 @@ func (h *Helm) packageValues(cosignKeyPath string) error { } // buildChartDependencies builds the helm chart dependencies -func (h *Helm) buildChartDependencies(spinner *message.Spinner) error { +func (h *Helm) buildChartDependencies() error { // Download and build the specified dependencies regClient, err := registry.NewClient(registry.ClientOptEnableCache(true)) if err != nil { - spinner.Fatalf(err, "Unable to create a new registry client") + return fmt.Errorf("unable to create a new registry client: %w", err) } h.settings = cli.New() diff --git a/src/pkg/cluster/state.go b/src/pkg/cluster/state.go index d194065125..ad4ff27736 100644 --- a/src/pkg/cluster/state.go +++ b/src/pkg/cluster/state.go @@ -78,7 +78,11 @@ func (c *Cluster) InitZarfState(ctx context.Context, initOptions types.ZarfInitO state.Distro = distro // Setup zarf agent PKI - state.AgentTLS = pki.GeneratePKI(config.ZarfAgentHost) + agentTLS, err := pki.GeneratePKI(config.ZarfAgentHost) + if err != nil { + return err + } + state.AgentTLS = agentTLS namespaceList, err := c.Clientset.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) if err != nil { @@ -359,7 +363,11 @@ func MergeZarfState(oldState *types.ZarfState, initOptions types.ZarfInitOptions } } if slices.Contains(services, message.AgentKey) { - newState.AgentTLS = pki.GeneratePKI(config.ZarfAgentHost) + agentTLS, err := pki.GeneratePKI(config.ZarfAgentHost) + if err != nil { + return nil, err + } + newState.AgentTLS = agentTLS } return &newState, nil diff --git a/src/pkg/cluster/state_test.go b/src/pkg/cluster/state_test.go index 19301432f1..524b0e3295 100644 --- a/src/pkg/cluster/state_test.go +++ b/src/pkg/cluster/state_test.go @@ -468,8 +468,10 @@ func TestMergeZarfStateArtifact(t *testing.T) { func TestMergeZarfStateAgent(t *testing.T) { t.Parallel() + agentTLS, err := pki.GeneratePKI("example.com") + require.NoError(t, err) oldState := &types.ZarfState{ - AgentTLS: pki.GeneratePKI("example.com"), + AgentTLS: agentTLS, } newState, err := MergeZarfState(oldState, types.ZarfInitOptions{}, []string{message.AgentKey}) require.NoError(t, err) diff --git a/src/pkg/message/message.go b/src/pkg/message/message.go index 68bf711219..591165559e 100644 --- a/src/pkg/message/message.go +++ b/src/pkg/message/message.go @@ -10,7 +10,6 @@ import ( "io" "net/http" "os" - "runtime/debug" "strings" "time" @@ -162,20 +161,6 @@ func WarnErrf(err any, format string, a ...any) { Warnf(format, a...) } -// Fatal prints a fatal error message and exits with a 1. -func Fatal(err any, message string) { - debugPrinter(2, err) - errorPrinter(2).Println(message) - debugPrinter(2, string(debug.Stack())) - os.Exit(1) -} - -// Fatalf prints a fatal error message and exits with a 1 with a given format. -func Fatalf(err any, format string, a ...any) { - message := Paragraph(format, a...) - Fatal(err, message) -} - // Info prints an info message. func Info(message string) { Infof("%s", message) @@ -351,7 +336,3 @@ func debugPrinter(offset int, a ...any) { Println(a...) } } - -func errorPrinter(offset int) *pterm.PrefixPrinter { - return pterm.Error.WithShowLineNumber(logLevel > 2).WithLineNumberOffset(offset) -} diff --git a/src/pkg/message/spinner.go b/src/pkg/message/spinner.go index cb2ea49a3a..93511a705c 100644 --- a/src/pkg/message/spinner.go +++ b/src/pkg/message/spinner.go @@ -134,18 +134,3 @@ func (p *Spinner) Errorf(err error, format string, a ...any) { Warnf(format, a...) debugPrinter(2, err) } - -// Fatal calls message.Fatalf with the given error. -func (p *Spinner) Fatal(err error) { - p.Fatalf(err, p.startText) -} - -// Fatalf calls message.Fatalf with the given error and format. -func (p *Spinner) Fatalf(err error, format string, a ...any) { - if p.spinner != nil { - p.spinner.RemoveWhenDone = true - _ = p.spinner.Stop() - activeSpinner = nil - } - Fatalf(err, format, a...) -} diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go index eddce07a53..86c55b3240 100644 --- a/src/pkg/packager/common.go +++ b/src/pkg/packager/common.go @@ -117,24 +117,6 @@ func New(cfg *types.PackagerConfig, mods ...Modifier) (*Packager, error) { return pkgr, nil } -/* -NewOrDie creates a new package instance with the provided config or throws a fatal error. - -Note: This function creates a tmp directory that should be cleaned up with p.ClearTempPaths(). -*/ -func NewOrDie(config *types.PackagerConfig, mods ...Modifier) *Packager { - var ( - err error - pkgr *Packager - ) - - if pkgr, err = New(config, mods...); err != nil { - message.Fatalf(err, "Unable to setup the package config: %s", err.Error()) - } - - return pkgr -} - // setTempDirectory sets the temp directory for the packager. func (p *Packager) setTempDirectory(path string) error { dir, err := utils.MakeTempDir(path) diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index 867706d957..716b672c75 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -472,7 +472,7 @@ func (p *Packager) setupState(ctx context.Context) (err error) { return nil }() if err != nil { - spinner.Fatalf(err, "Unable to create the zarf namespace") + return fmt.Errorf("unable to create the Zarf namespace: %w", err) } } diff --git a/src/pkg/packager/deploy_test.go b/src/pkg/packager/deploy_test.go index 8c69d1682f..5bb21bfcca 100644 --- a/src/pkg/packager/deploy_test.go +++ b/src/pkg/packager/deploy_test.go @@ -212,7 +212,8 @@ func TestGenerateValuesOverrides(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - p := NewOrDie(&types.PackagerConfig{DeployOpts: tt.deployOpts}, WithSource(&sources.TarballSource{})) + p, err := New(&types.PackagerConfig{DeployOpts: tt.deployOpts}, WithSource(&sources.TarballSource{})) + require.NoError(t, err) for k, v := range tt.setVariables { p.variableConfig.SetVariable(k, v, false, false, variables.RawVariableType) } diff --git a/src/pkg/pki/pki.go b/src/pkg/pki/pki.go index 00d1e212a6..6923042393 100644 --- a/src/pkg/pki/pki.go +++ b/src/pkg/pki/pki.go @@ -10,12 +10,12 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "fmt" "math/big" "net" "time" "github.com/defenseunicorns/pkg/helpers/v2" - "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/types" ) @@ -29,49 +29,41 @@ const org = "Zarf Cluster" const validFor = time.Hour * 24 * 375 // GeneratePKI create a CA and signed server keypair. -func GeneratePKI(host string, dnsNames ...string) types.GeneratedPKI { +func GeneratePKI(host string, dnsNames ...string) (types.GeneratedPKI, error) { results := types.GeneratedPKI{} - ca, caKey, err := generateCA(validFor) if err != nil { - message.Fatal(err, "Unable to generate the ephemeral CA") + return types.GeneratedPKI{}, fmt.Errorf("unable to generate the ephemeral CA: %w", err) } - hostCert, hostKey, err := generateCert(host, ca, caKey, validFor, dnsNames...) if err != nil { - message.Fatalf(err, "Unable to generate the cert for %s", host) + return types.GeneratedPKI{}, fmt.Errorf("unable to generate the cert for %s: %w", host, err) } - results.CA = pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: ca.Raw, }) - results.Cert = pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: hostCert.Raw, }) - results.Key = pem.EncodeToMemory(&pem.Block{ Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(hostKey), }) - - return results + return results, nil } // newCertificate creates a new template. -func newCertificate(validFor time.Duration) *x509.Certificate { - notBefore := time.Now() - notAfter := notBefore.Add(validFor) - +func newCertificate(validFor time.Duration) (*x509.Certificate, error) { serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) if err != nil { - message.Fatalf(err, "failed to generate the certificate serial number") + return nil, fmt.Errorf("failed to generate the certificate serial number: %w", err) } - - return &x509.Certificate{ + notBefore := time.Now() + notAfter := notBefore.Add(validFor) + cert := &x509.Certificate{ SerialNumber: serialNumber, Subject: pkix.Name{ Organization: []string{org}, @@ -83,6 +75,7 @@ func newCertificate(validFor time.Duration) *x509.Certificate { ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, BasicConstraintsValid: true, } + return cert, nil } // newPrivateKey creates a new private key. @@ -95,7 +88,10 @@ func newPrivateKey() (*rsa.PrivateKey, error) { // private key should never be saved to disk, but rather used to // immediately generate further certificates. func generateCA(validFor time.Duration) (*x509.Certificate, *rsa.PrivateKey, error) { - template := newCertificate(validFor) + template, err := newCertificate(validFor) + if err != nil { + return nil, nil, err + } template.IsCA = true template.KeyUsage = x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature template.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth} @@ -124,7 +120,10 @@ func generateCA(validFor time.Duration) (*x509.Certificate, *rsa.PrivateKey, err // provided certificate authority. The cert and key files are stored in // the provided files. func generateCert(host string, ca *x509.Certificate, caKey *rsa.PrivateKey, validFor time.Duration, dnsNames ...string) (*x509.Certificate, *rsa.PrivateKey, error) { - template := newCertificate(validFor) + template, err := newCertificate(validFor) + if err != nil { + return nil, nil, err + } template.IPAddresses = append(template.IPAddresses, net.ParseIP(helpers.IPV4Localhost))