WIP: Webhook test refactor #28753
WIP: Webhook test refactor #28753
Conversation
Starting in OCP 4.16, the `system:webhook` ClusterRole will not be granted to anonymous users by default. This will break most systems that use BuildConfig webhooks to trigger builds, since many can't be add an OpenShift auth token to their HTTP headers (ex: GitHub). Only new installations will be impacted; upgrades to 4.16 will continue to support unauthenticated BuildConfig webhooks. This test update verifies that BuildConfig webhooks can be triggered using a namespace-scoped RoleBinding for the `system:unauthenticated` group. RoleBindings are preferable to ClusterRoleBindings as they limit unauthenticated API calls to specific namespaces, reducing the potential attack surface. The core webhook tests were also updated to verify that unauthenticated webhooks fail if this rolebinding is missing. Use of BuildConfig webhooks should be discouraged in favor of Pipelines as Code, which has more robust mechanisms for securing webhook calls from external systems. It also does not rely on an aggregated apiserver and associated RBAC. Signed-off-by: Adam Kaplan <[email protected]>
Use Ginkgo v2's table-driven test functions to make the BuildConfig webhook tests more readable and produce more context when errors occur. Signed-off-by: Adam Kaplan <[email protected]>
|
An alternative/follow up to #28750 - I want to see if the table driven test breaks anything. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: adambkaplan The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
So good news? When the build test fails, it prints the name of the entry: |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@adambkaplan: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/close A worthwhile experiment, but I won't have time to chase down this NPE. |
|
@adambkaplan: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Starting in OCP 4.16, the
system:webhookClusterRole will not begranted to anonymous users by default. This will break most systems
that use BuildConfig webhooks to trigger builds, since many can't be
add an OpenShift auth token to their HTTP headers (ex: GitHub). Only
new installations will be impacted; upgrades to 4.16 will continue to
support unauthenticated BuildConfig webhooks.
This test update verifies that BuildConfig webhooks can be triggered
using a namespace-scoped RoleBinding for the
system:unauthenticatedgroup. RoleBindings are preferable to ClusterRoleBindings as they limit
unauthenticated API calls to specific namespaces, reducing the
potential attack surface. The core webhook tests were also updated to
verify that unauthenticated webhooks fail if this rolebinding is
missing.
Use of BuildConfig webhooks should be discouraged in favor of Pipelines
as Code, which has more robust mechanisms for securing webhook calls
from external systems. It also does not rely on an aggregated apiserver
and associated RBAC.
This PR also includes a refactor of the core BuildConfig webhook test suite, to take advantage of Ginkgo v2's table driven test functions. This makes the test more readable and provides missing context on error.