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

fix: replace debug logs with returning errors #2719

Merged
merged 1 commit into from
Jul 22, 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
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:[email protected]/
t.Parallel()

data := `https://wayne:[email protected]/
bad line
<really bad="line"/>
https://wayne:p%40ss%20word%[email protected]
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.

Loading