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

chore: ensure we return zeroed value when returning errors #2988

Merged
merged 2 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 5 additions & 2 deletions src/cmd/tools/helm/load_plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,11 +318,14 @@ func loadFile(path string) (*pluginCommand, error) {
cmds := new(pluginCommand)
b, err := os.ReadFile(path)
if err != nil {
return cmds, fmt.Errorf("file (%s) not provided by plugin. No plugin auto-completion possible", path)
return &pluginCommand{}, fmt.Errorf("file (%s) not provided by plugin. No plugin auto-completion possible", path)
mkcp marked this conversation as resolved.
Show resolved Hide resolved
}

err = yaml.Unmarshal(b, cmds)
return cmds, err
if err != nil {
return &pluginCommand{}, err
}
return cmds, nil
}

// pluginDynamicComp call the plugin.complete script of the plugin (if available)
Expand Down
3 changes: 3 additions & 0 deletions src/pkg/cluster/pvc.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import (
)

// UpdateGiteaPVC updates the existing Gitea persistent volume claim and tells Gitea whether to create or not.
// REVIEW(mkcp): It looks like we've got some error+signal coming back from these functions where we return a both a
mkcp marked this conversation as resolved.
Show resolved Hide resolved
// string true/false downstream but sometimes with errors. So I'm not going to make these `return "", err` but we may
// want to consider if returning the error is necessary and not, for example, better served with a new type.
func (c *Cluster) UpdateGiteaPVC(ctx context.Context, pvcName string, shouldRollBack bool) (string, error) {
if shouldRollBack {
pvc, err := c.Clientset.CoreV1().PersistentVolumeClaims(ZarfNamespaceName).Get(ctx, pvcName, metav1.GetOptions{})
Expand Down
7 changes: 4 additions & 3 deletions src/pkg/cluster/tunnel.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ func (c *Cluster) ListConnections(ctx context.Context) (types.ConnectStrings, er

// NewTargetTunnelInfo returns a new TunnelInfo object for the specified target.
func (c *Cluster) NewTargetTunnelInfo(ctx context.Context, target string) (TunnelInfo, error) {
var err error
zt := TunnelInfo{
Namespace: ZarfNamespaceName,
ResourceType: SvcResource,
Expand All @@ -102,9 +101,11 @@ func (c *Cluster) NewTargetTunnelInfo(ctx context.Context, target string) (Tunne
zt.RemotePort = ZarfInjectorPort
default:
if target != "" {
if zt, err = c.checkForZarfConnectLabel(ctx, target); err != nil {
ztNew, err := c.checkForZarfConnectLabel(ctx, target)
if err != nil {
return TunnelInfo{}, fmt.Errorf("problem looking for a zarf connect label in the cluster: %s", err.Error())
}
zt = ztNew
}
if zt.ResourceName == "" {
return TunnelInfo{}, fmt.Errorf("missing resource name")
Expand All @@ -113,7 +114,7 @@ func (c *Cluster) NewTargetTunnelInfo(ctx context.Context, target string) (Tunne
return TunnelInfo{}, fmt.Errorf("missing remote port")
}
}
return zt, err
return zt, nil
}

// Connect will establish a tunnel to the specified target.
Expand Down
17 changes: 12 additions & 5 deletions src/pkg/cluster/zarf.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ func (c *Cluster) GetDeployedZarfPackages(ctx context.Context) ([]types.Deployed
listOpts := metav1.ListOptions{LabelSelector: ZarfPackageInfoLabel}
secrets, err := c.Clientset.CoreV1().Secrets(ZarfNamespaceName).List(ctx, listOpts)
if err != nil {
return nil, err
return []types.DeployedPackage{}, err
mkcp marked this conversation as resolved.
Show resolved Hide resolved
}

errs := []error{}
deployedPackages := []types.DeployedPackage{}
errs := make([]error, 0)
deployedPackages := make([]types.DeployedPackage, 0)
mkcp marked this conversation as resolved.
Show resolved Hide resolved
for _, secret := range secrets.Items {
if !strings.HasPrefix(secret.Name, config.ZarfPackagePrefix) {
continue
Expand All @@ -52,7 +52,11 @@ func (c *Cluster) GetDeployedZarfPackages(ctx context.Context) ([]types.Deployed
deployedPackages = append(deployedPackages, deployedPackage)
}

return deployedPackages, errors.Join(errs...)
// REVIEW(mkcp): This seems like it could be a breaking behavior change. Can I get a second set of eyes here?
mkcp marked this conversation as resolved.
Show resolved Hide resolved
if 0 < len(errs) {
mkcp marked this conversation as resolved.
Show resolved Hide resolved
return []types.DeployedPackage{}, errors.Join(errs...)
}
return deployedPackages, nil
}

// GetDeployedPackage gets the metadata information about the package name provided (if it exists in the cluster).
Expand Down Expand Up @@ -325,7 +329,10 @@ func (c *Cluster) UpdateInternalArtifactServerToken(ctx context.Context, oldGitS
}
return nil
})
return newToken, err
if err != nil {
return "", err
}
return newToken, nil
}

// UpdateInternalGitServerSecret updates the internal gitea server secrets with the new git server info
Expand Down
10 changes: 5 additions & 5 deletions src/pkg/transform/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,16 @@ func GenTransformURL(targetBaseURL string, sourceURL string) (*url.URL, error) {
// Rebuild the generic URL
transformedURL := fmt.Sprintf("%s/generic/%s/%s/%s", targetBaseURL, packageNameGlobal, version, fileName)

url, err := url.Parse(transformedURL)
parsedUrl, err := url.Parse(transformedURL)
if err != nil {
return url, err
return &url.URL{}, err
}

// Drop the RawQuery and Fragment to avoid them being interpreted for generic packages
url.RawQuery = ""
url.Fragment = ""
parsedUrl.RawQuery = ""
parsedUrl.Fragment = ""

return url, err
return parsedUrl, nil
}

// transformRegistryPath transforms a given request path using a new base URL and regex.
Expand Down
9 changes: 6 additions & 3 deletions src/pkg/utils/io.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,18 @@ func GetFinalExecutablePath() (string, error) {

// In case the binary is symlinked somewhere else, get the final destination
linkedPath, err := filepath.EvalSymlinks(binaryPath)
return linkedPath, err
if err != nil {
return "", err
}
return linkedPath, nil
}

// GetFinalExecutableCommand returns the final path to the Zarf executable including and library prefixes and overrides.
func GetFinalExecutableCommand() (string, error) {
// In case the binary is symlinked somewhere else, get the final destination
zarfCommand, err := GetFinalExecutablePath()
if err != nil {
return zarfCommand, err
return "", err
}

if config.ActionsCommandZarfPrefix != "" {
Expand All @@ -60,5 +63,5 @@ func GetFinalExecutableCommand() (string, error) {
zarfCommand = "zarf"
}

return zarfCommand, err
return zarfCommand, nil
}
6 changes: 3 additions & 3 deletions src/pkg/utils/yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,12 @@ func SplitYAML(yamlData []byte) ([]*unstructured.Unstructured, error) {
var objs []*unstructured.Unstructured
ymls, err := SplitYAMLToString(yamlData)
if err != nil {
return nil, err
return []*unstructured.Unstructured{}, err
}
for _, yml := range ymls {
u := &unstructured.Unstructured{}
if err := k8syaml.Unmarshal([]byte(yml), u); err != nil {
return objs, fmt.Errorf("failed to unmarshal manifest: %w", err)
return []*unstructured.Unstructured{}, fmt.Errorf("failed to unmarshal manifest: %w", err)
}
objs = append(objs, u)
}
Expand All @@ -220,7 +220,7 @@ func SplitYAMLToString(yamlData []byte) ([]string, error) {
if errors.Is(err, io.EOF) {
break
}
return objs, fmt.Errorf("failed to unmarshal manifest: %w", err)
return []string{}, fmt.Errorf("failed to unmarshal manifest: %w", err)
}
ext.Raw = bytes.TrimSpace(ext.Raw)
if len(ext.Raw) == 0 || bytes.Equal(ext.Raw, []byte("null")) {
Expand Down
5 changes: 4 additions & 1 deletion src/pkg/zoci/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ func (r *Remote) PullPackage(ctx context.Context, destinationDir string, concurr
err = r.CopyToTarget(ctx, layersToPull, dst, copyOpts)
doneSaving <- err
<-doneSaving
return layersToPull, err
if err != nil {
return nil, err
}
return layersToPull, nil
}

// LayersFromRequestedComponents returns the descriptors for the given components from the root manifest.
Expand Down
Loading