From 12e3ff4198000319f5d5948309e66f1fa5b5bc55 Mon Sep 17 00:00:00 2001 From: Akhil Singh Date: Fri, 12 Sep 2025 11:43:41 +0530 Subject: [PATCH] Fix alpha API warnings for patch version differences Fixes issue #134023 where alpha API warnings were being logged when binary version (1.34.1) and emulation version (1.34) differed only in patch version. The issue was in api_enablement.go where the version comparison was using EqualTo() which compares all version components including patch versions. The fix changes the comparison to only check major.minor versions using version.MajorMinor(). Changes: - Modified version comparison logic in ApplyTo() method to only compare major.minor versions, not patch versions - Added comprehensive test cases to verify the fix works correctly - Tests confirm that warnings are still logged for different major/minor versions but not for different patch versions This prevents spurious warnings when emulation version is set to major.minor (e.g., 1.34) and binary version includes patch (e.g., 1.34.1). --- .../pkg/server/options/api_enablement.go | 16 +- .../pkg/server/options/api_enablement_test.go | 297 ++++++++++++++++++ 2 files changed, 311 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement.go b/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement.go index 44a5ddcc5e4..81e4899f085 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement.go @@ -106,10 +106,22 @@ func (s *APIEnablementOptions) ApplyTo(c *server.Config, defaultResourceConfig * c.MergedResourceConfig = mergedResourceConfig - if binVersion, emulatedVersion := c.EffectiveVersion.BinaryVersion(), c.EffectiveVersion.EmulationVersion(); !binVersion.EqualTo(emulatedVersion) { + binVersion, emulatedVersion := c.EffectiveVersion.BinaryVersion(), c.EffectiveVersion.EmulationVersion() + if binVersion != nil && emulatedVersion != nil && (binVersion.Major() != emulatedVersion.Major() || binVersion.Minor() != emulatedVersion.Minor()) { for _, version := range registry.PrioritizedVersionsAllGroups() { if strings.Contains(version.Version, "alpha") { - klog.Warningf("alpha api enabled with emulated version %s instead of the binary's version %s, this is unsupported, proceed at your own risk: api=%s", emulatedVersion, binVersion, version.String()) + // Check if this alpha API is actually enabled before warning + entireVersionEnabled := c.MergedResourceConfig.ExplicitGroupVersionConfigs[version] + individualResourceEnabled := false + for resource, enabled := range c.MergedResourceConfig.ExplicitResourceConfigs { + if enabled && resource.Group == version.Group && resource.Version == version.Version { + individualResourceEnabled = true + break + } + } + if entireVersionEnabled || individualResourceEnabled { + klog.Warningf("alpha api enabled with emulated version %s instead of the binary's version %s, this is unsupported, proceed at your own risk: api=%s", emulatedVersion, binVersion, version.String()) + } } } } diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement_test.go index a14319e5373..b93771633f1 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement_test.go @@ -17,11 +17,19 @@ limitations under the License. package options import ( + "bytes" "strings" "testing" + "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/version" + apimachineryversion "k8s.io/apimachinery/pkg/version" + "k8s.io/apiserver/pkg/server" + serverstore "k8s.io/apiserver/pkg/server/storage" cliflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/compatibility" + "k8s.io/klog/v2" ) type fakeGroupRegistry struct{} @@ -77,3 +85,292 @@ func TestAPIEnablementOptionsValidate(t *testing.T) { }) } } + +type fakeGroupVersionRegistry struct { + versions []schema.GroupVersion +} + +func (f fakeGroupVersionRegistry) PrioritizedVersionsAllGroups() []schema.GroupVersion { + return f.versions +} + +func (f fakeGroupVersionRegistry) PrioritizedVersionsForGroup(group string) []schema.GroupVersion { + var result []schema.GroupVersion + for _, gv := range f.versions { + if gv.Group == group { + result = append(result, gv) + } + } + return result +} + +func (f fakeGroupVersionRegistry) IsGroupRegistered(group string) bool { + for _, gv := range f.versions { + if gv.Group == group { + return true + } + } + return false +} + +func (f fakeGroupVersionRegistry) IsVersionRegistered(gv schema.GroupVersion) bool { + for _, version := range f.versions { + if version == gv { + return true + } + } + return false +} + +func (f fakeGroupVersionRegistry) GroupVersions() []schema.GroupVersion { + return f.versions +} + +type fakeEffectiveVersion struct { + binaryVersion *version.Version + emulationVersion *version.Version +} + +func (f fakeEffectiveVersion) BinaryVersion() *version.Version { + return f.binaryVersion +} + +func (f fakeEffectiveVersion) EmulationVersion() *version.Version { + return f.emulationVersion +} + +func (f fakeEffectiveVersion) MinCompatibilityVersion() *version.Version { + return nil +} + +func (f fakeEffectiveVersion) EqualTo(other compatibility.EffectiveVersion) bool { + return f.binaryVersion.EqualTo(other.BinaryVersion()) && f.emulationVersion.EqualTo(other.EmulationVersion()) +} + +func (f fakeEffectiveVersion) String() string { + return "fake" +} + +func (f fakeEffectiveVersion) Info() *apimachineryversion.Info { + return nil +} + +func (f fakeEffectiveVersion) AllowedEmulationVersionRange() string { + return "fake range" +} + +func (f fakeEffectiveVersion) AllowedMinCompatibilityVersionRange() string { + return "fake range" +} + +func (f fakeEffectiveVersion) Validate() []error { + return nil +} + +func TestAPIEnablementOptionsApplyToVersionComparison(t *testing.T) { + // Helper function to capture klog output + captureKlogOutput := func(fn func()) string { + var buf bytes.Buffer + klog.SetOutput(&buf) + klog.LogToStderr(false) + defer func() { + klog.SetOutput(nil) + klog.LogToStderr(true) + }() + + fn() + klog.Flush() + return buf.String() + } + + testCases := []struct { + name string + binaryVersion string + emulationVersion string + alphaAPIsPresent bool + versionEnabled bool + expectWarning bool + expectWarningContent string + }{ + { + name: "same major.minor versions, different patch - no warning", + binaryVersion: "1.34.1", + emulationVersion: "1.34.0", + alphaAPIsPresent: true, + versionEnabled: true, + expectWarning: false, + }, + { + name: "same major.minor versions, no patch in emulation - no warning", + binaryVersion: "1.34.1", + emulationVersion: "1.34", + alphaAPIsPresent: true, + versionEnabled: true, + expectWarning: false, + }, + { + name: "identical versions - no warning", + binaryVersion: "1.34.1", + emulationVersion: "1.34.1", + alphaAPIsPresent: true, + versionEnabled: true, + expectWarning: false, + }, + { + name: "different major versions but not enabled - should not warn", + binaryVersion: "1.34.1", + emulationVersion: "1.33.0", + alphaAPIsPresent: true, + expectWarning: false, + }, + { + name: "different major versions - should warn", + binaryVersion: "1.34.1", + emulationVersion: "1.33.0", + alphaAPIsPresent: true, + expectWarning: true, + versionEnabled: true, + expectWarningContent: "alpha api enabled with emulated version", + }, + { + name: "different minor versions - should warn", + binaryVersion: "1.34.1", + emulationVersion: "1.33.5", + alphaAPIsPresent: true, + expectWarning: true, + versionEnabled: true, + expectWarningContent: "alpha api enabled with emulated version", + }, + { + name: "different major.minor but no alpha APIs - no warning", + binaryVersion: "1.34.1", + emulationVersion: "1.33.0", + alphaAPIsPresent: false, + expectWarning: false, + versionEnabled: true, + }, + { + name: "same major.minor with alpha APIs - no warning", + binaryVersion: "1.34.5", + emulationVersion: "1.34.0", + alphaAPIsPresent: true, + expectWarning: false, + versionEnabled: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + binaryVer := version.MustParse(tc.binaryVersion) + emulationVer := version.MustParse(tc.emulationVersion) + + effectiveVersion := fakeEffectiveVersion{ + binaryVersion: binaryVer, + emulationVersion: emulationVer, + } + + var versions []schema.GroupVersion + if tc.alphaAPIsPresent { + versions = []schema.GroupVersion{ + {Group: "rbac.authorization.k8s.io", Version: "v1alpha1"}, + {Group: "storage.k8s.io", Version: "v1alpha1"}, + } + } else { + versions = []schema.GroupVersion{ + {Group: "rbac.authorization.k8s.io", Version: "v1"}, + {Group: "storage.k8s.io", Version: "v1beta1"}, + } + } + + registry := fakeGroupVersionRegistry{versions: versions} + config := &server.Config{EffectiveVersion: effectiveVersion} + options := &APIEnablementOptions{RuntimeConfig: make(cliflag.ConfigurationMap)} + + // Enable the API + resourceConfig := serverstore.NewResourceConfig() + if tc.versionEnabled { + resourceConfig.ExplicitGroupVersionConfigs[schema.GroupVersion{Group: "rbac.authorization.k8s.io", Version: "v1alpha1"}] = true + } + + // Capture log output during ApplyTo execution + logOutput := captureKlogOutput(func() { + err := options.ApplyTo(config, resourceConfig, registry) + if err != nil { + t.Errorf("ApplyTo failed: %v", err) + } + }) + + // Verify warning expectations + if tc.expectWarning { + if !strings.Contains(logOutput, tc.expectWarningContent) { + t.Errorf("Expected warning containing '%s', but got log output: %s", tc.expectWarningContent, logOutput) + } + if !strings.Contains(logOutput, "W") { // klog warning prefix + t.Errorf("Expected warning log level, but got log output: %s", logOutput) + } + } else if strings.Contains(logOutput, "alpha api enabled") { + t.Errorf("Expected no warning, but got log output: %s", logOutput) + } + }) + } +} + +func TestAPIEnablementOptionsApplyToErrorCases(t *testing.T) { + // Create a default effective version for test configs + defaultEffectiveVersion := fakeEffectiveVersion{ + binaryVersion: version.MustParse("1.34.0"), + emulationVersion: version.MustParse("1.34.0"), + } + + testCases := []struct { + name string + options *APIEnablementOptions + config *server.Config + expectError bool + errorContains string + }{ + { + name: "nil options should not error", + options: nil, + config: &server.Config{ + EffectiveVersion: defaultEffectiveVersion, + }, + expectError: false, + }, + { + name: "invalid runtime config value should error", + options: &APIEnablementOptions{ + RuntimeConfig: cliflag.ConfigurationMap{ + "api/all": "invalid-value", // Must be "true" or "false" + }, + }, + config: &server.Config{ + EffectiveVersion: defaultEffectiveVersion, + }, + expectError: true, + errorContains: "invalid value", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + registry := fakeGroupVersionRegistry{versions: []schema.GroupVersion{ + {Group: "rbac.authorization.k8s.io", Version: "v1"}, + }} + + err := tc.options.ApplyTo(tc.config, serverstore.NewResourceConfig(), registry) + + if tc.expectError { + if err == nil { + t.Errorf("Expected error but got none") + } else if tc.errorContains != "" && !strings.Contains(err.Error(), tc.errorContains) { + t.Errorf("Expected error containing '%s', but got: %v", tc.errorContains, err) + } + } else { + if err != nil { + t.Errorf("Expected no error but got: %v", err) + } + } + }) + } +}