OCPBUGS-33041: Add RoleBinding for BuildConfig Webhooks #28750
Conversation
openshift-ci-robot
added
jira/severity-important
|
@adambkaplan: This pull request references Jira Issue OCPBUGS-33041, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-bug
|
Job Failure Risk Analysis for sha: 4c03c56
|
|
Job Failure Risk Analysis for sha: cc972cf
|
adambkaplan
force-pushed
the
unauthenticated-webhook-bc
branch
from
cc972cf
to
8507f77
Compare
|
/assign @sayan-biswas /cc @apoorvajagtap |
|
/retest-required |
|
/test e2e-gcp-ovn-builds Builds had a bad PAT token for private git repo testing. 🤞 it's fixed now. |
|
/retest |
|
/test e2e-gcp-ovn-builds |
1 similar comment
|
/test e2e-gcp-ovn-builds |
|
Job Failure Risk Analysis for sha: 8507f77
|
adambkaplan
force-pushed
the
unauthenticated-webhook-bc
branch
from
8507f77
to
0ae4678
Compare
|
Job Failure Risk Analysis for sha: 0ae4678
|
| kind: RoleBinding | ||
| metadata: | ||
| annotations: | ||
| rbac.authorization.kubernetes.io/autoupdate: "true" |
There was a problem hiding this comment.
consider removing, this is the default
|
/approve |
openshift-ci
bot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/retest-required |
|
/retest-required |
|
Job Failure Risk Analysis for sha: ef7e238
|
test/extended/builds/webhook.go
Outdated
| client: &http.Client{ | ||
| Transport: &http.Transport{ | ||
| TLSClientConfig: &tls.Config{ | ||
| InsecureSkipVerify: true, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The transport needs Proxy: http.ProxyFromEnvironment. Some of our CI jobs (like metal) access the cluster through a proxy. If no proxy environment variables are set, this is a no-op.
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. See also https://issues.redhat.com/browse/AUTH-509 Signed-off-by: Adam Kaplan <[email protected]>
adambkaplan
force-pushed
the
unauthenticated-webhook-bc
branch
from
ef7e238
to
43b42d4
Compare
|
@stbenjam updated the transport to include the proxy configuration - PTAL. |
|
/retest-required |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan, sanchezl, sayan-biswas, sjenning, stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Job Failure Risk Analysis for sha: 43b42d4
|
|
/retest-required |
|
@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. |
openshift-merge-bot
bot
merged commit 75f7e06
into
openshift:master
|
@adambkaplan: Jira Issue OCPBUGS-33041: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-33041 has been moved to the MODIFIED state. 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 openshift-eng/jira-lifecycle-plugin repository. |
…ticated-webhook-bc" This reverts commit e3c844d, restoring the test changes in origin#28750.
TRT-1656: Revert #28750 "OCPBUGS-33041: Add RoleBinding for BuildConfig Webhooks"
…ticated-webhook-bc" This reverts commit e3c844d, restoring the test changes in origin#28750.
|
Fix included in accepted release 4.16.0-0.nightly-2024-05-08-222442 |
Starting in OCP 4.16, the
system:webhookClusterRole 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:unauthenticatedgroup. RoleBindings are preferable to ClusterRoleBindings as they limit unauthenticated API calls to specific namespaces, reducing the potential attack surface.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.