Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always generate a root "/" location block in NGINX config to handle unmatched requests with 404 response #356

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

kate-osborn
Copy link
Contributor

If no root path ("/") is defined by the user for a server, generate a location block for the root path that returns 404.

@kate-osborn kate-osborn requested a review from a team as a code owner January 10, 2023 21:01
@kate-osborn kate-osborn requested a review from pleshakov January 10, 2023 21:01
@github-actions github-actions bot added the enhancement New feature or request label Jan 10, 2023
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

👍

@pleshakov
Copy link
Contributor

A rule like below

  - matches:
    - path:
        type: PathPrefix
        value: " /"

leads to

        location  / {

              . . .



        }

        location / {



                return 404 ;





        }

and NGINX failing to reload:

2023/01/10 22:59:31 [emerg] 40#40: duplicate location "/" in /etc/nginx/conf.d/http.conf:63

I think that should be covered by the validation in #343

What do you think?

@kate-osborn
Copy link
Contributor Author

kate-osborn commented Jan 10, 2023

A rule like below

  - matches:
    - path:
        type: PathPrefix
        value: " /"

leads to

        location  / {

              . . .



        }

        location / {



                return 404 ;





        }

and NGINX failing to reload:

2023/01/10 22:59:31 [emerg] 40#40: duplicate location "/" in /etc/nginx/conf.d/http.conf:63

I think that should be covered by the validation in #343

What do you think?

That shouldn't happen with these changes. I verified that only one root location per server block will ever be generated.

(Edit): I missed the leading space in the path. Yeah, I think that case is covered by the validation work.

@kate-osborn kate-osborn force-pushed the feat/always-gen-root-location branch from acbdb67 to 180b383 Compare January 11, 2023 18:59
@kate-osborn kate-osborn merged commit 08fe11c into main Jan 11, 2023
@kate-osborn kate-osborn deleted the feat/always-gen-root-location branch January 11, 2023 19:08
@pleshakov pleshakov changed the title Always generate a root "/" location block Always generate a root "/" location block in NGINX config to handle unmatched requests with 404 response Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants