Skip to content

Commit

Permalink
refactor: remove use of named returns in packager (#2940)
Browse files Browse the repository at this point in the history
Signed-off-by: Philip Laine <philip.laine@gmail.com>
  • Loading branch information
phillebaba committed Aug 29, 2024
1 parent 41db046 commit 4b58792
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 38 deletions.
7 changes: 4 additions & 3 deletions src/pkg/packager/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,16 @@ func (p *Packager) GetVariableConfig() *variables.VariableConfig {
}

// connectToCluster attempts to connect to a cluster if a connection is not already established
func (p *Packager) connectToCluster(ctx context.Context) (err error) {
func (p *Packager) connectToCluster(ctx context.Context) error {
if p.isConnectedToCluster() {
return nil
}

p.cluster, err = cluster.NewClusterWithWait(ctx)
cluster, err := cluster.NewClusterWithWait(ctx)
if err != nil {
return err
}
p.cluster = cluster

return p.attemptClusterChecks(ctx)
}
Expand All @@ -150,7 +151,7 @@ func (p *Packager) isConnectedToCluster() bool {

// attemptClusterChecks attempts to connect to the cluster and check for useful metadata and config mismatches.
// NOTE: attemptClusterChecks should only return an error if there is a problem significant enough to halt a deployment, otherwise it should return nil and print a warning message.
func (p *Packager) attemptClusterChecks(ctx context.Context) (err error) {
func (p *Packager) attemptClusterChecks(ctx context.Context) error {
spinner := message.NewProgressSpinner("Gathering additional cluster information (if available)")
defer spinner.Stop()

Expand Down
61 changes: 34 additions & 27 deletions src/pkg/packager/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ func (p *Packager) Deploy(ctx context.Context) error {
}

// deployComponents loops through a list of ZarfComponents and deploys them.
func (p *Packager) deployComponents(ctx context.Context) (deployedComponents []types.DeployedComponent, err error) {
func (p *Packager) deployComponents(ctx context.Context) ([]types.DeployedComponent, error) {
deployedComponents := []types.DeployedComponent{}

// Process all the components we are deploying
for _, component := range p.cfg.Pkg.Components {
// Connect to cluster if a component requires it.
Expand Down Expand Up @@ -168,10 +170,11 @@ func (p *Packager) deployComponents(ctx context.Context) (deployedComponents []t

// Ensure we don't overwrite any installedCharts data when updating the package secret
if p.isConnectedToCluster() {
deployedComponent.InstalledCharts, err = p.cluster.GetInstalledChartsForComponent(ctx, p.cfg.Pkg.Metadata.Name, component)
installedCharts, err := p.cluster.GetInstalledChartsForComponent(ctx, p.cfg.Pkg.Metadata.Name, component)
if err != nil {
message.Debugf("Unable to fetch installed Helm charts for component '%s': %s", component.Name, err.Error())
}
deployedComponent.InstalledCharts = installedCharts
}

deployedComponents = append(deployedComponents, deployedComponent)
Expand Down Expand Up @@ -211,8 +214,7 @@ func (p *Packager) deployComponents(ctx context.Context) (deployedComponents []t
message.Debugf("Unable to record package deployment for component %q: this will affect features like `zarf package remove`: %s", component.Name, err.Error())
}
}

return deployedComponents, fmt.Errorf("unable to deploy component %q: %w", component.Name, deployErr)
return nil, fmt.Errorf("unable to deploy component %q: %w", component.Name, deployErr)
}

// Update the package secret to indicate that we successfully deployed this component
Expand All @@ -226,14 +228,14 @@ func (p *Packager) deployComponents(ctx context.Context) (deployedComponents []t

if err := actions.Run(ctx, onDeploy.Defaults, onDeploy.OnSuccess, p.variableConfig); err != nil {
onFailure()
return deployedComponents, fmt.Errorf("unable to run component success action: %w", err)
return nil, fmt.Errorf("unable to run component success action: %w", err)
}
}

return deployedComponents, nil
}

func (p *Packager) deployInitComponent(ctx context.Context, component v1alpha1.ZarfComponent) (charts []types.InstalledChart, err error) {
func (p *Packager) deployInitComponent(ctx context.Context, component v1alpha1.ZarfComponent) ([]types.InstalledChart, error) {
hasExternalRegistry := p.cfg.InitOpts.RegistryInfo.Address != ""
isSeedRegistry := component.Name == "zarf-seed-registry"
isRegistry := component.Name == "zarf-registry"
Expand All @@ -247,7 +249,7 @@ func (p *Packager) deployInitComponent(ctx context.Context, component v1alpha1.Z

// Always init the state before the first component that requires the cluster (on most deployments, the zarf-seed-registry)
if component.RequiresCluster() && p.state == nil {
err = p.cluster.InitZarfState(ctx, p.cfg.InitOpts)
err := p.cluster.InitZarfState(ctx, p.cfg.InitOpts)
if err != nil {
return nil, fmt.Errorf("unable to initialize Zarf state: %w", err)
}
Expand All @@ -271,7 +273,9 @@ func (p *Packager) deployInitComponent(ctx context.Context, component v1alpha1.Z
}
}

charts, err = p.deployComponent(ctx, component, isAgent /* skip img checksum if isAgent */, isSeedRegistry /* skip image push if isSeedRegistry */)
// Skip image checksum if component is agent.
// Skip image push if component is seed registry.
charts, err := p.deployComponent(ctx, component, isAgent, isSeedRegistry)
if err != nil {
return nil, err
}
Expand All @@ -287,7 +291,7 @@ func (p *Packager) deployInitComponent(ctx context.Context, component v1alpha1.Z
}

// Deploy a Zarf Component.
func (p *Packager) deployComponent(ctx context.Context, component v1alpha1.ZarfComponent, noImgChecksum bool, noImgPush bool) (charts []types.InstalledChart, err error) {
func (p *Packager) deployComponent(ctx context.Context, component v1alpha1.ZarfComponent, noImgChecksum bool, noImgPush bool) ([]types.InstalledChart, error) {
// Toggles for general deploy operations
componentPath := p.layout.Components.Dirs[component.Name]

Expand All @@ -305,9 +309,9 @@ func (p *Packager) deployComponent(ctx context.Context, component v1alpha1.ZarfC
if component.RequiresCluster() {
// Setup the state in the config
if p.state == nil {
err = p.setupState(ctx)
err := p.setupState(ctx)
if err != nil {
return charts, err
return nil, err
}
}

Expand All @@ -321,30 +325,30 @@ func (p *Packager) deployComponent(ctx context.Context, component v1alpha1.ZarfC
}
}

err = p.populateComponentAndStateTemplates(component.Name)
err := p.populateComponentAndStateTemplates(component.Name)
if err != nil {
return charts, err
return nil, err
}

if err = actions.Run(ctx, onDeploy.Defaults, onDeploy.Before, p.variableConfig); err != nil {
return charts, fmt.Errorf("unable to run component before action: %w", err)
return nil, fmt.Errorf("unable to run component before action: %w", err)
}

if hasFiles {
if err := p.processComponentFiles(component, componentPath.Files); err != nil {
return charts, fmt.Errorf("unable to process the component files: %w", err)
return nil, fmt.Errorf("unable to process the component files: %w", err)
}
}

if hasImages {
if err := p.pushImagesToRegistry(ctx, component.Images, noImgChecksum); err != nil {
return charts, fmt.Errorf("unable to push images to the registry: %w", err)
return nil, fmt.Errorf("unable to push images to the registry: %w", err)
}
}

if hasRepos {
if err = p.pushReposToRepository(ctx, componentPath.Repos, component.Repos); err != nil {
return charts, fmt.Errorf("unable to push the repos to the repository: %w", err)
return nil, fmt.Errorf("unable to push the repos to the repository: %w", err)
}
}

Expand All @@ -355,14 +359,15 @@ func (p *Packager) deployComponent(ctx context.Context, component v1alpha1.ZarfC
})
}

charts := []types.InstalledChart{}
if hasCharts || hasManifests {
if charts, err = p.installChartAndManifests(ctx, componentPath, component); err != nil {
return charts, err
return nil, err
}
}

if err = actions.Run(ctx, onDeploy.Defaults, onDeploy.After, p.variableConfig); err != nil {
return charts, fmt.Errorf("unable to run component after action: %w", err)
return nil, fmt.Errorf("unable to run component after action: %w", err)
}

err = g.Wait()
Expand Down Expand Up @@ -452,7 +457,7 @@ func (p *Packager) processComponentFiles(component v1alpha1.ZarfComponent, pkgLo
}

// setupState fetches the current ZarfState from the k8s cluster and sets the packager to use it
func (p *Packager) setupState(ctx context.Context) (err error) {
func (p *Packager) setupState(ctx context.Context) error {
// If we are touching K8s, make sure we can talk to it once per deployment
spinner := message.NewProgressSpinner("Loading the Zarf State from the Kubernetes cluster")
defer spinner.Stop()
Expand Down Expand Up @@ -637,7 +642,9 @@ func (p *Packager) generateValuesOverrides(chart v1alpha1.ZarfChart, componentNa
}

// Install all Helm charts and raw k8s manifests into the k8s cluster.
func (p *Packager) installChartAndManifests(ctx context.Context, componentPaths *layout.ComponentPaths, component v1alpha1.ZarfComponent) (installedCharts []types.InstalledChart, err error) {
func (p *Packager) installChartAndManifests(ctx context.Context, componentPaths *layout.ComponentPaths, component v1alpha1.ZarfComponent) ([]types.InstalledChart, error) {
installedCharts := []types.InstalledChart{}

for _, chart := range component.Charts {
// Do not wait for the chart to be ready if data injections are present.
if len(component.DataInjections) > 0 {
Expand All @@ -648,15 +655,15 @@ func (p *Packager) installChartAndManifests(ctx context.Context, componentPaths
for idx := range chart.ValuesFiles {
valueFilePath := helm.StandardValuesName(componentPaths.Values, chart, idx)
if err := p.variableConfig.ReplaceTextTemplate(valueFilePath); err != nil {
return installedCharts, err
return nil, err
}
}

// Create a Helm values overrides map from set Zarf `variables` and DeployOpts library inputs
// Values overrides are to be applied in order of Helm Chart Defaults -> Zarf `valuesFiles` -> Zarf `variables` -> DeployOpts overrides
valuesOverrides, err := p.generateValuesOverrides(chart, component.Name)
if err != nil {
return installedCharts, err
return nil, err
}

helmCfg := helm.New(
Expand All @@ -675,7 +682,7 @@ func (p *Packager) installChartAndManifests(ctx context.Context, componentPaths

addedConnectStrings, installedChartName, err := helmCfg.InstallOrUpgradeChart(ctx)
if err != nil {
return installedCharts, err
return nil, err
}
installedCharts = append(installedCharts, types.InstalledChart{Namespace: chart.Namespace, ChartName: installedChartName})

Expand All @@ -691,7 +698,7 @@ func (p *Packager) installChartAndManifests(ctx context.Context, componentPaths
// The path is likely invalid because of how we compose OCI components, add an index suffix to the filename
manifest.Files[idx] = fmt.Sprintf("%s-%d.yaml", manifest.Name, idx)
if helpers.InvalidPath(filepath.Join(componentPaths.Manifests, manifest.Files[idx])) {
return installedCharts, fmt.Errorf("unable to find manifest file %s", manifest.Files[idx])
return nil, fmt.Errorf("unable to find manifest file %s", manifest.Files[idx])
}
}
}
Expand Down Expand Up @@ -722,13 +729,13 @@ func (p *Packager) installChartAndManifests(ctx context.Context, componentPaths
p.cfg.PkgOpts.Retries),
)
if err != nil {
return installedCharts, err
return nil, err
}

// Install the chart.
addedConnectStrings, installedChartName, err := helmCfg.InstallOrUpgradeChart(ctx)
if err != nil {
return installedCharts, err
return nil, err
}

installedCharts = append(installedCharts, types.InstalledChart{Namespace: manifest.Namespace, ChartName: installedChartName})
Expand Down
2 changes: 1 addition & 1 deletion src/pkg/packager/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
)

// Generate generates a Zarf package definition.
func (p *Packager) Generate(ctx context.Context) (err error) {
func (p *Packager) Generate(ctx context.Context) error {
generatedZarfYAMLPath := filepath.Join(p.cfg.GenerateOpts.Output, layout.ZarfYAML)
spinner := message.NewProgressSpinner("Generating package for %q at %s", p.cfg.GenerateOpts.Name, generatedZarfYAMLPath)

Expand Down
5 changes: 3 additions & 2 deletions src/pkg/packager/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ import (
)

// Inspect list the contents of a package.
func (p *Packager) Inspect(ctx context.Context) (err error) {
func (p *Packager) Inspect(ctx context.Context) error {
wantSBOM := p.cfg.InspectOpts.ViewSBOM || p.cfg.InspectOpts.SBOMOutputDir != ""

p.cfg.Pkg, _, err = p.source.LoadPackageMetadata(ctx, p.layout, wantSBOM, true)
pkg, _, err := p.source.LoadPackageMetadata(ctx, p.layout, wantSBOM, true)
if err != nil {
return err
}
p.cfg.Pkg = pkg

if p.cfg.InspectOpts.ListImages {
imageList := []string{}
Expand Down
3 changes: 2 additions & 1 deletion src/pkg/packager/interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"github.com/zarf-dev/zarf/src/pkg/utils"
)

func (p *Packager) confirmAction(stage string, warnings []string, sbomViewFiles []string) (confirm bool) {
func (p *Packager) confirmAction(stage string, warnings []string, sbomViewFiles []string) bool {
pterm.Println()
message.HeaderInfof("📦 PACKAGE DEFINITION")
utils.ColorPrintYAML(p.cfg.Pkg, p.getPackageYAMLHints(stage), true)
Expand Down Expand Up @@ -77,6 +77,7 @@ func (p *Packager) confirmAction(stage string, warnings []string, sbomViewFiles
pterm.Println()

// Prompt the user for confirmation, on abort return false
var confirm bool
if err := survey.AskOne(prompt, &confirm); err != nil || !confirm {
// User aborted or declined, cancel the action
return false
Expand Down
4 changes: 2 additions & 2 deletions src/pkg/packager/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import (
)

// Pull pulls a Zarf package and saves it as a compressed tarball.
func (p *Packager) Pull(ctx context.Context) (err error) {
func (p *Packager) Pull(ctx context.Context) error {
if p.cfg.PkgOpts.OptionalComponents != "" {
return fmt.Errorf("pull does not support optional components")
}

_, err = p.source.Collect(ctx, p.cfg.PullOpts.OutputDirectory)
_, err := p.source.Collect(ctx, p.cfg.PullOpts.OutputDirectory)
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions src/pkg/packager/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
)

// Remove removes a package that was already deployed onto a cluster, uninstalling all installed helm charts.
func (p *Packager) Remove(ctx context.Context) (err error) {
func (p *Packager) Remove(ctx context.Context) error {
_, isClusterSource := p.source.(*sources.ClusterSource)
if isClusterSource {
p.cluster = p.source.(*sources.ClusterSource).Cluster
Expand All @@ -40,10 +40,11 @@ func (p *Packager) Remove(ctx context.Context) (err error) {

// we do not want to allow removal of signed packages without a signature if there are remove actions
// as this is arbitrary code execution from an untrusted source
p.cfg.Pkg, _, err = p.source.LoadPackageMetadata(ctx, p.layout, false, false)
pkg, _, err := p.source.LoadPackageMetadata(ctx, p.layout, false, false)
if err != nil {
return err
}
p.cfg.Pkg = pkg
packageName := p.cfg.Pkg.Metadata.Name

// Build a list of components to remove and determine if we need a cluster connection
Expand Down

0 comments on commit 4b58792

Please sign in to comment.