From 852b482d1aec6a1fb17b6c88395784921cd0cabc Mon Sep 17 00:00:00 2001 From: Wayne Starr Date: Wed, 25 Oct 2023 15:42:08 -0600 Subject: [PATCH] Fix checks and rehookup breaking changes warning --- src/config/lang/english.go | 2 +- src/internal/cluster/common.go | 6 +++ src/internal/cluster/state.go | 2 +- src/pkg/packager/common.go | 57 +++++++++++++++++++++++ src/pkg/packager/components.go | 13 +----- src/pkg/packager/deploy.go | 65 ++------------------------- src/pkg/packager/deprecated/common.go | 2 + src/pkg/packager/remove.go | 8 ++-- src/pkg/packager/sources/tarball.go | 9 ++-- src/pkg/utils/helpers/misc.go | 9 ++++ 10 files changed, 90 insertions(+), 83 deletions(-) diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 1642e605d1..e17ba3aac5 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -260,7 +260,7 @@ const ( CmdPackageDeployFlagSget = "[Deprecated] Path to public sget key file for remote packages signed via cosign. This flag will be removed in v1.0.0 please use the --key flag instead." CmdPackageDeployFlagSkipWebhooks = "[alpha] Skip waiting for external webhooks to execute as each package component is deployed" CmdPackageDeployValidateArchitectureErr = "this package architecture is %s, but the target cluster has the %s architecture. These architectures must be the same" - CmdPackageDeployValidateLastNonBreakingVersionWarn = "the version of this Zarf binary '%s' is less than the LastNonBreakingVersion of '%s'. You may need to upgrade your Zarf version to at least '%s' to deploy this package" + CmdPackageDeployValidateLastNonBreakingVersionWarn = "The version of this Zarf binary '%s' is less than the LastNonBreakingVersion of '%s'. You may need to upgrade your Zarf version to at least '%s' to deploy this package" CmdPackageDeployInvalidCLIVersionWarn = "CLIVersion is set to '%s' which can cause issues with package creation and deployment. To avoid such issues, please set the value to the valid semantic version for this version of Zarf." CmdPackageDeployErr = "Failed to deploy package: %s" diff --git a/src/internal/cluster/common.go b/src/internal/cluster/common.go index f17b3792f9..6a10a3bbd9 100644 --- a/src/internal/cluster/common.go +++ b/src/internal/cluster/common.go @@ -70,5 +70,11 @@ func NewCluster() (*Cluster, error) { return nil, err } + // Dogsled the version output. We just want to ensure no errors were returned to validate cluster connection. + _, err = c.GetServerVersion() + if err != nil { + return nil, err + } + return c, nil } diff --git a/src/internal/cluster/state.go b/src/internal/cluster/state.go index ba9888c26f..2e5519192e 100644 --- a/src/internal/cluster/state.go +++ b/src/internal/cluster/state.go @@ -40,7 +40,7 @@ func (c *Cluster) InitZarfState(initOptions types.ZarfInitOptions) error { err error ) - spinner := message.NewProgressSpinner("Gathering cluster information") + spinner := message.NewProgressSpinner("Gathering cluster state information") defer spinner.Stop() spinner.Updatef("Getting cluster architecture") diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go index 59ed1e1737..692044f29b 100644 --- a/src/pkg/packager/common.go +++ b/src/pkg/packager/common.go @@ -6,11 +6,13 @@ package packager import ( "encoding/json" + "errors" "fmt" "os" "path/filepath" "regexp" "strings" + "time" "github.com/Masterminds/semver/v3" "github.com/defenseunicorns/zarf/src/config/lang" @@ -25,6 +27,7 @@ import ( "github.com/defenseunicorns/zarf/src/pkg/layout" "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/oci" + "github.com/defenseunicorns/zarf/src/pkg/packager/deprecated" "github.com/defenseunicorns/zarf/src/pkg/packager/sources" "github.com/defenseunicorns/zarf/src/pkg/utils" ) @@ -201,6 +204,60 @@ func (p *Packager) ClearTempPaths() { _ = os.RemoveAll(layout.SBOMDir) } +// connectToCluster attempts to connect to a cluster if a connection is not already established +func (p *Packager) connectToCluster(timeout time.Duration) (err error) { + if p.isConnectedToCluster() { + return nil + } + + p.cluster, err = cluster.NewClusterWithWait(timeout) + if err != nil { + return err + } + + return p.attemptClusterChecks() +} + +// isConnectedToCluster returns whether the current packager instance is connected to a cluster +func (p *Packager) isConnectedToCluster() bool { + return p.cluster != nil +} + +// 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() (err error) { + + spinner := message.NewProgressSpinner("Gathering additional cluster information (if available)") + defer spinner.Stop() + + // Check if the package has already been deployed and get its generation + if existingDeployedPackage, _ := p.cluster.GetDeployedPackage(p.cfg.Pkg.Metadata.Name); existingDeployedPackage != nil { + // If this package has been deployed before, increment the package generation within the secret + p.generation = existingDeployedPackage.Generation + 1 + } + + // Check the clusters architecture matches the package spec + if err := p.validatePackageArchitecture(); err != nil { + if errors.Is(err, lang.ErrUnableToCheckArch) { + message.Warnf("Unable to validate package architecture: %s", err.Error()) + } else { + return err + } + } + + // Check for any breaking changes between the initialized Zarf version and this CLI + if existingInitPackage, _ := p.cluster.GetDeployedPackage("init"); existingInitPackage != nil { + // Use the build version instead of the metadata since this will support older Zarf versions + deprecated.PrintBreakingChanges(existingInitPackage.Data.Build.Version) + } else { + message.Warnf("Unable to retrieve the initialized Zarf version. There is potential for breaking changes.") + } + + spinner.Success() + + return nil +} + // validatePackageArchitecture validates that the package architecture matches the target cluster architecture. func (p *Packager) validatePackageArchitecture() error { // Ignore this check if the architecture is explicitly "multi" or we don't have a cluster connection diff --git a/src/pkg/packager/components.go b/src/pkg/packager/components.go index a7132c8526..d77db2c486 100644 --- a/src/pkg/packager/components.go +++ b/src/pkg/packager/components.go @@ -35,11 +35,11 @@ func (p *Packager) getValidComponents() []types.ZarfComponent { key = component.Name } else { // Otherwise, add the component name to the choice group list for later validation - choiceComponents = p.appendIfNotExists(choiceComponents, component.Name) + choiceComponents = helpers.AppendIfNotExists(choiceComponents, component.Name) } // Preserve component order - orderedKeys = p.appendIfNotExists(orderedKeys, key) + orderedKeys = helpers.AppendIfNotExists(orderedKeys, key) // Append the component to the list of components in the group componentGroups[key] = append(componentGroups[key], component) @@ -208,15 +208,6 @@ func (p *Packager) confirmChoiceGroup(componentGroup []types.ZarfComponent) type return componentGroup[chosen] } -func (p *Packager) appendIfNotExists(slice []string, item string) []string { - for _, s := range slice { - if s == item { - return slice - } - } - return append(slice, item) -} - func (p *Packager) requiresCluster(component types.ZarfComponent) bool { hasImages := len(component.Images) > 0 hasCharts := len(component.Charts) > 0 diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index 9f7af67df4..fddb7719b1 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -5,7 +5,6 @@ package packager import ( - "errors" "fmt" "os" "path/filepath" @@ -15,7 +14,6 @@ import ( "time" "github.com/defenseunicorns/zarf/src/config" - "github.com/defenseunicorns/zarf/src/config/lang" "github.com/defenseunicorns/zarf/src/internal/cluster" "github.com/defenseunicorns/zarf/src/internal/packager/git" "github.com/defenseunicorns/zarf/src/internal/packager/helm" @@ -37,6 +35,7 @@ func (p *Packager) Deploy() (err error) { if err = p.source.LoadPackage(p.layout, true); err != nil { return fmt.Errorf("unable to load the package: %w", err) } + if err = p.readZarfYAML(p.layout.ZarfYAML); err != nil { return err } @@ -87,64 +86,6 @@ func (p *Packager) Deploy() (err error) { return nil } -// 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() (err error) { - if !p.isConnectedToCluster() { - return nil - } - // Check if the package has already been deployed and get its generation - if existingDeployedPackage, _ := p.cluster.GetDeployedPackage(p.cfg.Pkg.Metadata.Name); existingDeployedPackage != nil { - // If this package has been deployed before, increment the package generation within the secret - p.generation = existingDeployedPackage.Generation + 1 - } - - // Check the clusters architecture vs the package spec - if err := p.validatePackageArchitecture(); err != nil { - if errors.Is(err, lang.ErrUnableToCheckArch) { - message.Warnf("Unable to validate package architecture: %s", err.Error()) - } else { - return err - } - } - - return nil -} - -func (p *Packager) isConnectedToCluster() bool { - return p.cluster != nil -} - -func (p *Packager) connectToCluster() (err error) { - if p.isConnectedToCluster() { - // TODO: what is the best way to handle this? - old := p.cluster.RestConfig.String() - - // If we are already connected to the cluster, check if the server name has changed - cluster, err := cluster.NewCluster() - if err != nil { - return fmt.Errorf("unable to connect to the Kubernetes cluster: %w", err) - } - - if old != cluster.RestConfig.String() { - message.Warnf("The Kubernetes cluster has changed from %q to %q", old, cluster.RestConfig.ServerName) - - p.cluster = cluster - - return p.attemptClusterChecks() - } - - return nil - } - - p.cluster, err = cluster.NewClusterWithWait(cluster.DefaultTimeout) - if err != nil { - return fmt.Errorf("unable to connect to the Kubernetes cluster: %w", err) - } - - return p.attemptClusterChecks() -} - // deployComponents loops through a list of ZarfComponents and deploys them. func (p *Packager) deployComponents() (deployedComponents []types.DeployedComponent, err error) { componentsToDeploy := p.getValidComponents() @@ -243,7 +184,7 @@ func (p *Packager) deployInitComponent(component types.ZarfComponent) (charts [] // Always init the state before the first component that requires the cluster (on most deployments, the zarf-seed-registry) if p.requiresCluster(component) && p.cfg.State == nil { - if err := p.connectToCluster(); err != nil { + if err := p.connectToCluster(5 * time.Minute); err != nil { return charts, fmt.Errorf("unable to connect to the Kubernetes cluster: %w", err) } @@ -312,7 +253,7 @@ func (p *Packager) deployComponent(component types.ZarfComponent, noImgChecksum if !p.valueTemplate.Ready() && p.requiresCluster(component) { // Make sure we have access to the cluster - if err := p.connectToCluster(); err != nil { + if err := p.connectToCluster(cluster.DefaultTimeout); err != nil { return charts, fmt.Errorf("unable to connect to the Kubernetes cluster: %w", err) } diff --git a/src/pkg/packager/deprecated/common.go b/src/pkg/packager/deprecated/common.go index fdffb3ba71..782e7916be 100644 --- a/src/pkg/packager/deprecated/common.go +++ b/src/pkg/packager/deprecated/common.go @@ -119,5 +119,7 @@ func PrintBreakingChanges(deployedZarfVersion string) { pterm.Printfln("\n - %s", pterm.Bold.Sprint("Mitigation:")) pterm.Printfln(" %s", strings.ReplaceAll(mitigationText, "\n", "\n ")) } + + message.HorizontalRule() } } diff --git a/src/pkg/packager/remove.go b/src/pkg/packager/remove.go index 407afa8060..7ccb55cfdd 100644 --- a/src/pkg/packager/remove.go +++ b/src/pkg/packager/remove.go @@ -68,11 +68,9 @@ func (p *Packager) Remove() (err error) { deployedPackage := &types.DeployedPackage{} if requiresCluster { - if !p.isConnectedToCluster() { - p.cluster, err = cluster.NewClusterWithWait(cluster.DefaultTimeout) - if err != nil { - return err - } + err = p.connectToCluster(cluster.DefaultTimeout) + if err != nil { + return err } deployedPackage, err = p.cluster.GetDeployedPackage(packageName) if err != nil { diff --git a/src/pkg/packager/sources/tarball.go b/src/pkg/packager/sources/tarball.go index 2a2b5ee666..dc422cf775 100644 --- a/src/pkg/packager/sources/tarball.go +++ b/src/pkg/packager/sources/tarball.go @@ -34,7 +34,10 @@ type TarballSource struct { func (s *TarballSource) LoadPackage(dst *layout.PackagePaths, unarchiveAll bool) (err error) { var pkg types.ZarfPackage - message.Debugf("Loading package from %q", s.PackageSource) + spinner := message.NewProgressSpinner("Loading package from %q", s.PackageSource) + defer spinner.Stop() + + pathsExtracted := []string{} if s.Shasum != "" { if err := utils.SHAsMatch(s.PackageSource, s.Shasum); err != nil { @@ -42,8 +45,6 @@ func (s *TarballSource) LoadPackage(dst *layout.PackagePaths, unarchiveAll bool) } } - pathsExtracted := []string{} - // Walk the package so that was can dynamically load a .tar or a .tar.zst without caring about filenames. err = archiver.Walk(s.PackageSource, func(f archiver.File) error { if f.IsDir() { @@ -122,6 +123,8 @@ func (s *TarballSource) LoadPackage(dst *layout.PackagePaths, unarchiveAll bool) } } + spinner.Success() + return nil } diff --git a/src/pkg/utils/helpers/misc.go b/src/pkg/utils/helpers/misc.go index 06f168c1d2..dbffa720ba 100644 --- a/src/pkg/utils/helpers/misc.go +++ b/src/pkg/utils/helpers/misc.go @@ -193,3 +193,12 @@ func StringToSlice(s string) []string { return []string{} } + +func AppendIfNotExists(slice []string, item string) []string { + for _, s := range slice { + if s == item { + return slice + } + } + return append(slice, item) +}