Skip to content

OCPBUGS-43501: Handle kargs.json correctly in minimal ISO #618

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.

Sign up for GitHub

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

Open
zaneb wants to merge 5 commits into
base: main
Choose a base branch
from
Open

OCPBUGS-43501: Handle kargs.json correctly in minimal ISO #618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e02ee1d
Use same quoting in isolinux.cfg as in grub.cfg
zaneb Aug 6, 2025
2f362d7
refactor: Edit minimal ISO kernel args in a single place
zaneb Dec 10, 2025
2771881
Narrow down locus of changes when editing minimal ISO files
zaneb Dec 10, 2025
5944696
Update default kargs in minimal ISO
zaneb Dec 10, 2025
b139cee
OCPBUGS-43501: Set correct kargs.json offsets in minimal ISO
zaneb Dec 10, 2025
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
272 changes: 232 additions & 40 deletions pkg/isoeditor/rhcos.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package isoeditor

import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"regexp"
"strings"

"github.com/openshift/assisted-image-service/internal/common"
log "github.com/sirupsen/logrus"
Expand All @@ -19,6 +21,36 @@ const (
RootfsImagePath = "images/pxeboot/rootfs.img"
)

// transformKernelArgs applies the standard kernel argument transformations:
// 1. Remove coreos.liveiso parameter
// 2. Add coreos.live.rootfs_url parameter at the specified insertion point
func transformKernelArgs(content string, insertionPattern string, rootFSURL string, fileEntry *kargsFileEntry) (string, error) {
// Validate rootfs URL
if strings.Contains(rootFSURL, "$") {
return "", fmt.Errorf("invalid rootfs URL: contains invalid character '$'")
}
if strings.Contains(rootFSURL, "\\") {
return "", fmt.Errorf("invalid rootfs URL: contains invalid character '\\'")
}

var err error

// Remove the coreos.liveiso parameter
content, err = editString(content, `\b(?P<replace>coreos\.liveiso=\S+ ?)`, "", fileEntry)
if err != nil {
return "", err
}

// Add the rootfs_url parameter at the specified insertion point
replacement := " coreos.live.rootfs_url=\"" + rootFSURL + "\""
content, err = editString(content, insertionPattern, replacement, fileEntry)
if err != nil {
return "", err
}

return content, nil
}

//go:generate mockgen -package=isoeditor -destination=mock_editor.go . Editor
type Editor interface {
CreateMinimalISOTemplate(fullISOPath, rootFSURL, arch, minimalISOPath, openshiftVersion, nmstatectlPath string) error
Expand Down Expand Up @@ -52,19 +84,11 @@ func CreateMinimalISO(extractDir, volumeID, rootFSURL, arch, minimalISOPath stri
includeNmstateRamDisk = true
}

if err := fixGrubConfig(rootFSURL, extractDir, includeNmstateRamDisk); err != nil {
log.WithError(err).Warnf("Failed to edit grub config")
if err := updateKargs(extractDir, rootFSURL, includeNmstateRamDisk, arch); err != nil {
log.WithError(err).Warnf("Failed to update kargs offsets and sizes")
return err
}

// ignore isolinux.cfg for ppc64le because it doesn't exist
if arch != "ppc64le" {
if err := fixIsolinuxConfig(rootFSURL, extractDir, includeNmstateRamDisk); err != nil {
log.WithError(err).Warnf("Failed to edit isolinux config")
return err
}
}

if err := Create(minimalISOPath, extractDir, volumeID); err != nil {
return err
}
Expand Down Expand Up @@ -148,79 +172,247 @@ func embedInitrdPlaceholders(extractDir string) error {
return nil
}

func fixGrubConfig(rootFSURL, extractDir string, includeNmstateRamDisk bool) error {
// fixGrubConfig modifies grub.cfg and updates kargs config in place
func fixGrubConfig(rootFSURL, extractDir string, includeNmstateRamDisk bool, kargs *kargsConfig) error {
availableGrubPaths := []string{"EFI/redhat/grub.cfg", "EFI/fedora/grub.cfg", "boot/grub/grub.cfg", "EFI/centos/grub.cfg"}
var foundGrubPath string
var fileEntry *kargsFileEntry
for _, pathSection := range availableGrubPaths {
path := filepath.Join(extractDir, pathSection)
if _, err := os.Stat(path); err == nil {
foundGrubPath = path
fileEntry = kargs.FindFileByPath(pathSection)
break
}
}
if len(foundGrubPath) == 0 {
return fmt.Errorf("no grub.cfg found, possible paths are %v", availableGrubPaths)
}

// Add the rootfs url
replacement := fmt.Sprintf("$1 $2 'coreos.live.rootfs_url=%s'", rootFSURL)
if err := editFile(foundGrubPath, `(?m)^(\s+linux) (.+| )+$`, replacement); err != nil {
// Read the file content
content, err := os.ReadFile(foundGrubPath)
if err != nil {
return err
}
contentStr := string(content)

// Remove the coreos.liveiso parameter
if err := editFile(foundGrubPath, ` coreos.liveiso=\S+`, ""); err != nil {
// Typical grub.cfg lines we're modifying:
//
// linux /images/pxeboot/vmlinuz rw coreos.liveiso=rhcos-9.6.20250523-0 ignition.firstboot ignition.platform.id=metal
// initrd /images/pxeboot/initrd.img /images/ignition.img

// Apply standard kernel argument transformations (remove coreos.liveiso, add rootfs_url)
contentStr, err = transformKernelArgs(contentStr, `(?m)^(\s+linux .+)(?P<replace>$)`, rootFSURL, fileEntry)
if err != nil {
return err
}

// Edit config to add custom ramdisk image to initrd
// Edit config to add custom ramdisk image to initrd - capture the end-of-line position to append our images
var initrdReplacement string
if includeNmstateRamDisk {
if err := editFile(foundGrubPath, `(?m)^(\s+initrd) (.+| )+$`, fmt.Sprintf("$1 $2 %s %s", ramDiskImagePath, nmstateDiskImagePath)); err != nil {
return err
}
initrdReplacement = " " + ramDiskImagePath + " " + nmstateDiskImagePath
} else {
if err := editFile(foundGrubPath, `(?m)^(\s+initrd) (.+| )+$`, fmt.Sprintf("$1 $2 %s", ramDiskImagePath)); err != nil {
return err
}
initrdReplacement = " " + ramDiskImagePath
}
contentStr, err = editString(contentStr, `(?m)^(\s+initrd .+)(?P<replace>$)`, initrdReplacement, fileEntry)
if err != nil {
return err
}

return nil
// Write the modified content back to the file
return os.WriteFile(foundGrubPath, []byte(contentStr), 0600)
}

func fixIsolinuxConfig(rootFSURL, extractDir string, includeNmstateRamDisk bool) error {
replacement := fmt.Sprintf("$1 $2 coreos.live.rootfs_url=%s", rootFSURL)
if err := editFile(filepath.Join(extractDir, "isolinux/isolinux.cfg"), `(?m)^(\s+append) (.+| )+$`, replacement); err != nil {
// fixIsolinuxConfig modifies isolinux.cfg and updates kargs config in place
func fixIsolinuxConfig(rootFSURL, extractDir string, includeNmstateRamDisk bool, kargs *kargsConfig) error {
relativeIsolinuxPath := strings.TrimPrefix(defaultIsolinuxFilePath, "/")
isolinuxPath := filepath.Join(extractDir, relativeIsolinuxPath)

// Read the file content
content, err := os.ReadFile(isolinuxPath)
if err != nil {
return err
}
contentStr := string(content)

if err := editFile(filepath.Join(extractDir, "isolinux/isolinux.cfg"), ` coreos.liveiso=\S+`, ""); err != nil {
// Typical isolinux.cfg line we're modifying:
//
// append initrd=/images/pxeboot/initrd.img,/images/ignition.img rw coreos.liveiso=rhcos-9.6.20250523-0 ignition.firstboot ignition.platform.id=metal

// Find the kargs file entry for this file
fileEntry := kargs.FindFileByPath(relativeIsolinuxPath)

// Apply standard kernel argument transformations (remove coreos.liveiso, add rootfs_url)
contentStr, err = transformKernelArgs(contentStr, `(?m)^(\s+append .+)(?P<replace>$)`, rootFSURL, fileEntry)
if err != nil {
return err
}

// Add ramdisk images to initrd specification - capture the position right after the initrd argument to append our images
var initrdReplacement string
if includeNmstateRamDisk {
if err := editFile(filepath.Join(extractDir, "isolinux/isolinux.cfg"), `(?m)^(\s+append.*initrd=\S+) (.*)$`, fmt.Sprintf("${1},%s,%s ${2}", ramDiskImagePath, nmstateDiskImagePath)); err != nil {
return err
}
initrdReplacement = "," + ramDiskImagePath + "," + nmstateDiskImagePath
} else {
if err := editFile(filepath.Join(extractDir, "isolinux/isolinux.cfg"), `(?m)^(\s+append.*initrd=\S+) (.*)$`, fmt.Sprintf("${1},%s ${2}", ramDiskImagePath)); err != nil {
return err
}
initrdReplacement = "," + ramDiskImagePath
}
contentStr, err = editString(contentStr, `(?m)^(\s+append.*initrd=\S+)(?P<replace>)`, initrdReplacement, fileEntry)
if err != nil {
return err
}

// Write the modified content back to the file
return os.WriteFile(isolinuxPath, []byte(contentStr), 0600)
}

// editString applies a regex replacement to a string and returns the modified string
// It looks for a named capture group called "replace" and replaces only that content, using precise string manipulation
func editString(content string, reString string, replacement string, fileEntry *kargsFileEntry) (string, error) {
re := regexp.MustCompile(reString)

// Get the index of the "replace" named capture group
replaceIndex := re.SubexpIndex("replace")
if replaceIndex == -1 {
return "", fmt.Errorf("regex must have a named capture group called 'replace'")
}

// Find the first match with subgroups
submatchIndexes := re.FindStringSubmatchIndex(content)
if submatchIndexes == nil {
// No match found
return content, nil
}

// submatchIndexes contains [fullMatchStart, fullMatchEnd, group1Start, group1End, group2Start, group2End, ...]
if len(submatchIndexes) < (replaceIndex+1)*2 {
return "", fmt.Errorf("regex match does not contain the 'replace' capture group")
}

replaceStart := submatchIndexes[replaceIndex*2]
replaceEnd := submatchIndexes[replaceIndex*2+1]

// Replace only the "replace" capturing group
newContent := content[:replaceStart] + replacement + content[replaceEnd:]

if content == newContent {
return content, nil
}

if fileEntry == nil || fileEntry.Offset == nil {
return newContent, nil
}

embedStart := *fileEntry.Offset
fileSizeChange := int64(len(newContent)) - int64(len(content))

replaceStartPos := int64(replaceStart)
replaceEndPos := int64(replaceEnd)

// Add boundary crossing check to ensure no replacements span across the embed area start boundary
if replaceStartPos < embedStart && replaceEndPos > embedStart {
return "", fmt.Errorf("replacement spans across embed area boundary (replace: %d-%d, embed starts: %d)", replaceStartPos, replaceEndPos, embedStart)
}

if replaceEndPos <= embedStart {
// Change is before embed area - affects offset
*fileEntry.Offset += fileSizeChange
}

return newContent, nil
}

type kargsFileEntry struct {
End *string `json:"end,omitempty"`
Offset *int64 `json:"offset,omitempty"`
Pad *string `json:"pad,omitempty"`
Path *string `json:"path,omitempty"`
}

type kargsConfig struct {
Default string `json:"default"`
Files []kargsFileEntry `json:"files"`
Size int64 `json:"size"`
}

// FindFileByPath searches for a file entry by its path and returns a pointer to it.
// Returns nil if no file with the specified path is found.
func (k *kargsConfig) FindFileByPath(path string) *kargsFileEntry {
if k == nil {
return nil
}
for i := range k.Files {
if k.Files[i].Path != nil && *k.Files[i].Path == path {
return &k.Files[i]
}
}
return nil
}

func editFile(fileName string, reString string, replacement string) error {
content, err := os.ReadFile(fileName)
// updateDefaultKargs modifies the default kernel arguments to match bootloader modifications
// and applies the embed area size change directly to config.Size
func updateDefaultKargs(config *kargsConfig, rootFSURL string) error {
originalDefault := config.Default

// Apply the same transformations we make to bootloader configs
// For default kargs, we append at the end (using $ insertion pattern)
// Pass nil for fileEntry since we don't track offsets for the default string
var err error
config.Default, err = transformKernelArgs(config.Default, `(?P<replace>$)`, rootFSURL, nil)
if err != nil {
return err
}

re := regexp.MustCompile(reString)
newContent := re.ReplaceAllString(string(content), replacement)
// Calculate and apply the embed area size change from the default kargs transformation
embedSizeChange := int64(len(config.Default)) - int64(len(originalDefault))
config.Size += embedSizeChange

if err := os.WriteFile(fileName, []byte(newContent), 0600); err != nil {
return err
return nil
}

// updateKargs reads kargs.json, applies fixes with embed area awareness, and updates kargs.json
func updateKargs(extractDir, rootFSURL string, includeNmstateRamDisk bool, arch string) error {
kargsPath := filepath.Join(extractDir, "coreos/kargs.json")

var config *kargsConfig

if _, err := os.Stat(kargsPath); !os.IsNotExist(err) {
kargsData, err := os.ReadFile(kargsPath)
if err != nil {
return fmt.Errorf("failed to read kargs.json: %v", err)
}

var kargsStruct kargsConfig
if err := json.Unmarshal(kargsData, &kargsStruct); err != nil {
return fmt.Errorf("failed to unmarshal kargs.json: %v", err)
}
config = &kargsStruct
}

// Apply bootloader config changes (without tracking size changes)
if err := fixGrubConfig(rootFSURL, extractDir, includeNmstateRamDisk, config); err != nil {
return fmt.Errorf("failed to fix grub config: %v", err)
}

// ignore isolinux.cfg for ppc64le because it doesn't exist
if arch != "ppc64le" {
if err := fixIsolinuxConfig(rootFSURL, extractDir, includeNmstateRamDisk, config); err != nil {
return fmt.Errorf("failed to fix isolinux config: %v", err)
}
}

if config != nil {
// Update the default kernel arguments and apply embed area size change
if err := updateDefaultKargs(config, rootFSURL); err != nil {
return fmt.Errorf("failed to update default kargs: %v", err)
}

updatedData, err := json.MarshalIndent(*config, "", " ")
if err != nil {
return fmt.Errorf("failed to marshal updated kargs.json: %v", err)
}

if err := os.WriteFile(kargsPath, updatedData, 0600); err != nil {
return fmt.Errorf("failed to write updated kargs.json: %v", err)
}
}

return nil
Expand Down
Loading