[release-4.21] OCPBUGS-84481: MCS fallback to latest v3

pkg/controller/common/helpers_test.go

@@ -157,7 +157,7 @@ func TestIgnitionConverterGetSupportedMinorVersion(t *testing.T) {
 	assert.True(t, matchingMinorVersion.Equal(*v350))
 
 	_, err = converter.GetSupportedMinorVersion(*semver.New("7.7.7"))
-	assert.ErrorIs(t, err, errIgnitionConverterUnknownVersion)
+	assert.ErrorIs(t, err, ErrIgnitionConverterUnknownVersion)
 }
 
 // TestIgnitionConverterConvert tests that the Ignition converter is able to handle the expected
@@ -230,13 +230,13 @@ func TestIgnitionConverterConvert(t *testing.T) {
 			inputConfig:   ign2Config,
 			inputVersion:  "3.5.0",
 			outputVersion: "3.1.0",
-			err:           errIgnitionConverterWrongSourceType,
+			err:           ErrIgnitionConverterWrongSourceType,
 		}, {
 			name:          "Conversion not supported",
 			inputConfig:   ign3Config,
 			inputVersion:  "3.1.0",
 			outputVersion: "3.5.0",
-			err:           errIgnitionConverterUnsupportedConversion,
+			err:           ErrIgnitionConverterUnsupportedConversion,
 		},
 	}
 

pkg/controller/common/ignition_converter.go

@@ -27,9 +27,9 @@ import (
 )
 
 var (
-	errIgnitionConverterWrongSourceType       = errors.New("wrong source type for the conversion")
-	errIgnitionConverterUnknownVersion        = errors.New("the requested version is unknown")
-	errIgnitionConverterUnsupportedConversion = errors.New("the requested conversion is not supported")
+	ErrIgnitionConverterWrongSourceType       = errors.New("wrong source type for the conversion")
+	ErrIgnitionConverterUnknownVersion        = errors.New("the requested version is unknown")
+	ErrIgnitionConverterUnsupportedConversion = errors.New("the requested conversion is not supported")
 	ignitionConverter                         = newIgnitionConverter(buildConverterList())
 )
 
@@ -100,7 +100,7 @@ func newFunctionConverterTyped[S any, T any](sourceVersion, targetVersion semver
 				return conversionFunc(input)
 			}
 			var empty T
-			return empty, errIgnitionConverterWrongSourceType
+			return empty, ErrIgnitionConverterWrongSourceType
 		})
 }
 
@@ -112,7 +112,7 @@ func newFunctionConverterTypedNoError[S any, T any](sourceVersion, targetVersion
 
 func (c *functionConverter) Convert(source any) (any, error) {
 	result, err := c.conversionFunc(source)
-	if err != nil && errors.Is(err, errIgnitionConverterWrongSourceType) {
+	if err != nil && errors.Is(err, ErrIgnitionConverterWrongSourceType) {
 		return result, fmt.Errorf("conversion from %v to %v failed: %w", c.tuple.sourceVersion, c.tuple.targetVersion, err)
 	}
 	return result, err
@@ -192,12 +192,12 @@ func newConverterConversionPathFromExisting(conversionPath *converterConversionP
 type IgnitionConverter interface {
 	// Convert performs the Ignition conversion of source (that uses Ignition version sourceVersion)
 	// to the version given by targetVersion.
-	// The conversion may fail with errIgnitionConverterUnsupportedConversion if there is no available conversion
+	// The conversion may fail with ErrIgnitionConverterUnsupportedConversion if there is no available conversion
 	// for the source-target version tuple.
 	Convert(source any, sourceVersion, targetVersion semver.Version) (any, error)
 
 	// GetSupportedMinorVersion retrieves the [semver.Version] that performs a translation of the given version.
-	// The method may return errIgnitionConverterUnknownVersion if the given version minor version (X.Y) does not
+	// The method may return ErrIgnitionConverterUnknownVersion if the given version minor version (X.Y) does not
 	// have a compatible version in the converter.
 	GetSupportedMinorVersion(version semver.Version) (semver.Version, error)
 
@@ -295,7 +295,7 @@ func (m *ignitionConverterImpl) Convert(source any, sourceVersion, targetVersion
 	tuple := conversionTuple{sourceVersion: sourceVersion, targetVersion: targetVersion}
 	conversionPath, ok := m.converterPaths[tuple]
 	if !ok {
-		return nil, fmt.Errorf("conversion for source version %v to target version %v is not supported %w", sourceVersion, targetVersion, errIgnitionConverterUnsupportedConversion)
+		return nil, fmt.Errorf("conversion for source version %v to target version %v is not supported %w", sourceVersion, targetVersion, ErrIgnitionConverterUnsupportedConversion)
 	}
 
 	var err error
@@ -315,7 +315,7 @@ func (m *ignitionConverterImpl) GetSupportedMinorVersion(version semver.Version)
 			return *ver, nil
 		}
 	}
-	return semver.Version{}, errIgnitionConverterUnknownVersion
+	return semver.Version{}, ErrIgnitionConverterUnknownVersion
 }
 
 func (m *ignitionConverterImpl) GetSupportedMinorVersions() []string {

pkg/server/api.go

@@ -3,6 +3,7 @@ package server
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"net/http"
 	"path"
@@ -12,10 +13,10 @@ import (
 
 	"github.com/clarketm/json"
 	"github.com/coreos/go-semver/semver"
+	ign3types "github.com/coreos/ignition/v2/config/v3_5/types"
+	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/certwatcher"
-
-	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 )
 
 const (
@@ -288,8 +289,19 @@ func detectSpecVersionFromAcceptHeader(acceptHeader string) (*semver.Version, er
 		if reqVersion.Compare(*v22Version) >= 0 && reqVersion.Major == v22Version.Major {
 			reqVersion = v22Version
 		}
+
 		version, err := ctrlcommon.IgnitionConverterSingleton().GetSupportedMinorVersion(*reqVersion)
 		if err != nil {
+			// Check if the error is because the client is asking for a minor version we still don't support
+			// We can assume it's safe to return our latest minor if the requested version is of the same
+			// major but higher minor version.
+			newerMinorRequested := ign3types.MaxVersion.Major == reqVersion.Major && ign3types.MaxVersion.Minor < reqVersion.Minor
+			if errors.Is(err, ctrlcommon.ErrIgnitionConverterUnknownVersion) && newerMinorRequested {
+				// The requested version is not in our supported list, but it's safe to return the latest supported
+				// version if the major version matches
+				return &ign3types.MaxVersion, nil
+
+			}
 			return nil, fmt.Errorf("unsupported Ignition version in Accept header: %s", acceptHeader)
 		}
 		return &version, nil

pkg/server/api_test.go

@@ -40,10 +40,11 @@ type scenario struct {
 const expectedContentLength int = 32
 
 type acceptHeaderScenario struct {
-	name       string
-	input      string
-	headerVals []acceptHeaderValue
-	versionOut *semver.Version
+	name        string
+	input       string
+	headerVals  []acceptHeaderValue
+	versionOut  *semver.Version
+	errExpected bool
 }
 
 func float32ToPtr(f32 float32) *float32 {
@@ -211,15 +212,57 @@ func TestAcceptHeaders(t *testing.T) {
 			headerVals: nil,
 			versionOut: v2_2,
 		},
+		{
+			name:  "IgnV3_unsupported_newer_minor_falls_back_to_max",
+			input: "application/vnd.coreos.ignition+json;version=3.99.0, */*;q=0.1",
+			headerVals: []acceptHeaderValue{
+				{
+					MIMEType:    "application",
+					MIMESubtype: "vnd.coreos.ignition+json",
+					SemVer:      semver.New("3.99.0"),
+					QValue:      float32ToPtr(1.0),
+				},
+				{
+					MIMEType:    "*",
+					MIMESubtype: "*",
+					SemVer:      nil,
+					QValue:      float32ToPtr(0.1),
+				},
+			},
+			versionOut: v3_5,
+		},
+		{
+			name:  "IgnV4_unsupported_major_errors",
+			input: "application/vnd.coreos.ignition+json;version=4.0.0, */*;q=0.1",
+			headerVals: []acceptHeaderValue{
+				{
+					MIMEType:    "application",
+					MIMESubtype: "vnd.coreos.ignition+json",
+					SemVer:      semver.New("4.0.0"),
+					QValue:      float32ToPtr(1.0),
+				},
+				{
+					MIMEType:    "*",
+					MIMESubtype: "*",
+					SemVer:      nil,
+					QValue:      float32ToPtr(0.1),
+				},
+			},
+			errExpected: true,
+		},
 	}
 
 	for _, header := range headers {
 		t.Run(header.name, func(t *testing.T) {
 			headers, _ := parseAcceptHeader(header.input)
 			assert.Equal(t, header.headerVals, headers)
 			version, err := detectSpecVersionFromAcceptHeader(header.input)
-			assert.Equal(t, header.versionOut, version)
-			require.NoError(t, err)
+			if header.errExpected {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, header.versionOut, version)
+			}
 		})
 	}
 }