Skip to content

Commit

Permalink
fix: do not create two Detached conditions when no port for the proto…
Browse files Browse the repository at this point in the history
…col available (#3257)

This prevents creating two Detached conditions for Listener that has no matching protocol in kongProtocolsToPort.
  • Loading branch information
czeslavo authored Dec 13, 2022
1 parent 6a03026 commit f5c2ddc
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 13 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ Adding a new version? You'll need three changes:
[#3132](https:/Kong/kubernetes-ingress-controller/pull/3132)

### Deprecated

- KongIngress' `proxy` and `route` fields are now deprecated in favor of
Service and Ingress annotations. The annotations will become the only
means of configuring those settings in 3.0 release.
Expand Down Expand Up @@ -200,6 +201,9 @@ Adding a new version? You'll need three changes:
if a route specifies `sectionName` in parentRefs, and no listener can
match the specified name, the route is not accepted.
[#3230](https:/Kong/kubernetes-ingress-controller/pull/3230)
- If there's no matching Kong listener for a protocol specified in a Gateway's
Listener, only one `Detached` condition is created in the Listener's status.
[#3257](https:/Kong/kubernetes-ingress-controller/pull/3257)

## [2.7.0]

Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/gateway/gateway_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l
// a single set of shared listens. We lack knowledge of whether this is compatible with user intent, and it may
// be incompatible with the spec, so we should consider evaluating cross-Gateway compatibility and raising error
// conditions in the event of a problem
listenerStatuses, err := r.getListenerStatus(ctx, gateway, kongListeners, referenceGrantList.Items)
listenerStatuses, err := getListenerStatus(ctx, gateway, kongListeners, referenceGrantList.Items, r.Client)
if err != nil {
return ctrl.Result{}, err
}
Expand Down
26 changes: 14 additions & 12 deletions internal/controllers/gateway/gateway_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,11 +277,12 @@ func canSharePort(requested, existing ProtocolType) bool {
}
}

func (r *GatewayReconciler) getListenerStatus(
func getListenerStatus(
ctx context.Context,
gateway *Gateway,
kongListens []Listener,
referenceGrants []gatewayv1alpha2.ReferenceGrant,
client client.Client,
) ([]ListenerStatus, error) {
statuses := make(map[SectionName]ListenerStatus, len(gateway.Spec.Listeners))
portToProtocol, portToHostname, listenerToAttached := initializeListenerMaps(gateway)
Expand Down Expand Up @@ -361,16 +362,17 @@ func (r *GatewayReconciler) getListenerStatus(
Reason: string(gatewayv1alpha2.ListenerReasonUnsupportedProtocol),
Message: "no Kong listen with the requested protocol is configured",
})
}
if _, ok := kongProtocolsToPort[listener.Protocol][listener.Port]; !ok {
status.Conditions = append(status.Conditions, metav1.Condition{
Type: string(gatewayv1alpha2.ListenerConditionDetached),
Status: metav1.ConditionTrue,
ObservedGeneration: gateway.Generation,
LastTransitionTime: metav1.Now(),
Reason: string(gatewayv1alpha2.ListenerReasonPortUnavailable),
Message: "no Kong listen with the requested protocol is configured for the requested port",
})
} else {
if _, ok := kongProtocolsToPort[listener.Protocol][listener.Port]; !ok {
status.Conditions = append(status.Conditions, metav1.Condition{
Type: string(gatewayv1alpha2.ListenerConditionDetached),
Status: metav1.ConditionTrue,
ObservedGeneration: gateway.Generation,
LastTransitionTime: metav1.Now(),
Reason: string(gatewayv1alpha2.ListenerReasonPortUnavailable),
Message: "no Kong listen with the requested protocol is configured for the requested port",
})
}
}

// finalize adding any general conditions
Expand Down Expand Up @@ -468,7 +470,7 @@ func (r *GatewayReconciler) getListenerStatus(
if certRef.Namespace != nil {
secretNamespace = string(*certRef.Namespace)
}
if err := r.Client.Get(ctx, types.NamespacedName{Namespace: secretNamespace, Name: string(certRef.Name)}, secret); err != nil {
if err := client.Get(ctx, types.NamespacedName{Namespace: secretNamespace, Name: string(certRef.Name)}, secret); err != nil {
if !k8serrors.IsNotFound(err) {
return nil, err
}
Expand Down
35 changes: 35 additions & 0 deletions internal/controllers/gateway/gateway_utils_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package gateway

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/kong/kubernetes-ingress-controller/v2/internal/util/address"
Expand Down Expand Up @@ -84,3 +88,34 @@ func TestGetListenerSupportedRouteKinds(t *testing.T) {
})
}
}

func TestGetListenerStatus_no_duplicated_Detached_condition(t *testing.T) {
ctx := context.Background()
client := fake.NewClientBuilder().Build()

statuses, err := getListenerStatus(ctx, &Gateway{
Spec: gatewayv1beta1.GatewaySpec{
GatewayClassName: "kong",
Listeners: []gatewayv1beta1.Listener{
{
Port: 80,
Protocol: "TCP",
},
},
},
}, nil, nil, client)
require.NoError(t, err)
require.Len(t, statuses, 1, "only one listener status expected as only one listener was defined")
listenerStatus := statuses[0]
assertOnlyOneConditionOfType(t, listenerStatus.Conditions, gatewayv1beta1.ListenerConditionDetached)
}

func assertOnlyOneConditionOfType(t *testing.T, conditions []metav1.Condition, typ gatewayv1beta1.ListenerConditionType) {
conditionNum := 0
for _, condition := range conditions {
if condition.Type == string(typ) {
conditionNum++
}
}
assert.Equal(t, 1, conditionNum)
}

0 comments on commit f5c2ddc

Please sign in to comment.