feat: Add validatingAdmissionPolicy and validatingAdmissionPolicyBinding #2635
Conversation
WalkthroughAdds two new Kubernetes resource wrapper classes—ValidatingAdmissionPolicy and ValidatingAdmissionPolicyBinding—with parameterized initializers and to_dict implementations, plus tests for both; also includes multiple docstring/content refinements in generated manifest test files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@tests/test_resources/test_validating_admission_policy_binding.py`:
- Around line 16-21: The test is asserting the bound method object instead of
calling it; change the assertion to invoke the exists method (use
validatingadmissionpolicybinding.exists()) so the test checks the actual
resource state; locate the assertion in
test_01_create_validatingadmissionpolicybinding and replace the property-style
reference validatingadmissionpolicybinding.exists with a call to the exists()
method.
- Around line 35-39: In test_04_delete_validatingadmissionpolicybinding, the
test negates the method object instead of calling it—change the assertion to
call the exists() method on validatingadmissionpolicybinding (i.e., use
validatingadmissionpolicybinding.exists()) so the runtime boolean result is
checked; update the assertion to assert not
validatingadmissionpolicybinding.exists() and ensure any other references to
validatingadmissionpolicybinding.exists are similarly invoked as a method.
In `@tests/test_resources/test_validating_admission_policy.py`:
- Around line 50-54: The test_04_delete_validatingadmissionpolicy uses the
property-like reference validatingadmissionpolicy.exists instead of calling the
method, so the assertion always evaluates incorrectly; update the assertion to
call the method as validatingadmissionpolicy.exists() after
validatingadmissionpolicy.clean_up(wait=False) to properly verify the resource
no longer exists (ensure you reference the exists() method on the
validatingadmissionpolicy object).
- Around line 31-36: The test test_01_create_validatingadmissionpolicy uses
validatingadmissionpolicy.exists as a property but exists is a method; update
the assertion to call the method (validatingadmissionpolicy.exists()) so it
evaluates to a boolean, e.g., replace "assert validatingadmissionpolicy.exists"
with "assert validatingadmissionpolicy.exists()".
RoniKishner
force-pushed
the
validating-admission-policy
branch
from
24ddf00 to
a840ac4
Compare
RoniKishner
changed the title
RoniKishner
force-pushed
the
validating-admission-policy
branch
from
a840ac4 to
0f1674d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@class_generator/tests/test_class_generator.py`:
- Line 157: Replace the brittle "assert False, failure_summary" with a pytest
API call: call pytest.fail(...) using the existing failure_summary so the test
reliably fails even under Python -O; locate the assertion in
test_class_generator (the line with "assert False" referencing failure_summary)
and replace it with pytest.fail(failure_summary).
🧹 Nitpick comments (3)
class_generator/tests/test_class_generator.py (3)
63-71: Explicitstrict=Falsewould clarify intent.The removal of
strict=Trueis intentional since different-length files are handled separately (lines 70-71). Consider adding explicitstrict=Falseto document this intent and silence the linter.💡 Suggested clarification
- for i, (gen_line, exp_line) in enumerate(zip(generated_lines, expected_lines)): + for i, (gen_line, exp_line) in enumerate(zip(generated_lines, expected_lines, strict=False)):
116-118: Unusedkindparameter is required by interface.The
kindparameter is unused but required by theexecute_parallel_tasksresult_processor signature (Callable[[T, R], Any]). Consider adding_prefix to indicate intentionally unused:_kind.💡 Suggested clarification
- def process_test_result(kind: str, result: Optional[Tuple[str, str]]) -> None: + def process_test_result(_kind: str, result: Optional[Tuple[str, str]]) -> None:
120-122: Consider f-string conversion flag (nitpick).Static analysis suggests using
{exc!s}instead of{str(exc)}for f-string idiomatic style. Both work identically.💡 Suggested change
def handle_test_error(kind: str, exc: Exception) -> None: - failures.append((kind, f"Task execution failed: {str(exc)}")) + failures.append((kind, f"Task execution failed: {exc!s}"))
RoniKishner
force-pushed
the
validating-admission-policy
branch
from
0f1674d to
c0f0458
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
class_generator/tests/test_class_generator.py (1)
94-111: AddValidatingAdmissionPolicyandValidatingAdmissionPolicyBindingto the test resource list.These auto-generated resources are missing from the
resource_kindslist for thetest_parse_explaintest. Test manifest files should also be created inclass_generator/tests/manifests/ValidatingAdmissionPolicy/andclass_generator/tests/manifests/ValidatingAdmissionPolicyBinding/to enable test coverage for these resources.
♻️ Duplicate comments (1)
class_generator/tests/test_class_generator.py (1)
152-157: Good improvement;pytest.fail()remains an option.The change from
assert Falsetoraise AssertionErrorcorrectly addresses the-Ooptimization concern (B011). The previous review suggestedpytest.fail()which is already imported and is the idiomatic pytest approach for explicit test failures. Either approach works;pytest.fail()provides slightly better integration with pytest's failure reporting.
🧹 Nitpick comments (2)
class_generator/tests/test_class_generator.py (2)
63-68: Consider addingstrict=Falseto clarify intent.Static analysis flags
zip()withoutstrict=(B905). The current behavior is correct—you want to compare as many lines as possible and handle length differences separately at lines 70-71. Adding explicitstrict=Falsewould silence the linter and document the intent.♻️ Proposed fix
- for i, (gen_line, exp_line) in enumerate(zip(generated_lines, expected_lines)): + for i, (gen_line, exp_line) in enumerate(zip(generated_lines, expected_lines, strict=False)):
116-118: Consider using_kindto indicate intentionally unused parameter.The
kindparameter is required by theexecute_parallel_tasksAPI but unused here. Using an underscore prefix silences the linter and documents the intent.♻️ Proposed fix
- def process_test_result(kind: str, result: Optional[Tuple[str, str]]) -> None: + def process_test_result(_kind: str, result: Optional[Tuple[str, str]]) -> None:
RoniKishner
force-pushed
the
validating-admission-policy
branch
from
c0f0458 to
43f3df5
Compare
|
/verified |
|
/hold |
|
/hold cancel |
6 participants
Short description:
Add validatingAdmissionPolicy and validatingAdmissionPolicyBinding
More details:
validatingAdmissionPolicyclassvalidatingAdmissionPolicyBindingclassWhat this PR does / why we need it:
Adding the resources for future use of the class
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.