diff --git a/pkg/controller/common/helpers_test.go b/pkg/controller/common/helpers_test.go index 797fb19669..d7f84eec2a 100644 --- a/pkg/controller/common/helpers_test.go +++ b/pkg/controller/common/helpers_test.go @@ -158,7 +158,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 @@ -231,13 +231,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, }, } diff --git a/pkg/controller/common/ignition_converter.go b/pkg/controller/common/ignition_converter.go index 86a14d2e25..3b08bfcd68 100644 --- a/pkg/controller/common/ignition_converter.go +++ b/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 { diff --git a/pkg/server/api.go b/pkg/server/api.go index 9ef0b35ba0..27bc7333c4 100644 --- a/pkg/server/api.go +++ b/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 diff --git a/pkg/server/api_test.go b/pkg/server/api_test.go index 73ac23779b..5e36f55279 100644 --- a/pkg/server/api_test.go +++ b/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,6 +212,44 @@ 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 { @@ -218,8 +257,12 @@ func TestAcceptHeaders(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) + } }) } }