Nested Classes benutzen?

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
peterpanda
User
Beiträge: 3
Registriert: Donnerstag 6. Oktober 2022, 08:30

Guten Morgen!
leider habe ich noch nicht viel Erfahrung mit objektorientierter Programmierung in Python.

Ich möchte eine Klasse zur Anlage von Firewall-Regeln schreiben (bislang haben wir alles nur in Funktionen und es wird langsam chatoisch).
Die Infos dazu stehen bereits in einem dict (rule_dict).Wir haben allerdings mehrere Firewall-Vendoren (im Beispiel Cisco und Fortinet), die mit verschiedenen Varianten angesteuert werden (ansible und paramiko) und basierend auf verschiedenen Dingen haben diese auch verschiedene Credentials.
Es gibt aber auch viele Gemeinsamkeiten, weshalb ich nicht einfach pro Vendor eine Klasse schreiben und einzelne Zeilen Copy-Pasten will.
Im Endeffekt ist die Idee, dass eine eierlegende Wollmilchsau unter den Klassen geben soll und diese nach Bedarf die Klassen der anderen Vendoren "hinzuzieht".

Generell erst einmal die Frage: Ist das der "richtige" Ansatz oder sollte ich einfach direkt unter der Hauptklasse (FirewallRules) Methoden für die einzelnen Vendoren anlegen? Meine Sorge war, dass das zu unübersichtlich wird.
Wenn ich aber "Unterklassen" habe, habe ich wieder Probleme an die Parent-Class Dinge zu übergeben.
Was ist der python-Weg für mein Problem?
Ich danke euch schonmal für eure Antworten :)

Code: Alles auswählen

class FirewallRules:
    def __init__(self, rule_dict):
        self.vendor = self._findVendor()
        self.credentials = self._get_credentials()
        if self.vendor == "Cisco":  # Ja, switch case wär cooler, aber ich muss aus Gründen python 3.6 nutzen.
            # Cisco-Klasse verwenden
        elif self.vendor == "Fortinet":
            # Fortinet-Klasse verwenden
    
    def _findVendor(self):
        # Hier soll der Vendor herausgefunden gewerden
        Vendor = "Cisco"
        
        return Vendor
    
    def _get_credentials(self):
        # Basierend auf verschiedenen Variablen sollen hier Zugangsdaten rausgefunden werden
        return ["user123", "sicherespasswort123"]

class Vendor_Cisco:
    def __init__(self):
        pass
    
    def execute(self):
        commands = "" # Hier sollen CLI Commands rausgelassen werden und per paramiko an die Firewall geschickt werden
        return commands

class Vendor_Fortinet:
    def __init__(self):
        pass
    
    def execute(self):
        # Hier soll ein Ansible-Plabook ausgeführt und das Ergebnis zurückgegeben werden
        playbook_status = "ok"
        return playbook_status
    


rule_dict = {
    "hostname": "tolle-firewall",
    "rules":[
        {
            "src_addr": ["1.2.3.4"],
            "dst_addr": ["2.3.4.5"],
            "services": ["TCP 22"],
            "comment": "total wichtige Regel"
        },
        {
            "src_addr": ["4.5.6.7"],
            "dst_addr": ["8.9.10.11"],
            "services": ["TCP 22"],
            "comment": "total wichtige Regel"
        },
    ]
}

firewall_ticket1 = FirewallRules(rule_dict)

# in etwa soll das so funktionieren
print(firewall_ticket1.execute)
Benutzeravatar
bwbg
User
Beiträge: 407
Registriert: Mittwoch 23. Januar 2008, 13:35

Nur weil Funktionen/Prozeduren bei dir unübersichtlich werden, heißt es nicht, dass Klassen das Allheilmittel sind. Einen kompletten Reim kann ich mir aus deinem Beispiel nicht machen. "Eierlegende Wollmilchsäue unter den Klassen" (aka Gottklassen) sind ein Anti-Pattern (sprich: Macht man nicht!).

Du bringst hier auch einige Begrifflichkeiten durcheinander. Eine "nested class" (verschachtelte Klasse) ist eine Klasse, welche innerhalb einer Klasse definiert ist (hier `Bar`):

Nestes Classes

Code: Alles auswählen

class Foo:
    class Bar:
        def barfoos(self) -> str:
            return 'fazbuzz'

    def fuz(self) -> None:
        pass
Subclass / Unterklasse

Eine "subclass" (abgeleitete Klasse, Unterklasse) erbt von ihrer Elternklasse (parent):

Code: Alles auswählen

class Foo:
    def fuz(self) -> None:
        pass
    
class Bar(Foo):
    def barfoos(self) -> str:
        return 'fazbuzz'
Hier entsteht eine "is-a" (ist ein(e)) Beziehung. Bar ist ein Foo oder alle Objekte vom Typ Bar sind auch Objekte vom Typ Foo.

Sonstiges

Folgendes klingt schräg:
peterpanda hat geschrieben:

Code: Alles auswählen

    firewall_ticket1 = FirewallRules(rule_dict)
Lies: "Erzeuge eine Instanz vom Typ "Sammlung von Firewall-Regeln", verwende dabei die Abbildung der Regeln und gib dieser den Namen Firewall-Ticket".
Ergo: "Ein Firewall-Ticket ist Element der Menge aller Sammlungen von Firewall-Regeln."

Don't copy be happy

Darüber hinaus verweise ich auf __blackjack__'s Ausführungen zu Namensgebungen in nahezu jedem Thread.
"Du bist der Messias! Und ich muss es wissen, denn ich bin schon einigen gefolgt!"
Benutzeravatar
__blackjack__
User
Beiträge: 13003
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@peterpanda: „Copy-Pasten“ klingt grundsätzlich erst einmal falsch, das macht man nicht. Ein Weg das zu vermeiden sind ja unter anderem Klassen und Vererbung um Gemeinsamkeiten nicht in jeder Klasse noch mal zu haben, sondern in einer Basisklasse *einmal* unterzubringen.

Grunddatentypen haben in Namen nichts verloren und man sollte auch keine Namen nummerieren. Schon gar nicht sinnlose 1en irgendwo dran hängen.

Letztlich ist aus dem Beispiel nicht ersichtlich was da überhaupt passieren soll, weil nirgends die Abhängigkeiten von den Daten ersichtlich sind, weil Du da nirgends irgendwelche Daten weitergibst, und nur Kommentare da stehen was dort auf magische Weise mit irgendwelchen Daten die wir nicht kennen gemacht werden soll.

`FirewallRules` hat keine `execute()`-Methode‽

Wenn die vendorspezifischen Klassen alle nur aus einer `__init__()` und einer Methode bestehen, dann sind das am Ende vielleicht einfach nur unnötig komplex geschriebene Funktionen‽ Falls damit Argumente für die ursprüngliche Funktion aufgesplittet werden sollen in welche die man immer gleich übergibt, und welche die sich bei jedem Aufruf ändern, könnte man eventuell auch `functools.partial()` einsetzen statt eine fast-nicht-wirklich-Klasse zu schreiben.

Da wo Du lieber ein ``switch``/``case`` verwenden willst macht das eigentlich keinen Sinn, da würde man einfach ein Wörterbuch anlegen das Vendornamen auf Klassen abbildet. Und das ging auch in Python 2.7 schon.

Die Namensschreibweisen entsprechen teilweise nicht der Python-Konvention.

Grundsätzlich würde ich keine Funktionen in Klassen stecken, nur um irgendwie Klassen zu haben. Im Fokus ist normalerweise erst einmal der Zustand, also welchen Zustand kapselt die Klasse eigentlich. Das ist aus Deinem Beispielcode überhaupt nicht ersichtlich weil die `__init__()`-Methoden entweder leer sind, oder Attribute aus Werten erstellen wo überhaupt nicht klar ist wo die herkommen und wo/wie die verwendet werden.

Eine weitere Faustregel für Klassen ist bei mir, dass die `__init__()` so einfach wie möglich ist. Am besten nur Zuweisung von übergebenen Werten an Attribute, eventuell noch Werte prüfen, in seltenen Fällen einfache Konvertierungen. Das macht das Testen wesentlich einfacher und man kann sich Arbeit ersparen mit Bibliotheken wie `attrs`. Alternative ”Konstruktoren” also wo mehr Arbeit gemacht wird schreibe ich als Klassenmethoden.

Ohne weitere Informationen wäre das mit Funktionen ja einfach nur das hier:

Code: Alles auswählen

HOST_DATA = {
    "hostname": "tolle-firewall",
    "rules": [
        {
            "src_addr": ["1.2.3.4"],
            "dst_addr": ["2.3.4.5"],
            "services": ["TCP 22"],
            "comment": "total wichtige Regel",
        },
        {
            "src_addr": ["4.5.6.7"],
            "dst_addr": ["8.9.10.11"],
            "services": ["TCP 22"],
            "comment": "total wichtige Regel",
        },
    ],
}


def find_vendor(host_data):
    return ...


def execute_rules_for_cisco(host_data):
    # Paramiko stuff.
    ...


def execute_rules_for_fortinet(host_data):
    # Ansible-Plabook stuff.
    ...


VENDORNAME_TO_EXECUTE = {
    "Cisco": execute_rules_for_cisco,
    "Fortinet": execute_rules_for_fortinet,
}


def execute_rules(host_data):
    VENDORNAME_TO_EXECUTE[find_vendor(host_data)](host_data)


def main():
    execute_rules(HOST_DATA)


if __name__ == "__main__":
    main()
Die Credentials habe ich weg gelassen, weil aus Deinem Beispiel der Datenfluss nicht ersichtlich wurde wie die überhaupt zustande kommen und wo sie verwendet werden. Und eventuell braucht auch nicht jede Funktion die komplette `host_data`-Struktur. Um den Vendor herauszufinden reicht beispielsweise vielleicht auch der Hostname aus der Struktur.

Die Datenstruktur könnte man vielleicht schon mal in Datenklassen verpacken, denn Wörterbücher die immer einen festen Satz von Schlüsseln haben, sind in der Regel eher Objekte als beliebige Abbildungen.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
Sirius3
User
Beiträge: 17710
Registriert: Sonntag 21. Oktober 2012, 17:20

Wenn in __init__ andere Methoden aufgerufen werden, dann ist das vom Design her nicht so optimal, denn das würde ja bedeuten, dass manche Methoden nur für die Initialisierung da sind, also eigentlich gar nicht auf der fertigen Instanz funktionieren.

Warum sollen die Firewall-Regeln etwas über die Credentials wissen? Das ist irgendwie verquer. Ich würde erwarten, dass es verschiedene Vendoren-Klassen gibt, denen man dann sagen kann, dass eine Firewall-Regel eingespielt werden soll. Also genau umgedreht zu Deinem Vorgehen.

Das Copy-Pasten hört sich so an, als ob Du nach Vererbung suchst, eine AllgemeineVendor-Klasse von denen dann die speziellen abgeleitet werden.
Datentypen haben in Variablennamen nichts zu suchen, hostname und rules wären eher zwei Argumente an eine Klasse, als das in einem Wörterbuch abzubilden.
imonbln
User
Beiträge: 149
Registriert: Freitag 3. Dezember 2021, 17:07

Ich finde dein Klassendesign fragwürdig, bei den Fragmenten, die ich sehe.

Die Klasse FirewallRules ist vom OOP Standpunkt zu mächtig, sie heißt ruleset versucht aber auch herauszubekommen welche Art von Firewall vorliegt, wenn hier wissen über die Firewall gebraucht wird, was ich bezweifle, sollte das Firewall-Objekt als Komposition übergeben werden.

Außerdem glaube ich, dass die Methode „_get_credentials“ kein Member von FirewallRules sein sollte. Vermutlich sind die Credentials abhängig von Vendor, vielleicht auch von der Firewall Entität (welche du noch gar nicht moduliert hast). Erstmal würde ich die Methode in den Vendor Klassen vermuten.

Das Herausbekommen welchen Firewall Hersteller du hast, sollte eine Factory für dich machen, welche das passende Objekt Instantiiert. Gegebenenfalls, macht eine abstrakte Basisklasse für den Vendor Sinn, welche dann für Cisco und Fortinet implementiert wird. Meine Vermutung ist, dass es sinnvoll sein könnte, der execute Methode der Vendor-Klassen die Firewall rules als Parameter zu übergeben, sodass die Firewallrules gar nicht wissen, auf welcher Firewall sie Appliziert werden.

Außerdem solltest du darüber nachdenken, ob es sinnvoll ist, die Firewall Rules in einem Dict und einer Klasse zu haben, ich empfehle dir nur eine Repräsentation der Regeln im Programm zu verwenden, ob Dict oder Klasse ist eigentlich egal (auch wenn das wahrscheinlich eine Designdiskussion auslösen wird, wie es richtig ist).
peterpanda
User
Beiträge: 3
Registriert: Donnerstag 6. Oktober 2022, 08:30

Guten Morgen und danke für eure Antworten!

bwbg hat geschrieben: Donnerstag 6. Oktober 2022, 12:15 Du bringst hier auch einige Begrifflichkeiten durcheinander. Eine "nested class" (verschachtelte Klasse) ist eine Klasse, welche innerhalb einer Klasse definiert ist (hier `Bar`):
[...]
Darüber hinaus verweise ich auf __blackjack__'s Ausführungen zu Namensgebungen in nahezu jedem Thread.
Der angehängte Code war nur ein skizziertes Beispiel und ich habe versucht in Kurzform darzustellen, was ich vorhabe.
Dabei habe ich auch die Einrückungen verhauen und Namenskonventionen ignoriert ... sorry dafür.

Um vielleicht nochmal etwas auszuholen. Wir bekommen regelmäßig Tickets für Firewall-Freischaltungen, denen dann je eine Kommunikations-Matrix angehängt ist.
Diese Tickets laden wir in einer "GUI" in Flask und beim Klicken auf "Firewall-Regeln anlegen" in der GUI wird das in meinem Codebeispiel skizzierte dict "rule_dict" übergeben, die Firewall-Regeln sollen auf der entsprechenden Firewall angelegt und der output der execute()-Methode in html dargestellt werden.

__blackjack__ hat geschrieben: `FirewallRules` hat keine `execute()`-Methode‽
Das ist Teil meines Problems. Es soll letztendlich je nach Vendor unterschiedlich verfahren werden, aber die execute-Methode soll quasi immer einen Output liefern.
Danke auf jeden Fall für deinen Vorschlag zu den Dictionaries zur "Vendorwahl"! Das lasse ich mir mal durch den Kopf gehen.
Vielleicht ist mein "Problem" ja tatsächlich kein Case für eine Klasse und ich mach, wie du es skizziert hast, ein Modul daraus.
imonbln hat geschrieben: Außerdem glaube ich, dass die Methode „_get_credentials“ kein Member von FirewallRules sein sollte. Vermutlich sind die Credentials abhängig von Vendor, vielleicht auch von der Firewall Entität (welche du noch gar nicht moduliert hast). Erstmal würde ich die Methode in den Vendor Klassen vermuten.
Am Ende sollte ja mit der execute()-Methode auf die Hosts gesprochen werden und dafür brauche ich die Credentials. Die Credentials selbst kriege ich von einer Funktion in einem Usermanagement-Modul zurück.
Die Credentials sind tatsächlich von vielen Faktoren abhängig (mehrere Teams haben unabhängig voneinander Firewalls mit unterschiedlichen AAA-Servern aufgebaut 8) )
Benutzeravatar
__blackjack__
User
Beiträge: 13003
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

Nach dem jetzt etwas mehr Informationen über den Datenfluss und die Bedeutung der Daten bekannt sind, würde ich meine Skizze in diese Richtung verändern:

Code: Alles auswählen

TICKET_DATA = {
    "hostname": "tolle-firewall",
    "rules": [
        {
            "src_addr": ["1.2.3.4"],
            "dst_addr": ["2.3.4.5"],
            "services": ["TCP 22"],
            "comment": "total wichtige Regel",
        },
        {
            "src_addr": ["4.5.6.7"],
            "dst_addr": ["8.9.10.11"],
            "services": ["TCP 22"],
            "comment": "total wichtige Regel",
        },
    ],
}


def find_vendor(database, hostname):
    return ...


def get_credentials(database, hostname):
    return ...


def execute_rules_for_cisco(hostname, credendials, rules):
    # Paramiko stuff.
    return ...


def execute_rules_for_fortinet(hostname, credendials, rules):
    # Ansible-Plabook stuff.
    return ...


VENDORNAME_TO_EXECUTE = {
    "Cisco": execute_rules_for_cisco,
    "Fortinet": execute_rules_for_fortinet,
}


def execute_rules(vendor, hostname, credentials, rules):
    return VENDORNAME_TO_EXECUTE[vendor](hostname, credentials, rules)


def main():
    database = ...

    hostname = TICKET_DATA["hostname"]
    result = execute_rules(
        find_vendor(database, hostname),
        hostname,
        get_credentials(database, hostname),
        TICKET_DATA["rules"],
    )
    print(result)


if __name__ == "__main__":
    main()
Wobei die allgemeine `execute_rules()` vielleicht auch gar nicht notwendig ist wenn man den Vendornamen nicht noch irgendwo anders als Zeichenkette braucht. Dann könnte man aus der `find_vendor()` vielleicht gleich eine `find_vendor_excecute()` machen, welche die passende Funktion als Ergebnis liefert.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
imonbln
User
Beiträge: 149
Registriert: Freitag 3. Dezember 2021, 17:07

peterpanda hat geschrieben: Freitag 7. Oktober 2022, 10:22 Am Ende sollte ja mit der execute()-Methode auf die Hosts gesprochen werden und dafür brauche ich die Credentials. Die Credentials selbst kriege ich von einer Funktion in einem Usermanagement-Modul zurück.
Die Credentials sind tatsächlich von vielen Faktoren abhängig (mehrere Teams haben unabhängig voneinander Firewalls mit unterschiedlichen AAA-Servern aufgebaut 8) )
Das Klingt dann danach das Credentials ein eigenes Objekt sein sollte und auch per Komposition übergeben werden sollte. Wenn es von vielen Variablen abhängt, ist es nach OOP Ansatz nicht Aufgabe der FirewallsRules die Variablen aufzudröseln. Außerdem schreibst du selbst, dass die execute Methoden die Credentials brauchen, deshalb sollten die Klassen VendorCisco und VendorFortinet in ihren execute Methoden wissen, wie sie auf das Gerät kommen. FirewallRuleset interessiert das nicht, die Klasse will nur wissen, ob die Regeln angekommen sind.
peterpanda
User
Beiträge: 3
Registriert: Donnerstag 6. Oktober 2022, 08:30

__blackjack__ hat geschrieben: Freitag 7. Oktober 2022, 12:10 Nach dem jetzt etwas mehr Informationen über den Datenfluss und die Bedeutung der Daten bekannt sind, würde ich meine Skizze in diese Richtung verändern:
Danke für die Tipps! Ich werde dann den Ansatz probieren :)
Antworten