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: trim named returns in pkg #2950 #2979

Merged
merged 16 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
4 changes: 3 additions & 1 deletion src/pkg/cluster/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 17 additions & 15 deletions src/pkg/cluster/zarf.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
mkcp marked this conversation as resolved.
Show resolved Hide resolved
// 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.
Expand All @@ -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
}

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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...)
Expand Down
12 changes: 1 addition & 11 deletions src/pkg/cluster/zarf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
deployedPackage *types.DeployedPackage
component v1alpha1.ZarfComponent
skipWebhooks bool
needsWait bool
waitSeconds int
hookName string
}
Expand All @@ -49,7 +48,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
Name: packageName,
ComponentWebhooks: map[string]map[string]types.Webhook{},
},
needsWait: false,
waitSeconds: 0,
hookName: "",
},
Expand All @@ -67,7 +65,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
},
},
},
needsWait: true,
waitSeconds: 10,
hookName: webhookName,
},
Expand All @@ -86,7 +83,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
},
},
},
needsWait: false,
waitSeconds: 0,
hookName: "",
},
Expand All @@ -103,7 +99,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
},
},
},
needsWait: false,
waitSeconds: 0,
hookName: "",
},
Expand All @@ -120,7 +115,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
},
},
},
needsWait: false,
waitSeconds: 0,
hookName: "",
},
Expand All @@ -137,7 +131,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
},
},
},
needsWait: false,
waitSeconds: 0,
hookName: "",
},
Expand All @@ -160,7 +153,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
},
},
},
needsWait: false,
waitSeconds: 0,
hookName: "",
},
Expand All @@ -179,7 +171,6 @@ func TestPackageSecretNeedsWait(t *testing.T) {
},
},
},
needsWait: false,
waitSeconds: 0,
hookName: "",
},
Expand All @@ -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)
})
Expand Down
9 changes: 7 additions & 2 deletions src/pkg/interactive/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

// SelectOptionalComponent prompts to confirm optional components
func SelectOptionalComponent(component v1alpha1.ZarfComponent) (confirm bool, err error) {
func SelectOptionalComponent(component v1alpha1.ZarfComponent) (bool, error) {
message.HorizontalRule()

displayComponent := component
Expand All @@ -30,7 +30,12 @@ func SelectOptionalComponent(component v1alpha1.ZarfComponent) (confirm bool, er
Default: component.Default,
}

return confirm, survey.AskOne(prompt, &confirm)
var confirm bool
err := survey.AskOne(prompt, &confirm)
if err != nil {
return false, err
}
return confirm, nil
}

// SelectChoiceGroup prompts to select component groups
Expand Down
15 changes: 12 additions & 3 deletions src/pkg/interactive/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ func PromptSigPassword() ([]byte, error) {
prompt := &survey.Password{
Message: "Private key password (empty for no password): ",
}
return []byte(password), survey.AskOne(prompt, &password)
err := survey.AskOne(prompt, &password)
if err != nil {
return []byte{}, err
}
return []byte(password), nil
}

// PromptVariable prompts the user for a value for a variable
func PromptVariable(variable v1alpha1.InteractiveVariable) (value string, err error) {
func PromptVariable(variable v1alpha1.InteractiveVariable) (string, error) {
if variable.Description != "" {
message.Question(variable.Description)
}
Expand All @@ -33,5 +37,10 @@ func PromptVariable(variable v1alpha1.InteractiveVariable) (value string, err er
Default: variable.Default,
}

return value, survey.AskOne(prompt, &value)
var value string
err := survey.AskOne(prompt, &value)
if err != nil {
return "", err
}
return value, nil
}
26 changes: 13 additions & 13 deletions src/pkg/layout/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type Components struct {
var ErrNotLoaded = fmt.Errorf("not loaded")

// Archive archives a component.
func (c *Components) Archive(component v1alpha1.ZarfComponent, cleanupTemp bool) (err error) {
func (c *Components) Archive(component v1alpha1.ZarfComponent, cleanupTemp bool) error {
name := component.Name
if _, ok := c.Dirs[name]; !ok {
return &fs.PathError{
Expand Down Expand Up @@ -75,7 +75,7 @@ func (c *Components) Archive(component v1alpha1.ZarfComponent, cleanupTemp bool)
}

// Unarchive unarchives a component.
func (c *Components) Unarchive(component v1alpha1.ZarfComponent) (err error) {
func (c *Components) Unarchive(component v1alpha1.ZarfComponent) error {
name := component.Name
tb, ok := c.Tarballs[name]
if !ok {
Expand Down Expand Up @@ -138,7 +138,7 @@ func (c *Components) Unarchive(component v1alpha1.ZarfComponent) (err error) {
}

// Create creates a new component directory structure.
func (c *Components) Create(component v1alpha1.ZarfComponent) (cp *ComponentPaths, err error) {
func (c *Components) Create(component v1alpha1.ZarfComponent) (*ComponentPaths, error) {
name := component.Name

_, ok := c.Tarballs[name]
Expand All @@ -150,41 +150,41 @@ func (c *Components) Create(component v1alpha1.ZarfComponent) (cp *ComponentPath
}
}

if err = helpers.CreateDirectory(c.Base, helpers.ReadWriteExecuteUser); err != nil {
if err := helpers.CreateDirectory(c.Base, helpers.ReadWriteExecuteUser); err != nil {
return nil, err
}

base := filepath.Join(c.Base, name)

if err = helpers.CreateDirectory(base, helpers.ReadWriteExecuteUser); err != nil {
if err := helpers.CreateDirectory(base, helpers.ReadWriteExecuteUser); err != nil {
return nil, err
}

cp = &ComponentPaths{
cp := &ComponentPaths{
Base: base,
}

cp.Temp = filepath.Join(base, TempDir)
if err = helpers.CreateDirectory(cp.Temp, helpers.ReadWriteExecuteUser); err != nil {
if err := helpers.CreateDirectory(cp.Temp, helpers.ReadWriteExecuteUser); err != nil {
return nil, err
}

if len(component.Files) > 0 {
cp.Files = filepath.Join(base, FilesDir)
if err = helpers.CreateDirectory(cp.Files, helpers.ReadWriteExecuteUser); err != nil {
if err := helpers.CreateDirectory(cp.Files, helpers.ReadWriteExecuteUser); err != nil {
return nil, err
}
}

if len(component.Charts) > 0 {
cp.Charts = filepath.Join(base, ChartsDir)
if err = helpers.CreateDirectory(cp.Charts, helpers.ReadWriteExecuteUser); err != nil {
if err := helpers.CreateDirectory(cp.Charts, helpers.ReadWriteExecuteUser); err != nil {
return nil, err
}
for _, chart := range component.Charts {
cp.Values = filepath.Join(base, ValuesDir)
if len(chart.ValuesFiles) > 0 {
if err = helpers.CreateDirectory(cp.Values, helpers.ReadWriteExecuteUser); err != nil {
if err := helpers.CreateDirectory(cp.Values, helpers.ReadWriteExecuteUser); err != nil {
return nil, err
}
break
Expand All @@ -194,21 +194,21 @@ func (c *Components) Create(component v1alpha1.ZarfComponent) (cp *ComponentPath

if len(component.Repos) > 0 {
cp.Repos = filepath.Join(base, ReposDir)
if err = helpers.CreateDirectory(cp.Repos, helpers.ReadWriteExecuteUser); err != nil {
if err := helpers.CreateDirectory(cp.Repos, helpers.ReadWriteExecuteUser); err != nil {
return nil, err
}
}

if len(component.Manifests) > 0 {
cp.Manifests = filepath.Join(base, ManifestsDir)
if err = helpers.CreateDirectory(cp.Manifests, helpers.ReadWriteExecuteUser); err != nil {
if err := helpers.CreateDirectory(cp.Manifests, helpers.ReadWriteExecuteUser); err != nil {
return nil, err
}
}

if len(component.DataInjections) > 0 {
cp.DataInjections = filepath.Join(base, DataInjectionsDir)
if err = helpers.CreateDirectory(cp.DataInjections, helpers.ReadWriteExecuteUser); err != nil {
if err := helpers.CreateDirectory(cp.DataInjections, helpers.ReadWriteExecuteUser); err != nil {
return nil, err
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/pkg/layout/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,14 @@ func New(baseDir string) *PackagePaths {

// ReadZarfYAML reads a zarf.yaml file into memory,
// checks if it's using the legacy layout, and migrates deprecated component configs.
func (pp *PackagePaths) ReadZarfYAML() (pkg v1alpha1.ZarfPackage, warnings []string, err error) {
func (pp *PackagePaths) ReadZarfYAML() (v1alpha1.ZarfPackage, []string, error) {
var pkg v1alpha1.ZarfPackage

if err := utils.ReadYaml(pp.ZarfYAML, &pkg); err != nil {
return v1alpha1.ZarfPackage{}, nil, fmt.Errorf("unable to read zarf.yaml: %w", err)
}

warnings := make([]string, 0)
if pp.IsLegacyLayout() {
warnings = append(warnings, "Detected deprecated package layout, migrating to new layout - support for this package will be dropped in v1.0.0")
}
Expand All @@ -74,7 +77,7 @@ func (pp *PackagePaths) ReadZarfYAML() (pkg v1alpha1.ZarfPackage, warnings []str
}

// MigrateLegacy migrates a legacy package layout to the new layout.
func (pp *PackagePaths) MigrateLegacy() (err error) {
func (pp *PackagePaths) MigrateLegacy() error {
var pkg v1alpha1.ZarfPackage
base := pp.Base

Expand Down
Loading
Loading