kesem0811 commented on code in PR #51080:
URL: https://github.com/apache/airflow/pull/51080#discussion_r2135403077
##########
helm-tests/tests/helm_tests/security/test_scc_rolebinding.py:
##########
@@ -61,6 +61,29 @@ def test_create_scc(self, rbac_enabled, scc_enabled,
created):
assert jmespath.search("subjects[7].name", docs[0]) ==
"release-name-airflow-create-user-job"
assert jmespath.search("subjects[8].name", docs[0]) ==
"release-name-airflow-cleanup"
+ @pytest.mark.parametrize(
+ "dag_processor_enabled",
+ [
+ (True),
+ (False),
+ ],
+ )
+ def test_scc_subjects_include_dag_processor(self, dag_processor_enabled):
Review Comment:
You're right that technically this could be merged into the existing test,
but I separated it for two main reasons:
Different focus of validation:
The first test checks whether the SCC RoleBinding object is created at all,
based on the rbac.create and rbac.createSCCRoleBinding values.
The second test focuses specifically on how the dagProcessor.enabled value
affects the list of subjects in the generated RoleBinding. It's verifying
whether the dag processor component is included or excluded depending on
whether it's enabled.
Special handling of the default value:
The dagProcessor.enabled field has a default value of ~ (i.e., null), unlike
the other components. That’s why I simulated both True, False, and unset (null)
scenarios in a controlled way.
Keeping this separate helped ensure the logic around the enabled value is
fully tested without overloading the first test. But I’m open to combining them
if you still think it better...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
