From fad59da29d65010c6e10d639d46ef2274d445e72 Mon Sep 17 00:00:00 2001 From: Praveen raj Mani Date: Tue, 29 Jan 2019 15:00:15 +0530 Subject: [PATCH] `clientID` removed in the MQTT config (#7157) More than one client can't use the same clientID for MQTT connection. This causes problem in distributed deployments where config is shared across nodes, as each Minio instance tries to connect to MQTT using the same clientID. This commit removes the clientID field in config, and allows MQTT client to create random clientID for each node. --- cmd/admin-handlers_test.go | 1 - cmd/config-current_test.go | 2 +- cmd/config-versions.go | 2 +- docs/bucket/notifications/README.md | 8 ++++---- docs/config/config.sample.json | 1 - pkg/event/target/mqtt.go | 13 +++++++++---- 6 files changed, 15 insertions(+), 12 deletions(-) diff --git a/cmd/admin-handlers_test.go b/cmd/admin-handlers_test.go index c50620e6c..74c984f02 100644 --- a/cmd/admin-handlers_test.go +++ b/cmd/admin-handlers_test.go @@ -119,7 +119,6 @@ var ( "broker": "", "topic": "", "qos": 0, - "clientId": "", "username": "", "password": "", "reconnectInterval": 0, diff --git a/cmd/config-current_test.go b/cmd/config-current_test.go index b95bcc8d9..bcd32709f 100644 --- a/cmd/config-current_test.go +++ b/cmd/config-current_test.go @@ -233,7 +233,7 @@ func TestValidateConfig(t *testing.T) { {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "redis": { "1": { "enable": true, "format": "namespace", "address": "example.com:80", "password": "xxx", "key": "key1" } }}}`, true}, // Test 27 - Test MQTT - {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "mqtt": { "1": { "enable": true, "broker": "", "topic": "", "qos": 0, "clientId": "", "username": "", "password": "", "queueDir": ""}}}}`, false}, + {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "mqtt": { "1": { "enable": true, "broker": "", "topic": "", "qos": 0, "username": "", "password": "", "queueDir": ""}}}}`, false}, // Test 28 - Test NSQ {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "nsq": { "1": { "enable": true, "nsqdAddress": "", "topic": ""} }}}`, false}, diff --git a/cmd/config-versions.go b/cmd/config-versions.go index b5b24a206..1ae5d6b8c 100644 --- a/cmd/config-versions.go +++ b/cmd/config-versions.go @@ -893,7 +893,7 @@ type serverConfigV32 struct { } `json:"policy"` } -// serverConfigV33 is just like version '32', removes clientID from NATS and adds queueDir with MQTT. +// serverConfigV33 is just like version '32', removes clientID from NATS and MQTT, and adds queueDir with MQTT. type serverConfigV33 struct { quick.Config `json:"-"` // ignore interfaces diff --git a/docs/bucket/notifications/README.md b/docs/bucket/notifications/README.md index 6340b43e5..54f004167 100644 --- a/docs/bucket/notifications/README.md +++ b/docs/bucket/notifications/README.md @@ -160,7 +160,6 @@ The Minio server configuration file is stored on the backend in json format. The | `broker` | _string_ | (Required) MQTT server endpoint, e.g. `tcp://localhost:1883` | | `topic` | _string_ | (Required) Name of the MQTT topic to publish on, e.g. `minio` | | `qos` | _int_ | Set the Quality of Service Level | -| `clientId` | _string_ | Unique ID for the MQTT broker to identify Minio | | `username` | _string_ | Username to connect to the MQTT server (if required) | | `password` | _string_ | Password to connect to the MQTT server (if required) | | `queueDir` | _string_ | Persistent store for events when MQTT broker is offline | @@ -174,7 +173,6 @@ An example configuration for MQTT is shown below: "broker": "tcp://localhost:1883", "topic": "minio", "qos": 1, - "clientId": "minio", "username": "", "password": "", "queueDir": "" @@ -222,12 +220,14 @@ import paho.mqtt.client as mqtt def on_connect(client, userdata, flags, rc): print("Connected with result code "+str(rc)) - client.subscribe("minio") + # qos level is set to 1 + client.subscribe("minio", 1) def on_message(client, userdata, msg): print(msg.payload) -client = mqtt.Client() +# client_id is a randomly generated unique ID for the mqtt broker to identify the connection. +client = mqtt.Client(client_id="myclientid",clean_session=False) client.on_connect = on_connect client.on_message = on_message diff --git a/docs/config/config.sample.json b/docs/config/config.sample.json index 5ff81e55f..b7ee5a83e 100644 --- a/docs/config/config.sample.json +++ b/docs/config/config.sample.json @@ -82,7 +82,6 @@ "broker": "", "topic": "", "qos": 0, - "clientId": "", "username": "", "password": "", "reconnectInterval": 0, diff --git a/pkg/event/target/mqtt.go b/pkg/event/target/mqtt.go index 340d9043f..8c8d60ba1 100644 --- a/pkg/event/target/mqtt.go +++ b/pkg/event/target/mqtt.go @@ -36,7 +36,6 @@ type MQTTArgs struct { Broker xnet.URL `json:"broker"` Topic string `json:"topic"` QoS byte `json:"qos"` - ClientID string `json:"clientId"` User string `json:"username"` Password string `json:"password"` MaxReconnectInterval time.Duration `json:"reconnectInterval"` @@ -59,9 +58,15 @@ func (m MQTTArgs) Validate() error { default: return errors.New("unknown protocol in broker address") } - if m.QueueDir != "" && !filepath.IsAbs(m.QueueDir) { - return errors.New("queueDir path should be absolute") + if m.QueueDir != "" { + if !filepath.IsAbs(m.QueueDir) { + return errors.New("queueDir path should be absolute") + } + if m.QoS == 0 { + return errors.New("qos should be set to 1 or 2 if queueDir is set") + } } + return nil } @@ -120,7 +125,7 @@ func (target *MQTTTarget) Close() error { // NewMQTTTarget - creates new MQTT target. func NewMQTTTarget(id string, args MQTTArgs) (*MQTTTarget, error) { options := mqtt.NewClientOptions(). - SetClientID(args.ClientID). + SetClientID(""). SetCleanSession(true). SetUsername(args.User). SetPassword(args.Password).