refactor(pkg/cluster): simplify PackageSecretNeedsWait and remove unn…

…ecessary named returns

Signed-off-by: Kit Patella <kit@defenseunicorns.com>

src/pkg/cluster/state.go

@@ -193,12 +193,14 @@ func (c *Cluster) InitZarfState(ctx context.Context, initOptions types.ZarfInitO
 }
 
 // LoadZarfState returns the current zarf/zarf-state secret data or an empty ZarfState.
-func (c *Cluster) LoadZarfState(ctx context.Context) (state *types.ZarfState, err error) {
+func (c *Cluster) LoadZarfState(ctx context.Context) (*types.ZarfState, error) {
 	stateErr := errors.New("failed to load the Zarf State from the cluster, has Zarf been initiated?")
 	secret, err := c.Clientset.CoreV1().Secrets(ZarfNamespaceName).Get(ctx, ZarfStateSecretName, metav1.GetOptions{})
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", stateErr, err)
 	}
+
+	state := &types.ZarfState{}
 	err = json.Unmarshal(secret.Data[ZarfStateDataKey], &state)
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", stateErr, err)

src/pkg/cluster/zarf.go

@@ -109,28 +109,29 @@ func (c *Cluster) StripZarfLabelsAndSecretsFromNamespaces(ctx context.Context) {
 	spinner.Success()
 }
 
-// PackageSecretNeedsWait checks if a package component has a running webhook that needs to be waited on.
-func (c *Cluster) PackageSecretNeedsWait(deployedPackage *types.DeployedPackage, component v1alpha1.ZarfComponent, skipWebhooks bool) (needsWait bool, waitSeconds int, hookName string) {
+// PackageSecretNeedsWait checks if a package component has a running webhook that needs to be waited on. Returns the
+// number of seconds remaining to wait and the name of the webhook. If seconds is zero there's no need to wait.
+func (c *Cluster) PackageSecretNeedsWait(deployedPackage *types.DeployedPackage, component v1alpha1.ZarfComponent, skipWebhooks bool) (int, string) {
 	// Skip checking webhook status when '--skip-webhooks' flag is provided and for YOLO packages
 	if skipWebhooks || deployedPackage == nil || deployedPackage.Data.Metadata.YOLO {
-		return false, 0, ""
+		return 0, ""
 	}
 
 	// Look for the specified component
 	hookMap, componentExists := deployedPackage.ComponentWebhooks[component.Name]
 	if !componentExists {
-		return false, 0, "" // Component not found, no need to wait
+		return 0, "" // Component not found, no need to wait
 	}
 
 	// Check if there are any "Running" webhooks for the component that we need to wait for
 	for hookName, webhook := range hookMap {
 		if webhook.Status == types.WebhookStatusRunning {
-			return true, webhook.WaitDurationSeconds, hookName
+			return webhook.WaitDurationSeconds, hookName
 		}
 	}
 
 	// If we get here, the component doesn't need to wait for a webhook to run
-	return false, 0, ""
+	return 0, ""
 }
 
 // RecordPackageDeploymentAndWait records the deployment of a package to the cluster and waits for any webhooks to complete.
@@ -140,9 +141,9 @@ func (c *Cluster) RecordPackageDeploymentAndWait(ctx context.Context, pkg v1alph
 		return nil, err
 	}
 
-	packageNeedsWait, waitSeconds, hookName := c.PackageSecretNeedsWait(deployedPackage, component, skipWebhooks)
+	waitSeconds, hookName := c.PackageSecretNeedsWait(deployedPackage, component, skipWebhooks)
 	// If no webhooks need to complete, we can return immediately.
-	if !packageNeedsWait {
+	if waitSeconds == 0 {
 		return deployedPackage, nil
 	}
 
@@ -160,8 +161,8 @@ func (c *Cluster) RecordPackageDeploymentAndWait(ctx context.Context, pkg v1alph
 		if err != nil {
 			return nil, err
 		}
-		packageNeedsWait, _, _ = c.PackageSecretNeedsWait(deployedPackage, component, skipWebhooks)
-		if !packageNeedsWait {
+		waitSeconds, _ = c.PackageSecretNeedsWait(deployedPackage, component, skipWebhooks)
+		if waitSeconds == 0 {
 			return deployedPackage, nil
 		}
 		return deployedPackage, nil
@@ -174,7 +175,7 @@ func (c *Cluster) RecordPackageDeploymentAndWait(ctx context.Context, pkg v1alph
 }
 
 // RecordPackageDeployment saves metadata about a package that has been deployed to the cluster.
-func (c *Cluster) RecordPackageDeployment(ctx context.Context, pkg v1alpha1.ZarfPackage, components []types.DeployedComponent, generation int) (deployedPackage *types.DeployedPackage, err error) {
+func (c *Cluster) RecordPackageDeployment(ctx context.Context, pkg v1alpha1.ZarfPackage, components []types.DeployedComponent, generation int) (*types.DeployedPackage, error) {
 	packageName := pkg.Metadata.Name
 
 	// Attempt to load information about webhooks for the package
@@ -187,7 +188,7 @@ func (c *Cluster) RecordPackageDeployment(ctx context.Context, pkg v1alpha1.Zarf
 		componentWebhooks = existingPackageSecret.ComponentWebhooks
 	}
 
-	// TODO: This is done for backwards compartibility and could be removed in the future.
+	// TODO: This is done for backwards compatibility and could be removed in the future.
 	connectStrings := types.ConnectStrings{}
 	for _, comp := range components {
 		for _, chart := range comp.InstalledCharts {
@@ -197,7 +198,7 @@ func (c *Cluster) RecordPackageDeployment(ctx context.Context, pkg v1alpha1.Zarf
 		}
 	}
 
-	deployedPackage = &types.DeployedPackage{
+	deployedPackage := &types.DeployedPackage{
 		Name:               packageName,
 		CLIVersion:         config.CLIVersion,
 		Data:               pkg,
@@ -285,12 +286,13 @@ func (c *Cluster) DisableRegHPAScaleDown(ctx context.Context) error {
 }
 
 // GetInstalledChartsForComponent returns any installed Helm Charts for the provided package component.
-func (c *Cluster) GetInstalledChartsForComponent(ctx context.Context, packageName string, component v1alpha1.ZarfComponent) (installedCharts []types.InstalledChart, err error) {
+func (c *Cluster) GetInstalledChartsForComponent(ctx context.Context, packageName string, component v1alpha1.ZarfComponent) ([]types.InstalledChart, error) {
 	deployedPackage, err := c.GetDeployedPackage(ctx, packageName)
 	if err != nil {
-		return installedCharts, err
+		return nil, err
 	}
 
+	installedCharts := make([]types.InstalledChart, 0)
 	for _, deployedComponent := range deployedPackage.DeployedComponents {
 		if deployedComponent.Name == component.Name {
 			installedCharts = append(installedCharts, deployedComponent.InstalledCharts...)

src/pkg/cluster/zarf_test.go

@@ -30,7 +30,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 		deployedPackage *types.DeployedPackage
 		component       v1alpha1.ZarfComponent
 		skipWebhooks    bool
-		needsWait       bool
 		waitSeconds     int
 		hookName        string
 	}
@@ -49,7 +48,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 				Name:              packageName,
 				ComponentWebhooks: map[string]map[string]types.Webhook{},
 			},
-			needsWait:   false,
 			waitSeconds: 0,
 			hookName:    "",
 		},
@@ -67,7 +65,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 					},
 				},
 			},
-			needsWait:   true,
 			waitSeconds: 10,
 			hookName:    webhookName,
 		},
@@ -86,7 +83,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 					},
 				},
 			},
-			needsWait:   false,
 			waitSeconds: 0,
 			hookName:    "",
 		},
@@ -103,7 +99,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 					},
 				},
 			},
-			needsWait:   false,
 			waitSeconds: 0,
 			hookName:    "",
 		},
@@ -120,7 +115,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 					},
 				},
 			},
-			needsWait:   false,
 			waitSeconds: 0,
 			hookName:    "",
 		},
@@ -137,7 +131,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 					},
 				},
 			},
-			needsWait:   false,
 			waitSeconds: 0,
 			hookName:    "",
 		},
@@ -160,7 +153,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 					},
 				},
 			},
-			needsWait:   false,
 			waitSeconds: 0,
 			hookName:    "",
 		},
@@ -179,7 +171,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 					},
 				},
 			},
-			needsWait:   false,
 			waitSeconds: 0,
 			hookName:    "",
 		},
@@ -193,9 +184,8 @@ func TestPackageSecretNeedsWait(t *testing.T) {
 
 			c := &Cluster{}
 
-			needsWait, waitSeconds, hookName := c.PackageSecretNeedsWait(testCase.deployedPackage, testCase.component, testCase.skipWebhooks)
+			waitSeconds, hookName := c.PackageSecretNeedsWait(testCase.deployedPackage, testCase.component, testCase.skipWebhooks)
 
-			require.Equal(t, testCase.needsWait, needsWait)
 			require.Equal(t, testCase.waitSeconds, waitSeconds)
 			require.Equal(t, testCase.hookName, hookName)
 		})