diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 9ccea6da60..f9d9fd43d2 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -412,7 +412,7 @@ func (t *Translator) processSecurityPolicyForRoute( // Check if merging is enabled if policy.Spec.MergeType == nil { // No merging - use existing translation logic - if err := t.translateSecurityPolicyForRoute(policy, targetedRoute, currTarget, resources, xdsIR, nil, nil); err != nil { + if err := t.translateSecurityPolicyForRoute(policy, &securityPolicyOwners{}, targetedRoute, currTarget, resources, xdsIR, nil, nil); err != nil { status.SetTranslationErrorForPolicyAncestors(&policy.Status, ancestorRefs, t.GatewayControllerName, @@ -442,7 +442,7 @@ func (t *Translator) processSecurityPolicyForRoute( if gwPolicy == nil && listenerPolicy == nil { // No parent policy found, fall back to current policy - if err := t.translateSecurityPolicyForRoute(policy, targetedRoute, currTarget, resources, xdsIR, &gwNN, &listener.Name); err != nil { + if err := t.translateSecurityPolicyForRoute(policy, &securityPolicyOwners{}, targetedRoute, currTarget, resources, xdsIR, &gwNN, &listener.Name); err != nil { status.SetConditionForPolicyAncestor(&policy.Status, &ancestorRef, t.GatewayControllerName, @@ -461,7 +461,7 @@ func (t *Translator) processSecurityPolicyForRoute( } // Merge with parent policy - mergedPolicy, err := mergeSecurityPolicy(policy, parentPolicy) + mergedPolicy, owners, err := mergeSecurityPolicy(policy, parentPolicy) if err != nil { status.SetConditionForPolicyAncestor(&policy.Status, &ancestorRef, @@ -487,7 +487,7 @@ func (t *Translator) processSecurityPolicyForRoute( } // Apply merged policy - if err := t.translateSecurityPolicyForRoute(mergedPolicy, targetedRoute, currTarget, resources, xdsIR, &gwNN, &listener.Name); err != nil { + if err := t.translateSecurityPolicyForRoute(mergedPolicy, owners, targetedRoute, currTarget, resources, xdsIR, &gwNN, &listener.Name); err != nil { status.SetConditionForPolicyAncestor(&policy.Status, &ancestorRef, t.GatewayControllerName, @@ -865,6 +865,7 @@ func resolveSecurityPolicyRouteTargetRef( func (t *Translator) translateSecurityPolicyForRoute( policy *egv1a1.SecurityPolicy, + owners *securityPolicyOwners, route RouteContext, target policyTargetReferenceWithSectionName, resources *resource.Resources, @@ -889,7 +890,9 @@ func (t *Translator) translateSecurityPolicyForRoute( if policy.Spec.BasicAuth != nil { if basicAuth, err = t.buildBasicAuth( policy, - resources); err != nil { + owners, + resources, + ); err != nil { err = perr.WithMessage(err, "BasicAuth") errs = errors.Join(errs, err) } @@ -898,14 +901,16 @@ func (t *Translator) translateSecurityPolicyForRoute( if policy.Spec.APIKeyAuth != nil { if apiKeyAuth, err = t.buildAPIKeyAuth( policy, - resources); err != nil { + owners, + resources, + ); err != nil { err = perr.WithMessage(err, "APIKeyAuth") errs = errors.Join(errs, err) } } if policy.Spec.Authorization != nil { - if authorization, err = t.buildAuthorization(policy); err != nil { + if authorization, err = t.buildAuthorization(policy, owners); err != nil { err = perr.WithMessage(err, "Authorization") errs = errors.Join(errs, err) } @@ -945,8 +950,10 @@ func (t *Translator) translateSecurityPolicyForRoute( if policy.Spec.ExtAuth != nil { if extAuth, extAuthErr = t.buildExtAuth( policy, + owners, resources, - gtwCtx); extAuthErr != nil { + gtwCtx, + ); extAuthErr != nil { extAuthErr = perr.WithMessage(extAuthErr, "ExtAuth") errs = errors.Join(errs, extAuthErr) } @@ -956,8 +963,10 @@ func (t *Translator) translateSecurityPolicyForRoute( if policy.Spec.OIDC != nil { if oidc, err = t.buildOIDC( policy, + owners, resources, - gtwCtx); err != nil { + gtwCtx, + ); err != nil { err = perr.WithMessage(err, "OIDC") errs = errors.Join(errs, err) hasNonExtAuthError = true @@ -968,8 +977,10 @@ func (t *Translator) translateSecurityPolicyForRoute( if policy.Spec.JWT != nil { if jwt, err = t.buildJWT( policy, + owners, resources, - gtwCtx); err != nil { + gtwCtx, + ); err != nil { err = perr.WithMessage(err, "JWT") errs = errors.Join(errs, err) hasNonExtAuthError = true @@ -1097,6 +1108,7 @@ func (t *Translator) translateSecurityPolicyForGateway( xdsIR resource.XdsIRMap, ) error { // Build IR + noOwners := &securityPolicyOwners{} var ( cors *ir.CORS jwt *ir.JWT @@ -1116,8 +1128,10 @@ func (t *Translator) translateSecurityPolicyForGateway( if policy.Spec.JWT != nil { if jwt, err = t.buildJWT( policy, + noOwners, resources, - gtwCtx); err != nil { + gtwCtx, + ); err != nil { err = perr.WithMessage(err, "JWT") errs = errors.Join(errs, err) } @@ -1126,8 +1140,10 @@ func (t *Translator) translateSecurityPolicyForGateway( if policy.Spec.OIDC != nil { if oidc, err = t.buildOIDC( policy, + noOwners, resources, - gtwCtx); err != nil { + gtwCtx, + ); err != nil { err = perr.WithMessage(err, "OIDC") errs = errors.Join(errs, err) } @@ -1136,7 +1152,9 @@ func (t *Translator) translateSecurityPolicyForGateway( if policy.Spec.BasicAuth != nil { if basicAuth, err = t.buildBasicAuth( policy, - resources); err != nil { + noOwners, + resources, + ); err != nil { err = perr.WithMessage(err, "BasicAuth") errs = errors.Join(errs, err) } @@ -1145,14 +1163,16 @@ func (t *Translator) translateSecurityPolicyForGateway( if policy.Spec.APIKeyAuth != nil { if apiKeyAuth, err = t.buildAPIKeyAuth( policy, - resources); err != nil { + noOwners, + resources, + ); err != nil { err = perr.WithMessage(err, "APIKeyAuth") errs = errors.Join(errs, err) } } if policy.Spec.Authorization != nil { - if authorization, err = t.buildAuthorization(policy); err != nil { + if authorization, err = t.buildAuthorization(policy, noOwners); err != nil { errs = errors.Join(errs, err) } } @@ -1162,8 +1182,10 @@ func (t *Translator) translateSecurityPolicyForGateway( if policy.Spec.ExtAuth != nil { if extAuth, extAuthErr = t.buildExtAuth( policy, + noOwners, resources, - gtwCtx); extAuthErr != nil { + gtwCtx, + ); extAuthErr != nil { extAuthErr = perr.WithMessage(extAuthErr, "ExtAuth") errs = errors.Join(errs, extAuthErr) } @@ -1337,6 +1359,7 @@ func wildcard2regex(wildcard string) string { func (t *Translator) buildJWT( policy *egv1a1.SecurityPolicy, + owners *securityPolicyOwners, resources *resource.Resources, gtwCtx *GatewayContext, ) (*ir.JWT, error) { @@ -1344,6 +1367,7 @@ func (t *Translator) buildJWT( return nil, err } + jwtOwnerPolicy := policyOwnerOr(owners.jwtProviders, policy) providers := make([]ir.JWTProvider, 0, len(policy.Spec.JWT.Providers)) for i, p := range policy.Spec.JWT.Providers { provider := ir.JWTProvider{ @@ -1355,13 +1379,13 @@ func (t *Translator) buildJWT( ExtractFrom: p.ExtractFrom, } if p.RemoteJWKS != nil { - remoteJWKS, err := t.buildRemoteJWKS(policy, p.RemoteJWKS, i, resources, gtwCtx) + remoteJWKS, err := t.buildRemoteJWKS(jwtOwnerPolicy, p.RemoteJWKS, i, resources, gtwCtx) if err != nil { return nil, err } provider.RemoteJWKS = remoteJWKS } else { - localJWKS, err := t.buildLocalJWKS(policy, p.LocalJWKS) + localJWKS, err := t.buildLocalJWKS(jwtOwnerPolicy, p.LocalJWKS) if err != nil { return nil, err } @@ -1552,6 +1576,7 @@ func (t *Translator) buildLocalJWKS( func (t *Translator) buildOIDC( policy *egv1a1.SecurityPolicy, + owners *securityPolicyOwners, resources *resource.Resources, gtwCtx *GatewayContext, ) (*ir.OIDC, error) { @@ -1570,21 +1595,22 @@ func (t *Translator) buildOIDC( err error ) - if provider, err = t.buildOIDCProvider(policy, resources, gtwCtx); err != nil { + if provider, err = t.buildOIDCProvider(policy, owners, resources, gtwCtx); err != nil { return nil, err } - from := crossNamespaceFrom{ - group: egv1a1.GroupName, - kind: resource.KindSecurityPolicy, - namespace: policy.Namespace, - } - // Client ID can be specified either as a string or as a reference to a secret. switch { case oidc.ClientID != nil: clientID = *oidc.ClientID case oidc.ClientIDRef != nil: + ownerPolicy := policyOwnerOr(owners.oidcClientIDRef, policy) + from := crossNamespaceFrom{ + group: egv1a1.GroupName, + kind: resource.KindSecurityPolicy, + namespace: ownerPolicy.Namespace, + } + var clientIDSecret *corev1.Secret if clientIDSecret, err = t.validateSecretRef(true, from, *oidc.ClientIDRef, resources); err != nil { return nil, err @@ -1599,6 +1625,12 @@ func (t *Translator) buildOIDC( return nil, fmt.Errorf("client ID must be specified in OIDC policy %s/%s", policy.Namespace, policy.Name) } + clientSecretOwner := policyOwnerOr(owners.oidcClientSecret, policy) + from := crossNamespaceFrom{ + group: egv1a1.GroupName, + kind: resource.KindSecurityPolicy, + namespace: clientSecretOwner.Namespace, + } if clientSecret, err = t.validateSecretRef(true, from, oidc.ClientSecret, resources); err != nil { return nil, err } @@ -1637,10 +1669,12 @@ func (t *Translator) buildOIDC( disableTokenEncryption = *oidc.DisableTokenEncryption } + oidcOwner := policyOwnerOr(owners.oidc, policy) + // Generate a unique cookie suffix for oauth filters. // This is to avoid cookie name collision when multiple security policies are applied // to the same route. - suffix := utils.Digest32(string(policy.UID)) + suffix := utils.Digest32(string(oidcOwner.UID)) // Get the HMAC secret. // HMAC secret is generated by the CertGen job and stored in a secret @@ -1657,7 +1691,7 @@ func (t *Translator) buildOIDC( } irOIDC := &ir.OIDC{ - Name: irConfigName(policy), + Name: irConfigName(oidcOwner), Provider: *provider, ClientID: clientID, ClientSecret: clientSecretBytes, @@ -1707,6 +1741,7 @@ func (t *Translator) buildOIDC( func (t *Translator) buildOIDCProvider( policy *egv1a1.SecurityPolicy, + owners *securityPolicyOwners, resources *resource.Resources, gtwCtx *GatewayContext, ) (*ir.OIDCProvider, error) { @@ -1739,9 +1774,10 @@ func (t *Translator) buildOIDCProvider( protocol = ir.HTTP } + oidcProviderOwner := policyOwnerOr(owners.oidcProviderBackendRefs, policy) if len(provider.BackendRefs) > 0 { if rd, err = t.translateExtServiceBackendRefs( - policy, provider.BackendRefs, protocol, resources, gtwCtx, "oidc", 0); err != nil { + oidcProviderOwner, provider.BackendRefs, protocol, resources, gtwCtx, "oidc", 0); err != nil { return nil, err } } @@ -2021,12 +2057,14 @@ func validateTokenEndpoint(tokenEndpoint string) error { func (t *Translator) buildAPIKeyAuth( policy *egv1a1.SecurityPolicy, + owners *securityPolicyOwners, resources *resource.Resources, ) (*ir.APIKeyAuth, error) { + ownerPolicy := policyOwnerOr(owners.apiKeyAuthCredentialRefs, policy) from := crossNamespaceFrom{ group: egv1a1.GroupName, kind: resource.KindSecurityPolicy, - namespace: policy.Namespace, + namespace: ownerPolicy.Namespace, } expected := len(policy.Spec.APIKeyAuth.CredentialRefs) @@ -2077,6 +2115,7 @@ func (t *Translator) buildAPIKeyAuth( func (t *Translator) buildBasicAuth( policy *egv1a1.SecurityPolicy, + owners *securityPolicyOwners, resources *resource.Resources, ) (*ir.BasicAuth, error) { var ( @@ -2085,10 +2124,11 @@ func (t *Translator) buildBasicAuth( err error ) + ownerPolicy := policyOwnerOr(owners.basicAuth, policy) from := crossNamespaceFrom{ group: egv1a1.GroupName, kind: resource.KindSecurityPolicy, - namespace: policy.Namespace, + namespace: ownerPolicy.Namespace, } if usersSecret, err = t.validateSecretRef(true, from, basicAuth.Users, resources); err != nil { return nil, err @@ -2111,7 +2151,7 @@ func (t *Translator) buildBasicAuth( } return &ir.BasicAuth{ - Name: irConfigName(policy), + Name: irConfigName(ownerPolicy), Users: usersSecretBytes, ForwardUsernameHeader: basicAuth.ForwardUsernameHeader, }, nil @@ -2150,6 +2190,7 @@ func validateHtpasswdFormat(data []byte) error { func (t *Translator) buildExtAuth( policy *egv1a1.SecurityPolicy, + owners *securityPolicyOwners, resources *resource.Resources, gtwCtx *GatewayContext, ) (*ir.ExtAuth, error) { @@ -2166,6 +2207,8 @@ func (t *Translator) buildExtAuth( contextExtensions []*ir.ContextExtention ) + backendRefsOwnerPolicy := policyOwnerOr(owners.extAuthBackendRefs, policy) + // These are sanity checks, they should never happen because the API server // should have caught them if http == nil && grpc == nil { @@ -2210,7 +2253,7 @@ func (t *Translator) buildExtAuth( } if rd, err = t.translateExtServiceBackendRefs( - policy, backendRefs, protocol, resources, gtwCtx, "extauth", 0); err != nil { + backendRefsOwnerPolicy, backendRefs, protocol, resources, gtwCtx, "extauth", 0); err != nil { return nil, err } @@ -2220,7 +2263,7 @@ func (t *Translator) buildExtAuth( // When translated to XDS, the authority is used on the filter level not on the cluster level. // There's no way to translate to XDS and use a different authority for each backendref if authority == "" { - authority = t.backendRefAuthority(&backendRef.BackendObjectReference, policy) + authority = t.backendRefAuthority(&backendRef.BackendObjectReference, backendRefsOwnerPolicy) } } @@ -2228,12 +2271,13 @@ func (t *Translator) buildExtAuth( return nil, err } - if contextExtensions, err = t.buildContextExtensions(policy.Spec.ExtAuth.ContextExtensions, policy.Namespace); err != nil { + if contextExtensions, err = t.buildContextExtensions(policy.Spec.ExtAuth.ContextExtensions, owners, policy); err != nil { return nil, err } + extAuthOwner := policyOwnerOr(owners.extAuth, policy) extAuth := &ir.ExtAuth{ - Name: irConfigName(policy), + Name: irConfigName(extAuthOwner), HeadersToExtAuth: policy.Spec.ExtAuth.HeadersToExtAuth, ContextExtensions: contextExtensions, FailOpen: policy.Spec.ExtAuth.FailOpen, @@ -2284,7 +2328,8 @@ func parseExtAuthTimeout(timeout *gwapiv1.Duration) *metav1.Duration { func (t *Translator) buildContextExtensions( contextExtensions []*egv1a1.ContextExtension, - policyNs string, + owners *securityPolicyOwners, + defaultOwner *egv1a1.SecurityPolicy, ) ([]*ir.ContextExtention, error) { if len(contextExtensions) == 0 { return nil, nil @@ -2294,8 +2339,9 @@ func (t *Translator) buildContextExtensions( for _, ext := range contextExtensions { var value ir.PrivateBytes if ext.Type == egv1a1.ContextExtensionValueTypeValueRef { + ownerPolicy := policyOwnerOr(owners.extAuthContextExtensions[ext.Name], defaultOwner) var err error - if value, err = t.getContextExtensionValueFromRef(ext.ValueRef, policyNs); err != nil { + if value, err = t.getContextExtensionValueFromRef(ext.ValueRef, ownerPolicy.Namespace); err != nil { return nil, err } } else if ext.Value != nil { @@ -2382,7 +2428,10 @@ func (t *Translator) backendRefAuthority( return fmt.Sprintf("%s.%s", backendRef.Name, backendNamespace) } -func (t *Translator) buildAuthorization(policy *egv1a1.SecurityPolicy) (*ir.Authorization, error) { +func (t *Translator) buildAuthorization( + policy *egv1a1.SecurityPolicy, + owners *securityPolicyOwners, +) (*ir.Authorization, error) { var ( authorization = policy.Spec.Authorization irAuth = &ir.Authorization{} @@ -2390,6 +2439,8 @@ func (t *Translator) buildAuthorization(policy *egv1a1.SecurityPolicy) (*ir.Auth defaultAction = egv1a1.AuthorizationActionDeny ) + ownerPolicy := policyOwnerOr(owners.authorizationRules, policy) + if authorization.DefaultAction != nil { defaultAction = *authorization.DefaultAction } @@ -2416,7 +2467,7 @@ func (t *Translator) buildAuthorization(policy *egv1a1.SecurityPolicy) (*ir.Auth if rule.Name != nil && *rule.Name != "" { name = *rule.Name } else { - name = defaultAuthorizationRuleName(policy, i) + name = defaultAuthorizationRuleName(ownerPolicy, i) } irAuth.Rules = append(irAuth.Rules, &ir.AuthorizationRule{ Name: name, @@ -2516,13 +2567,120 @@ func defaultAuthorizationRuleName(policy *egv1a1.SecurityPolicy, index int) stri strconv.Itoa(index)) } +type securityPolicyOwners struct { + basicAuth *egv1a1.SecurityPolicy + apiKeyAuthCredentialRefs *egv1a1.SecurityPolicy + authorizationRules *egv1a1.SecurityPolicy + extAuth *egv1a1.SecurityPolicy + extAuthBackendRefs *egv1a1.SecurityPolicy + extAuthContextExtensions map[string]*egv1a1.SecurityPolicy + oidc *egv1a1.SecurityPolicy + oidcProviderBackendRefs *egv1a1.SecurityPolicy + oidcClientIDRef *egv1a1.SecurityPolicy + oidcClientSecret *egv1a1.SecurityPolicy + jwtProviders *egv1a1.SecurityPolicy +} + +// policyOwnerOr returns owner if non-nil, otherwise fallback. +// Used to resolve per-field owners from securityPolicyOwners: the owner is the policy +// that contributed the field (route overrides parent), falling back to the active policy +// when no merge occurred or the field was not set by either side. +func policyOwnerOr(owner, fallback *egv1a1.SecurityPolicy) *egv1a1.SecurityPolicy { + if owner != nil { + return owner + } + return fallback +} + // mergeSecurityPolicy merges a route-level SecurityPolicy with a parent (Gateway/Listener) SecurityPolicy. -func mergeSecurityPolicy(routePolicy, parentPolicy *egv1a1.SecurityPolicy) (*egv1a1.SecurityPolicy, error) { +func mergeSecurityPolicy(routePolicy, parentPolicy *egv1a1.SecurityPolicy) (*egv1a1.SecurityPolicy, *securityPolicyOwners, error) { if routePolicy.Spec.MergeType == nil || parentPolicy == nil { - return routePolicy, nil + return routePolicy, nil, nil + } + mergedPolicy, err := utils.Merge[*egv1a1.SecurityPolicy](parentPolicy, routePolicy, *routePolicy.Spec.MergeType) + if err != nil { + return nil, nil, err } + return mergedPolicy, buildSecurityPolicyOwners(routePolicy, parentPolicy), nil +} - return utils.Merge(parentPolicy, routePolicy, *routePolicy.Spec.MergeType) +// ownerOf returns route if routeOwns(route) is true, otherwise parent. +// Use this when ownership of a merged field is determined by a single predicate. +func ownerOf( + route, parent *egv1a1.SecurityPolicy, + routeOwns func(*egv1a1.SecurityPolicy) bool, +) *egv1a1.SecurityPolicy { + if routeOwns(route) { + return route + } + return parent +} + +// buildSecurityPolicyOwners determines, for each merged field, which policy +// (route or parent) is considered the owner. The owner is used later to resolve +// references (e.g. Secrets, BackendRefs) scoped to the owning policy's namespace, +// and to derive IR resource names tied to the owning policy. +func buildSecurityPolicyOwners(route, parent *egv1a1.SecurityPolicy) *securityPolicyOwners { + return &securityPolicyOwners{ + basicAuth: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + return p.Spec.BasicAuth != nil + }), + apiKeyAuthCredentialRefs: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + return p.Spec.APIKeyAuth != nil && len(p.Spec.APIKeyAuth.CredentialRefs) > 0 + }), + authorizationRules: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + return p.Spec.Authorization != nil && len(p.Spec.Authorization.Rules) > 0 + }), + extAuth: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + return p.Spec.ExtAuth != nil + }), + extAuthBackendRefs: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + ea := p.Spec.ExtAuth + if ea == nil { + return false + } + if ea.HTTP != nil && (len(ea.HTTP.BackendRefs) > 0 || ea.HTTP.BackendRef != nil) { + return true + } + if ea.GRPC != nil && (len(ea.GRPC.BackendRefs) > 0 || ea.GRPC.BackendRef != nil) { + return true + } + return false + }), + extAuthContextExtensions: buildExtAuthContextExtensionOwners(route, parent), + oidc: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + return p.Spec.OIDC != nil + }), + oidcProviderBackendRefs: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + return p.Spec.OIDC != nil && len(p.Spec.OIDC.Provider.BackendRefs) > 0 + }), + oidcClientIDRef: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + return p.Spec.OIDC != nil && p.Spec.OIDC.ClientIDRef != nil + }), + oidcClientSecret: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + return p.Spec.OIDC != nil + }), + jwtProviders: ownerOf(route, parent, func(p *egv1a1.SecurityPolicy) bool { + return p.Spec.JWT != nil && len(p.Spec.JWT.Providers) > 0 + }), + } +} + +// buildExtAuthContextExtensionOwners returns a per-key owner map for ExtAuth ContextExtensions. +// Parent keys are added first so that route-level extensions take precedence on conflict. +func buildExtAuthContextExtensionOwners(route, parent *egv1a1.SecurityPolicy) map[string]*egv1a1.SecurityPolicy { + owners := make(map[string]*egv1a1.SecurityPolicy) + if parent.Spec.ExtAuth != nil { + for _, ext := range parent.Spec.ExtAuth.ContextExtensions { + owners[ext.Name] = parent + } + } + if route.Spec.ExtAuth != nil { + for _, ext := range route.Spec.ExtAuth.ContextExtensions { + owners[ext.Name] = route + } + } + return owners } // securityPolicyCopiesWithStatusDeepCopy returns shallow copies with deep-copied Status fields. diff --git a/internal/gatewayapi/securitypolicy_test.go b/internal/gatewayapi/securitypolicy_test.go index 5ef480a1cb..189db2ff32 100644 --- a/internal/gatewayapi/securitypolicy_test.go +++ b/internal/gatewayapi/securitypolicy_test.go @@ -1421,9 +1421,14 @@ func Test_validateAuthorizationGeoIPForHTTP(t *testing.T) { func Test_buildContextExtensions(t *testing.T) { policyNs := "default" + defaultOwner := &egv1a1.SecurityPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: policyNs}, + } tests := []struct { name string contextExtensions []*egv1a1.ContextExtension + owners *securityPolicyOwners + defaultOwner *egv1a1.SecurityPolicy translatorContext *TranslatorContext want []*ir.ContextExtention wantErr bool @@ -1431,11 +1436,13 @@ func Test_buildContextExtensions(t *testing.T) { { name: "Nil", contextExtensions: nil, + owners: &securityPolicyOwners{}, want: nil, }, { name: "Empty", contextExtensions: []*egv1a1.ContextExtension{}, + owners: &securityPolicyOwners{}, want: nil, }, { @@ -1443,11 +1450,13 @@ func Test_buildContextExtensions(t *testing.T) { contextExtensions: []*egv1a1.ContextExtension{ {Name: "foo", Value: new("bar")}, }, - want: []*ir.ContextExtention{{Name: "foo", Value: ir.PrivateBytes("bar")}}, + owners: &securityPolicyOwners{}, + want: []*ir.ContextExtention{{Name: "foo", Value: ir.PrivateBytes("bar")}}, }, { name: "TypeValueEmpty", contextExtensions: []*egv1a1.ContextExtension{{Name: "foo"}}, + owners: &securityPolicyOwners{}, want: []*ir.ContextExtention{{Name: "foo", Value: nil}}, }, { @@ -1459,13 +1468,15 @@ func Test_buildContextExtensions(t *testing.T) { Value: new("bar"), }, }, - want: []*ir.ContextExtention{{Name: "foo", Value: ir.PrivateBytes("bar")}}, + owners: &securityPolicyOwners{}, + want: []*ir.ContextExtention{{Name: "foo", Value: ir.PrivateBytes("bar")}}, }, { name: "TypeValueRefNil", contextExtensions: []*egv1a1.ContextExtension{ {Name: "foo", Type: egv1a1.ContextExtensionValueTypeValueRef}, }, + owners: &securityPolicyOwners{}, wantErr: true, }, { @@ -1481,6 +1492,7 @@ func Test_buildContextExtensions(t *testing.T) { Key: "test-key", }, }}, + owners: &securityPolicyOwners{}, translatorContext: &TranslatorContext{}, wantErr: true, }, @@ -1497,6 +1509,7 @@ func Test_buildContextExtensions(t *testing.T) { Key: "test-key", }, }}, + owners: &securityPolicyOwners{}, translatorContext: &TranslatorContext{ ConfigMapMap: map[types.NamespacedName]*corev1.ConfigMap{ {Namespace: policyNs, Name: "test-cm"}: {}, @@ -1517,6 +1530,7 @@ func Test_buildContextExtensions(t *testing.T) { Key: "test-key", }, }}, + owners: &securityPolicyOwners{}, translatorContext: &TranslatorContext{ ConfigMapMap: map[types.NamespacedName]*corev1.ConfigMap{ {Namespace: policyNs, Name: "test-cm"}: { @@ -1526,6 +1540,56 @@ func Test_buildContextExtensions(t *testing.T) { }, want: []*ir.ContextExtention{{Name: "foo", Value: ir.PrivateBytes("bar")}}, }, + { + name: "TypeValueRefUsesPerKeyOwnerNamespace", + contextExtensions: []*egv1a1.ContextExtension{ + { + Name: "parent-only", + Type: egv1a1.ContextExtensionValueTypeValueRef, + ValueRef: &egv1a1.LocalObjectKeyReference{ + LocalObjectReference: gwapiv1.LocalObjectReference{ + Kind: resource.KindConfigMap, + Name: "parent-cm", + }, + Key: "test-key", + }, + }, + { + Name: "route-only", + Type: egv1a1.ContextExtensionValueTypeValueRef, + ValueRef: &egv1a1.LocalObjectKeyReference{ + LocalObjectReference: gwapiv1.LocalObjectReference{ + Kind: resource.KindConfigMap, + Name: "route-cm", + }, + Key: "test-key", + }, + }, + }, + owners: &securityPolicyOwners{ + extAuthContextExtensions: map[string]*egv1a1.SecurityPolicy{ + "parent-only": {ObjectMeta: metav1.ObjectMeta{Namespace: "parent-ns"}}, + "route-only": {ObjectMeta: metav1.ObjectMeta{Namespace: "route-ns"}}, + }, + }, + defaultOwner: &egv1a1.SecurityPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default-ns"}, + }, + translatorContext: &TranslatorContext{ + ConfigMapMap: map[types.NamespacedName]*corev1.ConfigMap{ + {Namespace: "parent-ns", Name: "parent-cm"}: { + Data: map[string]string{"test-key": "parent-bar"}, + }, + {Namespace: "route-ns", Name: "route-cm"}: { + Data: map[string]string{"test-key": "route-bar"}, + }, + }, + }, + want: []*ir.ContextExtention{ + {Name: "parent-only", Value: ir.PrivateBytes("parent-bar")}, + {Name: "route-only", Value: ir.PrivateBytes("route-bar")}, + }, + }, { name: "TypeValueRefSecretNotFound", contextExtensions: []*egv1a1.ContextExtension{{ @@ -1539,6 +1603,7 @@ func Test_buildContextExtensions(t *testing.T) { Key: "test-key", }, }}, + owners: &securityPolicyOwners{}, translatorContext: &TranslatorContext{}, wantErr: true, }, @@ -1555,6 +1620,7 @@ func Test_buildContextExtensions(t *testing.T) { Key: "test-key", }, }}, + owners: &securityPolicyOwners{}, translatorContext: &TranslatorContext{ SecretMap: map[types.NamespacedName]*corev1.Secret{ {Namespace: policyNs, Name: "test-secret"}: {}, @@ -1575,6 +1641,7 @@ func Test_buildContextExtensions(t *testing.T) { Key: "test-key", }, }}, + owners: &securityPolicyOwners{}, translatorContext: &TranslatorContext{ SecretMap: map[types.NamespacedName]*corev1.Secret{ {Namespace: policyNs, Name: "test-secret"}: { @@ -1597,13 +1664,18 @@ func Test_buildContextExtensions(t *testing.T) { Key: "test-key", }, }}, + owners: &securityPolicyOwners{}, wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { translator := &Translator{TranslatorContext: tt.translatorContext} - got, err := translator.buildContextExtensions(tt.contextExtensions, policyNs) + owner := tt.defaultOwner + if owner == nil { + owner = defaultOwner + } + got, err := translator.buildContextExtensions(tt.contextExtensions, tt.owners, owner) if tt.wantErr { require.Error(t, err) require.Nil(t, got) @@ -1834,7 +1906,7 @@ func TestMergeSecurityPolicy(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := mergeSecurityPolicy(tt.routePolicy, tt.parentPolicy) + got, _, err := mergeSecurityPolicy(tt.routePolicy, tt.parentPolicy) if (err != nil) != tt.wantErr { t.Errorf("mergeSecurityPolicy() error = %v, wantErr %v", err, tt.wantErr) return @@ -1850,3 +1922,142 @@ func TestMergeSecurityPolicy(t *testing.T) { }) } } + +func Test_securityPolicyOwnerChoose(t *testing.T) { + t.Run("route policy overrides parent for the same owner fields", func(t *testing.T) { + parentPolicy := &egv1a1.SecurityPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "parent", Namespace: "parent-ns"}, + Spec: egv1a1.SecurityPolicySpec{ + BasicAuth: &egv1a1.BasicAuth{ + Users: gwapiv1.SecretObjectReference{Name: "parent-users"}, + }, + APIKeyAuth: &egv1a1.APIKeyAuth{ + CredentialRefs: []gwapiv1.SecretObjectReference{{Name: "parent-cred"}}, + }, + Authorization: &egv1a1.Authorization{ + Rules: []egv1a1.AuthorizationRule{{Name: new("parent-rule")}}, + }, + ExtAuth: &egv1a1.ExtAuth{ + HTTP: &egv1a1.HTTPExtAuthService{ + BackendCluster: egv1a1.BackendCluster{ + BackendRefs: []egv1a1.BackendRef{{ + BackendObjectReference: gwapiv1.BackendObjectReference{Name: "parent-http-backend-refs"}, + }}, + }, + }, + ContextExtensions: []*egv1a1.ContextExtension{ + {Name: "shared", Type: egv1a1.ContextExtensionValueTypeValue, Value: new("parent-shared")}, + {Name: "parent-only", Type: egv1a1.ContextExtensionValueTypeValue, Value: new("parent-only")}, + }, + }, + OIDC: &egv1a1.OIDC{ + ClientSecret: gwapiv1.SecretObjectReference{Name: "parent-client-secret"}, + ClientIDRef: &gwapiv1.SecretObjectReference{Name: "parent-client-id"}, + Provider: egv1a1.OIDCProvider{ + Issuer: "https://parent.example.com", + BackendCluster: egv1a1.BackendCluster{ + BackendRefs: []egv1a1.BackendRef{{ + BackendObjectReference: gwapiv1.BackendObjectReference{Name: "parent-oidc-provider"}, + }}, + }, + }, + }, + JWT: &egv1a1.JWT{ + Providers: []egv1a1.JWTProvider{{Name: "parent-jwt-provider"}}, + }, + }, + } + + routePolicy := &egv1a1.SecurityPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "route", Namespace: "route-ns"}, + Spec: egv1a1.SecurityPolicySpec{ + MergeType: new(egv1a1.StrategicMerge), + BasicAuth: &egv1a1.BasicAuth{ + Users: gwapiv1.SecretObjectReference{Name: "route-users"}, + }, + APIKeyAuth: &egv1a1.APIKeyAuth{ + CredentialRefs: []gwapiv1.SecretObjectReference{{Name: "route-cred"}}, + }, + Authorization: &egv1a1.Authorization{ + Rules: []egv1a1.AuthorizationRule{{Name: new("route-rule")}}, + }, + ExtAuth: &egv1a1.ExtAuth{ + HTTP: &egv1a1.HTTPExtAuthService{ + BackendCluster: egv1a1.BackendCluster{ + BackendRefs: []egv1a1.BackendRef{{ + BackendObjectReference: gwapiv1.BackendObjectReference{Name: "route-http-backend-refs"}, + }}, + }, + }, + ContextExtensions: []*egv1a1.ContextExtension{ + {Name: "shared", Type: egv1a1.ContextExtensionValueTypeValue, Value: new("route-shared")}, + {Name: "route-only", Type: egv1a1.ContextExtensionValueTypeValue, Value: new("route-only")}, + }, + }, + OIDC: &egv1a1.OIDC{ + ClientSecret: gwapiv1.SecretObjectReference{Name: "route-client-secret"}, + ClientIDRef: &gwapiv1.SecretObjectReference{Name: "route-client-id"}, + Provider: egv1a1.OIDCProvider{ + Issuer: "https://route.example.com", + BackendCluster: egv1a1.BackendCluster{ + BackendRefs: []egv1a1.BackendRef{{ + BackendObjectReference: gwapiv1.BackendObjectReference{Name: "route-oidc-provider"}, + }}, + }, + }, + }, + JWT: &egv1a1.JWT{ + Providers: []egv1a1.JWTProvider{{Name: "route-jwt-provider"}}, + }, + }, + } + + _, owners, err := mergeSecurityPolicy(routePolicy, parentPolicy) + require.NoError(t, err) + require.NotNil(t, owners) + + assert.Same(t, routePolicy, owners.basicAuth) + assert.Same(t, routePolicy, owners.apiKeyAuthCredentialRefs) + assert.Same(t, routePolicy, owners.authorizationRules) + assert.Same(t, routePolicy, owners.extAuth) + assert.Same(t, routePolicy, owners.extAuthBackendRefs) + assert.Same(t, routePolicy, owners.oidc) + assert.Same(t, routePolicy, owners.oidcClientIDRef) + assert.Same(t, routePolicy, owners.oidcClientSecret) + assert.Same(t, routePolicy, owners.oidcProviderBackendRefs) + assert.Same(t, routePolicy, owners.jwtProviders) + assert.Same(t, routePolicy, owners.extAuthContextExtensions["shared"]) + assert.Same(t, routePolicy, owners.extAuthContextExtensions["route-only"]) + assert.Same(t, parentPolicy, owners.extAuthContextExtensions["parent-only"]) + }) + + t.Run("uses parent owner for grpc backend fields when route does not set them", func(t *testing.T) { + parentPolicy := &egv1a1.SecurityPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "parent", Namespace: "parent-ns"}, + Spec: egv1a1.SecurityPolicySpec{ + MergeType: new(egv1a1.StrategicMerge), + ExtAuth: &egv1a1.ExtAuth{ + GRPC: &egv1a1.GRPCExtAuthService{ + BackendCluster: egv1a1.BackendCluster{ + BackendRefs: []egv1a1.BackendRef{{ + BackendObjectReference: gwapiv1.BackendObjectReference{Name: "parent-grpc-backend-refs"}, + }}, + }, + }, + }, + }, + } + + routePolicy := &egv1a1.SecurityPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "route", Namespace: "route-ns"}, + Spec: egv1a1.SecurityPolicySpec{MergeType: new(egv1a1.StrategicMerge)}, + } + + _, owners, err := mergeSecurityPolicy(routePolicy, parentPolicy) + require.NoError(t, err) + require.NotNil(t, owners) + + assert.Same(t, parentPolicy, owners.extAuth) + assert.Same(t, parentPolicy, owners.extAuthBackendRefs) + }) +} diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-gateway-same-listener.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-gateway-same-listener.in.yaml index e13c11c4b5..ee8b0bb9d9 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-gateway-same-listener.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-gateway-same-listener.in.yaml @@ -108,20 +108,6 @@ services: - port: 9001 name: http protocol: TCP -referenceGrants: -- apiVersion: gateway.networking.k8s.io/v1alpha2 - kind: ReferenceGrant - metadata: - namespace: envoy-gateway - name: allow-securitypolicy-to-auth-service - spec: - from: - - group: gateway.envoyproxy.io - kind: SecurityPolicy - namespace: default - to: - - group: "" - kind: Service endpointSlices: - apiVersion: discovery.k8s.io/v1 kind: EndpointSlice @@ -149,11 +135,3 @@ secrets: type: Opaque data: .htpasswd: dXNlcjE6e1NIQX15LzJzWUFqNXlyUUlONFRMMFlkUGRtR05LcGM9 -- apiVersion: v1 - kind: Secret - metadata: - namespace: default - name: users-secret - type: Opaque - data: - .htpasswd: dXNlcjE6e1NIQX15LzJzWUFqNXlyUUlONFRMMFlkUGRtR05LcGM9 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-gateway-same-listener.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-gateway-same-listener.out.yaml index 96bfbf8a65..90fae2f530 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-gateway-same-listener.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-gateway-same-listener.out.yaml @@ -372,7 +372,7 @@ xdsIR: prefix: /foo security: basicAuth: - name: securitypolicy/default/policy-for-route + name: securitypolicy/envoy-gateway/policy-for-gateway-1 users: '[redacted]' cors: allowMethods: @@ -473,9 +473,9 @@ xdsIR: destination: metadata: kind: SecurityPolicy - name: policy-for-route - namespace: default - name: securitypolicy/default/policy-for-route/extauth/0 + name: policy-for-gateway-2 + namespace: envoy-gateway + name: securitypolicy/envoy-gateway/policy-for-gateway-2/extauth/0 settings: - addressType: IP endpoints: @@ -486,11 +486,11 @@ xdsIR: name: auth-service namespace: envoy-gateway sectionName: "9001" - name: securitypolicy/default/policy-for-route/extauth/0/backend/0 + name: securitypolicy/envoy-gateway/policy-for-gateway-2/extauth/0/backend/0 protocol: HTTP weight: 1 path: "" - name: securitypolicy/default/policy-for-route + name: securitypolicy/envoy-gateway/policy-for-gateway-2 readyListener: address: 0.0.0.0 ipFamily: IPv4 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-listener.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-listener.in.yaml index 5dc3bb2a76..4b1592dfc3 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-listener.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-listener.in.yaml @@ -104,20 +104,6 @@ services: - port: 9001 name: http protocol: TCP -referenceGrants: -- apiVersion: gateway.networking.k8s.io/v1alpha2 - kind: ReferenceGrant - metadata: - namespace: envoy-gateway - name: allow-securitypolicy-to-auth-service - spec: - from: - - group: gateway.envoyproxy.io - kind: SecurityPolicy - namespace: default - to: - - group: "" - kind: Service endpointSlices: - apiVersion: discovery.k8s.io/v1 kind: EndpointSlice @@ -145,11 +131,3 @@ secrets: type: Opaque data: .htpasswd: dXNlcjE6e1NIQX15LzJzWUFqNXlyUUlONFRMMFlkUGRtR05LcGM9 -- apiVersion: v1 - kind: Secret - metadata: - namespace: default - name: users-secret-a - type: Opaque - data: - .htpasswd: dXNlcjE6e1NIQX15LzJzWUFqNXlyUUlONFRMMFlkUGRtR05LcGM9 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-listener.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-listener.out.yaml index 19999e159b..5c344fd5ea 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-listener.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-multi-listener.out.yaml @@ -350,7 +350,7 @@ xdsIR: prefix: /foo security: basicAuth: - name: securitypolicy/default/policy-for-route + name: securitypolicy/envoy-gateway/policy-for-listener-a users: '[redacted]' cors: allowMethods: @@ -420,9 +420,9 @@ xdsIR: destination: metadata: kind: SecurityPolicy - name: policy-for-route - namespace: default - name: securitypolicy/default/policy-for-route/extauth/0 + name: policy-for-listener-b + namespace: envoy-gateway + name: securitypolicy/envoy-gateway/policy-for-listener-b/extauth/0 settings: - addressType: IP endpoints: @@ -433,11 +433,11 @@ xdsIR: name: auth-service namespace: envoy-gateway sectionName: "9001" - name: securitypolicy/default/policy-for-route/extauth/0/backend/0 + name: securitypolicy/envoy-gateway/policy-for-listener-b/extauth/0/backend/0 protocol: HTTP weight: 1 path: "" - name: securitypolicy/default/policy-for-route + name: securitypolicy/envoy-gateway/policy-for-listener-b readyListener: address: 0.0.0.0 ipFamily: IPv4 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-needs-fieldowner.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-needs-fieldowner.in.yaml new file mode 100644 index 0000000000..691512bb28 --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-needs-fieldowner.in.yaml @@ -0,0 +1,231 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/foo" + backendRefs: + - name: service-1 + port: 8080 +services: +- apiVersion: v1 + kind: Service + metadata: + namespace: envoy-gateway + name: grpc-backend + spec: + ports: + - port: 9000 + name: grpc + protocol: TCP +- apiVersion: v1 + kind: Service + metadata: + namespace: default + name: grpc-backend + spec: + ports: + - port: 9000 + name: grpc + protocol: TCP +endpointSlices: +- apiVersion: discovery.k8s.io/v1 + kind: EndpointSlice + metadata: + namespace: envoy-gateway + name: endpointslice-grpc-backend + labels: + kubernetes.io/service-name: grpc-backend + addressType: IPv4 + ports: + - name: grpc + protocol: TCP + port: 9000 + endpoints: + - addresses: + - 8.8.8.8 + conditions: + ready: true +- apiVersion: discovery.k8s.io/v1 + kind: EndpointSlice + metadata: + namespace: default + name: endpointslice-grpc-backend + labels: + kubernetes.io/service-name: grpc-backend + addressType: IPv4 + ports: + - name: grpc + protocol: TCP + port: 9000 + endpoints: + - addresses: + - 9.9.9.9 + conditions: + ready: true +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: envoy-gateway + name: policy-for-gateway + # check: oidc suffix value [c3556d06] + uid: "aaaaaaaa-0000-0000-0000-000000000001" + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: http + # merge basicAuth check: + # - can resolve auth secret in parent ns (envoy-gateway) + basicAuth: + users: + name: users-secret + # merge extAuth check: + # - can resolve backendRef in parent ns (envoy-gateway) + # - merge contextExtensions by name and resolve each item from its owning policy namespace + extAuth: + grpc: + backendRefs: + - name: grpc-backend + port: 9000 + contextExtensions: + - name: shared + type: ValueRef + valueRef: + Group: "" + Kind: ConfigMap + Name: context-extension-for-gateway + key: data-shared + - name: parent-only + type: ValueRef + valueRef: + Group: "" + Kind: ConfigMap + Name: context-extension-for-gateway + key: data + # merge oidc check: + # - can resolve provider backendRef in parent ns (envoy-gateway) + # - can resolve client secret in parent ns (envoy-gateway) + oidc: + provider: + backendRefs: + - group: gateway.envoyproxy.io + kind: Backend + name: backend-ip + port: 3000 + issuer: "https://oauth.foo.com" + authorizationEndpoint: "https://oauth.foo.com/oauth2/v2/auth" + tokenEndpoint: "https://oauth.foo.com/token" + clientID: "client1.apps.googleusercontent.com" + clientSecret: + name: "client-secret" +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-for-route + # check: oidc suffix value [6ac6170b] + uid: "bbbbbbbb-0000-0000-0000-000000000002" + spec: + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + extAuth: + grpc: + backendRefs: + - name: grpc-backend + port: 9000 + contextExtensions: + - name: shared + type: ValueRef + valueRef: + Group: "" + Kind: ConfigMap + Name: context-extension-for-route + key: data-shared + - name: route-only + type: ValueRef + valueRef: + Group: "" + Kind: ConfigMap + Name: context-extension-for-route + key: data +backends: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: Backend + metadata: + namespace: envoy-gateway + name: backend-ip + spec: + endpoints: + - ip: + address: 7.7.7.7 + port: 3000 +secrets: +- apiVersion: v1 + kind: Secret + metadata: + namespace: envoy-gateway + name: users-secret + type: Opaque + data: + .htpasswd: dXNlcjE6e1NIQX15LzJzWUFqNXlyUUlONFRMMFlkUGRtR05LcGM9 +- apiVersion: v1 + kind: Secret + metadata: + namespace: envoy-gateway + name: client-secret + data: + client-secret: Y2xpZW50MTpzZWNyZXQK +- apiVersion: v1 + kind: Secret + metadata: + namespace: envoy-gateway-system + name: envoy-oidc-hmac + data: + hmac-secret: qrOYACHXoe7UEDI/raOjNSx+Z9ufXSc/22C3T6X/zPY= +configmaps: +- apiVersion: v1 + kind: ConfigMap + metadata: + namespace: envoy-gateway + name: context-extension-for-gateway + data: + data-shared: test-key-gateway + data: test-key-gateway +- apiVersion: v1 + kind: ConfigMap + metadata: + namespace: default + name: context-extension-for-route + data: + data-shared: test-key-route + data: test-key-route diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge-needs-fieldowner.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge-needs-fieldowner.out.yaml new file mode 100644 index 0000000000..6abe6abfb9 --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge-needs-fieldowner.out.yaml @@ -0,0 +1,379 @@ +backends: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: Backend + metadata: + name: backend-ip + namespace: envoy-gateway + spec: + endpoints: + - ip: + address: 7.7.7.7 + port: 3000 + status: + conditions: + - lastTransitionTime: null + message: The Backend was accepted + reason: Accepted + status: "True" + type: Accepted +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: httproute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /foo + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: policy-for-route + namespace: default + uid: bbbbbbbb-0000-0000-0000-000000000002 + spec: + extAuth: + contextExtensions: + - name: shared + type: ValueRef + valueRef: + group: "" + key: data-shared + kind: ConfigMap + name: context-extension-for-route + - name: route-only + type: ValueRef + valueRef: + group: "" + key: data + kind: ConfigMap + name: context-extension-for-route + grpc: + backendRefs: + - name: grpc-backend + port: 9000 + mergeType: StrategicMerge + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Merged with policy envoy-gateway/policy-for-gateway + reason: Merged + status: "True" + type: Merged + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + name: policy-for-gateway + namespace: envoy-gateway + uid: aaaaaaaa-0000-0000-0000-000000000001 + spec: + basicAuth: + users: + name: users-secret + extAuth: + contextExtensions: + - name: shared + type: ValueRef + valueRef: + group: "" + key: data-shared + kind: ConfigMap + name: context-extension-for-gateway + - name: parent-only + type: ValueRef + valueRef: + group: "" + key: data + kind: ConfigMap + name: context-extension-for-gateway + grpc: + backendRefs: + - name: grpc-backend + port: 9000 + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: + name: client-secret + provider: + authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth + backendRefs: + - group: gateway.envoyproxy.io + kind: Backend + name: backend-ip + port: 3000 + issuer: https://oauth.foo.com + tokenEndpoint: https://oauth.foo.com/token + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: http + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: 'This policy is being merged by other securityPolicies for these + routes: [default/httproute-1]' + reason: Merged + status: "True" + type: Merged + controllerName: gateway.envoyproxy.io/gatewayclass-controller +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-1/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /foo + security: + basicAuth: + name: securitypolicy/envoy-gateway/policy-for-gateway + users: '[redacted]' + extAuth: + contextExtensions: + - name: shared + value: '[redacted]' + - name: route-only + value: '[redacted]' + - name: parent-only + value: '[redacted]' + grpc: + authority: grpc-backend.default:9000 + destination: + metadata: + kind: SecurityPolicy + name: policy-for-route + namespace: default + name: securitypolicy/default/policy-for-route/extauth/0 + settings: + - addressType: IP + endpoints: + - host: 9.9.9.9 + port: 9000 + metadata: + kind: Service + name: grpc-backend + namespace: default + sectionName: "9000" + name: securitypolicy/default/policy-for-route/extauth/0/backend/0 + protocol: GRPC + weight: 1 + name: securitypolicy/default/policy-for-route + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: '[redacted]' + cookieSuffix: c3556d06 + hmacSecret: '[redacted]' + logoutPath: /logout + name: securitypolicy/envoy-gateway/policy-for-gateway + provider: + authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth + destination: + metadata: + kind: SecurityPolicy + name: policy-for-gateway + namespace: envoy-gateway + name: securitypolicy/envoy-gateway/policy-for-gateway/oidc/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 3000 + metadata: + kind: Backend + name: backend-ip + namespace: envoy-gateway + name: securitypolicy/envoy-gateway/policy-for-gateway/oidc/0/backend/0 + protocol: HTTPS + weight: 1 + tokenEndpoint: https://oauth.foo.com/token + redirectPath: /oauth2/callback + redirectURL: '%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback' + refreshToken: true + scopes: + - openid + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-merge.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-merge.out.yaml index ee2655e8a0..b155bfe2ed 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-merge.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-merge.out.yaml @@ -242,7 +242,7 @@ xdsIR: prefix: /foo security: basicAuth: - name: securitypolicy/default/policy-for-route + name: securitypolicy/envoy-gateway/policy-for-gateway users: '[redacted]' cors: allowHeaders: diff --git a/internal/utils/merge_test.go b/internal/utils/merge_test.go index 73f6a584c9..2c91e27bcd 100644 --- a/internal/utils/merge_test.go +++ b/internal/utils/merge_test.go @@ -19,41 +19,55 @@ import ( "github.com/envoyproxy/gateway/internal/utils/test" ) -func TestMergeBackendTrafficPolicy(t *testing.T) { +func TestMergePolicy(t *testing.T) { baseDir := "testdata" - caseFiles, err := filepath.Glob(filepath.Join(baseDir, "backendtrafficpolicy_*.in.yaml")) + caseFiles, err := filepath.Glob(filepath.Join(baseDir, "*policy_*.in.yaml")) require.NoError(t, err) for _, caseFile := range caseFiles { - // get case name from path - caseName := strings.TrimPrefix(strings.TrimSuffix(caseFile, ".in.yaml"), baseDir+"/backendtrafficpolicy_") + caseName := strings.TrimPrefix(strings.TrimSuffix(caseFile, ".in.yaml"), baseDir+"/") + policyType := strings.SplitN(caseName, "_", 2)[0] + t.Run(caseName, func(t *testing.T) { - for _, mergeType := range []egv1a1.MergeType{egv1a1.StrategicMerge, egv1a1.JSONMerge} { - patchedInput := strings.Replace(caseFile, ".in.yaml", ".patch.yaml", 1) - var output string - if mergeType == egv1a1.StrategicMerge { - output = strings.Replace(caseFile, ".in.yaml", ".strategicmerge.out.yaml", 1) - } else { - output = strings.Replace(caseFile, ".in.yaml", ".jsonmerge.out.yaml", 1) - } + switch policyType { + case "backendtrafficpolicy": + runMergePolicyTest[*egv1a1.BackendTrafficPolicy](t, caseFile) + case "securitypolicy": + runMergePolicyTest[*egv1a1.SecurityPolicy](t, caseFile) + default: + t.Fatalf("unsupported policy type %q in %s", policyType, caseFile) + } + }) + } +} - original := readObject[*egv1a1.BackendTrafficPolicy](t, caseFile) - patch := readObject[*egv1a1.BackendTrafficPolicy](t, patchedInput) +func runMergePolicyTest[T client.Object](t *testing.T, caseFile string) { + t.Helper() - got, err := Merge(original, patch, mergeType) - require.NoError(t, err) + for _, mergeType := range []egv1a1.MergeType{egv1a1.StrategicMerge, egv1a1.JSONMerge} { + patchedInput := strings.Replace(caseFile, ".in.yaml", ".patch.yaml", 1) + var output string + if mergeType == egv1a1.StrategicMerge { + output = strings.Replace(caseFile, ".in.yaml", ".strategicmerge.out.yaml", 1) + } else { + output = strings.Replace(caseFile, ".in.yaml", ".jsonmerge.out.yaml", 1) + } - if test.OverrideTestData() { - b, err := yaml.Marshal(got) - require.NoError(t, err) - require.NoError(t, os.WriteFile(output, b, 0o600)) - continue - } + original := readObject[T](t, caseFile) + patch := readObject[T](t, patchedInput) - expected := readObject[*egv1a1.BackendTrafficPolicy](t, output) - require.Equal(t, expected, got) - } - }) + got, err := Merge(original, patch, mergeType) + require.NoError(t, err) + + if test.OverrideTestData() { + b, err := yaml.Marshal(got) + require.NoError(t, err) + require.NoError(t, os.WriteFile(output, b, 0o600)) + continue + } + + expected := readObject[T](t, output) + require.Equal(t, expected, got) } } diff --git a/internal/utils/testdata/securitypolicy_all.in.yaml b/internal/utils/testdata/securitypolicy_all.in.yaml new file mode 100644 index 0000000000..2524a7a0f9 --- /dev/null +++ b/internal/utils/testdata/securitypolicy_all.in.yaml @@ -0,0 +1,49 @@ +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: SecurityPolicy +metadata: + name: original +spec: + basicAuth: + users: + name: users-parent + apiKeyAuth: + credentialRefs: + - name: api-keys-parent + authorization: + rules: + - name: parent-rule + action: Allow + principal: + headers: + - name: x-role + values: + - parent + extAuth: + http: + backendRefs: + - name: ext-auth-parent + port: 9001 + contextExtensions: + - name: shared + type: Value + value: parent + - name: parent-only + type: Value + value: parent-only + oidc: + provider: + issuer: https://oidc.parent.example.com + backendRefs: + - name: oidc-parent + port: 443 + clientIDRef: + name: oidc-client-id-parent + clientSecret: + name: oidc-client-secret-parent + jwt: + providers: + - name: jwt-parent + issuer: https://issuer.parent.example.com + localJWKS: + type: Inline + inline: "{}" diff --git a/internal/utils/testdata/securitypolicy_all.jsonmerge.out.yaml b/internal/utils/testdata/securitypolicy_all.jsonmerge.out.yaml new file mode 100644 index 0000000000..eeb4cb0d16 --- /dev/null +++ b/internal/utils/testdata/securitypolicy_all.jsonmerge.out.yaml @@ -0,0 +1,58 @@ +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: SecurityPolicy +metadata: + name: patched +spec: + apiKeyAuth: + credentialRefs: + - name: api-keys-route + extractFrom: null + sanitize: true + authorization: + defaultAction: null + rules: + - action: Deny + name: route-rule + principal: + headers: + - name: x-role + values: + - route + basicAuth: + users: + name: users-route + extAuth: + contextExtensions: + - name: shared + type: Value + value: route + - name: route-only + type: Value + value: route-only + grpc: + backendRefs: + - name: ext-auth-route + port: 9002 + http: + backendRefs: + - name: ext-auth-parent + port: 9001 + jwt: + providers: + - issuer: https://issuer.route.example.com + localJWKS: + inline: '{}' + type: Inline + name: jwt-route + oidc: + clientIDRef: + name: oidc-client-id-route + clientSecret: + name: oidc-client-secret-route + provider: + backendRefs: + - name: oidc-route + port: 8443 + issuer: "" +status: + ancestors: null diff --git a/internal/utils/testdata/securitypolicy_all.patch.yaml b/internal/utils/testdata/securitypolicy_all.patch.yaml new file mode 100644 index 0000000000..fcee7aca7d --- /dev/null +++ b/internal/utils/testdata/securitypolicy_all.patch.yaml @@ -0,0 +1,50 @@ +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: SecurityPolicy +metadata: + name: patched +spec: + basicAuth: + users: + name: users-route + apiKeyAuth: + credentialRefs: + - name: api-keys-route + sanitize: true + authorization: + rules: + - name: route-rule + action: Deny + principal: + headers: + - name: x-role + values: + - route + extAuth: + grpc: + backendRefs: + - name: ext-auth-route + port: 9002 + # arrays all replace + contextExtensions: + - name: shared + type: Value + value: route + - name: route-only + type: Value + value: route-only + oidc: + provider: + backendRefs: + - name: oidc-route + port: 8443 + clientIDRef: + name: oidc-client-id-route + clientSecret: + name: oidc-client-secret-route + jwt: + providers: + - name: jwt-route + issuer: https://issuer.route.example.com + localJWKS: + type: Inline + inline: "{}" diff --git a/internal/utils/testdata/securitypolicy_all.strategicmerge.out.yaml b/internal/utils/testdata/securitypolicy_all.strategicmerge.out.yaml new file mode 100644 index 0000000000..8eeabc5212 --- /dev/null +++ b/internal/utils/testdata/securitypolicy_all.strategicmerge.out.yaml @@ -0,0 +1,61 @@ +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: SecurityPolicy +metadata: + name: patched +spec: + apiKeyAuth: + credentialRefs: + - name: api-keys-route + extractFrom: null + sanitize: true + authorization: + defaultAction: null + rules: + - action: Deny + name: route-rule + principal: + headers: + - name: x-role + values: + - route + basicAuth: + users: + name: users-route + extAuth: + contextExtensions: + - name: shared + type: Value + value: route + - name: route-only + type: Value + value: route-only + - name: parent-only + type: Value + value: parent-only + grpc: + backendRefs: + - name: ext-auth-route + port: 9002 + http: + backendRefs: + - name: ext-auth-parent + port: 9001 + jwt: + providers: + - issuer: https://issuer.route.example.com + localJWKS: + inline: '{}' + type: Inline + name: jwt-route + oidc: + clientIDRef: + name: oidc-client-id-route + clientSecret: + name: oidc-client-secret-route + provider: + backendRefs: + - name: oidc-route + port: 8443 + issuer: "" +status: + ancestors: null diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 3271a4f907..7211ceb5ea 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -2,6 +2,7 @@ date: Pending # Changes that are expected to cause an incompatibility with previous versions, such as deletions or modifications to existing APIs. breaking changes: | + Merged SecurityPolicy IR/xDS resource names (OIDC, BasicAuth, ExtAuth, JWT) now derive from the policy that contributes the field (parent or route) rather than always using the route-level policy. EnvoyPatchPolicy users who reference those generated names must update their patch targets. # Updates addressing vulnerabilities, security flaws, or compliance requirements. security updates: | @@ -10,6 +11,7 @@ security updates: | new features: | bug fixes: | + Fixed SecurityPolicy merge using the wrong policy as the owner for resource references and IR generation. # Enhancements that improve performance. performance improvements: | diff --git a/site/content/en/latest/concepts/gateway_api_extensions/security-policy.md b/site/content/en/latest/concepts/gateway_api_extensions/security-policy.md index 8a2ccd710b..d1454f2f03 100644 --- a/site/content/en/latest/concepts/gateway_api_extensions/security-policy.md +++ b/site/content/en/latest/concepts/gateway_api_extensions/security-policy.md @@ -268,17 +268,7 @@ In this example, the route-level policy merges with the gateway-level policy, re - When `mergeType` is unset, no merging occurs - only the most specific policy takes effect - The merged configuration combines both policies, enabling layered security strategies - When the same security feature is configured in both parent and child policies (e.g., both define CORS), the child policy's configuration takes precedence for that specific feature - -### Important: Namespace Behavior with Secret References - -When policies are merged, secret references inherited from parent policies must be resolvable from the **route policy's namespace**. This is because the merged policy retains the identity (including namespace) of the route-level policy. - -**Example scenario:** -- Gateway policy in namespace `envoy-gateway` references `basic-auth-secret` -- Route policy in namespace `default` merges with the gateway policy -- The secret `basic-auth-secret` must exist in the `default` namespace for the merged policy to work - -**Best Practice:** When using policy merging with secret-based authentication (BasicAuth, OIDC, JWT, APIKeyAuth), ensure that required secrets are available in each route's namespace, or design your namespace strategy accordingly. +- Secret references and backend references are resolved against the namespace of the **policy that originally configured the field** (either route or parent). For example, if a Gateway policy defines BasicAuth, its secret is looked up in the Gateway policy's namespace even after merging. ## Related Resources - [API Key Authentication](../../tasks/security/apikey-auth.md)