|
| 1 | +# PR 39 Review TODO (Arbeitsliste) |
| 2 | + |
| 3 | +Quelle: Review-Kommentare aus https://github.com/stackitcloud/stackit-landing-zone/pull/39 |
| 4 | +Stand: 2026-06-17 |
| 5 | + |
| 6 | +Hinweis zur Nutzung: |
| 7 | + |
| 8 | +- Pro Thema bitte genau eine Entscheidung markieren: |
| 9 | + - `[ ] Vorschlag umsetzen` |
| 10 | + - `[ ] Alternative wählen` |
| 11 | +- Bei `Alternative wählen` bitte eure Zielvariante unter `Alternative / Notiz` ergänzen. |
| 12 | +- `Ref` verweist auf die extrahierten Review-Kommentar-IDs (1..28). |
| 13 | + |
| 14 | +## Offene Themen als Checkliste (nach Bereich) |
| 15 | + |
| 16 | +### Architektur und Scope |
| 17 | + |
| 18 | +1. [x] Ref 05 - Kubernetes-Demo nicht fest im Landing-Zone-Terraform |
| 19 | + Review-Thema: Die Demo sollte optional sein und nicht Teil des produktionsnahen Standardpfads. |
| 20 | + Vorgeschlagene Lösung: Demo-Ressourcen aus `src/namespace-service.tf` in optionales Submodul auslagern (`demo_enabled`), Default `false`. |
| 21 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 22 | + Alternative / Notiz: |
| 23 | + |
| 24 | +2. [x] Ref 11 - Debug-Bastion als eigenes Modul |
| 25 | + Review-Thema: Debug-Bastion ist fachlich ein eigener Baustein. |
| 26 | + Vorgeschlagene Lösung: `src/modules/platform-kubernetes/5-debug-bastion.tf` in Submodul `modules/debug-bastion` auslagern und optional aufrufen. |
| 27 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 28 | + Alternative / Notiz: |
| 29 | + |
| 30 | +### Ingress und Security |
| 31 | + |
| 32 | +3. [x] Ref 04 - NGINX Ingress durch Gateway Controller ersetzen |
| 33 | + Review-Thema: NGINX-Ingress wurde aus Security-Gründen kritisch bewertet. |
| 34 | + Vorgeschlagene Lösung: Namespace-Service-Demo auf Gateway API Controller umstellen (Gateway/HTTPRoute). Wenn nicht sofort möglich: per Feature-Flag standardmäßig deaktivieren. |
| 35 | + Entscheidung: [ ] Vorschlag umsetzen [x] Alternative wählen |
| 36 | + Alternative / Notiz: Gateway API Controller bitte mittels Envoy Gateway umsetzen |
| 37 | + |
| 38 | +### Terraform Core und Provider |
| 39 | + |
| 40 | +4. [x] Ref 06 - null Provider durch terraform_data ersetzen |
| 41 | + Review-Thema: `null_resource` wird nicht mehr benötigt. |
| 42 | + Vorgeschlagene Lösung: `null_resource` auf `terraform_data` migrieren und `hashicorp/null` aus `required_providers` entfernen. |
| 43 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 44 | + Alternative / Notiz: |
| 45 | + |
| 46 | +5. [x] Ref 07 - Provider-Versionen planbar pinnen |
| 47 | + Review-Thema: `>=`-Constraints sind zu offen und reduzieren Vorhersagbarkeit. |
| 48 | + Vorgeschlagene Lösung: Constraints auf planbare Ranges umstellen (z. B. `~>`), danach Lockfile bewusst aktualisieren. |
| 49 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 50 | + Alternative / Notiz: |
| 51 | + |
| 52 | +6. [x] Ref 25, 26 - providers.tf fachlich vereinfachen |
| 53 | + Review-Thema: Provider-Setup und Region-Check wirken unnötig komplex. |
| 54 | + Vorgeschlagene Lösung: `src/providers.tf` auf notwendige Konfiguration reduzieren, Region-Check entfernen oder in Input-Validation verlagern. |
| 55 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 56 | + Alternative / Notiz: |
| 57 | + |
| 58 | +### Validierung und API-Design |
| 59 | + |
| 60 | +7. [x] Ref 08, 09 - Input-Validierung an richtige Stelle verschieben |
| 61 | + Review-Thema: `check`-Blöcke wurden für Input-Validierung genutzt. |
| 62 | + Vorgeschlagene Lösung: Input-Validierung in `variable.validation` (oder gezielt `precondition`) verschieben; `check` nur für Laufzeit-/State-Prüfungen verwenden. |
| 63 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 64 | + Alternative / Notiz: |
| 65 | + |
| 66 | +8. [x] Ref 28 - namespace_service.enabled vereinfachen/deprecaten |
| 67 | + Review-Thema: `enabled` ist redundant, wenn das Objekt selbst schon Aktivierung signalisiert. |
| 68 | + Vorgeschlagene Lösung: `namespace_service = null` als deaktiviert, Objekt gesetzt als aktiviert; `enabled` deprecaten und später entfernen. |
| 69 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 70 | + Alternative / Notiz: |
| 71 | + |
| 72 | +9. [x] Ref 27 - Variablen-Scope bereinigen |
| 73 | + Review-Thema: Ein Variablenblock liegt laut Review im falschen Scope. |
| 74 | + Vorgeschlagene Lösung: Variable ins fachlich passende Modul verschieben; Root-Variablen nur für echte Root-API behalten. |
| 75 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 76 | + Alternative / Notiz: |
| 77 | + |
| 78 | +### Kubernetes Platform, Netzwerk und Node-Pools |
| 79 | + |
| 80 | +10. [x] Ref 01, 02, 03 - Default Node-Pools sauber modellieren |
| 81 | + Review-Thema: Default-Node-Pools sollten als Variable-Defaults definiert werden; Trennung in `system`/`application` wird gewünscht. |
| 82 | + Vorgeschlagene Lösung: `var.cluster.node_pools` mit strukturiertem Default, `allow_system_components = true` nur im `system`-Pool. |
| 83 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 84 | + Alternative / Notiz: separaten application node pool vorsehen. |
| 85 | + |
| 86 | +11. [x] Ref 16, 17, 23 - SNA Input-Modell vereinfachen |
| 87 | + Review-Thema: `mode`-String plus zusätzliche locals gelten als unnötig. |
| 88 | + Vorgeschlagene Lösung: `sna_enabled` (bool) und optional `sna_network_area_id` als primäres Modell; `mode` deprecaten. |
| 89 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 90 | + Alternative / Notiz: Wir brauchen Dinge noch nicht deprecaten, sondern können vollständig umstellen, da wir ja noch nicht live waren. |
| 91 | + |
| 92 | +12. [x] Ref 12 - SNA Egress-Routing über Firewall klarstellen |
| 93 | + Review-Thema: Ohne Routing-Tabelle könnte Internet-Traffic Firewall-Bypass haben. |
| 94 | + Vorgeschlagene Lösung: Routing-Pfad fachlich fixieren und ggf. dedizierte Routing-Tabelle + Default-Route via Firewall umsetzen. |
| 95 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 96 | + Alternative / Notiz: |
| 97 | + |
| 98 | +13. [x] Ref 13 - Netzwerkdateien zusammenführen |
| 99 | + Review-Thema: `2-dns-zones.tf`, `2-network-area-membership.tf`, `2-sna-network.tf` sollen konsolidiert werden. |
| 100 | + Vorgeschlagene Lösung: Zusammenführen in `2-network.tf` als Struktur-Refactor ohne Logikänderung. |
| 101 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 102 | + Alternative / Notiz: |
| 103 | + |
| 104 | +14. [x] Ref 10 - Deprecated API-Version aktualisieren |
| 105 | + Review-Thema: Verwendete API-Version wurde als deprecated markiert. |
| 106 | + Vorgeschlagene Lösung: Auf aktuelle, unterstützte Version aktualisieren und gegen Provider/API-Matrix verifizieren. |
| 107 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 108 | + Alternative / Notiz: |
| 109 | + |
| 110 | +### Outputs und Verträge |
| 111 | + |
| 112 | +15. [x] Ref 15 - Grafana User/Password nicht als Output |
| 113 | + Review-Thema: Zugangsdaten sollen nicht als Terraform-Output exponiert werden. |
| 114 | + Vorgeschlagene Lösung: Sensitive Outputs entfernen; Zugriff über Secrets Manager bzw. dokumentierten Abrufpfad. |
| 115 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 116 | + Alternative / Notiz: |
| 117 | + |
| 118 | +16. [x] Ref 22 - Kubernetes-Output nur bei echter Nutzung |
| 119 | + Review-Thema: Output nur behalten, wenn es Downstream-Nutzung gibt. |
| 120 | + Vorgeschlagene Lösung: Nutzung nachweisen; ungenutzten Output entfernen oder klar als intern markieren. |
| 121 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 122 | + Alternative / Notiz: |
| 123 | + |
| 124 | +17. [x] Ref 24 - Root Outputs auf Public Contract reduzieren |
| 125 | + Review-Thema: Teile in `src/outputs.tf` wirken ohne klaren Mehrwert. |
| 126 | + Vorgeschlagene Lösung: Outputs auf stabile Public-Contract-Schnitt minimieren; interne Felder streichen. |
| 127 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 128 | + Alternative / Notiz: |
| 129 | + |
| 130 | +### Code-Style und Lesbarkeit |
| 131 | + |
| 132 | +18. [x] Ref 18 - Single-use local entfernen |
| 133 | + Review-Thema: `local.effective_observability_instance_id` wird nicht wiederverwendet. |
| 134 | + Vorgeschlagene Lösung: Ausdruck inline setzen, nur mehrfach genutzte locals behalten. |
| 135 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 136 | + Alternative / Notiz: |
| 137 | + |
| 138 | +19. [x] Ref 19 - depends_on ans Ressourcenende |
| 139 | + Review-Thema: Lesbarkeit nach HashiCorp-Style. |
| 140 | + Vorgeschlagene Lösung: Betroffene Ressourcen so umordnen, dass `depends_on` am Ende steht. |
| 141 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 142 | + Alternative / Notiz: |
| 143 | + |
| 144 | +20. [x] Ref 20 - Maintenance-Zuweisung vereinfachen |
| 145 | + Review-Thema: 1:1 Mapping ist unnötig komplex. |
| 146 | + Vorgeschlagene Lösung: Direktzuweisung `maintenance = var.cluster.maintenance`, sofern Typen kompatibel sind. |
| 147 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 148 | + Alternative / Notiz: |
| 149 | + |
| 150 | +21. [x] Ref 21 - Ausdruck für SSH-Key-Fallback vereinfachen |
| 151 | + Review-Thema: Der `try(trimspace(...), file(...))`-Ausdruck kann lesbarer sein. |
| 152 | + Vorgeschlagene Lösung: Ausdruck gemäß Vorschlag refactoren und mit Input-Validation kombinieren. |
| 153 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 154 | + Alternative / Notiz: |
| 155 | + |
| 156 | +22. [x] Ref 14 - Naming-Konvention für Suffixe vereinheitlichen |
| 157 | + Review-Thema: Suffix `-obs` ist inkonsistent. |
| 158 | + Vorgeschlagene Lösung: Einheitliche Suffix-Strategie festlegen (`ohne`, `-default` oder `-common`) und referenzkonsistent umsetzen. |
| 159 | + Entscheidung: [x] Vorschlag umsetzen [ ] Alternative wählen |
| 160 | + Alternative / Notiz: |
| 161 | + |
| 162 | +## Vorschlag für Umsetzung in Wellen |
| 163 | + |
| 164 | +1. **Welle A (sicher, low risk)**: 18, 19, 20, 21, 14, 13 |
| 165 | +2. **Welle B (API/Contract Changes)**: 01/02/03, 16/17/23, 28, 24, 27 |
| 166 | +3. **Welle C (Security/Architecture)**: 04, 05, 11, 12, 15, 22, 25/26, 06, 07, 10 |
| 167 | + |
| 168 | +## Entscheidungsprotokoll |
| 169 | + |
| 170 | +- Datum: 17.06.2026 |
| 171 | +- Teilnehmer: Lukas Weberruß |
| 172 | +- Beschluss pro Ref-Gruppe: Alle Themen werden umgesetzt. Bei Ref 04 erfolgt die Umsetzung als Gateway API Controller mit Envoy Gateway. |
| 173 | +- Offene Fragen: |
| 174 | +- Nächster Implementierungs-PR: |
0 commit comments