Conversation
WalkthroughThis change introduces the initial implementation of the KRR Enforcer component, which enforces Kubernetes Resource Recommender (KRR) recommendations via a mutating webhook. It includes the FastAPI-based webhook server, data models, data access layer, resource patching utilities, owner and recommendation stores, metrics, environment variable management, Kubernetes resource loader, and utilities for handling custom CA certificates. Helm chart templates, deployment manifests, and supporting scripts are also added for installation and configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant K8sAPI as Kubernetes API Server
participant Enforcer as KRR Enforcer Webhook (FastAPI)
participant Supabase as Supabase DAL
participant OwnerStore as Owner Store
participant RecStore as Recommendation Store
K8sAPI->>Enforcer: AdmissionReview (POST /mutate)
Enforcer->>OwnerStore: handle_rs_admission or get_pod_owner
OwnerStore-->>Enforcer: Owner info (if Pod)
Enforcer->>RecStore: get_recommendations(namespace, kind, name)
RecStore->>Supabase: get_latest_krr_scan(current_scan_id)
Supabase-->>RecStore: Recommendations (if new scan)
RecStore-->>Enforcer: WorkloadRecommendation
Enforcer->>Enforcer: patch_container_resources (if enforcement enabled)
Enforcer-->>K8sAPI: AdmissionResponse (mutate or allow)
sequenceDiagram
participant User as User
participant Enforcer as KRR Enforcer Webhook (FastAPI)
participant RecStore as Recommendation Store
User->>Enforcer: GET /recommendations/{namespace}/{kind}/{name}
Enforcer->>RecStore: get_recommendations(namespace, kind, name)
RecStore-->>Enforcer: WorkloadRecommendation or 404
Enforcer-->>User: JSON response with recommendations or not found
sequenceDiagram
participant Prometheus as Prometheus
participant Enforcer as KRR Enforcer Webhook (FastAPI)
Prometheus->>Enforcer: GET /metrics
Enforcer-->>Prometheus: Prometheus metrics (including mutation/admission stats)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🔭 Outside diff range comments (1)
enforcer/requirements.txt (1)
1-9: 💡 Verification agent🧩 Analysis chain
Verify package versions for security and currency.
The requirements use exact version pinning, which ensures reproducible builds but may miss security updates. Consider verifying that these versions are current and secure.
🌐 Web query:
What are the latest stable versions of FastAPI, Uvicorn, Pydantic, Supabase Python client, PyYAML, Cachetools, Prometheus-client, and Kubernetes Python client as of May 2025? Are there any known security vulnerabilities in the specified versions?💡 Result:
Latest Stable Versions as of May 2025
Based on the latest public sources and official repositories, the most recent stable versions for each specified library are:
Library Latest Stable Version Release Date FastAPI 0.115.12 March 23, 2025 Uvicorn Not explicitly found, but 0.11.7+ is required for security See notes below Pydantic (PydanticAI) 0.2.0 (PydanticAI) May 12, 2025 Supabase Python Client Not explicitly found N/A PyYAML Not explicitly found N/A cachetools Not explicitly found N/A prometheus-client Not explicitly found N/A Kubernetes Python Client Not explicitly found N/A FastAPI: The latest stable release is 0.115.12, as of March 23, 2025[1][4][5].
Uvicorn: While the specific latest version number is not provided in the results, historical advisories indicate versions before 0.11.7 are insecure; ensure any installed version is at least 0.11.7 or newer to avoid known response splitting vulnerabilities[6]. It is likely that a more recent version exists, bundled with FastAPI[3], but users should verify at install time.
Pydantic (PydanticAI): The most recent release is 0.2.0, published May 12, 2025. PydanticAI is intended to reach 1.0 in Summer 2025 and is still subject to breaking changes[7].
Supabase Python Client, PyYAML, cachetools, prometheus-client, Kubernetes Python Client: Latest explicit versions are not returned in the current search results. For these, refer directly to PyPI or official repositories for up-to-date information as of your usage.
Security Vulnerabilities in Specified Versions
- FastAPI 0.115.12: No known security vulnerabilities are reported in this latest release from the available search results.
- Uvicorn: Versions prior to 0.11.7 are vulnerable to HTTP response splitting and log pollution attacks (CVE-2020-7694, CVE-2020-7695). It is critical to use 0.11.7 or newer; no explicit vulnerabilities are indicated for recent versions[6].
- Pydantic (PydanticAI 0.2.0): No security advisories or vulnerabilities are noted for this version in the search results, but it is still pre-1.0 and may undergo breaking changes[7].
- Supabase Python Client, PyYAML, cachetools, prometheus-client, Kubernetes Python Client: No specific current vulnerabilities are reported in the provided results. However, for enterprise or production environments, always consult the latest CVE feeds and security advisories for new vulnerabilities, as package status may change rapidly.
Summary
- FastAPI 0.115.12, Uvicorn 0.11.7 or newer, and PydanticAI 0.2.0 are all up-to-date and have no known active vulnerabilities as of May 2025, based on search results.
- Older versions of Uvicorn (before 0.11.7) are vulnerable and should not be used.
- For the remaining libraries, consult PyPI and security advisories for the absolute latest versions and vulnerability status, as this information was not present in the current search results.
Citations:
- 1: https://pypi.org/project/fastapi/
- 2: https://fastapi.tiangolo.com/release-notes/
- 3: https://github.com/fastapi/fastapi-cli/releases
- 4: https://en.wikipedia.org/wiki/FastAPI
- 5: https://github.com/fastapi/fastapi/releases
- 6: tiangolo/uvicorn-gunicorn-fastapi-docker#85
- 7: https://ai.pydantic.dev/changelog/
- 8: https://github.com/trallnag/prometheus-fastapi-instrumentator/blob/master/CHANGELOG.md
- 9: https://aloa.co/blog/fast-api
- 10: https://www.cisa.gov/news-events/bulletins/sb25-069
Align dependency versions and add trailing newline
- FastAPI is pinned to 0.109.2, but the latest stable release is 0.115.12 (Mar 23, 2025). Update the pin or loosen it (e.g.,
fastapi>=0.109.2,<0.116).- Uvicorn
==0.27.1is above the security threshold (≥0.11.7) and safe.- Pydantic
==2.6.1may be behind the latest core Pydantic release—please verify on PyPI and consider a version range (e.g.,pydantic>=2.6.1,<3).- Supabase, PyYAML, cachetools, prometheus-client, and the Kubernetes Python client: no explicit “latest” versions were found in the search results. Check each on PyPI and similarly allow non-breaking patch updates.
- Add a final newline to enforcer/requirements.txt to satisfy POSIX conventions.
🧹 Nitpick comments (43)
enforcer/metrics.py (1)
1-1: Add module docstring for better documentation.The module lacks a docstring explaining its purpose and the metrics it defines.
+""" +Prometheus metrics for the KRR Enforcer webhook server. + +This module defines metrics to monitor pod admission mutations, ReplicaSet operations, +owner map size, and admission operation durations. +""" + from prometheus_client import Counter, Histogram, Gauge🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[error] 1-1: Unable to import 'prometheus_client'
(E0401)
enforcer/utils.py (2)
1-7: Add module docstring and consider security implications.The module lacks documentation explaining its purpose and the security considerations of modifying the CA bundle.
+""" +Certificate utilities for managing custom CA certificates. + +This module provides functions to add custom CA certificates to the system's +certificate bundle for secure communication with external services. + +Security Note: This module modifies the global certificate bundle, which affects +all SSL/TLS connections made by the Python process. +""" + import base64🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
29-41: Improve error handling and documentation.The main interface function handles the fallback logic well but could benefit from better documentation and more specific error handling.
def add_custom_certificate(custom_ca: str) -> bool: + """Add a custom CA certificate to the system's CA bundle. + + Args: + custom_ca: Base64-encoded CA certificate + + Returns: + True if certificate was added successfully, False if input was empty + + Raises: + ValueError: If certificate data is invalid + """ if not custom_ca: return False # NOTE: Sometimes (Openshift) the certifi.where() is not writable, so we need to # use a temporary file in case of PermissionError. try: append_custom_certificate(custom_ca) - except PermissionError: + except (PermissionError, OSError) as e: + # Fall back to temporary file if we can't write to the system bundle create_temporary_certificate(custom_ca) return True🧰 Tools
🪛 Pylint (3.3.7)
[convention] 29-29: Missing function or method docstring
(C0116)
enforcer/requirements.txt (1)
1-9: Consider using compatible version ranges for dependencies.While exact pinning ensures reproducibility, it can make security updates more difficult. Consider using compatible version ranges (e.g.,
fastapi>=0.109.2,<0.110.0) for patch-level updates.-fastapi==0.109.2 -uvicorn==0.27.1 -pydantic==2.6.1 -supabase==2.5 -PyYAML==6.0.1 -cachetools==5.3.3 -prometheus-client==0.20.0 -kubernetes==26.1.0 +fastapi>=0.109.2,<0.110.0 +uvicorn>=0.27.1,<0.28.0 +pydantic>=2.6.1,<2.7.0 +supabase>=2.5,<3.0 +PyYAML>=6.0.1,<7.0.0 +cachetools>=5.3.3,<6.0.0 +prometheus-client>=0.20.0,<0.21.0 +kubernetes>=26.1.0,<27.0.0 +helm/krr-enforcer/Chart.yaml (1)
6-8: Fix typo in comment.There's a typo in the comment: "version`" should be "version".
-# we use 0.0.1 as a placeholder for the version` because Helm wont allow `0.0.0` and we want to be able to run +# we use 0.0.1 as a placeholder for the version because Helm wont allow `0.0.0` and we want to be able to runenforcer/dal/robusta_config.py (3)
1-3: Add module docstring for better documentation.The module is missing a docstring that would help developers understand the purpose of these configuration models.
+"""Configuration models for Robusta platform integration.""" from typing import List, Dict from pydantic import BaseModel🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[error] 2-2: Unable to import 'pydantic'
(E0401)
5-7: Add class docstring for RobustaConfig.The class would benefit from documentation explaining its purpose and field meanings.
class RobustaConfig(BaseModel): + """Configuration model for Robusta platform settings.""" sinks_config: List[Dict[str, Dict]] global_config: dict🧰 Tools
🪛 Pylint (3.3.7)
[convention] 5-5: Missing class docstring
(C0115)
[refactor] 5-5: Too few public methods (0/2)
(R0903)
9-14: Add class docstring and fix missing final newline.The RobustaToken class needs documentation and the file is missing a final newline.
class RobustaToken(BaseModel): + """Authentication token model for Robusta platform access.""" store_url: str api_key: str account_id: str email: str password: str +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 14-14: Final newline missing
(C0304)
[convention] 9-9: Missing class docstring
(C0115)
[refactor] 9-9: Too few public methods (0/2)
(R0903)
enforcer/Dockerfile (1)
1-2: Fix the comment to match the actual base image version.The comment mentions Python 3.9 but the Dockerfile uses Python 3.12-slim.
-# Use the official Python 3.9 slim image as the base image +# Use the official Python 3.12 slim image as the base imagehelm/krr-enforcer/templates/enforcer-webhook.yaml (1)
16-16: Consider making the default CA bundle more explicit.The default CA bundle value "Cg==" (base64 for newline) should be replaced by the certificate generation job. Consider adding a comment explaining this placeholder.
- caBundle: {{ .Values.caBundle | default "Cg==" }} + caBundle: {{ .Values.caBundle | default "Cg==" }} # Placeholder, replaced by cert jobhelm/krr-enforcer/values.yaml (1)
15-16: Consider setting CPU limits for resource management.The CPU limit is explicitly disabled (
cpu: ~). While this allows burst capacity, it may lead to resource contention in constrained environments.Consider setting a reasonable CPU limit:
limits: - cpu: ~ + cpu: 500menforcer/resources/kubernetes_resource_loader.py (3)
17-18: Add class docstring.The class needs documentation explaining its purpose and usage.
class KubernetesResourceLoader: + """Loads Kubernetes resources using batch processing for efficient discovery."""🧰 Tools
🪛 Pylint (3.3.7)
[convention] 17-17: Missing class docstring
(C0115)
[refactor] 17-17: Too few public methods (1/2)
(R0903)
19-21: Add method docstring and improve type hints.The method needs documentation and could benefit from better type hints.
@staticmethod def load_replicasets() -> List[RsOwner]: + """Load all ReplicaSets from the cluster in batches. + + Returns: + List of RsOwner objects representing ReplicaSet ownership relationships. + """🧰 Tools
🪛 Pylint (3.3.7)
[convention] 20-20: Missing function or method docstring
(C0116)
38-38: Handle protected member access properly.Accessing the protected
_continuemember triggers a warning. Consider using the public API or document why this is necessary.The current approach works but accesses a protected member. This is likely the only way to get the continuation token from the Kubernetes client, so consider adding a comment:
- continue_ref = replicasets.metadata._continue + # Using protected member as it's the only way to access continuation token + continue_ref = replicasets.metadata._continue🧰 Tools
🪛 Pylint (3.3.7)
[warning] 38-38: Access to a protected member _continue of a client class
(W0212)
helm/krr-enforcer/templates/enforcer.yaml (2)
96-113: Simplify resource configuration logic.The current resource configuration is overly verbose and can be simplified while maintaining the same functionality.
- {{- if .Values.resources.requests }} - requests: - {{- if .Values.resources.requests.cpu }} - cpu: {{ .Values.resources.requests.cpu }} - {{- end }} - {{- if .Values.resources.requests.memory }} - memory: {{ .Values.resources.requests.memory }} - {{- end }} - {{- end }} - {{- if .Values.resources.limits }} - limits: - {{- if .Values.resources.limits.cpu }} - cpu: {{ .Values.resources.limits.cpu }} - {{- end }} - {{- if .Values.resources.limits.memory }} - memory: {{ .Values.resources.limits.memory }} - {{- end }} - {{- end }} + {{- toYaml .Values.resources | nindent 12 }}
86-87: Consider adjusting liveness probe timing.The liveness probe
initialDelaySeconds: 30might be too aggressive for a webhook server that needs to initialize TLS certificates and connect to external services.Consider increasing to 60 seconds to allow for proper initialization:
- initialDelaySeconds: 30 + initialDelaySeconds: 60enforcer/env_vars.py (2)
1-24: Add module documentation and fix formatting issues.The module is missing documentation and has formatting issues that should be addressed.
+""" +Environment variable configuration module for the krr-enforcer component. + +Centralizes all environment-dependent configuration values with appropriate +defaults and type casting. +""" import os ROBUSTA_CONFIG_PATH = os.environ.get( "ROBUSTA_CONFIG_PATH", "/etc/robusta/config/active_playbooks.yaml" ) ROBUSTA_ACCOUNT_ID = os.environ.get("ROBUSTA_ACCOUNT_ID", "") STORE_URL = os.environ.get("STORE_URL", "") STORE_API_KEY = os.environ.get("STORE_API_KEY", "") STORE_EMAIL = os.environ.get("STORE_EMAIL", "") STORE_PASSWORD = os.environ.get("STORE_PASSWORD", "") DISCOVERY_MAX_BATCHES = int(os.environ.get("DISCOVERY_MAX_BATCHES", 50)) DISCOVERY_BATCH_SIZE = int(os.environ.get("DISCOVERY_BATCH_SIZE", 30000)) UPDATE_THRESHOLD = float(os.environ.get("UPDATE_THRESHOLD", 20.0)) SCAN_RELOAD_INTERVAL = int(os.environ.get("SCAN_RELOAD_INTERVAL", 3600)) KRR_MUTATION_MODE_DEFAULT = os.environ.get("KRR_MUTATION_MODE_DEFAULT", "enforce") REPLICA_SET_CLEANUP_INTERVAL = int(os.environ.get("REPLICA_SET_CLEANUP_INTERVAL", 600)) REPLICA_SET_DELETION_WAIT = int(os.environ.get("REPLICA_SET_DELETION_WAIT", 600)) SCAN_AGE_HOURS_THRESHOLD = int(os.environ.get("SCAN_AGE_HOURS_THRESHOLD", 360)) # 15 days ENFORCER_SSL_KEY_FILE = os.environ.get("ENFORCER_SSL_KEY_FILE", "") ENFORCER_SSL_CERT_FILE = os.environ.get("ENFORCER_SSL_CERT_FILE", "") +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 24-24: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
6-10: Consider security implications of credential handling.Storing credentials in environment variables is acceptable but consider additional security measures for production environments.
For enhanced security, consider:
- Using Kubernetes secrets mounted as files instead of environment variables
- Implementing credential rotation mechanisms
- Adding validation for required credentials at startup
- Using tools like HashiCorp Vault or AWS Secrets Manager for sensitive data
enforcer/resources/recommendation_store.py (2)
1-8: Add module and class documentation.The module and class are missing documentation which is essential for maintainability.
+""" +Manages workload resource recommendations with automatic background reloading. + +The RecommendationStore fetches recommendations from Supabase and maintains +an in-memory cache that is periodically refreshed. +""" import logging import threading from typing import Dict, Optional, Tuple from enforcer.dal.supabase_dal import SupabaseDal from enforcer.env_vars import SCAN_RELOAD_INTERVAL from enforcer.model import WorkloadRecommendation class RecommendationStore: + """ + Manages workload resource recommendations with background reloading. + + Maintains an in-memory cache of recommendations that is automatically + refreshed at configured intervals. + """🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[error] 5-5: Unable to import 'enforcer.dal.supabase_dal'
(E0401)
[error] 6-6: Unable to import 'enforcer.env_vars'
(E0401)
[error] 7-7: Unable to import 'enforcer.model'
(E0401)
41-41: Simplify dictionary get operation.The static analysis correctly identifies an unnecessary default value.
- workload_recommendation: WorkloadRecommendation = scan_recommendations.get(store_key, None) + workload_recommendation: WorkloadRecommendation = scan_recommendations.get(store_key)🧰 Tools
🪛 Ruff (0.11.9)
41-41: Use
scan_recommendations.get(store_key)instead ofscan_recommendations.get(store_key, None)Replace
scan_recommendations.get(store_key, None)withscan_recommendations.get(store_key)(SIM910)
🪛 Pylint (3.3.7)
[convention] 41-41: Line too long (111/100)
(C0301)
enforcer/resources/owner_store.py (4)
1-11: Add module and class documentation.The module and class lack documentation which is essential for understanding the complex ownership tracking logic.
+""" +Manages Kubernetes ReplicaSet owners and pod ownership tracking. + +Provides thread-safe tracking of ReplicaSet ownership information and +automatic cleanup of deleted ReplicaSets. +""" import logging import threading import time from typing import Dict, Any, Optional, List from enforcer.env_vars import REPLICA_SET_CLEANUP_INTERVAL, REPLICA_SET_DELETION_WAIT from enforcer.model import PodOwner, RsOwner from enforcer.resources.kubernetes_resource_loader import KubernetesResourceLoader class OwnerStore: + """ + Thread-safe store for tracking Kubernetes ReplicaSet ownership. + + Maintains ReplicaSet owner information and provides pod ownership + resolution with automatic cleanup of deleted ReplicaSets. + """🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[error] 6-6: Unable to import 'enforcer.env_vars'
(E0401)
[error] 7-7: Unable to import 'enforcer.model'
(E0401)
[error] 8-8: Unable to import 'enforcer.resources.kubernetes_resource_loader'
(E0401)
[convention] 11-11: Missing class docstring
(C0115)
68-68: Remove unnecessary f-string prefix.The logging statement doesn't use any string interpolation.
- logging.debug(f"handle_rs_admission %s", request) + logging.debug("handle_rs_admission %s", request)🧰 Tools
🪛 Ruff (0.11.9)
68-68: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[warning] 68-68: Using an f-string that does not have any interpolated variables
(W1309)
117-118: Fix exception handling issues.The exception variable is unused and the f-string has no placeholders.
- except Exception as e: - logging.exception(f"Failed to cleanup deleted replicasets") + except Exception: + logging.exception("Failed to cleanup deleted replicasets")🧰 Tools
🪛 Ruff (0.11.9)
117-117: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
118-118: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[warning] 117-117: Catching too general exception Exception
(W0718)
[warning] 118-118: Use lazy % formatting in logging functions
(W1203)
[warning] 118-118: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 117-118: Unused variable 'e'
(W0612)
54-54: Simplify dictionary get operation.Similar to the recommendation store, unnecessary default value.
- rs_owner = self.rs_owners.get(self._rs_key(controller.get("name"), namespace), None) + rs_owner = self.rs_owners.get(self._rs_key(controller.get("name"), namespace))🧰 Tools
🪛 Pylint (3.3.7)
[convention] 54-54: Line too long (108/100)
(C0301)
helm/krr-enforcer/templates/enforcer-cert-job.yaml (3)
42-42: Consider extending certificate validity or implementing rotation.The CA certificate is only valid for 365 days. Consider extending the validity period or implementing a certificate rotation mechanism to avoid service disruption.
- openssl req -x509 -new -nodes -key ca.key -subj "/CN=robusta-krr-enforcer-ca" -days 365 -out ca.crt + openssl req -x509 -new -nodes -key ca.key -subj "/CN=robusta-krr-enforcer-ca" -days 3650 -out ca.crt
49-49: Remove trailing spaces.Static analysis detected trailing spaces on multiple lines.
- DNS_NAME=${SERVICE_NAME}.${NAMESPACE}.svc - + DNS_NAME=${SERVICE_NAME}.${NAMESPACE}.svc +Apply similar fixes to lines 67, 84, and 141.
Also applies to: 67-67, 84-84, 141-141
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 49-49: trailing spaces
(trailing-spaces)
94-97: Improve readability of the jq command.The jq command for updating the secret could be more readable by using proper formatting.
- kubectl get secret ${SERVICE_NAME}-certs -o json | \ - jq --arg ca "$(cat ca.crt | base64 | tr -d '\n')" \ - '.data["ca.crt"] = $ca' | \ - kubectl apply -f - + CA_CERT_BASE64=$(cat ca.crt | base64 -w 0) + kubectl patch secret ${SERVICE_NAME}-certs \ + --type='json' \ + -p='[{"op": "add", "path": "/data/ca.crt", "value": "'${CA_CERT_BASE64}'"}]'enforcer/model.py (2)
1-6: Add module docstring.Add a module-level docstring to describe the purpose of this module.
+""" +Data models for Kubernetes workload resource recommendations. + +This module defines Pydantic models to represent pod/replicaset ownership +and container resource recommendations (CPU/memory requests and limits). +""" import logging from typing import Optional, Dict, Any, List from pydantic import BaseModel🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[error] 4-4: Unable to import 'pydantic'
(E0401)
65-65: Add missing newline at end of file.Files should end with a newline character.
def add(self, container: str, recommendation: ResourcesRecommendation): self.container_recommendations[container] = recommendation +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 65-65: Final newline missing
(C0304)
enforcer/README.md (3)
83-83: Fix hyphenation in "self-signed".This compound word should be hyphenated.
-| `certificate` | Base64-encoded custom CA certificate - for self signed certificates | `""` | +| `certificate` | Base64-encoded custom CA certificate - for self-signed certificates | `""` |🧰 Tools
🪛 LanguageTool
[misspelling] ~83-~83: This word is normally spelled with a hyphen.
Context: ...e64-encoded custom CA certificate - for self signed certificates |""| | `serviceMonitor...(EN_COMPOUNDS_SELF_SIGNED)
200-200: Add commas before "and" in compound sentences.Add commas to improve readability.
-1. **Certificate Errors**: Ensure TLS certificates are properly configured and valid +1. **Certificate Errors**: Ensure TLS certificates are properly configured, and valid-3. **No Recommendations**: Check that KRR has generated recommendations and they're accessible +3. **No Recommendations**: Check that KRR has generated recommendations, and they're accessibleAlso applies to: 202-202
🧰 Tools
🪛 LanguageTool
[uncategorized] ~200-~200: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...TLS certificates are properly configured and valid 2. Permission Denied: Verify ...(COMMA_COMPOUND_SENTENCE_2)
58-71: Consider adding a more complete configuration example.The example could include additional common configuration options like resource limits and log level.
# enforcer-values.yaml logLevel: INFO resources: requests: cpu: 100m memory: 256Mi limits: cpu: 500m memory: 512Mi serviceMonitor: enabled: true additionalEnvVars: - name: CLUSTER_NAME value: my-cluster-name - name: ROBUSTA_UI_TOKEN valueFrom: secretKeyRef: name: robusta-secrets key: robustaSinkToken - name: KRR_MUTATION_MODE_DEFAULT value: "enforce"enforcer/dal/supabase_dal.py (1)
221-222: Remove trailing newlines.File should end with exactly one newline.
- - +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 221-221: Trailing newlines
(C0305)
enforcer/patch_manager.py (5)
1-1: Add module docstring.Please add a module docstring to describe the purpose and functionality of this module.
+""" +Module for managing Kubernetes container resource patches. + +This module provides utilities for parsing resource strings, validating resource +configurations, and generating JSON patches for container resource updates based +on KRR recommendations. +""" import copy🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
121-127: Add docstring for significant_diff function.Please add documentation to explain the function's purpose and behavior.
def significant_diff(old: Optional[float], new: float, percent_threshold: float) -> bool: + """ + Check if the difference between old and new values exceeds the threshold. + + Args: + old: Original value (None or 0 treated as significant difference) + new: New value to compare + percent_threshold: Percentage threshold for significant difference + + Returns: + True if difference is significant, False otherwise + """ if not old: # old cpu is none or 0 return True🧰 Tools
🪛 Pylint (3.3.7)
[convention] 121-121: Missing function or method docstring
(C0116)
129-133: Add docstring for add_resource_value function.Please add documentation to explain this helper function.
def add_resource_value(resources: Dict[str, Any], resource_type: str, resource_name: str, resource_value: Any) -> None: + """ + Add or update a resource value in the resources dictionary. + + Args: + resources: Resources dictionary to update + resource_type: Type of resource (e.g., 'requests', 'limits') + resource_name: Name of resource (e.g., 'cpu', 'memory') + resource_value: Value to set (will be converted to string) + """ if resource_type not in resources: resources[resource_type] = {}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 129-129: Line too long (119/100)
(C0301)
[convention] 129-129: Missing function or method docstring
(C0116)
174-226: Remove trailing whitespace.The function logic is correct, but please remove trailing whitespace from lines 190, 194, 197, 202, 207, 211, 214, 219, 224.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 190-190: Trailing whitespace
(C0303)
[convention] 194-194: Trailing whitespace
(C0303)
[convention] 197-197: Trailing whitespace
(C0303)
[convention] 202-202: Trailing whitespace
(C0303)
[convention] 207-207: Trailing whitespace
(C0303)
[convention] 211-211: Trailing whitespace
(C0303)
[convention] 214-214: Trailing whitespace
(C0303)
[convention] 219-219: Trailing whitespace
(C0303)
[convention] 224-224: Trailing whitespace
(C0303)
[warning] 200-200: Use lazy % formatting in logging functions
(W1203)
[warning] 205-205: Use lazy % formatting in logging functions
(W1203)
[warning] 217-217: Use lazy % formatting in logging functions
(W1203)
[warning] 222-222: Use lazy % formatting in logging functions
(W1203)
246-253: Combine nested if statements.As suggested by static analysis, these nested if statements can be combined for better readability.
- if resources != updated_resources: - if validate_resources(updated_resources): - patches.append({ - "op": "replace" if had_resources else "add", - "path": f"/spec/containers/{container_index}/resources", - "value": updated_resources - }) + if resources != updated_resources and validate_resources(updated_resources): + patches.append({ + "op": "replace" if had_resources else "add", + "path": f"/spec/containers/{container_index}/resources", + "value": updated_resources + })🧰 Tools
🪛 Ruff (0.11.9)
246-247: Use a single
ifstatement instead of nestedifstatementsCombine
ifstatements usingand(SIM102)
enforcer/enforcer_main.py (5)
65-75: Add docstring for admission_allowed function.Please add documentation for this helper function.
def admission_allowed(request: AdmissionReview) -> Dict[str, Any]: + """ + Create an allowed admission response. + + Args: + request: The admission review request + + Returns: + Admission review response with allowed=True + """ return \🧰 Tools
🪛 Pylint (3.3.7)
[convention] 65-65: Missing function or method docstring
(C0116)
76-84: Simplify enforce_pod logic and add documentation.The function can be simplified by removing the unnecessary elif.
def enforce_pod(pod: Dict[str, Any]) -> bool: + """ + Determine if pod resources should be enforced based on annotations. + + Args: + pod: Pod specification dictionary + + Returns: + True if enforcement should be applied, False otherwise + """ mode = pod.get('metadata', {}).get('annotations', {}).get("admission.robusta.dev/krr-mutation-mode", None) if mode == ENFORCE: return True - elif mode == IGNORE: + if mode == IGNORE: return False - else: - return KRR_MUTATION_MODE_DEFAULT == ENFORCE + return KRR_MUTATION_MODE_DEFAULT == ENFORCE🧰 Tools
🪛 Pylint (3.3.7)
[convention] 77-77: Line too long (110/100)
(C0301)
[convention] 76-76: Missing function or method docstring
(C0116)
[refactor] 78-83: Unnecessary "elif" after "return", remove the leading "el" from "elif"
(R1705)
99-99: Remove unnecessary f-string prefixes in logging.These logging statements don't use any placeholders, so the f-string prefix is unnecessary.
- logging.debug(f"Admission request received %s", request) + logging.debug("Admission request received %s", request)- logger.debug(f"Processing pod: %s", owner_store.get_pod_name(object_to_review.get("metadata", {}))) + logger.debug("Processing pod: %s", owner_store.get_pod_name(object_to_review.get("metadata", {})))- logger.debug(f"Pod owner %s", pod_owner) + logger.debug("Pod owner %s", pod_owner)- logger.debug(f"Pod Recommendations %s", recommendations) + logger.debug("Pod Recommendations %s", recommendations)- logger.debug(f"Pod patches %s", patches) + logger.debug("Pod patches %s", patches)Also applies to: 118-118, 134-134, 146-146, 160-160
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 99-99: Using an f-string that does not have any interpolated variables
(W1309)
🪛 Ruff (0.11.9)
99-99: f-string without any placeholders
Remove extraneous
fprefix(F541)
173-175: Use proper exception chaining.When re-raising exceptions, use
from eto maintain the exception context.except Exception as e: logger.exception("Error processing webhook request") - raise HTTPException(status_code=500, detail=str(e)) + raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools
🪛 Pylint (3.3.7)
[warning] 175-175: Consider explicitly re-raising using 'raise HTTPException(status_code=500, detail=str(e)) from e'
(W0707)
🪛 Ruff (0.11.9)
175-175: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
230-232: Use proper exception chaining.When re-raising exceptions, use
from eto maintain the exception context.except Exception as e: logger.exception("Error retrieving recommendations") - raise HTTPException(status_code=500, detail=str(e)) + raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools
🪛 Pylint (3.3.7)
[warning] 232-232: Consider explicitly re-raising using 'raise HTTPException(status_code=500, detail=str(e)) from e'
(W0707)
🪛 Ruff (0.11.9)
232-232: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
enforcer/Dockerfile(1 hunks)enforcer/README.md(1 hunks)enforcer/dal/robusta_config.py(1 hunks)enforcer/dal/supabase_dal.py(1 hunks)enforcer/enforcer_main.py(1 hunks)enforcer/env_vars.py(1 hunks)enforcer/metrics.py(1 hunks)enforcer/model.py(1 hunks)enforcer/patch_manager.py(1 hunks)enforcer/requirements.txt(1 hunks)enforcer/resources/kubernetes_resource_loader.py(1 hunks)enforcer/resources/owner_store.py(1 hunks)enforcer/resources/recommendation_store.py(1 hunks)enforcer/utils.py(1 hunks)helm/krr-enforcer/.helmignore(1 hunks)helm/krr-enforcer/Chart.yaml(1 hunks)helm/krr-enforcer/templates/enforcer-cert-job.yaml(1 hunks)helm/krr-enforcer/templates/enforcer-service-account.yaml(1 hunks)helm/krr-enforcer/templates/enforcer-webhook.yaml(1 hunks)helm/krr-enforcer/templates/enforcer.yaml(1 hunks)helm/krr-enforcer/templates/service-monitor.yaml(1 hunks)helm/krr-enforcer/values.yaml(1 hunks)helm/upload_chart.sh(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
enforcer/resources/kubernetes_resource_loader.py (1)
enforcer/model.py (1)
RsOwner(12-17)
🪛 YAMLlint (1.37.1)
helm/krr-enforcer/templates/enforcer-service-account.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
helm/krr-enforcer/templates/enforcer-webhook.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
helm/krr-enforcer/templates/service-monitor.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
helm/krr-enforcer/templates/enforcer.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
helm/krr-enforcer/templates/enforcer-cert-job.yaml
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 84-84: trailing spaces
(trailing-spaces)
[error] 141-141: trailing spaces
(trailing-spaces)
[error] 4-4: syntax error: expected , but found ''
(syntax)
🪛 Pylint (3.3.7)
enforcer/dal/robusta_config.py
[convention] 14-14: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
[error] 2-2: Unable to import 'pydantic'
(E0401)
[convention] 5-5: Missing class docstring
(C0115)
[refactor] 5-5: Too few public methods (0/2)
(R0903)
[convention] 9-9: Missing class docstring
(C0115)
[refactor] 9-9: Too few public methods (0/2)
(R0903)
enforcer/metrics.py
[convention] 25-25: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
[error] 1-1: Unable to import 'prometheus_client'
(E0401)
enforcer/utils.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 9-9: Missing function or method docstring
(C0116)
[convention] 16-16: Missing function or method docstring
(C0116)
[convention] 29-29: Missing function or method docstring
(C0116)
enforcer/resources/recommendation_store.py
[convention] 24-24: Line too long (141/100)
(C0301)
[convention] 40-40: Line too long (103/100)
(C0301)
[convention] 41-41: Line too long (111/100)
(C0301)
[convention] 46-46: Line too long (102/100)
(C0301)
[convention] 48-48: Line too long (105/100)
(C0301)
[convention] 74-74: Line too long (108/100)
(C0301)
[convention] 76-76: Trailing newlines
(C0305)
[convention] 1-1: Missing module docstring
(C0114)
[error] 5-5: Unable to import 'enforcer.dal.supabase_dal'
(E0401)
[error] 6-6: Unable to import 'enforcer.env_vars'
(E0401)
[error] 7-7: Unable to import 'enforcer.model'
(E0401)
[convention] 10-10: Missing class docstring
(C0115)
[warning] 47-47: Catching too general exception Exception
(W0718)
[warning] 48-48: Use lazy % formatting in logging functions
(W1203)
[warning] 67-67: Catching too general exception Exception
(W0718)
[warning] 68-68: Use lazy % formatting in logging functions
(W1203)
[convention] 70-70: Missing function or method docstring
(C0116)
[convention] 74-74: Missing function or method docstring
(C0116)
enforcer/resources/owner_store.py
[convention] 54-54: Line too long (108/100)
(C0301)
[convention] 61-61: Line too long (107/100)
(C0301)
[convention] 106-106: Line too long (122/100)
(C0301)
[convention] 108-108: Trailing whitespace
(C0303)
[convention] 126-126: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
[error] 6-6: Unable to import 'enforcer.env_vars'
(E0401)
[error] 7-7: Unable to import 'enforcer.model'
(E0401)
[error] 8-8: Unable to import 'enforcer.resources.kubernetes_resource_loader'
(E0401)
[convention] 11-11: Missing class docstring
(C0115)
[convention] 32-32: Missing function or method docstring
(C0116)
[convention] 36-36: Missing function or method docstring
(C0116)
[warning] 62-62: Catching too general exception Exception
(W0718)
[warning] 63-63: Use lazy % formatting in logging functions
(W1203)
[convention] 67-67: Missing function or method docstring
(C0116)
[warning] 71-71: Redefining built-in 'object'
(W0622)
[warning] 68-68: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 96-96: Use lazy % formatting in logging functions
(W1203)
[warning] 117-117: Catching too general exception Exception
(W0718)
[warning] 118-118: Use lazy % formatting in logging functions
(W1203)
[warning] 118-118: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 117-118: Unused variable 'e'
(W0612)
[convention] 120-120: Missing function or method docstring
(C0116)
[convention] 124-124: Missing function or method docstring
(C0116)
enforcer/model.py
[convention] 49-49: Line too long (104/100)
(C0301)
[convention] 65-65: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
[error] 4-4: Unable to import 'pydantic'
(E0401)
[convention] 7-7: Missing class docstring
(C0115)
[refactor] 7-7: Too few public methods (0/2)
(R0903)
[convention] 12-12: Missing class docstring
(C0115)
[refactor] 12-12: Too few public methods (0/2)
(R0903)
[convention] 19-19: Missing class docstring
(C0115)
[refactor] 19-19: Too few public methods (0/2)
(R0903)
[convention] 24-24: Missing class docstring
(C0115)
[refactor] 24-24: Too few public methods (0/2)
(R0903)
[convention] 28-28: Missing class docstring
(C0115)
[convention] 32-32: Missing function or method docstring
(C0116)
[convention] 36-36: Missing function or method docstring
(C0116)
[warning] 49-49: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 53-53: Using an f-string that does not have any interpolated variables
(W1309)
[refactor] 36-36: Either all return statements in a function should return an expression, or none of them should.
(R1710)
[convention] 64-64: Missing function or method docstring
(C0116)
enforcer/env_vars.py
[convention] 24-24: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
enforcer/patch_manager.py
[convention] 129-129: Line too long (119/100)
(C0301)
[convention] 135-135: Line too long (112/100)
(C0301)
[convention] 190-190: Trailing whitespace
(C0303)
[convention] 194-194: Trailing whitespace
(C0303)
[convention] 197-197: Trailing whitespace
(C0303)
[convention] 202-202: Trailing whitespace
(C0303)
[convention] 207-207: Trailing whitespace
(C0303)
[convention] 211-211: Trailing whitespace
(C0303)
[convention] 214-214: Trailing whitespace
(C0303)
[convention] 219-219: Trailing whitespace
(C0303)
[convention] 224-224: Trailing whitespace
(C0303)
[convention] 1-1: Missing module docstring
(C0114)
[warning] 47-47: Use lazy % formatting in logging functions
(W1203)
[warning] 101-101: Use lazy % formatting in logging functions
(W1203)
[warning] 110-110: Use lazy % formatting in logging functions
(W1203)
[warning] 117-117: Use lazy % formatting in logging functions
(W1203)
[refactor] 51-51: Too many return statements (8/6)
(R0911)
[convention] 121-121: Missing function or method docstring
(C0116)
[convention] 129-129: Missing function or method docstring
(C0116)
[convention] 135-135: Missing function or method docstring
(C0116)
[refactor] 135-135: Too many branches (16/12)
(R0912)
[warning] 200-200: Use lazy % formatting in logging functions
(W1203)
[warning] 205-205: Use lazy % formatting in logging functions
(W1203)
[warning] 217-217: Use lazy % formatting in logging functions
(W1203)
[warning] 222-222: Use lazy % formatting in logging functions
(W1203)
enforcer/enforcer_main.py
[convention] 4-4: Trailing whitespace
(C0303)
[convention] 15-15: Line too long (108/100)
(C0301)
[convention] 32-32: Line too long (102/100)
(C0301)
[convention] 35-35: Line too long (111/100)
(C0301)
[convention] 77-77: Line too long (110/100)
(C0301)
[convention] 104-104: Line too long (114/100)
(C0301)
[convention] 118-118: Line too long (107/100)
(C0301)
[convention] 149-149: Trailing whitespace
(C0303)
[convention] 153-153: Line too long (104/100)
(C0301)
[convention] 154-154: Trailing whitespace
(C0303)
[convention] 169-169: Line too long (101/100)
(C0301)
[convention] 172-172: Trailing whitespace
(C0303)
[convention] 204-204: Trailing whitespace
(C0303)
[convention] 206-206: Line too long (101/100)
(C0301)
[convention] 207-207: Trailing whitespace
(C0303)
[convention] 209-209: Line too long (106/100)
(C0301)
[convention] 220-220: Trailing whitespace
(C0303)
[convention] 227-227: Trailing whitespace
(C0303)
[convention] 249-249: Final newline missing
(C0304)
[convention] 249-249: Line too long (140/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[error] 7-7: Unable to import 'enforcer.utils'
(E0401)
[convention] 7-7: Import "from enforcer.utils import add_custom_certificate" should be placed at the top of the module
(C0413)
[convention] 17-17: Import "import logging" should be placed at the top of the module
(C0413)
[warning] 18-18: Reimport 'os' (imported line 2)
(W0404)
[convention] 18-18: Import "import os" should be placed at the top of the module
(C0413)
[warning] 19-19: Reimport 'sys' (imported line 1)
(W0404)
[convention] 19-19: Import "import sys" should be placed at the top of the module
(C0413)
[convention] 20-20: Import "import base64" should be placed at the top of the module
(C0413)
[convention] 21-21: Import "import json" should be placed at the top of the module
(C0413)
[convention] 22-22: Import "import re" should be placed at the top of the module
(C0413)
[convention] 23-23: Import "import time" should be placed at the top of the module
(C0413)
[convention] 24-24: Import "from typing import Dict, Any" should be placed at the top of the module
(C0413)
[error] 25-25: Unable to import 'fastapi'
(E0401)
[convention] 25-25: Import "from fastapi import FastAPI, HTTPException, Response" should be placed at the top of the module
(C0413)
[error] 26-26: Unable to import 'pydantic'
(E0401)
[convention] 26-26: Import "from pydantic import BaseModel" should be placed at the top of the module
(C0413)
[error] 27-27: Unable to import 'prometheus_client'
(E0401)
[convention] 27-27: Import "from prometheus_client import generate_latest, CONTENT_TYPE_LATEST" should be placed at the top of the module
(C0413)
[error] 29-29: Unable to import 'enforcer.dal.supabase_dal'
(E0401)
[convention] 29-29: Import "from enforcer.dal.supabase_dal import SupabaseDal" should be placed at the top of the module
(C0413)
[error] 30-30: Unable to import 'enforcer.patch_manager'
(E0401)
[convention] 30-30: Import "from enforcer.patch_manager import patch_container_resources" should be placed at the top of the module
(C0413)
[error] 31-31: Unable to import 'enforcer.model'
(E0401)
[convention] 31-31: Import "from enforcer.model import PodOwner, WorkloadRecommendation" should be placed at the top of the module
(C0413)
[error] 32-32: Unable to import 'enforcer.env_vars'
(E0401)
[convention] 32-32: Import "from enforcer.env_vars import ENFORCER_SSL_KEY_FILE, ENFORCER_SSL_CERT_FILE, KRR_MUTATION_MODE_DEFAULT" should be placed at the top of the module
(C0413)
[error] 33-33: Unable to import 'enforcer.resources.owner_store'
(E0401)
[convention] 33-33: Import "from enforcer.resources.owner_store import OwnerStore" should be placed at the top of the module
(C0413)
[error] 34-34: Unable to import 'enforcer.resources.recommendation_store'
(E0401)
[convention] 34-34: Import "from enforcer.resources.recommendation_store import RecommendationStore" should be placed at the top of the module
(C0413)
[error] 35-35: Unable to import 'enforcer.metrics'
(E0401)
[convention] 35-35: Import "from enforcer.metrics import pod_admission_mutations, replicaset_admissions, rs_owners_size, admission_duration" should be placed at the top of the module
(C0413)
[convention] 60-60: Missing class docstring
(C0115)
[refactor] 60-60: Too few public methods (0/2)
(R0903)
[convention] 65-65: Missing function or method docstring
(C0116)
[convention] 76-76: Missing function or method docstring
(C0116)
[refactor] 78-83: Unnecessary "elif" after "return", remove the leading "el" from "elif"
(R1705)
[warning] 99-99: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 115-115: Use lazy % formatting in logging functions
(W1203)
[warning] 118-118: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 134-134: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 146-146: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 160-160: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 175-175: Consider explicitly re-raising using 'raise HTTPException(status_code=500, detail=str(e)) from e'
(W0707)
[warning] 232-232: Consider explicitly re-raising using 'raise HTTPException(status_code=500, detail=str(e)) from e'
(W0707)
[error] 247-247: Unable to import 'uvicorn'
(E0401)
[convention] 17-17: standard import "logging" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 18-18: standard import "os" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 19-19: standard import "sys" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 20-20: standard import "base64" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 21-21: standard import "json" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 22-22: standard import "re" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 23-23: standard import "time" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 24-24: standard import "typing.Dict" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 25-25: third party import "fastapi.FastAPI" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 26-26: third party import "pydantic.BaseModel" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 27-27: third party import "prometheus_client.generate_latest" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 18-18: Imports from package os are not grouped
(C0412)
[convention] 19-19: Imports from package sys are not grouped
(C0412)
[convention] 29-29: Imports from package enforcer are not grouped
(C0412)
enforcer/resources/kubernetes_resource_loader.py
[convention] 1-1: Missing module docstring
(C0114)
[error] 4-4: Unable to import 'enforcer.env_vars'
(E0401)
[error] 5-5: Unable to import 'kubernetes'
(E0401)
[error] 6-6: Unable to import 'kubernetes.client'
(E0401)
[error] 7-7: Unable to import 'kubernetes'
(E0401)
[error] 9-9: Unable to import 'enforcer.model'
(E0401)
[convention] 17-17: Missing class docstring
(C0115)
[convention] 20-20: Missing function or method docstring
(C0116)
[warning] 38-38: Access to a protected member _continue of a client class
(W0212)
[refactor] 17-17: Too few public methods (1/2)
(R0903)
[convention] 5-5: third party import "kubernetes.client" should be placed before first party import "enforcer.env_vars.DISCOVERY_MAX_BATCHES"
(C0411)
[convention] 6-6: third party import "kubernetes.client.V1ReplicaSetList" should be placed before first party import "enforcer.env_vars.DISCOVERY_MAX_BATCHES"
(C0411)
[convention] 7-7: third party import "kubernetes.config" should be placed before first party import "enforcer.env_vars.DISCOVERY_MAX_BATCHES"
(C0411)
[convention] 9-9: Imports from package enforcer are not grouped
(C0412)
enforcer/dal/supabase_dal.py
[convention] 44-44: Line too long (121/100)
(C0301)
[convention] 48-48: Line too long (108/100)
(C0301)
[convention] 99-99: Line too long (132/100)
(C0301)
[convention] 117-117: Line too long (154/100)
(C0301)
[convention] 121-121: Line too long (149/100)
(C0301)
[convention] 124-124: Line too long (109/100)
(C0301)
[convention] 128-128: Line too long (108/100)
(C0301)
[convention] 131-131: Line too long (117/100)
(C0301)
[convention] 135-135: Line too long (156/100)
(C0301)
[convention] 175-175: Line too long (107/100)
(C0301)
[convention] 189-189: Line too long (103/100)
(C0301)
[convention] 194-194: Line too long (111/100)
(C0301)
[convention] 201-201: Line too long (128/100)
(C0301)
[convention] 221-221: Trailing newlines
(C0305)
[convention] 1-1: Missing module docstring
(C0114)
[error] 11-11: Unable to import 'enforcer.dal.robusta_config'
(E0401)
[error] 12-12: Unable to import 'supabase'
(E0401)
[error] 13-13: Unable to import 'supabase.lib.client_options'
(E0401)
[error] 14-14: Unable to import 'cachetools'
(E0401)
[error] 15-15: Unable to import 'postgrest._sync.request_builder'
(E0401)
[error] 16-16: Unable to import 'postgrest.exceptions'
(E0401)
[error] 18-26: Unable to import 'enforcer.env_vars'
(E0401)
[warning] 28-28: os.getenv default type is builtins.int. Expected str or None.
(W1508)
[convention] 34-34: Missing class docstring
(C0115)
[refactor] 34-34: Too many instance attributes (12/7)
(R0902)
[warning] 47-49: Use lazy % formatting in logging functions
(W1203)
[convention] 58-58: Missing function or method docstring
(C0116)
[refactor] 67-77: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[warning] 88-88: Use lazy % formatting in logging functions
(W1203)
[warning] 88-88: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 94-96: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('binascii.Error encountered. The Robusta UI token is not a valid base64.') from exc'
(W0707)
[warning] 94-96: Raising too general exception: Exception
(W0719)
[warning] 98-100: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('json.JSONDecodeError encountered. The Robusta UI token could not be parsed as JSON after being base64 decoded.') from exc'
(W0707)
[warning] 98-100: Raising too general exception: Exception
(W0719)
[warning] 103-103: Use lazy % formatting in logging functions
(W1203)
[warning] 106-106: Use lazy % formatting in logging functions
(W1203)
[warning] 107-107: Using open without explicitly specifying an encoding
(W1514)
[warning] 114-118: Raising too general exception: Exception
(W0719)
[warning] 130-132: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('binascii.Error encountered. The robusta token provided to Holmes is not a valid base64.') from exc'
(W0707)
[warning] 130-132: Raising too general exception: Exception
(W0719)
[warning] 134-136: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('json.JSONDecodeError encountered. The Robusta token provided to Holmes could not be parsed as JSON after being base64 decoded.') from exc'
(W0707)
[warning] 134-136: Raising too general exception: Exception
(W0719)
[warning] 159-159: Raising too general exception: Exception
(W0719)
[convention] 164-164: Missing function or method docstring
(C0116)
[convention] 175-175: Missing function or method docstring
(C0116)
[warning] 216-216: Catching too general exception Exception
(W0718)
[convention] 188-188: Do not use len(SEQUENCE) without comparison to determine if a sequence is empty
(C1802)
[warning] 189-189: Use lazy % formatting in logging functions
(W1203)
[warning] 194-194: Use lazy % formatting in logging functions
(W1203)
[warning] 201-201: Use lazy % formatting in logging functions
(W1203)
[convention] 212-212: Do not use len(SEQUENCE) without comparison to determine if a sequence is empty
(C1802)
[refactor] 175-175: Too many return statements (7/6)
(R0911)
[convention] 12-12: third party import "supabase.create_client" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 13-13: third party import "supabase.lib.client_options.ClientOptions" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 14-14: third party import "cachetools.TTLCache" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 15-15: third party import "postgrest._sync.request_builder.SyncQueryRequestBuilder" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 16-16: third party import "postgrest.exceptions.APIError" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 18-26: Imports from package enforcer are not grouped
(C0412)
🪛 Shellcheck (0.10.0)
helm/upload_chart.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🪛 Ruff (0.11.9)
enforcer/resources/recommendation_store.py
41-41: Use scan_recommendations.get(store_key) instead of scan_recommendations.get(store_key, None)
Replace scan_recommendations.get(store_key, None) with scan_recommendations.get(store_key)
(SIM910)
enforcer/resources/owner_store.py
68-68: f-string without any placeholders
Remove extraneous f prefix
(F541)
117-117: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
118-118: f-string without any placeholders
Remove extraneous f prefix
(F541)
enforcer/model.py
46-46: Use recommended.get("limit") instead of recommended.get("limit", None)
Replace recommended.get("limit", None) with recommended.get("limit")
(SIM910)
49-49: f-string without any placeholders
Remove extraneous f prefix
(F541)
53-53: f-string without any placeholders
Remove extraneous f prefix
(F541)
enforcer/patch_manager.py
246-247: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
enforcer/enforcer_main.py
99-99: f-string without any placeholders
Remove extraneous f prefix
(F541)
118-118: f-string without any placeholders
Remove extraneous f prefix
(F541)
134-134: f-string without any placeholders
Remove extraneous f prefix
(F541)
146-146: f-string without any placeholders
Remove extraneous f prefix
(F541)
160-160: f-string without any placeholders
Remove extraneous f prefix
(F541)
175-175: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
232-232: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
enforcer/dal/supabase_dal.py
88-88: f-string without any placeholders
Remove extraneous f prefix
(F541)
94-96: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
98-100: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
111-111: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
130-132: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
134-136: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 LanguageTool
enforcer/README.md
[misspelling] ~83-~83: This word is normally spelled with a hyphen.
Context: ...e64-encoded custom CA certificate - for self signed certificates | "" | | `serviceMonitor...
(EN_COMPOUNDS_SELF_SIGNED)
[uncategorized] ~200-~200: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...TLS certificates are properly configured and valid 2. Permission Denied: Verify ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~202-~202: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...k that KRR has generated recommendations and they're accessible 4. **Webhook Timeout...
(COMMA_COMPOUND_SENTENCE)
🔇 Additional comments (17)
helm/krr-enforcer/.helmignore (1)
1-24: LGTM! Standard Helm ignore patterns are correctly configured.The
.helmignorefile follows Helm best practices by excluding common system files, version control directories, backup files, and IDE-specific files from the package build.enforcer/metrics.py (1)
4-25: Well-designed metrics for webhook monitoring.The Prometheus metrics are well-structured and follow naming conventions. The labels provide good observability into the enforcer's behavior:
pod_admission_mutationstracks mutation success/failurereplicaset_admissionsmonitors ReplicaSet operationsrs_owners_sizeprovides insight into memory usageadmission_durationhelps with performance monitoring🧰 Tools
🪛 Pylint (3.3.7)
[convention] 25-25: Final newline missing
(C0304)
helm/krr-enforcer/Chart.yaml (1)
1-10: LGTM! Well-structured Helm chart descriptor.The chart structure follows Helm best practices with appropriate version management for CI/CD workflows.
helm/upload_chart.sh (1)
4-12: LGTM! Chart packaging and upload logic is sound.The Helm chart packaging, index merging, and GCS upload workflow is correctly implemented. The cache control header update is a good practice for ensuring fresh index downloads.
helm/krr-enforcer/templates/enforcer-service-account.yaml (2)
19-25: LGTM! Appropriate RBAC permissions for the enforcer webhook.The ClusterRole grants the minimum necessary permissions (get, list, watch) on pods and replicasets, which aligns with the enforcer's functionality as a mutating webhook that tracks ReplicaSet ownership.
1-40: Well-structured Kubernetes RBAC configuration.The ServiceAccount, ClusterRole, and ClusterRoleBinding are properly configured with dynamic naming using Helm templating. The YAMLlint syntax error appears to be a false positive as the Helm template syntax is valid.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 4-4: syntax error: expected , but found ''
(syntax)
enforcer/dal/robusta_config.py (1)
5-14: LGTM! Well-structured Pydantic models for configuration.The models provide proper type validation for Robusta configuration and authentication data. The pylint warnings about "too few public methods" are false positives as these are data models, not classes requiring behavior.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 14-14: Final newline missing
(C0304)
[convention] 5-5: Missing class docstring
(C0115)
[refactor] 5-5: Too few public methods (0/2)
(R0903)
[convention] 9-9: Missing class docstring
(C0115)
[refactor] 9-9: Too few public methods (0/2)
(R0903)
enforcer/Dockerfile (1)
8-11: Good security practice for CVE mitigation.Installing libexpat1 to address the specified CVEs is a proactive security measure. The cleanup of apt cache helps keep the image size minimal.
helm/krr-enforcer/templates/enforcer-webhook.yaml (2)
26-27: Good configuration for production readiness.The failure policy "Ignore" and 30-second timeout are appropriate for a webhook that shouldn't block pod creation if it fails.
22-25:Details
❓ Verification inconclusive
Verify ReplicaSet DELETE operations are necessary.
The webhook listens for both CREATE and DELETE operations on ReplicaSets. Ensure DELETE operations are actually needed for the enforcer's functionality.
🏁 Script executed:
#!/bin/bash # Description: Search for DELETE operation handling in enforcer code # Expected: Find code that handles ReplicaSet deletion events rg -A 10 -B 5 "DELETE.*replicaset|replicaset.*DELETE" --type py ast-grep --pattern 'def $_(request): $$$ if request.operation == "DELETE": $$$'Length of output: 155
Confirm necessity of ReplicaSet DELETE operations
I searched the enforcer codebase for any handling of ReplicaSet “DELETE” events but didn’t find matching logic. Please verify whether DELETE operations on ReplicaSets are actually required for the webhook’s functionality. If you don’t need to react to ReplicaSet deletions, remove
"DELETE"from the operations list.• File:
helm/krr-enforcer/templates/enforcer-webhook.yaml
Lines 22–25- operations: ["CREATE", "DELETE"] + # If DELETE handling isn’t implemented, remove it: + - operations: ["CREATE"]helm/krr-enforcer/templates/service-monitor.yaml (1)
1-36: Well-structured ServiceMonitor configuration.The conditional rendering and label matching are properly configured. The metrics endpoint path and port selection align with typical Prometheus setups.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
helm/krr-enforcer/values.yaml (2)
23-30: Excellent security context configuration.The security settings properly disable privilege escalation, run as non-root user (1000), and disable privileged mode. This follows Kubernetes security best practices.
35-40: ServiceMonitor configuration looks good.The enabled service monitor with reasonable scraping intervals (30s/10s) will provide good observability for the enforcer component.
enforcer/resources/kubernetes_resource_loader.py (2)
28-36: Good implementation of owner reference handling.The logic correctly checks for owner references and safely accesses the first owner. The batch loading pattern efficiently handles large clusters.
23-26:Details
❓ Verification inconclusive
Verify batch size configuration values.
Ensure the environment variables
DISCOVERY_MAX_BATCHESandDISCOVERY_BATCH_SIZEhave reasonable defaults to prevent excessive API calls or memory usage.
🏁 Script executed:
#!/bin/bash # Description: Check the default values for batch size configuration # Expected: Find reasonable default values for batch processing rg -A 5 -B 5 "DISCOVERY_MAX_BATCHES|DISCOVERY_BATCH_SIZE" --type pyLength of output: 2746
Verify default discovery batch configurations
We’ve confirmed in
enforcer/env_vars.pythat:
DISCOVERY_MAX_BATCHESdefaults to50DISCOVERY_BATCH_SIZEdefaults to30000Together these could fetch up to 1.5 million ReplicaSets in one run. Please ensure these values are appropriate for your cluster size and performance constraints to avoid excessive API calls or high memory usage.
Files to review:
- enforcer/env_vars.py (lines defining
DISCOVERY_MAX_BATCHES,DISCOVERY_BATCH_SIZE)- enforcer/resources/kubernetes_resource_loader.py (usage of those values)
enforcer/patch_manager.py (2)
14-49: Well-implemented CPU parsing function.The function correctly handles both millicpu format and regular CPU values with proper error handling.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 47-47: Use lazy % formatting in logging functions
(W1203)
51-119: Comprehensive memory parsing implementation.The function correctly handles all Kubernetes memory formats including binary and decimal suffixes.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 101-101: Use lazy % formatting in logging functions
(W1203)
[warning] 110-110: Use lazy % formatting in logging functions
(W1203)
[warning] 117-117: Use lazy % formatting in logging functions
(W1203)
[refactor] 51-51: Too many return statements (8/6)
(R0911)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (12)
enforcer/resources/kubernetes_resource_loader.py (1)
1-11: Fix import order and add module docstring.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[error] 5-5: Unable to import 'enforcer.env_vars'
(E0401)
[error] 6-6: Unable to import 'kubernetes'
(E0401)
[error] 7-7: Unable to import 'kubernetes.client'
(E0401)
[error] 8-8: Unable to import 'kubernetes'
(E0401)
[error] 10-10: Unable to import 'enforcer.model'
(E0401)
[convention] 6-6: third party import "kubernetes.client" should be placed before first party import "enforcer.env_vars.DISCOVERY_MAX_BATCHES"
(C0411)
[convention] 7-7: third party import "kubernetes.client.V1ReplicaSetList" should be placed before first party import "enforcer.env_vars.DISCOVERY_MAX_BATCHES"
(C0411)
[convention] 8-8: third party import "kubernetes.config" should be placed before first party import "enforcer.env_vars.DISCOVERY_MAX_BATCHES"
(C0411)
[convention] 10-10: Imports from package enforcer are not grouped
(C0412)
enforcer/resources/recommendation_store.py (2)
49-50: Improve exception handling specificity and logging format.Additionally, apply lazy logging format fixes:
- logging.exception(f"Failed to load container recommendation: {container_recommendation}") + logging.exception("Failed to load container recommendation: %s", container_recommendation) - logging.error(f"Failed to reload recommendations: {e}") + logging.error("Failed to reload recommendations: %s", e)Also applies to: 70-71
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 50-50: Line too long (105/100)
(C0301)
[warning] 49-49: Catching too general exception Exception
(W0718)
[warning] 50-50: Use lazy % formatting in logging functions
(W1203)
73-75: Add graceful shutdown timeout.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 73-73: Missing function or method docstring
(C0116)
enforcer/resources/owner_store.py (1)
154-156: Add graceful shutdown timeout and fix formatting.Similar to previous recommendations, the stop method needs timeout for graceful shutdown and fix the missing newline.
def stop(self): self._stop_event.set() - self._cleanup_thread.join() + self._cleanup_thread.join(timeout=30.0) + if self._cleanup_thread.is_alive(): + logging.warning("Owner store cleanup thread did not stop gracefully") +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 156-156: Final newline missing
(C0304)
[convention] 154-154: Missing function or method docstring
(C0116)
enforcer/dal/supabase_dal.py (4)
10-26: Fix import ordering.Third-party imports should come before first-party imports as mentioned in previous reviews.
import yaml -from enforcer.dal.robusta_config import RobustaConfig, RobustaToken from supabase import create_client from supabase.lib.client_options import ClientOptions from cachetools import TTLCache from postgrest._sync.request_builder import SyncQueryRequestBuilder from postgrest.exceptions import APIError as PGAPIError +from enforcer.dal.robusta_config import RobustaConfig, RobustaToken from enforcer.env_vars import ( ROBUSTA_CONFIG_PATH, ROBUSTA_ACCOUNT_ID, STORE_URL, STORE_API_KEY, STORE_EMAIL, STORE_PASSWORD, SCAN_AGE_HOURS_THRESHOLD, )🧰 Tools
🪛 Pylint (3.3.7)
[error] 11-11: Unable to import 'enforcer.dal.robusta_config'
(E0401)
[error] 12-12: Unable to import 'supabase'
(E0401)
[error] 13-13: Unable to import 'supabase.lib.client_options'
(E0401)
[error] 14-14: Unable to import 'cachetools'
(E0401)
[error] 15-15: Unable to import 'postgrest._sync.request_builder'
(E0401)
[error] 16-16: Unable to import 'postgrest.exceptions'
(E0401)
[error] 18-26: Unable to import 'enforcer.env_vars'
(E0401)
[convention] 12-12: third party import "supabase.create_client" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 13-13: third party import "supabase.lib.client_options.ClientOptions" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 14-14: third party import "cachetools.TTLCache" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 15-15: third party import "postgrest._sync.request_builder.SyncQueryRequestBuilder" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 16-16: third party import "postgrest.exceptions.APIError" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 18-26: Imports from package enforcer are not grouped
(C0412)
105-105: Specify encoding when opening files.Always specify encoding to avoid platform-dependent behavior as mentioned in previous reviews.
- with open(config_file_path) as file: + with open(config_file_path, encoding='utf-8') as file:🧰 Tools
🪛 Pylint (3.3.7)
[warning] 105-105: Using open without explicitly specifying an encoding
(W1514)
92-98: Improve exception handling with context.Re-raise exceptions with proper context using
fromclause as mentioned in previous reviews.except binascii.Error: raise Exception( "binascii.Error encountered. The Robusta UI token is not a valid base64." - ) + ) from None except json.JSONDecodeError: raise Exception( "json.JSONDecodeError encountered. The Robusta UI token could not be parsed as JSON after being base64 decoded." - ) + ) from None🧰 Tools
🪛 Pylint (3.3.7)
[convention] 97-97: Line too long (132/100)
(C0301)
[warning] 92-94: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('binascii.Error encountered. The Robusta UI token is not a valid base64.') from exc'
(W0707)
[warning] 92-94: Raising too general exception: Exception
(W0719)
[warning] 96-98: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('json.JSONDecodeError encountered. The Robusta UI token could not be parsed as JSON after being base64 decoded.') from exc'
(W0707)
[warning] 96-98: Raising too general exception: Exception
(W0719)
🪛 Ruff (0.11.9)
92-94: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
96-98: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
222-224: Fix inconsistent return types.The method should consistently return a tuple in all cases as mentioned in previous reviews.
except Exception: logging.exception("Supabase error while retrieving krr scan data") - return None + return None, None🧰 Tools
🪛 Pylint (3.3.7)
[warning] 222-222: Catching too general exception Exception
(W0718)
enforcer/patch_manager.py (3)
4-5: 🛠️ Refactor suggestionUse relative imports for consistency.
Since this is part of the enforcer package, use relative imports to avoid potential import issues.
-from enforcer.model import ContainerRecommendation -from enforcer.env_vars import UPDATE_THRESHOLD +from .model import ContainerRecommendation +from .env_vars import UPDATE_THRESHOLD🧰 Tools
🪛 Pylint (3.3.7)
[error] 4-4: Unable to import 'enforcer.model'
(E0401)
[error] 5-5: Unable to import 'enforcer.env_vars'
(E0401)
137-175: 🛠️ Refactor suggestionAdd documentation for complex resource update logic.
This function has complex branching logic that would benefit from clear documentation.
Add this docstring at the beginning of the function:
def get_updated_resources(resources: Dict[str, Any], recommendation: ContainerRecommendation) -> Dict[str, Any]: """ Update container resources based on recommendations. Args: resources: Current container resources dictionary recommendation: Resource recommendations containing CPU and memory recommendations Returns: Updated resources dictionary with applied recommendations Note: - Only updates if difference exceeds UPDATE_THRESHOLD - Can remove limits if recommendation.limit is None - Preserves original structure of resources dict """🧰 Tools
🪛 Pylint (3.3.7)
[convention] 137-137: Line too long (112/100)
(C0301)
[convention] 137-137: Missing function or method docstring
(C0116)
[refactor] 137-137: Too many branches (16/12)
(R0912)
244-256:⚠️ Potential issueCritical bug: Using original resources instead of deep-copied resources.
You're passing the original container resources to
get_updated_resourcesinstead of the deep-copiedresourcesvariable, which could lead to unexpected mutations.Apply this fix:
had_resources = "resources" in container resources = copy.deepcopy(container.get('resources', {})) - updated_resources = get_updated_resources(container.get('resources', {}), recommendation) + updated_resources = get_updated_resources(resources, recommendation)Additionally, simplify the nested if statements:
- if resources != updated_resources: - if validate_resources(updated_resources): - patches.append({ + if resources != updated_resources and validate_resources(updated_resources): + patches.append({🧰 Tools
🪛 Ruff (0.11.9)
248-249: Use a single
ifstatement instead of nestedifstatementsCombine
ifstatements usingand(SIM102)
enforcer/enforcer_main.py (1)
4-5: 🛠️ Refactor suggestionConsider using proper package structure instead of sys.path manipulation.
Modifying
sys.pathcan lead to unpredictable import behavior and maintenance issues.Consider restructuring the project to use proper Python packages with
__init__.pyfiles and rely onPYTHONPATHenvironment variable or use a more robust approach:-# Add parent directory to Python path so we can import enforcer modules -sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +# Ensure enforcer package is importable +from pathlib import Path +enforcer_root = Path(__file__).parent.parent +if str(enforcer_root) not in sys.path: + sys.path.insert(0, str(enforcer_root))🧰 Tools
🪛 Pylint (3.3.7)
[convention] 4-4: Trailing whitespace
(C0303)
🧹 Nitpick comments (22)
enforcer/metrics.py (1)
1-25: Add module docstring and fix formatting.The metrics definitions are well-structured, but the module needs documentation and proper formatting.
Apply this diff to add documentation and fix formatting:
+"""Prometheus metrics for monitoring KRR Enforcer webhook operations.""" from prometheus_client import Counter, Histogram, Gauge # Prometheus metrics pod_admission_mutations = Counter( 'krr_pod_admission_mutations_total', 'Total pod admission mutations', ['mutated', 'reason'] # labels: 'true' or 'false', reason for success/failure ) replicaset_admissions = Counter( 'krr_replicaset_admissions_total', 'Total replicaset admissions', ['operation'] # labels: CREATE, DELETE, etc. ) rs_owners_size = Gauge( 'krr_rs_owners_map_size', 'Current size of the rs_owners map' ) admission_duration = Histogram( 'krr_admission_duration_seconds', 'Duration of admission operations', ['kind'] # labels: Pod, ReplicaSet ) +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 25-25: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
[error] 1-1: Unable to import 'prometheus_client'
(E0401)
enforcer/resources/kubernetes_resource_loader.py (2)
18-54: Add class and method docstrings for better code documentation.The implementation logic is correct, but documentation is missing.
Apply this diff to add proper documentation:
class KubernetesResourceLoader: + """Loads Kubernetes resources with batch processing support.""" @staticmethod def load_replicasets() -> List[RsOwner]: + """ + Load ReplicaSets from all namespaces with pagination support. + + Returns a list of RsOwner objects representing ReplicaSet ownership relationships. + Uses environment-configured batch sizes and limits for scalable data retrieval. + """🧰 Tools
🪛 Pylint (3.3.7)
[convention] 34-34: Line too long (106/100)
(C0301)
[convention] 35-35: Line too long (109/100)
(C0301)
[convention] 49-49: Trailing whitespace
(C0303)
[convention] 53-53: Trailing whitespace
(C0303)
[convention] 18-18: Missing class docstring
(C0115)
[convention] 21-21: Missing function or method docstring
(C0116)
[warning] 34-34: Use lazy % formatting in logging functions
(W1203)
[warning] 46-46: Access to a protected member _continue of a client class
(W0212)
[warning] 52-52: Use lazy % formatting in logging functions
(W1203)
[refactor] 18-18: Too few public methods (1/2)
(R0903)
34-35: Fix formatting and logging issues.Address static analysis hints for better code quality.
Apply these fixes:
- logging.warning(f"ReplicasSet with multiple owner_references: {owner_references}") - controllers = [owner for owner in owner_references if owner.get("controller", False)] + logging.warning("ReplicaSet with multiple owner_references: %s", owner_references) + controllers = [owner for owner in owner_references + if owner.get("controller", False)] - continue_ref = replicasets.metadata._continue + continue_ref = replicasets.metadata.continue_ if not continue_ref: break - + if batch_num == DISCOVERY_MAX_BATCHES - 1: replicas_limit = DISCOVERY_MAX_BATCHES * DISCOVERY_BATCH_SIZE - logging.warning(f"Reached replicas loading limit: {replicas_limit}.") - + logging.warning("Reached replicas loading limit: %d", replicas_limit) +Also applies to: 46-46, 49-49, 52-53
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 34-34: Line too long (106/100)
(C0301)
[convention] 35-35: Line too long (109/100)
(C0301)
[warning] 34-34: Use lazy % formatting in logging functions
(W1203)
enforcer/resources/recommendation_store.py (2)
1-81: Add module and class docstrings for better documentation.The implementation is well-structured but lacks proper documentation.
Apply this diff to add documentation:
+""" +Recommendation store for managing workload resource recommendations. + +Maps flat container recommendations from Supabase backend to hierarchical +WorkloadRecommendation objects for use by the admission webhook. +""" import logging import threading from typing import Dict, Optional, Tuple from enforcer.dal.supabase_dal import SupabaseDal from enforcer.env_vars import SCAN_RELOAD_INTERVAL from enforcer.model import WorkloadRecommendation, ContainerRecommendation class RecommendationStore: + """ + Thread-safe store for workload resource recommendations. + + Maintains recommendations fetched from Supabase backend and automatically + reloads them in the background at configured intervals. + """🧰 Tools
🪛 Ruff (0.11.9)
43-43: Use
scan_recommendations.get(store_key)instead ofscan_recommendations.get(store_key, None)Replace
scan_recommendations.get(store_key, None)withscan_recommendations.get(store_key)(SIM910)
🪛 Pylint (3.3.7)
[convention] 25-25: Line too long (141/100)
(C0301)
[convention] 42-42: Line too long (103/100)
(C0301)
[convention] 43-43: Line too long (111/100)
(C0301)
[convention] 48-48: Line too long (102/100)
(C0301)
[convention] 50-50: Line too long (105/100)
(C0301)
[convention] 77-77: Line too long (108/100)
(C0301)
[convention] 80-80: Trailing newlines
(C0305)
[convention] 1-1: Missing module docstring
(C0114)
[error] 5-5: Unable to import 'enforcer.dal.supabase_dal'
(E0401)
[error] 6-6: Unable to import 'enforcer.env_vars'
(E0401)
[error] 7-7: Unable to import 'enforcer.model'
(E0401)
[convention] 10-10: Missing class docstring
(C0115)
[warning] 49-49: Catching too general exception Exception
(W0718)
[warning] 50-50: Use lazy % formatting in logging functions
(W1203)
[warning] 70-70: Catching too general exception Exception
(W0718)
[warning] 71-71: Use lazy % formatting in logging functions
(W1203)
[convention] 73-73: Missing function or method docstring
(C0116)
[convention] 77-77: Missing function or method docstring
(C0116)
43-43: Apply minor code style improvements.Address static analysis suggestions for cleaner code.
Apply these minor improvements:
- workload_recommendation: WorkloadRecommendation = scan_recommendations.get(store_key, None) + workload_recommendation: WorkloadRecommendation = scan_recommendations.get(store_key) def get_recommendations(self, name: str, namespace: str, kind: str) -> Optional[WorkloadRecommendation]: + """Get recommendations for a specific workload.""" with self._recommendations_lock: return self.recommendations.get(self._store_key(name, namespace, kind)) -Also applies to: 77-79
🧰 Tools
🪛 Ruff (0.11.9)
43-43: Use
scan_recommendations.get(store_key)instead ofscan_recommendations.get(store_key, None)Replace
scan_recommendations.get(store_key, None)withscan_recommendations.get(store_key)(SIM910)
🪛 Pylint (3.3.7)
[convention] 43-43: Line too long (111/100)
(C0301)
enforcer/resources/owner_store.py (3)
54-54: Remove unnecessary f-string prefix.The f-string doesn't contain any placeholders, making the prefix redundant.
- logging.exception(f"Failed to load ReplicaSet owners") + logging.exception("Failed to load ReplicaSet owners")🧰 Tools
🪛 Ruff (0.11.9)
54-54: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[warning] 54-54: Use lazy % formatting in logging functions
(W1203)
[warning] 54-54: Using an f-string that does not have any interpolated variables
(W1309)
147-148: Fix unused variable and f-string formatting.The exception variable is captured but never used, and the f-string lacks placeholders.
- except Exception as e: - logging.exception(f"Failed to cleanup deleted replicasets") + except Exception: + logging.exception("Failed to cleanup deleted replicasets")🧰 Tools
🪛 Ruff (0.11.9)
147-147: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
148-148: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[warning] 147-147: Catching too general exception Exception
(W0718)
[warning] 148-148: Use lazy % formatting in logging functions
(W1203)
[warning] 148-148: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 147-148: Unused variable 'e'
(W0612)
98-98: Remove unnecessary f-string prefix.The logging debug statement doesn't contain placeholders.
- logging.debug(f"handle_rs_admission %s", request) + logging.debug("handle_rs_admission %s", request)🧰 Tools
🪛 Ruff (0.11.9)
98-98: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[warning] 98-98: Using an f-string that does not have any interpolated variables
(W1309)
enforcer/model.py (3)
39-39: Simplify dictionary get operation.Using
get("limit", None)is redundant sinceget()returnsNoneby default.- limit = recommended.get("limit", None) + limit = recommended.get("limit")🧰 Tools
🪛 Ruff (0.11.9)
39-39: Use
recommended.get("limit")instead ofrecommended.get("limit", None)Replace
recommended.get("limit", None)withrecommended.get("limit")(SIM910)
63-63: Simplify dictionary get operation.The default value
Noneis redundant sinceget()returnsNoneby default.- return self.container_recommendations.get(container, None) + return self.container_recommendations.get(container)
67-67: Add missing final newline.The file should end with a newline character for proper formatting.
def add(self, container: str, recommendation: ContainerRecommendation): self.container_recommendations[container] = recommendation +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 67-67: Final newline missing
(C0304)
helm/krr-enforcer/templates/enforcer-cert-job.yaml (4)
49-49: Remove trailing whitespace.Trailing spaces should be removed for clean formatting.
- DNS_NAME=${SERVICE_NAME}.${NAMESPACE}.svc + DNS_NAME=${SERVICE_NAME}.${NAMESPACE}.svc🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 49-49: trailing spaces
(trailing-spaces)
67-67: Remove trailing whitespace.Trailing spaces should be removed for clean formatting.
- EOF + EOF🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 67-67: trailing spaces
(trailing-spaces)
84-84: Remove trailing whitespace.Trailing spaces should be removed for clean formatting.
- EOF + EOF🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 84-84: trailing spaces
(trailing-spaces)
141-151: Improve error handling in certificate generation script.The bash script should have better error handling and validation to ensure certificate generation doesn't silently fail.
# Restart the enforcer deployment to pick up new certificates (only if it exists) echo "Checking if enforcer deployment exists..." if kubectl get deployment ${SERVICE_NAME} -n ${NAMESPACE} >/dev/null 2>&1; then echo "Restarting enforcer deployment..." kubectl rollout restart deployment ${SERVICE_NAME} -n ${NAMESPACE} + kubectl rollout status deployment ${SERVICE_NAME} -n ${NAMESPACE} --timeout=300s else echo "Deployment ${SERVICE_NAME} does not exist yet, skipping restart" fi + # Validate certificates were created successfully + if [[ ! -f server.crt ]] || [[ ! -f server.key ]] || [[ ! -f ca.crt ]]; then + echo "ERROR: Certificate generation failed - missing certificate files" + exit 1 + fi + echo "Job completed successfully!"🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 141-141: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
enforcer/dal/supabase_dal.py (3)
86-86: Remove unnecessary f-string prefix.The f-string doesn't contain any placeholders.
- logging.info(f"Loading Robusta env configuration - ROBUSTA_UI_TOKEN_OVERRIDE") + logging.info("Loading Robusta env configuration - ROBUSTA_UI_TOKEN_OVERRIDE")🧰 Tools
🪛 Pylint (3.3.7)
[warning] 86-86: Use lazy % formatting in logging functions
(W1203)
[warning] 86-86: Using an f-string that does not have any interpolated variables
(W1309)
🪛 Ruff (0.11.9)
86-86: f-string without any placeholders
Remove extraneous
fprefix(F541)
190-190: Remove unnecessary f-string prefix.The f-string doesn't contain any placeholders.
- logging.warning(f"Multiple latest scans found. Using newest scan") + logging.warning("Multiple latest scans found. Using newest scan")🧰 Tools
🪛 Pylint (3.3.7)
[warning] 190-190: Use lazy % formatting in logging functions
(W1203)
[warning] 190-190: Using an f-string that does not have any interpolated variables
(W1309)
🪛 Ruff (0.11.9)
190-190: f-string without any placeholders
Remove extraneous
fprefix(F541)
109-109: Simplify dictionary key check.Use
key in dictinstead ofkey in dict.keys()for better performance.- if "robusta_sink" in conf.keys(): + if "robusta_sink" in conf:🧰 Tools
🪛 Ruff (0.11.9)
109-109: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
enforcer/README.md (3)
51-54: Improve grammar and clarity in the certificate section.The certificate section could be clearer with minor grammatical improvements.
Apply these changes for better readability:
-### Certificate - -- Each helm install/upgrade, a new certificate is created and deployed for the admission webhook. -- <B>The certificate is set to expire after 1 year.</b> -- In order to avoid certificate expiration, you must upgrade the enforcer helm release, <b>at least once a year</b>. +### Certificate + +- On each helm install/upgrade, a new certificate is created and deployed for the admission webhook. +- <B>The certificate is set to expire after 1 year.</b> +- To avoid certificate expiration, you must upgrade the enforcer helm release <b>at least once a year</b>.🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: Possible missing preposition found.
Context: ...re KRR scan results ### Certificate - Each helm install/upgrade, a new certificate...(AI_HYDRA_LEO_MISSING_IN)
[style] ~53-~53: Consider a more concise word here.
Context: ...te is set to expire after 1 year. - In order to avoid certificate expiration, you must ...(IN_ORDER_TO_PREMIUM)
89-89: Fix compound word hyphenation.The compound adjective "self signed" should be hyphenated.
-| `certificate` | Base64-encoded custom CA certificate - for self signed certificates | `""` | +| `certificate` | Base64-encoded custom CA certificate - for self-signed certificates | `""` |🧰 Tools
🪛 LanguageTool
[misspelling] ~89-~89: This word is normally spelled with a hyphen.
Context: ...e64-encoded custom CA certificate - for self signed certificates |""| | `serviceMonitor...(EN_COMPOUNDS_SELF_SIGNED)
206-210: Add commas for clarity in compound sentences.Consider adding commas before "and" in compound sentences for better readability.
-1. **Certificate Errors**: Ensure TLS certificates are properly configured and valid +1. **Certificate Errors**: Ensure TLS certificates are properly configured, and valid 2. **Permission Denied**: Verify the ServiceAccount has proper RBAC permissions -3. **No Recommendations**: Check that KRR has generated recommendations and they're accessible +3. **No Recommendations**: Check that KRR has generated recommendations, and they're accessible 4. **Webhook Timeout**: Increase `timeoutSeconds` in MutatingWebhookConfiguration🧰 Tools
🪛 LanguageTool
[uncategorized] ~206-~206: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...TLS certificates are properly configured and valid 2. Permission Denied: Verify ...(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~208-~208: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...k that KRR has generated recommendations and they're accessible 4. **Webhook Timeout...(COMMA_COMPOUND_SENTENCE)
enforcer/patch_manager.py (1)
123-129: Add docstring for clarity.This function would benefit from documentation explaining its purpose and parameters.
def significant_diff(old: Optional[float], new: float, percent_threshold: float) -> bool: + """ + Check if the difference between old and new values exceeds the percentage threshold. + + Args: + old: Current value (None or 0 returns True) + new: Recommended value + percent_threshold: Threshold percentage for significant difference + + Returns: + True if difference is significant, False otherwise + """ if not old: # old cpu is none or 0 return True🧰 Tools
🪛 Pylint (3.3.7)
[convention] 123-123: Missing function or method docstring
(C0116)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
enforcer/Dockerfile(1 hunks)enforcer/README.md(1 hunks)enforcer/dal/supabase_dal.py(1 hunks)enforcer/enforcer_main.py(1 hunks)enforcer/metrics.py(1 hunks)enforcer/model.py(1 hunks)enforcer/patch_manager.py(1 hunks)enforcer/resources/kubernetes_resource_loader.py(1 hunks)enforcer/resources/owner_store.py(1 hunks)enforcer/resources/recommendation_store.py(1 hunks)helm/krr-enforcer/Chart.yaml(1 hunks)helm/krr-enforcer/templates/enforcer-cert-job.yaml(1 hunks)helm/krr-enforcer/templates/enforcer.yaml(1 hunks)helm/krr-enforcer/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- helm/krr-enforcer/Chart.yaml
- helm/krr-enforcer/values.yaml
- enforcer/Dockerfile
🧰 Additional context used
🪛 Ruff (0.11.9)
enforcer/resources/owner_store.py
54-54: f-string without any placeholders
Remove extraneous f prefix
(F541)
98-98: f-string without any placeholders
Remove extraneous f prefix
(F541)
147-147: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
148-148: f-string without any placeholders
Remove extraneous f prefix
(F541)
enforcer/model.py
39-39: Use recommended.get("limit") instead of recommended.get("limit", None)
Replace recommended.get("limit", None) with recommended.get("limit")
(SIM910)
enforcer/enforcer_main.py
97-97: f-string without any placeholders
Remove extraneous f prefix
(F541)
116-116: f-string without any placeholders
Remove extraneous f prefix
(F541)
132-132: f-string without any placeholders
Remove extraneous f prefix
(F541)
144-144: f-string without any placeholders
Remove extraneous f prefix
(F541)
159-159: f-string without any placeholders
Remove extraneous f prefix
(F541)
178-178: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
236-236: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
enforcer/patch_manager.py
248-249: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
enforcer/dal/supabase_dal.py
86-86: f-string without any placeholders
Remove extraneous f prefix
(F541)
92-94: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
96-98: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
109-109: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
127-129: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
131-133: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
190-190: f-string without any placeholders
Remove extraneous f prefix
(F541)
enforcer/resources/recommendation_store.py
43-43: Use scan_recommendations.get(store_key) instead of scan_recommendations.get(store_key, None)
Replace scan_recommendations.get(store_key, None) with scan_recommendations.get(store_key)
(SIM910)
🪛 Pylint (3.3.7)
enforcer/resources/owner_store.py
[convention] 31-31: Trailing whitespace
(C0303)
[convention] 36-36: Trailing whitespace
(C0303)
[convention] 40-40: Trailing whitespace
(C0303)
[convention] 45-45: Trailing whitespace
(C0303)
[convention] 49-49: Trailing whitespace
(C0303)
[convention] 52-52: Trailing whitespace
(C0303)
[convention] 84-84: Line too long (108/100)
(C0301)
[convention] 91-91: Line too long (107/100)
(C0301)
[convention] 136-136: Line too long (122/100)
(C0301)
[convention] 138-138: Trailing whitespace
(C0303)
[convention] 156-156: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
[error] 6-6: Unable to import 'enforcer.env_vars'
(E0401)
[error] 7-7: Unable to import 'enforcer.metrics'
(E0401)
[error] 8-8: Unable to import 'enforcer.model'
(E0401)
[error] 9-9: Unable to import 'enforcer.resources.kubernetes_resource_loader'
(E0401)
[convention] 12-12: Missing class docstring
(C0115)
[refactor] 33-33: Consider using 'with' for resource-allocating operations
(R1732)
[warning] 53-53: Catching too general exception Exception
(W0718)
[warning] 51-51: Use lazy % formatting in logging functions
(W1203)
[warning] 54-54: Use lazy % formatting in logging functions
(W1203)
[warning] 54-54: Using an f-string that does not have any interpolated variables
(W1309)
[convention] 59-59: Missing function or method docstring
(C0116)
[convention] 63-63: Missing function or method docstring
(C0116)
[warning] 92-92: Catching too general exception Exception
(W0718)
[warning] 78-78: Use lazy % formatting in logging functions
(W1203)
[warning] 93-93: Use lazy % formatting in logging functions
(W1203)
[convention] 97-97: Missing function or method docstring
(C0116)
[warning] 98-98: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 126-126: Use lazy % formatting in logging functions
(W1203)
[warning] 147-147: Catching too general exception Exception
(W0718)
[warning] 148-148: Use lazy % formatting in logging functions
(W1203)
[warning] 148-148: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 147-148: Unused variable 'e'
(W0612)
[convention] 150-150: Missing function or method docstring
(C0116)
[convention] 154-154: Missing function or method docstring
(C0116)
enforcer/model.py
[convention] 42-42: Line too long (103/100)
(C0301)
[convention] 67-67: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
[error] 4-4: Unable to import 'pydantic'
(E0401)
[convention] 7-7: Missing class docstring
(C0115)
[refactor] 7-7: Too few public methods (0/2)
(R0903)
[convention] 12-12: Missing class docstring
(C0115)
[refactor] 12-12: Too few public methods (0/2)
(R0903)
[convention] 19-19: Missing class docstring
(C0115)
[refactor] 19-19: Too few public methods (0/2)
(R0903)
[convention] 24-24: Missing class docstring
(C0115)
[convention] 29-29: Missing function or method docstring
(C0116)
[refactor] 24-24: Too few public methods (1/2)
(R0903)
[convention] 58-58: Missing class docstring
(C0115)
[convention] 62-62: Missing function or method docstring
(C0116)
[convention] 66-66: Missing function or method docstring
(C0116)
enforcer/enforcer_main.py
[convention] 4-4: Trailing whitespace
(C0303)
[convention] 15-15: Line too long (108/100)
(C0301)
[convention] 30-30: Line too long (102/100)
(C0301)
[convention] 33-33: Line too long (111/100)
(C0301)
[convention] 75-75: Line too long (110/100)
(C0301)
[convention] 102-102: Line too long (114/100)
(C0301)
[convention] 116-116: Line too long (107/100)
(C0301)
[convention] 147-147: Trailing whitespace
(C0303)
[convention] 151-151: Line too long (104/100)
(C0301)
[convention] 152-152: Trailing whitespace
(C0303)
[convention] 168-168: Line too long (101/100)
(C0301)
[convention] 171-171: Trailing whitespace
(C0303)
[convention] 188-188: Line too long (116/100)
(C0301)
[convention] 208-208: Trailing whitespace
(C0303)
[convention] 210-210: Line too long (101/100)
(C0301)
[convention] 211-211: Trailing whitespace
(C0303)
[convention] 213-213: Line too long (106/100)
(C0301)
[convention] 224-224: Trailing whitespace
(C0303)
[convention] 231-231: Trailing whitespace
(C0303)
[convention] 253-253: Final newline missing
(C0304)
[convention] 253-253: Line too long (140/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[error] 7-7: Unable to import 'enforcer.utils'
(E0401)
[convention] 7-7: Import "from enforcer.utils import add_custom_certificate" should be placed at the top of the module
(C0413)
[convention] 17-17: Import "import logging" should be placed at the top of the module
(C0413)
[convention] 18-18: Import "import base64" should be placed at the top of the module
(C0413)
[convention] 19-19: Import "import json" should be placed at the top of the module
(C0413)
[convention] 20-20: Import "import re" should be placed at the top of the module
(C0413)
[convention] 21-21: Import "import time" should be placed at the top of the module
(C0413)
[convention] 22-22: Import "from typing import Dict, Any" should be placed at the top of the module
(C0413)
[error] 23-23: Unable to import 'fastapi'
(E0401)
[convention] 23-23: Import "from fastapi import FastAPI, HTTPException, Response" should be placed at the top of the module
(C0413)
[error] 24-24: Unable to import 'pydantic'
(E0401)
[convention] 24-24: Import "from pydantic import BaseModel" should be placed at the top of the module
(C0413)
[error] 25-25: Unable to import 'prometheus_client'
(E0401)
[convention] 25-25: Import "from prometheus_client import generate_latest, CONTENT_TYPE_LATEST" should be placed at the top of the module
(C0413)
[error] 27-27: Unable to import 'enforcer.dal.supabase_dal'
(E0401)
[convention] 27-27: Import "from enforcer.dal.supabase_dal import SupabaseDal" should be placed at the top of the module
(C0413)
[error] 28-28: Unable to import 'enforcer.patch_manager'
(E0401)
[convention] 28-28: Import "from enforcer.patch_manager import patch_container_resources" should be placed at the top of the module
(C0413)
[error] 29-29: Unable to import 'enforcer.model'
(E0401)
[convention] 29-29: Import "from enforcer.model import PodOwner, WorkloadRecommendation" should be placed at the top of the module
(C0413)
[error] 30-30: Unable to import 'enforcer.env_vars'
(E0401)
[convention] 30-30: Import "from enforcer.env_vars import ENFORCER_SSL_KEY_FILE, ENFORCER_SSL_CERT_FILE, KRR_MUTATION_MODE_DEFAULT" should be placed at the top of the module
(C0413)
[error] 31-31: Unable to import 'enforcer.resources.owner_store'
(E0401)
[convention] 31-31: Import "from enforcer.resources.owner_store import OwnerStore" should be placed at the top of the module
(C0413)
[error] 32-32: Unable to import 'enforcer.resources.recommendation_store'
(E0401)
[convention] 32-32: Import "from enforcer.resources.recommendation_store import RecommendationStore" should be placed at the top of the module
(C0413)
[error] 33-33: Unable to import 'enforcer.metrics'
(E0401)
[convention] 33-33: Import "from enforcer.metrics import pod_admission_mutations, replicaset_admissions, rs_owners_size, admission_duration" should be placed at the top of the module
(C0413)
[convention] 58-58: Missing class docstring
(C0115)
[refactor] 58-58: Too few public methods (0/2)
(R0903)
[convention] 63-63: Missing function or method docstring
(C0116)
[convention] 74-74: Missing function or method docstring
(C0116)
[refactor] 76-81: Unnecessary "elif" after "return", remove the leading "el" from "elif"
(R1705)
[warning] 97-97: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 113-113: Use lazy % formatting in logging functions
(W1203)
[warning] 116-116: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 132-132: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 144-144: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 159-159: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 178-178: Consider explicitly re-raising using 'raise HTTPException(status_code=500, detail=str(e)) from e'
(W0707)
[warning] 236-236: Consider explicitly re-raising using 'raise HTTPException(status_code=500, detail=str(e)) from e'
(W0707)
[error] 251-251: Unable to import 'uvicorn'
(E0401)
[convention] 17-17: standard import "logging" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 18-18: standard import "base64" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 19-19: standard import "json" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 20-20: standard import "re" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 21-21: standard import "time" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 22-22: standard import "typing.Dict" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 23-23: third party import "fastapi.FastAPI" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 24-24: third party import "pydantic.BaseModel" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 25-25: third party import "prometheus_client.generate_latest" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 27-27: Imports from package enforcer are not grouped
(C0412)
enforcer/patch_manager.py
[convention] 131-131: Line too long (119/100)
(C0301)
[convention] 137-137: Line too long (112/100)
(C0301)
[convention] 192-192: Trailing whitespace
(C0303)
[convention] 196-196: Trailing whitespace
(C0303)
[convention] 199-199: Trailing whitespace
(C0303)
[convention] 204-204: Trailing whitespace
(C0303)
[convention] 209-209: Trailing whitespace
(C0303)
[convention] 213-213: Trailing whitespace
(C0303)
[convention] 216-216: Trailing whitespace
(C0303)
[convention] 221-221: Trailing whitespace
(C0303)
[convention] 226-226: Trailing whitespace
(C0303)
[convention] 1-1: Missing module docstring
(C0114)
[error] 4-4: Unable to import 'enforcer.model'
(E0401)
[error] 5-5: Unable to import 'enforcer.env_vars'
(E0401)
[warning] 49-49: Use lazy % formatting in logging functions
(W1203)
[warning] 103-103: Use lazy % formatting in logging functions
(W1203)
[warning] 112-112: Use lazy % formatting in logging functions
(W1203)
[warning] 119-119: Use lazy % formatting in logging functions
(W1203)
[refactor] 53-53: Too many return statements (8/6)
(R0911)
[convention] 123-123: Missing function or method docstring
(C0116)
[convention] 131-131: Missing function or method docstring
(C0116)
[convention] 137-137: Missing function or method docstring
(C0116)
[refactor] 137-137: Too many branches (16/12)
(R0912)
[warning] 202-202: Use lazy % formatting in logging functions
(W1203)
[warning] 207-207: Use lazy % formatting in logging functions
(W1203)
[warning] 219-219: Use lazy % formatting in logging functions
(W1203)
[warning] 224-224: Use lazy % formatting in logging functions
(W1203)
enforcer/dal/supabase_dal.py
[convention] 46-46: Line too long (108/100)
(C0301)
[convention] 97-97: Line too long (132/100)
(C0301)
[convention] 118-118: Line too long (149/100)
(C0301)
[convention] 121-121: Line too long (109/100)
(C0301)
[convention] 125-125: Line too long (108/100)
(C0301)
[convention] 128-128: Line too long (107/100)
(C0301)
[convention] 132-132: Line too long (146/100)
(C0301)
[convention] 172-172: Line too long (107/100)
(C0301)
[convention] 186-186: Line too long (103/100)
(C0301)
[convention] 192-192: Line too long (108/100)
(C0301)
[convention] 196-196: Trailing whitespace
(C0303)
[convention] 200-200: Line too long (111/100)
(C0301)
[convention] 207-207: Line too long (128/100)
(C0301)
[convention] 227-227: Trailing newlines
(C0305)
[convention] 1-1: Missing module docstring
(C0114)
[error] 11-11: Unable to import 'enforcer.dal.robusta_config'
(E0401)
[error] 12-12: Unable to import 'supabase'
(E0401)
[error] 13-13: Unable to import 'supabase.lib.client_options'
(E0401)
[error] 14-14: Unable to import 'cachetools'
(E0401)
[error] 15-15: Unable to import 'postgrest._sync.request_builder'
(E0401)
[error] 16-16: Unable to import 'postgrest.exceptions'
(E0401)
[error] 18-26: Unable to import 'enforcer.env_vars'
(E0401)
[warning] 28-28: os.getenv default type is builtins.int. Expected str or None.
(W1508)
[convention] 34-34: Missing class docstring
(C0115)
[refactor] 34-34: Too many instance attributes (12/7)
(R0902)
[warning] 45-47: Use lazy % formatting in logging functions
(W1203)
[convention] 56-56: Missing function or method docstring
(C0116)
[refactor] 65-75: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[warning] 86-86: Use lazy % formatting in logging functions
(W1203)
[warning] 86-86: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 92-94: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('binascii.Error encountered. The Robusta UI token is not a valid base64.') from exc'
(W0707)
[warning] 92-94: Raising too general exception: Exception
(W0719)
[warning] 96-98: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('json.JSONDecodeError encountered. The Robusta UI token could not be parsed as JSON after being base64 decoded.') from exc'
(W0707)
[warning] 96-98: Raising too general exception: Exception
(W0719)
[warning] 101-101: Use lazy % formatting in logging functions
(W1203)
[warning] 104-104: Use lazy % formatting in logging functions
(W1203)
[warning] 105-105: Using open without explicitly specifying an encoding
(W1514)
[warning] 112-115: Raising too general exception: Exception
(W0719)
[warning] 127-129: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('binascii.Error encountered. The robusta token provided is not a valid base64.') from exc'
(W0707)
[warning] 127-129: Raising too general exception: Exception
(W0719)
[warning] 131-133: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('json.JSONDecodeError encountered. The Robusta token provided could not be parsed as JSON after being base64 decoded.') from exc'
(W0707)
[warning] 131-133: Raising too general exception: Exception
(W0719)
[warning] 156-156: Raising too general exception: Exception
(W0719)
[convention] 161-161: Missing function or method docstring
(C0116)
[convention] 172-172: Missing function or method docstring
(C0116)
[warning] 222-222: Catching too general exception Exception
(W0718)
[convention] 185-185: Do not use len(SEQUENCE) without comparison to determine if a sequence is empty
(C1802)
[warning] 186-186: Use lazy % formatting in logging functions
(W1203)
[warning] 190-190: Use lazy % formatting in logging functions
(W1203)
[warning] 190-190: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 200-200: Use lazy % formatting in logging functions
(W1203)
[warning] 207-207: Use lazy % formatting in logging functions
(W1203)
[convention] 218-218: Do not use len(SEQUENCE) without comparison to determine if a sequence is empty
(C1802)
[refactor] 172-172: Too many return statements (7/6)
(R0911)
[convention] 12-12: third party import "supabase.create_client" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 13-13: third party import "supabase.lib.client_options.ClientOptions" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 14-14: third party import "cachetools.TTLCache" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 15-15: third party import "postgrest._sync.request_builder.SyncQueryRequestBuilder" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 16-16: third party import "postgrest.exceptions.APIError" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 18-26: Imports from package enforcer are not grouped
(C0412)
enforcer/metrics.py
[convention] 25-25: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
[error] 1-1: Unable to import 'prometheus_client'
(E0401)
enforcer/resources/kubernetes_resource_loader.py
[convention] 34-34: Line too long (106/100)
(C0301)
[convention] 35-35: Line too long (109/100)
(C0301)
[convention] 49-49: Trailing whitespace
(C0303)
[convention] 53-53: Trailing whitespace
(C0303)
[convention] 1-1: Missing module docstring
(C0114)
[error] 5-5: Unable to import 'enforcer.env_vars'
(E0401)
[error] 6-6: Unable to import 'kubernetes'
(E0401)
[error] 7-7: Unable to import 'kubernetes.client'
(E0401)
[error] 8-8: Unable to import 'kubernetes'
(E0401)
[error] 10-10: Unable to import 'enforcer.model'
(E0401)
[convention] 18-18: Missing class docstring
(C0115)
[convention] 21-21: Missing function or method docstring
(C0116)
[warning] 34-34: Use lazy % formatting in logging functions
(W1203)
[warning] 46-46: Access to a protected member _continue of a client class
(W0212)
[warning] 52-52: Use lazy % formatting in logging functions
(W1203)
[refactor] 18-18: Too few public methods (1/2)
(R0903)
[convention] 6-6: third party import "kubernetes.client" should be placed before first party import "enforcer.env_vars.DISCOVERY_MAX_BATCHES"
(C0411)
[convention] 7-7: third party import "kubernetes.client.V1ReplicaSetList" should be placed before first party import "enforcer.env_vars.DISCOVERY_MAX_BATCHES"
(C0411)
[convention] 8-8: third party import "kubernetes.config" should be placed before first party import "enforcer.env_vars.DISCOVERY_MAX_BATCHES"
(C0411)
[convention] 10-10: Imports from package enforcer are not grouped
(C0412)
enforcer/resources/recommendation_store.py
[convention] 25-25: Line too long (141/100)
(C0301)
[convention] 42-42: Line too long (103/100)
(C0301)
[convention] 43-43: Line too long (111/100)
(C0301)
[convention] 48-48: Line too long (102/100)
(C0301)
[convention] 50-50: Line too long (105/100)
(C0301)
[convention] 77-77: Line too long (108/100)
(C0301)
[convention] 80-80: Trailing newlines
(C0305)
[convention] 1-1: Missing module docstring
(C0114)
[error] 5-5: Unable to import 'enforcer.dal.supabase_dal'
(E0401)
[error] 6-6: Unable to import 'enforcer.env_vars'
(E0401)
[error] 7-7: Unable to import 'enforcer.model'
(E0401)
[convention] 10-10: Missing class docstring
(C0115)
[warning] 49-49: Catching too general exception Exception
(W0718)
[warning] 50-50: Use lazy % formatting in logging functions
(W1203)
[warning] 70-70: Catching too general exception Exception
(W0718)
[warning] 71-71: Use lazy % formatting in logging functions
(W1203)
[convention] 73-73: Missing function or method docstring
(C0116)
[convention] 77-77: Missing function or method docstring
(C0116)
🪛 LanguageTool
enforcer/README.md
[uncategorized] ~51-~51: Possible missing preposition found.
Context: ...re KRR scan results ### Certificate - Each helm install/upgrade, a new certificate...
(AI_HYDRA_LEO_MISSING_IN)
[style] ~53-~53: Consider a more concise word here.
Context: ...te is set to expire after 1 year. - In order to avoid certificate expiration, you must ...
(IN_ORDER_TO_PREMIUM)
[misspelling] ~89-~89: This word is normally spelled with a hyphen.
Context: ...e64-encoded custom CA certificate - for self signed certificates | "" | | `serviceMonitor...
(EN_COMPOUNDS_SELF_SIGNED)
[uncategorized] ~206-~206: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...TLS certificates are properly configured and valid 2. Permission Denied: Verify ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~208-~208: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...k that KRR has generated recommendations and they're accessible 4. **Webhook Timeout...
(COMMA_COMPOUND_SENTENCE)
🪛 YAMLlint (1.37.1)
helm/krr-enforcer/templates/enforcer-cert-job.yaml
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 84-84: trailing spaces
(trailing-spaces)
[error] 141-141: trailing spaces
(trailing-spaces)
[error] 150-150: trailing spaces
(trailing-spaces)
[error] 4-4: syntax error: expected , but found ''
(syntax)
helm/krr-enforcer/templates/enforcer.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (6)
helm/krr-enforcer/templates/enforcer.yaml (2)
61-65: Image configuration logic is correctly implemented.The image configuration logic properly handles both
fullImageand constructed image paths, addressing the previous review concern.
1-154: Well-structured Kubernetes template following best practices.The Helm template correctly implements:
- Proper labeling and selection
- TLS certificate mounting
- Health checks with appropriate timeouts
- Configurable resources and security contexts
- Service account integration
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 4-4: syntax error: expected , but found ''
(syntax)
enforcer/patch_manager.py (2)
42-44: Good implementation of CPU suffix handling.The function correctly handles both 'm' (millicpu) and 'k' (kilo) suffixes, addressing the previous review comment about real-world usage.
176-228: Well-implemented resource validation.The validation function properly addresses the concern from previous reviews about ensuring limits are not less than requests. The documentation clearly explains the validation rules.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 192-192: Trailing whitespace
(C0303)
[convention] 196-196: Trailing whitespace
(C0303)
[convention] 199-199: Trailing whitespace
(C0303)
[convention] 204-204: Trailing whitespace
(C0303)
[convention] 209-209: Trailing whitespace
(C0303)
[convention] 213-213: Trailing whitespace
(C0303)
[convention] 216-216: Trailing whitespace
(C0303)
[convention] 221-221: Trailing whitespace
(C0303)
[convention] 226-226: Trailing whitespace
(C0303)
[warning] 202-202: Use lazy % formatting in logging functions
(W1203)
[warning] 207-207: Use lazy % formatting in logging functions
(W1203)
[warning] 219-219: Use lazy % formatting in logging functions
(W1203)
[warning] 224-224: Use lazy % formatting in logging functions
(W1203)
enforcer/enforcer_main.py (2)
188-189: Verify the initialization timing for owner store.The comment suggests owner initialization happens after accepting API requests, which could lead to missing owner information for early admission requests.
Could this lead to a race condition where early pod admission requests are processed before the owner store is fully initialized? Consider initializing the owner store before the server starts accepting requests, or document why this lazy initialization is safe.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 188-188: Line too long (116/100)
(C0301)
246-248: Good practice: Updating metrics before returning.Updating the ReplicaSet owners count metric before generating the response ensures that Prometheus always gets the most current value.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
enforcer/enforcer_main.py (4)
4-5: Persistent sys.path manipulation issue remains unaddressed.This is a duplicate of a previous review comment. The risky
sys.pathmanipulation approach is still present despite earlier feedback recommending safer alternatives.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 4-4: Trailing whitespace
(C0303)
97-97: Remove unnecessary f-string prefix from logging statement.This is a duplicate of a previous review comment. The logging statement uses an f-string without variable interpolation while using % formatting.
116-116: Remove unnecessary f-string prefixes from logging statements.These logging statements are duplicates of previous review comments. They use f-strings without variable interpolation while using % formatting.
Also applies to: 132-132, 144-144, 159-159
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 116-116: Line too long (106/100)
(C0301)
172-178: Missing exception chaining in error handling.This is a duplicate of a previous review comment. Exception chaining should be used to preserve the original traceback for better debugging.
Also applies to: 232-236
🧰 Tools
🪛 Ruff (0.11.9)
178-178: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
🪛 Pylint (3.3.7)
[warning] 178-178: Consider explicitly re-raising using 'raise HTTPException(status_code=500, detail=str(e)) from e'
(W0707)
🧹 Nitpick comments (5)
enforcer/enforcer_main.py (5)
74-82: Simplify conditional logic by removing unnecessary elif.The
elifstatement after areturnis unnecessary and can be simplified for better readability.def enforce_pod(pod: Dict[str, Any]) -> bool: mode = pod.get('metadata', {}).get('annotations', {}).get("admission.robusta.dev/krr-mutation-mode", None) if mode == ENFORCE: return True - elif mode == IGNORE: + if mode == IGNORE: return False - else: - return KRR_MUTATION_MODE_DEFAULT == ENFORCE + return KRR_MUTATION_MODE_DEFAULT == ENFORCE🧰 Tools
🪛 Pylint (3.3.7)
[convention] 75-75: Line too long (110/100)
(C0301)
[convention] 74-74: Missing function or method docstring
(C0116)
[refactor] 76-81: Unnecessary "elif" after "return", remove the leading "el" from "elif"
(R1705)
113-113: Use lazy % formatting in logging functions.The logging statement should use % formatting instead of f-string for better performance, as suggested by static analysis.
- logger.warning(f"Received unexpected resource mutation: {kind}") + logger.warning("Received unexpected resource mutation: %s", kind)🧰 Tools
🪛 Pylint (3.3.7)
[warning] 113-113: Use lazy % formatting in logging functions
(W1203)
188-188: Consider moving owner initialization to application startup.The health check endpoint triggers owner initialization, which seems like a side effect for a health check. Consider moving this to a proper application startup event.
@app.on_event("startup") async def startup_event(): """Initialize resources on application startup.""" owner_store.finalize_owner_initialization() @app.get("/health") async def health_check(): """Health check endpoint.""" return {"status": "healthy"}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 188-188: Line too long (116/100)
(C0301)
58-61: Add docstring to AdmissionReview model.The Pydantic model lacks documentation explaining its purpose and structure.
class AdmissionReview(BaseModel): + """Kubernetes AdmissionReview request model.""" apiVersion: str kind: str request: Dict[str, Any]🧰 Tools
🪛 Pylint (3.3.7)
[convention] 58-58: Missing class docstring
(C0115)
[refactor] 58-58: Too few public methods (0/2)
(R0903)
63-72: Add docstring to admission_allowed function.The function lacks documentation explaining its purpose and return structure.
def admission_allowed(request: AdmissionReview) -> Dict[str, Any]: + """ + Generate an admission response that allows the request without modifications. + + Args: + request: The admission review request + + Returns: + Dict containing the admission response allowing the request + """🧰 Tools
🪛 Pylint (3.3.7)
[convention] 63-63: Missing function or method docstring
(C0116)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
enforcer/enforcer_main.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
enforcer/enforcer_main.py
178-178: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
236-236: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 Pylint (3.3.7)
enforcer/enforcer_main.py
[convention] 4-4: Trailing whitespace
(C0303)
[convention] 15-15: Line too long (108/100)
(C0301)
[convention] 30-30: Line too long (102/100)
(C0301)
[convention] 33-33: Line too long (111/100)
(C0301)
[convention] 75-75: Line too long (110/100)
(C0301)
[convention] 102-102: Line too long (114/100)
(C0301)
[convention] 116-116: Line too long (106/100)
(C0301)
[convention] 147-147: Trailing whitespace
(C0303)
[convention] 151-151: Line too long (104/100)
(C0301)
[convention] 152-152: Trailing whitespace
(C0303)
[convention] 168-168: Line too long (101/100)
(C0301)
[convention] 171-171: Trailing whitespace
(C0303)
[convention] 188-188: Line too long (116/100)
(C0301)
[convention] 208-208: Trailing whitespace
(C0303)
[convention] 210-210: Line too long (101/100)
(C0301)
[convention] 211-211: Trailing whitespace
(C0303)
[convention] 213-213: Line too long (106/100)
(C0301)
[convention] 224-224: Trailing whitespace
(C0303)
[convention] 231-231: Trailing whitespace
(C0303)
[convention] 253-253: Final newline missing
(C0304)
[convention] 253-253: Line too long (140/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[error] 7-7: Unable to import 'enforcer.utils'
(E0401)
[convention] 7-7: Import "from enforcer.utils import add_custom_certificate" should be placed at the top of the module
(C0413)
[convention] 17-17: Import "import logging" should be placed at the top of the module
(C0413)
[convention] 18-18: Import "import base64" should be placed at the top of the module
(C0413)
[convention] 19-19: Import "import json" should be placed at the top of the module
(C0413)
[convention] 20-20: Import "import re" should be placed at the top of the module
(C0413)
[convention] 21-21: Import "import time" should be placed at the top of the module
(C0413)
[convention] 22-22: Import "from typing import Dict, Any" should be placed at the top of the module
(C0413)
[error] 23-23: Unable to import 'fastapi'
(E0401)
[convention] 23-23: Import "from fastapi import FastAPI, HTTPException, Response" should be placed at the top of the module
(C0413)
[error] 24-24: Unable to import 'pydantic'
(E0401)
[convention] 24-24: Import "from pydantic import BaseModel" should be placed at the top of the module
(C0413)
[error] 25-25: Unable to import 'prometheus_client'
(E0401)
[convention] 25-25: Import "from prometheus_client import generate_latest, CONTENT_TYPE_LATEST" should be placed at the top of the module
(C0413)
[error] 27-27: Unable to import 'enforcer.dal.supabase_dal'
(E0401)
[convention] 27-27: Import "from enforcer.dal.supabase_dal import SupabaseDal" should be placed at the top of the module
(C0413)
[error] 28-28: Unable to import 'enforcer.patch_manager'
(E0401)
[convention] 28-28: Import "from enforcer.patch_manager import patch_container_resources" should be placed at the top of the module
(C0413)
[error] 29-29: Unable to import 'enforcer.model'
(E0401)
[convention] 29-29: Import "from enforcer.model import PodOwner, WorkloadRecommendation" should be placed at the top of the module
(C0413)
[error] 30-30: Unable to import 'enforcer.env_vars'
(E0401)
[convention] 30-30: Import "from enforcer.env_vars import ENFORCER_SSL_KEY_FILE, ENFORCER_SSL_CERT_FILE, KRR_MUTATION_MODE_DEFAULT" should be placed at the top of the module
(C0413)
[error] 31-31: Unable to import 'enforcer.resources.owner_store'
(E0401)
[convention] 31-31: Import "from enforcer.resources.owner_store import OwnerStore" should be placed at the top of the module
(C0413)
[error] 32-32: Unable to import 'enforcer.resources.recommendation_store'
(E0401)
[convention] 32-32: Import "from enforcer.resources.recommendation_store import RecommendationStore" should be placed at the top of the module
(C0413)
[error] 33-33: Unable to import 'enforcer.metrics'
(E0401)
[convention] 33-33: Import "from enforcer.metrics import pod_admission_mutations, replicaset_admissions, rs_owners_size, admission_duration" should be placed at the top of the module
(C0413)
[convention] 58-58: Missing class docstring
(C0115)
[refactor] 58-58: Too few public methods (0/2)
(R0903)
[convention] 63-63: Missing function or method docstring
(C0116)
[convention] 74-74: Missing function or method docstring
(C0116)
[refactor] 76-81: Unnecessary "elif" after "return", remove the leading "el" from "elif"
(R1705)
[warning] 113-113: Use lazy % formatting in logging functions
(W1203)
[warning] 178-178: Consider explicitly re-raising using 'raise HTTPException(status_code=500, detail=str(e)) from e'
(W0707)
[warning] 236-236: Consider explicitly re-raising using 'raise HTTPException(status_code=500, detail=str(e)) from e'
(W0707)
[error] 251-251: Unable to import 'uvicorn'
(E0401)
[convention] 17-17: standard import "logging" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 18-18: standard import "base64" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 19-19: standard import "json" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 20-20: standard import "re" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 21-21: standard import "time" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 22-22: standard import "typing.Dict" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 23-23: third party import "fastapi.FastAPI" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 24-24: third party import "pydantic.BaseModel" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 25-25: third party import "prometheus_client.generate_latest" should be placed before first party import "enforcer.utils.add_custom_certificate"
(C0411)
[convention] 27-27: Imports from package enforcer are not grouped
(C0412)
🔇 Additional comments (1)
enforcer/enforcer_main.py (1)
246-247: Reconsider updating metrics in the metrics endpoint.Based on past review comments, there was discussion about whether to update metrics here. The current approach updates the metric right before returning, which could affect metric accuracy if multiple requests are handled concurrently.
Consider whether updating metrics in the
/metricsendpoint is the best approach, or if metrics should be updated at the source of the state changes.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
enforcer/dal/supabase_dal.py (5)
1-28: 🛠️ Refactor suggestionFix import ordering and add module docstring.
Third-party imports should come before first-party imports according to PEP 8.
Apply this diff to fix the import ordering:
+"""Data access layer for interacting with Supabase backend to manage KRR scan data.""" import base64 import binascii import json import logging import os import threading from datetime import datetime, timezone, timedelta from typing import Dict, Optional, List import yaml -from enforcer.dal.robusta_config import RobustaConfig, RobustaToken -from supabase import create_client -from supabase.lib.client_options import ClientOptions from cachetools import TTLCache from postgrest._sync.request_builder import SyncQueryRequestBuilder from postgrest.exceptions import APIError as PGAPIError +from supabase import create_client +from supabase.lib.client_options import ClientOptions + +from enforcer.dal.robusta_config import RobustaConfig, RobustaToken from enforcer.env_vars import ( ROBUSTA_CONFIG_PATH, ROBUSTA_ACCOUNT_ID, STORE_URL, STORE_API_KEY, STORE_EMAIL, STORE_PASSWORD, SCAN_AGE_HOURS_THRESHOLD, ) from enforcer.params_utils import get_env_replacement🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[error] 11-11: Unable to import 'enforcer.dal.robusta_config'
(E0401)
[error] 12-12: Unable to import 'supabase'
(E0401)
[error] 13-13: Unable to import 'supabase.lib.client_options'
(E0401)
[error] 14-14: Unable to import 'cachetools'
(E0401)
[error] 15-15: Unable to import 'postgrest._sync.request_builder'
(E0401)
[error] 16-16: Unable to import 'postgrest.exceptions'
(E0401)
[error] 18-26: Unable to import 'enforcer.env_vars'
(E0401)
[error] 27-27: Unable to import 'enforcer.params_utils'
(E0401)
[convention] 12-12: third party import "supabase.create_client" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 13-13: third party import "supabase.lib.client_options.ClientOptions" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 14-14: third party import "cachetools.TTLCache" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 15-15: third party import "postgrest._sync.request_builder.SyncQueryRequestBuilder" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 16-16: third party import "postgrest.exceptions.APIError" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 18-26: Imports from package enforcer are not grouped
(C0412)
93-99: 🛠️ Refactor suggestionImprove exception handling with context.
Re-raise exceptions with proper context using
fromclause.except binascii.Error as e: raise Exception( "binascii.Error encountered. The Robusta UI token is not a valid base64." - ) + ) from e except json.JSONDecodeError as e: raise Exception( "json.JSONDecodeError encountered. The Robusta UI token could not be parsed as JSON after being base64 decoded." - ) + ) from e🧰 Tools
🪛 Ruff (0.11.9)
93-95: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
97-99: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
🪛 Pylint (3.3.7)
[convention] 98-98: Line too long (132/100)
(C0301)
[warning] 93-95: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('binascii.Error encountered. The Robusta UI token is not a valid base64.') from exc'
(W0707)
[warning] 93-95: Raising too general exception: Exception
(W0719)
[warning] 97-99: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('json.JSONDecodeError encountered. The Robusta UI token could not be parsed as JSON after being base64 decoded.') from exc'
(W0707)
[warning] 97-99: Raising too general exception: Exception
(W0719)
106-107: 🛠️ Refactor suggestionSpecify encoding when opening files.
Always specify encoding to avoid platform-dependent behavior.
logging.info(f"loading config {config_file_path}") - with open(config_file_path) as file: + with open(config_file_path, encoding='utf-8') as file: yaml_content = yaml.safe_load(file)🧰 Tools
🪛 Pylint (3.3.7)
[warning] 106-106: Using open without explicitly specifying an encoding
(W1514)
132-138: 🛠️ Refactor suggestionImprove exception handling in token decoding.
Use proper exception chaining for better debugging context.
except binascii.Error as e: raise Exception( "binascii.Error encountered. The robusta token provided is not a valid base64." - ) + ) from e except json.JSONDecodeError as e: raise Exception( "json.JSONDecodeError encountered. The Robusta token provided could not be parsed as JSON after being base64 decoded." - ) + ) from e🧰 Tools
🪛 Ruff (0.11.9)
132-134: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
136-138: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
🪛 Pylint (3.3.7)
[convention] 133-133: Line too long (107/100)
(C0301)
[convention] 137-137: Line too long (146/100)
(C0301)
[warning] 132-134: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('binascii.Error encountered. The robusta token provided is not a valid base64.') from exc'
(W0707)
[warning] 132-134: Raising too general exception: Exception
(W0719)
[warning] 136-138: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('json.JSONDecodeError encountered. The Robusta token provided could not be parsed as JSON after being base64 decoded.') from exc'
(W0707)
[warning] 136-138: Raising too general exception: Exception
(W0719)
227-229:⚠️ Potential issueFix inconsistent return types.
The method should consistently return a tuple in all cases.
except Exception: logging.exception("Supabase error while retrieving krr scan data") - return None + return None, None🧰 Tools
🪛 Pylint (3.3.7)
[warning] 227-227: Catching too general exception Exception
(W0718)
🧹 Nitpick comments (14)
helm/krr-enforcer/values.yaml (4)
1-2: Use a Kubernetes Secret for TLS certificate instead of embedding in values.yaml
Embedding a base64-encoded certificate directly invalues.yamlcan lead to large diffs, accidental leaks, and harder rotations. Consider:
- Moving the cert data into a Kubernetes Secret (e.g.
krr-enforcer-tls) and mounting it, then reference its name here (e.g.certificateSecretName).- Providing only the secret name in values and leaving the cert data out of the chart.
4-10: Support fullImage override and digest pinning for immutability
You’ve commented outfullImage, but it’s often useful to allow users to bypassrepository/name:tagcomposition entirely. Also consider adding an optionalimage.digestfield so teams can pin to an exact SHA. For example:+ fullImage: "" # override repository/name:tag or repository/name@sha256:digest + digest: "" # optional, e.g. sha256:abcd... image: repository: … name: … tag: …
31-34: Allow customizing ServiceAccount creation and name
Right now only annotations are exposed. It’s common to let users choose whether to create a new ServiceAccount or use an existing one:serviceAccount: create: true name: "" # if empty and create==true, chart will generate a name annotations: {}This gives more flexibility in RBAC scenarios.
35-40: Enhance ServiceMonitor with namespaceSelector and relabeling
Out-of-the-box settings work, but for multi-namespace Prometheus you may need:
namespaceSelector: to target the chart’s namespace or all namespacesrelabelings/metricRelabelings: to filter or rewrite metrics labels
Consider expanding this block:serviceMonitor: enabled: true namespaceSelector: matchNames: [ "{{ .Release.Namespace }}" ] labels: {} relabelings: [] metricRelabelings: [] interval: 30s scrapeTimeout: 10sThis makes the integration with Prometheus Operator more robust.
enforcer/params_utils.py (1)
1-7: Add module docstring.The module is missing documentation explaining its purpose.
Add a module docstring at the beginning of the file:
"""Utility functions for handling environment variable replacements in configuration parameters. This module provides functions to detect and replace environment variable placeholders in the format {{ env.VAR_NAME }} within configuration values. """🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[error] 6-6: Unable to import 'pydantic.types'
(E0401)
enforcer/README.md (3)
49-54: Fix minor grammar issues in certificate section.Apply these grammar fixes:
### Certificate -- Each helm install/upgrade, a new certificate is created and deployed for the admission webhook. +- On each helm install/upgrade, a new certificate is created and deployed for the admission webhook. - <B>The certificate is set to expire after 1 year.</b> -- In order to avoid certificate expiration, you must upgrade the enforcer helm release, <b>at least once a year</b>. +- To avoid certificate expiration, you must upgrade the enforcer helm release, <b>at least once a year</b>.🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: Possible missing preposition found.
Context: ...re KRR scan results ### Certificate - Each helm install/upgrade, a new certificate...(AI_HYDRA_LEO_MISSING_IN)
[style] ~53-~53: Consider a more concise word here.
Context: ...te is set to expire after 1 year. - In order to avoid certificate expiration, you must ...(IN_ORDER_TO_PREMIUM)
92-99: Fix hyphenation in certificate parameter description.Apply this fix:
| Parameter | Description | Default | |-----------|---------------------------------------------------------------------|---------| | `logLevel` | Log level (DEBUG, INFO, WARN, ERROR) | `INFO` | -| `certificate` | Base64-encoded custom CA certificate - for self signed certificates | `""` | +| `certificate` | Base64-encoded custom CA certificate - for self-signed certificates | `""` | | `serviceMonitor.enabled` | Enable Prometheus ServiceMonitor | `true` |🧰 Tools
🪛 LanguageTool
[misspelling] ~95-~95: This word is normally spelled with a hyphen.
Context: ...e64-encoded custom CA certificate - for self signed certificates |""| | `serviceMonitor...(EN_COMPOUNDS_SELF_SIGNED)
210-216: Improve punctuation in troubleshooting section.Add commas before "and" in compound sentences for better readability.
Apply these punctuation fixes:
### Common Issues -1. **Certificate Errors**: Ensure TLS certificates are properly configured and valid +1. **Certificate Errors**: Ensure TLS certificates are properly configured, and valid 2. **Permission Denied**: Verify the ServiceAccount has proper RBAC permissions -3. **No Recommendations**: Check that KRR has generated recommendations and they're accessible +3. **No Recommendations**: Check that KRR has generated recommendations, and they're accessible 4. **Webhook Timeout**: Increase `timeoutSeconds` in MutatingWebhookConfiguration🧰 Tools
🪛 LanguageTool
[uncategorized] ~212-~212: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...TLS certificates are properly configured and valid 2. Permission Denied: Verify ...(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~214-~214: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...k that KRR has generated recommendations and they're accessible 4. **Webhook Timeout...(COMMA_COMPOUND_SENTENCE)
enforcer/dal/supabase_dal.py (6)
87-87: Remove unnecessary f-string prefix.The f-string doesn't contain any placeholders.
- logging.info(f"Loading Robusta env configuration - ROBUSTA_UI_TOKEN_OVERRIDE") + logging.info("Loading Robusta env configuration - ROBUSTA_UI_TOKEN_OVERRIDE")🧰 Tools
🪛 Ruff (0.11.9)
87-87: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[warning] 87-87: Use lazy % formatting in logging functions
(W1203)
[warning] 87-87: Using an f-string that does not have any interpolated variables
(W1309)
166-175: Add docstring to sign_in method.Add documentation to explain the method's purpose:
def sign_in(self) -> str: + """Authenticate with Supabase and set up session tokens. + + Returns: + The authenticated user ID. + """ logging.info("Supabase DAL login")🧰 Tools
🪛 Pylint (3.3.7)
[convention] 166-166: Missing function or method docstring
(C0116)
190-192: Simplify empty list check.Use more Pythonic empty check.
- if not len(scans_meta_response.data): + if not scans_meta_response.data: logging.warning(f"No scans found for account {self.account_id} cluster {self.cluster}") return None, None🧰 Tools
🪛 Pylint (3.3.7)
[convention] 191-191: Line too long (103/100)
(C0301)
[convention] 190-190: Do not use
len(SEQUENCE)without comparison to determine if a sequence is empty(C1802)
[warning] 191-191: Use lazy % formatting in logging functions
(W1203)
195-195: Remove unnecessary f-string prefix.The f-string doesn't contain any placeholders.
- logging.warning(f"Multiple latest scans found. Using newest scan") + logging.warning("Multiple latest scans found. Using newest scan")🧰 Tools
🪛 Ruff (0.11.9)
195-195: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[warning] 195-195: Use lazy % formatting in logging functions
(W1203)
[warning] 195-195: Using an f-string that does not have any interpolated variables
(W1309)
223-224: Simplify empty list check.Use more Pythonic empty check.
- if not len(scans_results_response.data): + if not scans_results_response.data: return None, None🧰 Tools
🪛 Pylint (3.3.7)
[convention] 223-223: Do not use
len(SEQUENCE)without comparison to determine if a sequence is empty(C1802)
201-201: Clean up trailing whitespace and newlines.Remove trailing whitespace on line 201 and extra newlines at the end of the file.
else: latest_scan_data = scans_meta_response.data[0] - + latest_scan_id = latest_scan_data["scan_id"]And remove the extra blank lines at the end of the file (line 232).
Also applies to: 232-232
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 201-201: Trailing whitespace
(C0303)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
enforcer/README.md(1 hunks)enforcer/dal/supabase_dal.py(1 hunks)enforcer/params_utils.py(1 hunks)helm/krr-enforcer/Chart.yaml(1 hunks)helm/krr-enforcer/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- helm/krr-enforcer/Chart.yaml
🧰 Additional context used
🪛 Pylint (3.3.7)
enforcer/params_utils.py
[convention] 1-1: Missing module docstring
(C0114)
[error] 6-6: Unable to import 'pydantic.types'
(E0401)
[convention] 8-8: Missing function or method docstring
(C0116)
[warning] 15-15: Raising too general exception: Exception
(W0719)
[convention] 20-20: Missing function or method docstring
(C0116)
enforcer/dal/supabase_dal.py
[convention] 47-47: Line too long (108/100)
(C0301)
[convention] 98-98: Line too long (132/100)
(C0301)
[convention] 123-123: Line too long (149/100)
(C0301)
[convention] 126-126: Line too long (109/100)
(C0301)
[convention] 130-130: Line too long (108/100)
(C0301)
[convention] 133-133: Line too long (107/100)
(C0301)
[convention] 137-137: Line too long (146/100)
(C0301)
[convention] 177-177: Line too long (107/100)
(C0301)
[convention] 191-191: Line too long (103/100)
(C0301)
[convention] 197-197: Line too long (108/100)
(C0301)
[convention] 201-201: Trailing whitespace
(C0303)
[convention] 205-205: Line too long (111/100)
(C0301)
[convention] 212-212: Line too long (128/100)
(C0301)
[convention] 232-232: Trailing newlines
(C0305)
[convention] 1-1: Missing module docstring
(C0114)
[error] 11-11: Unable to import 'enforcer.dal.robusta_config'
(E0401)
[error] 12-12: Unable to import 'supabase'
(E0401)
[error] 13-13: Unable to import 'supabase.lib.client_options'
(E0401)
[error] 14-14: Unable to import 'cachetools'
(E0401)
[error] 15-15: Unable to import 'postgrest._sync.request_builder'
(E0401)
[error] 16-16: Unable to import 'postgrest.exceptions'
(E0401)
[error] 18-26: Unable to import 'enforcer.env_vars'
(E0401)
[error] 27-27: Unable to import 'enforcer.params_utils'
(E0401)
[warning] 29-29: os.getenv default type is builtins.int. Expected str or None.
(W1508)
[convention] 35-35: Missing class docstring
(C0115)
[refactor] 35-35: Too many instance attributes (12/7)
(R0902)
[warning] 46-48: Use lazy % formatting in logging functions
(W1203)
[convention] 57-57: Missing function or method docstring
(C0116)
[refactor] 66-76: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[warning] 87-87: Use lazy % formatting in logging functions
(W1203)
[warning] 87-87: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 93-95: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('binascii.Error encountered. The Robusta UI token is not a valid base64.') from exc'
(W0707)
[warning] 93-95: Raising too general exception: Exception
(W0719)
[warning] 97-99: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('json.JSONDecodeError encountered. The Robusta UI token could not be parsed as JSON after being base64 decoded.') from exc'
(W0707)
[warning] 97-99: Raising too general exception: Exception
(W0719)
[warning] 102-102: Use lazy % formatting in logging functions
(W1203)
[warning] 105-105: Use lazy % formatting in logging functions
(W1203)
[warning] 106-106: Using open without explicitly specifying an encoding
(W1514)
[warning] 113-116: Raising too general exception: Exception
(W0719)
[warning] 132-134: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('binascii.Error encountered. The robusta token provided is not a valid base64.') from exc'
(W0707)
[warning] 132-134: Raising too general exception: Exception
(W0719)
[warning] 136-138: Consider explicitly re-raising using 'except Exception as exc' and 'raise Exception('json.JSONDecodeError encountered. The Robusta token provided could not be parsed as JSON after being base64 decoded.') from exc'
(W0707)
[warning] 136-138: Raising too general exception: Exception
(W0719)
[warning] 161-161: Raising too general exception: Exception
(W0719)
[convention] 166-166: Missing function or method docstring
(C0116)
[convention] 177-177: Missing function or method docstring
(C0116)
[warning] 227-227: Catching too general exception Exception
(W0718)
[convention] 190-190: Do not use len(SEQUENCE) without comparison to determine if a sequence is empty
(C1802)
[warning] 191-191: Use lazy % formatting in logging functions
(W1203)
[warning] 195-195: Use lazy % formatting in logging functions
(W1203)
[warning] 195-195: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 205-205: Use lazy % formatting in logging functions
(W1203)
[warning] 212-212: Use lazy % formatting in logging functions
(W1203)
[convention] 223-223: Do not use len(SEQUENCE) without comparison to determine if a sequence is empty
(C1802)
[refactor] 177-177: Too many return statements (7/6)
(R0911)
[convention] 12-12: third party import "supabase.create_client" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 13-13: third party import "supabase.lib.client_options.ClientOptions" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 14-14: third party import "cachetools.TTLCache" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 15-15: third party import "postgrest._sync.request_builder.SyncQueryRequestBuilder" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 16-16: third party import "postgrest.exceptions.APIError" should be placed before first party import "enforcer.dal.robusta_config.RobustaConfig"
(C0411)
[convention] 18-26: Imports from package enforcer are not grouped
(C0412)
🪛 LanguageTool
enforcer/README.md
[uncategorized] ~51-~51: Possible missing preposition found.
Context: ...re KRR scan results ### Certificate - Each helm install/upgrade, a new certificate...
(AI_HYDRA_LEO_MISSING_IN)
[style] ~53-~53: Consider a more concise word here.
Context: ...te is set to expire after 1 year. - In order to avoid certificate expiration, you must ...
(IN_ORDER_TO_PREMIUM)
[style] ~67-~67: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ariable to the Enforcer pod as well. If the Enforcer is installed on a differ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[misspelling] ~95-~95: This word is normally spelled with a hyphen.
Context: ...e64-encoded custom CA certificate - for self signed certificates | "" | | `serviceMonitor...
(EN_COMPOUNDS_SELF_SIGNED)
[uncategorized] ~212-~212: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...TLS certificates are properly configured and valid 2. Permission Denied: Verify ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~214-~214: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...k that KRR has generated recommendations and they're accessible 4. **Webhook Timeout...
(COMMA_COMPOUND_SENTENCE)
🪛 Ruff (0.11.9)
enforcer/dal/supabase_dal.py
87-87: f-string without any placeholders
Remove extraneous f prefix
(F541)
93-95: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
97-99: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
110-110: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
132-134: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
136-138: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
195-195: f-string without any placeholders
Remove extraneous f prefix
(F541)
pavangudiwada
changed the title
2 participants
New Features
Documentation
Chores
.helmignorefor streamlined chart management.