Distro: fail on OSTree image options being provided for non-OSTree based image types

pkg/distro/distro_test.go

@@ -81,8 +81,10 @@ func TestImageTypePipelineNames(t *testing.T) {
 					seed := int64(0)
 
 					// Add ostree options for image types that require them
-					options.OSTree = &ostree.ImageOptions{
-						URL: "https://example.com",
+					if imageType.OSTreeRef() != "" {
+						options.OSTree = &ostree.ImageOptions{
+							URL: "https://example.com",
+						}
 					}
 
 					// Pipelines that require package sets will fail if none
@@ -397,8 +399,10 @@ func TestPipelineRepositories(t *testing.T) {
 							options := distro.ImageOptions{}
 
 							// Add ostree options for image types that require them
-							options.OSTree = &ostree.ImageOptions{
-								URL: "https://example.com",
+							if imageType.OSTreeRef() != "" {
+								options.OSTree = &ostree.ImageOptions{
+									URL: "https://example.com",
+								}
 							}
 
 							repos := tCase.repos
@@ -567,3 +571,63 @@ func TestDistro_ManifestFIPSWarning(t *testing.T) {
 		}
 	}
 }
+
+// Test that passing options.OSTree for non-OSTree image types results in an error
+func TestOSTreeOptionsErrorForNonOSTreeImgTypes(t *testing.T) {
+	assert := assert.New(t)
+	distroFactory := distrofactory.NewDefault()
+	assert.NotNil(distroFactory)
+
+	distros := distro_test_common.ListTestedDistros(t)
+	assert.NotEmpty(distros)
+
+	for _, distroName := range distros {
+		d := distroFactory.GetDistro(distroName)
+		assert.NotNil(d)
+
+		arches := d.ListArches()
+		assert.NotEmpty(arches)
+
+		for _, archName := range arches {
+			arch, err := d.GetArch(archName)
+			assert.Nil(err)
+
+			imgTypes := arch.ListImageTypes()
+			assert.NotEmpty(imgTypes)
+
+			for _, imageTypeName := range imgTypes {
+				t.Run(fmt.Sprintf("%s/%s/%s", distroName, archName, imageTypeName), func(t *testing.T) {
+					imageType, err := arch.GetImageType(imageTypeName)
+					assert.Nil(err)
+
+					// set up bare minimum args for image type
+					var customizations *blueprint.Customizations
+					if imageType.Name() == "edge-simplified-installer" || imageType.Name() == "iot-simplified-installer" {
+						customizations = &blueprint.Customizations{
+							InstallationDevice: "/dev/null",
+						}
+					}
+					bp := blueprint.Blueprint{
+						Customizations: customizations,
+					}
+					options := distro.ImageOptions{
+						OSTree: &ostree.ImageOptions{
+							URL: "https://example.com",
+						},
+					}
+
+					_, _, err = imageType.Manifest(&bp, options, nil, 0)
+					if imageType.OSTreeRef() == "" {
+						assert.Errorf(err,
+							"OSTree options should not be allowed for non-OSTree image type %s/%s/%s",
+							imageTypeName, archName, distroName)
+					} else {
+						assert.NoErrorf(err,
+							"OSTree options should be allowed for OSTree image type %s/%s/%s",
+							imageTypeName, archName, distroName)
+					}
+				})
+			}
+		}
+	}
+}

pkg/distro/distro_test_common/distro_test_common.go

@@ -28,9 +28,14 @@ var knownKernels = []string{"kernel", "kernel-debug", "kernel-rt"}
 
 // Returns the number of known kernels in the package list
 func kernelCount(imgType distro.ImageType, bp blueprint.Blueprint) int {
-	ostreeOptions := &ostree.ImageOptions{
-		URL: "https://example.com", // required by some image types
+	var ostreeOptions *ostree.ImageOptions
+	// OSTree image types require OSTree options
+	if imgType.OSTreeRef() != "" {
+		ostreeOptions = &ostree.ImageOptions{
+			URL: "https://example.com",
+		}
 	}
+
 	manifest, _, err := imgType.Manifest(&bp, distro.ImageOptions{OSTree: ostreeOptions}, nil, 0)
 	if err != nil {
 		panic(err)
@@ -202,6 +207,11 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
 				imgType, err := arch.GetImageType(typeName)
 				assert.NoError(err)
 
+				if imgType.OSTreeRef() == "" {
+					// image type doesn't support OSTree options
+					t.Skipf("OSTree options not supported for image type %s/%s/%s", typeName, archName, d.Name())
+				}
+
 				ostreeOptions := ostree.ImageOptions{}
 				if typesWithPayload[typeName] {
 					// payload types require URL
@@ -250,6 +260,11 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
 				imgType, err := arch.GetImageType(typeName)
 				assert.NoError(err)
 
+				if imgType.OSTreeRef() == "" {
+					// image type doesn't support OSTree options
+					t.Skipf("OSTree options not supported for image type %s/%s/%s", typeName, archName, d.Name())
+				}
+
 				ostreeOptions := ostree.ImageOptions{
 					ImageRef: "test/x86_64/01",
 				}

pkg/distro/fedora/imagetype.go

@@ -288,9 +288,12 @@ func (t *imageType) Manifest(bp *blueprint.Blueprint,
 // checkOptions checks the validity and compatibility of options and customizations for the image type.
 // Returns ([]string, error) where []string, if non-nil, will hold any generated warnings (e.g. deprecation notices).
 func (t *imageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOptions) ([]string, error) {
-
 	customizations := bp.Customizations
 
+	if !t.rpmOstree && options.OSTree != nil {
+		return nil, fmt.Errorf("OSTree is not supported for %q", t.Name())
+	}
+
 	// we do not support embedding containers on ostree-derived images, only on commits themselves
 	if len(bp.Containers) > 0 && t.rpmOstree && (t.name != "iot-commit" && t.name != "iot-container") {
 		return nil, fmt.Errorf("embedding containers is not supported for %s on %s", t.name, t.arch.distro.name)

pkg/distro/rhel/imagetype.go

@@ -333,6 +333,10 @@ func (t *ImageType) Manifest(bp *blueprint.Blueprint,
 // checkOptions checks the validity and compatibility of options and customizations for the image type.
 // Returns ([]string, error) where []string, if non-nil, will hold any generated warnings (e.g. deprecation notices).
 func (t *ImageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOptions) ([]string, error) {
+	if !t.RPMOSTree && options.OSTree != nil {
+		return nil, fmt.Errorf("OSTree is not supported for %q", t.Name())
+	}
+
 	if t.arch.distro.CheckOptions != nil {
 		return t.arch.distro.CheckOptions(t, bp, options)
 	}