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: remove use of named returns in packager #2940

Merged
merged 1 commit into from
Aug 29, 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
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