Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: replace warning logs with returning errors #2762

Merged
merged 1 commit into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/config/lang/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 3 additions & 7 deletions src/internal/agent/hooks/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand Down
11 changes: 2 additions & 9 deletions src/internal/agent/http/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
7 changes: 1 addition & 6 deletions src/internal/packager/git/checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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{
Expand Down
8 changes: 2 additions & 6 deletions src/internal/packager/git/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
}

Expand Down
1 change: 0 additions & 1 deletion src/pkg/packager/actions/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Loading