feat: Add new routes in nginx config of stats.jenkins.io

[Vatsal-Verma]

stats.jenkins.io is a SPA becuase of which it shows 200 OK status on the pages that don't exist or are invlid. So, I found this ngnix config where we can define strict routing.

Currently, the routes present are of old.stats.jenkins.io, However new routes such as:

/dep-graph
/statistics
/plugin-versions
/plugin-trends

are not yet added.

Question from maintainers

• should we remove new.stats.jenkins.io as host which is present in this file since new.stats.jenkins.io is not live

fixes jenkins-infra/stats.jenkins.io#671

[dduportal]

While we are reviewing this in the upcoming days, could you describe how did you test this proposal change to ensure it works as expected (nominal cases)?

[dduportal]

After reading your issue (thanks for the many details in the issue! It helps), this PR won't solve the issue alas.

The key is the location / block which handles "default" requests: it has a try_files instruction which check for existence of the requested file/directory ($uri) or falls back to the SPA on/index.html:

  location / {
      root   /usr/share/nginx/html;
      index  index.html index.htm;
      autoindex on;
      try_files $uri /index.html;
  }

Given the SPA routes does not change often in this application (we have 4 and it is not expected to change), adding the 4 routes is a good trade-off (because we have to maintain this list of 4 endpoints in both the SPA code and the Nginx application).

But we'll have to decide how to handle the root URI: I guess we shall capture the exact location / to have it redirecting all the time on index.html. Something like (in addition to the 4 other routes):

location = / {
      root   /usr/share/nginx/html;
      index  index.html index.htm;
      autoindex on;
}

But we have to see if we shouldn't keep the normal location / {} block (without the try_file) and see how it interacts with unwanted requests.

[Vatsal-Verma]

So, should we remove try_files from location = / ?
I mean it makes sense. I think try_file will force to return an html because of which it may show 200 again ?
I mean we can try removing it and also I can maybe try this on my nginx server on local for testing before moving forward

[dduportal]

So, should we remove try_files from location = / ? I mean it makes sense. I think try_file will force to return an html because of which it may show 200 again ? I mean we can try removing it and also I can maybe try this on my nginx server on local for testing before moving forward

Are you ok to reproduce by running the helm-chart into a local k3s cluster on your machine? Or maybe by using the overrideLocation content on your local nginx at least?

(Sorry been busy today to work on reproducing to help you)

[Vatsal-Verma]

So, should we remove try_files from location = / ? I mean it makes sense. I think try_file will force to return an html because of which it may show 200 again ? I mean we can try removing it and also I can maybe try this on my nginx server on local for testing before moving forward

Are you ok to reproduce by running the helm-chart into a local k3s cluster on your machine? Or maybe by using the overrideLocation content on your local nginx at least?

(Sorry been busy today to work on reproducing to help you)

I can definitely do the second thing i.e. "Or maybe by using the overrideLocation content on your local nginx at least".
I have also tested that partially as mentioned in the issue attached (at the top). I'll replace my present nginx config with these and soon present the results.

btw earlier I was testing for this configs for routes before I found the main configs for which the project is defined.

  location = / {
            try_files /index.html =404;
        }

        location = /statistics {
            try_files /index.html =404;
        }

        location = /plugin-trends {
            try_files /index.html =404;
        }

        location = /plugin-versions {
            try_files /index.html =404;
        }

        location = /dep-graph {
            try_files /index.html =404;
        }

this was based on the fact "Serve index.html. if it doesn't exist, return 404"

[Vatsal-Verma]

Here is a follow up, I have configured my nginx config according to the case here.
here are the results I've got.

route	valid - route	invalid - route
/plugin-verions	200 ok	404 NF
/plugin-trends	200 ok	404 NF
/statistics	200 ok	404 NF
/dep-graph	200 ok	404 NF

Here is my config

Screenshot (CLI)

ScreenShots (web)

Note: I tested with and without try_file in /
but I did not see any difference 😅

[dduportal]

That's good, thanks for your tests. It helps!

FTR, the content from "old.stats.jenkins.io" is retrieved by https://github.com/jenkins-infra/stats.jenkins.io/blob/main/retrieve-infra-statistics-data.sh when building the stats.jenkins.io in https://github.com/jenkins-infra/stats.jenkins.io/blob/a55d61a2daea1c911913df48a01a0a28e94d98b2/Jenkinsfile#L46-L50.

It happens before building the SPA: any collision in the /dist is resolved by using files from the SPA. For instance: index.html. But it merges the content of both old.stats.jenkins.io and the SPA into the dist directory of stats.jenkins.io.

It means you can increase your tests: if you use the shell script above, you can do the same on your local nginx. Would be useful to test the "merged" dist dir and reproduce the whole production setup.

[Vatsal-Verma]

That's good, thanks for your tests. It helps!

FTR, the content from "old.stats.jenkins.io" is retrieved by https://github.com/jenkins-infra/stats.jenkins.io/blob/main/retrieve-infra-statistics-data.sh when building the stats.jenkins.io in https://github.com/jenkins-infra/stats.jenkins.io/blob/a55d61a2daea1c911913df48a01a0a28e94d98b2/Jenkinsfile#L46-L50.

It happens before building the SPA: any collision in the /dist is resolved by using files from the SPA. For instance: index.html. But it merges the content of both old.stats.jenkins.io and the SPA into the dist directory of stats.jenkins.io.

It means you can increase your tests: if you use the shell script above, you can do the same on your local nginx. Would be useful to test the "merged" dist dir and reproduce the whole production setup.

Thanks, I completely forgot to run that sh command (it takes a lot of time to retrieve the data when i tried it last time tbh 😅). I agree, That dist would much be better to test.

[dduportal]

That's good, thanks for your tests. It helps!
FTR, the content from "old.stats.jenkins.io" is retrieved by https://github.com/jenkins-infra/stats.jenkins.io/blob/main/retrieve-infra-statistics-data.sh when building the stats.jenkins.io in https://github.com/jenkins-infra/stats.jenkins.io/blob/a55d61a2daea1c911913df48a01a0a28e94d98b2/Jenkinsfile#L46-L50.
It happens before building the SPA: any collision in the /dist is resolved by using files from the SPA. For instance: index.html. But it merges the content of both old.stats.jenkins.io and the SPA into the dist directory of stats.jenkins.io.
It means you can increase your tests: if you use the shell script above, you can do the same on your local nginx. Would be useful to test the "merged" dist dir and reproduce the whole production setup.

Thanks, I completely forgot to run that sh command (it takes a lot of time to retrieve the data when i tried it last time tbh 😅). I agree, That dist would much be better to test.

You may directly retrieve the gh-pages.zip directly: it does not change a lot so it's safe to test with the same version.

[Vatsal-Verma]

I would like to put something forward. I've noticed that the routes on my local nginx are throwing 404 if even they exist.
It was showing proper results earlier since it was working on my previous config where I was using try_files on each route to show 404 that I've shown earlier ( I've killed the previous process and restarted nginx. ).

#7621 (comment)

So, here is the problem (Currently this is working on the production level dist SPA + old data).

  location = /plugin-trends {
            root D:/nginx-1.28.3/html;
            index index.html index.htm;
       }

What this block is doing. It will check nginx/html if there is a file with this name or a dir? if no then it will give 404 and in our case there is nothing like so in the dir. There is indeed the previous/old data which has dir becuase of which it's working fine for those previous routes. So, I want to say is that, it will give 404 for every case. So, I propose to make the use of try_files for these new routes. Since, we need to understand earlier we were handling the dir and now the SPA routes.

location = /plugin-trends {
            root D:/nginx-1.28.3/html;
            index index.html index.htm;
            try_files /index.html =404;
        }

So, what it does is that, it serves index.html. if it doesn't exist, return 404

without try_files (on new routes)

With try_files (on new routes)

Screenshot (web with try_files on each new route)

[dduportal]

Hello @Vatsal-Verma , a quick head up to let you know that we'll look at it (we don't forget you!) but expect some delays as some team member are in vacations

[dduportal]

Sorry, we've been busy and did not had enough time. We'll try our best in the upcoming days