Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove check for reference grant for route to gateway #2283

Merged
merged 3 commits into from
Jun 6, 2023
Merged

Remove check for reference grant for route to gateway #2283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee8c769
Remove check for reference grant for route to gateway
jm96441n Jun 6, 2023
abc79e2
Fix tenancy tests
jm96441n Jun 6, 2023
b8937af
Final cleaning up of acceptance test
jm96441n Jun 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions acceptance/tests/api-gateway/api_gateway_tenancy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ import (
"time"

terratestk8s "github.com/gruntwork-io/terratest/modules/k8s"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/hashicorp/consul-k8s/acceptance/framework/config"
"github.com/hashicorp/consul-k8s/acceptance/framework/consul"
"github.com/hashicorp/consul-k8s/acceptance/framework/environment"
Expand All @@ -28,11 +34,6 @@ import (
"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
)

var (
Expand Down Expand Up @@ -142,7 +143,7 @@ func TestAPIGateway_Tenancy(t *testing.T) {
checkStatusCondition(r, gateway.Status.Conditions, falseCondition("Synced", "SyncError"))

require.Len(r, gateway.Status.Listeners, 1)
require.EqualValues(r, 0, gateway.Status.Listeners[0].AttachedRoutes)
require.EqualValues(r, 1, gateway.Status.Listeners[0].AttachedRoutes)
checkStatusCondition(r, gateway.Status.Listeners[0].Conditions, trueCondition("Accepted", "Accepted"))
checkStatusCondition(r, gateway.Status.Listeners[0].Conditions, falseCondition("Conflicted", "NoConflicts"))
checkStatusCondition(r, gateway.Status.Listeners[0].Conditions, falseCondition("ResolvedRefs", "RefNotPermitted"))
Expand All @@ -162,19 +163,15 @@ func TestAPIGateway_Tenancy(t *testing.T) {
require.EqualValues(r, "gateway", httproute.Status.Parents[0].ParentRef.Name)
require.NotNil(r, httproute.Status.Parents[0].ParentRef.Namespace)
require.EqualValues(r, gatewayNamespace, *httproute.Status.Parents[0].ParentRef.Namespace)
checkStatusCondition(r, httproute.Status.Parents[0].Conditions, falseCondition("Accepted", "RefNotPermitted"))
checkStatusCondition(r, httproute.Status.Parents[0].Conditions, trueCondition("Accepted", "Accepted"))
checkStatusCondition(r, httproute.Status.Parents[0].Conditions, falseCondition("ResolvedRefs", "RefNotPermitted"))
})

// since we're not bound to anything, check to make sure that the route doesn't get created in Consul.
checkConsulNotExists(t, consulClient, api.HTTPRoute, "route", namespaceForConsul(c.namespaceMirroring, routeNamespace))

// we only sync validly referenced certificates over, so check to make sure it is not created.
checkConsulNotExists(t, consulClient, api.InlineCertificate, "certificate", namespaceForConsul(c.namespaceMirroring, certificateNamespace))

// now create reference grants
createReferenceGrant(t, k8sClient, "gateway-certificate", gatewayNamespace, certificateNamespace)
createReferenceGrant(t, k8sClient, "route-gateway", routeNamespace, gatewayNamespace)
createReferenceGrant(t, k8sClient, "route-service", routeNamespace, serviceNamespace)

// gateway updated with references allowed
Expand Down
24 changes: 12 additions & 12 deletions control-plane/api-gateway/binding/binder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,17 @@ import (

logrtest "github.com/go-logr/logr/testing"
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
"github.com/hashicorp/consul/api"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
"github.com/hashicorp/consul/api"
)

func init() {
Expand All @@ -34,9 +35,8 @@ func init() {
}

const (
testGatewayClassName = "gateway-class"
testControllerName = "test-controller"
routeListenerReferenceGrantErrorMessage = `http-listener-allowed-selector: reference not permitted due to lack of ReferenceGrant; http-listener-default-same: reference not permitted due to lack of ReferenceGrant; http-listener-explicit-all-allowed: reference not permitted due to lack of ReferenceGrant; http-listener-explicit-allowed-same: reference not permitted due to lack of ReferenceGrant; http-listener-hostname: reference not permitted due to lack of ReferenceGrant; http-listener-mismatched-kind-allowed: reference not permitted due to lack of ReferenceGrant; http-listener-tls: reference not permitted due to lack of ReferenceGrant; tcp-listener-allowed-selector: reference not permitted due to lack of ReferenceGrant; tcp-listener-default-same: reference not permitted due to lack of ReferenceGrant; tcp-listener-explicit-all-allowed: reference not permitted due to lack of ReferenceGrant; tcp-listener-explicit-allowed-same: reference not permitted due to lack of ReferenceGrant; tcp-listener-mismatched-kind-allowed: reference not permitted due to lack of ReferenceGrant; tcp-listener-tls: reference not permitted due to lack of ReferenceGrant`
testGatewayClassName = "gateway-class"
testControllerName = "test-controller"
)

var (
Expand Down Expand Up @@ -1113,9 +1113,9 @@ func TestBinder_BindingRulesKitchenSink(t *testing.T) {
Message: "resolved backend references",
}, {
Type: "Accepted",
Status: metav1.ConditionFalse,
Reason: "NotAllowedByListeners",
Message: routeListenerReferenceGrantErrorMessage,
Status: metav1.ConditionTrue,
Reason: "Accepted",
Message: "route accepted",
},
}},
}),
Expand Down Expand Up @@ -1628,9 +1628,9 @@ func TestBinder_BindingRulesKitchenSink(t *testing.T) {
Message: "resolved backend references",
}, {
Type: "Accepted",
Status: metav1.ConditionFalse,
Reason: "NotAllowedByListeners",
Message: routeListenerReferenceGrantErrorMessage,
Status: metav1.ConditionTrue,
Reason: "Accepted",
Message: "route accepted",
},
}},
}),
Expand Down
31 changes: 2 additions & 29 deletions control-plane/api-gateway/binding/reference_grant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
package binding

import (
"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
)

type referenceValidator struct {
Expand Down Expand Up @@ -45,20 +46,6 @@ func (rv *referenceValidator) GatewayCanReferenceSecret(gateway gwv1beta1.Gatewa
return rv.referenceAllowed(fromGK, fromNS, toGK, toNS, string(secretRef.Name))
}

func (rv *referenceValidator) HTTPRouteCanReferenceGateway(httproute gwv1beta1.HTTPRoute, parentRef gwv1beta1.ParentReference) bool {
fromNS := httproute.GetNamespace()
fromGK := metav1.GroupKind{
Group: httproute.GroupVersionKind().Group,
Kind: httproute.GroupVersionKind().Kind,
}

// Kind should default to Gateway if not set
// https://github.com/kubernetes-sigs/gateway-api/blob/v0.6.2/apis/v1beta1/shared_types.go#L48
toNS, toGK := createValuesFromRef(parentRef.Namespace, parentRef.Group, parentRef.Kind, common.BetaGroup, common.KindGateway)

return rv.referenceAllowed(fromGK, fromNS, toGK, toNS, string(parentRef.Name))
}

func (rv *referenceValidator) HTTPRouteCanReferenceBackend(httproute gwv1beta1.HTTPRoute, backendRef gwv1beta1.BackendRef) bool {
fromNS := httproute.GetNamespace()
fromGK := metav1.GroupKind{
Expand All @@ -73,20 +60,6 @@ func (rv *referenceValidator) HTTPRouteCanReferenceBackend(httproute gwv1beta1.H
return rv.referenceAllowed(fromGK, fromNS, toGK, toNS, string(backendRef.Name))
}

func (rv *referenceValidator) TCPRouteCanReferenceGateway(tcpRoute gwv1alpha2.TCPRoute, parentRef gwv1beta1.ParentReference) bool {
fromNS := tcpRoute.GetNamespace()
fromGK := metav1.GroupKind{
Group: tcpRoute.GroupVersionKind().Group,
Kind: tcpRoute.GroupVersionKind().Kind,
}

// Kind should default to Gateway if not set
// https://github.com/kubernetes-sigs/gateway-api/blob/v0.6.2/apis/v1beta1/shared_types.go#L48
toNS, toGK := createValuesFromRef(parentRef.Namespace, parentRef.Group, parentRef.Kind, common.BetaGroup, common.KindGateway)

return rv.referenceAllowed(fromGK, fromNS, toGK, toNS, string(parentRef.Name))
}

func (rv *referenceValidator) TCPRouteCanReferenceBackend(tcpRoute gwv1alpha2.TCPRoute, backendRef gwv1beta1.BackendRef) bool {
fromNS := tcpRoute.GetNamespace()
fromGK := metav1.GroupKind{
Expand Down
158 changes: 0 additions & 158 deletions control-plane/api-gateway/binding/reference_grant_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,85 +105,6 @@ func TestGatewayCanReferenceSecret(t *testing.T) {
}
}

func TestHTTPRouteCanReferenceGateway(t *testing.T) {
t.Parallel()

objName := gwv1beta1.ObjectName("mygateway")

basicValidReferenceGrant := gwv1beta1.ReferenceGrant{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Namespace: ToNamespace,
},
Spec: gwv1beta1.ReferenceGrantSpec{
From: []gwv1beta1.ReferenceGrantFrom{
{
Group: Group,
Kind: HTTPRouteKind,
Namespace: FromNamespace,
},
},
To: []gwv1beta1.ReferenceGrantTo{
{
Group: Group,
Kind: GatewayKind,
Name: &objName,
},
},
},
}

gatewayRefGroup := gwv1beta1.Group(Group)
gatewayRefKind := gwv1beta1.Kind(GatewayKind)
gatewayRefNamespace := gwv1beta1.Namespace(ToNamespace)

cases := map[string]struct {
canReference bool
err error
ctx context.Context
httpRoute gwv1beta1.HTTPRoute
gatewayRef gwv1beta1.ParentReference
k8sReferenceGrants []gwv1beta1.ReferenceGrant
}{
"httproute allowed to gateway": {
canReference: true,
err: nil,
ctx: context.TODO(),
httpRoute: gwv1beta1.HTTPRoute{
TypeMeta: metav1.TypeMeta{
Kind: HTTPRouteKind,
APIVersion: Group + V1Beta1,
},
ObjectMeta: metav1.ObjectMeta{
Namespace: FromNamespace,
},
Spec: gwv1beta1.HTTPRouteSpec{},
Status: gwv1beta1.HTTPRouteStatus{},
},
gatewayRef: gwv1beta1.ParentReference{
Group: &gatewayRefGroup,
Kind: &gatewayRefKind,
Namespace: &gatewayRefNamespace,
Name: objName,
SectionName: nil,
Port: nil,
},
k8sReferenceGrants: []gwv1beta1.ReferenceGrant{
basicValidReferenceGrant,
},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
rv := NewReferenceValidator(tc.k8sReferenceGrants)
canReference := rv.HTTPRouteCanReferenceGateway(tc.httpRoute, tc.gatewayRef)

require.Equal(t, tc.canReference, canReference)
})
}
}

func TestHTTPRouteCanReferenceBackend(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -265,85 +186,6 @@ func TestHTTPRouteCanReferenceBackend(t *testing.T) {
}
}

func TestTCPRouteCanReferenceGateway(t *testing.T) {
t.Parallel()

objName := gwv1beta1.ObjectName("mygateway")

basicValidReferenceGrant := gwv1beta1.ReferenceGrant{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Namespace: ToNamespace,
},
Spec: gwv1beta1.ReferenceGrantSpec{
From: []gwv1beta1.ReferenceGrantFrom{
{
Group: Group,
Kind: TCPRouteKind,
Namespace: FromNamespace,
},
},
To: []gwv1beta1.ReferenceGrantTo{
{
Group: Group,
Kind: GatewayKind,
Name: &objName,
},
},
},
}

gatewayRefGroup := gwv1beta1.Group(Group)
gatewayRefKind := gwv1beta1.Kind(GatewayKind)
gatewayRefNamespace := gwv1beta1.Namespace(ToNamespace)

cases := map[string]struct {
canReference bool
err error
ctx context.Context
tcpRoute gwv1alpha2.TCPRoute
gatewayRef gwv1beta1.ParentReference
k8sReferenceGrants []gwv1beta1.ReferenceGrant
}{
"tcpRoute allowed to gateway": {
canReference: true,
err: nil,
ctx: context.TODO(),
tcpRoute: gwv1alpha2.TCPRoute{
TypeMeta: metav1.TypeMeta{
Kind: TCPRouteKind,
APIVersion: Group + V1Alpha2,
},
ObjectMeta: metav1.ObjectMeta{
Namespace: FromNamespace,
},
Spec: gwv1alpha2.TCPRouteSpec{},
Status: gwv1alpha2.TCPRouteStatus{},
},
gatewayRef: gwv1beta1.ParentReference{
Group: &gatewayRefGroup,
Kind: &gatewayRefKind,
Namespace: &gatewayRefNamespace,
Name: objName,
SectionName: nil,
Port: nil,
},
k8sReferenceGrants: []gwv1beta1.ReferenceGrant{
basicValidReferenceGrant,
},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
rv := NewReferenceValidator(tc.k8sReferenceGrants)
canReference := rv.TCPRouteCanReferenceGateway(tc.tcpRoute, tc.gatewayRef)

require.Equal(t, tc.canReference, canReference)
})
}
}

func TestTCPRouteCanReferenceBackend(t *testing.T) {
t.Parallel()

Expand Down
15 changes: 6 additions & 9 deletions control-plane/api-gateway/binding/result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,18 @@ import (
"strings"

mapset "github.com/deckarep/golang-set"
"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
)

var (
// override function for tests.
timeFunc = metav1.Now
"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
)

var (
// This is used for any error related to a lack of proper reference grant creation.
errRefNotPermitted = errors.New("reference not permitted due to lack of ReferenceGrant")
)
// override function for tests.
var timeFunc = metav1.Now

// This is used for any error related to a lack of proper reference grant creation.
var errRefNotPermitted = errors.New("reference not permitted due to lack of ReferenceGrant")

var (
// Each of the below are specified in the Gateway spec under RouteConditionReason
Expand Down
Loading