diff --git a/test/extended/builds/start.go b/test/extended/builds/start.go index 4c436107b4f8..786fc5493cba 100644 --- a/test/extended/builds/start.go +++ b/test/extended/builds/start.go @@ -14,8 +14,6 @@ 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" @@ -411,43 +409,6 @@ 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() diff --git a/test/extended/builds/webhook.go b/test/extended/builds/webhook.go index 6206e516df78..41894a6991e6 100644 --- a/test/extended/builds/webhook.go +++ b/test/extended/builds/webhook.go @@ -3,12 +3,10 @@ package builds import ( "bytes" "context" - "crypto/tls" "encoding/json" "fmt" - "io" + "io/ioutil" "net/http" - "os" "time" g "github.com/onsi/ginkgo/v2" @@ -27,10 +25,7 @@ import ( var _ = g.Describe("[sig-builds][Feature:Builds][webhook]", func() { defer g.GinkgoRecover() - - var ( - oc = exutil.NewCLIWithPodSecurityLevel("build-webhooks", admissionapi.LevelBaseline) - ) + oc := exutil.NewCLIWithPodSecurityLevel("build-webhooks", admissionapi.LevelBaseline) g.It("TestWebhook [apigroup:build.openshift.io][apigroup:image.openshift.io]", func() { TestWebhook(g.GinkgoT(), oc) @@ -48,7 +43,6 @@ 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") @@ -59,12 +53,10 @@ 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 - client *http.Client - expectedStatus int + Name string + Payload string + HeaderFunc func(*http.Header) + URLs []string }{ { Name: "generic", @@ -73,8 +65,6 @@ 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", @@ -83,8 +73,6 @@ 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", @@ -93,8 +81,6 @@ 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", @@ -103,29 +89,6 @@ 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, }, } @@ -136,12 +99,8 @@ func TestWebhook(t g.GinkgoTInterface, oc *exutil.CLI) { clusterAdminClientConfig := oc.AdminConfig() g.By("executing the webhook to get the build object") - body := postFile(test.client, test.HeaderFunc, test.Payload, clusterAdminClientConfig.Host+s, test.expectedStatus, t, oc) + body := postFile(clusterAdminBuildClient.RESTClient(), test.HeaderFunc, test.Payload, clusterAdminClientConfig.Host+s, http.StatusOK, 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{} @@ -165,7 +124,6 @@ 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{ @@ -215,7 +173,7 @@ func TestWebhookGitHubPushWithImage(t g.GinkgoTInterface, oc *exutil.CLI) { } { // trigger build event sending push notification - body := postFile(adminHTTPClient, githubHeaderFunc, "github/testdata/pushevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc) + body := postFile(clusterAdminBuildClient.RESTClient(), 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") } @@ -243,6 +201,7 @@ 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) } @@ -255,7 +214,6 @@ 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{ @@ -307,7 +265,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(adminHTTPClient, githubHeaderFunc, "github/testdata/pushevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc) + postFile(clusterAdminBuildClient.RESTClient(), githubHeaderFunc, "github/testdata/pushevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc) var build *buildv1.Build @@ -333,7 +291,6 @@ 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") @@ -355,7 +312,7 @@ func TestWebhookGitHubPing(t g.GinkgoTInterface, oc *exutil.CLI) { // trigger build event sending push notification clusterAdminClientConfig := oc.AdminConfig() - postFile(adminHTTPClient, githubHeaderFuncPing, "github/testdata/pingevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc) + postFile(clusterAdminBuildClient.RESTClient(), githubHeaderFuncPing, "github/testdata/pingevent.json", clusterAdminClientConfig.Host+s, http.StatusOK, t, oc) // TODO: improve negative testing timer := time.NewTimer(time.Second * 5) @@ -369,9 +326,9 @@ func TestWebhookGitHubPing(t g.GinkgoTInterface, oc *exutil.CLI) { } } -func postFile(client *http.Client, headerFunc func(*http.Header), filename, url string, expStatusCode int, t g.GinkgoTInterface, oc *exutil.CLI) []byte { +func postFile(client rest.Interface, 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 := os.ReadFile(path) + data, err := ioutil.ReadFile(path) if err != nil { t.Fatalf("Failed to open %s: %v", filename, err) } @@ -380,11 +337,11 @@ func postFile(client *http.Client, headerFunc func(*http.Header), filename, url t.Fatalf("Error creating POST request: %v", err) } headerFunc(&req.Header) - resp, err := client.Do(req) + resp, err := client.(*rest.RESTClient).Client.Do(req) if err != nil { t.Fatalf("Failed posting webhook: %v", err) } - body, _ := io.ReadAll(resp.Body) + body, _ := ioutil.ReadAll(resp.Body) if resp.StatusCode != expStatusCode { t.Errorf("Wrong response code, expecting %d, got %d: %s!", expStatusCode, resp.StatusCode, string(body)) }