From bb4607a0b4648ca0256efde0ddb218c54cb0a35e Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 10 Dec 2025 14:24:15 +0000 Subject: [PATCH 01/16] Add support for Basic Auth through AuthenticationFilter --- apis/v1alpha1/authenticationfilter_types.go | 4 + apis/v1alpha1/register.go | 2 + .../templates/clusterrole.yaml | 2 + internal/controller/handler.go | 6 + internal/controller/manager.go | 7 + internal/controller/manager_test.go | 5 + internal/controller/nginx/config/generator.go | 28 ++++ .../controller/nginx/config/http/config.go | 12 ++ internal/controller/nginx/config/servers.go | 57 ++++++++ .../nginx/config/servers_template.go | 5 + internal/controller/state/change_processor.go | 38 +++-- .../controller/state/conditions/conditions.go | 22 +++ .../state/dataplane/configuration.go | 36 +++-- .../state/dataplane/configuration_test.go | 4 +- .../controller/state/dataplane/convert.go | 28 ++++ internal/controller/state/dataplane/types.go | 55 ++++--- .../state/graph/authentication_filter.go | 137 ++++++++++++++++++ .../controller/state/graph/common_filter.go | 6 +- .../state/graph/extension_ref_filter.go | 15 +- internal/controller/state/graph/graph.go | 39 +++-- internal/controller/state/graph/graph_test.go | 76 ++++++++++ internal/controller/state/graph/grpcroute.go | 25 +++- .../controller/state/graph/grpcroute_test.go | 5 +- internal/controller/state/graph/httproute.go | 24 ++- .../controller/state/graph/httproute_test.go | 5 +- .../controller/state/graph/route_common.go | 7 +- internal/controller/state/graph/secret.go | 26 +++- .../controller/status/prepare_requests.go | 33 +++++ internal/controller/status/status_setters.go | 61 ++++++++ internal/framework/kinds/kinds.go | 2 + 30 files changed, 675 insertions(+), 97 deletions(-) create mode 100644 internal/controller/state/graph/authentication_filter.go diff --git a/apis/v1alpha1/authenticationfilter_types.go b/apis/v1alpha1/authenticationfilter_types.go index b441e0f721..682d8e287a 100644 --- a/apis/v1alpha1/authenticationfilter_types.go +++ b/apis/v1alpha1/authenticationfilter_types.go @@ -57,6 +57,10 @@ const ( AuthTypeBasic AuthType = "Basic" ) +const ( + AuthKeyBasic = "auth" // Key in the Secret data for Basic Auth credentials. +) + // BasicAuth configures HTTP Basic Authentication. type BasicAuth struct { // SecretRef allows referencing a Secret in the same namespace. diff --git a/apis/v1alpha1/register.go b/apis/v1alpha1/register.go index d01bfcd6e1..2e57073ccc 100644 --- a/apis/v1alpha1/register.go +++ b/apis/v1alpha1/register.go @@ -40,6 +40,8 @@ func addKnownTypes(scheme *runtime.Scheme) error { &SnippetsFilterList{}, &UpstreamSettingsPolicy{}, &UpstreamSettingsPolicyList{}, + &AuthenticationFilter{}, + &AuthenticationFilterList{}, ) // AddToGroupVersion allows the serialization of client types like ListOptions. metav1.AddToGroupVersion(scheme, SchemeGroupVersion) diff --git a/charts/nginx-gateway-fabric/templates/clusterrole.yaml b/charts/nginx-gateway-fabric/templates/clusterrole.yaml index 5d9ea4a5be..94928efd03 100644 --- a/charts/nginx-gateway-fabric/templates/clusterrole.yaml +++ b/charts/nginx-gateway-fabric/templates/clusterrole.yaml @@ -129,6 +129,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters {{- if .Values.nginxGateway.snippetsFilters.enable }} - snippetsfilters {{- end }} @@ -142,6 +143,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status {{- if .Values.nginxGateway.snippetsFilters.enable }} - snippetsfilters/status {{- end }} diff --git a/internal/controller/handler.go b/internal/controller/handler.go index 35fdec45ff..bdc66326eb 100644 --- a/internal/controller/handler.go +++ b/internal/controller/handler.go @@ -383,6 +383,11 @@ func (h *eventHandlerImpl) updateStatuses(ctx context.Context, gr *graph.Graph, transitionTime, h.cfg.gatewayCtlrName, ) + authenticationFilterReqs := status.PrepareAuthenticationFilterRequests( + gr.AuthenticationFilters, + transitionTime, + h.cfg.gatewayCtlrName, + ) // unfortunately, status is not on clusterState stored by the change processor, so we need to make a k8sAPI call here ipList := &inference.InferencePoolList{} @@ -418,6 +423,7 @@ func (h *eventHandlerImpl) updateStatuses(ctx context.Context, gr *graph.Graph, reqs = append(reqs, polReqs...) reqs = append(reqs, ngfPolReqs...) reqs = append(reqs, snippetsFilterReqs...) + reqs = append(reqs, authenticationFilterReqs...) reqs = append(reqs, inferencePoolReqs...) h.cfg.statusUpdater.UpdateGroup(ctx, groupAllExceptGateways, reqs...) diff --git a/internal/controller/manager.go b/internal/controller/manager.go index d4e2114e8a..8a5feed2d5 100644 --- a/internal/controller/manager.go +++ b/internal/controller/manager.go @@ -529,6 +529,12 @@ func registerControllers( controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), }, }, + { + objectType: &ngfAPIv1alpha1.AuthenticationFilter{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + }, + }, } if cfg.ExperimentalFeatures { @@ -770,6 +776,7 @@ func prepareFirstEventBatchPreparerArgs(cfg config.Config) ([]client.Object, []c &ngfAPIv1alpha1.ClientSettingsPolicyList{}, &ngfAPIv1alpha2.ObservabilityPolicyList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, + &ngfAPIv1alpha1.AuthenticationFilterList{}, partialObjectMetadataList, } diff --git a/internal/controller/manager_test.go b/internal/controller/manager_test.go index 88aee02ac8..ccc1e93f92 100644 --- a/internal/controller/manager_test.go +++ b/internal/controller/manager_test.go @@ -68,6 +68,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &ngfAPIv1alpha1.ClientSettingsPolicyList{}, &ngfAPIv1alpha2.ObservabilityPolicyList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, + &ngfAPIv1alpha1.AuthenticationFilterList{}, }, }, { @@ -96,6 +97,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &ngfAPIv1alpha1.ClientSettingsPolicyList{}, &ngfAPIv1alpha2.ObservabilityPolicyList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, + &ngfAPIv1alpha1.AuthenticationFilterList{}, }, }, { @@ -124,6 +126,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { partialObjectMetadataList, &inference.InferencePoolList{}, &gatewayv1.GatewayList{}, + &ngfAPIv1alpha1.AuthenticationFilterList{}, }, }, { @@ -152,6 +155,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &ngfAPIv1alpha2.ObservabilityPolicyList{}, &ngfAPIv1alpha1.SnippetsFilterList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, + &ngfAPIv1alpha1.AuthenticationFilterList{}, }, }, { @@ -184,6 +188,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &ngfAPIv1alpha2.ObservabilityPolicyList{}, &ngfAPIv1alpha1.SnippetsFilterList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, + &ngfAPIv1alpha1.AuthenticationFilterList{}, }, }, } diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index 6fa7602c04..48c10f92e9 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -9,6 +9,7 @@ import ( pb "github.com/nginx/agent/v3/api/grpc/mpi/v1" filesHelper "github.com/nginx/agent/v3/pkg/files" + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" ngfConfig "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/config" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" @@ -67,6 +68,8 @@ const ( // nginxPlusConfigFile is the path to the file containing the NGINX Plus API config. nginxPlusConfigFile = httpFolder + "/plus-api.conf" + + basicAuthUserFile = configFolder + "/secrets/%s" ) // Generator generates NGINX configuration files. @@ -133,6 +136,19 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []agent.File { files = append(files, generateCertBundle(id, bundle)) } + for _, server := range conf.HTTPServers { + for _, rule := range server.PathRules { + for _, matchRule := range rule.MatchRules { + if matchRule.Filters.AuthenticationFilter != nil { + if matchRule.Filters.AuthenticationFilter.Basic != nil { + id := fmt.Sprintf("%s/%s", matchRule.Filters.AuthenticationFilter.Basic.SecretName, ngfAPI.AuthKeyBasic) + data := matchRule.Filters.AuthenticationFilter.Basic.Data + files = append(files, generateAuthBasicUserFile(id, data)) + } + } + } + } + } return files } @@ -252,3 +268,15 @@ func generateCertBundle(id dataplane.CertBundleID, cert []byte) agent.File { func generateCertBundleFileName(id dataplane.CertBundleID) string { return filepath.Join(secretsFolder, string(id)+".crt") } + +func generateAuthBasicUserFile(id string, data []byte) agent.File { + return agent.File{ + Meta: &pb.FileMeta{ + Name: fmt.Sprintf(basicAuthUserFile, id), + Hash: filesHelper.GenerateHash(data), + Permissions: file.SecretFileMode, + Size: int64(len(data)), + }, + Contents: data, + } +} diff --git a/internal/controller/nginx/config/http/config.go b/internal/controller/nginx/config/http/config.go index dedfd04349..68d7ed8820 100644 --- a/internal/controller/nginx/config/http/config.go +++ b/internal/controller/nginx/config/http/config.go @@ -64,6 +64,8 @@ type Location struct { Type LocationType // Path is the NGINX location path. Path string + // AuthBasic contains the configuration for basic authentication. + AuthBasic *AuthBasic // ResponseHeaders are custom response headers to be sent. ResponseHeaders ResponseHeaders // ProxySetHeaders are headers to set when proxying requests upstream. @@ -158,6 +160,16 @@ type ProxySSLVerify struct { Name string } +type AuthBasic struct { + Realm string + Data AuthBasicData +} + +type AuthBasicData struct { + FileName string + FileData []byte +} + // ServerConfig holds configuration for an HTTP server and IP family to be used by NGINX. type ServerConfig struct { Servers []Server diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 717664c87c..4db7343a8c 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -9,6 +9,9 @@ import ( "strings" gotemplate "text/template" + ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" @@ -93,8 +96,11 @@ func (g GeneratorImpl) executeServers( includeFileResults := createIncludeExecuteResultsFromServers(servers) + authBasicUserFileResults := createExecuteResultsForAuthBasicUserFile(servers) + allResults := make([]executeResult, 0, len(includeFileResults)+2) allResults = append(allResults, includeFileResults...) + allResults = append(allResults, authBasicUserFileResults...) allResults = append(allResults, serverResult, httpMatchResult) return allResults @@ -704,6 +710,7 @@ func updateLocation( location = updateLocationMirrorRoute(location, pathRule.Path, grpc) location.Includes = append(location.Includes, createIncludesFromLocationSnippetsFilters(filters.SnippetsFilters)...) + location = updateLocationAuthenticationFilter(location, filters.AuthenticationFilter) if filters.RequestRedirect != nil { return updateLocationRedirectFilter(location, filters.RequestRedirect, listenerPort, pathRule) @@ -716,6 +723,56 @@ func updateLocation( return location } +func updateLocationAuthenticationFilter( + location http.Location, + authenticationFilter *dataplane.AuthenticationFilter, +) http.Location { + logger := ctlrZap.New().WithName("update-location-auth-filter") + + // TODO: Remove logging after debugging + if authenticationFilter == nil { + logger.Info("Missing AuthenticationFilter for location", "locationPath", location.Path) + } else if authenticationFilter.Basic == nil { + logger.Info("No Basic authentication configured for location", "locationPath", location.Path) + } + + if authenticationFilter != nil { + logger.Info("Applying authentication filter to location", "locationPath", location.Path) + if authenticationFilter.Basic != nil { + userFilePathAndData := fmt.Sprintf("%s/%s", authenticationFilter.Basic.SecretName, ngfAPI.AuthKeyBasic) + location.AuthBasic = &http.AuthBasic{ + Realm: authenticationFilter.Basic.Realm, + Data: http.AuthBasicData{ + FileName: fmt.Sprintf(basicAuthUserFile, userFilePathAndData), + FileData: authenticationFilter.Basic.Data, + }, + } + logger.Info("location.AuthBasic configured", "locationPath", + location.Path, + "realm", + authenticationFilter.Basic.Realm) + } + } + return location +} + +func createExecuteResultsForAuthBasicUserFile(servers []http.Server) []executeResult { + results := []executeResult{} + for _, server := range servers { + for _, location := range server.Locations { + if location.AuthBasic != nil { + userFilePathAndData := fmt.Sprintf("%s/%s", location.AuthBasic.Data.FileName, ngfAPI.AuthKeyBasic) + result := executeResult{ + dest: fmt.Sprintf(basicAuthUserFile, userFilePathAndData), + data: location.AuthBasic.Data.FileData, + } + results = append(results, result) + } + } + } + return results +} + func updateLocationMirrorRoute(location http.Location, path string, grpc bool) http.Location { if strings.HasPrefix(path, http.InternalMirrorRoutePathPrefix) { location.Type = http.InternalLocationType diff --git a/internal/controller/nginx/config/servers_template.go b/internal/controller/nginx/config/servers_template.go index 9575b77480..d206ee4f3e 100644 --- a/internal/controller/nginx/config/servers_template.go +++ b/internal/controller/nginx/config/servers_template.go @@ -106,6 +106,11 @@ server { include {{ $i.Name }}; {{- end -}} + {{- if $l.AuthBasic }} + auth_basic "{{ $l.AuthBasic.Realm }}"; + auth_basic_user_file {{ $l.AuthBasic.Data.FileName }}; + {{- end }} + {{ range $r := $l.Rewrites }} rewrite {{ $r }}; {{- end }} diff --git a/internal/controller/state/change_processor.go b/internal/controller/state/change_processor.go index d661903b8c..2925e67804 100644 --- a/internal/controller/state/change_processor.go +++ b/internal/controller/state/change_processor.go @@ -86,22 +86,23 @@ type ChangeProcessorImpl struct { // NewChangeProcessorImpl creates a new ChangeProcessorImpl for the Gateway resource with the configured namespace name. func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { clusterStore := graph.ClusterState{ - GatewayClasses: make(map[types.NamespacedName]*v1.GatewayClass), - Gateways: make(map[types.NamespacedName]*v1.Gateway), - HTTPRoutes: make(map[types.NamespacedName]*v1.HTTPRoute), - Services: make(map[types.NamespacedName]*apiv1.Service), - Namespaces: make(map[types.NamespacedName]*apiv1.Namespace), - ReferenceGrants: make(map[types.NamespacedName]*v1beta1.ReferenceGrant), - Secrets: make(map[types.NamespacedName]*apiv1.Secret), - CRDMetadata: make(map[types.NamespacedName]*metav1.PartialObjectMetadata), - BackendTLSPolicies: make(map[types.NamespacedName]*v1.BackendTLSPolicy), - ConfigMaps: make(map[types.NamespacedName]*apiv1.ConfigMap), - NginxProxies: make(map[types.NamespacedName]*ngfAPIv1alpha2.NginxProxy), - GRPCRoutes: make(map[types.NamespacedName]*v1.GRPCRoute), - TLSRoutes: make(map[types.NamespacedName]*v1alpha2.TLSRoute), - NGFPolicies: make(map[graph.PolicyKey]policies.Policy), - SnippetsFilters: make(map[types.NamespacedName]*ngfAPIv1alpha1.SnippetsFilter), - InferencePools: make(map[types.NamespacedName]*inference.InferencePool), + GatewayClasses: make(map[types.NamespacedName]*v1.GatewayClass), + Gateways: make(map[types.NamespacedName]*v1.Gateway), + HTTPRoutes: make(map[types.NamespacedName]*v1.HTTPRoute), + Services: make(map[types.NamespacedName]*apiv1.Service), + Namespaces: make(map[types.NamespacedName]*apiv1.Namespace), + ReferenceGrants: make(map[types.NamespacedName]*v1beta1.ReferenceGrant), + Secrets: make(map[types.NamespacedName]*apiv1.Secret), + CRDMetadata: make(map[types.NamespacedName]*metav1.PartialObjectMetadata), + BackendTLSPolicies: make(map[types.NamespacedName]*v1.BackendTLSPolicy), + ConfigMaps: make(map[types.NamespacedName]*apiv1.ConfigMap), + NginxProxies: make(map[types.NamespacedName]*ngfAPIv1alpha2.NginxProxy), + GRPCRoutes: make(map[types.NamespacedName]*v1.GRPCRoute), + TLSRoutes: make(map[types.NamespacedName]*v1alpha2.TLSRoute), + NGFPolicies: make(map[graph.PolicyKey]policies.Policy), + SnippetsFilters: make(map[types.NamespacedName]*ngfAPIv1alpha1.SnippetsFilter), + AuthenticationFilters: make(map[types.NamespacedName]*ngfAPIv1alpha1.AuthenticationFilter), + InferencePools: make(map[types.NamespacedName]*inference.InferencePool), } processor := &ChangeProcessorImpl{ @@ -225,6 +226,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { store: newObjectStoreMapAdapter(clusterStore.SnippetsFilters), predicate: nil, // we always want to write status to SnippetsFilters so we don't filter them out }, + { + gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.AuthenticationFilter{}), + store: newObjectStoreMapAdapter(clusterStore.AuthenticationFilters), + predicate: nil, // Not sure if this should be nil or if we should track references + }, }, ) diff --git a/internal/controller/state/conditions/conditions.go b/internal/controller/state/conditions/conditions.go index 8e186fe1a4..7b4e6719d1 100644 --- a/internal/controller/state/conditions/conditions.go +++ b/internal/controller/state/conditions/conditions.go @@ -1123,6 +1123,28 @@ func NewSnippetsFilterAccepted() Condition { } } +// NewAuthenticationFilterInvalid returns a Condition that indicates that the AuthenticationFilter is not accepted +// because it is syntactically or semantically invalid. +func NewAuthenticationFilterInvalid(msg string) Condition { + return Condition{ + Type: string(ngfAPI.AuthenticationFilterConditionTypeAccepted), + Status: metav1.ConditionFalse, + Reason: string(ngfAPI.AuthenticationFilterConditionReasonInvalid), + Message: msg, + } +} + +// NewAuthenticationFilterAccepted returns a Condition that indicates that the AuthenticationFilter is accepted +// because it is valid. +func NewAuthenticationFilterAccepted() Condition { + return Condition{ + Type: string(ngfAPI.AuthenticationFilterConditionTypeAccepted), + Status: metav1.ConditionTrue, + Reason: string(ngfAPI.AuthenticationFilterConditionReasonAccepted), + Message: "The AuthenticationFilter is accepted", + } +} + // NewObservabilityPolicyAffected returns a Condition that indicates that an ObservabilityPolicy // is applied to the resource. func NewObservabilityPolicyAffected() Condition { diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 26bc3c58ee..1d7826f44e 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -58,7 +58,7 @@ func BuildConfiguration( baseHTTPConfig := buildBaseHTTPConfig(gateway, gatewaySnippetsFilters) baseStreamConfig := buildBaseStreamConfig(gateway) - httpServers, sslServers := buildServers(gateway, g.ReferencedServices) + httpServers, sslServers := buildServers(gateway, g.ReferencedServices, g.ReferencedSecrets) backendGroups := buildBackendGroups(append(httpServers, sslServers...)) upstreams := buildUpstreams( ctx, @@ -459,6 +459,7 @@ func convertBackendTLS(btp *graph.BackendTLSPolicy, gwNsName types.NamespacedNam func buildServers( gateway *graph.Gateway, referencedServices map[types.NamespacedName]*graph.ReferencedService, + referencedSecrets map[types.NamespacedName]*graph.Secret, ) (http, ssl []VirtualServer) { rulesForProtocol := map[v1.ProtocolType]portPathRules{ v1.HTTPProtocolType: make(portPathRules), @@ -476,7 +477,7 @@ func buildServers( rulesForProtocol[l.Source.Protocol][l.Source.Port] = rules } - rules.upsertListener(l, gateway, referencedServices) + rules.upsertListener(l, gateway, referencedServices, referencedSecrets) } } @@ -541,6 +542,7 @@ func (hpr *hostPathRules) upsertListener( l *graph.Listener, gateway *graph.Gateway, referencedServices map[types.NamespacedName]*graph.ReferencedService, + referencedSecrets map[types.NamespacedName]*graph.Secret, ) { hpr.listenersExist = true hpr.port = l.Source.Port @@ -554,7 +556,7 @@ func (hpr *hostPathRules) upsertListener( continue } - hpr.upsertRoute(r, l, gateway, referencedServices) + hpr.upsertRoute(r, l, gateway, referencedServices, referencedSecrets) } } @@ -563,6 +565,7 @@ func (hpr *hostPathRules) upsertRoute( listener *graph.Listener, gateway *graph.Gateway, referencedServices map[types.NamespacedName]*graph.ReferencedService, + referencedSecrets map[types.NamespacedName]*graph.Secret, ) { var hostnames []string GRPC := route.RouteType == graph.RouteTypeGRPC @@ -608,7 +611,7 @@ func (hpr *hostPathRules) upsertRoute( var filters HTTPFilters if rule.Filters.Valid { - filters = createHTTPFilters(rule.Filters.Filters, idx, routeNsName) + filters = createHTTPFilters(rule.Filters.Filters, idx, routeNsName, referencedSecrets) } else { filters = HTTPFilters{ InvalidFilter: &InvalidHTTPFilter{}, @@ -893,7 +896,12 @@ func getPath(path *v1.HTTPPathMatch) string { return *path.Value } -func createHTTPFilters(filters []graph.Filter, ruleIdx int, routeNsName types.NamespacedName) HTTPFilters { +func createHTTPFilters( + filters []graph.Filter, + ruleIdx int, + routeNsName types.NamespacedName, + referencedSecrets map[types.NamespacedName]*graph.Secret, +) HTTPFilters { var result HTTPFilters for _, f := range filters { @@ -924,11 +932,19 @@ func createHTTPFilters(filters []graph.Filter, ruleIdx int, routeNsName types.Na result.ResponseHeaderModifiers = convertHTTPHeaderFilter(f.ResponseHeaderModifier) } case graph.FilterExtensionRef: - if f.ResolvedExtensionRef != nil && f.ResolvedExtensionRef.SnippetsFilter != nil { - result.SnippetsFilters = append( - result.SnippetsFilters, - convertSnippetsFilter(f.ResolvedExtensionRef.SnippetsFilter), - ) + if f.ResolvedExtensionRef != nil { + if f.ResolvedExtensionRef.SnippetsFilter != nil { + result.SnippetsFilters = append( + result.SnippetsFilters, + convertSnippetsFilter(f.ResolvedExtensionRef.SnippetsFilter), + ) + } + if f.ResolvedExtensionRef.AuthenticationFilter != nil { + result.AuthenticationFilter = convertAuthenticationFilter( + f.ResolvedExtensionRef.AuthenticationFilter, + referencedSecrets, + ) + } } } } diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 330a0cef79..3bb37a9d00 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -2485,7 +2485,7 @@ func TestUpsertRoute_PathRuleHasInferenceBackend(t *testing.T) { } hpr := newHostPathRules() - hpr.upsertRoute(route, listener, gateway, nil) + hpr.upsertRoute(route, listener, gateway, nil, nil) // Find the PathRule for "/infer" found := false @@ -2829,7 +2829,7 @@ func TestCreateFilters(t *testing.T) { t.Parallel() g := NewWithT(t) routeNsName := types.NamespacedName{Namespace: "test", Name: "route1"} - result := createHTTPFilters(test.filters, 0, routeNsName) + result := createHTTPFilters(test.filters, 0, routeNsName, nil) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) }) diff --git a/internal/controller/state/dataplane/convert.go b/internal/controller/state/dataplane/convert.go index e2db5c99d1..1271800d74 100644 --- a/internal/controller/state/dataplane/convert.go +++ b/internal/controller/state/dataplane/convert.go @@ -191,6 +191,34 @@ func convertSnippetsFilter(filter *graph.SnippetsFilter) SnippetsFilter { return result } +func convertAuthenticationFilter( + filter *graph.AuthenticationFilter, + referencedSecrets map[types.NamespacedName]*graph.Secret, +) *AuthenticationFilter { + result := &AuthenticationFilter{} + + // Do not convert invalid filters; graph validation will have emitted a condition. + if filter == nil || !filter.Valid { + return result + } + + if specBasic := filter.Source.Spec.Basic; specBasic != nil { + // It is safe to assume the referenced secret exists and is valid due to prior validation. + referencedSecret := referencedSecrets[types.NamespacedName{ + Namespace: filter.Source.Namespace, + Name: specBasic.SecretRef.Name, + }] + + result.Basic = &BasicAuth{ + SecretName: specBasic.SecretRef.Name, + Data: referencedSecret.Source.Data[ngfAPI.AuthKeyBasic], + Realm: specBasic.Realm, + } + } + + return result +} + func convertDNSResolverAddresses(addresses []ngfAPIv1alpha2.DNSResolverAddress) []string { if len(addresses) == 0 { return nil diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 1b0ccbde6a..dab52d7694 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -71,6 +71,10 @@ type SSLKeyPairID string // The ID is safe to use as a file name. type CertBundleID string +// AuthBasicUserFileID is a unique identifier for a basic auth user file. +// The ID is safe to use as a file name. +type AuthBasicUserFileID string + // CertBundle is a Certificate bundle. type CertBundle []byte @@ -149,22 +153,14 @@ type InvalidHTTPFilter struct{} // HTTPFilters hold the filters for a MatchRule. type HTTPFilters struct { - // InvalidFilter is a special filter that indicates whether the filters are invalid. If this is the case, - // the data plane must return 500 error, and all other filters are nil. - InvalidFilter *InvalidHTTPFilter - // RequestRedirect holds the HTTPRequestRedirectFilter. - RequestRedirect *HTTPRequestRedirectFilter - // RequestURLRewrite holds the HTTPURLRewriteFilter. - RequestURLRewrite *HTTPURLRewriteFilter - // RequestMirrors holds the HTTPRequestMirrorFilters. There could be more than one specified. - RequestMirrors []*HTTPRequestMirrorFilter - // RequestHeaderModifiers holds the HTTPHeaderFilter. - RequestHeaderModifiers *HTTPHeaderFilter - // ResponseHeaderModifiers holds the HTTPHeaderFilter. + InvalidFilter *InvalidHTTPFilter + RequestRedirect *HTTPRequestRedirectFilter + RequestURLRewrite *HTTPURLRewriteFilter + RequestHeaderModifiers *HTTPHeaderFilter ResponseHeaderModifiers *HTTPHeaderFilter - // SnippetsFilters holds all the SnippetsFilters for the MatchRule. - // Unlike the core and extended filters, there can be more than one SnippetsFilters defined on a routing rule. - SnippetsFilters []SnippetsFilter + AuthenticationFilter *AuthenticationFilter + RequestMirrors []*HTTPRequestMirrorFilter + SnippetsFilters []SnippetsFilter } // SnippetsFilter holds the location and server snippets in a SnippetsFilter. @@ -176,6 +172,27 @@ type SnippetsFilter struct { ServerSnippet *Snippet } +// Snippet is a snippet of configuration. +type Snippet struct { + // Name is the name of the snippet. + Name string + // Contents is the content of the snippet. + Contents string +} + +// AuthenticationFilter represents an authentication filter. +type AuthenticationFilter struct { + // This is where the data is extracted to. + Basic *BasicAuth +} + +// BasicAuth holds the basic authentication configuration. +type BasicAuth struct { + SecretName string + Realm string + Data []byte +} + // HTTPHeader represents an HTTP header. type HTTPHeader struct { // Name is the name of the header. @@ -413,14 +430,6 @@ type BaseStreamConfig struct { DNSResolver *DNSResolverConfig } -// Snippet is a snippet of configuration. -type Snippet struct { - // Name is the name of the snippet. - Name string - // Contents is the content of the snippet. - Contents string -} - // RewriteClientIPSettings defines configuration for rewriting the client IP to the original client's IP. type RewriteClientIPSettings struct { // Mode specifies the mode for rewriting the client IP. diff --git a/internal/controller/state/graph/authentication_filter.go b/internal/controller/state/graph/authentication_filter.go new file mode 100644 index 0000000000..0716f1021c --- /dev/null +++ b/internal/controller/state/graph/authentication_filter.go @@ -0,0 +1,137 @@ +package graph + +import ( + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + v1 "sigs.k8s.io/gateway-api/apis/v1" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" +) + +// AuthenticationFilter represents a ngfAPI.AuthenticationFilter. +type AuthenticationFilter struct { + // Source is the AuthenticationFilter. + Source *ngfAPI.AuthenticationFilter + // Conditions define the conditions to be reported in the status of the AuthenticationFilter. + Conditions []conditions.Condition + // Valid indicates whether the AuthenticationFilter is semantically and syntactically valid. + Valid bool + // Referenced indicates whether the AuthenticationFilter is referenced by a Route. + Referenced bool +} + +func getAuthenticationFilterResolverForNamespace( + authenticationFilters map[types.NamespacedName]*AuthenticationFilter, + namespace string, +) resolveExtRefFilter { + return func(ref v1.LocalObjectReference) *ExtensionRefFilter { + if len(authenticationFilters) == 0 { + return nil + } + + if ref.Group != ngfAPI.GroupName || ref.Kind != kinds.AuthenticationFilter { + return nil + } + + af := authenticationFilters[types.NamespacedName{Namespace: namespace, Name: string(ref.Name)}] + if af == nil { + return nil + } + + af.Referenced = true + + return &ExtensionRefFilter{AuthenticationFilter: af, Valid: af.Valid} + } +} + +func processAuthenticationFilters( + authenticationFilters map[types.NamespacedName]*ngfAPI.AuthenticationFilter, + secretResolver *secretResolver, +) map[types.NamespacedName]*AuthenticationFilter { + if len(authenticationFilters) == 0 { + return nil + } + + processed := make(map[types.NamespacedName]*AuthenticationFilter) + + for nsname, af := range authenticationFilters { + if cond := validateAuthenticationFilter(af, secretResolver); cond != nil { + processed[nsname] = &AuthenticationFilter{ + Source: af, + Conditions: []conditions.Condition{*cond}, + Valid: false, + } + + continue + } + processed[nsname] = &AuthenticationFilter{ + Source: af, + Valid: true, + } + } + + return processed +} + +func validateAuthenticationFilter( + af *ngfAPI.AuthenticationFilter, + secretResolver *secretResolver, +) *conditions.Condition { + var allErrs field.ErrorList + + //revive:disable-next-line:unnecessary-stmt future-proof switch form; additional auth types will be added + switch af.Spec.Type { + case ngfAPI.AuthTypeBasic: + + secretNsName := getAuthenticationFilterReferencedSecret(af) + if err := secretResolver.resolve(*secretNsName); err != nil { + path := field.NewPath("spec.basic.secretRef") + valErr := field.Invalid(path, secretNsName, err.Error()) + allErrs = append(allErrs, valErr) + } + + resolvedSecrets := secretResolver.getResolvedSecrets() + + for nsname, secret := range resolvedSecrets { + if nsname.Namespace == af.Namespace && nsname.Name == af.Spec.Basic.SecretRef.Name { + msg := "referenced secret is invalid or missing" + if secret == nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec.basic.secretRef"), af.Spec.Basic.SecretRef.Name, msg)) + break + } + if secret.Source == nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec.basic.secretRef"), af.Spec.Basic.SecretRef.Name, msg)) + break + } + if _, exists := secret.Source.Data[ngfAPI.AuthKeyBasic]; !exists { + msg = "referenced secret does not contain required 'auth' key" + allErrs = append(allErrs, field.Invalid(field.NewPath("spec.basic.secretRef"), af.Spec.Basic.SecretRef.Name, msg)) + } + break + } + } + + if af.Spec.Basic.Realm == "" { + allErrs = append(allErrs, field.Required(field.NewPath("spec.basic.realm"), "realm cannot be empty")) + } + default: + // Currently, only Basic auth is supported. + } + + if allErrs != nil { + cond := conditions.NewAuthenticationFilterInvalid(allErrs.ToAggregate().Error()) + return &cond + } + + return nil +} + +func getAuthenticationFilterReferencedSecret(af *ngfAPI.AuthenticationFilter) *types.NamespacedName { + secretRef := af.Spec.Basic.SecretRef + return &types.NamespacedName{ + Namespace: af.Namespace, + Name: secretRef.Name, + } +} diff --git a/internal/controller/state/graph/common_filter.go b/internal/controller/state/graph/common_filter.go index ac5a25efe6..7db5f7ea22 100644 --- a/internal/controller/state/graph/common_filter.go +++ b/internal/controller/state/graph/common_filter.go @@ -112,7 +112,7 @@ func processRouteRuleFilters( filters []Filter, path *field.Path, validator validation.HTTPFieldsValidator, - resolveExtRefFunc resolveExtRefFilter, + extRefFilterResolvers map[string]resolveExtRefFilter, ) (RouteRuleFilters, routeRuleErrors) { errors := routeRuleErrors{} valid := true @@ -128,7 +128,9 @@ func processRouteRuleFilters( } if f.FilterType == FilterExtensionRef && f.ExtensionRef != nil { - resolved := resolveExtRefFunc(*f.ExtensionRef) + // Will need to test how this behaves if we don't get a match. + extRefFilterResolver := extRefFilterResolvers[string(f.ExtensionRef.Kind)] + resolved := extRefFilterResolver(*f.ExtensionRef) if resolved == nil { err := field.NotFound(filterPath.Child("extensionRef"), f.ExtensionRef) diff --git a/internal/controller/state/graph/extension_ref_filter.go b/internal/controller/state/graph/extension_ref_filter.go index d9857b848a..8314a59682 100644 --- a/internal/controller/state/graph/extension_ref_filter.go +++ b/internal/controller/state/graph/extension_ref_filter.go @@ -10,10 +10,13 @@ import ( // ExtensionRefFilter are NGF-specific extensions to the "filter" behavior. type ExtensionRefFilter struct { - // SnippetsFilter contains the SnippetsFilter. Will be non-nil if the Ref.Kind is SnippetsFilter and the - // SnippetsFilter exists. + // SnippetsFilter contains the SnippetsFilter. + // Will be non-nil if the Ref.Kind is SnippetsFilter and the SnippetsFilter exists. // Once we support more filters, we can extend this struct with more filter kinds. SnippetsFilter *SnippetsFilter + // AuthenticationFilter contains the AuthenticationFilter. + // Will be non-nil if the Ref.Kind is AuthenticationFilter and the AuthenticationFilter exists. + AuthenticationFilter *AuthenticationFilter // Valid indicates whether the filter is valid. Valid bool } @@ -41,8 +44,14 @@ func validateExtensionRefFilter(ref *v1.LocalObjectReference, path *field.Path) switch ref.Kind { case kinds.SnippetsFilter: + case kinds.AuthenticationFilter: default: - allErrs = append(allErrs, field.NotSupported(extRefPath, ref.Kind, []string{kinds.SnippetsFilter})) + allErrs = append(allErrs, + field.NotSupported( + extRefPath, + ref.Kind, + []string{kinds.SnippetsFilter, kinds.AuthenticationFilter}), + ) } return allErrs diff --git a/internal/controller/state/graph/graph.go b/internal/controller/state/graph/graph.go index ffd86ac6e0..ae4dbe5d2c 100644 --- a/internal/controller/state/graph/graph.go +++ b/internal/controller/state/graph/graph.go @@ -25,22 +25,23 @@ import ( // ClusterState includes cluster resources necessary to build the Graph. type ClusterState struct { - GatewayClasses map[types.NamespacedName]*gatewayv1.GatewayClass - Gateways map[types.NamespacedName]*gatewayv1.Gateway - HTTPRoutes map[types.NamespacedName]*gatewayv1.HTTPRoute - TLSRoutes map[types.NamespacedName]*v1alpha2.TLSRoute - Services map[types.NamespacedName]*v1.Service - Namespaces map[types.NamespacedName]*v1.Namespace - ReferenceGrants map[types.NamespacedName]*v1beta1.ReferenceGrant - Secrets map[types.NamespacedName]*v1.Secret - CRDMetadata map[types.NamespacedName]*metav1.PartialObjectMetadata - BackendTLSPolicies map[types.NamespacedName]*gatewayv1.BackendTLSPolicy - ConfigMaps map[types.NamespacedName]*v1.ConfigMap - NginxProxies map[types.NamespacedName]*ngfAPIv1alpha2.NginxProxy - GRPCRoutes map[types.NamespacedName]*gatewayv1.GRPCRoute - NGFPolicies map[PolicyKey]policies.Policy - SnippetsFilters map[types.NamespacedName]*ngfAPIv1alpha1.SnippetsFilter - InferencePools map[types.NamespacedName]*inference.InferencePool + GatewayClasses map[types.NamespacedName]*gatewayv1.GatewayClass + Gateways map[types.NamespacedName]*gatewayv1.Gateway + HTTPRoutes map[types.NamespacedName]*gatewayv1.HTTPRoute + TLSRoutes map[types.NamespacedName]*v1alpha2.TLSRoute + Services map[types.NamespacedName]*v1.Service + Namespaces map[types.NamespacedName]*v1.Namespace + ReferenceGrants map[types.NamespacedName]*v1beta1.ReferenceGrant + Secrets map[types.NamespacedName]*v1.Secret + CRDMetadata map[types.NamespacedName]*metav1.PartialObjectMetadata + BackendTLSPolicies map[types.NamespacedName]*gatewayv1.BackendTLSPolicy + ConfigMaps map[types.NamespacedName]*v1.ConfigMap + NginxProxies map[types.NamespacedName]*ngfAPIv1alpha2.NginxProxy + GRPCRoutes map[types.NamespacedName]*gatewayv1.GRPCRoute + NGFPolicies map[PolicyKey]policies.Policy + SnippetsFilters map[types.NamespacedName]*ngfAPIv1alpha1.SnippetsFilter + AuthenticationFilters map[types.NamespacedName]*ngfAPIv1alpha1.AuthenticationFilter + InferencePools map[types.NamespacedName]*inference.InferencePool } // Graph is a Graph-like representation of Gateway API resources. @@ -79,6 +80,8 @@ type Graph struct { NGFPolicies map[PolicyKey]*Policy // SnippetsFilters holds all the SnippetsFilters. SnippetsFilters map[types.NamespacedName]*SnippetsFilter + // AuthenticationFilters holds all the AuthenticationFilters. + AuthenticationFilters map[types.NamespacedName]*AuthenticationFilter // PlusSecrets holds the secrets related to NGINX Plus licensing. PlusSecrets map[types.NamespacedName][]PlusSecretFile } @@ -254,12 +257,15 @@ func BuildGraph( processedSnippetsFilters := processSnippetsFilters(state.SnippetsFilters) + processedAuthenticationFilters := processAuthenticationFilters(state.AuthenticationFilters, secretResolver) + routes := buildRoutesForGateways( validators.HTTPFieldsValidator, state.HTTPRoutes, state.GRPCRoutes, gws, processedSnippetsFilters, + processedAuthenticationFilters, state.InferencePools, ) @@ -316,6 +322,7 @@ func BuildGraph( BackendTLSPolicies: processedBackendTLSPolicies, NGFPolicies: processedPolicies, SnippetsFilters: processedSnippetsFilters, + AuthenticationFilters: processedAuthenticationFilters, PlusSecrets: plusSecrets, } diff --git a/internal/controller/state/graph/graph_test.go b/internal/controller/state/graph/graph_test.go index ed0e39082f..d1b09bbeb1 100644 --- a/internal/controller/state/graph/graph_test.go +++ b/internal/controller/state/graph/graph_test.go @@ -165,6 +165,53 @@ func TestBuildGraph(t *testing.T) { }, } + // AuthenticationFilter to be used in tests + refAuthenticationFilterExtensionRef := &gatewayv1.LocalObjectReference{ + Group: ngfAPIv1alpha1.GroupName, + Kind: kinds.AuthenticationFilter, + Name: "ref-authentication-filter", + } + + unreferencedAuthenticationFilter := &ngfAPIv1alpha1.AuthenticationFilter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unref-authentication-filter", + Namespace: testNs, + }, + Spec: ngfAPIv1alpha1.AuthenticationFilterSpec{ + Basic: &ngfAPIv1alpha1.BasicAuth{ + SecretRef: ngfAPIv1alpha1.LocalObjectReference{ + Name: "basic-auth-secret", + }, + }, + }, + } + + referencedAuthenticationFilter := &ngfAPIv1alpha1.AuthenticationFilter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ref-authentication-filter", + Namespace: testNs, + }, + Spec: ngfAPIv1alpha1.AuthenticationFilterSpec{ + Basic: &ngfAPIv1alpha1.BasicAuth{ + SecretRef: ngfAPIv1alpha1.LocalObjectReference{ + Name: "basic-auth-secret", + }, + }, + }, + } + + processedUnrefAuthenticationFilter := &AuthenticationFilter{ + Source: unreferencedAuthenticationFilter, + Valid: true, + Referenced: false, + } + + processedRefAuthenticationFilter := &AuthenticationFilter{ + Source: referencedAuthenticationFilter, + Valid: true, + Referenced: true, + } + createValidRuleWithBackendRefs := func(matches []gatewayv1.HTTPRouteMatch) RouteRule { refs := []BackendRef{ { @@ -209,6 +256,15 @@ func TestBuildGraph(t *testing.T) { Valid: true, }, }, + { + RouteType: routeType, + FilterType: FilterExtensionRef, + ExtensionRef: refAuthenticationFilterExtensionRef, + ResolvedExtensionRef: &ExtensionRefFilter{ + AuthenticationFilter: processedRefAuthenticationFilter, + Valid: true, + }, + }, }, Valid: true, } @@ -342,6 +398,14 @@ func TestBuildGraph(t *testing.T) { ExtensionRef: refSnippetsFilterExtensionRef, }, ) + addFilterToPath( + hr1, + "/", + gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterExtensionRef, + ExtensionRef: refAuthenticationFilterExtensionRef, + }, + ) hr2 := createRoute("hr-2", "wrong-gateway", "listener-80-1") hr3 := createRoute("hr-3", "gateway-1", "listener-443-1") // https listener; should not conflict with hr1 @@ -380,6 +444,10 @@ func TestBuildGraph(t *testing.T) { Type: gatewayv1.GRPCRouteFilterExtensionRef, ExtensionRef: refSnippetsFilterExtensionRef, }, + { + Type: gatewayv1.GRPCRouteFilterExtensionRef, + ExtensionRef: refAuthenticationFilterExtensionRef, + }, }, }, }, @@ -855,6 +923,10 @@ func TestBuildGraph(t *testing.T) { client.ObjectKeyFromObject(unreferencedSnippetsFilter): unreferencedSnippetsFilter, client.ObjectKeyFromObject(referencedSnippetsFilter): referencedSnippetsFilter, }, + AuthenticationFilters: map[types.NamespacedName]*ngfAPIv1alpha1.AuthenticationFilter{ + client.ObjectKeyFromObject(unreferencedAuthenticationFilter): unreferencedAuthenticationFilter, + client.ObjectKeyFromObject(referencedAuthenticationFilter): referencedAuthenticationFilter, + }, } } @@ -1412,6 +1484,10 @@ func TestBuildGraph(t *testing.T) { client.ObjectKeyFromObject(unreferencedSnippetsFilter): processedUnrefSnippetsFilter, client.ObjectKeyFromObject(referencedSnippetsFilter): processedRefSnippetsFilter, }, + AuthenticationFilters: map[types.NamespacedName]*AuthenticationFilter{ + client.ObjectKeyFromObject(unreferencedAuthenticationFilter): processedUnrefAuthenticationFilter, + client.ObjectKeyFromObject(referencedAuthenticationFilter): processedRefAuthenticationFilter, + }, PlusSecrets: map[types.NamespacedName][]PlusSecretFile{ client.ObjectKeyFromObject(plusSecret): { { diff --git a/internal/controller/state/graph/grpcroute.go b/internal/controller/state/graph/grpcroute.go index 5d6e7489e3..576e5dfc29 100644 --- a/internal/controller/state/graph/grpcroute.go +++ b/internal/controller/state/graph/grpcroute.go @@ -14,6 +14,7 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/mirror" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/validation" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" ) func buildGRPCRoute( @@ -21,6 +22,7 @@ func buildGRPCRoute( ghr *v1.GRPCRoute, gws map[types.NamespacedName]*Gateway, snippetsFilters map[types.NamespacedName]*SnippetsFilter, + authenticationFilters map[types.NamespacedName]*AuthenticationFilter, ) *L7Route { r := &L7Route{ Source: ghr, @@ -52,10 +54,22 @@ func buildGRPCRoute( r.Spec.Hostnames = ghr.Spec.Hostnames r.Attachable = true + extRefFilterResolvers := make(map[string]resolveExtRefFilter) + + extRefFilterResolvers[kinds.SnippetsFilter] = getSnippetsFilterResolverForNamespace( + snippetsFilters, + r.Source.GetNamespace(), + ) + + extRefFilterResolvers[kinds.AuthenticationFilter] = getAuthenticationFilterResolverForNamespace( + authenticationFilters, + r.Source.GetNamespace(), + ) + rules, valid, conds := processGRPCRouteRules( ghr.Spec.Rules, validator, - getSnippetsFilterResolverForNamespace(snippetsFilters, r.Source.GetNamespace()), + extRefFilterResolvers, ) r.Spec.Rules = rules @@ -107,6 +121,7 @@ func buildGRPCMirrorRoutes( tmpMirrorRoute, gateways, snippetsFilters, + nil, ) if mirrorRoute != nil { @@ -159,7 +174,7 @@ func processGRPCRouteRule( specRule v1.GRPCRouteRule, rulePath *field.Path, validator validation.HTTPFieldsValidator, - resolveExtRefFunc resolveExtRefFilter, + extRefFilterResolvers map[string]resolveExtRefFilter, ) (RouteRule, routeRuleErrors) { var errors routeRuleErrors @@ -184,7 +199,7 @@ func processGRPCRouteRule( convertGRPCRouteFilters(specRule.Filters), rulePath.Child("filters"), validator, - resolveExtRefFunc, + extRefFilterResolvers, ) errors = errors.append(filterErrors) @@ -234,7 +249,7 @@ func processGRPCRouteRule( func processGRPCRouteRules( specRules []v1.GRPCRouteRule, validator validation.HTTPFieldsValidator, - resolveExtRefFunc resolveExtRefFilter, + extRefFilterResolvers map[string]resolveExtRefFilter, ) (rules []RouteRule, valid bool, conds []conditions.Condition) { rules = make([]RouteRule, len(specRules)) @@ -250,7 +265,7 @@ func processGRPCRouteRules( rule, rulePath, validator, - resolveExtRefFunc, + extRefFilterResolvers, ) if rr.ValidMatches && rr.Filters.Valid { diff --git a/internal/controller/state/graph/grpcroute_test.go b/internal/controller/state/graph/grpcroute_test.go index 032b6a891f..06bdf88f55 100644 --- a/internal/controller/state/graph/grpcroute_test.go +++ b/internal/controller/state/graph/grpcroute_test.go @@ -233,6 +233,7 @@ func TestBuildGRPCRoutes(t *testing.T) { test.gateways, snippetsFilters, nil, + nil, ) g.Expect(helpers.Diff(test.expected, routes)).To(BeEmpty()) }) @@ -1199,7 +1200,7 @@ func TestBuildGRPCRoute(t *testing.T) { snippetsFilters := map[types.NamespacedName]*SnippetsFilter{ {Namespace: "test", Name: "sf"}: {Valid: true}, } - route := buildGRPCRoute(test.validator, test.gr, gws, snippetsFilters) + route := buildGRPCRoute(test.validator, test.gr, gws, snippetsFilters, nil) g.Expect(helpers.Diff(test.expected, route)).To(BeEmpty()) }) } @@ -1352,7 +1353,7 @@ func TestBuildGRPCRouteWithMirrorRoutes(t *testing.T) { g := NewWithT(t) routes := map[RouteKey]*L7Route{} - l7route := buildGRPCRoute(validator, gr, gateways, snippetsFilters) + l7route := buildGRPCRoute(validator, gr, gateways, snippetsFilters, nil) g.Expect(l7route).NotTo(BeNil()) buildGRPCMirrorRoutes(routes, l7route, gr, gateways, snippetsFilters) diff --git a/internal/controller/state/graph/httproute.go b/internal/controller/state/graph/httproute.go index a8ee8ab158..50cad99636 100644 --- a/internal/controller/state/graph/httproute.go +++ b/internal/controller/state/graph/httproute.go @@ -30,6 +30,7 @@ func buildHTTPRoute( ghr *v1.HTTPRoute, gws map[types.NamespacedName]*Gateway, snippetsFilters map[types.NamespacedName]*SnippetsFilter, + authenticationFilters map[types.NamespacedName]*AuthenticationFilter, inferencePools map[types.NamespacedName]*inference.InferencePool, ) *L7Route { r := &L7Route{ @@ -63,10 +64,22 @@ func buildHTTPRoute( r.Spec.Hostnames = ghr.Spec.Hostnames r.Attachable = true + extRefFilterResolvers := make(map[string]resolveExtRefFilter) + + extRefFilterResolvers[kinds.SnippetsFilter] = getSnippetsFilterResolverForNamespace( + snippetsFilters, + r.Source.GetNamespace(), + ) + + extRefFilterResolvers[kinds.AuthenticationFilter] = getAuthenticationFilterResolverForNamespace( + authenticationFilters, + r.Source.GetNamespace(), + ) + rules, valid, conds := processHTTPRouteRules( ghr.Spec.Rules, validator, - getSnippetsFilterResolverForNamespace(snippetsFilters, r.Source.GetNamespace()), + extRefFilterResolvers, inferencePools, r.Source.GetNamespace(), ) @@ -121,6 +134,7 @@ func buildHTTPMirrorRoutes( gateways, snippetsFilters, nil, + nil, ) if mirrorRoute != nil { @@ -173,7 +187,7 @@ func processHTTPRouteRule( specRule v1.HTTPRouteRule, rulePath *field.Path, validator validation.HTTPFieldsValidator, - resolveExtRefFunc resolveExtRefFilter, + extRefFilterResolvers map[string]resolveExtRefFilter, inferencePools map[types.NamespacedName]*inference.InferencePool, routeNamespace string, ) (RouteRule, routeRuleErrors) { @@ -200,7 +214,7 @@ func processHTTPRouteRule( convertHTTPRouteFilters(specRule.Filters), rulePath.Child("filters"), validator, - resolveExtRefFunc, + extRefFilterResolvers, ) errors = errors.append(filterErrors) @@ -283,7 +297,7 @@ func processHTTPRouteRule( func processHTTPRouteRules( specRules []v1.HTTPRouteRule, validator validation.HTTPFieldsValidator, - resolveExtRefFunc resolveExtRefFilter, + extRefFilterResolvers map[string]resolveExtRefFilter, inferencePools map[types.NamespacedName]*inference.InferencePool, routeNamespace string, ) (rules []RouteRule, valid bool, conds []conditions.Condition) { @@ -301,7 +315,7 @@ func processHTTPRouteRules( rule, rulePath, validator, - resolveExtRefFunc, + extRefFilterResolvers, inferencePools, routeNamespace, ) diff --git a/internal/controller/state/graph/httproute_test.go b/internal/controller/state/graph/httproute_test.go index 01bd75ed9d..3ba8a0468f 100644 --- a/internal/controller/state/graph/httproute_test.go +++ b/internal/controller/state/graph/httproute_test.go @@ -277,6 +277,7 @@ func TestBuildHTTPRoutes(t *testing.T) { test.gateways, snippetsFilters, nil, + nil, ) g.Expect(helpers.Diff(test.expected, routes)).To(BeEmpty()) }) @@ -1109,7 +1110,7 @@ func TestBuildHTTPRoute(t *testing.T) { {Namespace: "test", Name: "ipool"}: {}, } - route := buildHTTPRoute(test.validator, test.hr, gws, snippetsFilters, inferencePools) + route := buildHTTPRoute(test.validator, test.hr, gws, snippetsFilters, nil, inferencePools) g.Expect(helpers.Diff(test.expected, route)).To(BeEmpty()) }) } @@ -1241,7 +1242,7 @@ func TestBuildHTTPRouteWithMirrorRoutes(t *testing.T) { g := NewWithT(t) routes := map[RouteKey]*L7Route{} - l7route := buildHTTPRoute(validator, hr, gateways, snippetsFilters, nil) + l7route := buildHTTPRoute(validator, hr, gateways, snippetsFilters, nil, nil) g.Expect(l7route).NotTo(BeNil()) buildHTTPMirrorRoutes(routes, l7route, hr, gateways, snippetsFilters) diff --git a/internal/controller/state/graph/route_common.go b/internal/controller/state/graph/route_common.go index c6bf225f43..cb3708e938 100644 --- a/internal/controller/state/graph/route_common.go +++ b/internal/controller/state/graph/route_common.go @@ -248,13 +248,14 @@ func buildL4RoutesForGateways( return routes } -// buildGRPCRoutesForGateways builds routes from HTTP/GRPCRoutes that reference any of the specified Gateways. +// buildRoutesForGateways builds routes from HTTP/GRPCRoutes that reference any of the specified Gateways. func buildRoutesForGateways( validator validation.HTTPFieldsValidator, httpRoutes map[types.NamespacedName]*v1.HTTPRoute, grpcRoutes map[types.NamespacedName]*v1.GRPCRoute, gateways map[types.NamespacedName]*Gateway, snippetsFilters map[types.NamespacedName]*SnippetsFilter, + authenticationFilters map[types.NamespacedName]*AuthenticationFilter, inferencePools map[types.NamespacedName]*inference.InferencePool, ) map[RouteKey]*L7Route { if len(gateways) == 0 { @@ -264,7 +265,7 @@ func buildRoutesForGateways( routes := make(map[RouteKey]*L7Route) for _, route := range httpRoutes { - r := buildHTTPRoute(validator, route, gateways, snippetsFilters, inferencePools) + r := buildHTTPRoute(validator, route, gateways, snippetsFilters, authenticationFilters, inferencePools) if r == nil { continue } @@ -276,7 +277,7 @@ func buildRoutesForGateways( } for _, route := range grpcRoutes { - r := buildGRPCRoute(validator, route, gateways, snippetsFilters) + r := buildGRPCRoute(validator, route, gateways, snippetsFilters, authenticationFilters) if r == nil { continue } diff --git a/internal/controller/state/graph/secret.go b/internal/controller/state/graph/secret.go index 7515c514f7..5ca4962e36 100644 --- a/internal/controller/state/graph/secret.go +++ b/internal/controller/state/graph/secret.go @@ -6,6 +6,8 @@ import ( apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" ) // Secret represents a Secret resource. @@ -51,10 +53,7 @@ func (r *secretResolver) resolve(nsname types.NamespacedName) error { case !exist: validationErr = errors.New("secret does not exist") - case secret.Type != apiv1.SecretTypeTLS: - validationErr = fmt.Errorf("secret type must be %q not %q", apiv1.SecretTypeTLS, secret.Type) - - default: + case secret.Type == apiv1.SecretTypeTLS: // A TLS Secret is guaranteed to have these data fields. cert := &Certificate{ TLSCert: secret.Data[apiv1.TLSCertKey], @@ -62,16 +61,27 @@ func (r *secretResolver) resolve(nsname types.NamespacedName) error { } validationErr = validateTLS(cert.TLSCert, cert.TLSPrivateKey) - // Not always guaranteed to have a ca certificate in the secret. - // Cert-Manager puts this at ca.crt and thus this is statically placed like so. - // To follow the convention setup by kubernetes for a service account root ca - // for optional root certificate authority + // Optional CA certificate. if _, exists := secret.Data[CAKey]; exists { cert.CACert = secret.Data[CAKey] validationErr = validateCA(cert.CACert) } certBundle = NewCertificateBundle(nsname, "Secret", cert) + + // TODO: Define our own Secret types for other auth methods (e.g., OAuth) as needed. + case secret.Type == apiv1.SecretTypeOpaque: + // Allow Opaque secrets specifically when they contain the "auth" key. + //nolint:revive // may need to consider our own secret types. + if _, hasAuth := secret.Data[ngfAPI.AuthKeyBasic]; hasAuth { + // Accept: no TLS/CA validation, no CertificateBundle. + // Leave validationErr and certBundle as nil. + } else { + validationErr = fmt.Errorf("secret type must be %q not %q", apiv1.SecretTypeTLS, secret.Type) + } + + default: + validationErr = fmt.Errorf("secret type must be %q not %q", apiv1.SecretTypeTLS, secret.Type) } r.resolvedSecrets[nsname] = &secretEntry{ diff --git a/internal/controller/status/prepare_requests.go b/internal/controller/status/prepare_requests.go index ff51805d1d..bdc089c47d 100644 --- a/internal/controller/status/prepare_requests.go +++ b/internal/controller/status/prepare_requests.go @@ -486,6 +486,39 @@ func PrepareSnippetsFilterRequests( return reqs } +func PrepareAuthenticationFilterRequests( + authenticationFilters map[types.NamespacedName]*graph.AuthenticationFilter, + transitionTime metav1.Time, + gatewayCtlrName string, +) []UpdateRequest { + reqs := make([]UpdateRequest, 0, len(authenticationFilters)) + + for nsname, authenticationFilter := range authenticationFilters { + allConds := make([]conditions.Condition, 0, len(authenticationFilter.Conditions)+1) + allConds = append(allConds, conditions.NewAuthenticationFilterAccepted()) + allConds = append(allConds, authenticationFilter.Conditions...) + + conds := conditions.DeduplicateConditions(allConds) + apiConds := conditions.ConvertConditions(conds, authenticationFilter.Source.GetGeneration(), transitionTime) + status := ngfAPI.AuthenticationFilterStatus{ + Controllers: []ngfAPI.ControllerStatus{ + { + Conditions: apiConds, + ControllerName: v1alpha2.GatewayController(gatewayCtlrName), + }, + }, + } + + reqs = append(reqs, UpdateRequest{ + NsName: nsname, + ResourceType: authenticationFilter.Source, + Setter: newAuthenticationFilterStatusSetter(status, gatewayCtlrName), + }) + } + + return reqs +} + // ControlPlaneUpdateResult describes the result of a control plane update. type ControlPlaneUpdateResult struct { // Error is the error that occurred during the update. diff --git a/internal/controller/status/status_setters.go b/internal/controller/status/status_setters.go index a7147bfa7d..5aebc8c26e 100644 --- a/internal/controller/status/status_setters.go +++ b/internal/controller/status/status_setters.go @@ -430,6 +430,67 @@ func snippetsStatusEqual(status1, status2 ngfAPI.ControllerStatus) bool { return ConditionsEqual(status1.Conditions, status2.Conditions) } +func newAuthenticationFilterStatusSetter(status ngfAPI.AuthenticationFilterStatus, gatewayCtlrName string) Setter { + return func(obj client.Object) (wasSet bool) { + af := helpers.MustCastObject[*ngfAPI.AuthenticationFilter](obj) + + maxControllerStatus := 1 + len(af.Status.Controllers) + controllerStatuses := make([]ngfAPI.ControllerStatus, 0, maxControllerStatus) + + for _, status := range af.Status.Controllers { + if string(status.ControllerName) != gatewayCtlrName { + controllerStatuses = append(controllerStatuses, status) + } + } + + controllerStatuses = append(controllerStatuses, status.Controllers...) + status.Controllers = controllerStatuses + + if authenticationFilterStatusEqual(gatewayCtlrName, status.Controllers, af.Status.Controllers) { + return false + } + + af.Status = status + return true + } +} + +func authenticationFilterStatusEqual(gatewayCtlrName string, currStatus, prevStatus []ngfAPI.ControllerStatus) bool { + for _, prev := range prevStatus { + if prev.ControllerName != gatewayv1.GatewayController(gatewayCtlrName) { + continue + } + + exists := slices.ContainsFunc(currStatus, func(currStatus ngfAPI.ControllerStatus) bool { + return authenticationStatusEqual(currStatus, prev) + }) + + if !exists { + return false + } + } + + for _, curr := range currStatus { + exists := slices.ContainsFunc(prevStatus, func(prevStatus ngfAPI.ControllerStatus) bool { + return authenticationStatusEqual(curr, prevStatus) + }) + + if !exists { + return false + } + } + + return true +} + +func authenticationStatusEqual(status1, status2 ngfAPI.ControllerStatus) bool { + if status1.ControllerName != status2.ControllerName { + return false + } + + return ConditionsEqual(status1.Conditions, status2.Conditions) +} + func newInferencePoolStatusSetter(status inference.InferencePoolStatus) Setter { return func(obj client.Object) (wasSet bool) { ip := helpers.MustCastObject[*inference.InferencePool](obj) diff --git a/internal/framework/kinds/kinds.go b/internal/framework/kinds/kinds.go index b59b06df96..11a14db7a8 100644 --- a/internal/framework/kinds/kinds.go +++ b/internal/framework/kinds/kinds.go @@ -23,6 +23,8 @@ const ( TLSRoute = "TLSRoute" // BackendTLSPolicy is the BackendTLSPolicy kind. BackendTLSPolicy = "BackendTLSPolicy" + // AuthenticationFilter is the AuthenticationFilter kind. + AuthenticationFilter = "AuthenticationFilter" ) // Gateway API Inference Extension kinds. From 0a48fff5a6cf0c1a9f42769108c95f4e28f31535 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 10 Dec 2025 15:51:22 +0000 Subject: [PATCH 02/16] make generate-all --- deploy/azure/deploy.yaml | 2 ++ deploy/default/deploy.yaml | 2 ++ deploy/experimental-nginx-plus/deploy.yaml | 2 ++ deploy/experimental/deploy.yaml | 2 ++ deploy/inference-nginx-plus/deploy.yaml | 2 ++ deploy/inference/deploy.yaml | 2 ++ deploy/nginx-plus/deploy.yaml | 2 ++ deploy/nodeport/deploy.yaml | 2 ++ deploy/openshift/deploy.yaml | 2 ++ deploy/snippets-filters-nginx-plus/deploy.yaml | 2 ++ deploy/snippets-filters/deploy.yaml | 2 ++ 11 files changed, 22 insertions(+) diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index ed1699b656..0ad542ec22 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status verbs: - update - apiGroups: diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index 2651f0bc3c..7a9909e0fd 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status verbs: - update - apiGroups: diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index 4ea4257289..4e6d93d862 100644 --- a/deploy/experimental-nginx-plus/deploy.yaml +++ b/deploy/experimental-nginx-plus/deploy.yaml @@ -171,6 +171,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters verbs: - list - watch @@ -181,6 +182,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status verbs: - update - apiGroups: diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index 6b9ff2594b..ef2dd56d73 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -171,6 +171,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters verbs: - list - watch @@ -181,6 +182,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status verbs: - update - apiGroups: diff --git a/deploy/inference-nginx-plus/deploy.yaml b/deploy/inference-nginx-plus/deploy.yaml index f2be0e2902..e9d5ed61df 100644 --- a/deploy/inference-nginx-plus/deploy.yaml +++ b/deploy/inference-nginx-plus/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status verbs: - update - apiGroups: diff --git a/deploy/inference/deploy.yaml b/deploy/inference/deploy.yaml index 129573c45c..79f916a865 100644 --- a/deploy/inference/deploy.yaml +++ b/deploy/inference/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status verbs: - update - apiGroups: diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml index 6610a43166..39d1fc697b 100644 --- a/deploy/nginx-plus/deploy.yaml +++ b/deploy/nginx-plus/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status verbs: - update - apiGroups: diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index f2538143ed..ad7acb8f7b 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status verbs: - update - apiGroups: diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml index 09b25596fc..5b8ea4d26e 100644 --- a/deploy/openshift/deploy.yaml +++ b/deploy/openshift/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status verbs: - update - apiGroups: diff --git a/deploy/snippets-filters-nginx-plus/deploy.yaml b/deploy/snippets-filters-nginx-plus/deploy.yaml index b62bbb40df..cf76e14cec 100644 --- a/deploy/snippets-filters-nginx-plus/deploy.yaml +++ b/deploy/snippets-filters-nginx-plus/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters - snippetsfilters verbs: - list @@ -180,6 +181,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status - snippetsfilters/status verbs: - update diff --git a/deploy/snippets-filters/deploy.yaml b/deploy/snippets-filters/deploy.yaml index 0ef73c141b..adf7b7ffcd 100644 --- a/deploy/snippets-filters/deploy.yaml +++ b/deploy/snippets-filters/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - authenticationfilters - snippetsfilters verbs: - list @@ -180,6 +181,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - authenticationfilters/status - snippetsfilters/status verbs: - update From 0e5f51eb15a8b056804bde7a2ec126f1c2ce17c2 Mon Sep 17 00:00:00 2001 From: Shaun Date: Wed, 10 Dec 2025 15:56:11 +0000 Subject: [PATCH 03/16] Set dest from `location.AuthBasic.Data.FileName` Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/controller/nginx/config/servers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 4db7343a8c..db8135abeb 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -761,7 +761,7 @@ func createExecuteResultsForAuthBasicUserFile(servers []http.Server) []executeRe for _, server := range servers { for _, location := range server.Locations { if location.AuthBasic != nil { - userFilePathAndData := fmt.Sprintf("%s/%s", location.AuthBasic.Data.FileName, ngfAPI.AuthKeyBasic) + userFilePathAndData := location.AuthBasic.Data.FileName result := executeResult{ dest: fmt.Sprintf(basicAuthUserFile, userFilePathAndData), data: location.AuthBasic.Data.FileData, From 5a4903cbec8333825f1f3f25bbe5f90b140dc9ce Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 10 Dec 2025 16:45:11 +0000 Subject: [PATCH 04/16] Update dest value --- internal/controller/nginx/config/servers.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index db8135abeb..4910bca923 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -761,9 +761,8 @@ func createExecuteResultsForAuthBasicUserFile(servers []http.Server) []executeRe for _, server := range servers { for _, location := range server.Locations { if location.AuthBasic != nil { - userFilePathAndData := location.AuthBasic.Data.FileName result := executeResult{ - dest: fmt.Sprintf(basicAuthUserFile, userFilePathAndData), + dest: location.AuthBasic.Data.FileName, data: location.AuthBasic.Data.FileData, } results = append(results, result) From e35da9937efbbb69f8ffd05f4812400ef84e1027 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 11 Dec 2025 09:26:10 +0000 Subject: [PATCH 05/16] Remove duplicate call to secret resolved --- .../state/graph/authentication_filter.go | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/internal/controller/state/graph/authentication_filter.go b/internal/controller/state/graph/authentication_filter.go index 0716f1021c..f28a81cad5 100644 --- a/internal/controller/state/graph/authentication_filter.go +++ b/internal/controller/state/graph/authentication_filter.go @@ -1,6 +1,8 @@ package graph import ( + "fmt" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" v1 "sigs.k8s.io/gateway-api/apis/v1" @@ -48,7 +50,7 @@ func getAuthenticationFilterResolverForNamespace( func processAuthenticationFilters( authenticationFilters map[types.NamespacedName]*ngfAPI.AuthenticationFilter, - secretResolver *secretResolver, + resolvedSecrets map[types.NamespacedName]*Secret, ) map[types.NamespacedName]*AuthenticationFilter { if len(authenticationFilters) == 0 { return nil @@ -57,7 +59,7 @@ func processAuthenticationFilters( processed := make(map[types.NamespacedName]*AuthenticationFilter) for nsname, af := range authenticationFilters { - if cond := validateAuthenticationFilter(af, secretResolver); cond != nil { + if cond := validateAuthenticationFilter(af, resolvedSecrets); cond != nil { processed[nsname] = &AuthenticationFilter{ Source: af, Conditions: []conditions.Condition{*cond}, @@ -77,7 +79,7 @@ func processAuthenticationFilters( func validateAuthenticationFilter( af *ngfAPI.AuthenticationFilter, - secretResolver *secretResolver, + resolvedSecrets map[types.NamespacedName]*Secret, ) *conditions.Condition { var allErrs field.ErrorList @@ -85,14 +87,10 @@ func validateAuthenticationFilter( switch af.Spec.Type { case ngfAPI.AuthTypeBasic: - secretNsName := getAuthenticationFilterReferencedSecret(af) - if err := secretResolver.resolve(*secretNsName); err != nil { - path := field.NewPath("spec.basic.secretRef") - valErr := field.Invalid(path, secretNsName, err.Error()) - allErrs = append(allErrs, valErr) - } - - resolvedSecrets := secretResolver.getResolvedSecrets() + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec.basic.secretRef"), + af.Spec.Basic.SecretRef.Name, + validateReferencedSecret(af, resolvedSecrets).Error())) for nsname, secret := range resolvedSecrets { if nsname.Namespace == af.Namespace && nsname.Name == af.Spec.Basic.SecretRef.Name { @@ -128,10 +126,16 @@ func validateAuthenticationFilter( return nil } -func getAuthenticationFilterReferencedSecret(af *ngfAPI.AuthenticationFilter) *types.NamespacedName { - secretRef := af.Spec.Basic.SecretRef - return &types.NamespacedName{ +func validateReferencedSecret( + af *ngfAPI.AuthenticationFilter, + resolvedSecrets map[types.NamespacedName]*Secret, +) error { + secretNsName := &types.NamespacedName{ Namespace: af.Namespace, - Name: secretRef.Name, + Name: af.Spec.Basic.SecretRef.Name, + } + if _, exists := resolvedSecrets[*secretNsName]; !exists { + return fmt.Errorf("referenced secret is invalid or missing") } + return nil } From 1b7dd251d6981268f5cc70e1065c30209e202852 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 11 Dec 2025 10:20:04 +0000 Subject: [PATCH 06/16] Remove duplicate call to secret resolver --- internal/controller/state/graph/graph.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/controller/state/graph/graph.go b/internal/controller/state/graph/graph.go index ae4dbe5d2c..252c1c030a 100644 --- a/internal/controller/state/graph/graph.go +++ b/internal/controller/state/graph/graph.go @@ -257,7 +257,9 @@ func BuildGraph( processedSnippetsFilters := processSnippetsFilters(state.SnippetsFilters) - processedAuthenticationFilters := processAuthenticationFilters(state.AuthenticationFilters, secretResolver) + referencedSecrets := secretResolver.getResolvedSecrets() + + processedAuthenticationFilters := processAuthenticationFilters(state.AuthenticationFilters, referencedSecrets) routes := buildRoutesForGateways( validators.HTTPFieldsValidator, @@ -313,7 +315,7 @@ func BuildGraph( Routes: routes, L4Routes: l4routes, IgnoredGatewayClasses: processedGwClasses.Ignored, - ReferencedSecrets: secretResolver.getResolvedSecrets(), + ReferencedSecrets: referencedSecrets, ReferencedNamespaces: referencedNamespaces, ReferencedServices: referencedServices, ReferencedInferencePools: referencedInferencePools, From edb8348cebbfefb6ed7d5d6e212efde9a9938608 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 11 Dec 2025 10:22:27 +0000 Subject: [PATCH 07/16] Re-add removed comment --- internal/controller/state/graph/secret.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/controller/state/graph/secret.go b/internal/controller/state/graph/secret.go index 5ca4962e36..ca44e2dd9f 100644 --- a/internal/controller/state/graph/secret.go +++ b/internal/controller/state/graph/secret.go @@ -61,7 +61,10 @@ func (r *secretResolver) resolve(nsname types.NamespacedName) error { } validationErr = validateTLS(cert.TLSCert, cert.TLSPrivateKey) - // Optional CA certificate. + // Not always guaranteed to have a ca certificate in the secret. + // Cert-Manager puts this at ca.crt and thus this is statically placed like so. + // To follow the convention setup by kubernetes for a service account root ca + // for optional root certificate authority if _, exists := secret.Data[CAKey]; exists { cert.CACert = secret.Data[CAKey] validationErr = validateCA(cert.CACert) From 816382dc518053b7a38ff6ca74dc584efa571ec9 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 11 Dec 2025 11:45:55 +0000 Subject: [PATCH 08/16] Process custom secret type --- apis/v1alpha1/authenticationfilter_types.go | 4 --- internal/controller/nginx/config/generator.go | 4 +-- internal/controller/nginx/config/servers.go | 4 +-- .../controller/state/dataplane/convert.go | 2 +- .../state/graph/authentication_filter.go | 2 +- internal/controller/state/graph/secret.go | 30 +++++++++++-------- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/apis/v1alpha1/authenticationfilter_types.go b/apis/v1alpha1/authenticationfilter_types.go index 682d8e287a..b441e0f721 100644 --- a/apis/v1alpha1/authenticationfilter_types.go +++ b/apis/v1alpha1/authenticationfilter_types.go @@ -57,10 +57,6 @@ const ( AuthTypeBasic AuthType = "Basic" ) -const ( - AuthKeyBasic = "auth" // Key in the Secret data for Basic Auth credentials. -) - // BasicAuth configures HTTP Basic Authentication. type BasicAuth struct { // SecretRef allows referencing a Secret in the same namespace. diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index 48c10f92e9..f422d8c4a2 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -9,7 +9,6 @@ import ( pb "github.com/nginx/agent/v3/api/grpc/mpi/v1" filesHelper "github.com/nginx/agent/v3/pkg/files" - ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" ngfConfig "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/config" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" @@ -18,6 +17,7 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/observability" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/upstreamsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/graph" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/file" ) @@ -141,7 +141,7 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []agent.File { for _, matchRule := range rule.MatchRules { if matchRule.Filters.AuthenticationFilter != nil { if matchRule.Filters.AuthenticationFilter.Basic != nil { - id := fmt.Sprintf("%s/%s", matchRule.Filters.AuthenticationFilter.Basic.SecretName, ngfAPI.AuthKeyBasic) + id := fmt.Sprintf("%s/%s", matchRule.Filters.AuthenticationFilter.Basic.SecretName, graph.AuthKeyBasic) data := matchRule.Filters.AuthenticationFilter.Basic.Data files = append(files, generateAuthBasicUserFile(id, data)) } diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 4910bca923..d8f4f3d931 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -11,11 +11,11 @@ import ( ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" - ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/graph" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" ) @@ -739,7 +739,7 @@ func updateLocationAuthenticationFilter( if authenticationFilter != nil { logger.Info("Applying authentication filter to location", "locationPath", location.Path) if authenticationFilter.Basic != nil { - userFilePathAndData := fmt.Sprintf("%s/%s", authenticationFilter.Basic.SecretName, ngfAPI.AuthKeyBasic) + userFilePathAndData := fmt.Sprintf("%s/%s", authenticationFilter.Basic.SecretName, graph.AuthKeyBasic) location.AuthBasic = &http.AuthBasic{ Realm: authenticationFilter.Basic.Realm, Data: http.AuthBasicData{ diff --git a/internal/controller/state/dataplane/convert.go b/internal/controller/state/dataplane/convert.go index 1271800d74..02a5b7d875 100644 --- a/internal/controller/state/dataplane/convert.go +++ b/internal/controller/state/dataplane/convert.go @@ -211,7 +211,7 @@ func convertAuthenticationFilter( result.Basic = &BasicAuth{ SecretName: specBasic.SecretRef.Name, - Data: referencedSecret.Source.Data[ngfAPI.AuthKeyBasic], + Data: referencedSecret.Source.Data[graph.AuthKeyBasic], Realm: specBasic.Realm, } } diff --git a/internal/controller/state/graph/authentication_filter.go b/internal/controller/state/graph/authentication_filter.go index f28a81cad5..09ce673799 100644 --- a/internal/controller/state/graph/authentication_filter.go +++ b/internal/controller/state/graph/authentication_filter.go @@ -103,7 +103,7 @@ func validateAuthenticationFilter( allErrs = append(allErrs, field.Invalid(field.NewPath("spec.basic.secretRef"), af.Spec.Basic.SecretRef.Name, msg)) break } - if _, exists := secret.Source.Data[ngfAPI.AuthKeyBasic]; !exists { + if _, exists := secret.Source.Data[AuthKeyBasic]; !exists { msg = "referenced secret does not contain required 'auth' key" allErrs = append(allErrs, field.Invalid(field.NewPath("spec.basic.secretRef"), af.Spec.Basic.SecretRef.Name, msg)) } diff --git a/internal/controller/state/graph/secret.go b/internal/controller/state/graph/secret.go index ca44e2dd9f..a1b5874ed1 100644 --- a/internal/controller/state/graph/secret.go +++ b/internal/controller/state/graph/secret.go @@ -6,8 +6,6 @@ import ( apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - - ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" ) // Secret represents a Secret resource. @@ -25,6 +23,18 @@ type secretEntry struct { err error } +type SecretType string + +const ( + // SecretTypeHtpasswd represents a Secret containing an htpasswd file for Basic Auth. + SecretTypeHtpasswd SecretType = "nginx.org/htpasswd" +) + +const ( + // Key in the Secret data for Basic Auth credentials. + AuthKeyBasic = "auth" +) + // secretResolver wraps the cluster Secrets so that they can be resolved (includes validation). All resolved // Secrets are saved to be used later. type secretResolver struct { @@ -72,19 +82,13 @@ func (r *secretResolver) resolve(nsname types.NamespacedName) error { certBundle = NewCertificateBundle(nsname, "Secret", cert) - // TODO: Define our own Secret types for other auth methods (e.g., OAuth) as needed. - case secret.Type == apiv1.SecretTypeOpaque: - // Allow Opaque secrets specifically when they contain the "auth" key. - //nolint:revive // may need to consider our own secret types. - if _, hasAuth := secret.Data[ngfAPI.AuthKeyBasic]; hasAuth { - // Accept: no TLS/CA validation, no CertificateBundle. - // Leave validationErr and certBundle as nil. - } else { - validationErr = fmt.Errorf("secret type must be %q not %q", apiv1.SecretTypeTLS, secret.Type) + case secret.Type == apiv1.SecretType(SecretTypeHtpasswd): + // Validate Htpasswd secret + if _, exists := secret.Data[AuthKeyBasic]; !exists { + validationErr = fmt.Errorf("missing required key %q in secret type %q", AuthKeyBasic, secret.Type) } - default: - validationErr = fmt.Errorf("secret type must be %q not %q", apiv1.SecretTypeTLS, secret.Type) + validationErr = fmt.Errorf("unsupported secret type %q", secret.Type) } r.resolvedSecrets[nsname] = &secretEntry{ From 3fa25c0c34750a5178cd5f8b9981abcb99e20217 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 11 Dec 2025 12:59:47 +0000 Subject: [PATCH 09/16] Update basicAuthUserFile const --- internal/controller/nginx/config/generator.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index f422d8c4a2..c3d1ee12a8 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -69,7 +69,9 @@ const ( // nginxPlusConfigFile is the path to the file containing the NGINX Plus API config. nginxPlusConfigFile = httpFolder + "/plus-api.conf" - basicAuthUserFile = configFolder + "/secrets/%s" + // basicAuthUserFile is the path to the file containing basic authentication user credentials. + // Example format: /etc/nginx/secrets/namespace/secretname/auth + basicAuthUserFile = secretsFolder + "/%s" ) // Generator generates NGINX configuration files. From d2f170bddf2a65ab176f61dc82438082cde98886 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 11 Dec 2025 16:49:06 +0000 Subject: [PATCH 10/16] Ensure secrets are resolved --- internal/controller/nginx/config/generator.go | 5 +--- internal/controller/nginx/config/servers.go | 9 +++++-- .../state/graph/authentication_filter.go | 27 +++++++++++-------- internal/controller/state/graph/graph.go | 6 ++++- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index c3d1ee12a8..c29a500729 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -68,10 +68,6 @@ const ( // nginxPlusConfigFile is the path to the file containing the NGINX Plus API config. nginxPlusConfigFile = httpFolder + "/plus-api.conf" - - // basicAuthUserFile is the path to the file containing basic authentication user credentials. - // Example format: /etc/nginx/secrets/namespace/secretname/auth - basicAuthUserFile = secretsFolder + "/%s" ) // Generator generates NGINX configuration files. @@ -272,6 +268,7 @@ func generateCertBundleFileName(id dataplane.CertBundleID) string { } func generateAuthBasicUserFile(id string, data []byte) agent.File { + basicAuthUserFile := secretsFolder + "/%s" return agent.File{ Meta: &pb.FileMeta{ Name: fmt.Sprintf(basicAuthUserFile, id), diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index d8f4f3d931..357857d30a 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -739,11 +739,16 @@ func updateLocationAuthenticationFilter( if authenticationFilter != nil { logger.Info("Applying authentication filter to location", "locationPath", location.Path) if authenticationFilter.Basic != nil { - userFilePathAndData := fmt.Sprintf("%s/%s", authenticationFilter.Basic.SecretName, graph.AuthKeyBasic) + // TODO: Include namespace + userFilePathAndData := fmt.Sprintf("%s/%s/%s", + secretsFolder, + authenticationFilter.Basic.SecretName, + graph.AuthKeyBasic, + ) location.AuthBasic = &http.AuthBasic{ Realm: authenticationFilter.Basic.Realm, Data: http.AuthBasicData{ - FileName: fmt.Sprintf(basicAuthUserFile, userFilePathAndData), + FileName: userFilePathAndData, FileData: authenticationFilter.Basic.Data, }, } diff --git a/internal/controller/state/graph/authentication_filter.go b/internal/controller/state/graph/authentication_filter.go index 09ce673799..4b085bde3e 100644 --- a/internal/controller/state/graph/authentication_filter.go +++ b/internal/controller/state/graph/authentication_filter.go @@ -50,6 +50,7 @@ func getAuthenticationFilterResolverForNamespace( func processAuthenticationFilters( authenticationFilters map[types.NamespacedName]*ngfAPI.AuthenticationFilter, + secretResolver *secretResolver, resolvedSecrets map[types.NamespacedName]*Secret, ) map[types.NamespacedName]*AuthenticationFilter { if len(authenticationFilters) == 0 { @@ -59,7 +60,7 @@ func processAuthenticationFilters( processed := make(map[types.NamespacedName]*AuthenticationFilter) for nsname, af := range authenticationFilters { - if cond := validateAuthenticationFilter(af, resolvedSecrets); cond != nil { + if cond := validateAuthenticationFilter(af, secretResolver, resolvedSecrets); cond != nil { processed[nsname] = &AuthenticationFilter{ Source: af, Conditions: []conditions.Condition{*cond}, @@ -79,6 +80,7 @@ func processAuthenticationFilters( func validateAuthenticationFilter( af *ngfAPI.AuthenticationFilter, + secretResolver *secretResolver, resolvedSecrets map[types.NamespacedName]*Secret, ) *conditions.Condition { var allErrs field.ErrorList @@ -87,10 +89,17 @@ func validateAuthenticationFilter( switch af.Spec.Type { case ngfAPI.AuthTypeBasic: - allErrs = append(allErrs, field.Invalid( - field.NewPath("spec.basic.secretRef"), - af.Spec.Basic.SecretRef.Name, - validateReferencedSecret(af, resolvedSecrets).Error())) + secretNsName := &types.NamespacedName{ + Namespace: af.Namespace, + Name: af.Spec.Basic.SecretRef.Name, + } + + if err := secretResolver.resolve(*secretNsName); err != nil { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec.basic.secretRef"), + af.Spec.Basic.SecretRef.Name, + resolveReferencedSecret(secretNsName, resolvedSecrets).Error())) + } for nsname, secret := range resolvedSecrets { if nsname.Namespace == af.Namespace && nsname.Name == af.Spec.Basic.SecretRef.Name { @@ -126,14 +135,10 @@ func validateAuthenticationFilter( return nil } -func validateReferencedSecret( - af *ngfAPI.AuthenticationFilter, +func resolveReferencedSecret( + secretNsName *types.NamespacedName, resolvedSecrets map[types.NamespacedName]*Secret, ) error { - secretNsName := &types.NamespacedName{ - Namespace: af.Namespace, - Name: af.Spec.Basic.SecretRef.Name, - } if _, exists := resolvedSecrets[*secretNsName]; !exists { return fmt.Errorf("referenced secret is invalid or missing") } diff --git a/internal/controller/state/graph/graph.go b/internal/controller/state/graph/graph.go index 252c1c030a..e9ffde63dd 100644 --- a/internal/controller/state/graph/graph.go +++ b/internal/controller/state/graph/graph.go @@ -259,7 +259,11 @@ func BuildGraph( referencedSecrets := secretResolver.getResolvedSecrets() - processedAuthenticationFilters := processAuthenticationFilters(state.AuthenticationFilters, referencedSecrets) + processedAuthenticationFilters := processAuthenticationFilters( + state.AuthenticationFilters, + secretResolver, + referencedSecrets, + ) routes := buildRoutesForGateways( validators.HTTPFieldsValidator, From a681ba544a83920627da2a309527cde82c0137fc Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Fri, 12 Dec 2025 09:13:35 +0000 Subject: [PATCH 11/16] Resolve nil pointer error --- examples/basic-authentication/basic-auth.yaml | 4 +- .../state/graph/authentication_filter.go | 37 +++++++------------ internal/controller/state/graph/graph.go | 3 +- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/examples/basic-authentication/basic-auth.yaml b/examples/basic-authentication/basic-auth.yaml index 993cb31fef..e24910b94d 100644 --- a/examples/basic-authentication/basic-auth.yaml +++ b/examples/basic-authentication/basic-auth.yaml @@ -2,7 +2,7 @@ apiVersion: v1 kind: Secret metadata: name: basic-auth1 -type: Opaque +type: nginx.org/htpasswd data: # Base64 of "htpasswd -bn user1 password1" auth: dXNlcjE6JGFwcjEkWEFKeU5yekgkY0Rjdy9YMVBCZTFmTjltQVBweXpxMA== @@ -23,7 +23,7 @@ apiVersion: v1 kind: Secret metadata: name: basic-auth2 -type: Opaque +type: nginx.org/htpasswd data: # Base64 of "htpasswd -bn user2 password2" auth: dXNlcjI6JGFwcjEkd0lKUUpjZEUkSXUuYjVhMlBGODdtQi5zT0x4aUg5MQ== diff --git a/internal/controller/state/graph/authentication_filter.go b/internal/controller/state/graph/authentication_filter.go index 4b085bde3e..700030790b 100644 --- a/internal/controller/state/graph/authentication_filter.go +++ b/internal/controller/state/graph/authentication_filter.go @@ -1,13 +1,12 @@ package graph import ( - "fmt" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" v1 "sigs.k8s.io/gateway-api/apis/v1" ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" ) @@ -50,7 +49,6 @@ func getAuthenticationFilterResolverForNamespace( func processAuthenticationFilters( authenticationFilters map[types.NamespacedName]*ngfAPI.AuthenticationFilter, - secretResolver *secretResolver, resolvedSecrets map[types.NamespacedName]*Secret, ) map[types.NamespacedName]*AuthenticationFilter { if len(authenticationFilters) == 0 { @@ -60,7 +58,7 @@ func processAuthenticationFilters( processed := make(map[types.NamespacedName]*AuthenticationFilter) for nsname, af := range authenticationFilters { - if cond := validateAuthenticationFilter(af, secretResolver, resolvedSecrets); cond != nil { + if cond := validateAuthenticationFilter(af, resolvedSecrets); cond != nil { processed[nsname] = &AuthenticationFilter{ Source: af, Conditions: []conditions.Condition{*cond}, @@ -80,7 +78,6 @@ func processAuthenticationFilters( func validateAuthenticationFilter( af *ngfAPI.AuthenticationFilter, - secretResolver *secretResolver, resolvedSecrets map[types.NamespacedName]*Secret, ) *conditions.Condition { var allErrs field.ErrorList @@ -89,18 +86,6 @@ func validateAuthenticationFilter( switch af.Spec.Type { case ngfAPI.AuthTypeBasic: - secretNsName := &types.NamespacedName{ - Namespace: af.Namespace, - Name: af.Spec.Basic.SecretRef.Name, - } - - if err := secretResolver.resolve(*secretNsName); err != nil { - allErrs = append(allErrs, field.Invalid( - field.NewPath("spec.basic.secretRef"), - af.Spec.Basic.SecretRef.Name, - resolveReferencedSecret(secretNsName, resolvedSecrets).Error())) - } - for nsname, secret := range resolvedSecrets { if nsname.Namespace == af.Namespace && nsname.Name == af.Spec.Basic.SecretRef.Name { msg := "referenced secret is invalid or missing" @@ -135,12 +120,16 @@ func validateAuthenticationFilter( return nil } -func resolveReferencedSecret( - secretNsName *types.NamespacedName, - resolvedSecrets map[types.NamespacedName]*Secret, -) error { - if _, exists := resolvedSecrets[*secretNsName]; !exists { - return fmt.Errorf("referenced secret is invalid or missing") +func resolveAuthenticationFilterSecrets( + authenticationFilters map[types.NamespacedName]*ngfAPI.AuthenticationFilter, + secretResolver *secretResolver, +) { + for nsname, af := range authenticationFilters { + if af.Spec.Type == ngfAPIv1alpha1.AuthTypeBasic && af.Spec.Basic != nil { + sec := types.NamespacedName{Namespace: nsname.Namespace, Name: af.Spec.Basic.SecretRef.Name} + // resolve populates resolvedSecrets whether valid or invalid; errors are stored per-entry + _ = secretResolver.resolve(sec) + } } - return nil + } diff --git a/internal/controller/state/graph/graph.go b/internal/controller/state/graph/graph.go index e9ffde63dd..77b3793433 100644 --- a/internal/controller/state/graph/graph.go +++ b/internal/controller/state/graph/graph.go @@ -257,11 +257,12 @@ func BuildGraph( processedSnippetsFilters := processSnippetsFilters(state.SnippetsFilters) + resolveAuthenticationFilterSecrets(state.AuthenticationFilters, secretResolver) + referencedSecrets := secretResolver.getResolvedSecrets() processedAuthenticationFilters := processAuthenticationFilters( state.AuthenticationFilters, - secretResolver, referencedSecrets, ) From 5effdbc6de5558cd3b540e5cce9124f13041e3c7 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Fri, 12 Dec 2025 11:17:48 +0000 Subject: [PATCH 12/16] Remove redundant error handling --- examples/basic-authentication/basic-auth.yaml | 2 +- .../state/graph/authentication_filter.go | 31 +++---------------- internal/controller/state/graph/graph.go | 11 +++---- 3 files changed, 11 insertions(+), 33 deletions(-) diff --git a/examples/basic-authentication/basic-auth.yaml b/examples/basic-authentication/basic-auth.yaml index e24910b94d..3826642428 100644 --- a/examples/basic-authentication/basic-auth.yaml +++ b/examples/basic-authentication/basic-auth.yaml @@ -23,7 +23,7 @@ apiVersion: v1 kind: Secret metadata: name: basic-auth2 -type: nginx.org/htpasswd +type: Opaque data: # Base64 of "htpasswd -bn user2 password2" auth: dXNlcjI6JGFwcjEkd0lKUUpjZEUkSXUuYjVhMlBGODdtQi5zT0x4aUg5MQ== diff --git a/internal/controller/state/graph/authentication_filter.go b/internal/controller/state/graph/authentication_filter.go index 700030790b..fa48b2a043 100644 --- a/internal/controller/state/graph/authentication_filter.go +++ b/internal/controller/state/graph/authentication_filter.go @@ -49,7 +49,6 @@ func getAuthenticationFilterResolverForNamespace( func processAuthenticationFilters( authenticationFilters map[types.NamespacedName]*ngfAPI.AuthenticationFilter, - resolvedSecrets map[types.NamespacedName]*Secret, ) map[types.NamespacedName]*AuthenticationFilter { if len(authenticationFilters) == 0 { return nil @@ -58,7 +57,7 @@ func processAuthenticationFilters( processed := make(map[types.NamespacedName]*AuthenticationFilter) for nsname, af := range authenticationFilters { - if cond := validateAuthenticationFilter(af, resolvedSecrets); cond != nil { + if cond := validateAuthenticationFilter(af); cond != nil { processed[nsname] = &AuthenticationFilter{ Source: af, Conditions: []conditions.Condition{*cond}, @@ -78,33 +77,12 @@ func processAuthenticationFilters( func validateAuthenticationFilter( af *ngfAPI.AuthenticationFilter, - resolvedSecrets map[types.NamespacedName]*Secret, ) *conditions.Condition { var allErrs field.ErrorList //revive:disable-next-line:unnecessary-stmt future-proof switch form; additional auth types will be added switch af.Spec.Type { case ngfAPI.AuthTypeBasic: - - for nsname, secret := range resolvedSecrets { - if nsname.Namespace == af.Namespace && nsname.Name == af.Spec.Basic.SecretRef.Name { - msg := "referenced secret is invalid or missing" - if secret == nil { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec.basic.secretRef"), af.Spec.Basic.SecretRef.Name, msg)) - break - } - if secret.Source == nil { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec.basic.secretRef"), af.Spec.Basic.SecretRef.Name, msg)) - break - } - if _, exists := secret.Source.Data[AuthKeyBasic]; !exists { - msg = "referenced secret does not contain required 'auth' key" - allErrs = append(allErrs, field.Invalid(field.NewPath("spec.basic.secretRef"), af.Spec.Basic.SecretRef.Name, msg)) - } - break - } - } - if af.Spec.Basic.Realm == "" { allErrs = append(allErrs, field.Required(field.NewPath("spec.basic.realm"), "realm cannot be empty")) } @@ -123,13 +101,14 @@ func validateAuthenticationFilter( func resolveAuthenticationFilterSecrets( authenticationFilters map[types.NamespacedName]*ngfAPI.AuthenticationFilter, secretResolver *secretResolver, -) { +) error { + var err error for nsname, af := range authenticationFilters { if af.Spec.Type == ngfAPIv1alpha1.AuthTypeBasic && af.Spec.Basic != nil { sec := types.NamespacedName{Namespace: nsname.Namespace, Name: af.Spec.Basic.SecretRef.Name} // resolve populates resolvedSecrets whether valid or invalid; errors are stored per-entry - _ = secretResolver.resolve(sec) + err = secretResolver.resolve(sec) } } - + return err } diff --git a/internal/controller/state/graph/graph.go b/internal/controller/state/graph/graph.go index 77b3793433..b495df4dad 100644 --- a/internal/controller/state/graph/graph.go +++ b/internal/controller/state/graph/graph.go @@ -257,14 +257,13 @@ func BuildGraph( processedSnippetsFilters := processSnippetsFilters(state.SnippetsFilters) - resolveAuthenticationFilterSecrets(state.AuthenticationFilters, secretResolver) - + err := resolveAuthenticationFilterSecrets(state.AuthenticationFilters, secretResolver) referencedSecrets := secretResolver.getResolvedSecrets() + if err != nil { + // handle error appropriately, e.g., log or return + } - processedAuthenticationFilters := processAuthenticationFilters( - state.AuthenticationFilters, - referencedSecrets, - ) + processedAuthenticationFilters := processAuthenticationFilters(state.AuthenticationFilters) routes := buildRoutesForGateways( validators.HTTPFieldsValidator, From b8f26a649a81097d8456d35d23ebe6e9b26bab82 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Fri, 12 Dec 2025 11:46:14 +0000 Subject: [PATCH 13/16] Do not resolve Opaque secrets --- examples/basic-authentication/basic-auth.yaml | 2 +- internal/controller/nginx/config/servers.go | 19 ----------- .../state/graph/authentication_filter.go | 33 +++++++++---------- internal/controller/state/graph/graph.go | 10 ++---- 4 files changed, 19 insertions(+), 45 deletions(-) diff --git a/examples/basic-authentication/basic-auth.yaml b/examples/basic-authentication/basic-auth.yaml index 3826642428..e24910b94d 100644 --- a/examples/basic-authentication/basic-auth.yaml +++ b/examples/basic-authentication/basic-auth.yaml @@ -23,7 +23,7 @@ apiVersion: v1 kind: Secret metadata: name: basic-auth2 -type: Opaque +type: nginx.org/htpasswd data: # Base64 of "htpasswd -bn user2 password2" auth: dXNlcjI6JGFwcjEkd0lKUUpjZEUkSXUuYjVhMlBGODdtQi5zT0x4aUg5MQ== diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 357857d30a..221d346e6d 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -96,11 +96,8 @@ func (g GeneratorImpl) executeServers( includeFileResults := createIncludeExecuteResultsFromServers(servers) - authBasicUserFileResults := createExecuteResultsForAuthBasicUserFile(servers) - allResults := make([]executeResult, 0, len(includeFileResults)+2) allResults = append(allResults, includeFileResults...) - allResults = append(allResults, authBasicUserFileResults...) allResults = append(allResults, serverResult, httpMatchResult) return allResults @@ -761,22 +758,6 @@ func updateLocationAuthenticationFilter( return location } -func createExecuteResultsForAuthBasicUserFile(servers []http.Server) []executeResult { - results := []executeResult{} - for _, server := range servers { - for _, location := range server.Locations { - if location.AuthBasic != nil { - result := executeResult{ - dest: location.AuthBasic.Data.FileName, - data: location.AuthBasic.Data.FileData, - } - results = append(results, result) - } - } - } - return results -} - func updateLocationMirrorRoute(location http.Location, path string, grpc bool) http.Location { if strings.HasPrefix(path, http.InternalMirrorRoutePathPrefix) { location.Type = http.InternalLocationType diff --git a/internal/controller/state/graph/authentication_filter.go b/internal/controller/state/graph/authentication_filter.go index fa48b2a043..a7f1378441 100644 --- a/internal/controller/state/graph/authentication_filter.go +++ b/internal/controller/state/graph/authentication_filter.go @@ -6,7 +6,6 @@ import ( v1 "sigs.k8s.io/gateway-api/apis/v1" ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" - ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" ) @@ -49,6 +48,7 @@ func getAuthenticationFilterResolverForNamespace( func processAuthenticationFilters( authenticationFilters map[types.NamespacedName]*ngfAPI.AuthenticationFilter, + secretResolver *secretResolver, ) map[types.NamespacedName]*AuthenticationFilter { if len(authenticationFilters) == 0 { return nil @@ -57,7 +57,7 @@ func processAuthenticationFilters( processed := make(map[types.NamespacedName]*AuthenticationFilter) for nsname, af := range authenticationFilters { - if cond := validateAuthenticationFilter(af); cond != nil { + if cond := validateAuthenticationFilter(af, nsname, secretResolver); cond != nil { processed[nsname] = &AuthenticationFilter{ Source: af, Conditions: []conditions.Condition{*cond}, @@ -77,6 +77,8 @@ func processAuthenticationFilters( func validateAuthenticationFilter( af *ngfAPI.AuthenticationFilter, + nsname types.NamespacedName, + secretResolver *secretResolver, ) *conditions.Condition { var allErrs field.ErrorList @@ -86,6 +88,18 @@ func validateAuthenticationFilter( if af.Spec.Basic.Realm == "" { allErrs = append(allErrs, field.Required(field.NewPath("spec.basic.realm"), "realm cannot be empty")) } + if af.Spec.Basic.SecretRef.Name == "" { + allErrs = append(allErrs, field.Required(field.NewPath("spec.basic.secretRef.name"), "secretRef.name cannot be empty")) + } else { + sec := types.NamespacedName{Namespace: nsname.Namespace, Name: af.Spec.Basic.SecretRef.Name} + if err := secretResolver.resolve(sec); err != nil { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec.basic.secretRef"), + af.Spec.Basic.SecretRef.Name, + err.Error(), + )) + } + } default: // Currently, only Basic auth is supported. } @@ -97,18 +111,3 @@ func validateAuthenticationFilter( return nil } - -func resolveAuthenticationFilterSecrets( - authenticationFilters map[types.NamespacedName]*ngfAPI.AuthenticationFilter, - secretResolver *secretResolver, -) error { - var err error - for nsname, af := range authenticationFilters { - if af.Spec.Type == ngfAPIv1alpha1.AuthTypeBasic && af.Spec.Basic != nil { - sec := types.NamespacedName{Namespace: nsname.Namespace, Name: af.Spec.Basic.SecretRef.Name} - // resolve populates resolvedSecrets whether valid or invalid; errors are stored per-entry - err = secretResolver.resolve(sec) - } - } - return err -} diff --git a/internal/controller/state/graph/graph.go b/internal/controller/state/graph/graph.go index b495df4dad..ae4dbe5d2c 100644 --- a/internal/controller/state/graph/graph.go +++ b/internal/controller/state/graph/graph.go @@ -257,13 +257,7 @@ func BuildGraph( processedSnippetsFilters := processSnippetsFilters(state.SnippetsFilters) - err := resolveAuthenticationFilterSecrets(state.AuthenticationFilters, secretResolver) - referencedSecrets := secretResolver.getResolvedSecrets() - if err != nil { - // handle error appropriately, e.g., log or return - } - - processedAuthenticationFilters := processAuthenticationFilters(state.AuthenticationFilters) + processedAuthenticationFilters := processAuthenticationFilters(state.AuthenticationFilters, secretResolver) routes := buildRoutesForGateways( validators.HTTPFieldsValidator, @@ -319,7 +313,7 @@ func BuildGraph( Routes: routes, L4Routes: l4routes, IgnoredGatewayClasses: processedGwClasses.Ignored, - ReferencedSecrets: referencedSecrets, + ReferencedSecrets: secretResolver.getResolvedSecrets(), ReferencedNamespaces: referencedNamespaces, ReferencedServices: referencedServices, ReferencedInferencePools: referencedInferencePools, From 50dfd77bc0dcdcc0e9ba1708d0dfe2ca8a343583 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Fri, 12 Dec 2025 13:58:38 +0000 Subject: [PATCH 14/16] Add secret namespace to file path --- internal/controller/nginx/config/generator.go | 3 ++- internal/controller/nginx/config/servers.go | 3 ++- internal/controller/state/dataplane/convert.go | 7 ++++--- internal/controller/state/dataplane/types.go | 7 ++++--- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index c29a500729..0110b20b5a 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -139,7 +139,8 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []agent.File { for _, matchRule := range rule.MatchRules { if matchRule.Filters.AuthenticationFilter != nil { if matchRule.Filters.AuthenticationFilter.Basic != nil { - id := fmt.Sprintf("%s/%s", matchRule.Filters.AuthenticationFilter.Basic.SecretName, graph.AuthKeyBasic) + ns := matchRule.Filters.AuthenticationFilter.Basic.SecretNamespace + id := fmt.Sprintf("%s/%s/%s", ns, matchRule.Filters.AuthenticationFilter.Basic.SecretName, graph.AuthKeyBasic) data := matchRule.Filters.AuthenticationFilter.Basic.Data files = append(files, generateAuthBasicUserFile(id, data)) } diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 221d346e6d..c5413ba7ed 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -737,8 +737,9 @@ func updateLocationAuthenticationFilter( logger.Info("Applying authentication filter to location", "locationPath", location.Path) if authenticationFilter.Basic != nil { // TODO: Include namespace - userFilePathAndData := fmt.Sprintf("%s/%s/%s", + userFilePathAndData := fmt.Sprintf("%s/%s/%s/%s", secretsFolder, + authenticationFilter.Basic.SecretNamespace, authenticationFilter.Basic.SecretName, graph.AuthKeyBasic, ) diff --git a/internal/controller/state/dataplane/convert.go b/internal/controller/state/dataplane/convert.go index 02a5b7d875..9a92443c1c 100644 --- a/internal/controller/state/dataplane/convert.go +++ b/internal/controller/state/dataplane/convert.go @@ -210,9 +210,10 @@ func convertAuthenticationFilter( }] result.Basic = &BasicAuth{ - SecretName: specBasic.SecretRef.Name, - Data: referencedSecret.Source.Data[graph.AuthKeyBasic], - Realm: specBasic.Realm, + SecretName: specBasic.SecretRef.Name, + SecretNamespace: referencedSecret.Source.Namespace, + Data: referencedSecret.Source.Data[graph.AuthKeyBasic], + Realm: specBasic.Realm, } } diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index dab52d7694..3950206a01 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -188,9 +188,10 @@ type AuthenticationFilter struct { // BasicAuth holds the basic authentication configuration. type BasicAuth struct { - SecretName string - Realm string - Data []byte + SecretName string + SecretNamespace string + Realm string + Data []byte } // HTTPHeader represents an HTTP header. From 5b6ccd1a5288b962b2c99802d7126e5e5e704cf4 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Fri, 12 Dec 2025 14:55:24 +0000 Subject: [PATCH 15/16] Update status variable name --- internal/controller/status/status_setters.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/controller/status/status_setters.go b/internal/controller/status/status_setters.go index 5aebc8c26e..06aed74c21 100644 --- a/internal/controller/status/status_setters.go +++ b/internal/controller/status/status_setters.go @@ -430,7 +430,7 @@ func snippetsStatusEqual(status1, status2 ngfAPI.ControllerStatus) bool { return ConditionsEqual(status1.Conditions, status2.Conditions) } -func newAuthenticationFilterStatusSetter(status ngfAPI.AuthenticationFilterStatus, gatewayCtlrName string) Setter { +func newAuthenticationFilterStatusSetter(afStatus ngfAPI.AuthenticationFilterStatus, gatewayCtlrName string) Setter { return func(obj client.Object) (wasSet bool) { af := helpers.MustCastObject[*ngfAPI.AuthenticationFilter](obj) @@ -443,14 +443,14 @@ func newAuthenticationFilterStatusSetter(status ngfAPI.AuthenticationFilterStatu } } - controllerStatuses = append(controllerStatuses, status.Controllers...) - status.Controllers = controllerStatuses + controllerStatuses = append(controllerStatuses, afStatus.Controllers...) + afStatus.Controllers = controllerStatuses - if authenticationFilterStatusEqual(gatewayCtlrName, status.Controllers, af.Status.Controllers) { + if authenticationFilterStatusEqual(gatewayCtlrName, afStatus.Controllers, af.Status.Controllers) { return false } - af.Status = status + af.Status = afStatus return true } } From 12d6e5425cf253d4c3c9737118c62a5b80a6966c Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Fri, 12 Dec 2025 16:22:18 +0000 Subject: [PATCH 16/16] Update logic for generating auth basic user file --- internal/controller/nginx/config/generator.go | 25 +++++-------- .../state/dataplane/configuration.go | 36 +++++++++++++++++++ .../controller/state/dataplane/convert.go | 2 +- internal/controller/state/dataplane/types.go | 11 ++++-- .../controller/state/graph/secret_test.go | 4 +-- 5 files changed, 55 insertions(+), 23 deletions(-) diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index 0110b20b5a..15d63cefc5 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -17,7 +17,6 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/observability" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/upstreamsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" - "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/graph" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/file" ) @@ -134,19 +133,8 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []agent.File { files = append(files, generateCertBundle(id, bundle)) } - for _, server := range conf.HTTPServers { - for _, rule := range server.PathRules { - for _, matchRule := range rule.MatchRules { - if matchRule.Filters.AuthenticationFilter != nil { - if matchRule.Filters.AuthenticationFilter.Basic != nil { - ns := matchRule.Filters.AuthenticationFilter.Basic.SecretNamespace - id := fmt.Sprintf("%s/%s/%s", ns, matchRule.Filters.AuthenticationFilter.Basic.SecretName, graph.AuthKeyBasic) - data := matchRule.Filters.AuthenticationFilter.Basic.Data - files = append(files, generateAuthBasicUserFile(id, data)) - } - } - } - } + for id, data := range conf.AuthBasicSecrets { + files = append(files, generateAuthBasicUserFile(id, data)) } return files } @@ -268,11 +256,10 @@ func generateCertBundleFileName(id dataplane.CertBundleID) string { return filepath.Join(secretsFolder, string(id)+".crt") } -func generateAuthBasicUserFile(id string, data []byte) agent.File { - basicAuthUserFile := secretsFolder + "/%s" +func generateAuthBasicUserFile(id dataplane.AuthBasicUserFileID, data []byte) agent.File { return agent.File{ Meta: &pb.FileMeta{ - Name: fmt.Sprintf(basicAuthUserFile, id), + Name: generateAuthBasicUserFileName(id), Hash: filesHelper.GenerateHash(data), Permissions: file.SecretFileMode, Size: int64(len(data)), @@ -280,3 +267,7 @@ func generateAuthBasicUserFile(id string, data []byte) agent.File { Contents: data, } } + +func generateAuthBasicUserFileName(id dataplane.AuthBasicUserFileID) string { + return filepath.Join(secretsFolder, string(id)) +} diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 1d7826f44e..29f48974a8 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -8,6 +8,7 @@ import ( "sort" "github.com/go-logr/logr" + coreV1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -93,6 +94,7 @@ func BuildConfiguration( buildRefCertificateBundles(g.ReferencedSecrets, g.ReferencedCaCertConfigMaps), backendGroups, ), + AuthBasicSecrets: buildAuthBasicSecrets(g.ReferencedSecrets), Telemetry: buildTelemetry(g, gateway), BaseHTTPConfig: baseHTTPConfig, BaseStreamConfig: baseStreamConfig, @@ -345,6 +347,36 @@ func buildCertBundles( return bundles } +func buildAuthBasicSecrets(secrets map[types.NamespacedName]*graph.Secret) map[AuthBasicUserFileID]AuthBasicUserData { + authBasics := make(map[AuthBasicUserFileID]AuthBasicUserData) + + for nsname, secret := range secrets { + if secret.Source.Type == coreV1.SecretType(graph.SecretTypeHtpasswd) { + id := generateAuthBasicUserFileID(fmt.Sprintf("%s/%s/%s", nsname.Namespace, nsname.Name, graph.AuthKeyBasic)) + authBasics[id] = secret.Source.Data[graph.AuthKeyBasic] + } + } + return authBasics +} + +// func getAuthBasicSecretIDsFromServers(authBasics map[AuthBasicUserFileID]AuthBasicUserData, servers []VirtualServer) []AuthBasicUserFileID { +// userFileIDs := make([]AuthBasicUserFileID, 0) +// for _, server := range servers { +// for _, rule := range server.PathRules { +// for _, matchRule := range rule.MatchRules { +// if matchRule.Filters.AuthenticationFilter != nil { +// if matchRule.Filters.AuthenticationFilter.Basic != nil { +// ns := matchRule.Filters.AuthenticationFilter.Basic.SecretNamespace +// name := matchRule.Filters.AuthenticationFilter.Basic.SecretName +// id := generateAuthBasicUserFileID(fmt.Sprintf("%s/%s/%s", ns, name, graph.AuthKeyBasic)) +// userFileIDs = append(userFileIDs, id) +// } +// } +// } +// } +// } +// } + func buildBackendGroups(servers []VirtualServer) []BackendGroup { type key struct { nsname types.NamespacedName @@ -980,6 +1012,10 @@ func generateCertBundleID(caCertRef types.NamespacedName) CertBundleID { return CertBundleID(fmt.Sprintf("cert_bundle_%s_%s", caCertRef.Namespace, caCertRef.Name)) } +func generateAuthBasicUserFileID(id string) AuthBasicUserFileID { + return AuthBasicUserFileID(id) +} + func telemetryEnabled(gw *graph.Gateway) bool { if gw == nil { return false diff --git a/internal/controller/state/dataplane/convert.go b/internal/controller/state/dataplane/convert.go index 9a92443c1c..22a90dc43e 100644 --- a/internal/controller/state/dataplane/convert.go +++ b/internal/controller/state/dataplane/convert.go @@ -209,7 +209,7 @@ func convertAuthenticationFilter( Name: specBasic.SecretRef.Name, }] - result.Basic = &BasicAuth{ + result.Basic = &AuthBasic{ SecretName: specBasic.SecretRef.Name, SecretNamespace: referencedSecret.Source.Namespace, Data: referencedSecret.Source.Data[graph.AuthKeyBasic], diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 3950206a01..a541044248 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -32,6 +32,8 @@ type Configuration struct { BaseStreamConfig BaseStreamConfig // SSLKeyPairs holds all unique SSLKeyPairs. SSLKeyPairs map[SSLKeyPairID]SSLKeyPair + // AuthBasicSecrets holds all unique secrets for basic authentication. + AuthBasicSecrets map[AuthBasicUserFileID]AuthBasicUserData // AuxiliarySecrets contains additional secret data, like certificates/keys/tokens that are not related to // Gateway API resources. AuxiliarySecrets map[graph.SecretFileType][]byte @@ -78,6 +80,9 @@ type AuthBasicUserFileID string // CertBundle is a Certificate bundle. type CertBundle []byte +// AuthBasicUserData is the data for a basic auth user file. +type AuthBasicUserData []byte + // SSLKeyPair is an SSL private/public key pair. type SSLKeyPair struct { // Cert is the certificate. @@ -183,11 +188,11 @@ type Snippet struct { // AuthenticationFilter represents an authentication filter. type AuthenticationFilter struct { // This is where the data is extracted to. - Basic *BasicAuth + Basic *AuthBasic } -// BasicAuth holds the basic authentication configuration. -type BasicAuth struct { +// AuthBasic holds the basic authentication configuration. +type AuthBasic struct { SecretName string SecretNamespace string Realm string diff --git a/internal/controller/state/graph/secret_test.go b/internal/controller/state/graph/secret_test.go index 3ef2fbeec3..e3a1375ac5 100644 --- a/internal/controller/state/graph/secret_test.go +++ b/internal/controller/state/graph/secret_test.go @@ -197,12 +197,12 @@ func TestSecretResolver(t *testing.T) { { name: "invalid secret type", nsname: client.ObjectKeyFromObject(invalidSecretType), - expectedErrMsg: `secret type must be "kubernetes.io/tls" not "kubernetes.io/dockercfg"`, + expectedErrMsg: `unsupported secret type "kubernetes.io/dockercfg"`, }, { name: "invalid secret type, again", nsname: client.ObjectKeyFromObject(invalidSecretType), - expectedErrMsg: `secret type must be "kubernetes.io/tls" not "kubernetes.io/dockercfg"`, + expectedErrMsg: `unsupported secret type "kubernetes.io/dockercfg"`, }, { name: "invalid secret cert",