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

Subdirectory deployment #1092

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Subdirectory deployment #1092

wants to merge 20 commits into from

Conversation

trogper
Copy link
Contributor

@trogper trogper commented Dec 24, 2021

Description

This adds support for Kuma deployment in http path subfolder. I have added two environment variables, UPTIME_KUMA_BASE_PATH and BASE_PATH. The value should start and end with /, but there is sanitization, which will add them.
I have changed relevant paths in vue and html from absolute to relative and added html base tag, which is used byt the browser to build absolute paths from relative. Then I made express mount the former root to the specified BASE_PATH. Additionally I have fixed a few <a> tags in vue to proper <router-link>
Socket.io does not support base tag by itself (it builds the connection url manually), so we have to prepend the basePath manually.

Fixes #147

TODO

There are a few things I would like someone to help me with:

  • where is the proper place to put basePath calculation/variable? Currently it is in
    const basePath = document.querySelector("head base").getAttribute("href");
    socket = io(url, {
    path: basePath + "socket.io",
    });

    autoGetPrimaryBaseURL() {
    const basePath = document.querySelector("head base").getAttribute("href");
    this.settings.primaryBaseURL = location.protocol + "//" + location.host + basePath;
    },
  • create tests for it
  • test it manually on their server/deployment (I have deployed it on mine already)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and test it
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

@trogper trogper changed the title Subdirectory Subdirectory deployment Dec 24, 2021
remove unnecessary regex escape
src/mixins/socket.js Outdated Show resolved Hide resolved
Co-authored-by: Adam Stachowicz <saibamenppl@gmail.com>
@trogper trogper marked this pull request as ready for review December 30, 2021 20:50
@trogper
Copy link
Contributor Author

trogper commented Feb 10, 2022

@louislam can you please help me with the implementation details? I have a few questions in TODO in the first comment.

@ckocyigit
Copy link

ckocyigit commented Feb 17, 2022

I have merged this PR in my fork and experienced a strange behaviour.

When pressing the Edit Button on an already existing Notification it redirects me to the Status page.

I wanted to open a Issue to Kuma directly when it occurred to me that I have this probably "not fully tested" PR in my fork.

Maybe @trogper can you check if this happens on your site too?

@zacharlie

This comment was marked as off-topic.

@ckocyigit
Copy link

This is helpful, but It seems that this Pull Request in its current state is not compatible with current master as your Dockerfile describes.

@trogper
Copy link
Contributor Author

trogper commented Apr 11, 2022

Yes, the are minor conflicts. I'll solve them when I get some time for it

@zacharlie
Copy link

@ckocyigit That is true. At the time I posted there were no conflicts. If you need it immediately I do have an older version on dockerhub you can use in the interim: https://hub.docker.com/r/zacharlie/dbstack-monitor

@zacharlie
Copy link

@trogper just a heads up I also tested and noticed the issue described above with notification editing when merging this

@trogper
Copy link
Contributor Author

trogper commented Apr 11, 2022

Thanks, I know about the issue already

@trogper

This comment was marked as resolved.

trogper added 2 commits July 3, 2022 23:40
replaced <a> with <router-link>,
replaced 'location.href =' navigation by router.push()
@github-actions github-actions bot added pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again and removed pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again labels May 19, 2024
@Ionys320
Copy link
Contributor

Ionys320 commented Nov 14, 2024

Question to @CommanderStorm or @louislam: could we run a npm run build after Docker pull process? Let me explain: for the moment, we don't really use Vite base path as it is supposed to work. Therefore, we lost a lot of functionalities who would simplify this PR. By being capable to run npm run buid after Docker pull, we could build the frontend using Vite by providing it the right base path, based on the ENV variable BASE_PATH.

I'm checking on my side, and it seems I only need to change a few files (including the times I change a tag to router-link tag to let Vite take care of it, and adding import.meta.env.BASE_URL), it appears I woudln't need to change more than 15 files. I of course need to do more tests and I can optimize some stuff, but I think this solution would be more maintainable than the one @trogper is working on, mainly because it gets around the problem without using the tools normally used for (Not a critic @trogper ofc, your work is awesome regarding the contrainst you got).

Here is my WIP branch about how we could use Vite base: https://github.com/Ionys320/uptime-kuma/tree/feat/subpath. Matching commit: Ionys320@8ccbbee

@louislam
Copy link
Owner

could we run a npm run build after Docker pull process

Cannot, unless the docker image includes all dev dependencies / build tools which probably is more complicated and I don't want to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:deployment related to how uptime kuma can be deployed help wanted May need your help to test or answer needs:resolve-merge-conflict pr:please address review comments this PR needs a bit more work to be mergable pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subfolder support (publicPath)