Generate CRD for AuthenticationFilter with BasicAuth #4349

shaun-nx · 2025-11-27T11:23:44Z

Proposed changes

This change adds a new GoLang API type to apis/v1alpha1 for the AuthenticationFilter
This change only adds the types required for Basic Auth to work, and does not include types specific to JWT Auth. These will be added in future work.

Closes #4309

Example manifest for basic auth

apiVersion: gateway.nginx.org/v1alpha1
kind: AuthenticationFilter
metadata:
  name: basic-auth
spec:
  type: Basic
  basic:
    secretRef:
      name: basic-auth-users
    realm: "Restricted"
    onFailure:
      statusCode: 401
      scheme: Basic

Example output from kubectl describe authenticationfilters.gateway.nginx.org basic-auth

Name:         basic-auth
Namespace:    default
Labels:       <none>
Annotations:  <none>
API Version:  gateway.nginx.org/v1alpha1
Kind:         AuthenticationFilter
Metadata:
  Creation Timestamp:  2025-11-27T10:49:42Z
  Generation:          1
  Resource Version:    58448
  UID:                 b0f0a18a-ba43-453c-b315-dcecf39ad06f
Spec:
  Basic:
    On Failure:
      Body Policy:  Unauthorized
      Scheme:       Basic
      Status Code:  401
    Realm:          Restricted
    Secret Ref:
      Name:  basic-auth-users
  Type:      Basic
Events:      <none>

Validation

Performed manual checks on x-Kubernetes-Validations for CRD:

Rule: for type=Basic, spec.basic must be set

File used:

apiVersion: gateway.nginx.org/v1alpha1
kind: AuthenticationFilter
metadata:
  name: basic-auth
spec:
  type: Basic
  # spec.basic is missing

Result:

The AuthenticationFilter "basic-auth" is invalid: 
* spec.basic: Required value
* <nil>: Invalid value: null: some validation rules were not checked because the object was invalid; correct the existing errors to complete validation

Rule: when spec.basic is set, type must be 'Basic'

Files used:

where type is not defined
where type is set to Blah

apiVersion: gateway.nginx.org/v1alpha1
kind: AuthenticationFilter
metadata:
  name: basic-auth-empty-type
spec:
  # type field is missing
  basic:
    secretRef:
      name: basic-auth-users
    realm: "Restricted"    
    onFailure:                
      statusCode: 401
      scheme: Basic
---
apiVersion: gateway.nginx.org/v1alpha1
kind: AuthenticationFilter
metadata:
  name: basic-auth-empty-blah
spec:
  type: Blah  # type field is invalid
  basic:
    secretRef:
      name: basic-auth-users
    realm: "Restricted"    
    onFailure:                
      statusCode: 401
      scheme: Basic

Results:

Error from server (Invalid): error when creating "STDIN": AuthenticationFilter.gateway.nginx.org "basic-auth-empty-type" is invalid: [spec.type: Required value, <nil>: Invalid value: null: some validation rules were not checked because the object was invalid; correct the existing errors to complete validation]

Error from server (Invalid): error when creating "STDIN": AuthenticationFilter.gateway.nginx.org "basic-auth-empty-blah" is invalid: [spec.type: Unsupported value: "Blah": supported values: "Basic", <nil>: Invalid value: null: some validation rules were not checked because the object was invalid; correct the existing errors to complete validation]

Checklist

Before creating a PR, run through this checklist and mark each as complete.

I have read the CONTRIBUTING doc
I have added tests that prove my fix is effective or that my feature works
I have checked that all unit tests pass after adding my changes
I have updated necessary documentation
I have rebased my branch onto main
I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

NONE

codecov · 2025-11-27T11:26:22Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (feat/authentication-filter-basic-auth@82a86f4). Learn more about missing BASE report.

Additional details and impacted files

@@                           Coverage Diff                            @@
##             feat/authentication-filter-basic-auth    #4349   +/-   ##
========================================================================
  Coverage                                         ?   86.11%           
========================================================================
  Files                                            ?      132           
  Lines                                            ?    14343           
  Branches                                         ?       35           
========================================================================
  Hits                                             ?    12351           
  Misses                                           ?     1787           
  Partials                                         ?      205

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

tataruty · 2025-11-27T11:34:20Z

the target should be feat/authentication-filter-basic-auth i guess

…auth-crd

shaun-nx · 2025-11-27T15:47:32Z

The linter seemed to have problems with the comments the AuthenticationFilterSpec and BasicAuth structs: a127e6b
This does affect the description given for secret ref. See here: https://github.com/nginx/nginx-gateway-fabric/pull/4349/commits/da972012a91784412d617785b81d79210db1fb10#diff-085e561f7a49fee[…]9fc1703ae43dbL97-R93
I'm not sure if there is a way around this. What do you all think?

apis/v1alpha1/authenticationfilter_types.go

sjberman · 2025-11-28T05:00:35Z

The linter seemed to have problems with the comments the AuthenticationFilterSpec and BasicAuth structs

@shaun-nx I think I wrote about this on another PR, but the linter (fieldalignment) does not have a problem with comments. The linter automatically reorganizes fields within a struct to optimize it for the compiler. There is a bug that causes comments to be removed when it does this, so you have to manually add the comments back in.

shaun-nx · 2025-11-28T09:25:48Z

The linter seemed to have problems with the comments the AuthenticationFilterSpec and BasicAuth structs

@shaun-nx I think I wrote about this on another PR, but the linter (fieldalignment) does not have a problem with comments. The linter automatically reorganizes fields within a struct to optimize it for the compiler. There is a bug that causes comments to be removed when it does this, so you have to manually add the comments back in.

Thanks Saylor. We managed to fix it too. @tataruty pointed out the same thing. Will probably be one of those things that I need to see a few times before I remember haha.

tataruty · 2025-12-01T10:37:48Z

apis/v1alpha1/authenticationfilter_types.go

+
+const (
+	AuthSchemeBasic  AuthScheme = "Basic"
+	AuthSchemeBearer AuthScheme = "Bearer"


should we mention in comment that bearer is for JWT auth?

Good catch! Since we're just implementing Basic Auth now, I'll remove Bearer as an option.

bjee19 · 2025-12-01T19:56:15Z

apis/v1alpha1/authenticationfilter_types.go

Following a response to a comment in my RL doc, we typically have our fields as pointers. What makes a field optional is the // +optional tag for kubebuilder not if it is a pointer or not.

From what I remember, we only use pointers on our Optional fields.
Are you saying all fields should be pointers?

hmm, as i checked the other apis we have, it seems like i was mistaken, yea we only use pointers on our optional fields, so you can disregard my comment

You have a lot of fields defined here that should be optional but aren't.

Scratch my last comment, I misread. However, BasicAuth in the top level spec should be optional.

Thanks for calling that one out. I had planned on having it as a required field and making it optional only after JWT auth was added as well, but that might break the API. I'll update that to be optional

sjberman

See #4266 for an example of API structure and other things that should be implemented as part of this.

sjberman · 2025-12-01T19:58:15Z

apis/v1alpha1/authenticationfilter_types.go

+	//
+	// +optional
+	// +kubebuilder:default=401
+	// +kubebuilder:validation:XValidation:message="statusCode must be 401 or 403",rule="self in [401, 403]"


You don't need CEL for this, you can use an enum.

// +kubebuilder:validation:Enum=401;403

Ah, that's a good point. I'll change that.

sjberman · 2025-12-01T20:00:00Z

apis/v1alpha1/authenticationfilter_types.go

Please ensure that all fields have comments associated with them that describe what the field is.

When through them all. From what I can see they should all have descriptive comments.
Some of the type fields were missing the same comment separation as well so I added them were I saw they were missing here

constants should have comments as well

Should be updated now.

sjberman · 2025-12-01T20:01:15Z

apis/v1alpha1/authenticationfilter_types.go

+// AuthFailureBodyPolicy controls the failure response body behavior.
+// +kubebuilder:validation:Enum=Unauthorized;Forbidden;Empty


Suggested change

// AuthFailureBodyPolicy controls the failure response body behavior.

// +kubebuilder:validation:Enum=Unauthorized;Forbidden;Empty

// AuthFailureBodyPolicy controls the failure response body behavior.

//

// +kubebuilder:validation:Enum=Unauthorized;Forbidden;Empty

We like the separation between descriptive and functional comments. Be sure to apply to all other fields in the same way.

sjberman · 2025-12-01T20:01:27Z

apis/v1alpha1/authenticationfilter_types.go

+// AuthFailureResponse customizes 401/403 failures.
+//
+


comment malformed

…auth-crd

sjberman · 2025-12-02T19:27:35Z

apis/v1alpha1/authenticationfilter_types.go

+	// Status defines the state of the AuthenticationFilter, following the same
+	// pattern as SnippetsFilter: per-controller conditions with an Accepted condition.
+	//
+	// +optional


Suggested change

// Status defines the state of the AuthenticationFilter, following the same

// pattern as SnippetsFilter: per-controller conditions with an Accepted condition.

//

// +optional

// Status defines the state of the AuthenticationFilter.

No need to reference another filter in here, can just keep this isolated. It also shouldn't be marked as optional.

sjberman · 2025-12-02T19:28:32Z

apis/v1alpha1/authenticationfilter_types.go

+}
+
+// AuthenticationFilterSpec defines the desired configuration.
+// +kubebuilder:validation:XValidation:message="for type=Basic, spec.basic must be set",rule="self.type == 'Basic' ? self.basic != null : true"


Since Basic is optional, no need for this validation.

sjberman · 2025-12-02T19:29:44Z

apis/v1alpha1/authenticationfilter_types.go

+	// Basic configures HTTP Basic Authentication.
+	//
+	// +optional
+	Basic *BasicAuth `json:"basic"`


Suggested change

Basic *BasicAuth `json:"basic"`

Basic *BasicAuth `json:"basic,omitempty"`

Unless this isn't needed due to being a nested struct?

sjberman · 2025-12-02T19:30:53Z

apis/v1alpha1/authenticationfilter_types.go

+
+// BasicAuth configures HTTP Basic Authentication.
+type BasicAuth struct {
+	// OnFailure customizes the 401 response for failed authentication.


Suggested change

// OnFailure customizes the 401 response for failed authentication.

// OnFailure customizes the response for failed authentication.

sjberman · 2025-12-02T19:32:44Z

apis/v1alpha1/authenticationfilter_types.go

+type AuthScheme string
+
+const (
+	AuthSchemeBasic AuthScheme = "Basic" // For Basic Auth.


Comment should be above the field and follow the usual format.

sjberman · 2025-12-02T19:33:21Z

apis/v1alpha1/authenticationfilter_types.go

+	AuthFailureBodyPolicyEmpty AuthFailureBodyPolicy = "Empty"
+)
+
+// AuthFailureResponse customizes 401/403 failures.


Suggested change

// AuthFailureResponse customizes 401/403 failures.

// AuthFailureResponse customizes auth response failures.

sjberman · 2025-12-02T19:33:43Z

apis/v1alpha1/authenticationfilter_types.go

+	// Allowed: 401, 403.
+	// Default: 401.


Need a more descriptive comment on what the StatusCode field actually is.

Is the 401 default provided by nginx? If not, then we'll need to set the default with the kubebuilder tag in the spec.

sjberman · 2025-12-02T19:34:55Z

apis/v1alpha1/authenticationfilter_types.go

+	// Configures WWW-Authenticate header in error page location.
+	//
+	// +optional
+	// +kubebuilder:default=Basic


If we infer from the filter type, then we don't need to set a default in here. Otherwise, it's impossible to omit.

sjberman · 2025-12-02T19:35:23Z

apis/v1alpha1/authenticationfilter_types.go

+	// Default: Unauthorized.
+	//
+	// +optional
+	// +kubebuilder:default=Unauthorized


Is this an nginx default or our own default?

shaun-nx added 2 commits November 27, 2025 10:58

Add base AuthenticationFilter CRD for Basic Auth

a7bb5df

Fix validation for StatusCode

494a8d1

shaun-nx requested a review from a team as a code owner November 27, 2025 11:23

github-project-automation bot added this to NGINX Gateway Fabric Nov 27, 2025

github-project-automation bot moved this to 🆕 New in NGINX Gateway Fabric Nov 27, 2025

github-actions bot added the enhancement New feature or request label Nov 27, 2025

shaun-nx changed the base branch from main to feat/authentication-filter-basic-auth November 27, 2025 11:41

shaun-nx added 3 commits November 27, 2025 15:44

Merge branch 'feat/authentication-filter-basic-auth' into feat/basic-…

b7ef465

…auth-crd

Run linter

a127e6b

Re-generate CRD

da97201

tataruty reviewed Nov 27, 2025

View reviewed changes

apis/v1alpha1/authenticationfilter_types.go Show resolved Hide resolved

apis/v1alpha1/authenticationfilter_types.go Show resolved Hide resolved

apis/v1alpha1/authenticationfilter_types.go Show resolved Hide resolved

Re-generate CRD

1c6843d

shaun-nx requested a review from tataruty November 28, 2025 09:26

tataruty reviewed Dec 1, 2025

View reviewed changes

tataruty mentioned this pull request Dec 1, 2025

Basic AuthenticationFilter examples #4363

Merged

6 tasks

shaun-nx added 3 commits December 1, 2025 12:29

Remove Bearer Auth Scheme

8f5f2a4

Re-generate CRD

89440ce

Fix lint error

429367a

shaun-nx requested a review from tataruty December 1, 2025 13:55

bjee19 reviewed Dec 1, 2025

View reviewed changes

sjberman reviewed Dec 1, 2025

View reviewed changes

shaun-nx added 4 commits December 1, 2025 23:11

Update comments. Make spec.basic an optional field

822ff85

Re-generate CRD. Format comments

0e495c1

Add comments to AuthFailureBodyPolicy constants and AuthType constants

87aae3b

Merge branch 'feat/authentication-filter-basic-auth' into feat/basic-…

bf5de8f

…auth-crd

Merge branch 'feat/authentication-filter-basic-auth' into feat/basic-…

069104e

…auth-crd

sjberman reviewed Dec 2, 2025

View reviewed changes

		// AuthFailureBodyPolicy controls the failure response body behavior.
		// +kubebuilder:validation:Enum=Unauthorized;Forbidden;Empty

	Basic *BasicAuth `json:"basic"`
	Basic *BasicAuth `json:"basic,omitempty"`

	// OnFailure customizes the 401 response for failed authentication.
	// OnFailure customizes the response for failed authentication.

	// AuthFailureResponse customizes 401/403 failures.
	// AuthFailureResponse customizes auth response failures.

Generate CRD for AuthenticationFilter with BasicAuth #4349

Are you sure you want to change the base?

Generate CRD for AuthenticationFilter with BasicAuth #4349

Conversation

shaun-nx commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Validation

Checklist

Release notes

Uh oh!

codecov bot commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tataruty commented Nov 27, 2025

Uh oh!

shaun-nx commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sjberman commented Nov 28, 2025

Uh oh!

shaun-nx commented Nov 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjberman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shaun-nx Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shaun-nx commented Nov 27, 2025 •

edited

Loading

codecov bot commented Nov 27, 2025 •

edited

Loading

shaun-nx commented Nov 27, 2025 •

edited

Loading

shaun-nx Dec 1, 2025 •

edited

Loading