Skip to content

Commit

Permalink
fix: replace debug logs with returning errors (#2719)
Browse files Browse the repository at this point in the history
Signed-off-by: Philip Laine <philip.laine@gmail.com>
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
  • Loading branch information
phillebaba authored and AustinAbro321 committed Jul 23, 2024
1 parent 26ce9b6 commit 8f1edc1
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 88 deletions.
2 changes: 1 addition & 1 deletion src/cmd/destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ var destroyCmd = &cobra.Command{
// Don't remove scripts we can't execute so the user can try to manually run
continue
} else if err != nil {
message.Debugf("Received error when trying to execute the script (%s): %#v", script, err)
return fmt.Errorf("received an error when executing the script %s: %w", script, err)
}

// Try to remove the script, but ignore any errors
Expand Down
4 changes: 3 additions & 1 deletion src/internal/agent/http/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ func proxyResponseTransform(resp *http.Response) error {
message.Debugf("Before Resp Location %#v", resp.Header.Get("Location"))

locationURL, err := url.Parse(resp.Header.Get("Location"))
message.Debugf("%#v", err)
if err != nil {
return err
}
locationURL.Path = transform.NoTransform + locationURL.Path
locationURL.Host = resp.Request.Header.Get("X-Forwarded-Host")

Expand Down
5 changes: 4 additions & 1 deletion src/internal/packager/git/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ func (g *Git) clone(ctx context.Context, gitURL string, ref plumbing.ReferenceNa
}

// Setup git credentials if we have them, ignore if we don't.
gitCred := utils.FindAuthForHost(gitURL)
gitCred, err := utils.FindAuthForHost(gitURL)
if err != nil {
return err
}
if gitCred != nil {
cloneOptions.Auth = &gitCred.Auth
}
Expand Down
6 changes: 5 additions & 1 deletion src/internal/packager/helm/post-render.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,10 @@ func (r *renderer) editHelmResources(ctx context.Context, resources []releaseuti
return err
}
resource, err := dc.Resource(mapping.Resource).Namespace(deployedNamespace).Get(ctx, rawData.GetName(), metav1.GetOptions{})
// Ignore resources that are yet to be created
if kerrors.IsNotFound(err) {
return nil
}
if err != nil {
return err
}
Expand All @@ -308,7 +312,7 @@ func (r *renderer) editHelmResources(ctx context.Context, resources []releaseuti
return nil
}()
if err != nil {
message.Debugf("Unable to adopt resource %s: %s", rawData.GetName(), err.Error())
return fmt.Errorf("unable to adopt the resource %s: %w", rawData.GetName(), err)
}
}
// Finally place this back onto the output buffer
Expand Down
5 changes: 4 additions & 1 deletion src/pkg/packager/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,10 @@ func (p *Packager) attemptClusterChecks(ctx context.Context) (err error) {
// Check for any breaking changes between the initialized Zarf version and this CLI
if existingInitPackage, _ := p.cluster.GetDeployedPackage(ctx, "init"); existingInitPackage != nil {
// Use the build version instead of the metadata since this will support older Zarf versions
deprecated.PrintBreakingChanges(os.Stderr, existingInitPackage.Data.Build.Version, config.CLIVersion)
err := deprecated.PrintBreakingChanges(os.Stderr, existingInitPackage.Data.Build.Version, config.CLIVersion)
if err != nil {
return err
}
}

spinner.Success()
Expand Down
13 changes: 9 additions & 4 deletions src/pkg/packager/deprecated/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package deprecated

import (
"errors"
"fmt"
"io"
"strings"
Expand Down Expand Up @@ -78,11 +79,14 @@ func MigrateComponent(build types.ZarfBuildData, component types.ZarfComponent)
}

// PrintBreakingChanges prints the breaking changes between the provided version and the current CLIVersion.
func PrintBreakingChanges(w io.Writer, deployedZarfVersion, cliVersion string) {
func PrintBreakingChanges(w io.Writer, deployedZarfVersion, cliVersion string) error {
deployedSemver, err := semver.NewVersion(deployedZarfVersion)
// Dev versions of Zarf are not semver.
if errors.Is(err, semver.ErrInvalidSemVer) {
return nil
}
if err != nil {
message.Debugf("Unable to check for breaking changes between Zarf versions")
return
return fmt.Errorf("unable to check for breaking changes between Zarf versions: %w", err)
}

// List of breaking changes to warn the user of.
Expand All @@ -103,7 +107,7 @@ func PrintBreakingChanges(w io.Writer, deployedZarfVersion, cliVersion string) {
}

if len(applicableBreakingChanges) == 0 {
return
return nil
}

// Print header information
Expand All @@ -123,4 +127,5 @@ func PrintBreakingChanges(w io.Writer, deployedZarfVersion, cliVersion string) {
}

message.HorizontalRule()
return nil
}
3 changes: 2 additions & 1 deletion src/pkg/packager/deprecated/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ func TestPrintBreakingChanges(t *testing.T) {
t.Parallel()
var output bytes.Buffer
message.InitializePTerm(&output)
PrintBreakingChanges(&output, tt.deployedVersion, tt.cliVersion)
err := PrintBreakingChanges(&output, tt.deployedVersion, tt.cliVersion)
require.NoError(t, err)
for _, bc := range tt.breakingChanges {
require.Contains(t, output.String(), bc.String())
}
Expand Down
73 changes: 34 additions & 39 deletions src/pkg/utils/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ package utils

import (
"bufio"
"io"
"errors"
"net/url"
"os"
"path/filepath"
"strings"

"github.com/go-git/go-git/v5/plumbing/transport/http"
"github.com/zarf-dev/zarf/src/pkg/message"
)

// Credential represents authentication for a given host.
Expand All @@ -23,47 +22,47 @@ type Credential struct {
}

// FindAuthForHost finds the authentication scheme for a given host using .git-credentials then .netrc.
func FindAuthForHost(baseURL string) *Credential {
func FindAuthForHost(baseURL string) (*Credential, error) {
homePath, _ := os.UserHomeDir()

// Read the ~/.git-credentials file
credentialsPath := filepath.Join(homePath, ".git-credentials")
// Dogsled the error since we are ok if this file doesn't exist (error message debugged on close)
credentialsFile, _ := os.Open(credentialsPath)
gitCreds := credentialParser(credentialsFile)
gitCreds, err := credentialParser(credentialsPath)
if err != nil {
return nil, err
}

// Read the ~/.netrc file
netrcPath := filepath.Join(homePath, ".netrc")
// Dogsled the error since we are ok if this file doesn't exist (error message debugged on close)
netrcFile, _ := os.Open(netrcPath)
netrcCreds := netrcParser(netrcFile)
netrcCreds, err := netrcParser(netrcPath)
if err != nil {
return nil, err
}

// Combine the creds together (.netrc second because it could have a default)
creds := append(gitCreds, netrcCreds...)

// Look for a match for the given host path in the creds file
for _, cred := range creds {
// An empty credPath means that we have reached the default from the .netrc
hasPath := strings.Contains(baseURL, cred.Path) || cred.Path == ""
if hasPath {
return &cred
return &cred, nil
}
}

return nil
return nil, nil
}

// credentialParser parses a user's .git-credentials file to find git creds for hosts.
func credentialParser(file io.ReadCloser) []Credential {
var credentials []Credential

defer func(file io.ReadCloser) {
err := file.Close()
if err != nil {
message.Debugf("Unable to load an existing git credentials file: %s", err.Error())
}
}(file)
func credentialParser(path string) ([]Credential, error) {
file, err := os.Open(path)
if errors.Is(err, os.ErrNotExist) {
return nil, nil
}
if err != nil {
return nil, err
}
defer file.Close()

var credentials []Credential
scanner := bufio.NewScanner(file)
for scanner.Scan() {
gitURL, err := url.Parse(scanner.Text())
Expand All @@ -80,27 +79,25 @@ func credentialParser(file io.ReadCloser) []Credential {
}
credentials = append(credentials, credential)
}

return credentials
return credentials, nil
}

// netrcParser parses a user's .netrc file using the method curl did pre 7.84.0: https://daniel.haxx.se/blog/2022/05/31/netrc-pains/.
func netrcParser(file io.ReadCloser) []Credential {
var credentials []Credential

defer func(file io.ReadCloser) {
err := file.Close()
if err != nil {
message.Debugf("Unable to load an existing netrc file: %s", err.Error())
}
}(file)
func netrcParser(path string) ([]Credential, error) {
file, err := os.Open(path)
if errors.Is(err, os.ErrNotExist) {
return nil, nil
}
if err != nil {
return nil, err
}
defer file.Close()

var credentials []Credential
scanner := bufio.NewScanner(file)

activeMacro := false
activeCommand := ""
var activeMachine map[string]string

for scanner.Scan() {
line := scanner.Text()

Expand Down Expand Up @@ -154,13 +151,11 @@ func netrcParser(file io.ReadCloser) []Credential {
}
}
}

// Append the last machine (if exists) at the end of the file
if activeMachine != nil {
credentials = appendNetrcMachine(activeMachine, credentials)
}

return credentials
return credentials, nil
}

func appendNetrcMachine(machine map[string]string, credentials []Credential) []Credential {
Expand Down
34 changes: 19 additions & 15 deletions src/pkg/utils/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,25 @@
package utils

import (
"os"
"path/filepath"
"testing"

"github.com/go-git/go-git/v5/plumbing/transport/http"
"github.com/stretchr/testify/require"
mocks "github.com/zarf-dev/zarf/src/test/mocks"
)

func TestCredentialParser(t *testing.T) {
credentialsFile := &mocks.MockReadCloser{
MockData: []byte(
`https://wayne:password@github.com/
t.Parallel()

data := `https://wayne:password@github.com/
bad line
<really bad="line"/>
https://wayne:p%40ss%20word%2520@zarf.dev
http://google.com`,
),
}
http://google.com`
path := filepath.Join(t.TempDir(), "file")
err := os.WriteFile(path, []byte(data), 0o644)
require.NoError(t, err)

expectedCreds := []Credential{
{
Expand All @@ -47,15 +49,15 @@ http://google.com`,
},
}

gitCredentials := credentialParser(credentialsFile)
gitCredentials, err := credentialParser(path)
require.NoError(t, err)
require.Equal(t, expectedCreds, gitCredentials)
}

func TestNetRCParser(t *testing.T) {
t.Parallel()

netrcFile := &mocks.MockReadCloser{
MockData: []byte(
`# top of file comment
data := `# top of file comment
machine github.com
login wayne
password password
Expand All @@ -70,9 +72,10 @@ machine google.com #comment password fun and login info!
default
login anonymous
password password`,
),
}
password password`
path := filepath.Join(t.TempDir(), "file")
err := os.WriteFile(path, []byte(data), 0o644)
require.NoError(t, err)

expectedCreds := []Credential{
{
Expand Down Expand Up @@ -105,6 +108,7 @@ default
},
}

netrcCredentials := netrcParser(netrcFile)
netrcCredentials, err := netrcParser(path)
require.NoError(t, err)
require.Equal(t, expectedCreds, netrcCredentials)
}
24 changes: 0 additions & 24 deletions src/test/mocks/read_closer_mock.go

This file was deleted.

0 comments on commit 8f1edc1

Please sign in to comment.