refactor(payload): separate arch load from metadata and runtime

The architecture is either read from release metadata when present (and in that case it must be a string and must be multi) or determined from runtime in all other cases. Refactor `loadReleaseFromMetadata` so that it only really reads the metadata, and move the "when metadata does not have architecture, read from runtime" logic to `loadPayloadMetadata` above (the `loadReleaseFromMetadata` caller). This will allow to reuse the release metadata loading code without involving more logic than necessary.

This also simplifies `loadReleaseFromMetadata` signature, improving readability.

Also turn some nested conditionals into guard clauses for readability.

Author: petr-muller

pkg/payload/payload.go

@@ -304,7 +304,7 @@ func ValidateDirectory(dir string) error {
 }
 
 func loadPayloadMetadata(releaseDir, releaseImage string) (*Update, error) {
-	release, arch, err := loadReleaseFromMetadata(releaseDir)
+	release, err := loadReleaseFromMetadata(releaseDir)
 	if err != nil {
 		return nil, err
 	}
@@ -315,6 +315,12 @@ func loadPayloadMetadata(releaseDir, releaseImage string) (*Update, error) {
 		return nil, err
 	}
 
+	arch := string(release.Architecture)
+	if arch == "" {
+		arch = runtime.GOARCH
+		klog.V(2).Infof("Architecture from %s (%s) retrieved from runtime: %q", cincinnatiJSONFile, release.Version, arch)
+	}
+
 	if imageRef.Name != release.Version {
 		return nil, fmt.Errorf("Version from %s (%s) differs from %s (%s)", imageReferencesFile, imageRef.Name, cincinnatiJSONFile, release.Version)
 	}
@@ -352,52 +358,49 @@ func loadPayloadTasks(releaseDir, cvoDir, releaseImage, clusterProfile string) [
 	}}
 }
 
-func loadReleaseFromMetadata(releaseDir string) (configv1.Release, string, error) {
+func loadReleaseFromMetadata(releaseDir string) (configv1.Release, error) {
 	var release configv1.Release
 	path := filepath.Join(releaseDir, cincinnatiJSONFile)
 	data, err := os.ReadFile(path)
 	if err != nil {
-		return release, "", err
+		return release, err
 	}
 
 	var metadata metadata
 	if err := json.Unmarshal(data, &metadata); err != nil {
-		return release, "", fmt.Errorf("unmarshal Cincinnati metadata: %w", err)
+		return release, fmt.Errorf("unmarshal Cincinnati metadata: %w", err)
 	}
 
 	if metadata.Kind != "cincinnati-metadata-v0" {
-		return release, "", fmt.Errorf("unrecognized Cincinnati metadata kind %q", metadata.Kind)
+		return release, fmt.Errorf("unrecognized Cincinnati metadata kind %q", metadata.Kind)
 	}
 
 	if metadata.Version == "" {
-		return release, "", errors.New("missing required Cincinnati metadata version")
+		return release, errors.New("missing required Cincinnati metadata version")
 	}
 
 	if _, err := semver.Parse(metadata.Version); err != nil {
-		return release, "", fmt.Errorf("Cincinnati metadata version %q is not a valid semantic version: %v", metadata.Version, err)
+		return release, fmt.Errorf("Cincinnati metadata version %q is not a valid semantic version: %v", metadata.Version, err)
 	}
 
 	release.Version = metadata.Version
 
-	var arch string
-	if archInterface, ok := metadata.Metadata["release.openshift.io/architecture"]; ok {
-		if archString, ok := archInterface.(string); ok {
-			if archString == releaseMultiArchID {
-				release.Architecture = configv1.ClusterVersionArchitectureMulti
-				arch = string(release.Architecture)
-			} else {
-				return release, "", fmt.Errorf("Architecture from %s (%s) contains invalid value: %q. Valid value is %q.",
-					cincinnatiJSONFile, release.Version, archString, releaseMultiArchID)
-			}
-			klog.V(2).Infof("Architecture from %s (%s) is multi: %q", cincinnatiJSONFile, release.Version, archString)
-		} else {
-			return release, "", fmt.Errorf("Architecture from %s (%s) is not a string: %v",
-				cincinnatiJSONFile, release.Version, archInterface)
+	if archRaw, hasArch := metadata.Metadata["release.openshift.io/architecture"]; hasArch {
+		arch, isString := archRaw.(string)
+		if !isString {
+			return release, fmt.Errorf("Architecture from %s (%s) is not a string: %v",
+				cincinnatiJSONFile, release.Version, archRaw)
 		}
-	} else {
-		arch = runtime.GOARCH
-		klog.V(2).Infof("Architecture from %s (%s) retrieved from runtime: %q", cincinnatiJSONFile, release.Version, arch)
+
+		if arch != releaseMultiArchID {
+			return release, fmt.Errorf("Architecture from %s (%s) contains invalid value: %q. Valid value is %q.",
+				cincinnatiJSONFile, release.Version, arch, releaseMultiArchID)
+		}
+
+		release.Architecture = configv1.ClusterVersionArchitectureMulti
+		klog.V(2).Infof("Architecture from %s (%s) is multi: %q", cincinnatiJSONFile, release.Version, arch)
 	}
+
 	if urlInterface, ok := metadata.Metadata["url"]; ok {
 		if urlString, ok := urlInterface.(string); ok {
 			release.URL = configv1.URL(urlString)
@@ -414,7 +417,7 @@ func loadReleaseFromMetadata(releaseDir string) (configv1.Release, string, error
 		}
 	}
 
-	return release, arch, nil
+	return release, nil
 }
 
 func loadImageReferences(releaseDir string) (*imagev1.ImageStream, error) {