topaLE
/
uptime-kuma
1
0
Fork 0
Code Issues 185 Pull Requests 26 Projects Releases 28 Wiki Activity
Labels Milestones

Feature monitor the resource over proxy #840

Open
ugurerkan wants to merge 4 commits from rebasesoftware/feature/request-with-http-proxy into master
Conversation 12 Commits 4 Files Changed 14
ugurerkan commented 3 years ago (Migrated from github.com)
Owner

Description

Added new proxy feature based on http and https proxy agents. Proxy feature works like notifications, there is many proxy could be related one proxy entry.

Supported features

  • Proxies can activate and disable in bulk
  • Proxies auto enabled by default for new monitors
  • Proxies could be applied in bulk to current monitors
  • Both authenticated and anonymous proxies supported
  • Export and import support for proxies
  • HTTP, HTTPS, SOCKS, SOCKS v5 SOCKS v4 protocols are supported

Reviews are welcome 🤩

Fixes #470

Type of change

  • User Interface
  • New feature (non-breaking change which adds functionality)

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

Screenshots

Screen Shot 2021-10-30 at 21 36 20
Screen Shot 2021-11-04 at 14 33 48
Screen Shot 2021-10-30 at 21 36 08
Screen Shot 2021-10-30 at 21 35 47

# Description Added new proxy feature based on http and https proxy agents. Proxy feature works like notifications, there is many proxy could be related one proxy entry. Supported features - Proxies can activate and disable in bulk - Proxies auto enabled by default for new monitors - Proxies could be applied in bulk to current monitors - Both authenticated and anonymous proxies supported - Export and import support for proxies - HTTP, HTTPS, SOCKS, SOCKS v5 SOCKS v4 protocols are supported **Reviews are welcome** 🤩 Fixes #470 ## Type of change - User Interface - New feature (non-breaking change which adds functionality) ## Checklist - [x] My code follows the style guidelines of this project - [x] I ran ESLint and other linters for modified files - [x] I have performed a self-review of my own code and test it - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings ## Screenshots ![Screen Shot 2021-10-30 at 21 36 20](https://user-images.githubusercontent.com/473959/139554813-65c229c8-074f-44d0-82dd-a5836d5453ee.png) ![Screen Shot 2021-11-04 at 14 33 48](https://user-images.githubusercontent.com/473959/140307689-2c396bf2-4957-47cd-a49c-a78827c9b77d.png) ![Screen Shot 2021-10-30 at 21 36 08](https://user-images.githubusercontent.com/473959/139554817-92a5247e-1767-4205-b744-3e2a4a4aad9d.png) ![Screen Shot 2021-10-30 at 21 35 47](https://user-images.githubusercontent.com/473959/139554974-cae3a355-5179-436e-bb7c-18141e20c539.png)
👍 5 ❤️ 3
Saibamen (Migrated from github.com) reviewed 3 years ago
server/model/monitor.js
Saibamen (Migrated from github.com) commented 3 years ago
Owner 
                            debug(`HTTP agents options: ${JSON.stringify({
```suggestion debug(`HTTP agents options: ${JSON.stringify({ ```
Saibamen (Migrated from github.com) commented 3 years ago
Owner 
                                "https": httpsProxyAgentOptions,
```suggestion "https": httpsProxyAgentOptions, ```
Saibamen (Migrated from github.com) commented 3 years ago
Owner 
                    debug(`Request options: ${JSON.stringify(options)}`);
```suggestion debug(`Request options: ${JSON.stringify(options)}`); ```
src/components/ProxyDialog.vue
Saibamen (Migrated from github.com) commented 3 years ago
Owner 
    methods: {
```suggestion methods: { ```
Saibamen (Migrated from github.com) reviewed 3 years ago
src/languages/en.js
Saibamen (Migrated from github.com) commented 3 years ago
Owner

to monitor or to function (missing or?)

to monitor or to function (missing `or`?)
src/pages/EditMonitor.vue
Saibamen (Migrated from github.com) commented 3 years ago
Owner

Missing i18n for Default

Missing i18n for `Default`
src/pages/Settings.vue
Saibamen (Migrated from github.com) commented 3 years ago
Owner

Missing i18n for Default

Missing i18n for `Default`
ugurerkan commented 3 years ago (Migrated from github.com)
Poster
Owner

@Saibamen Thank you for your quick reviews, I have updated them.

@Saibamen Thank you for your quick reviews, I have updated them.
👍 1
ugurerkan (Migrated from github.com) reviewed 3 years ago
src/languages/en.js
ugurerkan (Migrated from github.com) commented 3 years ago
Poster
Owner

This is based from notification description. It is tries to explain with "must be assigned to a monitor to function" the proxy does not effect until assigned to a monitor, so I think this should be stay same but if it is not descriptive enough sure we could update.

This is based from notification description. It is tries to explain with "must be assigned to a monitor to function" the proxy does not effect until assigned to a monitor, so I think this should be stay same but if it is not descriptive enough sure we could update.
Saibamen (Migrated from github.com) requested changes 3 years ago
src/pages/EditMonitor.vue
Saibamen (Migrated from github.com) commented 3 years ago
Owner

Please add Default key to en.js

Please add `Default` key to `en.js`
deefdragon commented 3 years ago (Migrated from github.com)
Owner

Looking good. As a note, I think the switches should be "Enabled" and "Set As Default" instead of "Activated" and "Default Enabled".

Looking good. As a note, I think the switches should be "Enabled" and "Set As Default" instead of "Activated" and "Default Enabled".
❤️ 1
louislam commented 3 years ago (Migrated from github.com)
Owner

Screen Shot 2021-10-30 at 21 35 47

Wondering why it looks like that it is allowed to select multiple proxies for a monitor.

If it is single selection, dropdown (select-option) would be better.

> ![Screen Shot 2021-10-30 at 21 35 47](https://user-images.githubusercontent.com/473959/139554974-cae3a355-5179-436e-bb7c-18141e20c539.png) Wondering why it looks like that it is allowed to select multiple proxies for a monitor. If it is single selection, dropdown (select-option) would be better.
👍 2
Saibamen commented 3 years ago (Migrated from github.com)
Owner

Or radio button

Radio button is also better for Edit URL and Default label

Or radio button Radio button is also better for `Edit` URL and `Default` label
ugurerkan commented 3 years ago (Migrated from github.com)
Poster
Owner

Wondering why it looks like that it is allowed to select multiple proxies for a monitor.

If it is single selection, dropdown (select-option) would be better.

@louislam I have used them as radio buttons and multiple proxy can not selected. I prefer radios over dropdown because look more fancy but dropdown also considerable.

https://user-images.githubusercontent.com/473959/140033912-d1dd6c87-b7fc-411d-bf28-83322126654c.mp4

Or radio button

Radio button is also better for Edit URL and Default label

@Saibamen I did not understand this, could you please clarify? 🤔
edit url triggers modal to edit proxy configuration and the default label is just indicator does not have interaction.

> Wondering why it looks like that it is allowed to select multiple proxies for a monitor. > > If it is single selection, dropdown (select-option) would be better. @louislam I have used them as [radio buttons](https://github.com/louislam/uptime-kuma/pull/840/files#diff-f672603317047f3e6f27b0d7a44f6f244b7dbb5d0d0a85f1059a6b0bc2cb9aa0R237) and multiple proxy can not selected. I prefer radios over dropdown because look more fancy but dropdown also considerable. https://user-images.githubusercontent.com/473959/140033912-d1dd6c87-b7fc-411d-bf28-83322126654c.mp4 > Or radio button > > Radio button is also better for `Edit` URL and `Default` label @Saibamen I did not understand this, could you please clarify? 🤔 `edit url` triggers modal to edit proxy configuration and the `default label` is just indicator does not have interaction.
👍 2
ugurerkan commented 3 years ago (Migrated from github.com)
Poster
Owner

Looking good. As a note, I think the switches should be "Enabled" and "Set As Default" instead of "Activated" and "Default Enabled".

@deefdragon Thank you for your advice, agree on this and updated accordingly. Now, it is more expressive.

> Looking good. As a note, I think the switches should be "Enabled" and "Set As Default" instead of "Activated" and "Default Enabled". @deefdragon Thank you for your advice, agree on this and updated accordingly. Now, it is more expressive.
Saibamen commented 3 years ago (Migrated from github.com)
Owner

@ugurerkan

I did not understand this, could you please clarify? 🤔

It is OK. I just wanted to say in my previous comment, that you can't implement working Edit or Default label in dropdown ;)

@ugurerkan > I did not understand this, could you please clarify? 🤔 It is OK. I just wanted to say in my previous comment, that you can't implement working `Edit` or `Default` label in dropdown ;)
🚀 1
thomasleveil commented 3 years ago (Migrated from github.com)
Owner

@ugurerkan nice work, there are a few monitors I cannot yet create because I need to go through a proxy. Would this feature work with sock5 type proxies ?

@ugurerkan nice work, there are a few monitors I cannot yet create because I need to go through a proxy. Would this feature work with sock5 type proxies ?
❤️ 1
ugurerkan commented 3 years ago (Migrated from github.com)
Poster
Owner

@thomasleveil thank you very much.

Currently, the feature supports only HTTP and HTTPS protocols. But, the base proxy agent library is supporting SOCKS and PAC proxy as well. So, this could be easily integrated. I will take a look and try to improve accordingly. SOCKS proxies have common usage, it looks like this feature should have it at least.

@thomasleveil thank you very much. Currently, the feature supports only HTTP and HTTPS protocols. But, the base [proxy agent](https://github.com/TooTallNate/node-proxy-agent) library is supporting SOCKS and PAC proxy as well. So, this could be easily integrated. I will take a look and try to improve accordingly. SOCKS proxies have common usage, it looks like this feature should have it at least.
ugurerkan commented 3 years ago (Migrated from github.com)
Poster
Owner

SOCKS proxy agent library and protocol support added.

Screen Shot 2021-11-04 at 14 33 48

[SOCKS proxy agent](https://github.com/TooTallNate/node-socks-proxy-agent) library and protocol support added. ![Screen Shot 2021-11-04 at 14 33 48](https://user-images.githubusercontent.com/473959/140307004-9aa196a9-e7a7-4e22-a19e-fe569f963906.png)
👍 1 ❤️ 1
thomasleveil commented 3 years ago (Migrated from github.com)
Owner

@ugurerkan may I suggest using {{ proxy.host }}:{{ proxy.port }} ({{ proxy.protocol }}) to describe a proxy
image

@ugurerkan may I suggest using `{{ proxy.host }}:{{ proxy.port }} ({{ proxy.protocol }})` to describe a proxy ![image](https://user-images.githubusercontent.com/42300/140494950-3d8743b1-8450-489b-9cd1-8b7d7224e950.png)
👍 2
ugurerkan commented 3 years ago (Migrated from github.com)
Poster
Owner

@thomasleveil good point, thank you.
Proxies was able to be mistaken by other protocols and ports with same host, updated accordingly.

@thomasleveil good point, thank you. Proxies was able to be mistaken by other protocols and ports with same host, updated accordingly.

Reviewers

Saibamen
This pull request can be merged automatically.
You are not authorized to merge this pull request.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
Saibamen
Labels
Apply labels
Clear labels
bug
Something isn't working dependencies
Pull requests that update a dependency file discussion doc
Improvements or additions to documentation duplicate
This issue or pull request already exists feature-request
New feature or request good first issue
Good for newcomers hacktoberfest hacktoberfest-accepted help help wanted
Extra attention is needed High
High Priority impossible invalid
This doesn't seem right investigating k8s Low
Low Priority Medium
Medium Priority News prerelease bug question
Further information is requested resolved Unknown wontfix
This will not be worked on
No Label bug dependencies discussion doc duplicate feature-request good first issue hacktoberfest hacktoberfest-accepted help help wanted High impossible invalid investigating k8s Low Medium News prerelease bug question resolved Unknown wontfix
Milestone
Set milestone
Clear milestone
No items
No Milestone
1.11.0
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
1 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'rebasesoftware/feature/request-with-http-proxy'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes