From 14d6a09bbfd6e935f19a09457569ca2e4259bed3 Mon Sep 17 00:00:00 2001 From: Richard Su Date: Fri, 9 Jan 2026 16:50:33 -0600 Subject: [PATCH 1/3] Add mirror-path support and fix custom registry handling Add support for using pre-mirrored images from oc-mirror workspace by specifying the mirror path in appliance-config.yaml. When mirrorPath is configured, the appliance skips running oc-mirror and uses the pre-mirrored registry data directly. Also fixes issues when using custom registries (non-quay.io) where oc adm release info may return incomplete image references missing port and repository path. Adds IDMS entries to ensure the cluster redirects pulls from custom registries to the appliance's internal registry. Changes: - Add MirrorPath field to top level of ApplianceConfig in types - Update appliance-config.yaml template to document the new field - Update code to read mirrorPath from ApplianceConfig.Config.MirrorPath - Add validateMirrorPath() to ApplianceConfig with comprehensive validation - Use pre-mirrored images when mirrorPath is provided - Add fixImageReference() to repair incomplete image refs from oc commands Example: registry.example.com@sha256:... becomes registry.example.com:5000/repo/image@sha256:... - Add addLocalRegistryIDMS() to generate IDMS for custom registry mirrors - Fix release image tag parsing to preserve port in registry URLs The mirror-path can be specified in appliance-config.yaml as: mirrorPath: /path/to/oc-mirror/workspace Assisted-by: Claude Sonnet 4.5 Add debug logging for IsLiveISO tracking --- pkg/asset/config/appliance_config.go | 48 ++++++- pkg/asset/config/env_config.go | 2 + pkg/asset/data/data_iso.go | 35 ++++- pkg/asset/ignition/bootstrap_ignition.go | 1 + pkg/release/release.go | 169 ++++++++++++++++++++--- pkg/types/appliance_config_type.go | 1 + 6 files changed, 238 insertions(+), 18 deletions(-) diff --git a/pkg/asset/config/appliance_config.go b/pkg/asset/config/appliance_config.go index 04bd8046..c00cd878 100644 --- a/pkg/asset/config/appliance_config.go +++ b/pkg/asset/config/appliance_config.go @@ -154,6 +154,12 @@ pullSecret: pull-secret # [Optional] # useBinary: %t +# Path to pre-mirrored images from oc-mirror workspace. +# When provided, skips image mirroring and uses the pre-mirrored registry data. +# The path should point to an oc-mirror workspace directory containing a 'data' subdirectory. +# [Optional] +# mirrorPath: /path/to/mirror/workspace + # Enable all default CatalogSources (on openshift-marketplace namespace). # Should be disabled for disconnected environments. # Default: false @@ -379,7 +385,7 @@ func (a *ApplianceConfig) GetRelease() (string, string, error) { return "", "", nil } releaseDigest = strings.Trim(releaseDigest, "'") - releaseImage = fmt.Sprintf("%s@%s", strings.Split(releaseImage, ":")[0], releaseDigest) + releaseImage = fmt.Sprintf("%s@%s", releaseImage, releaseDigest) } logrus.Debugf("Release image: %s", releaseImage) } @@ -431,6 +437,11 @@ func (a *ApplianceConfig) validateConfig(f asset.FileFetcher) field.ErrorList { } } + // Validate mirrorPath + if err := a.validateMirrorPath(); err != nil { + allErrs = append(allErrs, err...) + } + return allErrs } @@ -554,6 +565,41 @@ func (a *ApplianceConfig) validatePinnedImageSet() error { return nil } +func (a *ApplianceConfig) validateMirrorPath() field.ErrorList { + allErrs := field.ErrorList{} + + if a.Config.MirrorPath != nil { + mirrorPath := swag.StringValue(a.Config.MirrorPath) + if mirrorPath != "" { + // Validate mirror path exists and is a directory + info, err := os.Stat(mirrorPath) + if err != nil { + if os.IsNotExist(err) { + allErrs = append(allErrs, field.Invalid(field.NewPath("mirrorPath"), + mirrorPath, "mirror path does not exist")) + } else { + allErrs = append(allErrs, field.Invalid(field.NewPath("mirrorPath"), + mirrorPath, fmt.Sprintf("failed to access mirror path: %v", err))) + } + } else if !info.IsDir() { + allErrs = append(allErrs, field.Invalid(field.NewPath("mirrorPath"), + mirrorPath, "mirror path must be a directory")) + } else { + // Validate data subdirectory exists + dataDir := filepath.Join(mirrorPath, "data") + if _, err := os.Stat(dataDir); err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("mirrorPath"), + mirrorPath, "mirror path must contain a 'data' subdirectory (expected oc-mirror workspace structure)")) + } + } + + logrus.Infof("Using pre-mirrored images from: %s", mirrorPath) + } + } + + return allErrs +} + func (a *ApplianceConfig) storePullSecret() error { // Get home dir (~) homeDir, err := os.UserHomeDir() diff --git a/pkg/asset/config/env_config.go b/pkg/asset/config/env_config.go index 32a362a9..aba8abc0 100644 --- a/pkg/asset/config/env_config.go +++ b/pkg/asset/config/env_config.go @@ -52,6 +52,8 @@ func (e *EnvConfig) Generate(_ context.Context, dependencies asset.Parents) erro return err } + logrus.Debugf("EnvConfig.Generate() called with AssetsDir='%s', IsLiveISO=%v, DebugBaseIgnition=%v", e.AssetsDir, e.IsLiveISO, e.DebugBaseIgnition) + // Cache dir in 'version-arch' format cacheDirPattern := fmt.Sprintf("%s-%s", applianceConfig.Config.OcpRelease.Version, applianceConfig.GetCpuArchitecture()) diff --git a/pkg/asset/data/data_iso.go b/pkg/asset/data/data_iso.go index 53c9017c..5dab42e9 100644 --- a/pkg/asset/data/data_iso.go +++ b/pkg/asset/data/data_iso.go @@ -9,6 +9,7 @@ import ( "github.com/go-openapi/swag" "github.com/openshift/appliance/pkg/asset/config" "github.com/openshift/appliance/pkg/consts" + "github.com/openshift/appliance/pkg/executer" "github.com/openshift/appliance/pkg/genisoimage" "github.com/openshift/appliance/pkg/log" "github.com/openshift/appliance/pkg/registry" @@ -123,7 +124,39 @@ func (a *DataISO) Generate(_ context.Context, dependencies asset.Parents) error ) spinner.FileToMonitor = dataIsoName imageGen := genisoimage.NewGenIsoImage(nil) - if err = imageGen.GenerateImage(envConfig.CacheDir, dataIsoName, filepath.Join(envConfig.TempDir, dataDir), dataVolumeName); err != nil { + + // When mirror-path is provided, copy the Docker registry data from mirror-path/data + // to temp/data so it's in the same location as the registry container image (images/registry/registry.tar) + registryDataSourcePath := filepath.Join(envConfig.TempDir, dataDir) + if applianceConfig.Config.MirrorPath != nil && swag.StringValue(applianceConfig.Config.MirrorPath) != "" { + mirrorDataPath := filepath.Join(swag.StringValue(applianceConfig.Config.MirrorPath), dataDir) + dockerSrcPath := filepath.Join(mirrorDataPath, "docker") + dockerDstPath := filepath.Join(registryDataSourcePath, "docker") + + logrus.Infof("Copying Docker registry data from %s to %s", dockerSrcPath, dockerDstPath) + + // Validate source directory exists + if _, err := os.Stat(dockerSrcPath); err != nil { + return log.StopSpinner(spinner, fmt.Errorf("Docker registry data not found at %s (mirror-path may be invalid): %w", dockerSrcPath, err)) + } + + // Create destination directory + if err := os.MkdirAll(registryDataSourcePath, os.ModePerm); err != nil { + return log.StopSpinner(spinner, fmt.Errorf("failed to create directory for Docker registry data: %w", err)) + } + + // Copy directory recursively using cp command + // Note: Paths are safe here as they're program-generated from validated inputs + cpCmd := fmt.Sprintf("cp -r %s %s", dockerSrcPath, dockerDstPath) + exec := executer.NewExecuter() + if _, err := exec.Execute(cpCmd); err != nil { + return log.StopSpinner(spinner, fmt.Errorf("failed to copy Docker registry data from %s to %s: %w", dockerSrcPath, dockerDstPath, err)) + } + + logrus.Infof("Successfully copied Docker registry data") + } + + if err = imageGen.GenerateImage(envConfig.CacheDir, dataIsoName, registryDataSourcePath, dataVolumeName); err != nil { return log.StopSpinner(spinner, err) } return log.StopSpinner(spinner, a.updateAsset(envConfig)) diff --git a/pkg/asset/ignition/bootstrap_ignition.go b/pkg/asset/ignition/bootstrap_ignition.go index b6b6d7d7..c5ddd3fc 100644 --- a/pkg/asset/ignition/bootstrap_ignition.go +++ b/pkg/asset/ignition/bootstrap_ignition.go @@ -153,6 +153,7 @@ func (i *BootstrapIgnition) Generate(_ context.Context, dependencies asset.Paren coreosImagePath := envConfig.FindInCache(coreosImagePattern) // Add bootstrap scripts to ignition + logrus.Debugf("BootstrapIgnition rendering templates with IsLiveISO=%v", envConfig.IsLiveISO) templateData := templates.GetBootstrapIgnitionTemplateData( envConfig.IsLiveISO, swag.BoolValue(applianceConfig.Config.EnableInteractiveFlow), diff --git a/pkg/release/release.go b/pkg/release/release.go index 84725132..e1823c1b 100644 --- a/pkg/release/release.go +++ b/pkg/release/release.go @@ -99,9 +99,54 @@ func (r *release) GetImageFromRelease(imageName string) (string, error) { return "", err } + // Fix incomplete image references from local registries + image, err = r.fixImageReference(image, swag.StringValue(r.ApplianceConfig.Config.OcpRelease.URL)) + if err != nil { + return "", err + } + return image, nil } +// fixImageReference repairs incomplete image references returned by oc adm release info +// when querying local registries with custom ports. The oc command may return references +// like "registry.example.com@sha256:..." which are missing the port and repository path. +// This function reconstructs the full reference from the release URL. +func (r *release) fixImageReference(imageRef, releaseURL string) (string, error) { + // Check if this looks like an incomplete reference (has @ but no / after the hostname) + // Example: "virthost.ostest.test.metalkube.org@sha256:abc123" + if strings.Contains(imageRef, "@") && !strings.Contains(strings.Split(imageRef, "@")[0], "/") { + logrus.Debugf("Detected incomplete image reference: %s", imageRef) + + // Extract digest from the incomplete reference + parts := strings.SplitN(imageRef, "@", 2) + if len(parts) != 2 { + return imageRef, nil // Return as-is if we can't parse it + } + digest := parts[1] + + // Extract registry/port/repo from release URL + // Example: "virthost.ostest.test.metalkube.org:5000/openshift/release-images:tag" + // We want: "virthost.ostest.test.metalkube.org:5000/openshift/release-images" + releaseRef := releaseURL + + // Remove tag or digest from release URL + if idx := strings.LastIndex(releaseRef, ":"); idx > strings.LastIndex(releaseRef, "/") { + releaseRef = releaseRef[:idx] + } + if idx := strings.Index(releaseRef, "@"); idx != -1 { + releaseRef = releaseRef[:idx] + } + + // Reconstruct full image reference + fixedRef := fmt.Sprintf("%s@%s", releaseRef, digest) + logrus.Debugf("Fixed image reference: %s -> %s", imageRef, fixedRef) + return fixedRef, nil + } + + return imageRef, nil +} + func (r *release) extractFileFromImage(image, file, outputDir string) (string, error) { cmd := fmt.Sprintf(templateImageExtract, file, outputDir, image) logrus.Debugf("extracting %s to %s, %s", file, outputDir, cmd) @@ -138,35 +183,70 @@ func (r *release) execute(command string) (string, error) { } func (r *release) mirrorImages(imageSetFile, blockedImages, additionalImages, operators string) error { - if err := templates.RenderTemplateFile( - imageSetFile, - templates.GetImageSetTemplateData(r.ApplianceConfig, blockedImages, additionalImages, operators), - r.EnvConfig.TempDir); err != nil { + var tempDir string + + // If a mirror path is provided in appliance-config, use it directly instead of running oc-mirror + var mirrorPath string + if r.ApplianceConfig.Config.MirrorPath != nil { + mirrorPath = *r.ApplianceConfig.Config.MirrorPath + } + + if mirrorPath != "" { + logrus.Infof("Using pre-mirrored images from: %s", mirrorPath) + tempDir = mirrorPath + } else { + // Normal mirroring flow - run oc-mirror + if err := templates.RenderTemplateFile( + imageSetFile, + templates.GetImageSetTemplateData(r.ApplianceConfig, blockedImages, additionalImages, operators), + r.EnvConfig.TempDir); err != nil { + return err + } + + imageSetFilePath, err := filepath.Abs(templates.GetFilePathByTemplate(imageSetFile, r.EnvConfig.TempDir)) + if err != nil { + return err + } + + tempDir = filepath.Join(r.EnvConfig.TempDir, "oc-mirror") + registryPort := swag.IntValue(r.ApplianceConfig.Config.ImageRegistry.Port) + cmd := fmt.Sprintf(ocMirror, imageSetFilePath, registryPort, tempDir) + + logrus.Debugf("Fetching image from OCP release (%s)", cmd) + result, err := r.execute(cmd) + logrus.Debugf("mirroring result: %s", result) + if err != nil { + return err + } + } + + // Copy generated yaml files to cache dir (works for both mirror path and oc-mirror output) + if err := r.copyOutputYamls(tempDir); err != nil { return err } - imageSetFilePath, err := filepath.Abs(templates.GetFilePathByTemplate(imageSetFile, r.EnvConfig.TempDir)) - if err != nil { + // Copy mapping file (works for both mirror path and oc-mirror output) + if err := r.copyMappingFile(tempDir); err != nil { return err } - tempDir := filepath.Join(r.EnvConfig.TempDir, "oc-mirror") - registryPort := swag.IntValue(r.ApplianceConfig.Config.ImageRegistry.Port) - cmd := fmt.Sprintf(ocMirror, imageSetFilePath, registryPort, tempDir) + return nil +} - logrus.Debugf("Fetching image from OCP release (%s)", cmd) - result, err := r.execute(cmd) - logrus.Debugf("mirroring result: %s", result) +func (r *release) copyMappingFile(ocMirrorDir string) error { + mappingFiles, err := filepath.Glob(filepath.Join(ocMirrorDir, fmt.Sprintf("results-*/%s", consts.OcMirrorMappingFileName))) if err != nil { return err } - // Copy generated yaml files to cache dir - if err = r.copyOutputYamls(tempDir); err != nil { - return err + // The slice returned from Glob will have a single filename when running the application, but it will be empty when running the unit-tests since they don't create the files "oc mirror" generates + for _, mappingFile := range mappingFiles { + if err := fileutil.CopyFile(mappingFile, filepath.Join(r.EnvConfig.CacheDir, consts.OcMirrorMappingFileName)); err != nil { + return err + } } - return err + return nil } func (r *release) copyOutputYamls(ocMirrorDir string) error { @@ -186,6 +266,14 @@ func (r *release) copyOutputYamls(ocMirrorDir string) error { internalRegistryURI := fmt.Sprintf("%s:%d", registry.RegistryDomain, registry.RegistryPort) newYaml := strings.ReplaceAll(string(yamlBytes), buildRegistryURI, internalRegistryURI) + // Add IDMS entry for local registry mirror if using a custom release URL + if filepath.Base(yamlPath) == "idms-oc-mirror.yaml" { + newYaml, err = r.addLocalRegistryIDMS(newYaml, internalRegistryURI) + if err != nil { + return err + } + } + // Write edited yamls to cache if err = r.OSInterface.MkdirAll(filepath.Join(r.EnvConfig.CacheDir, consts.OcMirrorResourcesDir), os.ModePerm); err != nil { return err @@ -198,6 +286,55 @@ func (r *release) copyOutputYamls(ocMirrorDir string) error { return nil } +// addLocalRegistryIDMS adds an IDMS entry for the local registry mirror when using +// a custom release URL (not upstream quay.io). This ensures that pulls from the +// registry mirror are redirected to the appliance's internal registry. +func (r *release) addLocalRegistryIDMS(yamlContent, internalRegistryURI string) (string, error) { + releaseURL := swag.StringValue(r.ApplianceConfig.Config.OcpRelease.URL) + + // Check if using a custom registry (not upstream quay.io) + if !strings.Contains(releaseURL, "quay.io") && !strings.Contains(releaseURL, "registry.ci.openshift.org") { + // Extract registry host:port from release URL + localRegistry := releaseURL + // Remove digest if present + if idx := strings.Index(localRegistry, "@"); idx != -1 { + localRegistry = localRegistry[:idx] + } + // Remove tag if present (after last colon that comes after last slash) + if lastSlash := strings.LastIndex(localRegistry, "/"); lastSlash != -1 { + if lastColon := strings.LastIndex(localRegistry[lastSlash:], ":"); lastColon != -1 { + localRegistry = localRegistry[:lastSlash+lastColon] + } + } + + // Extract just the registry host:port (without repository path) + registryHost := localRegistry + if idx := strings.Index(localRegistry, "/"); idx != -1 { + registryHost = localRegistry[:idx] + } + + logrus.Infof("Adding IDMS entry for registry mirror: %s -> %s", registryHost, internalRegistryURI) + + // Append IDMS entry for registry mirror + // This maps all pulls from the registry mirror to the appliance's internal registry + additionalIDMS := fmt.Sprintf(`--- +apiVersion: config.openshift.io/v1 +kind: ImageDigestMirrorSet +metadata: + name: local-registry-mirror +spec: + imageDigestMirrors: + - mirrors: + - %s + source: %s +`, internalRegistryURI, registryHost) + + return yamlContent + "\n" + additionalIDMS, nil + } + + return yamlContent, nil +} + func (r *release) generateImagesList(images *[]types.Image) string { if images == nil { return "" diff --git a/pkg/types/appliance_config_type.go b/pkg/types/appliance_config_type.go index e011416a..ff650d55 100644 --- a/pkg/types/appliance_config_type.go +++ b/pkg/types/appliance_config_type.go @@ -18,6 +18,7 @@ type ApplianceConfig struct { SshKey *string `json:"sshKey"` UserCorePass *string `json:"userCorePass"` ImageRegistry *ImageRegistry `json:"imageRegistry"` + MirrorPath *string `json:"mirrorPath,omitempty"` EnableDefaultSources *bool `json:"enableDefaultSources"` EnableFips *bool `json:"enableFips"` StopLocalRegistry *bool `json:"stopLocalRegistry"` From d1df74e6d3c8b3d572f212c8ef2682f2f53645dd Mon Sep 17 00:00:00 2001 From: Richard Su Date: Fri, 9 Jan 2026 16:51:56 -0600 Subject: [PATCH 2/3] Add --registry-config to oc commands for custom registry authentication When using custom registries instead of official OpenShift release images, oc commands need authentication credentials to pull images. This adds the --registry-config flag to all oc commands and --authfile to skopeo. Changes: - Add --registry-config to command templates (cleaner than string replacement) - templateGetImage: oc adm release info - templateExtractCmd: oc adm release extract - ocMirror: oc mirror - Add --authfile to skopeo copy command - Store pull secret in temporary file (instead of ~/.docker/config.json) - Add GetPullSecretPath() and CleanupPullSecret() methods to ApplianceConfig - Add setupApplianceConfig() helper to ensure cleanup in all build commands - Automatic cleanup via defer prevents credential leakage This allows the appliance builder to work with images hosted in private registries that require authentication, not just public quay.io releases. Assisted-by: Claude Sonnet 4.5 --- cmd/build.go | 31 +++++++++++++ pkg/asset/config/appliance_config.go | 68 ++++++++++++++++++++-------- pkg/asset/deploy/deploy_iso.go | 3 +- pkg/registry/registry.go | 4 +- pkg/release/release.go | 22 +++++---- pkg/release/release_test.go | 3 +- pkg/skopeo/skopeo.go | 8 ++-- pkg/skopeo/skopeo_test.go | 8 ++-- 8 files changed, 111 insertions(+), 36 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index f5531686..1e8be7a4 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -94,6 +94,10 @@ func runBuild(cmd *cobra.Command, args []string) { cleanup := log.SetupFileHook(rootOpts.dir) defer cleanup() + // Load ApplianceConfig and setup cleanup of pull secret temp file + _, cleanupPullSecret := setupApplianceConfig(cmd) + defer cleanupPullSecret() + // Load ApplianceDiskImage asset to check whether a clean is required applianceDiskImage := appliance.ApplianceDiskImage{} if asset, err := getAssetStore().Load(&applianceDiskImage); err == nil && asset != nil { @@ -135,6 +139,10 @@ func runBuildISO(cmd *cobra.Command, args []string) { cleanup := log.SetupFileHook(rootOpts.dir) defer cleanup() + // Load ApplianceConfig and setup cleanup of pull secret temp file + _, cleanupPullSecret := setupApplianceConfig(cmd) + defer cleanupPullSecret() + // Generate DeployConfig asset if err := getAssetStore().Fetch(cmd.Context(), deployConfig); err != nil { logrus.Fatal(err) @@ -160,6 +168,10 @@ func runBuildUpgradeISO(cmd *cobra.Command, args []string) { cleanup := log.SetupFileHook(rootOpts.dir) defer cleanup() + // Load ApplianceConfig and setup cleanup of pull secret temp file + _, cleanupPullSecret := setupApplianceConfig(cmd) + defer cleanupPullSecret() + // Generate UpgradeConfig asset if err := getAssetStore().Fetch(cmd.Context(), deployConfig); err != nil { logrus.Fatal(err) @@ -190,6 +202,10 @@ func runBuildLiveISO(cmd *cobra.Command, args []string) { cleanup := log.SetupFileHook(rootOpts.dir) defer cleanup() + // Load ApplianceConfig and setup cleanup of pull secret temp file + _, cleanupPullSecret := setupApplianceConfig(cmd) + defer cleanupPullSecret() + // Load ApplianceLiveISO asset to check whether a clean is required applianceLiveISO := appliance.ApplianceLiveISO{} if asset, err := getAssetStore().Load(&applianceLiveISO); err == nil && asset != nil { @@ -253,3 +269,18 @@ func getAssetStore() asset.Store { } return assetStore } + +func setupApplianceConfig(cmd *cobra.Command) (*config.ApplianceConfig, func()) { + applianceConfig := &config.ApplianceConfig{} + if err := getAssetStore().Fetch(cmd.Context(), applianceConfig); err != nil { + logrus.Fatal(errors.Wrapf(err, "failed to fetch %s", applianceConfig.Name())) + } + + cleanup := func() { + if err := applianceConfig.CleanupPullSecret(); err != nil { + logrus.Warnf("Failed to cleanup pull secret: %v", err) + } + } + + return applianceConfig, cleanup +} diff --git a/pkg/asset/config/appliance_config.go b/pkg/asset/config/appliance_config.go index c00cd878..d8ccfcb9 100644 --- a/pkg/asset/config/appliance_config.go +++ b/pkg/asset/config/appliance_config.go @@ -48,8 +48,8 @@ const ( PodmanPull = "podman pull %s" // Release - templateGetVersion = "oc adm release info %s -o template --template '{{.metadata.version}}'" - templateGetDigest = "oc adm release info %s -o template --template '{{.digest}}'" + templateGetVersion = "oc adm release info --registry-config %s %s -o template --template '{{.metadata.version}}'" + templateGetDigest = "oc adm release info --registry-config %s %s -o template --template '{{.digest}}'" ) var ( @@ -59,9 +59,11 @@ var ( // ApplianceConfig reads the appliance-config.yaml file. type ApplianceConfig struct { - File *asset.File - Config *types.ApplianceConfig - Template string + File *asset.File + Config *types.ApplianceConfig + Template string + pullSecretPath string + pullSecretFile *os.File } var _ asset.WritableAsset = (*ApplianceConfig)(nil) @@ -368,18 +370,19 @@ func (a *ApplianceConfig) GetRelease() (string, string, error) { releaseImage = swag.StringValue(a.Config.OcpRelease.URL) // Get version - cmd := fmt.Sprintf(templateGetVersion, releaseImage) + cmd := fmt.Sprintf(templateGetVersion, a.pullSecretPath, releaseImage) releaseVersion, err = executer.NewExecuter().Execute(cmd) if err != nil { + logrus.Debugf("Error executing command: %s, error: %v", cmd, err) return "", "", nil } releaseVersion = strings.Trim(releaseVersion, "'") - logrus.Debugf("Release version: %s", releaseVersion) + logrus.Debugf("Got release version: '%s'", releaseVersion) // Get image if !strings.Contains(releaseImage, "@") { var releaseDigest string - cmd := fmt.Sprintf(templateGetDigest, releaseImage) + cmd := fmt.Sprintf(templateGetDigest, a.pullSecretPath, releaseImage) releaseDigest, err = executer.NewExecuter().Execute(cmd) if err != nil { return "", "", nil @@ -601,22 +604,51 @@ func (a *ApplianceConfig) validateMirrorPath() field.ErrorList { } func (a *ApplianceConfig) storePullSecret() error { - // Get home dir (~) - homeDir, err := os.UserHomeDir() + // Create temporary file for pull secret + tmpFile, err := os.CreateTemp("", "appliance-pull-secret-*.json") if err != nil { - return errors.Wrapf(err, "failed to get home directory") + return errors.Wrap(err, "failed to create temporary file for pull secret") } - // Create sub dirs - configPath := filepath.Join(homeDir, ".docker", "config.json") - if err = os.MkdirAll(filepath.Dir(configPath), os.ModePerm); err != nil { - return err + // Write pull secret to temp file + if _, err = tmpFile.WriteString(a.Config.PullSecret); err != nil { + tmpFile.Close() + os.Remove(tmpFile.Name()) + return errors.Wrap(err, "failed to write pull secret to temporary file") } - // Write pull secret - if err = os.WriteFile(configPath, []byte(a.Config.PullSecret), os.ModePerm); err != nil { - return errors.Wrap(err, "failed to write file") + if err = tmpFile.Close(); err != nil { + os.Remove(tmpFile.Name()) + return errors.Wrap(err, "failed to close temporary pull secret file") } + a.pullSecretPath = tmpFile.Name() + logrus.Debugf("Pull secret successfully written to temporary file: %s", a.pullSecretPath) + + return nil +} + +// GetPullSecretPath returns the path to the temporary pull secret file +// If the path is empty (e.g., after loading from state), it recreates the temp file +func (a *ApplianceConfig) GetPullSecretPath() string { + if a.pullSecretPath == "" && a.Config != nil { + logrus.Debugf("Pull secret path empty, recreating temporary file") + if err := a.storePullSecret(); err != nil { + logrus.Warnf("Failed to recreate pull secret temp file: %v", err) + return "" + } + } + return a.pullSecretPath +} + +// CleanupPullSecret removes the temporary pull secret file +func (a *ApplianceConfig) CleanupPullSecret() error { + if a.pullSecretPath != "" { + logrus.Debugf("Cleaning up temporary pull secret file: %s", a.pullSecretPath) + if err := os.Remove(a.pullSecretPath); err != nil && !os.IsNotExist(err) { + return errors.Wrapf(err, "failed to remove temporary pull secret file: %s", a.pullSecretPath) + } + a.pullSecretPath = "" + } return nil } diff --git a/pkg/asset/deploy/deploy_iso.go b/pkg/asset/deploy/deploy_iso.go index f91b25a0..b7ed63e8 100644 --- a/pkg/asset/deploy/deploy_iso.go +++ b/pkg/asset/deploy/deploy_iso.go @@ -127,8 +127,9 @@ func (i *DeployISO) buildDeploymentIso(envConfig *config.EnvConfig, applianceCon envConfig, ) applianceTarFile := filepath.Join(deployDir, consts.ApplianceImageTar) + authFile := applianceConfig.GetPullSecretPath() if err = skopeo.NewSkopeo(nil).CopyToFile( - consts.ApplianceImage, consts.ApplianceImageName, applianceTarFile); err != nil { + consts.ApplianceImage, consts.ApplianceImageName, applianceTarFile, authFile); err != nil { return err } diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go index 2863f53c..57e409a1 100644 --- a/pkg/registry/registry.go +++ b/pkg/registry/registry.go @@ -298,10 +298,12 @@ func CopyRegistryImageIfNeeded(envConfig *config.EnvConfig, applianceConfig *con // Pull the source registry image (docker-registry from OCP release or from appliance config) // and copy it to dir format to preserve digests logrus.Infof("Copying registry image from %s to %s", sourceRegistryUri, consts.RegistryImage) + authFile := applianceConfig.GetPullSecretPath() if err := skopeo.NewSkopeo(nil).CopyToFile( sourceRegistryUri, consts.RegistryImage, - fileInCachePath); err != nil { + fileInCachePath, + authFile); err != nil { return "", err } } diff --git a/pkg/release/release.go b/pkg/release/release.go index e1823c1b..3c0469ec 100644 --- a/pkg/release/release.go +++ b/pkg/release/release.go @@ -32,10 +32,10 @@ const ( ) const ( - templateGetImage = "oc adm release info --image-for=%s --insecure=%t %s" - templateExtractCmd = "oc adm release extract --command=%s --to=%s %s" - templateImageExtract = "oc image extract --path %s:%s --confirm %s" - ocMirror = "oc mirror --v2 --config=%s docker://127.0.0.1:%d --workspace=file://%s --src-tls-verify=false --dest-tls-verify=false --parallel-images=1 --parallel-layers=1 --retry-times=5" + templateGetImage = "oc adm release info --registry-config %s --image-for=%s %s" + templateExtractCmd = "oc adm release extract --registry-config %s --command=%s --to=%s %s" + templateImageExtract = "oc image extract --registry-config %s --path %s:%s --confirm %s" + ocMirror = "oc mirror --v2 --authfile %s --config=%s docker://127.0.0.1:%d --workspace=file://%s --src-tls-verify=false --dest-tls-verify=false --parallel-images=1 --parallel-layers=1 --retry-times=5" // ocMirrorDryRun is the command template for running oc mirror in dry-run mode to generate mapping.txt ocMirrorDryRun = "oc mirror --v2 --config=%s docker://127.0.0.1:%d --workspace=file://%s --src-tls-verify=false --dest-tls-verify=false --dry-run" ) @@ -91,7 +91,8 @@ func (r *release) ExtractFile(image string, filename string) (string, error) { } func (r *release) GetImageFromRelease(imageName string) (string, error) { - cmd := fmt.Sprintf(templateGetImage, imageName, true, swag.StringValue(r.ApplianceConfig.Config.OcpRelease.URL)) + configPath := r.ApplianceConfig.GetPullSecretPath() + cmd := fmt.Sprintf(templateGetImage, configPath, imageName, swag.StringValue(r.ApplianceConfig.Config.OcpRelease.URL)) logrus.Debugf("Fetching image from OCP release (%s)", cmd) image, err := r.execute(cmd) @@ -148,7 +149,8 @@ func (r *release) fixImageReference(imageRef, releaseURL string) (string, error) } func (r *release) extractFileFromImage(image, file, outputDir string) (string, error) { - cmd := fmt.Sprintf(templateImageExtract, file, outputDir, image) + configPath := r.ApplianceConfig.GetPullSecretPath() + cmd := fmt.Sprintf(templateImageExtract, configPath, file, outputDir, image) logrus.Debugf("extracting %s to %s, %s", file, outputDir, cmd) _, err := retry.Do(OcDefaultTries, OcDefaultRetryDelay, r.execute, cmd) if err != nil { @@ -165,7 +167,10 @@ func (r *release) extractFileFromImage(image, file, outputDir string) (string, e } func (r *release) ExtractCommand(command string, dest string) (string, error) { - cmd := fmt.Sprintf(templateExtractCmd, command, dest, *r.ApplianceConfig.Config.OcpRelease.URL) + releaseURL := *r.ApplianceConfig.Config.OcpRelease.URL + configPath := r.ApplianceConfig.GetPullSecretPath() + cmd := fmt.Sprintf(templateExtractCmd, configPath, command, dest, releaseURL) + logrus.Debugf("extracting %s to %s, %s", command, dest, cmd) stdout, err := r.execute(cmd) if err != nil { @@ -210,7 +215,8 @@ func (r *release) mirrorImages(imageSetFile, blockedImages, additionalImages, op tempDir = filepath.Join(r.EnvConfig.TempDir, "oc-mirror") registryPort := swag.IntValue(r.ApplianceConfig.Config.ImageRegistry.Port) - cmd := fmt.Sprintf(ocMirror, imageSetFilePath, registryPort, tempDir) + configPath := r.ApplianceConfig.GetPullSecretPath() + cmd := fmt.Sprintf(ocMirror, configPath, imageSetFilePath, registryPort, tempDir) logrus.Debugf("Fetching image from OCP release (%s)", cmd) result, err := r.execute(cmd) diff --git a/pkg/release/release_test.go b/pkg/release/release_test.go index 0d7e09b9..4b321ff1 100644 --- a/pkg/release/release_test.go +++ b/pkg/release/release_test.go @@ -115,7 +115,8 @@ var _ = Describe("Test Release", func() { It("GetImageFromRelease - success", func() { imageName := "machine-os-images" - cmd := fmt.Sprintf(templateGetImage, imageName, true, swag.StringValue(applianceConfig.Config.OcpRelease.URL)) + pullSecretPath := applianceConfig.GetPullSecretPath() + cmd := fmt.Sprintf(templateGetImage, pullSecretPath, imageName, swag.StringValue(applianceConfig.Config.OcpRelease.URL)) mockExecuter.EXPECT().Execute(cmd).Return("", nil).Times(1) _, err := testRelease.GetImageFromRelease(imageName) diff --git a/pkg/skopeo/skopeo.go b/pkg/skopeo/skopeo.go index e7d34ec3..3173cde5 100644 --- a/pkg/skopeo/skopeo.go +++ b/pkg/skopeo/skopeo.go @@ -26,11 +26,11 @@ const ( // // dir: format: Stores the image as a directory structure instead of a tar archive // This format preserves all image metadata and supports podman pull dir: for loading - templateCopyToFile = "skopeo copy --all --preserve-digests docker://%s dir:%s" + templateCopyToFile = "skopeo copy --authfile %s --all --preserve-digests docker://%s dir:%s" ) type Skopeo interface { - CopyToFile(imageUrl, imageName, filePath string) error + CopyToFile(imageUrl, imageName, filePath, authFile string) error } type skopeo struct { @@ -47,11 +47,11 @@ func NewSkopeo(exec executer.Executer) Skopeo { } } -func (s *skopeo) CopyToFile(imageUrl, imageName, filePath string) error { +func (s *skopeo) CopyToFile(imageUrl, imageName, filePath, authFile string) error { if err := os.MkdirAll(filepath.Dir(filePath), os.ModePerm); err != nil { return err } - _, err := s.executer.Execute(fmt.Sprintf(templateCopyToFile, imageUrl, filePath)) + _, err := s.executer.Execute(fmt.Sprintf(templateCopyToFile, authFile, imageUrl, filePath)) return err } diff --git a/pkg/skopeo/skopeo_test.go b/pkg/skopeo/skopeo_test.go index f30604d6..2132247d 100644 --- a/pkg/skopeo/skopeo_test.go +++ b/pkg/skopeo/skopeo_test.go @@ -29,18 +29,20 @@ var _ = Describe("Test Skopeo", func() { It("skopeo CopyToFile - success", func() { fakePath := "path/to/registry" - cmd := fmt.Sprintf(templateCopyToFile, consts.RegistryImage, fakePath) + fakeAuthFile := "/tmp/pull-secret.json" + cmd := fmt.Sprintf(templateCopyToFile, fakeAuthFile, consts.RegistryImage, fakePath) mockExecuter.EXPECT().Execute(cmd).Return("", nil).Times(1) - err := testSkopeo.CopyToFile(consts.RegistryImage, consts.RegistryImage, fakePath) + err := testSkopeo.CopyToFile(consts.RegistryImage, consts.RegistryImage, fakePath, fakeAuthFile) Expect(err).ToNot(HaveOccurred()) }) It("skopeo CopyToFile - failure", func() { fakePath := "path/to/registry" + fakeAuthFile := "/tmp/pull-secret.json" mockExecuter.EXPECT().Execute(gomock.Any()).Return("", errors.New("some error")).Times(1) - err := testSkopeo.CopyToFile(consts.RegistryImage, consts.RegistryImage, fakePath) + err := testSkopeo.CopyToFile(consts.RegistryImage, consts.RegistryImage, fakePath, fakeAuthFile) Expect(err).To(HaveOccurred()) }) }) From 5a6299ff26fd2197da829b6f038a8d6949e68ae2 Mon Sep 17 00:00:00 2001 From: Richard Su Date: Wed, 21 Jan 2026 10:10:09 -0600 Subject: [PATCH 3/3] Use singleton asset store to preserve EnvConfig state The issue was that getAssetStore() created a new store instance on each call. When setupApplianceConfig() called it, the new store instance would trigger EnvConfig regeneration with default values (IsLiveISO=false, AssetsDir=''), overwriting the correct values set in preRunBuild(). By caching the asset store instance and reusing it throughout the command execution, we ensure EnvConfig is only generated once with the correct values and not overwritten. This fixes the live ISO boot failure where mount-agent-data.sh would try to mount /dev/disk/by-partlabel/agentdata (disk image mode) instead of using the ISO mount strategy (live ISO mode). Assisted-by: Claude Sonnet 4.5 --- cmd/build.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index 1e8be7a4..303ae850 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -28,6 +28,12 @@ var ( envConfig config.EnvConfig deployConfig *config.DeployConfig + + // assetStoreInstance caches the asset store to ensure the same instance + // is reused throughout command execution. This prevents EnvConfig from + // being regenerated with default values when setupApplianceConfig() fetches + // ApplianceConfig, which would overwrite runtime flags like IsLiveISO. + assetStoreInstance asset.Store ) func NewBuildCmd() *cobra.Command { @@ -263,11 +269,14 @@ func preRunBuildLiveISO(cmd *cobra.Command, args []string) { } func getAssetStore() asset.Store { - assetStore, err := assetstore.NewStore(rootOpts.dir) - if err != nil { - logrus.Fatal(errors.Wrap(err, "failed to create asset store")) + if assetStoreInstance == nil { + assetStore, err := assetstore.NewStore(rootOpts.dir) + if err != nil { + logrus.Fatal(errors.Wrap(err, "failed to create asset store")) + } + assetStoreInstance = assetStore } - return assetStore + return assetStoreInstance } func setupApplianceConfig(cmd *cobra.Command) (*config.ApplianceConfig, func()) {