Added Send mail functionality & Dockerized application #1

Merged
PeterSurda merged 11 commits from cis-kuldeep/influx-smtp-gateway:master into master 2022-02-21 06:41:14 +00:00
Collaborator

Added Send mail functionality & Dockerized application

Added Send mail functionality & Dockerized application
cis-kuldeep added 2 commits 2022-02-03 15:39:01 +00:00
PeterSurda reviewed 2022-02-04 07:25:46 +00:00
@ -0,0 +9,4 @@
ports:
- 8081:8081
env_file:
- ./config.ini
Owner

env_file is for shell variables, it won't work in this case. Instead we need a simple entrypoint.sh which will for example format the config.ini

`env_file` is for shell variables, it won't work in this case. Instead we need a simple `entrypoint.sh` which will for example format the `config.ini`
PeterSurda marked this conversation as resolved
PeterSurda reviewed 2022-02-04 07:26:15 +00:00
@ -0,0 +12,4 @@
- ./config.ini
volumes:
mailsend_server:
Owner

we don't need a volume, the app will already be inside the dockerfile

we don't need a volume, the app will already be inside the dockerfile
PeterSurda marked this conversation as resolved
PeterSurda reviewed 2022-02-04 07:27:56 +00:00
@ -0,0 +3,4 @@
services:
web:
build: .
command: python main.py
Owner

perhaps this should be inside the Dockerfile

perhaps this should be inside the `Dockerfile`
PeterSurda marked this conversation as resolved
PeterSurda requested changes 2022-02-04 07:28:43 +00:00
PeterSurda left a comment
Owner
  • remove unused stuff
  • slight redesign in how config is passed into the container

In general it's in the direction I wanted.

- remove unused stuff - slight redesign in how config is passed into the container In general it's in the direction I wanted.
cis-kuldeep added 1 commit 2022-02-04 17:18:10 +00:00
PeterSurda requested changes 2022-02-07 06:36:56 +00:00
@ -0,0 +17,4 @@
import cherrypy
PATH = os.path.dirname(os.path.abspath(__file__))
Owner

this should go under __main__

this should go under `__main__`
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +24,4 @@
TO_MAIL = CONFIG["app"].get("to_mail")
FROM_MAIL = CONFIG["app"].get("from_mail")
FROM_MAIL_PASSWORD = CONFIG["app"].get("from_mail_password")
print("TO_MAIL: ", TO_MAIL)
Owner

debug we don't need

debug we don't need
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +29,4 @@
print("FROM_MAIL_PASSWORD: ", FROM_MAIL_PASSWORD)
class CloudInitRequest:
Owner

this whole class we don't need

this whole class we don't need
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +89,4 @@
self.hostinfo = (self.remoteip, )
class CloudInitApp:
Owner

different name

different name
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +95,4 @@
"""
@staticmethod
def _content_type(data):
Owner

remove

remove
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +146,4 @@
ENGINE = cherrypy.engine
CURRENT_DIR = os.path.dirname(os.path.abspath(__file__))
CONFIG = {
Owner

remove CONFIG

remove `CONFIG`
PeterSurda marked this conversation as resolved
cis-kuldeep added 1 commit 2022-02-07 15:29:17 +00:00
cis-kuldeep added 1 commit 2022-02-08 13:07:22 +00:00
PeterSurda requested changes 2022-02-09 07:11:55 +00:00
README.md Outdated
@ -4,1 +3,4 @@
SMTP gateway accessible from InfluxDB for sending alerts.
# create .env file with following parameters
Owner

remove as it's matched with docker service

remove as it's matched with docker service
PeterSurda marked this conversation as resolved
README.md Outdated
@ -5,0 +5,4 @@
# create .env file with following parameters
server_host = 0.0.0.0
server_port = 8081
Owner

add smtp server name

add smtp server name
PeterSurda marked this conversation as resolved
@ -0,0 +4,4 @@
web:
build: .
ports:
- 8081:8081
Owner

If we only use this internally inside a docker service, we should use expose, not port.

If we only use this internally inside a docker service, we should use `expose`, not `port`.
PeterSurda marked this conversation as resolved
@ -0,0 +7,4 @@
ports:
- 8081:8081
env_file:
- ./config.ini
Owner

wrong file

wrong file
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +1,77 @@
#!/usr/bin/env python3
"""
Serve cloud init files
Owner

change docstring

change docstring
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +13,4 @@
import cherrypy
class CloudInitApp:
Owner

class name

class name
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +15,4 @@
class CloudInitApp:
"""
Serve cloud init files
Owner

docstring

docstring
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +18,4 @@
import cherrypy
PATH = os.path.dirname(os.path.abspath(__file__))
CONFIG = configparser.ConfigParser()
Owner

remove CONFIG and only use environment variables
if some variable missing, display a helpful error message and quit with non-zero error code

remove `CONFIG` and only use environment variables if some variable missing, display a helpful error message and quit with non-zero error code
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +20,4 @@
PATH = os.path.dirname(os.path.abspath(__file__))
CONFIG = configparser.ConfigParser()
CONFIG.read(os.path.join(PATH, "config.ini"))
Owner

override from env variables

override from env variables
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +26,4 @@
req_body = json.loads(rawbody)
subject = req_body['subject']
body = req_body['body']
client = smtplib.SMTP('smtp.gmail.com')
Owner

port 587

port 587
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +45,4 @@
@cherrypy.expose
def send_mail(self):
"""
v1 api endpoint user-data
Owner

docstring

docstring
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +59,4 @@
TO_MAIL = os.environ["to_mail"]
FROM_MAIL = os.environ["from_mail"]
FROM_MAIL_PASSWORD = os.environ["from_mail_password"]
except: # noqa:E722
Owner

except KeyError

`except KeyError`
PeterSurda marked this conversation as resolved
cis-kuldeep added 1 commit 2022-02-09 17:35:02 +00:00
PeterSurda reviewed 2022-02-10 06:59:35 +00:00
Dockerfile Outdated
@ -0,0 +15,4 @@
# copy project
COPY . .
ENTRYPOINT ["/usr/src/app/entrypoint.sh"]
Owner

run as non-root

run as non-root
PeterSurda marked this conversation as resolved
PeterSurda reviewed 2022-02-10 07:00:20 +00:00
@ -0,0 +1,9 @@
version: '3.8'
services:
web:
Owner

smtp-gateway:

`smtp-gateway:`
PeterSurda marked this conversation as resolved
PeterSurda requested changes 2022-02-10 07:04:47 +00:00
PeterSurda left a comment
Owner
  • use constants
  • missing SMTP port
- use constants - missing SMTP port
cis-kuldeep added 1 commit 2022-02-10 15:46:19 +00:00
PeterSurda requested changes 2022-02-11 10:37:57 +00:00
Dockerfile Outdated
@ -0,0 +15,4 @@
# copy project
COPY . .
ENTRYPOINT ["/usr/src/app/entrypoint.sh", "--user"]
Owner

this doesn't do anything. The Dockerfile should create a non-privileged user and then just before ENTRYPOINT have a corresponding USER instruction.

this doesn't do anything. The `Dockerfile` should create a non-privileged user and then just before `ENTRYPOINT` have a corresponding `USER` instruction.
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +54,4 @@
if __name__ == "__main__":
try:
SERVER_HOST = os.environ["server_host"]
SERVER_PORT = os.environ["server_port"]
Owner

SERVER_PORT should default to 587 and be an integer.

`SERVER_PORT` should default to `587` and be an integer.
PeterSurda marked this conversation as resolved
cis-kuldeep added 1 commit 2022-02-11 17:15:55 +00:00
PeterSurda reviewed 2022-02-14 10:25:31 +00:00
Dockerfile Outdated
@ -0,0 +14,4 @@
RUN pip install -r requirements.txt
# add user
RUN adduser --disabled-password --gecos '' newuser
Owner

let's call it something else

let's call it something else
PeterSurda marked this conversation as resolved
PeterSurda requested changes 2022-02-15 07:02:52 +00:00
main.py Outdated
@ -0,0 +38,4 @@
client.sendmail(msg['From'], msg['To'], msg.as_string())
client.quit()
return "mail sent successfully"
except Exception as e:
Owner

ideally an exception per line rather than together, then you can also use more specific exceptions.

ideally an exception per line rather than together, then you can also use more specific exceptions.
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +39,4 @@
client.quit()
return "mail sent successfully"
except Exception as e:
return "some error: {}".format(e)
Owner
  • return an error code (in addition to error text) if there's an error
  • add a small delay if there's an error before returning, say 0.2s, and try to avoid sleep, instead look for a way to do this asynchronously
- return an error code (in addition to error text) if there's an error - add a small delay if there's an error before returning, say 0.2s, and try to avoid `sleep`, instead look for a way to do this asynchronously
PeterSurda marked this conversation as resolved
cis-kuldeep added 1 commit 2022-02-15 16:48:48 +00:00
PeterSurda requested changes 2022-02-16 06:36:49 +00:00
main.py Outdated
@ -0,0 +64,4 @@
client.starttls()
client.ehlo()
client.login(msg["From"], FROM_MAIL_PASSWORD)
except Exception as e:
Owner

except smtplib.SMTPException

`except smtplib.SMTPException`
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +70,4 @@
return {"status": 500, "message": "some error in from mail "
"login: {}".format(e)}
try:
Owner

merge with previous try/except

merge with previous `try`/`except`
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +72,4 @@
try:
client.sendmail(msg['From'], msg['To'], msg.as_string())
client.quit()
Owner

quit() should be in finally:

`quit()` should be in `finally:`
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +106,4 @@
SMTP_SERVER_HOST = "smtp.gmail.com"
TO_MAIL = "test111@mailinator.com"
FROM_MAIL = "cis.dev393@gmail.com"
FROM_MAIL_PASSWORD = "akeel@123#"
Owner

live data shouldn't be here, squash to get rid of it

live data shouldn't be here, squash to get rid of it
PeterSurda marked this conversation as resolved
cis-kuldeep force-pushed master from 4cca0e8aa9 to e7fe463b82 2022-02-16 07:46:22 +00:00 Compare
cis-kuldeep added 1 commit 2022-02-16 07:50:12 +00:00
PeterSurda requested changes 2022-02-17 04:35:30 +00:00
main.py Outdated
@ -0,0 +14,4 @@
import cherrypy
import logging
logging.basicConfig(filename='app.log', filemode='w',
Owner

should log to stdout or stderr (research which one is more appropriate)

should log to stdout or stderr (research which one is more appropriate)
PeterSurda marked this conversation as resolved
main.py Outdated
@ -0,0 +49,4 @@
try:
client = smtplib.SMTP(host=SMTP_SERVER_HOST, port=SMTP_SERVER_PORT)
except Exception as e:
Owner

except (SMTPConnectionError, TimeoutError) as e:

`except (SMTPConnectionError, TimeoutError) as e:`
PeterSurda marked this conversation as resolved
Owner

Recipient should be possible to specify inside the JSON

Recipient should be possible to specify inside the JSON
cis-kuldeep added 1 commit 2022-02-17 17:02:11 +00:00
cis-kuldeep added 1 commit 2022-02-18 16:11:40 +00:00
PeterSurda approved these changes 2022-02-21 06:41:06 +00:00
PeterSurda merged commit f4fc89c111 into master 2022-02-21 06:41:14 +00:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Sysdeploy/influx-smtp-gateway#1
No description provided.