-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
feat: add an tls option to the tcp-port monitor #5678
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
base: master
Are you sure you want to change the base?
Conversation
9b1e919
to
e9fa79e
Compare
src/pages/EditMonitor.vue
Outdated
@@ -21,6 +21,9 @@ | |||
<option value="port"> | |||
TCP Port | |||
</option> | |||
<option value="tls"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets go back a step:
how is this different from TCP Port
?
Why can this not be integrated/merged with said monitor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are absolutely right. We can probably integrate things with this monitor. I think I was afraid of breaking something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello
I rebased on master and pushed a new proposal. I kept the old commit, but once the proposal is approved, I will squash them. The behavior is slightly different because the probe will not go down when the certificate is about to expire. It will simply send a notification, as with other probes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once the proposal is approved, I will squash them
no need to squash (please don't). Makes reviewing a bit harder as I need to do this from scratch every time if you do.
The behavior is slightly different because the probe will not go down when the certificate is about to expire. It will simply send a notification, as with other probes.
If it is indeed expired or non responsive, it should go down. That would be a bug.
5be2b20
to
2e2acbd
Compare
TCP tls
monitor
Hello, I just pushed a new version addressing all your remarks. I hope it’s all good now. |
Hello, Could I please have an update regarding this PR? Thank you. |
the update to this PR is that it needs looking at. It is on the to-review list. Reviewer capacity from my side is severly limited this semester - I am taking 38 ECTS and have a working student job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7bac05b
to
1c69b24
Compare
Co-authored-by: Nelson Chan <3271800+chakflying@users.noreply.github.com>
5d195f3
to
179d959
Compare
Tick the checkbox if you understand [x]:
Description
Add a montior to check tls certificate on TCP connexion with others protocols thant HTTP.
Type of change
Checklist
Screenshots
Monitor Configuration
Monitor Résults when it's ok
Change settings of certificate notification
Résult when monitor failed