From f88aca59b17373fd4cfcffeefc881f636e09f272 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Thu, 25 Jul 2024 13:20:50 +0200 Subject: [PATCH] refactor: replace warning logs with returning errors Signed-off-by: Philip Laine --- src/config/lang/english.go | 1 - src/internal/agent/hooks/pods.go | 10 +++------- src/internal/agent/http/proxy.go | 11 ++--------- src/internal/packager/git/checkout.go | 7 +------ src/internal/packager/git/push.go | 8 ++------ src/pkg/packager/actions/actions.go | 1 - 6 files changed, 8 insertions(+), 30 deletions(-) diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 33698de7a5..780a50a3bc 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -611,7 +611,6 @@ const ( AgentErrGetState = "failed to load zarf state: %w" AgentErrParsePod = "failed to parse pod: %w" AgentErrHostnameMatch = "failed to complete hostname matching: %w" - AgentErrImageSwap = "Unable to swap the host for (%s)" AgentErrInvalidMethod = "invalid method only POST requests are allowed" AgentErrInvalidOp = "invalid operation: %s" AgentErrInvalidType = "only content type 'application/json' is supported" diff --git a/src/internal/agent/hooks/pods.go b/src/internal/agent/hooks/pods.go index 60362785a5..7d7a6a8023 100644 --- a/src/internal/agent/hooks/pods.go +++ b/src/internal/agent/hooks/pods.go @@ -13,7 +13,6 @@ import ( "github.com/zarf-dev/zarf/src/config/lang" "github.com/zarf-dev/zarf/src/internal/agent/operations" "github.com/zarf-dev/zarf/src/pkg/cluster" - "github.com/zarf-dev/zarf/src/pkg/message" "github.com/zarf-dev/zarf/src/pkg/transform" v1 "k8s.io/api/admission/v1" @@ -82,8 +81,7 @@ func mutatePod(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Clu path := fmt.Sprintf("/spec/initContainers/%d/image", idx) replacement, err := transform.ImageTransformHost(registryURL, container.Image) if err != nil { - message.Warnf(lang.AgentErrImageSwap, container.Image) - continue // Continue, because we might as well attempt to mutate the other containers for this pod + return nil, err } updatedAnnotations[getImageAnnotationKey(container.Name)] = container.Image patches = append(patches, operations.ReplacePatchOperation(path, replacement)) @@ -94,8 +92,7 @@ func mutatePod(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Clu path := fmt.Sprintf("/spec/ephemeralContainers/%d/image", idx) replacement, err := transform.ImageTransformHost(registryURL, container.Image) if err != nil { - message.Warnf(lang.AgentErrImageSwap, container.Image) - continue // Continue, because we might as well attempt to mutate the other containers for this pod + return nil, err } updatedAnnotations[getImageAnnotationKey(container.Name)] = container.Image patches = append(patches, operations.ReplacePatchOperation(path, replacement)) @@ -106,8 +103,7 @@ func mutatePod(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Clu path := fmt.Sprintf("/spec/containers/%d/image", idx) replacement, err := transform.ImageTransformHost(registryURL, container.Image) if err != nil { - message.Warnf(lang.AgentErrImageSwap, container.Image) - continue // Continue, because we might as well attempt to mutate the other containers for this pod + return nil, err } updatedAnnotations[getImageAnnotationKey(container.Name)] = container.Image patches = append(patches, operations.ReplacePatchOperation(path, replacement)) diff --git a/src/internal/agent/http/proxy.go b/src/internal/agent/http/proxy.go index 719ef17281..33709dfff7 100644 --- a/src/internal/agent/http/proxy.go +++ b/src/internal/agent/http/proxy.go @@ -146,22 +146,15 @@ func replaceBodyLinks(resp *http.Response) error { forwardedPrefix := fmt.Sprintf("%s%s%s", getTLSScheme(resp.Request.TLS), resp.Request.Header.Get("X-Forwarded-Host"), transform.NoTransform) targetPrefix := fmt.Sprintf("%s%s", getTLSScheme(resp.TLS), resp.Request.Host) - body, err := io.ReadAll(resp.Body) + b, err := io.ReadAll(resp.Body) if err != nil { return err } - err = resp.Body.Close() if err != nil { return err } - - bodyString := string(body) - message.Warnf("%s", bodyString) - - bodyString = strings.ReplaceAll(bodyString, targetPrefix, forwardedPrefix) - - message.Warnf("%s", bodyString) + bodyString := strings.ReplaceAll(string(b), targetPrefix, forwardedPrefix) // Setup the new reader, and correct the content length resp.Body = io.NopCloser(strings.NewReader(bodyString)) diff --git a/src/internal/packager/git/checkout.go b/src/internal/packager/git/checkout.go index 2ee91d96b5..5a9a960508 100644 --- a/src/internal/packager/git/checkout.go +++ b/src/internal/packager/git/checkout.go @@ -10,7 +10,6 @@ import ( "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" - "github.com/zarf-dev/zarf/src/pkg/message" ) // CheckoutTag performs a `git checkout` of the provided tag to a detached HEAD. @@ -54,7 +53,6 @@ func (g *Git) checkoutHashAsBranch(hash plumbing.Hash, branch plumbing.Reference if err != nil { return fmt.Errorf("not a valid git repo or unable to open: %w", err) } - objRef, err := repo.Object(plumbing.AnyObject, hash) if err != nil { return fmt.Errorf("an error occurred when getting the repo's object reference: %w", err) @@ -67,10 +65,7 @@ func (g *Git) checkoutHashAsBranch(hash plumbing.Hash, branch plumbing.Reference case *object.Commit: commitHash = objRef.Hash default: - // This shouldn't ever hit, but we should at least log it if someday it - // does get hit - message.Warnf("Checkout failed. Hash type %s not supported.", objRef.Type().String()) - return err + return fmt.Errorf("hash type %s not supported", objRef.Type().String()) } options := &git.CheckoutOptions{ diff --git a/src/internal/packager/git/push.go b/src/internal/packager/git/push.go index 494ee40db6..98236d2316 100644 --- a/src/internal/packager/git/push.go +++ b/src/internal/packager/git/push.go @@ -45,8 +45,7 @@ func (g *Git) PushRepo(srcURL, targetFolder string) error { repo, err := g.prepRepoForPush() if err != nil { - message.Warnf("error when prepping the repo for push.. %v", err) - return err + return fmt.Errorf("could not prepare the repo for push: %w", err) } if err := g.push(repo, spinner); err != nil { @@ -64,14 +63,11 @@ func (g *Git) PushRepo(srcURL, targetFolder string) error { remoteURL := remote.Config().URLs[0] repoName, err := transform.GitURLtoRepoName(remoteURL) if err != nil { - message.Warnf("Unable to add the read-only user to the repo: %s\n", repoName) return err } - err = g.addReadOnlyUserToRepo(g.Server.Address, repoName) if err != nil { - message.Warnf("Unable to add the read-only user to the repo: %s\n", repoName) - return err + return fmt.Errorf("unable to add the read only user to the repo %s: %w", repoName, err) } } diff --git a/src/pkg/packager/actions/actions.go b/src/pkg/packager/actions/actions.go index 541c73b0fe..250e7bf100 100644 --- a/src/pkg/packager/actions/actions.go +++ b/src/pkg/packager/actions/actions.go @@ -110,7 +110,6 @@ retryCmd: for _, v := range action.SetVariables { variableConfig.SetVariable(v.Name, out, v.Sensitive, v.AutoIndent, v.Type) if err := variableConfig.CheckVariablePattern(v.Name, v.Pattern); err != nil { - message.WarnErr(err, err.Error()) return err } }