fake_kubernetes_client/types.py (1)

1-2: Placeholder file adds no value yet—consider fleshing it out or omitting it until needed.

Right now the module contains only a docstring. If no shared type aliases or protocols are actually consumed by the new sub-modules, shipping an empty file clutters the package surface and invites dead-code drift. Either:

1. Populate it with the concrete TypedDict, Protocol, or Literal definitions the other modules will import, or
2. Delete it for now and re-add once you have real content.

Also, if you decide to keep it, end the docstring with a period to satisfy common docstring-lint rules.

class_generator/class_generator.py (1)

244-250: Consider logging a warning when copy fails.

While the silent failure approach prevents breaking the entire schema update process, it could lead to the fake client operating with stale resource mappings. Consider adding a warning log when the copy fails to help with debugging potential issues.

 # Copy the resources mapping file to fake_kubernetes_client for the fake client to use
 fake_client_mappings = Path("fake_kubernetes_client/__resources-mappings.json")
 try:
     shutil.copy2(RESOURCES_MAPPING_FILE, fake_client_mappings)
-except (OSError, IOError):
+except (OSError, IOError) as exp:
     # Don't fail the entire process if copy fails
-    pass
+    LOGGER.warning(f"Failed to copy resources mapping file to fake client: {exp}")

fake_kubernetes_client/resource_storage.py (1)

98-115: Consider documenting the limited label selector support.

The current implementation only supports equality (key=value), inequality (key!=value), and key presence checks. It doesn't support Kubernetes set-based selectors like in, notin, exists, or doesnotexist.

Consider adding a docstring note about these limitations or expanding support in the future.

Would you like me to generate an implementation that supports the full Kubernetes label selector syntax?

fake_kubernetes_client/README.md (2)

201-217: Use proper markdown heading instead of emphasis.

-**1. Testing CRDs (Custom Resource Definitions)**
+### 1. Testing CRDs (Custom Resource Definitions)

---

219-239: Use proper markdown heading instead of emphasis.

-**2. Multi-Version Resources**
+### 2. Multi-Version Resources

fake_kubernetes_client/kubernetes_client.py (1)

17-18: Use pipe syntax for union types.

Since the codebase uses modern Python typing, consider using the pipe syntax for better readability.

Apply this diff to use the newer pipe syntax:

-        configuration: Union[FakeConfiguration, None] = None,
-        dynamic_client: Union["FakeDynamicClient", None] = None,
+        configuration: FakeConfiguration | None = None,
+        dynamic_client: "FakeDynamicClient" | None = None,

fake_kubernetes_client/resource_field.py (1)

36-72: Reduce code duplication between __getattr__ and __getitem__.

The value wrapping logic is duplicated between __getattr__ (lines 52-60) and __getitem__ (lines 63-71). Consider extracting this into a helper method.

Apply this diff to reduce duplication:

+    def _wrap_value(self, value: Any) -> Any:
+        """Wrap value based on its type"""
+        if value is None:
+            return FakeResourceField(data={})
+        elif isinstance(value, dict):
+            return FakeResourceField(data=value)
+        elif isinstance(value, list):
+            return [FakeResourceField(data=item) if isinstance(item, dict) else item for item in value]
+        else:
+            return value
+
     def __getattr__(self, name: str) -> Any:
         # This is called ONLY when __getattribute__ raises AttributeError
         # Handle all dynamic attribute access here
 
         # Direct access to _data without using hasattr (avoids recursion)
         try:
             data = object.__getattribute__(self, "_data")
         except AttributeError:
             data = {}
 
         # For resource definition access, return simple values for common attributes
         # This ensures compatibility with ocp_resources code that expects strings
         if name in ["api_version", "group_version", "kind", "plural", "singular", "group", "version"]:
             return data.get(name, "")
 
         # Handle general data access
         value = data.get(name)
-        if value is None:
-            return FakeResourceField(data={})
-        elif isinstance(value, dict):
-            return FakeResourceField(data=value)
-        elif isinstance(value, list):
-            return [FakeResourceField(data=item) if isinstance(item, dict) else item for item in value]
-        else:
-            return value
+        return self._wrap_value(value)
 
     def __getitem__(self, key: str) -> Any:
         value = self._data.get(key)
-        if value is None:
-            return FakeResourceField(data={})
-        elif isinstance(value, dict):
-            return FakeResourceField(data=value)
-        elif isinstance(value, list):
-            return [FakeResourceField(data=item) if isinstance(item, dict) else item for item in value]
-        else:
-            return value
+        return self._wrap_value(value)

fake_kubernetes_client/status_schema_parser.py (1)

233-233: Consider making random IP generation deterministic for tests.

Random IP addresses could cause test flakiness if tests expect consistent values. Consider adding an option for deterministic generation.

You could add a seed parameter or use a deterministic approach:

         elif field_name.endswith("IP"):
-            return f"10.0.0.{random.randint(1, 254)}"
+            # Use a hash of the field name for deterministic IPs
+            import hashlib
+            hash_val = int(hashlib.md5(field_name.encode()).hexdigest()[:2], 16) % 254 + 1
+            return f"10.0.0.{hash_val}"

fake_kubernetes_client/resource_manager.py (1)

50-60: Enhance preferred version selection logic.

The current implementation only checks for "v1" specifically. Consider supporting other stable versions (v2, v3, etc.) and using a more robust version comparison.

-                if preferred:
-                    # Simple logic: prefer stable versions (v1 > v1beta1 > v1alpha1)
-                    for def_ in definitions:
-                        if def_["version"] == "v1":
-                            resource_def = def_
-                            break
-                    if not resource_def:
-                        resource_def = definitions[0]
+                if preferred:
+                    # Prefer stable versions: v1, v2, etc. > v1beta1 > v1alpha1
+                    stable_versions = []
+                    beta_versions = []
+                    alpha_versions = []
+                    
+                    for def_ in definitions:
+                        version = def_["version"]
+                        if version.startswith("v") and version[1:].isdigit():
+                            stable_versions.append((int(version[1:]), def_))
+                        elif "beta" in version:
+                            beta_versions.append(def_)
+                        elif "alpha" in version:
+                            alpha_versions.append(def_)
+                    
+                    if stable_versions:
+                        # Sort by version number and pick the highest
+                        resource_def = max(stable_versions, key=lambda x: x[0])[1]
+                    elif beta_versions:
+                        resource_def = beta_versions[0]
+                    elif alpha_versions:
+                        resource_def = alpha_versions[0]
+                    else:
+                        resource_def = definitions[0]

fake_kubernetes_client/dynamic_client.py (1)

130-142: Consider improving type annotations for better type safety.

The method uses Any types extensively, which reduces type safety. Consider using more specific types or Union types.

-    def get(self, resource: Any, *args: Any, **kwargs: Any) -> Any:
+    def get(
+        self, 
+        resource: Union[FakeResourceField, dict[str, Any]], 
+        *args: Any, 
+        **kwargs: Any
+    ) -> FakeResourceField:
         """Get resources based on resource definition"""
         # Extract resource definition from FakeResourceField if needed
         if hasattr(resource, "data"):
             resource_def = resource.data
         else:
             resource_def = resource

         # Create a resource instance for this resource type
         resource_instance = FakeResourceInstance(resource_def=resource_def, storage=self.storage, client=self)

         # Call get on the resource instance to list resources
         return resource_instance.get(*args, **kwargs)

fake_kubernetes_client/resource_instance.py (3)

93-95: Consider making default namespace configurable.

The default namespace is hardcoded to "default". Consider making this configurable or obtaining it from the client configuration.

         resource_namespace = (
-            body["metadata"].get("namespace", "default") if self.resource_def.get("namespaced") else None
+            body["metadata"].get("namespace") or self._get_default_namespace() if self.resource_def.get("namespaced") else None
         )

Add a helper method:

def _get_default_namespace(self) -> str:
    """Get default namespace from configuration or return 'default'"""
    if self.client and hasattr(self.client, 'configuration'):
        return getattr(self.client.configuration, 'default_namespace', 'default')
    return 'default'

---

70-150: Extract helper methods to improve readability.

The create method is quite long (80 lines). Consider extracting some logic into helper methods for better maintainability.

Extract metadata generation into a helper method:

def _generate_create_metadata(self, body: dict[str, Any]) -> None:
    """Generate metadata fields for resource creation"""
    body["metadata"].update({
        "uid": str(uuid.uuid4()),
        "resourceVersion": self._generate_resource_version(),
        "creationTimestamp": self._generate_timestamp(),
        "generation": 1,
        "labels": body["metadata"].get("labels", {}),
        "annotations": body["metadata"].get("annotations", {}),
    })

Then use it in the create method:

-        # Add generated metadata
-        body["metadata"].update({
-            "uid": str(uuid.uuid4()),
-            "resourceVersion": self._generate_resource_version(),
-            "creationTimestamp": self._generate_timestamp(),
-            "generation": 1,
-            "labels": body["metadata"].get("labels", {}),
-            "annotations": body["metadata"].get("annotations", {}),
-        })
+        # Add generated metadata
+        self._generate_create_metadata(body)

---

397-399: Preserve API version and kind from body if provided.

The method unconditionally sets apiVersion and kind, potentially overriding values provided in the body. Consider preserving user-provided values.

         # Set API version and kind
-        body["apiVersion"] = self.resource_def["api_version"]
-        body["kind"] = self.resource_def["kind"]
+        # Set API version and kind if not provided
+        body.setdefault("apiVersion", self.resource_def["api_version"])
+        body.setdefault("kind", self.resource_def["kind"])

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: EdDev
PR: RedHatQE/openshift-python-wrapper#2291
File: ocp_resources/resource.py:126-126
Timestamp: 2025-01-28T07:25:19.573Z
Learning: The environment variable `OPENSHIFT_PYTHON_WRAPPER_CLIENT_USE_PROXY` must be set and not empty to override the `use_proxy` parameter in the `get_client` function.

Learnt from: sbahar619
PR: RedHatQE/openshift-python-wrapper#2126
File: ocp_resources/user_defined_network.py:19-19
Timestamp: 2024-10-08T23:43:22.342Z
Learning: When reviewing code generated by the class generator, avoid suggesting changes that would alter its automatic version.

Learnt from: sbahar619
PR: RedHatQE/openshift-python-wrapper#2126
File: ocp_resources/user_defined_network.py:19-19
Timestamp: 2024-09-29T12:43:09.926Z
Learning: When reviewing code generated by the class generator, avoid suggesting changes that would alter its automatic version.

Learnt from: myakove
PR: RedHatQE/openshift-python-wrapper#2181
File: class_generator/class_generator.py:226-229
Timestamp: 2024-10-27T13:29:34.775Z
Learning: In the `update_kind_schema()` function in `class_generator.py`, the `tmp_schema_dir` directory contains only JSON files without subdirectories, so preserving the directory structure when copying files is unnecessary.

Learnt from: myakove
PR: RedHatQE/openshift-python-wrapper#2181
File: class_generator/schema/manualapprovalgatelist.json:12-18
Timestamp: 2024-10-27T14:07:35.575Z
Learning: All files under `class_generator/schema/` are auto-generated.

Learnt from: myakove
PR: RedHatQE/openshift-python-wrapper#2209
File: ocp_resources/volume_snapshot_class.py:0-0
Timestamp: 2024-12-03T08:02:11.880Z
Learning: In generated code files like `ocp_resources/volume_snapshot_class.py`, avoid suggesting adding validation checks or modifications, as generated code should not be manually altered.

Learnt from: myakove
PR: RedHatQE/openshift-python-wrapper#2152
File: class_generator/tests/manifests/ServingRuntime/serving_runtime.py:96-160
Timestamp: 2024-10-09T14:25:49.618Z
Learning: The code in the `class_generator/tests/manifests/ServingRuntime/serving_runtime.py` file is auto-generated. Avoid suggesting refactoring changes to auto-generated code in this path.

Learnt from: myakove
PR: RedHatQE/openshift-python-wrapper#2154
File: ocp_resources/serving_runtime.py:110-160
Timestamp: 2024-10-09T18:44:29.284Z
Learning: In `ocp_resources/serving_runtime.py`, the code is auto-generated and should not be manually modified.

Learnt from: myakove
PR: RedHatQE/openshift-python-wrapper#2232
File: tests/test_resources.py:21-23
Timestamp: 2024-12-15T08:13:40.715Z
Learning: In the `pod` fixture in `tests/test_resources.py`, it is expected that there will always be at least one pod available, and the test should fail if there are none.

fake_kubernetes_client/resource_storage.py (1)

129-146: String conversion in field selector matching may cause issues.

Converting field values to strings (lines 138, 144) might not work correctly for all data types:

• Booleans: Python's True becomes "True" instead of "true"
• None/null values: None becomes "None" instead of being handled as missing
• Numbers: Potential precision issues with floats

Consider implementing type-aware comparison:

 def _matches_field_selector(self, resource: dict[str, Any], selector: str) -> bool:
     """Check if resource matches field selector"""
     # Handle simple field selectors (field.path=value or field.path==value)
     parts = selector.split(",")
     for part in parts:
         if "==" in part:
             # Handle double equals
             field_path, value = part.split("==", 1)
             field_value = self._get_field_value(resource, field_path.strip())
-            if str(field_value) != value.strip():
+            if not self._compare_field_value(field_value, value.strip()):
                 return False
         elif "=" in part:
             # Handle single equals
             field_path, value = part.split("=", 1)
             field_value = self._get_field_value(resource, field_path.strip())
-            if str(field_value) != value.strip():
+            if not self._compare_field_value(field_value, value.strip()):
                 return False
     return True
+
+def _compare_field_value(self, field_value: Any, expected: str) -> bool:
+    """Compare field value with expected string value"""
+    if field_value is None:
+        return expected.lower() in ("", "null", "none")
+    elif isinstance(field_value, bool):
+        return str(field_value).lower() == expected.lower()
+    else:
+        return str(field_value) == expected

fake_kubernetes_client/status_schema_parser.py (2)

41-52: Complete the reference resolution implementation.

The comment indicates this is a placeholder implementation. Without proper reference resolution, the schema parser may not work correctly for resources with $ref in their status definitions.

Would you like me to help implement proper $ref resolution that follows JSON Schema references?

---

256-259: Remove unreachable return statement.

Line 258 contains an unreachable return statement after the else block already returns.

Apply this diff to remove the dead code:

         else:
             return 1
-        return 1

fake_kubernetes_client/resource_instance.py (2)

333-334: Replace assert with proper error handling.

Using assert for type narrowing is not ideal as assertions can be disabled with Python's -O flag. Consider explicit checking.

-        # Type assertion - at this point existing cannot be None
-        assert existing is not None
+        # Existing cannot be None at this point due to the check above
+        if existing is None:
+            # This should never happen, but handle it gracefully
+            self._create_not_found_error(name)
+            return FakeResourceField(data={})  # unreachable, but satisfies type checker

---

381-382: Replace assert with proper error handling in replace method.

Same issue as in the patch method - assertions can be disabled.

-        # Type assertion - at this point existing cannot be None
-        assert existing is not None
+        # Existing cannot be None at this point due to the check above
+        if existing is None:
+            # This should never happen, but handle it gracefully
+            self._create_not_found_error(name)
+            return FakeResourceField(data={})  # unreachable, but satisfies type checker

fake_kubernetes_client/resource_storage.py (1)

102-119: Consider documenting label selector limitations.

The current implementation only supports simple equality (key=value), inequality (key!=value), and key existence checks. It doesn't support Kubernetes set-based selectors like key in (value1,value2), key notin (value1,value2), or the !key syntax for non-existence.

Consider adding a docstring note about supported selector syntax:

 def _matches_label_selector(self, labels: dict[str, str], selector: str) -> bool:
-    """Check if labels match selector (simplified implementation)"""
+    """Check if labels match selector (simplified implementation).
+    
+    Supports:
+    - Equality: key=value
+    - Inequality: key!=value  
+    - Existence: key
+    
+    Does not support:
+    - Set-based: key in (value1,value2)
+    - Non-existence: !key
+    """

fake_kubernetes_client/resource_manager.py (1)

51-58: Enhance version preference logic for better stability ordering.

The current logic only checks for "v1" and doesn't handle other stable versions like "v2" or properly order beta/alpha versions.

Consider a more comprehensive version ordering:

                 if preferred:
                     # Simple logic: prefer stable versions (v1 > v1beta1 > v1alpha1)
-                    for def_ in definitions:
-                        if def_["version"] == "v1":
-                            resource_def = def_
-                            break
-                    if not resource_def:
-                        resource_def = definitions[0]
+                    # Sort by version stability
+                    def version_priority(def_: dict[str, Any]) -> tuple[int, str]:
+                        version = def_["version"]
+                        if version.startswith("v") and version[1:].isdigit():
+                            return (0, version)  # Stable versions first
+                        elif "beta" in version:
+                            return (1, version)
+                        elif "alpha" in version:
+                            return (2, version)
+                        else:
+                            return (3, version)
+                    
+                    resource_def = min(definitions, key=version_priority)

fake_kubernetes_client/resource_instance.py (1)

62-64: Consider adding uniqueness to resource version generation.

The current timestamp-based approach could theoretically produce collisions with rapid operations.

Consider adding a counter or random component:

+    def __init__(self, ...):
+        # ... existing init code ...
+        self._version_counter = 0
+
     def _generate_resource_version(self) -> str:
         """Generate unique resource version"""
-        return str(int(time.time() * 1000))
+        self._version_counter += 1
+        return f"{int(time.time() * 1000)}-{self._version_counter}"

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: tarukumar
PR: RedHatQE/openshift-python-wrapper#2151
File: ocp_resources/serving_runtime.py:130-134
Timestamp: 2024-10-09T13:40:48.030Z
Learning: In the `ServingRuntime` class in `ocp_resources/serving_runtime.py`, the `nodeSelector` key is correctly used in camelCase within the `_spec` dictionary. Avoid flagging this as an issue in future reviews.

Learnt from: myakove
PR: RedHatQE/openshift-python-wrapper#2209
File: ocp_resources/volume_snapshot_class.py:0-0
Timestamp: 2024-12-03T08:02:11.880Z
Learning: In generated code files like `ocp_resources/volume_snapshot_class.py`, avoid suggesting adding validation checks or modifications, as generated code should not be manually altered.