feat(service): add mqtt service #149

Eugeniosales · 2021-03-15T06:14:14Z

This adds a custom mqtt service as notification options that just publishes the message to a mqtt server such as mosquitto. This feature aims to contribute with the Issue 86 and Issue 710

url := "mqtt://example.com:1083?topic=custom_topic"
err := shoutrrr.Send(url, "Message published through mqtt")

The TLS connection option still needs to be added and some tests, but we would like to get a feedback and keep working on it. Thanks!

Co-authored-by: Mexazonic <[email protected]>

codecov · 2021-03-15T06:15:28Z

Codecov Report

Merging #149 (8c17b96) into main (e156cfd) will increase coverage by 1.09%.
The diff coverage is 100.00%.

❗ Current head 8c17b96 differs from pull request most recent head 37c66fa. Consider uploading reports for the commit 37c66fa to get more accurate results

@@            Coverage Diff             @@
##             main     #149      +/-   ##
==========================================
+ Coverage   63.08%   64.17%   +1.09%     
==========================================
  Files          79       70       -9     
  Lines        2487     2010     -477     
==========================================
- Hits         1569     1290     -279     
+ Misses        780      606     -174     
+ Partials      138      114      -24

Impacted Files	Coverage Δ
pkg/router/servicemap.go	`100.00% <100.00%> (ø)`
pkg/format/format.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/format/format_query.go	`0.00% <0.00%> (-78.13%)`	⬇️
pkg/format/prop_key_resolver.go	`0.00% <0.00%> (-76.93%)`	⬇️
pkg/format/field_info.go	`52.50% <0.00%> (-11.95%)`	⬇️
pkg/format/enum_formatter.go	`60.00% <0.00%> (-10.00%)`	⬇️
pkg/format/formatter.go	`63.33% <0.00%> (-3.34%)`	⬇️
pkg/services/teams/teams_config.go	`79.24% <0.00%> (-1.46%)`	⬇️
pkg/format/node.go	`90.44% <0.00%> (-0.74%)`	⬇️
pkg/router/router.go	`66.30% <0.00%> (-0.02%)`	⬇️
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e156cfd...37c66fa. Read the comment docs.

piksel

Yeah, there are some things that needs addressing. I know that this isn't the easiest API to implement, so don't be too disheartened😄

As a global comment, you probably need to run go fmt on the code, since it's not formatted.
Also, you do still have a lot of code copied from the telegram service that probably shouldn't be used (I added a comment about it as well, since you are now sending telegram API messages to MQTT)

pkg/services/mqtt/mqtt.go

pkg/services/mqtt/mqtt_config.go

pkg/services/mqtt/mqtt.go

pkg/services/mqtt/mqtt_json.go

Co-authored-by: Mexazonic <[email protected]>

Eugeniosales · 2021-04-07T02:15:39Z

@piksel , in this update, we solved the issues that you pointed out. Also, we added the TLS option and some docs.

Co-authored-by: Mexazonic <[email protected]>

simskij

Could you please complement your implementation with some tests verifying the intended behavior? Thanks

When generating a config object with/without optional arguments

simskij · 2021-05-09T19:17:24Z

I have nothing more to add. @piksel, if you feel that your feedback has been addressed, feel free to merge. Thank you @Mexazonic and @Eugeniosales for your contributions! 🔥

Eugeniosales · 2021-06-17T16:13:18Z

Is there anything more to be addressed, @piksel? If so, we'd like to get a feedback and keep working on it.

piksel · 2021-06-17T23:32:39Z

Sorry, I have just been busy with other projects. Will get to it soon, I promise!

piksel

Nice work! The username and password ought to be specified using URL user:pass fields, both for conistency and some planned logging features (hiding credentials by default in logging). I have marked these suggestions with (URL creds) so that they can be batch-merged if you want.
There is also some concern regarding the TLS configuration, but it should be fairly easy to fix!

piksel · 2021-06-20T14:06:46Z

pkg/services/mqtt/mqtt_config.go

+func (config *Config) getURL(resolver types.ConfigQueryResolver) *url.URL {
+
+	return &url.URL{
+		Host:       fmt.Sprintf("%s:%d", config.Host, config.Port),


(URL creds)

Suggested change

Host: fmt.Sprintf("%s:%d", config.Host, config.Port),

User: util.URLUserPassword(config.Username, config.Password),

Host: fmt.Sprintf("%s:%d", config.Host, config.Port),

piksel · 2021-06-20T14:08:28Z

pkg/services/mqtt/mqtt_config.go

+	"strconv"
+
+	"github.com/containrrr/shoutrrr/pkg/format"
+	"github.com/containrrr/shoutrrr/pkg/types"


(URL creds)

Suggested change

"github.com/containrrr/shoutrrr/pkg/types"

"github.com/containrrr/shoutrrr/pkg/types"

"github.com/containrrr/shoutrrr/pkg/util"

piksel · 2021-06-20T14:09:21Z

pkg/services/mqtt/mqtt_config.go

+	Username   string `key:"username" default:"" desc:"username for auth"`
+	Password   string `key:"password" default:"" desc:"password for auth"`


(URL creds)

Suggested change

Username string `key:"username" default:"" desc:"username for auth"`

Password string `key:"password" default:"" desc:"password for auth"`

Username string `key:"username" default:"" desc:"username for auth" url:"User"`

Password string `key:"password" default:"" desc:"password for auth" url:"Pass"`

piksel · 2021-06-20T14:18:50Z

pkg/services/mqtt/mqtt_config.go

+// GetTLSConfig returns the configuration with the certificates for TLS
+func (config *Config) GetTLSConfig() *tls.Config {
+	certpool := x509.NewCertPool()
+	ca, err := ioutil.ReadFile("ca.crt")


Requiring the files to have this specific name and be in the working directory of the consuming app makes this really hard to use. Ideally, we should add some kind of generic way to add files/blobs to services (mostly for TLS, so perhaps even a specific TLS-cert interface...)
Right now, this should at least be configurable in the config struct.

Implemented in #185

piksel · 2021-06-20T14:24:05Z

pkg/services/mqtt/mqtt_config.go

+		RootCAs:            certpool,
+		ClientAuth:         tls.NoClientCert,
+		ClientCAs:          nil,
+		InsecureSkipVerify: true,


What is the point of loading custom certs when verification is disabled? This should absolutely not be the default.

piksel · 2021-06-20T14:29:53Z

docs/services/mqtt.md

+## Parameters Description
+
+- **Host** - MQTT broker server hostname or IP address (**Required**)  
+  Default: _empty_  
+  Aliases: `host`
+
+- **Port** - MQTT server port, common ones are 8883 for TCP/TLS and 1883 for TCP (**Required**)
+  Default: `8883`
+
+- **Topic** - Topic where the message is sent (**Required**)
+  Default: _empty_  
+  Aliases: `Topic`
+
+- **DisableTLS** - disable TLS/SSL Configurations  
+  Default: `false`
+
+- **ClientID** - The client identifier (ClientID) identifies each MQTT client that connects to an MQTT  
+  Default: _empty_
+  Aliases: `clientID`
+
+- **Username** - name of the sender to auth  
+  Default: _empty_
+  Aliases: `clientID`
+
+- **Password** - authentication password or hash  
+  Default: _empty_  
+  Aliases: `password`


This can auto-generated from the config tags

Suggested change

## Parameters Description

- **Host** - MQTT broker server hostname or IP address (**Required**)

Default: _empty_

Aliases: `host`

- **Port** - MQTT server port, common ones are 8883 for TCP/TLS and 1883 for TCP (**Required**)

Default: `8883`

- **Topic** - Topic where the message is sent (**Required**)

Default: _empty_

Aliases: `Topic`

- **DisableTLS** - disable TLS/SSL Configurations

Default: `false`

- **ClientID** - The client identifier (ClientID) identifies each MQTT client that connects to an MQTT

Default: _empty_

Aliases: `clientID`

- **Username** - name of the sender to auth

Default: _empty_

Aliases: `clientID`

- **Password** - authentication password or hash

Default: _empty_

Aliases: `password`

--8<-- "docs/services/mqtt/config.md"

piksel · 2021-06-20T14:31:13Z

docs/services/mqtt.md

+## Optional parameters
+
+You can optionally specify the **`disableTLS`**, **`clientID`**, **`username`** and **`password`** parameters in the URL:  
+_mqtt://**`host`**:**`port`**?topic=**`topic`**&disableTLS=true&clientID=**`clientID`**&username=**`username`**&password:**`password`**_


(URL creds)

Suggested change

_mqtt://**`host`**:**`port`**?topic=**`topic`**&disableTLS=true&clientID=**`clientID`**&username=**`username`**&password:**`password`**_

_mqtt://**`username`**:**`password`**@**`host`**:**`port`**?topic=**`topic`**&disableTLS=true&clientID=**`clientID`**&username=**`username`**&password:**`password`**_

piksel · 2021-06-20T14:34:20Z

pkg/services/mqtt/mqtt_config.go

+
+func (config *Config) setURL(resolver types.ConfigQueryResolver, url *url.URL) error {
+
+	config.Host = url.Hostname()


(URL creds)

Suggested change

config.Host = url.Hostname()

config.Host = url.Hostname()

config.Username = url.User.Username()

password, _ := url.User.Password()

config.Password = password

lokanx · 2021-12-14T22:00:39Z

Any progress on this?
Would be an awesome feature to have.

simskij · 2022-05-26T09:40:28Z

This has been untouched for so long that I wouldn't want to merge it. If anyone wants to pick it up again, or provide a completely different implementation, feel free to open a new PR. Closing.

feat(service): add mqtt service

9d5e307

Co-authored-by: Mexazonic <[email protected]>

fix(mqtt): fix static issuesin mqtt

7b4e3a9

piksel requested changes Mar 15, 2021

View reviewed changes

piksel marked this pull request as draft March 15, 2021 17:58

Eugeniosales and others added 5 commits April 6, 2021 22:01

feat(mqtt): add tls support to mqtt

cba79ac

Co-authored-by: Mexazonic <[email protected]>

docs(mqtt): add docs to mqtt service

333ad76

Co-authored-by: Mexazonic <[email protected]>

feat(mqtt): improve codacy issues

2243be0

Co-authored-by: Mexazonic <[email protected]>

docs(mqtt): fix overview page of docs

365f8c3

Merge branch 'main' into feature/mqtt-service

6bef5f6

Eugeniosales marked this pull request as ready for review April 7, 2021 02:15

feat(mqtt): fix config description

87b65b1

piksel self-requested a review April 7, 2021 07:59

feat(mqtt): improve mqtt description

8c17b96

Co-authored-by: Mexazonic <[email protected]>

simskij requested changes Apr 23, 2021

View reviewed changes

test(mqtt): add mqtt tests When TLS is enabled and disabled

5e1b0bb

When generating a config object with/without optional arguments

Eugeniosales added 2 commits May 17, 2021 17:28

conflict(mqtt): Remove conflict in overview doc and servicemap

fca4330

Merge branch 'main' into feature/mqtt-service

37c66fa

Eugeniosales requested a review from simskij May 17, 2021 20:36

piksel requested changes Jun 20, 2021

View reviewed changes

piksel added the Type: Enhancement New feature or request label Aug 31, 2021

simskij closed this May 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(service): add mqtt service #149

feat(service): add mqtt service #149

Eugeniosales commented Mar 15, 2021 •

edited

Loading

codecov bot commented Mar 15, 2021 •

edited

Loading

piksel left a comment •

edited

Loading

Eugeniosales commented Apr 7, 2021 •

edited

Loading

simskij left a comment

simskij commented May 9, 2021

Eugeniosales commented Jun 17, 2021

piksel commented Jun 17, 2021

piksel left a comment

piksel Jun 20, 2021

piksel Jun 20, 2021

piksel Jun 20, 2021

piksel Jun 20, 2021

piksel Aug 31, 2021

piksel Jun 20, 2021

piksel Jun 20, 2021

piksel Jun 20, 2021

piksel Jun 20, 2021

lokanx commented Dec 14, 2021

simskij commented May 26, 2022

	Host: fmt.Sprintf("%s:%d", config.Host, config.Port),
	User: util.URLUserPassword(config.Username, config.Password),
	Host: fmt.Sprintf("%s:%d", config.Host, config.Port),

	"github.com/containrrr/shoutrrr/pkg/types"
	"github.com/containrrr/shoutrrr/pkg/types"
	"github.com/containrrr/shoutrrr/pkg/util"

		Username string `key:"username" default:"" desc:"username for auth"`
		Password string `key:"password" default:"" desc:"password for auth"`

	_mqtt://`host`:`port`?topic=`topic`&disableTLS=true&clientID=`clientID`&username=`username`&password:`password`_
	_mqtt://`username`:`password`@`host`:`port`?topic=`topic`&disableTLS=true&clientID=`clientID`&username=`username`&password:`password`_


		func (config Config) setURL(resolver types.ConfigQueryResolver, url url.URL) error {

		config.Host = url.Hostname()

-	config.Host = url.Hostname()
+	config.Host = url.Hostname()
+	config.Username = url.User.Username()
+	password, _ := url.User.Password()
+	config.Password = password

feat(service): add mqtt service #149

feat(service): add mqtt service #149

Conversation

Eugeniosales commented Mar 15, 2021 • edited Loading

codecov bot commented Mar 15, 2021 • edited Loading

Codecov Report

piksel left a comment • edited Loading

Choose a reason for hiding this comment

Eugeniosales commented Apr 7, 2021 • edited Loading

simskij left a comment

Choose a reason for hiding this comment

simskij commented May 9, 2021

Eugeniosales commented Jun 17, 2021

piksel commented Jun 17, 2021

piksel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lokanx commented Dec 14, 2021

simskij commented May 26, 2022

Eugeniosales commented Mar 15, 2021 •

edited

Loading

codecov bot commented Mar 15, 2021 •

edited

Loading

piksel left a comment •

edited

Loading

Eugeniosales commented Apr 7, 2021 •

edited

Loading