MGMT-17523: Fail when OS images CPU architecture is missing instead o…

…f giving it default value (#6256)
openshift · May 5, 2024 · 75eedfc · 75eedfc
1 parent 9462a1a
commit 75eedfc
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 27 deletions.
diff --git a/internal/versions/osimages.go b/internal/versions/osimages.go
@@ -51,8 +51,8 @@ func validateOSImage(osImage *models.OsImage) error {
 	if swag.StringValue(osImage.Version) == "" {
 		return errors.Errorf(fmt.Sprintf(missingValueTemplate, "version", *osImage.OpenshiftVersion))
 	}
-	if swag.StringValue(osImage.CPUArchitecture) == "" {
-		osImage.CPUArchitecture = swag.String(common.DefaultCPUArchitecture)
+	if osImage.CPUArchitecture == nil {
+		return errors.Errorf("osImage version '%s' CPU architecture is missing", *osImage.OpenshiftVersion)
 	}
 	if err := osImage.Validate(strfmt.Default); err != nil {
 		return errors.Wrapf(err, "osImage version '%s' CPU architecture is not valid", *osImage.OpenshiftVersion)

diff --git a/internal/versions/osimages_test.go b/internal/versions/osimages_test.go
@@ -35,7 +35,7 @@ var _ = Describe("NewOSImages", func() {
 		Expect(err).Should(HaveOccurred())
 	})
 
-	It("should not fail when missing CPU architecture, should set to default", func() {
+	It("should fail when missing CPU architecture", func() {
 		osImages := models.OsImages{
 			{
 				OpenshiftVersion: swag.String("4.14"),
@@ -45,8 +45,7 @@ var _ = Describe("NewOSImages", func() {
 		}
 
 		_, err := NewOSImages(osImages)
-		Expect(err).ShouldNot(HaveOccurred())
-		Expect(*osImages[0].CPUArchitecture).To(Equal(common.X86CPUArchitecture))
+		Expect(err).Should(HaveOccurred())
 	})
 
 	It("should fail when missing URL", func() {
@@ -261,7 +260,7 @@ var _ = Describe("GetCPUArchitectures", func() {
 		images OSImages
 	)
 
-	Context("with default imges", func() {
+	Context("with default images", func() {
 		BeforeEach(func() {
 			var err error
 			images, err = NewOSImages(defaultOsImages)
@@ -282,27 +281,6 @@ var _ = Describe("GetCPUArchitectures", func() {
 			Expect(images.GetCPUArchitectures("4.9.1")).Should(Equal(expected))
 		})
 	})
-
-	It("returns the default architecture", func() {
-		osImages := models.OsImages{
-			&models.OsImage{
-				CPUArchitecture:  swag.String(""),
-				OpenshiftVersion: swag.String("4.9"),
-				URL:              swag.String("rhcos_4.9"),
-				Version:          swag.String("version-49.123-0"),
-			},
-			&models.OsImage{
-				CPUArchitecture:  nil,
-				OpenshiftVersion: swag.String("4.9"),
-				URL:              swag.String("rhcos_4.9"),
-				Version:          swag.String("version-49.123-0"),
-			},
-		}
-		images, err := NewOSImages(osImages)
-		Expect(err).ShouldNot(HaveOccurred())
-
-		Expect(images.GetCPUArchitectures("4.9")).Should(Equal([]string{common.TestDefaultConfig.CPUArchitecture}))
-	})
 })
 
 var _ = Describe("NewOSImages", func() {
@@ -361,4 +339,28 @@ var _ = Describe("NewOSImages", func() {
 		}
 		Expect(validateImages(osImages)).NotTo(Succeed())
 	})
+
+	It("CPU architecture is not valid", func() {
+		osImages := models.OsImages{
+			&models.OsImage{
+				CPUArchitecture:  swag.String(""),
+				OpenshiftVersion: swag.String("4.9"),
+				URL:              swag.String("rhcos_4.9"),
+				Version:          swag.String("version-49.123-0"),
+			},
+		}
+		_, err := NewOSImages(osImages)
+		Expect(err).Should(HaveOccurred())
+
+		osImages = models.OsImages{
+			&models.OsImage{
+				CPUArchitecture:  nil,
+				OpenshiftVersion: swag.String("4.9"),
+				URL:              swag.String("rhcos_4.9"),
+				Version:          swag.String("version-49.123-0"),
+			},
+		}
+		_, err = NewOSImages(osImages)
+		Expect(err).Should(HaveOccurred())
+	})
 })