From ab6839b9933f44f9a3f0018131ce65a5d50a4080 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 4 Aug 2026 16:56:52 +0300 Subject: [PATCH 1/4] test(conformance): preserve Route status entries owned by other controllers RouteStatus.Parents is namespaced by (parentRef, controllerName), and an implementation MUST NOT update entries whose controllerName is not its own. Nothing in the suite checked that, and the harness could not have hosted such a check: RouteMustHaveParents required every entry in status.parents to carry the Route's current generation, so a stale entry left behind by a second controller stalled the wait until it timed out. Add a StaleControllerName sentinel that tests can put on a parent status entry to stand in for another implementation, and skip those entries in the observedGeneration check. Nothing reconciles them, so they cannot be expected to keep up with the Route's generation. On top of that, add a test that seeds such an entry, bumps the Route's generation to force a status write, and checks that the seeded entry comes back byte for byte alongside the implementation's own entry. The observedGeneration check also ran against the parent list read on the previous poll rather than the one just fetched. On the first iteration that list was still empty, so a Route whose status already matched satisfied the wait without the check running at all. It now runs on the snapshot it just read, which tightens every RouteMustHaveParents caller in the suite: a status that matches but has not caught up with the Route generation no longer ends the wait early. The check moves into staleParentStatus so both halves of it can be pinned directly. Signed-off-by: Aleksei Sviridkin --- .../httproute-preserve-foreign-status.go | 162 +++++++++++++ .../httproute-preserve-foreign-status.yaml | 19 ++ conformance/utils/kubernetes/helpers.go | 34 ++- conformance/utils/kubernetes/helpers_test.go | 222 ++++++++++++++++++ 4 files changed, 431 insertions(+), 6 deletions(-) create mode 100644 conformance/tests/httproute-preserve-foreign-status.go create mode 100644 conformance/tests/httproute-preserve-foreign-status.yaml diff --git a/conformance/tests/httproute-preserve-foreign-status.go b/conformance/tests/httproute-preserve-foreign-status.go new file mode 100644 index 0000000000..ca479b9409 --- /dev/null +++ b/conformance/tests/httproute-preserve-foreign-status.go @@ -0,0 +1,162 @@ +/* +Copyright 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 tests + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" + + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" + confsuite "sigs.k8s.io/gateway-api/conformance/utils/suite" + "sigs.k8s.io/gateway-api/pkg/features" +) + +var errForeignStatusNotStored = errors.New("the seeded status entry is missing from the update response") + +func init() { + ConformanceTests = append(ConformanceTests, HTTPRoutePreserveForeignStatus) +} + +var HTTPRoutePreserveForeignStatus = confsuite.ConformanceTest{ + ShortName: "HTTPRoutePreserveForeignStatus", + Description: "An implementation must not remove or modify HTTPRoute status.parents entries " + + "whose controllerName belongs to another implementation.", + Features: []features.FeatureName{ + features.SupportGateway, + features.SupportHTTPRoute, + }, + Manifests: []string{"tests/httproute-preserve-foreign-status.yaml"}, + Provisional: true, + Test: func(t *testing.T, suite *confsuite.ConformanceTestSuite) { + ns := confsuite.InfrastructureNamespace + routeNN := types.NamespacedName{Name: "preserve-foreign-status", Namespace: ns} + gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + + kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + + var seeded gatewayv1.RouteParentStatus + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + route := &gatewayv1.HTTPRoute{} + if err := suite.Client.Get(t.Context(), routeNN, route); err != nil { + return err + } + route.Status.Parents = append(route.Status.Parents, gatewayv1.RouteParentStatus{ + ParentRef: gatewayv1.ParentReference{ + Group: new(gatewayv1.Group(gatewayv1.GroupVersion.Group)), + Kind: new(gatewayv1.Kind("Gateway")), + Name: "unmanaged-gateway", + Namespace: new(gatewayv1.Namespace(ns)), + }, + ControllerName: kubernetes.StaleControllerName, + Conditions: []metav1.Condition{{ + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.RouteReasonAccepted), + ObservedGeneration: route.Generation, + LastTransitionTime: metav1.Now(), + }}, + }) + if err := suite.Client.Status().Update(t.Context(), route); err != nil { + return err + } + // Read the entry back off the update response rather than reusing + // the value written above: the API server truncates + // lastTransitionTime to second precision. + stored := foreignParentStatus(route.Status.Parents) + if stored == nil { + return errForeignStatusNotStored + } + seeded = *stored + return nil + }) + require.NoError(t, err, "error seeding a foreign controller's status entry on HTTPRoute %s", routeNN) + + // Bump the generation so the implementation has to run a full + // read-modify-write cycle on a status that already holds the seeded entry. + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + route := &gatewayv1.HTTPRoute{} + if getErr := suite.Client.Get(t.Context(), routeNN, route); getErr != nil { + return getErr + } + route.Spec.Rules[0].Matches[0].Path.Value = new("/preserve-foreign-status-updated") + return suite.Client.Update(t.Context(), route) + }) + require.NoError(t, err, "error updating HTTPRoute %s", routeNN) + + // The implementation's own entry catching up to the bumped generation is + // what proves it wrote status while the seeded entry was there. + kubernetes.HTTPRouteMustHaveParents(t, suite.Client, suite.TimeoutConfig, routeNN, + []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{ + Group: new(gatewayv1.Group(gatewayv1.GroupVersion.Group)), + Kind: new(gatewayv1.Kind("Gateway")), + Name: gatewayv1.ObjectName(gwNN.Name), + Namespace: new(gatewayv1.Namespace(ns)), + }, + ControllerName: gatewayv1.GatewayController(suite.ControllerName), + Conditions: []metav1.Condition{ + { + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }, + }, + }, + { + ParentRef: seeded.ParentRef, + ControllerName: kubernetes.StaleControllerName, + Conditions: []metav1.Condition{ + { + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.RouteReasonAccepted), + }, + }, + }, + }, + // The Route and the Gateway share a namespace, so an implementation + // that leaves parentRef.namespace unset still matches. + false, + ) + + route := &gatewayv1.HTTPRoute{} + require.NoError(t, suite.Client.Get(t.Context(), routeNN, route), "error fetching HTTPRoute %s", routeNN) + + // HTTPRouteMustHaveParents compares conditions by type, status and reason, + // so it cannot see a rewrite that keeps the entry Accepted but restamps + // observedGeneration or lastTransitionTime. + stored := foreignParentStatus(route.Status.Parents) + require.NotNilf(t, stored, "HTTPRoute %s: status entry owned by %s was removed", routeNN, kubernetes.StaleControllerName) + require.Equalf(t, seeded, *stored, "HTTPRoute %s: status entry owned by %s was modified", routeNN, kubernetes.StaleControllerName) + }, +} + +func foreignParentStatus(parents []gatewayv1.RouteParentStatus) *gatewayv1.RouteParentStatus { + for i := range parents { + if parents[i].ControllerName == kubernetes.StaleControllerName { + return &parents[i] + } + } + + return nil +} diff --git a/conformance/tests/httproute-preserve-foreign-status.yaml b/conformance/tests/httproute-preserve-foreign-status.yaml new file mode 100644 index 0000000000..df3c712092 --- /dev/null +++ b/conformance/tests/httproute-preserve-foreign-status.yaml @@ -0,0 +1,19 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: preserve-foreign-status + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + # No Gateway by this name exists; the test seeds a status entry for it under a + # stale controllerName and checks that the implementation leaves it alone. + - name: unmanaged-gateway + rules: + - matches: + - path: + type: PathPrefix + value: /preserve-foreign-status + backendRefs: + - name: infra-backend-v1 + port: 8080 diff --git a/conformance/utils/kubernetes/helpers.go b/conformance/utils/kubernetes/helpers.go index 95512f8a6e..c0ad27512d 100644 --- a/conformance/utils/kubernetes/helpers.go +++ b/conformance/utils/kubernetes/helpers.go @@ -52,6 +52,12 @@ import ( // tests which validate fixing broken Gateways, e.t.c. const GatewayExcludedFromReadinessChecks = "gateway-api/skip-this-for-readiness" +// StaleControllerName is a controllerName that tests can put on a Route +// status.parents entry to stand in for another implementation sharing the +// cluster. Nothing reconciles such an entry, so RouteMustHaveParents leaves its +// observedGeneration alone. +const StaleControllerName = gatewayv1.GatewayController("gateway.networking.k8s.io/stale-controller") + const GatewayKind = gatewayv1.Kind("Gateway") // GatewayRef is a tiny type for specifying an HTTP Route ParentRef without @@ -793,6 +799,23 @@ func RouteTypeMustHaveParentsField(t *testing.T, routeType any) string { return routeTypeName } +// staleParentStatus returns the first status entry whose conditions have not +// caught up with the object generation. Entries owned by StaleControllerName +// are exempt: nothing reconciles them, so their observedGeneration never moves. +func staleParentStatus(obj metav1.Object, parents []gatewayv1.RouteParentStatus) (gatewayv1.RouteParentStatus, error) { + for _, parent := range parents { + if parent.ControllerName == StaleControllerName { + continue + } + + if err := ConditionsHaveLatestObservedGeneration(obj, parent.Conditions); err != nil { + return parent, err + } + } + + return gatewayv1.RouteParentStatus{}, nil +} + func RouteMustHaveParents(t *testing.T, cli client.Client, timeoutConfig config.TimeoutConfig, routeName types.NamespacedName, parents []gatewayv1.RouteParentStatus, namespaceRequired bool, routeType any) { t.Helper() @@ -811,14 +834,13 @@ func RouteMustHaveParents(t *testing.T, cli client.Client, timeoutConfig config. return false, fmt.Errorf("error fetching %s: %w", routeTypeName, err) } - for _, parent := range actual { - if err := ConditionsHaveLatestObservedGeneration(metaObj, parent.Conditions); err != nil { - tlog.Logf(t, "%s(controller=%v,ref=%#v) %v", routeTypeName, parent.ControllerName, parent, err) - return false, nil - } + actual = reflect.ValueOf(cliObj).Elem().FieldByName("Status").FieldByName("Parents").Interface().([]v1alpha2.RouteParentStatus) + + if parent, err := staleParentStatus(metaObj, actual); err != nil { + tlog.Logf(t, "%s(controller=%v,ref=%#v) %v", routeTypeName, parent.ControllerName, parent, err) + return false, nil } - actual = reflect.ValueOf(cliObj).Elem().FieldByName("Status").FieldByName("Parents").Interface().([]v1alpha2.RouteParentStatus) return parentsForRouteMatch(t, routeName, parents, actual, namespaceRequired), nil }) require.NoErrorf(t, waitErr, "error waiting for %s to have parents matching expectations", routeTypeName) diff --git a/conformance/utils/kubernetes/helpers_test.go b/conformance/utils/kubernetes/helpers_test.go index 1a77a1c821..352cf2eb05 100644 --- a/conformance/utils/kubernetes/helpers_test.go +++ b/conformance/utils/kubernetes/helpers_test.go @@ -29,7 +29,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -379,3 +381,223 @@ func Test_listenersMatch(t *testing.T) { }) } } + +// TestRouteMustHaveParentsIgnoresStaleControllerEntries seeds an entry that is +// stale on purpose: it sits at generation 1 while the Route is at generation 2, +// so the wait only succeeds if entries owned by StaleControllerName are left out +// of the observedGeneration check. +func TestRouteMustHaveParentsIgnoresStaleControllerEntries(t *testing.T) { + const ownController = gatewayv1.GatewayController("example.com/gateway-controller") + + scheme := runtime.NewScheme() + require.NoError(t, InstallGatewayV1(scheme)) + + routeNN := types.NamespacedName{Name: "test-route", Namespace: "default"} + gwNamespace := gatewayv1.Namespace("default") + + acceptedCondition := func(observedGeneration int64) []metav1.Condition { + return []metav1.Condition{{ + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.RouteReasonAccepted), + ObservedGeneration: observedGeneration, + LastTransitionTime: metav1.Now(), + }} + } + + route := &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: routeNN.Name, + Namespace: routeNN.Namespace, + Generation: 2, + }, + Status: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{ + Name: "test-gateway", + Namespace: &gwNamespace, + }, + ControllerName: ownController, + Conditions: acceptedCondition(2), + }, + { + ParentRef: gatewayv1.ParentReference{ + Name: "unmanaged-gateway", + Namespace: &gwNamespace, + }, + ControllerName: StaleControllerName, + Conditions: acceptedCondition(1), + }, + }, + }, + }, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(route).Build() + + timeoutConfig := config.TimeoutConfig{ + RouteMustHaveParents: 2 * time.Second, + DefaultPollInterval: 100 * time.Millisecond, + } + + HTTPRouteMustHaveParents(t, c, timeoutConfig, routeNN, []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{ + Name: "test-gateway", + Namespace: &gwNamespace, + }, + ControllerName: ownController, + Conditions: []metav1.Condition{{ + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }, + }, true) +} + +func Test_staleParentStatus(t *testing.T) { + const ( + ownController = gatewayv1.GatewayController("example.com/gateway-controller") + otherController = gatewayv1.GatewayController("example.com/other-controller") + ) + + route := &gatewayv1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Generation: 2}} + + parent := func(controller gatewayv1.GatewayController, observedGeneration int64) gatewayv1.RouteParentStatus { + return gatewayv1.RouteParentStatus{ + ParentRef: gatewayv1.ParentReference{Name: "test-gateway"}, + ControllerName: controller, + Conditions: []metav1.Condition{{ + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.RouteReasonAccepted), + ObservedGeneration: observedGeneration, + }}, + } + } + + tests := []struct { + name string + parents []gatewayv1.RouteParentStatus + wantOwner gatewayv1.GatewayController + }{ + { + name: "every entry has caught up", + parents: []gatewayv1.RouteParentStatus{parent(ownController, 2), parent(otherController, 2)}, + }, + { + name: "own entry is behind", + parents: []gatewayv1.RouteParentStatus{parent(ownController, 1)}, + wantOwner: ownController, + }, + { + name: "sentinel entry is exempt while it is behind", + parents: []gatewayv1.RouteParentStatus{parent(ownController, 2), parent(StaleControllerName, 1)}, + }, + { + name: "the exemption does not extend to other foreign entries", + parents: []gatewayv1.RouteParentStatus{parent(ownController, 2), parent(otherController, 1)}, + wantOwner: otherController, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stale, err := staleParentStatus(route, tt.parents) + + if tt.wantOwner == "" { + require.NoError(t, err) + return + } + + require.Error(t, err) + require.Equal(t, tt.wantOwner, stale.ControllerName) + }) + } +} + +// TestRouteMustHaveParentsChecksTheStatusItJustRead pins the order of the two +// steps inside the poll: the status has to be read before the +// observedGeneration check runs against it. When the read came last, the first +// iteration checked an empty slice, so a Route whose conditions still carried a +// stale observedGeneration satisfied the wait right away and the check never +// ran at all on the common fast path. +func TestRouteMustHaveParentsChecksTheStatusItJustRead(t *testing.T) { + const ownController = gatewayv1.GatewayController("example.com/gateway-controller") + + scheme := runtime.NewScheme() + require.NoError(t, InstallGatewayV1(scheme)) + + routeNN := types.NamespacedName{Name: "test-route", Namespace: "default"} + gwNamespace := gatewayv1.Namespace("default") + + route := &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: routeNN.Name, + Namespace: routeNN.Namespace, + Generation: 2, + }, + Status: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{{ + ParentRef: gatewayv1.ParentReference{ + Name: "test-gateway", + Namespace: &gwNamespace, + }, + ControllerName: ownController, + Conditions: []metav1.Condition{{ + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.RouteReasonAccepted), + ObservedGeneration: 1, + LastTransitionTime: metav1.Now(), + }}, + }}, + }, + }, + } + + var reads int + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(route). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, cli client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if err := cli.Get(ctx, key, obj, opts...); err != nil { + return err + } + + // The implementation catches up, but only after the first read. + reads++ + if reads > 1 { + obj.(*gatewayv1.HTTPRoute).Status.Parents[0].Conditions[0].ObservedGeneration = 2 + } + + return nil + }, + }). + Build() + + timeoutConfig := config.TimeoutConfig{ + RouteMustHaveParents: 2 * time.Second, + DefaultPollInterval: 100 * time.Millisecond, + } + + HTTPRouteMustHaveParents(t, c, timeoutConfig, routeNN, []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{ + Name: "test-gateway", + Namespace: &gwNamespace, + }, + ControllerName: ownController, + Conditions: []metav1.Condition{{ + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }, + }, true) + + require.Greater(t, reads, 1, "wait was satisfied by the first read, leaving the stale observedGeneration unchecked") +} From 8bd8c8df99ec7cc056704db74df5653acbb4a7bd Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Sun, 30 Aug 2026 19:07:12 +0000 Subject: [PATCH 2/4] conformance: exempt seeded foreign status from observedGeneration checks Tests that seed a status entry owned by another controller need the status helpers to stop waiting for that entry to catch up with the object generation: nothing reconciles it, so it never does. Extend the StaleControllerName exemption RouteMustHaveParents already has to the BackendTLSPolicy ancestor helpers, and add a StaleConditionType exemption to FilterStaleConditions for plain condition lists (Gateway and Listener conditions have no controllerName to key on, so those tests seed a foreign condition type instead). Signed-off-by: Aleksei Sviridkin --- conformance/utils/kubernetes/helpers.go | 27 ++++++- conformance/utils/kubernetes/helpers_test.go | 78 ++++++++++++++++++++ 2 files changed, 101 insertions(+), 4 deletions(-) diff --git a/conformance/utils/kubernetes/helpers.go b/conformance/utils/kubernetes/helpers.go index c0ad27512d..c3eb8c7065 100644 --- a/conformance/utils/kubernetes/helpers.go +++ b/conformance/utils/kubernetes/helpers.go @@ -53,11 +53,17 @@ import ( const GatewayExcludedFromReadinessChecks = "gateway-api/skip-this-for-readiness" // StaleControllerName is a controllerName that tests can put on a Route -// status.parents entry to stand in for another implementation sharing the -// cluster. Nothing reconciles such an entry, so RouteMustHaveParents leaves its -// observedGeneration alone. +// status.parents or Policy status.ancestors entry to stand in for another +// implementation sharing the cluster. Nothing reconciles such an entry, so the +// status helpers leave its observedGeneration alone. const StaleControllerName = gatewayv1.GatewayController("gateway.networking.k8s.io/stale-controller") +// StaleConditionType is a condition type that tests can seed into a status +// conditions list to stand in for a condition owned by another controller. +// Nothing reconciles such a condition, so FilterStaleConditions leaves its +// observedGeneration alone. +const StaleConditionType = "conformance.gateway.networking.k8s.io/StaleCondition" + const GatewayKind = gatewayv1.Kind("Gateway") // GatewayRef is a tiny type for specifying an HTTP Route ParentRef without @@ -245,10 +251,15 @@ func ConditionsHaveLatestObservedGeneration(obj metav1.Object, conditions []meta } // FilterStaleConditions returns the list of status condition whose observedGeneration does not -// match the object's metadata.Generation +// match the object's metadata.Generation. Conditions of StaleConditionType are +// exempt: they belong to no controller, so they never catch up. func FilterStaleConditions(obj metav1.Object, conditions []metav1.Condition) []metav1.Condition { stale := make([]metav1.Condition, 0, len(conditions)) for _, condition := range conditions { + if condition.Type == StaleConditionType { + continue + } + if obj.GetGeneration() != condition.ObservedGeneration { stale = append(stale, condition) } @@ -1650,6 +1661,10 @@ func BackendTLSPolicyMustHaveCondition(t *testing.T, client client.Client, timeo } for _, parent := range policy.Status.Ancestors { + if parent.ControllerName == StaleControllerName { + continue + } + if err := ConditionsHaveLatestObservedGeneration(policy, parent.Conditions); err != nil { tlog.Logf(t, "BackendTLSPolicy %s (parentRef=%v) %v", policyNN, parentRefToString(parent.AncestorRef), err, @@ -1684,6 +1699,10 @@ func BackendTLSPolicyMustHaveLatestConditions(t *testing.T, r *gatewayv1.Backend t.Helper() for _, ancestor := range r.Status.Ancestors { + if ancestor.ControllerName == StaleControllerName { + continue + } + if err := ConditionsHaveLatestObservedGeneration(r, ancestor.Conditions); err != nil { tlog.Fatalf(t, "BackendTLSPolicy(controller=%v, ancestorRef=%#v) %v", ancestor.ControllerName, parentRefToString(ancestor.AncestorRef), err) } diff --git a/conformance/utils/kubernetes/helpers_test.go b/conformance/utils/kubernetes/helpers_test.go index 352cf2eb05..3326d07085 100644 --- a/conformance/utils/kubernetes/helpers_test.go +++ b/conformance/utils/kubernetes/helpers_test.go @@ -107,6 +107,23 @@ func TestVerifyConditionsMatchGeneration(t *testing.T) { {Type: "FakeCondition3", ObservedGeneration: 20}, }, }, + { + name: "a StaleConditionType condition is exempt from the generation check", + obj: &gatewayv1.Gateway{ObjectMeta: metav1.ObjectMeta{Name: "fake-gateway", Generation: 20}}, + conditions: []metav1.Condition{ + {Type: "FakeCondition1", ObservedGeneration: 20}, + {Type: StaleConditionType, ObservedGeneration: 3}, + }, + }, + { + name: "the StaleConditionType exemption does not extend to other conditions", + obj: &gatewayv1.Gateway{ObjectMeta: metav1.ObjectMeta{Name: "fake-gateway", Generation: 20}}, + conditions: []metav1.Condition{ + {Type: "FakeCondition1", ObservedGeneration: 19}, + {Type: StaleConditionType, ObservedGeneration: 3}, + }, + expected: fmt.Errorf("expected observedGeneration to be updated to 20 for all conditions, only 1/2 were updated. stale conditions are: FakeCondition1 (generation 19)"), + }, { name: "conditions where one does not match the generation fail verification", obj: &gatewayv1.Gateway{ObjectMeta: metav1.ObjectMeta{Name: "fake-gateway", Generation: 20}}, @@ -518,6 +535,67 @@ func Test_staleParentStatus(t *testing.T) { } } +// TestBackendTLSPolicyMustHaveConditionIgnoresStaleAncestors seeds an ancestor +// that is stale on purpose: it sits at generation 1 while the policy is at +// generation 2, so the wait only succeeds if ancestors owned by +// StaleControllerName are left out of the observedGeneration check. +func TestBackendTLSPolicyMustHaveConditionIgnoresStaleAncestors(t *testing.T) { + const ownController = gatewayv1.GatewayController("example.com/gateway-controller") + + scheme := runtime.NewScheme() + require.NoError(t, InstallGatewayV1(scheme)) + + policyNN := types.NamespacedName{Name: "test-policy", Namespace: "default"} + gwNN := types.NamespacedName{Name: "test-gateway", Namespace: "default"} + gwNamespace := gatewayv1.Namespace(gwNN.Namespace) + + ancestor := func(name gatewayv1.ObjectName, controller gatewayv1.GatewayController, observedGeneration int64) gatewayv1.PolicyAncestorStatus { + return gatewayv1.PolicyAncestorStatus{ + AncestorRef: gatewayv1.ParentReference{ + Name: name, + Namespace: &gwNamespace, + }, + ControllerName: controller, + Conditions: []metav1.Condition{{ + Type: string(gatewayv1.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.PolicyReasonAccepted), + ObservedGeneration: observedGeneration, + LastTransitionTime: metav1.Now(), + }}, + } + } + + policy := &gatewayv1.BackendTLSPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: policyNN.Name, + Namespace: policyNN.Namespace, + Generation: 2, + }, + Status: gatewayv1.PolicyStatus{ + Ancestors: []gatewayv1.PolicyAncestorStatus{ + ancestor(gatewayv1.ObjectName(gwNN.Name), ownController, 2), + ancestor("unmanaged-gateway", StaleControllerName, 1), + }, + }, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(policy).Build() + + timeoutConfig := config.TimeoutConfig{ + HTTPRouteMustHaveCondition: 2 * time.Second, + DefaultPollInterval: 100 * time.Millisecond, + } + + BackendTLSPolicyMustHaveCondition(t, c, timeoutConfig, policyNN, gwNN, metav1.Condition{ + Type: string(gatewayv1.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.PolicyReasonAccepted), + }) + + BackendTLSPolicyMustHaveLatestConditions(t, policy) +} + // TestRouteMustHaveParentsChecksTheStatusItJustRead pins the order of the two // steps inside the poll: the status has to be read before the // observedGeneration check runs against it. When the read came last, the first From 396b680453010e7c9aef5fc0eed3e7adcc81e151 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Sun, 30 Aug 2026 19:08:39 +0000 Subject: [PATCH 3/4] test(conformance): preserve BackendTLSPolicy ancestors owned by other controllers Same contract as the HTTPRoute foreign-status test, applied to the other status list that is scoped by controllerName: policy status.ancestors. The test seeds an ancestor entry under a stale controllerName, forces a generation bump, waits for the implementation's own ancestor to catch up, and requires the seeded entry to survive byte for byte. Signed-off-by: Aleksei Sviridkin --- ...ackendtlspolicy-preserve-foreign-status.go | 129 ++++++++++++++++++ ...kendtlspolicy-preserve-foreign-status.yaml | 55 ++++++++ 2 files changed, 184 insertions(+) create mode 100644 conformance/tests/backendtlspolicy-preserve-foreign-status.go create mode 100644 conformance/tests/backendtlspolicy-preserve-foreign-status.yaml diff --git a/conformance/tests/backendtlspolicy-preserve-foreign-status.go b/conformance/tests/backendtlspolicy-preserve-foreign-status.go new file mode 100644 index 0000000000..828358dae6 --- /dev/null +++ b/conformance/tests/backendtlspolicy-preserve-foreign-status.go @@ -0,0 +1,129 @@ +/* +Copyright 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 tests + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" + + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" + confsuite "sigs.k8s.io/gateway-api/conformance/utils/suite" + "sigs.k8s.io/gateway-api/pkg/features" +) + +func init() { + ConformanceTests = append(ConformanceTests, BackendTLSPolicyPreserveForeignStatus) +} + +var BackendTLSPolicyPreserveForeignStatus = confsuite.ConformanceTest{ + ShortName: "BackendTLSPolicyPreserveForeignStatus", + Description: "An implementation must not remove or modify BackendTLSPolicy status.ancestors " + + "entries whose controllerName belongs to another implementation.", + Features: []features.FeatureName{ + features.SupportGateway, + features.SupportHTTPRoute, + features.SupportBackendTLSPolicy, + }, + Manifests: []string{"tests/backendtlspolicy-preserve-foreign-status.yaml"}, + Provisional: true, + Test: func(t *testing.T, suite *confsuite.ConformanceTestSuite) { + ns := confsuite.InfrastructureNamespace + routeNN := types.NamespacedName{Name: "backendtlspolicy-preserve-foreign-status", Namespace: ns} + gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + policyNN := types.NamespacedName{Name: "backendtlspolicy-preserve-foreign-status", Namespace: ns} + + kubernetes.NamespacesMustBeReady(t, suite.Client, suite.TimeoutConfig, []string{ns}) + kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + kubernetes.BackendTLSPolicyMustHaveAcceptedConditionTrue(t, suite.Client, suite.TimeoutConfig, policyNN, gwNN) + + var seeded gatewayv1.PolicyAncestorStatus + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + policy := &gatewayv1.BackendTLSPolicy{} + if err := suite.Client.Get(t.Context(), policyNN, policy); err != nil { + return err + } + policy.Status.Ancestors = append(policy.Status.Ancestors, gatewayv1.PolicyAncestorStatus{ + AncestorRef: gatewayv1.ParentReference{ + Group: new(gatewayv1.Group(gatewayv1.GroupVersion.Group)), + Kind: new(gatewayv1.Kind("Gateway")), + Name: "unmanaged-gateway", + Namespace: new(gatewayv1.Namespace(ns)), + }, + ControllerName: kubernetes.StaleControllerName, + Conditions: []metav1.Condition{{ + Type: string(gatewayv1.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.PolicyReasonAccepted), + ObservedGeneration: policy.Generation, + LastTransitionTime: metav1.Now(), + }}, + }) + if err := suite.Client.Status().Update(t.Context(), policy); err != nil { + return err + } + // Read the entry back off the update response rather than reusing + // the value written above: the API server truncates + // lastTransitionTime to second precision. + stored := foreignAncestorStatus(policy.Status.Ancestors) + if stored == nil { + return errForeignStatusNotStored + } + seeded = *stored + return nil + }) + require.NoError(t, err, "error seeding a foreign controller's status entry on BackendTLSPolicy %s", policyNN) + + // Bump the generation so the implementation has to run a full + // read-modify-write cycle on a status that already holds the seeded entry. + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + policy := &gatewayv1.BackendTLSPolicy{} + if getErr := suite.Client.Get(t.Context(), policyNN, policy); getErr != nil { + return getErr + } + policy.Spec.Validation.Hostname = "second.example.com" + return suite.Client.Update(t.Context(), policy) + }) + require.NoError(t, err, "error updating BackendTLSPolicy %s", policyNN) + + // The implementation's own entry catching up to the bumped generation is + // what proves it wrote status while the seeded entry was there. + kubernetes.BackendTLSPolicyMustHaveAcceptedConditionTrue(t, suite.Client, suite.TimeoutConfig, policyNN, gwNN) + + policy := &gatewayv1.BackendTLSPolicy{} + require.NoError(t, suite.Client.Get(t.Context(), policyNN, policy), "error fetching BackendTLSPolicy %s", policyNN) + kubernetes.BackendTLSPolicyMustHaveLatestConditions(t, policy) + + stored := foreignAncestorStatus(policy.Status.Ancestors) + require.NotNilf(t, stored, "BackendTLSPolicy %s: status entry owned by %s was removed", policyNN, kubernetes.StaleControllerName) + require.Equalf(t, seeded, *stored, "BackendTLSPolicy %s: status entry owned by %s was modified", policyNN, kubernetes.StaleControllerName) + }, +} + +func foreignAncestorStatus(ancestors []gatewayv1.PolicyAncestorStatus) *gatewayv1.PolicyAncestorStatus { + for i := range ancestors { + if ancestors[i].ControllerName == kubernetes.StaleControllerName { + return &ancestors[i] + } + } + + return nil +} diff --git a/conformance/tests/backendtlspolicy-preserve-foreign-status.yaml b/conformance/tests/backendtlspolicy-preserve-foreign-status.yaml new file mode 100644 index 0000000000..6c4cae7543 --- /dev/null +++ b/conformance/tests/backendtlspolicy-preserve-foreign-status.yaml @@ -0,0 +1,55 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: backendtlspolicy-preserve-foreign-status + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + namespace: gateway-conformance-infra + hostnames: + - preserve.example.com + rules: + - backendRefs: + - group: "" + kind: Service + name: backendtlspolicy-preserve-foreign-status-test + port: 443 + matches: + - path: + type: Exact + value: /backendtlspolicy-preserve-foreign-status +--- +apiVersion: v1 +kind: Service +metadata: + name: backendtlspolicy-preserve-foreign-status-test + namespace: gateway-conformance-infra +spec: + selector: + app: tls-backend + ports: + - name: "btls" + protocol: TCP + port: 443 + targetPort: 8443 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: BackendTLSPolicy +metadata: + name: backendtlspolicy-preserve-foreign-status + namespace: gateway-conformance-infra +spec: + targetRefs: + - group: "" + kind: Service + name: "backendtlspolicy-preserve-foreign-status-test" + sectionName: "btls" + validation: + caCertificateRefs: + - group: "" + kind: ConfigMap + # This ConfigMap is generated dynamically by the test suite. + # It contains the CA certificate used to sign the tls-backend serving certificate. + name: "tls-checks-ca-certificate" + hostname: "abc.example.com" From 00da1c52a460e7ec7807f611508b1e85b0d5494d Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Sun, 30 Aug 2026 19:10:02 +0000 Subject: [PATCH 4/4] test(conformance): preserve foreign Gateway and Listener status conditions Gateway and Listener conditions carry no controllerName, so the cross-controller contract there is different from status.parents and status.ancestors: implementations must not remove, change or update conditions whose type they are not responsible for. The test seeds a condition of a foreign type on the Gateway and on one listener, forces a generation bump, waits for the implementation's own conditions to catch up on both levels, and requires the seeded conditions to survive byte for byte. Signed-off-by: Aleksei Sviridkin --- .../gateway-preserve-foreign-conditions.go | 179 ++++++++++++++++++ .../gateway-preserve-foreign-conditions.yaml | 14 ++ 2 files changed, 193 insertions(+) create mode 100644 conformance/tests/gateway-preserve-foreign-conditions.go create mode 100644 conformance/tests/gateway-preserve-foreign-conditions.yaml diff --git a/conformance/tests/gateway-preserve-foreign-conditions.go b/conformance/tests/gateway-preserve-foreign-conditions.go new file mode 100644 index 0000000000..fc75875d5f --- /dev/null +++ b/conformance/tests/gateway-preserve-foreign-conditions.go @@ -0,0 +1,179 @@ +/* +Copyright 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 tests + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/util/retry" + + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" + confsuite "sigs.k8s.io/gateway-api/conformance/utils/suite" + "sigs.k8s.io/gateway-api/pkg/features" +) + +var errForeignConditionNotStored = errors.New("the seeded condition is missing from the update response") + +func init() { + ConformanceTests = append(ConformanceTests, GatewayPreserveForeignConditions) +} + +var GatewayPreserveForeignConditions = confsuite.ConformanceTest{ + ShortName: "GatewayPreserveForeignConditions", + Description: "An implementation must not remove or modify Gateway and Listener status conditions " + + "whose type it is not responsible for.", + Features: []features.FeatureName{ + features.SupportGateway, + }, + Manifests: []string{"tests/gateway-preserve-foreign-conditions.yaml"}, + Provisional: true, + Test: func(t *testing.T, suite *confsuite.ConformanceTestSuite) { + ns := confsuite.InfrastructureNamespace + gwNN := types.NamespacedName{Name: "gateway-preserve-foreign-conditions", Namespace: ns} + listenerName := gatewayv1.SectionName("http") + + kubernetes.NamespacesMustBeReady(t, suite.Client, suite.TimeoutConfig, []string{ns}) + kubernetes.GatewayMustHaveLatestConditions(t, suite.Client, suite.TimeoutConfig, gwNN) + + // Listener status is where the second seeded condition goes, so its + // entry has to exist first. + waitErr := wait.PollUntilContextTimeout(context.Background(), suite.TimeoutConfig.DefaultPollInterval, suite.TimeoutConfig.GatewayStatusMustHaveListeners, true, func(ctx context.Context) (bool, error) { + gw := &gatewayv1.Gateway{} + if err := suite.Client.Get(ctx, gwNN, gw); err != nil { + return false, err + } + return listenerStatus(gw, listenerName) != nil, nil + }) + require.NoErrorf(t, waitErr, "error waiting for Gateway %s to report status for listener %s", gwNN, listenerName) + + foreignCondition := func(observedGeneration int64) metav1.Condition { + return metav1.Condition{ + Type: kubernetes.StaleConditionType, + Status: metav1.ConditionTrue, + Reason: "Seeded", + ObservedGeneration: observedGeneration, + LastTransitionTime: metav1.Now(), + } + } + + var seededGw, seededListener metav1.Condition + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + gw := &gatewayv1.Gateway{} + if err := suite.Client.Get(t.Context(), gwNN, gw); err != nil { + return err + } + ls := listenerStatus(gw, listenerName) + if ls == nil { + return errForeignConditionNotStored + } + gw.Status.Conditions = append(gw.Status.Conditions, foreignCondition(gw.Generation)) + ls.Conditions = append(ls.Conditions, foreignCondition(gw.Generation)) + if err := suite.Client.Status().Update(t.Context(), gw); err != nil { + return err + } + // Read the conditions back off the update response rather than + // reusing the values written above: the API server truncates + // lastTransitionTime to second precision. + storedGw := foreignConditionIn(gw.Status.Conditions) + ls = listenerStatus(gw, listenerName) + if storedGw == nil || ls == nil { + return errForeignConditionNotStored + } + storedListener := foreignConditionIn(ls.Conditions) + if storedListener == nil { + return errForeignConditionNotStored + } + seededGw = *storedGw + seededListener = *storedListener + return nil + }) + require.NoError(t, err, "error seeding foreign conditions on Gateway %s", gwNN) + + // Bump the generation so the implementation has to run a full + // read-modify-write cycle on a status that already holds the seeded + // conditions. + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + gw := &gatewayv1.Gateway{} + if getErr := suite.Client.Get(t.Context(), gwNN, gw); getErr != nil { + return getErr + } + gw.Spec.Listeners[0].Hostname = new(gatewayv1.Hostname("second.example.com")) + return suite.Client.Update(t.Context(), gw) + }) + require.NoError(t, err, "error updating Gateway %s", gwNN) + + // The implementation's own conditions catching up to the bumped + // generation, on the Gateway and on every listener, is what proves it + // wrote status while the seeded conditions were there. + waitErr = wait.PollUntilContextTimeout(context.Background(), suite.TimeoutConfig.DefaultPollInterval, suite.TimeoutConfig.LatestObservedGenerationSet, true, func(ctx context.Context) (bool, error) { + gw := &gatewayv1.Gateway{} + if err := suite.Client.Get(ctx, gwNN, gw); err != nil { + return false, err + } + if err := kubernetes.ConditionsHaveLatestObservedGeneration(gw, gw.Status.Conditions); err != nil { + return false, nil + } + for _, ls := range gw.Status.Listeners { + if err := kubernetes.ConditionsHaveLatestObservedGeneration(gw, ls.Conditions); err != nil { + return false, nil + } + } + return true, nil + }) + require.NoErrorf(t, waitErr, "error waiting for Gateway %s conditions to catch up with the bumped generation", gwNN) + + gw := &gatewayv1.Gateway{} + require.NoError(t, suite.Client.Get(t.Context(), gwNN, gw), "error fetching Gateway %s", gwNN) + + stored := foreignConditionIn(gw.Status.Conditions) + require.NotNilf(t, stored, "Gateway %s: the %s condition was removed", gwNN, kubernetes.StaleConditionType) + require.Equalf(t, seededGw, *stored, "Gateway %s: the %s condition was modified", gwNN, kubernetes.StaleConditionType) + + ls := listenerStatus(gw, listenerName) + require.NotNilf(t, ls, "Gateway %s: status for listener %s is missing", gwNN, listenerName) + stored = foreignConditionIn(ls.Conditions) + require.NotNilf(t, stored, "Gateway %s listener %s: the %s condition was removed", gwNN, listenerName, kubernetes.StaleConditionType) + require.Equalf(t, seededListener, *stored, "Gateway %s listener %s: the %s condition was modified", gwNN, listenerName, kubernetes.StaleConditionType) + }, +} + +func listenerStatus(gw *gatewayv1.Gateway, name gatewayv1.SectionName) *gatewayv1.ListenerStatus { + for i := range gw.Status.Listeners { + if gw.Status.Listeners[i].Name == name { + return &gw.Status.Listeners[i] + } + } + + return nil +} + +func foreignConditionIn(conditions []metav1.Condition) *metav1.Condition { + for i := range conditions { + if conditions[i].Type == kubernetes.StaleConditionType { + return &conditions[i] + } + } + + return nil +} diff --git a/conformance/tests/gateway-preserve-foreign-conditions.yaml b/conformance/tests/gateway-preserve-foreign-conditions.yaml new file mode 100644 index 0000000000..8ac847f592 --- /dev/null +++ b/conformance/tests/gateway-preserve-foreign-conditions.yaml @@ -0,0 +1,14 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: gateway-preserve-foreign-conditions + namespace: gateway-conformance-infra +spec: + gatewayClassName: "{GATEWAY_CLASS_NAME}" + listeners: + - name: http + port: 80 + protocol: HTTP + allowedRoutes: + namespaces: + from: Same