search

ndelitski / rancher-alarms

Will kick your ass if found unhealthy service in Rancher environment
85 stars 20 forks source link

Anonymous Email Sending Failed #1

Closed jayhding closed 8 years ago

jayhding commented 8 years ago

Existing email sending schema requires auth method. It is necessary for public SMTP server but corporation internal SMTP can be used for anonymous email sending.

Therefore, default configuration will give following error

rancher-alarms_1 | [INFO]   2016-2-4 4:44:5:975       sending email notification to xxx.xxx@abc.com
rancher-alarms_1 | [ERROR]  2016-2-4 4:44:10:993      { [Error: Invalid login: 504 5.7.4 Unrecognized authentication type]
rancher-alarms_1 |   cause:
rancher-alarms_1 |    { [Error: Invalid login: 504 5.7.4 Unrecognized authentication type]
rancher-alarms_1 |      code: 'EAUTH',
rancher-alarms_1 |      response: '504 5.7.4 Unrecognized authentication type',
rancher-alarms_1 |      responseCode: 504 },
rancher-alarms_1 |   isOperational: true,
rancher-alarms_1 |   code: 'EAUTH',
rancher-alarms_1 |   response: '504 5.7.4 Unrecognized authentication type',
rancher-alarms_1 |   responseCode: 504 } Error: Invalid login: 504 5.7.4 Unrecognized authentication type
rancher-alarms_1 |     at SMTPConnection._formatError (/usr/src/app/node_modules/nodemailer/node_modules/nodemailer-smtp-transport/node_modules/smtp-connection/src/smtp-connection.js:384:15)
rancher-alarms_1 |     at SMTPConnection._actionAUTHComplete (/usr/src/app/node_modules/nodemailer/node_modules/nodemailer-smtp-transport/node_modules/smtp-connection/src/smtp-connection.js:878:30)
rancher-alarms_1 |     at SMTPConnection.<anonymous> (/usr/src/app/node_modules/nodemailer/node_modules/nodemailer-smtp-transport/node_modules/smtp-connection/src/smtp-connection.js:239:22)
rancher-alarms_1 |     at SMTPConnection._processResponse (/usr/src/app/node_modules/nodemailer/node_modules/nodemailer-smtp-transport/node_modules/smtp-connection/src/smtp-connection.js:516:16)
rancher-alarms_1 |     at SMTPConnection._onData (/usr/src/app/node_modules/nodemailer/node_modules/nodemailer-smtp-transport/node_modules/smtp-connection/src/smtp-connection.js:353:10)
rancher-alarms_1 |     at emitOne (events.js:77:13)
rancher-alarms_1 |     at Socket.emit (events.js:169:7)
rancher-alarms_1 |     at readableAddChunk (_stream_readable.js:146:16)
rancher-alarms_1 |     at Socket.Readable.push (_stream_readable.js:110:10)
rancher-alarms_1 |     at TCP.onread (net.js:523:20)

If the auth part is removed in email.es6 then it's fine

this._sender = promisifyAll(nodemailer.createTransport({
      port: smtp.port,
      host: smtp.host,
      from: smtp.from
    }));
}
flaccid commented 8 years ago

@ndelitski would you entertain a pull request for supporting noauth?

ndelitski commented 8 years ago

@flaccid surely! sry didn't see notifications earlier. we will definitely fix it soon!

flaccid commented 8 years ago

@ndelitski no worries, we also would like templating of the emails including html; I sent you an email, but basically just wondering if pull requests are ok or if you would rather do in your own way - templating for example is not a minor change.

ndelitski commented 8 years ago

PR with fixes are totally ok! but for features like email templating project needs significant refactoring as you said, it would be better if I do it by myself. I'll try release templating these week, it depends on my current workload

ndelitski commented 8 years ago

Mounting a template file(something like .hbs) into rancher-alarms container sounds good for you?

flaccid commented 8 years ago

No problem, we'll keep an eye out and then after these 2 are addressed see if we can PR any fixes that might be laying around. I think for feature changes that are not minor, we'll start with an issue first.

Re: templating, handlebars is fine but we do need a basic way to do this in compose directly in rancher without using volumes if possible though it might make sense that the rancher-alarms container can simply take a volumes-from of a data container with custom templates inside.

ndelitski commented 8 years ago

@JayHaoDing @flaccid just released a prepatch version image with noauth fix, please check it out with your smtp server docker pull ndelitski/rancher-alarms:0.1.1-0 and then I release 0.1.1 and update latest image

ndelitski commented 8 years ago

FYI code changes since 0.1.0 https://github.com/ndelitski/rancher-alarms/commit/aafeabb7e637c585f245b0863d1cf637bd70fce3

jayhding commented 8 years ago

@ndelitski test result showed the solution did not work. I did not configure ALARM_EMAIL_USER and ALARM_EMAIL_PASS but seems smtp.auth is still not NULL.

npm info it worked if it ends with ok
npm info using npm@2.14.7
npm info using node@v4.2.1
npm info prestart rancher-alarms@0.1.0
npm info start rancher-alarms@0.1.0

> rancher-alarms@0.1.0 start /usr/src/app
> node bin/rancher-alarms.js

[INFO]   2016-3-1 22:45:42:554     trying to compose config from env variables
[INFO]   2016-3-1 22:45:42:572     started with config:
{
    "rancher": {
        "address": "http://rancher.xxx.xxx/v1/projects/***",
        "auth": {
            "accessKey": "***",
            "secretKey": "***"
        },
        "projectId": "***"
    },
    "pollServicesInterval": 60000,
    "filter": [
        "***"
    ],
    "notifications": {
        "*": {
            "targets": {
                "email": {
                    "recipients": [
                        "xxx.xxx@***.***"
                    ]
                }
            },
            "healthcheck": {
                "pollInterval": 15000,
                "healthyThreshold": 3,
                "unhealthyThreshold": 4
            }
        }
    },
    "targets": {
        "email": {
            "smtp": {
                "from": "***@***",
                "auth": {},
                "host": "***",
                "secureConnection": true,
                "port": "25"
            }
        }
    }
}
[ERROR]  2016-3-1 22:45:43:139     { [AssertionError: undefined == true]
  name: 'AssertionError',
  actual: undefined,
  expected: true,
  operator: '==',
  message: 'undefined == true',
  generatedMessage: true } AssertionError: undefined == true
    at new EmailTarget (/usr/src/app/src/notifications/email.es6:18:7)
    at Function.init (/usr/src/app/src/notifications/target.es6:12:12)
    at ServiceStateMonitor.setupNotificationsTargets (/usr/src/app/src/monitor.es6:76:33)
    at new ServiceStateMonitor (/usr/src/app/src/monitor.es6:69:12)
    at initServiceMonitor$ (/usr/src/app/src/server.es6:52:12)
    at tryCatch (/usr/src/app/node_modules/babel-core/node_modules/regenerator/runtime.js:61:40)
    at GeneratorFunctionPrototype.invoke [as _invoke] (/usr/src/app/node_modules/babel-core/node_modules/regenerator/runtime.js:328:22)
    at GeneratorFunctionPrototype.prototype.(anonymous function) [as next] (/usr/src/app/node_modules/babel-core/node_modules/regenerator/runtime.js:94:21)
    at invoke (/usr/src/app/node_modules/babel-core/node_modules/regenerator/runtime.js:136:37)
    at callInvokeWithMethodAndArg (/usr/src/app/node_modules/babel-core/node_modules/regenerator/runtime.js:172:16)
    at previousPromise (/usr/src/app/node_modules/babel-core/node_modules/regenerator/runtime.js:194:19)
    at new Promise (/usr/src/app/node_modules/babel-core/node_modules/core-js/modules/es6.promise.js:197:7)
    at AsyncIterator.enqueue (/usr/src/app/node_modules/babel-core/node_modules/regenerator/runtime.js:193:13)
    at AsyncIterator.prototype.(anonymous function) [as next] (/usr/src/app/node_modules/babel-core/node_modules/regenerator/runtime.js:94:21)
    at Object.runtime.async (/usr/src/app/node_modules/babel-core/node_modules/regenerator/runtime.js:215:14)
    at initServiceMonitor (/usr/src/app/src/server.es6:51:22)

npm info rancher-alarms@0.1.0 Failed to exec start script

And just FYI, due to proxy issue we cannot use the rancher-alarms image directly so we have touse own Dockerfile and looks like

FROM node:4.2.1

RUN mkdir -p /usr/src/app
WORKDIR /usr/src/app

COPY package.json /usr/src/app/

RUN npm config set http-proxy http://x.x.x.x:8080
RUN npm config set https-proxy http://x.x.x.x:8080
RUN npm install

COPY . /usr/src/app

CMD [ "npm", "start" ]
ndelitski commented 8 years ago

@JayHaoDing no need to pass empty hash in auth parameter, current logic is – if auth present it should have credentials or it is invalid

jayhding commented 8 years ago

But it didn't work according to my test. I simpley did not set any value (not mention at all) for USER and PASS in docker compose file. What else shall I do? To me it looked like the auth{} did not make the if check be failed. 

Regards, Jay Ding

_____________________________

From: Nick Delitski notifications@github.com Sent: Wednesday, March 2, 2016 17:25 Subject: Re: [rancher-alarms] Anonymous Email Sending Failed (#1) To: ndelitski/rancher-alarms rancher-alarms@noreply.github.com Cc: Jay (Hao) Ding jay.dinghao@outlook.com

@JayHaoDing no need to pass empty hash in auth parameter, current logic is – if auth present it should have credentials or it is invalid

— Reply to this email directly or view it on GitHub.

flaccid commented 8 years ago

Perhaps support either method and document best method in readme?

ndelitski commented 8 years ago

@JayHaoDing you are right, I forgot a part where environment variables are composed into config. In a future refactoring I am planning to move config asserts into one place. Updated master, this should fix. will update readme later

jayhding commented 8 years ago

@ndelitski new changes have fixed this issue, thanks!

flaccid commented 8 years ago

Good to close (after merge to master)?

ndelitski commented 8 years ago

will close soon after updating docs

flaccid commented 8 years ago

Possible to wrap this one up soon?