From d56a4e69d9752aba3f6cf58b37e20aaea752c02f Mon Sep 17 00:00:00 2001
From: Achilleas Koutsou <achilleas@koutsou.net>
Date: Tue, 14 May 2024 02:03:28 +0200
Subject: [PATCH] customizations/kickstart: unify option validation

Validate kickstart options in a Validate() function so unify option
compatibility handling.  The function is called from the kickstart.New()
initialiser, but we also call it before stage creation to make sure
everything is valid right before stage creation.

Co-authored-by: Michael Vogt <michael.vogt@gmail.com>
---
 pkg/customizations/kickstart/kickstart.go   | 22 ++++++++++++
 pkg/manifest/anaconda_installer_iso_tree.go | 39 +++++++--------------
 2 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/pkg/customizations/kickstart/kickstart.go b/pkg/customizations/kickstart/kickstart.go
index 7b92f276bc..4b5897d590 100644
--- a/pkg/customizations/kickstart/kickstart.go
+++ b/pkg/customizations/kickstart/kickstart.go
@@ -1,6 +1,8 @@
 package kickstart
 
 import (
+	"fmt"
+
 	"github.com/osbuild/images/pkg/blueprint"
 	"github.com/osbuild/images/pkg/customizations/users"
 )
@@ -67,5 +69,25 @@ func New(customizations *blueprint.Customizations) (*Options, error) {
 		}
 	}
 
+	if err := options.Validate(); err != nil {
+		return nil, err
+	}
 	return options, nil
 }
+
+func (options Options) Validate() error {
+	if options.UserFile != nil {
+		// users, groups, and other kickstart options are not allowed when
+		// users add their own kickstarts
+		if options.Unattended {
+			return fmt.Errorf("kickstart unattended options are not compatible with user-supplied kickstart content")
+		}
+		if len(options.SudoNopasswd) > 0 {
+			return fmt.Errorf("kickstart sudo nopasswd drop-in file creation is not compatible with user-supplied kickstart content")
+		}
+		if len(options.Users)+len(options.Groups) > 0 {
+			return fmt.Errorf("kickstart users and/or groups are not compatible with user-supplied kickstart content")
+		}
+	}
+	return nil
+}
diff --git a/pkg/manifest/anaconda_installer_iso_tree.go b/pkg/manifest/anaconda_installer_iso_tree.go
index 9c313e97ab..bcf0c58396 100644
--- a/pkg/manifest/anaconda_installer_iso_tree.go
+++ b/pkg/manifest/anaconda_installer_iso_tree.go
@@ -422,17 +422,17 @@ func (p *AnacondaInstallerISOTree) bootcInstallerKickstartStages() []*osbuild.St
 		panic(fmt.Sprintf("failed to create kickstart stage options: %v", err))
 	}
 
+	// kickstart.New() already validates the options but they may have been
+	// modified since then, so validate them before we create the stages
+	if err := p.Kickstart.Validate(); err != nil {
+		panic(err)
+	}
+
 	if p.Kickstart.UserFile != nil {
+
 		// when a user defines their own kickstart, we create a kickstart that
 		// takes care of the installation and let the user kickstart handle
 		// everything else
-
-		// users and groups are NOT allowed when users add their own kickstarts
-		if len(kickstartOptions.Users)+len(kickstartOptions.Groups) > 0 {
-			// this is a programming error - the combinations should have been verified already
-			panic("kickstart users and/or groups are not compatible with user-supplied kickstart content")
-		}
-
 		stages = append(stages, osbuild.NewKickstartStage(kickstartOptions))
 		kickstartFile, err := kickstartOptions.IncludeRaw(p.Kickstart.UserFile.Contents)
 		if err != nil {
@@ -548,26 +548,13 @@ func (p *AnacondaInstallerISOTree) makeKickstartStages(stageOptions *osbuild.Kic
 
 	stages := make([]*osbuild.Stage, 0)
 
-	// users, groups, and other kickstart options are not allowed when users
-	// add their own kickstarts
-	if kickstartOptions.UserFile != nil {
-		// check if any other option is set and panic - these combinations
-		// should be verified by the caller
-		if kickstartOptions.Unattended {
-			panic("kickstart unattended options are not compatible with user-supplied kickstart content")
-		}
-
-		if len(kickstartOptions.SudoNopasswd) > 0 {
-			panic("kickstart sudo nopasswd drop-in file creation is not compatible with user-supplied kickstart content")
-		}
-
-		// options are usually already initialised from outside this function
-		// with the payload options (ostree commit or tarball), but might also
-		// have Users and Groups added
-		if len(kickstartOptions.Users)+len(kickstartOptions.Groups) > 0 {
-			panic("kickstart users and/or groups are not compatible with user-supplied kickstart content")
-		}
+	// kickstart.New() already validates the options but they may have been
+	// modified since then, so validate them before we create the stages
+	if err := p.Kickstart.Validate(); err != nil {
+		panic(err)
+	}
 
+	if kickstartOptions.UserFile != nil {
 		stages = append(stages, osbuild.NewKickstartStage(stageOptions))
 		if kickstartOptions.UserFile != nil {
 			kickstartFile, err := stageOptions.IncludeRaw(kickstartOptions.UserFile.Contents)