Skip to content

Commit

Permalink
chore: clean up cross ns checking for policies (envoyproxy#3961)
Browse files Browse the repository at this point in the history
* clean up cross ns checking for policies

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix lint

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
zhaohuabing authored Jul 30, 2024
1 parent 04c5ce2 commit 36764c0
Showing 6 changed files with 13 additions and 147 deletions.
29 changes: 2 additions & 27 deletions internal/gatewayapi/backendtrafficpolicy.go
Original file line number Diff line number Diff line change
@@ -221,11 +221,10 @@ func (t *Translator) ProcessBackendTrafficPolicies(backendTrafficPolicies []*egv
}

func resolveBTPolicyGatewayTargetRef(policy *egv1a1.BackendTrafficPolicy, target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName, gateways map[types.NamespacedName]*policyGatewayTargetContext) (*GatewayContext, *status.PolicyResolveError) {
targetNs := policy.Namespace
// Check if the gateway exists
key := types.NamespacedName{
Name: string(target.Name),
Namespace: targetNs,
Namespace: policy.Namespace,
}
gateway, ok := gateways[key]

@@ -234,17 +233,6 @@ func resolveBTPolicyGatewayTargetRef(policy *egv1a1.BackendTrafficPolicy, target
return nil, nil
}

// Ensure Policy and target are in the same namespace
if policy.Namespace != targetNs {
message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, BackendTrafficPolicy can only target a resource in the same namespace.",
policy.Namespace, targetNs)

return gateway.GatewayContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonInvalid,
Message: message,
}
}

// Check if another policy targeting the same Gateway exists
if gateway.attached {
message := fmt.Sprintf("Unable to target Gateway %s, another BackendTrafficPolicy has already attached to it",
@@ -264,13 +252,11 @@ func resolveBTPolicyGatewayTargetRef(policy *egv1a1.BackendTrafficPolicy, target
}

func resolveBTPolicyRouteTargetRef(policy *egv1a1.BackendTrafficPolicy, target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName, routes map[policyTargetRouteKey]*policyRouteTargetContext) (RouteContext, *status.PolicyResolveError) {
targetNs := policy.Namespace

// Check if the route exists
key := policyTargetRouteKey{
Kind: string(target.Kind),
Name: string(target.Name),
Namespace: targetNs,
Namespace: policy.Namespace,
}

route, ok := routes[key]
@@ -279,17 +265,6 @@ func resolveBTPolicyRouteTargetRef(policy *egv1a1.BackendTrafficPolicy, target g
return nil, nil
}

// Ensure Policy and target are in the same namespace
if policy.Namespace != targetNs {
message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, BackendTrafficPolicy can only target a resource in the same namespace.",
policy.Namespace, targetNs)

return route.RouteContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonInvalid,
Message: message,
}
}

// Check if another policy targeting the same xRoute exists
if route.attached {
message := fmt.Sprintf("Unable to target %s %s, another BackendTrafficPolicy has already attached to it",
15 changes: 1 addition & 14 deletions internal/gatewayapi/clienttrafficpolicy.go
Original file line number Diff line number Diff line change
@@ -292,12 +292,10 @@ func resolveCTPolicyTargetRef(
targetRef *gwapiv1a2.LocalPolicyTargetReferenceWithSectionName,
gateways map[types.NamespacedName]*policyGatewayTargetContext,
) (*GatewayContext, *status.PolicyResolveError) {
targetNs := policy.Namespace

// Check if the gateway exists
key := types.NamespacedName{
Name: string(targetRef.Name),
Namespace: targetNs,
Namespace: policy.Namespace,
}
gateway, ok := gateways[key]

@@ -306,17 +304,6 @@ func resolveCTPolicyTargetRef(
return nil, nil
}

// Ensure Policy and target Gateway are in the same namespace
if policy.Namespace != targetNs {
message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, ClientTrafficPolicy can only target a Gateway in the same namespace.",
policy.Namespace, targetNs)

return gateway.GatewayContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonInvalid,
Message: message,
}
}

// If sectionName is set, make sure its valid
sectionName := targetRef.SectionName
if sectionName != nil {
30 changes: 2 additions & 28 deletions internal/gatewayapi/envoyextensionpolicy.go
Original file line number Diff line number Diff line change
@@ -222,12 +222,10 @@ func (t *Translator) ProcessEnvoyExtensionPolicies(envoyExtensionPolicies []*egv
}

func resolveEEPolicyGatewayTargetRef(policy *egv1a1.EnvoyExtensionPolicy, target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName, gateways map[types.NamespacedName]*policyGatewayTargetContext) (*GatewayContext, *status.PolicyResolveError) {
targetNs := policy.Namespace

// Check if the gateway exists
key := types.NamespacedName{
Name: string(target.Name),
Namespace: targetNs,
Namespace: policy.Namespace,
}
gateway, ok := gateways[key]

@@ -236,17 +234,6 @@ func resolveEEPolicyGatewayTargetRef(policy *egv1a1.EnvoyExtensionPolicy, target
return nil, nil
}

// Ensure Policy and target are in the same namespace
if policy.Namespace != targetNs {
message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, EnvoyExtensionPolicy can only target a resource in the same namespace.",
policy.Namespace, targetNs)

return gateway.GatewayContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonInvalid,
Message: message,
}
}

// Check if another policy targeting the same Gateway exists
if gateway.attached {
message := fmt.Sprintf("Unable to target Gateway %s, another EnvoyExtensionPolicy has already attached to it",
@@ -266,13 +253,11 @@ func resolveEEPolicyGatewayTargetRef(policy *egv1a1.EnvoyExtensionPolicy, target
}

func resolveEEPolicyRouteTargetRef(policy *egv1a1.EnvoyExtensionPolicy, target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName, routes map[policyTargetRouteKey]*policyRouteTargetContext) (RouteContext, *status.PolicyResolveError) {
targetNs := policy.Namespace

// Check if the route exists
key := policyTargetRouteKey{
Kind: string(target.Kind),
Name: string(target.Name),
Namespace: targetNs,
Namespace: policy.Namespace,
}

route, ok := routes[key]
@@ -281,17 +266,6 @@ func resolveEEPolicyRouteTargetRef(policy *egv1a1.EnvoyExtensionPolicy, target g
return nil, nil
}

// Ensure Policy and target are in the same namespace
if policy.Namespace != targetNs {
message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, EnvoyExtensionPolicy can only target a resource in the same namespace.",
policy.Namespace, targetNs)

return route.RouteContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonInvalid,
Message: message,
}
}

// Check if another policy targeting the same xRoute exists
if route.attached {
message := fmt.Sprintf("Unable to target %s %s, another EnvoyExtensionPolicy has already attached to it",
23 changes: 1 addition & 22 deletions internal/gatewayapi/envoypatchpolicy.go
Original file line number Diff line number Diff line change
@@ -33,8 +33,6 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
irKey string
)

targetNs := policy.Namespace

if t.MergeGateways {
targetKind = KindGatewayClass
irKey = string(t.GatewayClassName)
@@ -49,7 +47,7 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
} else {
targetKind = KindGateway
gatewayNN := types.NamespacedName{
Namespace: targetNs,
Namespace: policy.Namespace,
Name: string(policy.Spec.TargetRef.Name),
}
// It must exist since the gateways have already been processed
@@ -109,25 +107,6 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
continue
}

// Ensure EnvoyPatchPolicy and target Gateway are in the same namespace
if policy.Namespace != targetNs {
message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, EnvoyPatchPolicy can only target a %s in the same namespace.",
policy.Namespace, targetNs, targetKind)

resolveErr = &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonInvalid,
Message: message,
}
status.SetResolveErrorForPolicyAncestors(&policy.Status,
ancestorRefs,
t.GatewayControllerName,
policy.Generation,
resolveErr,
)

continue
}

// Save the patch
for _, patch := range policy.Spec.JSONPatches {
irPatch := ir.JSONPatchConfig{}
31 changes: 5 additions & 26 deletions internal/gatewayapi/extensionserverpolicy.go
Original file line number Diff line number Diff line change
@@ -14,9 +14,7 @@ import (

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
gwapiv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/internal/gatewayapi/status"
@@ -63,18 +61,12 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst
}

// Negative statuses have already been assigned so its safe to skip
gateway, resolveErr := resolveExtServerPolicyGatewayTargetRef(policy, currTarget, gatewayMap)
gateway := resolveExtServerPolicyGatewayTargetRef(policy, currTarget, gatewayMap)
if gateway == nil {
// unable to find a matching Gateway for policy
continue
}

// Skip the gateway. Don't add anything to the policy status.
if resolveErr != nil {
// The targetRef part is somehow wrong, this policy can't be attached.
continue
}

// Set conditions for translation if it got any
if t.translateExtServerPolicyForGateway(policy, gateway, currTarget, xdsIR) {
// Set Accepted condition if it is unset
@@ -125,33 +117,20 @@ func policyStatusToUnstructured(policyStatus gwapiv1a2.PolicyStatus) map[string]
return ret
}

func resolveExtServerPolicyGatewayTargetRef(policy *unstructured.Unstructured, target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName, gateways map[types.NamespacedName]*policyGatewayTargetContext) (*GatewayContext, *status.PolicyResolveError) {
targetNs := ptr.To(gwapiv1b1.Namespace(policy.GetNamespace()))

func resolveExtServerPolicyGatewayTargetRef(policy *unstructured.Unstructured, target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName, gateways map[types.NamespacedName]*policyGatewayTargetContext) *GatewayContext {
// Check if the gateway exists
key := types.NamespacedName{
Name: string(target.Name),
Namespace: string(*targetNs),
Namespace: policy.GetNamespace(),
}
gateway, ok := gateways[key]

// Gateway not found
if !ok {
return nil, nil
}

// Ensure Policy and target are in the same namespace
if policy.GetNamespace() != string(*targetNs) {
message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, extension server policies can only target a resource in the same namespace.",
policy.GetNamespace(), *targetNs)

return gateway.GatewayContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonInvalid,
Message: message,
}
return nil
}

return gateway.GatewayContext, nil
return gateway.GatewayContext
}

func (t *Translator) translateExtServerPolicyForGateway(
32 changes: 2 additions & 30 deletions internal/gatewayapi/securitypolicy.go
Original file line number Diff line number Diff line change
@@ -248,12 +248,10 @@ func resolveSecurityPolicyGatewayTargetRef(
target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName,
gateways map[types.NamespacedName]*policyGatewayTargetContext,
) (*GatewayContext, *status.PolicyResolveError) {
targetNs := policy.Namespace

// Find the Gateway
key := types.NamespacedName{
Name: string(target.Name),
Namespace: targetNs,
Namespace: policy.Namespace,
}
gateway, ok := gateways[key]

@@ -265,18 +263,6 @@ func resolveSecurityPolicyGatewayTargetRef(
return nil, nil
}

// Ensure Policy and target are in the same namespace
if policy.Namespace != targetNs {
// TODO zhaohuabing use CEL to validate cross-namespace reference
message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, SecurityPolicy can only target a resource in the same namespace.",
policy.Namespace, targetNs)

return gateway.GatewayContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonInvalid,
Message: message,
}
}

// Check if another policy targeting the same Gateway exists
if gateway.attached {
message := fmt.Sprintf("Unable to target Gateway %s, another SecurityPolicy has already attached to it",
@@ -300,13 +286,11 @@ func resolveSecurityPolicyRouteTargetRef(
target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName,
routes map[policyTargetRouteKey]*policyRouteTargetContext,
) (RouteContext, *status.PolicyResolveError) {
targetNs := policy.Namespace

// Check if the route exists
key := policyTargetRouteKey{
Kind: string(target.Kind),
Name: string(target.Name),
Namespace: targetNs,
Namespace: policy.Namespace,
}
route, ok := routes[key]

@@ -318,18 +302,6 @@ func resolveSecurityPolicyRouteTargetRef(
return nil, nil
}

// Ensure Policy and target are in the same namespace
// TODO zhaohuabing use CEL to validate cross-namespace reference
if policy.Namespace != targetNs {
message := fmt.Sprintf("Namespace:%s TargetRef.Namespace:%s, SecurityPolicy can only target a resource in the same namespace.",
policy.Namespace, targetNs)

return route.RouteContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonInvalid,
Message: message,
}
}

// Check if another policy targeting the same xRoute exists
if route.attached {
message := fmt.Sprintf("Unable to target %s %s, another SecurityPolicy has already attached to it",

0 comments on commit 36764c0

Please sign in to comment.