Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OCPBUGS-33041: Add RoleBinding for BuildConfig Webhooks #28750

Merged
merged 1 commit into from
May 7, 2024
Merged

OCPBUGS-33041: Add RoleBinding for BuildConfig Webhooks #28750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 39 additions & 0 deletions test/extended/builds/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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