diff --git a/pkg/distro/distro_test.go b/pkg/distro/distro_test.go index 5257f73eb3..55ac80d483 100644 --- a/pkg/distro/distro_test.go +++ b/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) + } + }) + } + } + } +} diff --git a/pkg/distro/distro_test_common/distro_test_common.go b/pkg/distro/distro_test_common/distro_test_common.go index 4e4858ad0a..94a54c1768 100644 --- a/pkg/distro/distro_test_common/distro_test_common.go +++ b/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", } diff --git a/pkg/distro/fedora/imagetype.go b/pkg/distro/fedora/imagetype.go index 8738158d1a..30f1373276 100644 --- a/pkg/distro/fedora/imagetype.go +++ b/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) diff --git a/pkg/distro/rhel/imagetype.go b/pkg/distro/rhel/imagetype.go index 15b63f67c2..36aa6485cd 100644 --- a/pkg/distro/rhel/imagetype.go +++ b/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) }