Beginner web scraper for Nagios

Question

I am attempting to learn Python. It was suggested to me to try a web scraper, so I thought to get myself to look at multiple Nagios instances. I have not programmed in Python before, but learned from other code reviews to adapt my standards to PEP8. The online checker says the code passes.

from bs4 import BeautifulSoup
import requests
from requests.auth import HTTPDigestAuth
from requests.auth import HTTPBasicAuth
import urllib.request
import time

allNagiosInfo = [
    ('http://example.com/nagios', 'user', 'password', 'Basic'),
    ('https://network2.example.com/nagios', 'user', 'password', 'Digest'),
    ('https://anothernetwork.example.com/nagios', 'user', 'password', 'Basic')]

for nagiosEntry in allNagiosInfo:
    nagiosBaseURL = nagiosEntry[0]
    nagiosUsername = nagiosEntry[1]
    nagiosPassword = nagiosEntry[2]
    nagiosAuthType = nagiosEntry[3]
    # print("Current values: {}\t{}\t{}\t{}".format(
    #      nagiosBaseURL, nagiosUsername, nagiosPassword, nagiosAuthType))

    if nagiosAuthType == "Basic":
        response = requests.get(nagiosBaseURL + "/cgi-bin/status.cgi?host=all",
                                auth=HTTPBasicAuth(nagiosUsername,
                                                   nagiosPassword))
    if nagiosAuthType == "Digest":
        response = requests.get(nagiosBaseURL + "/cgi-bin/status.cgi?host=all",
                                auth=HTTPDigestAuth(nagiosUsername,
                                                    nagiosPassword))

    html = BeautifulSoup(response.text, "html.parser")
    for i, items in enumerate(html.select('td')):
        if i == 3:
            hostsAll = items.text.split('\n')
            hostsUp = hostsAll[12]
            hostsDown = hostsAll[13]
            hostsUnreachable = hostsAll[14]
            hostsPending = hostsAll[15]
            hostsProblems = hostsAll[24]
            hostsTypes = hostsAll[25]
        if i == 12:
            serviceAll = items.text.split('\n')
            serviceOK = serviceAll[13]
            serviceWarning = serviceAll[14]
            serviceUnknown = serviceAll[15]
            serviceCritical = serviceAll[16]
            serviceProblems = serviceAll[26]
            serviceTypes = serviceAll[27]
        # print(i, items.text) ## To get the index and text
    print("Nagios Server at {}@{}:".format(nagiosUsername, nagiosBaseURL))
    print("    Hosts")
    print("\tUp          - {}".format(hostsUp))
    print("\tDown        - {}".format(hostsDown))
    print("\tUnreachable - {}".format(hostsUnreachable))
    print("\tPending     - {}".format(hostsPending))
    print("\tProblems    - {}".format(hostsProblems))
    print("\tTypes       - {}".format(hostsTypes))

    print("    Services")
    print("\tOK          - {}".format(serviceOK))
    print("\tWarning     - {}".format(serviceWarning))
    print("\tUnknown     - {}".format(serviceUnknown))
    print("\tCritical    - {}".format(serviceCritical))
    print("\tProblems    - {}".format(serviceProblems))
    print("\tTypes       - {}".format(serviceTypes))
    # print("Request returned:\n\n{}".format(html.text))
    # To get the full request

Currently, it outputs my three instances properly:

Nagios Server at user@http://example.com/nagios:
    Hosts
        Up          - 24
        Down        - 0
        Unreachable - 0
        Pending     - 0
        Problems    - 0
        Types       - 24
    Services
        OK          - 142
        Warning     - 0
        Unknown     - 0
        Critical    - 0
        Problems    - 0
        Types       - 142
Nagios Server at user@https://network2.example.com/nagios:
    Hosts
        Up          - 3
        Down        - 0
        Unreachable - 0
        Pending     - 0
        Problems    - 0
        Types       - 3
    Services
        OK          - 28
        Warning     - 0
        Unknown     - 0
        Critical    - 0
        Problems    - 0
        Types       - 28
Nagios Server at user@https://anothernetwork.example.com/nagios:
    Hosts
        Up          - 56
        Down        - 0
        Unreachable - 0
        Pending     - 0
        Problems    - 0
        Types       - 56
    Services
        OK          - 130
        Warning     - 0
        Unknown     - 0
        Critical    - 0
        Problems    - 0
        Types       - 130

I understand I do not have error checking (i.e. is the username/password correct? Auth Type correct? Server URL right?), so that will be my next thing to learn. I also understand I should not have variables (especially with passwords) hard-coded into the code, but I do not know what is the "proper" way of dealing with secure variables.

Answer 1

First of all, as per PEP8 I'd recommend you follow the best practices.

Naming conventions

• allNagiosInfo -> this is most likely a constant, so: ALL_NAGIOS_INFO
• nagiosEntry, nagiosBaseURL, nagiosUsername, nagiosPassword, nagiosAuthType -> nagios_entry, nagios_base_url, nagios_username, nagios_password, nagios_auth_type.

And so on with the naming, you got the idea.

Summary:

Variable names should be lowercase, with words separated by underscores as necessary to improve readability.

Imports

Imports should be grouped in the following order:

• Standard library imports.
• Related third party imports.
• Local application/library specific imports.
• You should put a blank line between each group of imports.

Also, don't import modules that you're not using (e.g: time, urllib). So, that means you could rewrite your imports as follows:

import requests
from requests.auth import HTTPBasicAuth, HTTPDigestAuth
from bs4 import BeautifulSoup

---

This:

nagios_base_url = nagios_entry[0]
nagios_username = nagios_entry[1]
nagios_password = nagios_entry[2]
nagios_auth_type = nagios_entry[3]

Can be rewritten as:

nagios_base_url, nagios_username, nagios_password, nagios_auth_type = nagios_entry

Since all the requests that you're doing are suffixed with the same url, I'd just define that before the ifs:

# ...
for nagios_entry in ALL_NAGIOS_INFO:
    nagios_base_url, nagios_username, nagios_password, nagios_auth_type = nagios_entry
    full_url = "{}/cgi-bin/status.cgi?host=all".format(nagios_base_url)

    if nagios_auth_type == "Basic":
        response = requests.get(full_url, auth=HTTPBasicAuth(nagios_username, nagios_password))

    if nagios_auth_type == "Digest":
        response = requests.get(full_url, auth=HTTPDigestAuth(nagios_username, nagios_password))

You have way too many prints which can be rewritten as a single print:

print("""Nagios Server at {}@{}: 

Hosts:

    Up          - {}
    Down        - {}
    Unreachable - {}
    Pending     - {}
    Problems    - {}
    Types       - {}

Services:

    OK          - {}
    Warning     - {}
    Unknown     - {}
    Critical    - {}
    Problems    - {}
    Types       - {}
""".format(
        nagios_username, nagios_base_url, hosts_up, hosts_down, hosts_unreachable, hosts_pending,
        hosts_problems, hosts_types, service_ok, service_warning, service_unknown, service_critical,
        service_problems, service_types
    )
)

You can also look at textwrap.dedent if you want to play around with the \ts.

---

Security

As you said, it's not recommended to keep your passwords in the code. There's no perfect solution for this, but we can enumerate some of the most used ones:

• config files which have to be added to .gitignore so they get excluded from the repo
• environment variables (mostly used on Unix based distros)
• some powerful hashing technique to generate a master password (master password -> secure key-> encrypt data by the key)

---

Scraping

When scraping things, you can usualy go a specific element by using an xpath or a selector. I usually use the lxml module when I have to quickly put together something because it allows me to use xpaths (though bs might as well offer that - didn't actually play with that for a while now)

Example for lxml usage

from lxml import html
import requests

res = requests.get(some_url)
tree = html.fromstring(res.content)
tds = tree.xpath('//table//td[@class="plaintext"]/text()[normalize-space()]')

---

Now, your code is not very portable and if you plan on adding more servers, you'll find yourself having a hard time integrating those as well. The first thing that I'd do is to find the proper data structure for my data. In your scenario, a dictionary looks like the best choice out there:

NAGIOS_DATA = {
    'http://example.com/nagios/cgi-bin/status.cgi?host=all': {
        'user': 'user',
        'password': 'password',
        'auth_type': 'Basic'
    },
    'https://network2.example.com/nagios/cgi-bin/status.cgi?host=all': {
        'user': 'user',
        'password': 'password',
        'auth_type': 'Digest'
    },
    'https://anothernetwork.example.com/nagios/cgi-bin/status.cgi?host=all': {
        'user': 'user',
        'password': 'password',
        'auth_type': 'Basic'
    },
}

INFO: don't forget that the password can be defined in a config file and imported here

Now, that's debatable if it's the best structure that you can use because I don't know the entire logic of your app, but it'll do for now.

Using the above, we can now do:

import requests
from requests.auth import HTTPBasicAuth, HTTPDigestAuth
from bs4 import BeautifulSoup


NAGIOS_DATA = {
    'http://example.com/nagios/cgi-bin/status.cgi?host=all': {
        'user': 'user',
        'password': 'password',
        'auth_type': 'Basic'
    },
    'https://network2.example.com/nagios/cgi-bin/status.cgi?host=all': {
        'user': 'user',
        'password': 'password',
        'auth_type': 'Digest'
    },
    'https://anothernetwork.example.com/nagios/cgi-bin/status.cgi?host=all': {
        'user': 'user',
        'password': 'password',
        'auth_type': 'Basic'
    },
}


def get_url_response(url, user, password, auth_type):
    """Get the response from a URL.

    Args:
        url (str): Nagios url
        user (str): Nagios username
        password (str): Nagios password
        auth_type (str): Nagios auth_type

    Returns: Response object
    """

    if auth_type == "Basic":
        return requests.get(url, auth=HTTPBasicAuth(user, password)) 
    return requests.get(url, auth=HTTPDigestAuth(user, password))


def main():
    """
    Main entry to the program.
    """

    for url, auth_data in NAGIOS_DATA.items():
        user, password, auth_type = auth_data["user"], auth_data["password"], auth_data["auth_type"]
        response = get_url_response(url, user, password, auth_type)

        if response.status_code == 200:
            html = BeautifulSoup(response.text, "html.parser")
            # ... and so on


if __name__ == '__main__':
    main()

I haven't finished the whole review because I want you to go ahead and do the rest. Some things that I've added to the program:

• functions: writing functions will let you reuse the functionality they contain at a later point + it's easier to test the code inside them. Homework: could you think of a way of splitting them even further?
• docstrings: having docstrings within a function will tell others (and most important, the future-you) what that function does.