Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions api/v1alpha1/securitypolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ type SecurityPolicy struct {
//
// +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.group == 'gateway.networking.k8s.io' : true", message="this policy can only have a targetRef.group of gateway.networking.k8s.io"
// +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute'] : true", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute"
// +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? !has(self.targetRef.sectionName) : true",message="this policy does not yet support the sectionName field"
// +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.kind == 'Gateway' || !has(self.targetRef.sectionName) : true",message="this policy supports the sectionName field only for kind Gateway"
// +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.group == 'gateway.networking.k8s.io') : true ", message="this policy can only have a targetRefs[*].group of gateway.networking.k8s.io"
// +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute']) : true ", message="this policy can only have a targetRefs[*].kind of Gateway/HTTPRoute/GRPCRoute"
// +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, !has(ref.sectionName)) : true",message="this policy does not yet support the sectionName field"
// +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind == 'Gateway' || !has(ref.sectionName)) : true",message="this policy supports the sectionName field only for kind Gateway"
// +kubebuilder:validation:XValidation:rule="(has(self.authorization) && has(self.authorization.rules) && self.authorization.rules.exists(r, has(r.principal.jwt))) ? has(self.jwt) : true", message="if authorization.rules.principal.jwt is used, jwt must be defined"
//
// SecurityPolicySpec defines the desired state of SecurityPolicy.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4870,17 +4870,18 @@ spec:
- message: this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute
rule: 'has(self.targetRef) ? self.targetRef.kind in [''Gateway'', ''HTTPRoute'',
''GRPCRoute''] : true'
- message: this policy does not yet support the sectionName field
rule: 'has(self.targetRef) ? !has(self.targetRef.sectionName) : true'
- message: this policy supports the sectionName field only for kind Gateway
rule: 'has(self.targetRef) ? self.targetRef.kind == ''Gateway'' || !has(self.targetRef.sectionName)
: true'
- message: this policy can only have a targetRefs[*].group of gateway.networking.k8s.io
rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, ref.group ==
''gateway.networking.k8s.io'') : true '
- message: this policy can only have a targetRefs[*].kind of Gateway/HTTPRoute/GRPCRoute
rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind in [''Gateway'',
''HTTPRoute'', ''GRPCRoute'']) : true '
- message: this policy does not yet support the sectionName field
rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, !has(ref.sectionName))
: true'
- message: this policy supports the sectionName field only for kind Gateway
rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind == ''Gateway''
|| !has(ref.sectionName)) : true'
- message: if authorization.rules.principal.jwt is used, jwt must be defined
rule: '(has(self.authorization) && has(self.authorization.rules) &&
self.authorization.rules.exists(r, has(r.principal.jwt))) ? has(self.jwt)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4869,17 +4869,18 @@ spec:
- message: this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute
rule: 'has(self.targetRef) ? self.targetRef.kind in [''Gateway'', ''HTTPRoute'',
''GRPCRoute''] : true'
- message: this policy does not yet support the sectionName field
rule: 'has(self.targetRef) ? !has(self.targetRef.sectionName) : true'
- message: this policy supports the sectionName field only for kind Gateway
rule: 'has(self.targetRef) ? self.targetRef.kind == ''Gateway'' || !has(self.targetRef.sectionName)
: true'
- message: this policy can only have a targetRefs[*].group of gateway.networking.k8s.io
rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, ref.group ==
''gateway.networking.k8s.io'') : true '
- message: this policy can only have a targetRefs[*].kind of Gateway/HTTPRoute/GRPCRoute
rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind in [''Gateway'',
''HTTPRoute'', ''GRPCRoute'']) : true '
- message: this policy does not yet support the sectionName field
rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, !has(ref.sectionName))
: true'
- message: this policy supports the sectionName field only for kind Gateway
rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind == ''Gateway''
|| !has(ref.sectionName)) : true'
- message: if authorization.rules.principal.jwt is used, jwt must be defined
rule: '(has(self.authorization) && has(self.authorization.rules) &&
self.authorization.rules.exists(r, has(r.principal.jwt))) ? has(self.jwt)
Expand Down
3 changes: 2 additions & 1 deletion internal/gatewayapi/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,8 @@ type policyRouteTargetContext struct {

type policyGatewayTargetContext struct {
*GatewayContext
attached bool
attached bool
attachedToListeners sets.Set[string]
}

// listenersWithSameHTTPPort returns a list of the names of all other HTTP listeners
Expand Down
244 changes: 167 additions & 77 deletions internal/gatewayapi/securitypolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,19 @@
gatewayMap := map[types.NamespacedName]*policyGatewayTargetContext{}
for _, gw := range gateways {
key := utils.NamespacedName(gw)
gatewayMap[key] = &policyGatewayTargetContext{GatewayContext: gw}
gatewayMap[key] = &policyGatewayTargetContext{GatewayContext: gw, attachedToListeners: make(sets.Set[string])}
}

// Map of Gateway to the routes attached to it
gatewayRouteMap := make(map[string]sets.Set[string])
// Map of Gateway to the routes attached to it.
// The routes are grouped by sectionNames of their targetRefs
gatewayRouteMap := make(map[string]map[string]sets.Set[string])

handledPolicies := make(map[types.NamespacedName]*egv1a1.SecurityPolicy)

// Translate
// 1. First translate Policies targeting xRoutes
// 2. Finally, the policies targeting Gateways
// 2. Then translate Policies targeting Listeners
// 3. Finally, the policies targeting Gateways

// Process the policies targeting xRoutes
for _, currPolicy := range securityPolicies {
Expand Down Expand Up @@ -134,9 +136,17 @@

key := gwNN.String()
if _, ok := gatewayRouteMap[key]; !ok {
gatewayRouteMap[key] = make(sets.Set[string])
gatewayRouteMap[key] = make(map[string]sets.Set[string])
}
gatewayRouteMap[key].Insert(utils.NamespacedName(targetedRoute).String())
listenerRouteMap := gatewayRouteMap[key]
sectionName := ""
if p.SectionName != nil {
sectionName = string(*p.SectionName)
}
if _, ok := listenerRouteMap[sectionName]; !ok {
listenerRouteMap[sectionName] = make(sets.Set[string])
}
listenerRouteMap[sectionName].Insert(utils.NamespacedName(targetedRoute).String())
parentGateways = append(parentGateways, getAncestorRefForPolicy(gwNN, p.SectionName))
}
}
Expand Down Expand Up @@ -179,87 +189,148 @@
}
}

// Process the policies targeting Gateways
// Process the policies targeting Listeners
for _, currPolicy := range securityPolicies {
policyName := utils.NamespacedName(currPolicy)
targetRefs := getPolicyTargetRefs(currPolicy.Spec.PolicyTargetReferences, gateways)
for _, currTarget := range targetRefs {
if currTarget.Kind == resource.KindGateway {
var (
targetedGateway *GatewayContext
resolveErr *status.PolicyResolveError
)

if currTarget.Kind == resource.KindGateway && currTarget.SectionName != nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
handledPolicies[policyName] = policy
res = append(res, policy)
}

targetedGateway, resolveErr = resolveSecurityPolicyGatewayTargetRef(policy, currTarget, gatewayMap)
// Skip if the gateway is not found
// It's not necessarily an error because the SecurityPolicy may be
// reconciled by multiple controllers. And the other controller may
// have the target gateway.
if targetedGateway == nil {
continue
t.processSecurityPolicyForGateway(resources, xdsIR,
gatewayMap, gatewayRouteMap, policy, currTarget)
}
}
}
// Process the policies targeting Gateways
for _, currPolicy := range securityPolicies {
policyName := utils.NamespacedName(currPolicy)
targetRefs := getPolicyTargetRefs(currPolicy.Spec.PolicyTargetReferences, gateways)
for _, currTarget := range targetRefs {
if currTarget.Kind == resource.KindGateway && currTarget.SectionName == nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
handledPolicies[policyName] = policy
res = append(res, policy)
}

// Find its ancestor reference by resolved gateway, even with resolve error
gatewayNN := utils.NamespacedName(targetedGateway)
parentGateways := []gwapiv1a2.ParentReference{
getAncestorRefForPolicy(gatewayNN, nil),
}
t.processSecurityPolicyForGateway(resources, xdsIR,
gatewayMap, gatewayRouteMap, policy, currTarget)
}
}
}

// Set conditions for resolve error, then skip current gateway
if resolveErr != nil {
status.SetResolveErrorForPolicyAncestors(&policy.Status,
parentGateways,
t.GatewayControllerName,
policy.Generation,
resolveErr,
)
return res
}

continue
}
func (t *Translator) processSecurityPolicyForGateway(
resources *resource.Resources,
xdsIR resource.XdsIRMap,
gatewayMap map[types.NamespacedName]*policyGatewayTargetContext,
gatewayRouteMap map[string]map[string]sets.Set[string],
policy *egv1a1.SecurityPolicy,
currTarget gwapiv1a2.LocalPolicyTargetReferenceWithSectionName,
) {
var (
targetedGateway *GatewayContext
resolveErr *status.PolicyResolveError
)

if err := t.translateSecurityPolicyForGateway(policy, targetedGateway, currTarget, resources, xdsIR); err != nil {
status.SetTranslationErrorForPolicyAncestors(&policy.Status,
parentGateways,
t.GatewayControllerName,
policy.Generation,
status.Error2ConditionMsg(err),
)
}
targetedGateway, resolveErr = resolveSecurityPolicyGatewayTargetRef(policy, currTarget, gatewayMap)
// Skip if the gateway is not found
// It's not necessarily an error because the SecurityPolicy may be
// reconciled by multiple controllers. And the other controller may
// have the target gateway.
if targetedGateway == nil {
return
}

// Set Accepted condition if it is unset
status.SetAcceptedForPolicyAncestors(&policy.Status, parentGateways, t.GatewayControllerName)
// Find its ancestor reference by resolved gateway, even with resolve error
gatewayNN := utils.NamespacedName(targetedGateway)
parentGateways := []gwapiv1a2.ParentReference{
getAncestorRefForPolicy(gatewayNN, currTarget.SectionName),
}

// Check if this policy is overridden by other policies targeting
// at route level
if r, ok := gatewayRouteMap[gatewayNN.String()]; ok {
// Maintain order here to ensure status/string does not change with the same data
routes := r.UnsortedList()
sort.Strings(routes)
message := fmt.Sprintf(
"This policy is being overridden by other securityPolicies for these routes: %v",
routes)
status.SetConditionForPolicyAncestors(&policy.Status,
parentGateways,
t.GatewayControllerName,
egv1a1.PolicyConditionOverridden,
metav1.ConditionTrue,
egv1a1.PolicyReasonOverridden,
message,
policy.Generation,
)
}
}
}
// Set conditions for resolve error, then skip current gateway
if resolveErr != nil {
status.SetResolveErrorForPolicyAncestors(&policy.Status,
parentGateways,
t.GatewayControllerName,
policy.Generation,
resolveErr,
)

return
}

return res
if err := t.translateSecurityPolicyForGateway(policy, targetedGateway, currTarget, resources, xdsIR); err != nil {
status.SetTranslationErrorForPolicyAncestors(&policy.Status,
parentGateways,
t.GatewayControllerName,
policy.Generation,
status.Error2ConditionMsg(err),
)
}

// Set Accepted condition if it is unset
status.SetAcceptedForPolicyAncestors(&policy.Status, parentGateways, t.GatewayControllerName)

// Check if this policy is overridden by other policies targeting at route and listener levels
overriddenTargetsMessage := getOverriddenTargetsMessage(
gatewayMap[gatewayNN], gatewayRouteMap[gatewayNN.String()], currTarget.SectionName)
if overriddenTargetsMessage != "" {
status.SetConditionForPolicyAncestors(&policy.Status,
parentGateways,
t.GatewayControllerName,
egv1a1.PolicyConditionOverridden,
metav1.ConditionTrue,
egv1a1.PolicyReasonOverridden,
"This policy is being overridden by other securityPolicies for "+overriddenTargetsMessage,
policy.Generation,
)
}
}

func getOverriddenTargetsMessage(
targetContext *policyGatewayTargetContext,
listenerRouteMap map[string]sets.Set[string],
sectionName *gwapiv1.SectionName,
) string {
var listeners, routes []string
if sectionName == nil {
if targetContext != nil {
listeners = targetContext.attachedToListeners.UnsortedList()
}
for _, routeSet := range listenerRouteMap {
routes = append(routes, routeSet.UnsortedList()...)
}
} else if listenerRouteMap != nil {
if routeSet, ok := listenerRouteMap[string(*sectionName)]; ok {
routes = routeSet.UnsortedList()
}
if routeSet, ok := listenerRouteMap[""]; ok {
routes = append(routes, routeSet.UnsortedList()...)
}
}
if len(listeners) > 0 {
sort.Strings(listeners)
if len(routes) > 0 {
sort.Strings(routes)
return fmt.Sprintf("these listeners: %v and these routes: %v", listeners, routes)
} else {
return fmt.Sprintf("these listeners: %v", listeners)
}

Check warning on line 328 in internal/gatewayapi/securitypolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/securitypolicy.go#L327-L328

Added lines #L327 - L328 were not covered by tests
} else if len(routes) > 0 {
sort.Strings(routes)
return fmt.Sprintf("these routes: %v", routes)
}
return ""
}

// validateSecurityPolicy validates the SecurityPolicy.
Expand Down Expand Up @@ -327,19 +398,32 @@
return nil, nil
}

// 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",
string(target.Name))
if target.SectionName == nil {
// 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",
string(target.Name))

return gateway.GatewayContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonConflicted,
Message: message,
return gateway.GatewayContext, &status.PolicyResolveError{
Reason: gwapiv1a2.PolicyReasonConflicted,
Message: message,
}
}
gateway.attached = true
} else {
listenerName := string(*target.SectionName)
if gateway.attachedToListeners.Has(listenerName) {
message := fmt.Sprintf("Unable to target Listener %s/%s, another SecurityPolicy has already attached to it",
string(target.Name), listenerName)

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

Check warning on line 422 in internal/gatewayapi/securitypolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/securitypolicy.go#L416-L422

Added lines #L416 - L422 were not covered by tests
}
gateway.attachedToListeners.Insert(listenerName)
}

// Set context and save
gateway.attached = true
gateways[key] = gateway

return gateway.GatewayContext, nil
Expand Down Expand Up @@ -586,10 +670,16 @@

policyTarget := irStringKey(policy.Namespace, string(target.Name))
for _, h := range x.HTTP {
gatewayName := h.Name[0:strings.LastIndex(h.Name, "/")]
// A HTTPListener name has the format namespace/gatewayName/listenerName
gatewayNameEnd := strings.LastIndex(h.Name, "/")
gatewayName := h.Name[0:gatewayNameEnd]
if t.MergeGateways && gatewayName != policyTarget {
continue
}
// If specified the sectionName must match the listenerName part of the HTTPListener name
if target.SectionName != nil && string(*target.SectionName) != h.Name[gatewayNameEnd+1:] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment for this ? can't understand this code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"A HTTPListener name has the format namespace/gatewayName/listenerName" and "If specified the sectionName must match the listenerName part of the HTTPListener name" added in f1a1fd5262b6609b7617e39edee602514d3c4595

continue
}
// A Policy targeting the most specific scope(xRoute) wins over a policy
// targeting a lesser specific scope(Gateway).
for _, r := range h.Routes {
Expand Down
Loading