Skip to content

Commit

Permalink
Merge pull request #28750 from adambkaplan/unauthenticated-webhook-bc
Browse files Browse the repository at this point in the history 
OCPBUGS-33041: Add RoleBinding for BuildConfig Webhooks
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] committed May 7, 2024
2 parents 15c3521 + 43b42d4 commit 75f7e06
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 15 deletions.
39 changes: 39 additions & 0 deletions test/extended/builds/start.go
View file
Expand Up @@ -14,6 +14,8 @@ import (
o "github.com/onsi/gomega"

corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
e2e "k8s.io/kubernetes/test/e2e/framework"
Expand Down Expand Up @@ -409,6 +411,43 @@ var _ = g.Describe("[sig-builds][Feature:Builds][Slow] starting a build using CL
})

g.Describe("start a build via a webhook", func() {

// AUTH-509: Webhooks do not allow unauthenticated requests by default.
// Create a role binding which allows unauthenticated webhooks.
g.BeforeEach(func() {
ctx := context.Background()
adminRoleBindingsClient := oc.AdminKubeClient().RbacV1().RoleBindings(oc.Namespace())
_, err := adminRoleBindingsClient.Get(ctx, "webooks-unauth", metav1.GetOptions{})
if apierrors.IsNotFound(err) {
unauthWebhooksRB := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "webooks-unauth",
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "ClusterRole",
Name: "system:webhook",
},
Subjects: []rbacv1.Subject{
{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Group",
Name: "system:authenticated",
},
{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Group",
Name: "system:unauthenticated",
},
},
}
_, err = adminRoleBindingsClient.Create(ctx, unauthWebhooksRB, metav1.CreateOptions{})
o.Expect(err).NotTo(o.HaveOccurred(), "creating webhook role binding")
return
}
o.Expect(err).NotTo(o.HaveOccurred(), "checking if webhook role binding exists")
})

g.It("should be able to start builds via the webhook with valid secrets and fail with invalid secrets [apigroup:build.openshift.io]", func() {
g.By("clearing existing builds")
_, err := oc.Run("delete").Args("builds", "--all").Output()
Expand Down
73 changes: 58 additions & 15 deletions test/extended/builds/webhook.go
View file
Expand Up @@ -3,10 +3,12 @@ package builds
import (
"bytes"
"context"
"crypto/tls"
"encoding/json"
"fmt"
"io/ioutil"
"io"
"net/http"
"os"
"time"

g "github.com/onsi/ginkgo/v2"
Expand All @@ -25,7 +27,10 @@ import (

var _ = g.Describe("[sig-builds][Feature:Builds][webhook]", func() {
defer g.GinkgoRecover()
oc := exutil.NewCLIWithPodSecurityLevel("build-webhooks", admissionapi.LevelBaseline)

var (
oc = exutil.NewCLIWithPodSecurityLevel("build-webhooks", admissionapi.LevelBaseline)
)

g.It("TestWebhook [apigroup:build.openshift.io][apigroup:image.openshift.io]", func() {
TestWebhook(g.GinkgoT(), oc)
Expand All @@ -43,6 +48,7 @@ var _ = g.Describe("[sig-builds][Feature:Builds][webhook]", func() {

func TestWebhook(t g.GinkgoTInterface, oc *exutil.CLI) {
clusterAdminBuildClient := oc.AdminBuildClient().BuildV1()
adminHTTPClient := clusterAdminBuildClient.RESTClient().(*rest.RESTClient).Client

// create buildconfig
buildConfig := mockBuildConfigImageParms("originalimage", "imagestream", "validtag")
Expand All @@ -53,10 +59,12 @@ func TestWebhook(t g.GinkgoTInterface, oc *exutil.CLI) {
// Bug #1752581: reduce number of URLs per test case
// OCP 4.2 tests on GCP had high flake levels because namespaces took too long to tear down
tests := []struct {
Name string
Payload string
HeaderFunc func(*http.Header)
URLs []string
Name string
Payload string
HeaderFunc func(*http.Header)
URLs []string
client *http.Client
expectedStatus int
}{
{
Name: "generic",
Expand All @@ -65,6 +73,8 @@ func TestWebhook(t g.GinkgoTInterface, oc *exutil.CLI) {
URLs: []string{
"/apis/build.openshift.io/v1/namespaces/" + oc.Namespace() + "/buildconfigs/pushbuild/webhooks/secret200/generic",
},
client: adminHTTPClient,
expectedStatus: http.StatusOK,
},
{
Name: "github",
Expand All @@ -73,6 +83,8 @@ func TestWebhook(t g.GinkgoTInterface, oc *exutil.CLI) {
URLs: []string{
"/apis/build.openshift.io/v1/namespaces/" + oc.Namespace() + "/buildconfigs/pushbuild/webhooks/secret100/github",
},
client: adminHTTPClient,
expectedStatus: http.StatusOK,
},
{
Name: "gitlab",
Expand All @@ -81,6 +93,8 @@ func TestWebhook(t g.GinkgoTInterface, oc *exutil.CLI) {
URLs: []string{
"/apis/build.openshift.io/v1/namespaces/" + oc.Namespace() + "/buildconfigs/pushbuild/webhooks/secret300/gitlab",
},
client: adminHTTPClient,
expectedStatus: http.StatusOK,
},
{
Name: "bitbucket",
Expand All @@ -89,6 +103,29 @@ func TestWebhook(t g.GinkgoTInterface, oc *exutil.CLI) {
URLs: []string{
"/apis/build.openshift.io/v1/namespaces/" + oc.Namespace() + "/buildconfigs/pushbuild/webhooks/secret400/bitbucket",
},
client: adminHTTPClient,
expectedStatus: http.StatusOK,
},
{
// AUTH-509: Webhooks do not allow unauthenticated requests by default.
// Test will verify that an unauthenticated request fails with 403 Forbidden.
Name: "unauthenticated forbidden",
Payload: "generic/testdata/push-generic.json",
HeaderFunc: genericHeaderFunc,
URLs: []string{
"/apis/build.openshift.io/v1/namespaces/" + oc.Namespace() + "/buildconfigs/pushbuild/webhooks/secret200/generic",
},
// Need client to skip TLS verification - CI clusters have self-signed certificates.
// Transport also needs to accept proxy information from *_PROXY environment variables.
client: &http.Client{
Transport: &http.Transport{
Proxy: http.ProxyFromEnvironment,
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
},
},
expectedStatus: http.StatusForbidden,
},
}

Expand All @@ -99,8 +136,12 @@ func TestWebhook(t g.GinkgoTInterface, oc *exutil.CLI) {
clusterAdminClientConfig := oc.AdminConfig()

g.By("executing the webhook to get the build object")
body := postFile(clusterAdminBuildClient.RESTClient(), test.HeaderFunc, test.Payload, clusterAdminClientConfig.Host+s, http.StatusOK, t, oc)
body := postFile(test.client, test.HeaderFunc, test.Payload, clusterAdminClientConfig.Host+s, test.expectedStatus, t, oc)
o.Expect(body).NotTo(o.BeEmpty())
// If expected HTTP status is not 200 OK, continue as we will not receive a Build object in the response body.
if test.expectedStatus != http.StatusOK {
continue
}

g.By("Unmarshalling the build object")
returnedBuild := &buildv1.Build{}
Expand All @@ -124,6 +165,7 @@ func TestWebhookGitHubPushWithImage(t g.GinkgoTInterface, oc *exutil.CLI) {
clusterAdminClientConfig := oc.AdminConfig()
clusterAdminImageClient := oc.AdminImageClient().ImageV1()
clusterAdminBuildClient := oc.AdminBuildClient().BuildV1()
adminHTTPClient := clusterAdminBuildClient.RESTClient().(*rest.RESTClient).Client

// create imagerepo
imageStream := &imagev1.ImageStream{
Expand Down Expand Up @@ -173,7 +215,7 @@ func TestWebhookGitHubPushWithImage(t g.GinkgoTInterface, oc *exutil.CLI) {
} {

// trigger build event sending push notification
body := postFile(clusterAdminBuildClient.RESTClient(), githubHeaderFunc, "github/testdata/pushevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc)
body := postFile(adminHTTPClient, githubHeaderFunc, "github/testdata/pushevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc)
if len(body) == 0 {
t.Errorf("Webhook did not return Build in body")
}
Expand Down Expand Up @@ -201,7 +243,6 @@ func TestWebhookGitHubPushWithImage(t g.GinkgoTInterface, oc *exutil.CLI) {
if actual.Spec.Strategy.DockerStrategy.From.Name != "originalimage" {
t.Errorf("Expected %s, got %s", "originalimage", actual.Spec.Strategy.DockerStrategy.From.Name)
}

if actual.Name != returnedBuild.Name {
t.Errorf("Build returned in response body does not match created Build. Expected %s, got %s", actual.Name, returnedBuild.Name)
}
Expand All @@ -214,6 +255,7 @@ func TestWebhookGitHubPushWithImageStream(t g.GinkgoTInterface, oc *exutil.CLI)
clusterAdminClientConfig := oc.AdminConfig()
clusterAdminImageClient := oc.AdminImageClient().ImageV1()
clusterAdminBuildClient := oc.AdminBuildClient().BuildV1()
adminHTTPClient := clusterAdminBuildClient.RESTClient().(*rest.RESTClient).Client

// create imagerepo
imageStream := &imagev1.ImageStream{
Expand Down Expand Up @@ -265,7 +307,7 @@ func TestWebhookGitHubPushWithImageStream(t g.GinkgoTInterface, oc *exutil.CLI)
s := "/apis/build.openshift.io/v1/namespaces/" + oc.Namespace() + "/buildconfigs/pushbuild/webhooks/secret101/github"

// trigger build event sending push notification
postFile(clusterAdminBuildClient.RESTClient(), githubHeaderFunc, "github/testdata/pushevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc)
postFile(adminHTTPClient, githubHeaderFunc, "github/testdata/pushevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc)

var build *buildv1.Build

Expand All @@ -291,6 +333,7 @@ Loop:

func TestWebhookGitHubPing(t g.GinkgoTInterface, oc *exutil.CLI) {
clusterAdminBuildClient := oc.AdminBuildClient().BuildV1()
adminHTTPClient := clusterAdminBuildClient.RESTClient().(*rest.RESTClient).Client

// create buildconfig
buildConfig := mockBuildConfigImageParms("originalimage", "imagestream", "validtag")
Expand All @@ -312,7 +355,7 @@ func TestWebhookGitHubPing(t g.GinkgoTInterface, oc *exutil.CLI) {
// trigger build event sending push notification
clusterAdminClientConfig := oc.AdminConfig()

postFile(clusterAdminBuildClient.RESTClient(), githubHeaderFuncPing, "github/testdata/pingevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc)
postFile(adminHTTPClient, githubHeaderFuncPing, "github/testdata/pingevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc)

// TODO: improve negative testing
timer := time.NewTimer(time.Second * 5)
Expand All @@ -326,9 +369,9 @@ func TestWebhookGitHubPing(t g.GinkgoTInterface, oc *exutil.CLI) {
}
}

func postFile(client rest.Interface, headerFunc func(*http.Header), filename, url string, expStatusCode int, t g.GinkgoTInterface, oc *exutil.CLI) []byte {
func postFile(client *http.Client, headerFunc func(*http.Header), filename, url string, expStatusCode int, t g.GinkgoTInterface, oc *exutil.CLI) []byte {
path := exutil.FixturePath("testdata", "builds", "webhook", filename)
data, err := ioutil.ReadFile(path)
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("Failed to open %s: %v", filename, err)
}
Expand All @@ -337,11 +380,11 @@ func postFile(client rest.Interface, headerFunc func(*http.Header), filename, ur
t.Fatalf("Error creating POST request: %v", err)
}
headerFunc(&req.Header)
resp, err := client.(*rest.RESTClient).Client.Do(req)
resp, err := client.Do(req)
if err != nil {
t.Fatalf("Failed posting webhook: %v", err)
}
body, _ := ioutil.ReadAll(resp.Body)
body, _ := io.ReadAll(resp.Body)
if resp.StatusCode != expStatusCode {
t.Errorf("Wrong response code, expecting %d, got %d: %s!", expStatusCode, resp.StatusCode, string(body))
}
Expand Down

0 comments on commit 75f7e06

Please sign in to comment.