chore: address feedback and fix airgap required version setup

Author: JGAntunes

cmd/installer/cli/upgrade.go

@@ -258,8 +258,8 @@ func preRunUpgrade(ctx context.Context, flags UpgradeCmdFlags, upgradeConfig *up
 	}
 	upgradeConfig.license = l
 
-	// sync the license if a license is provided and we are not in airgap mode
-	if upgradeConfig.license != nil && flags.airgapBundle == "" {
+	// sync the license and initialize the replicated api client if we are not in airgap mode
+	if flags.airgapBundle == "" {
 		replicatedAPI, err := newReplicatedAPIClient(upgradeConfig.license, upgradeConfig.clusterID)
 		if err != nil {
 			return fmt.Errorf("failed to create replicated API client: %w", err)
@@ -607,11 +607,9 @@ func validateIsReleaseUpgradable(ctx context.Context, upgradeConfig upgradeConfi
 
 	// Build validation options
 	opts := validation.UpgradableOptions{
-		IsAirgap:         isAirgap,
 		CurrentECVersion: currentECVersion,
 		TargetECVersion:  targetECVersion,
 		License:          upgradeConfig.license,
-		AirgapMetadata:   upgradeConfig.airgapMetadata,
 	}
 
 	// Add current app version info if available
@@ -624,10 +622,15 @@ func validateIsReleaseUpgradable(ctx context.Context, upgradeConfig upgradeConfi
 	opts.TargetAppVersion = channelRelease.VersionLabel
 	opts.TargetAppSequence = channelRelease.ChannelSequence
 
-	// For online upgrades, add the replicated API client and channel ID
-	if upgradeConfig.replicatedAPIClient != nil {
-		opts.ReplicatedAPI = upgradeConfig.replicatedAPIClient
-		opts.ChannelID = channelRelease.ChannelID
+	// Extract the required releases depending on if it's airgap or online
+	if isAirgap {
+		if err := opts.WithAirgapRequiredReleases(upgradeConfig.airgapMetadata); err != nil {
+			return fmt.Errorf("failed to extract required releases from airgap metadata: %w", err)
+		}
+	} else {
+		if err := opts.WithOnlineRequiredReleases(ctx, upgradeConfig.replicatedAPIClient); err != nil {
+			return fmt.Errorf("failed to extract required releases from replidated API's pending release call: %w", err)
+		}
 	}
 
 	// Perform validation

pkg-new/replicatedapi/mock.go

@@ -17,15 +17,6 @@ func (e *ValidationError) Error() string {
 	return e.Message
 }
 
-// IsValidationError returns true if err is or wraps a ValidationError
-func IsValidationError(err error) bool {
-	if err == nil {
-		return false
-	}
-	_, ok := err.(*ValidationError)
-	return ok
-}
-
 // NewRequiredReleasesError creates a ValidationError indicating that intermediate
 // required releases must be installed before upgrading to the target version
 func NewRequiredReleasesError(requiredVersions []string, targetVersion string) *ValidationError {

pkg-new/validation/errors.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"regexp"
+	"strconv"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/replicatedhq/embedded-cluster/pkg-new/replicatedapi"
@@ -16,17 +17,70 @@ var k8sBuildRegex = regexp.MustCompile(`k8s-(\d+\.\d+)`)
 
 // UpgradableOptions holds configuration for validating release deployability
 type UpgradableOptions struct {
-	IsAirgap           bool
 	CurrentAppVersion  string
 	CurrentAppSequence int64
 	CurrentECVersion   string
 	TargetAppVersion   string
 	TargetAppSequence  int64
 	TargetECVersion    string
 	License            *kotsv1beta1.License
-	AirgapMetadata     *airgap.AirgapMetadata
-	ReplicatedAPI      replicatedapi.Client
-	ChannelID          string
+	requiredReleases   []string
+}
+
+func (opts *UpgradableOptions) WithAirgapRequiredReleases(metadata *airgap.AirgapMetadata) error {
+	if metadata == nil || metadata.AirgapInfo == nil {
+		return fmt.Errorf("airgap metadata is required for validating airgap required releases")
+	}
+
+	// RequiredReleases are in descending order, we need to iterate through the required releases of the target release until we find releases lower than the current installed release
+	requiredReleases := metadata.AirgapInfo.Spec.RequiredReleases
+	if len(requiredReleases) > 0 {
+		// Extract version labels from required releases
+		for _, release := range requiredReleases {
+			sequence, err := strconv.ParseInt(release.UpdateCursor, 10, 64)
+			if err != nil {
+				return fmt.Errorf("failed to parse airgap spec required release update cursor %s: %w", release.UpdateCursor, err)
+			}
+			// We've hit a release that is less than or equal to the current installed release, we can stop
+			if sequence <= opts.CurrentAppSequence {
+				return nil
+			}
+			opts.requiredReleases = append(opts.requiredReleases, release.VersionLabel)
+		}
+	}
+	return nil
+}
+
+func (opts *UpgradableOptions) WithOnlineRequiredReleases(ctx context.Context, replAPIClient replicatedapi.Client) error {
+	if opts.License == nil {
+		return fmt.Errorf("license is required to check online upgrade required releases")
+	}
+	options := &replicatedapi.PendingReleasesOptions{
+		IsSemverSupported: opts.License.Spec.IsSemverRequired,
+		SortOrder:         replicatedapi.SortOrderAscending,
+	}
+	// Get pending releases from the current app sequence in asceding order
+	pendingReleases, err := replAPIClient.GetPendingReleases(ctx, opts.License.Spec.ChannelID, opts.CurrentAppSequence, options)
+	if err != nil {
+		return fmt.Errorf("failed to get pending releases while checking required releases for upgrade: %w", err)
+	}
+	if pendingReleases != nil {
+		opts.handlePendingReleases(pendingReleases.ChannelReleases)
+	}
+	return nil
+}
+
+func (opts *UpgradableOptions) handlePendingReleases(pendingReleases []replicatedapi.ChannelRelease) {
+	// Find required releases between current and target sequence
+	for _, release := range pendingReleases {
+		// Releases are in asceding order, we've hit the target sequence so we can break
+		if release.ChannelSequence == opts.TargetAppSequence {
+			break
+		}
+		if release.IsRequired {
+			opts.requiredReleases = append(opts.requiredReleases, release.VersionLabel)
+		}
+	}
 }
 
 // ValidateIsReleaseUpgradable validates that a target release can be safely deployed
@@ -56,54 +110,8 @@ func ValidateIsReleaseUpgradable(ctx context.Context, opts UpgradableOptions) er
 
 // validateRequiredReleases checks if any required releases are being skipped
 func validateRequiredReleases(ctx context.Context, opts UpgradableOptions) error {
-	requiredVersions := []string{}
-	if opts.IsAirgap {
-		// For airgap, check RequiredReleases field in the airgap metadata
-		if opts.AirgapMetadata == nil || opts.AirgapMetadata.AirgapInfo == nil {
-			return fmt.Errorf("airgap metadata is required for airgap validation")
-		}
-
-		requiredReleases := opts.AirgapMetadata.AirgapInfo.Spec.RequiredReleases
-		if len(requiredReleases) > 0 {
-			// Extract version labels from required releases
-			for _, release := range requiredReleases {
-				requiredVersions = append(requiredVersions, release.VersionLabel)
-			}
-		}
-	} else {
-		// For online, call the API
-		if opts.ReplicatedAPI == nil {
-			return fmt.Errorf("replicated API client is required for online validation")
-		}
-
-		if opts.ChannelID == "" {
-			return fmt.Errorf("channel ID is required for online validation")
-		}
-
-		options := &replicatedapi.PendingReleasesOptions{
-			IsSemverSupported: opts.License.Spec.IsSemverRequired,
-			SortOrder:         replicatedapi.SortOrderAscending,
-		}
-		// Get pending releases from the current app seqeuence in asceding order
-		pendingReleases, err := opts.ReplicatedAPI.GetPendingReleases(ctx, opts.ChannelID, opts.CurrentAppSequence, options)
-		if err != nil {
-			return fmt.Errorf("failed to fetch pending releases: %w", err)
-		}
-
-		// Find required releases between current and target sequence
-		for _, release := range pendingReleases.ChannelReleases {
-			// Releases are in asceding order, we've hit the target sequence so we can break
-			if release.ChannelSequence == opts.TargetAppSequence {
-				break
-			}
-			if release.IsRequired {
-				requiredVersions = append(requiredVersions, release.VersionLabel)
-			}
-		}
-	}
-
-	if len(requiredVersions) > 0 {
-		return NewRequiredReleasesError(requiredVersions, opts.TargetAppVersion)
+	if len(opts.requiredReleases) > 0 {
+		return NewRequiredReleasesError(opts.requiredReleases, opts.TargetAppVersion)
 	}
 
 	return nil