diff --git a/pkg/registry/core/service/storage/patch_rest.go b/pkg/registry/core/service/storage/patch_rest.go new file mode 100644 index 0000000000000..6e45b111c9f66 --- /dev/null +++ b/pkg/registry/core/service/storage/patch_rest.go @@ -0,0 +1,26 @@ +package storage + +import ( + "context" + + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/warning" + api "k8s.io/kubernetes/pkg/apis/core" +) + +// OpenShift 4.8 and 4.9 only - BZ 2045576 +// In kube 1.21 and 1.22 (OCP 4.8 and 4.9), the apiserver would default the value of `ipFamilyPolicy` to `RequireDualStack` +// if you created a Service with two `ipFamilies` or two `clusterIPs` but no explicitly-specified `ipFamilyPolicy`. +// In 1.23 / 4.10, you MUST explicitly specify either "ipFamilyPolicy: PreferDualStack" or "ipFamilyPolicy: RequireDualStack" +// for the service to be valid. +// Emit a warning in 4.8 and 4.9 if such services are created or updated. +// Using a mutating or validating admission controller webhook for services is a big "no". Therefore, we implement this downstream +// only warning message via OpenShift's kube-apiserver. +// Carry this change forward only in 4.8 and 4.9 and drop this for all other versions. +func warnDualStackIPFamilyPolicy(ctx context.Context, service *api.Service) { + if service.Spec.IPFamilyPolicy == nil && (len(service.Spec.IPFamilies) == 2 || len(service.Spec.ClusterIPs) == 2) { + msg := field.NewPath("service", "spec", "ipFamilyPolicy").String() + " must be RequireDualStack or PreferDualStack when " + + "multiple 'ipFamilies' are specified, this operation will fail starting with Red Hat OpenShift Platform 4.10." + warning.AddWarning(ctx, "", msg) + } +} diff --git a/pkg/registry/core/service/storage/rest.go b/pkg/registry/core/service/storage/rest.go index b207f0f3eb445..41ea244c4d20e 100644 --- a/pkg/registry/core/service/storage/rest.go +++ b/pkg/registry/core/service/storage/rest.go @@ -208,6 +208,9 @@ func (rs *REST) Create(ctx context.Context, obj runtime.Object, createValidation } }() + // OpenShift 4.8 and 4.9 only - BZ 2045576 + warnDualStackIPFamilyPolicy(ctx, service) + // try set ip families (for missing ip families) // we do it here, since we want this to be visible // even when dryRun == true @@ -463,6 +466,9 @@ func (rs *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObj nodePortOp := portallocator.StartOperation(rs.serviceNodePorts, dryrun.IsDryRun(options.DryRun)) defer nodePortOp.Finish() + // OpenShift 4.8 and 4.9 only - BZ 2045576 + warnDualStackIPFamilyPolicy(ctx, service) + // try set ip families (for missing ip families) if err := rs.tryDefaultValidateServiceClusterIPFields(oldService, service); err != nil { return nil, false, err diff --git a/test/integration/dualstack/dualstack_warnings_test.go b/test/integration/dualstack/dualstack_warnings_test.go new file mode 100644 index 0000000000000..a0cf2aaf31031 --- /dev/null +++ b/test/integration/dualstack/dualstack_warnings_test.go @@ -0,0 +1,164 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package dualstack + +import ( + "bytes" + "context" + "fmt" + "strings" + "testing" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/apiserver/pkg/util/feature" + clientset "k8s.io/client-go/kubernetes" + restclient "k8s.io/client-go/rest" + featuregatetesting "k8s.io/component-base/featuregate/testing" + kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/pkg/features" + "k8s.io/kubernetes/test/integration/framework" +) + +func Test_ServiceDualStackIPFamilyPolicy(t *testing.T) { + // Create an IPv4IPv6 dual stack control-plane + serviceCIDR := "10.0.0.0/16" + secondaryServiceCIDR := "2001:db8:1::/112" + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IPv6DualStack, true)() + + s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--service-cluster-ip-range", serviceCIDR + "," + secondaryServiceCIDR}, framework.SharedEtcd()) + defer s.TearDownFn() + + b := &bytes.Buffer{} + warningWriter := restclient.NewWarningWriter(b, restclient.WarningWriterOptions{}) + s.ClientConfig.WarningHandler = warningWriter + client := clientset.NewForConfigOrDie(s.ClientConfig) + + singleStack := v1.IPFamilyPolicySingleStack + preferDualStack := v1.IPFamilyPolicyPreferDualStack + requireDualStack := v1.IPFamilyPolicyRequireDualStack + var testcases = []struct { + name string + clusterIP string + clusterIPs []string + ipFamilies []v1.IPFamily + ipFamilyPolicy *v1.IPFamilyPolicyType + warnings int + }{ + // tests with hardcoded addresses first + { + name: "Dual Stack - multiple cluster IPs - IPFamilyPolicy PreferDualStack - no warning", + clusterIP: "2001:db8:1::11", + clusterIPs: []string{"2001:db8:1::11", "10.0.0.11"}, + ipFamilyPolicy: &preferDualStack, + }, + { + name: "Dual Stack - multiple clusterIPs - IPFamilyPolicy nil - warning", + clusterIP: "2001:db8:1::10", + clusterIPs: []string{"2001:db8:1::10", "10.0.0.10"}, + warnings: 1, + }, + // dynamic allocation + { + name: "Single Stack - IPFamilyPolicy set - no warning", + ipFamilies: []v1.IPFamily{v1.IPv4Protocol}, + ipFamilyPolicy: &singleStack, + }, + { + name: "Single Stack - IPFamilyPolicy nil - no warning", + ipFamilies: []v1.IPFamily{v1.IPv4Protocol}, + ipFamilyPolicy: nil, + }, + { + name: "Dual Stack - multiple ipFamilies - IPFamilyPolicy RequireDualStack - no warning", + ipFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + ipFamilyPolicy: &requireDualStack, + }, + { + name: "Dual Stack - multiple ipFamilies - IPFamilyPolicy nil - warning", + clusterIPs: []string{}, + ipFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + ipFamilyPolicy: nil, + warnings: 1, + }, + } + + expectedWarningCount := 0 + for i, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + + svc := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("svc-test-%d", i), // use different services for each test + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeClusterIP, + ClusterIP: tc.clusterIP, + ClusterIPs: tc.clusterIPs, + IPFamilies: tc.ipFamilies, + IPFamilyPolicy: tc.ipFamilyPolicy, + Ports: []v1.ServicePort{ + { + Port: 443, + TargetPort: intstr.FromInt(443), + }, + }, + }, + } + + // create a service + _, err := client.CoreV1().Services(metav1.NamespaceDefault).Create(context.TODO(), svc, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + if tc.warnings > 0 { + expectedWarningCount += tc.warnings + expectedPatchWarning := field.NewPath("service", "spec", "ipFamilyPolicy").String() + " must be RequireDualStack or PreferDualStack when multiple 'ipFamilies' are specified, this operation will fail starting with Red Hat OpenShift Platform 4.10." + assertWarningMessage(t, b, expectedPatchWarning) + } + assertWarningCount(t, warningWriter, expectedWarningCount) + }) + } +} + +type warningCounter interface { + WarningCount() int +} + +func assertWarningCount(t *testing.T, counter warningCounter, expected int) { + if counter.WarningCount() != expected { + t.Errorf("unexpected warning count, expected: %v, got: %v", expected, counter.WarningCount()) + } +} + +func assertWarningMessage(t *testing.T, b *bytes.Buffer, expected string) { + defer b.Reset() + actual := b.String() + if len(expected) == 0 && len(actual) != 0 { + t.Errorf("unexpected warning message, expected no warning, got: %v", actual) + } + if len(expected) == 0 { + return + } + if !strings.Contains(actual, expected) { + t.Errorf("unexpected warning message, expected: %v, got: %v", expected, actual) + } +}