Skip to content

Commit

Permalink
Merge pull request #12573 from oatmealraisin/rymurphy/bz1373441
Browse files Browse the repository at this point in the history
Merged by openshift-bot
  • Loading branch information
OpenShift Bot authored Mar 17, 2017
2 parents 6857796 + 5909f5f commit 769c079
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 25 deletions.
18 changes: 16 additions & 2 deletions pkg/build/registry/buildconfig/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import (

kapi "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/errors"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/runtime"
utilruntime "k8s.io/kubernetes/pkg/util/runtime"

buildapi "github.com/openshift/origin/pkg/build/api"
"github.com/openshift/origin/pkg/build/client"
Expand All @@ -15,8 +18,9 @@ import (
)

// NewWebHookREST returns the webhook handler wrapped in a rest.WebHook object.
func NewWebHookREST(registry Registry, instantiator client.BuildConfigInstantiator, plugins map[string]webhook.Plugin) *rest.WebHook {
func NewWebHookREST(registry Registry, instantiator client.BuildConfigInstantiator, groupVersion unversioned.GroupVersion, plugins map[string]webhook.Plugin) *rest.WebHook {
hook := &WebHook{
groupVersion: groupVersion,
registry: registry,
instantiator: instantiator,
plugins: plugins,
Expand All @@ -25,6 +29,7 @@ func NewWebHookREST(registry Registry, instantiator client.BuildConfigInstantiat
}

type WebHook struct {
groupVersion unversioned.GroupVersion
registry Registry
instantiator client.BuildConfigInstantiator
plugins map[string]webhook.Plugin
Expand Down Expand Up @@ -73,9 +78,18 @@ func (w *WebHook) ServeHTTP(writer http.ResponseWriter, req *http.Request, ctx k
Env: envvars,
DockerStrategyOptions: dockerStrategyOptions,
}
if _, err := w.instantiator.Instantiate(config.Namespace, request); err != nil {
newBuild, err := w.instantiator.Instantiate(config.Namespace, request)
if err != nil {
return errors.NewInternalError(fmt.Errorf("could not generate a build: %v", err))
}

// Send back the build name so that the client can alert the user.
if newBuildEncoded, err := runtime.Encode(kapi.Codecs.LegacyCodec(w.groupVersion), newBuild); err != nil {
utilruntime.HandleError(err)
} else {
writer.Write(newBuildEncoded)
}

return warning
}

Expand Down
50 changes: 39 additions & 11 deletions pkg/build/registry/buildconfig/webhook_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package buildconfig

import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
"reflect"
Expand All @@ -14,7 +16,9 @@ import (
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/runtime"

_ "github.com/openshift/origin/pkg/api/install"
"github.com/openshift/origin/pkg/build/api"
buildapiv1 "github.com/openshift/origin/pkg/build/api/v1"
"github.com/openshift/origin/pkg/build/registry/test"
"github.com/openshift/origin/pkg/build/webhook"
"github.com/openshift/origin/pkg/build/webhook/github"
Expand All @@ -29,7 +33,15 @@ type buildConfigInstantiator struct {

func (i *buildConfigInstantiator) Instantiate(namespace string, request *api.BuildRequest) (*api.Build, error) {
i.Request = request
return i.Build, i.Err
if i.Build != nil {
return i.Build, i.Err
}
return &api.Build{
ObjectMeta: kapi.ObjectMeta{
Name: request.Name,
Namespace: namespace,
},
}, i.Err
}

type plugin struct {
Expand All @@ -48,7 +60,7 @@ func (p *plugin) Extract(buildCfg *api.BuildConfig, secret, path string, req *ht
func newStorage() (*rest.WebHook, *buildConfigInstantiator, *test.BuildConfigRegistry) {
mockRegistry := &test.BuildConfigRegistry{}
bci := &buildConfigInstantiator{}
hook := NewWebHookREST(mockRegistry, bci, map[string]webhook.Plugin{
hook := NewWebHookREST(mockRegistry, bci, buildapiv1.SchemeGroupVersion, map[string]webhook.Plugin{
"ok": &plugin{Proceed: true},
"okenv": &plugin{
Env: []kapi.EnvVar{
Expand Down Expand Up @@ -145,7 +157,18 @@ func TestConnectWebHook(t *testing.T) {
Obj: &api.BuildConfig{ObjectMeta: kapi.ObjectMeta{Name: "test", Namespace: "default"}},
ErrFn: func(err error) bool { return err == nil },
WFn: func(w *httptest.ResponseRecorder) bool {
return w.Code == http.StatusOK
body, _ := ioutil.ReadAll(w.Body)
// We want to make sure that we return the created build in the body.
if w.Code == http.StatusOK && len(body) > 0 {
// The returned json needs to be a v1 Build specifically
newBuild := &buildapiv1.Build{}
err := json.Unmarshal(body, newBuild)
if err == nil {
return true
}
return false
}
return false
},
Instantiate: true,
},
Expand Down Expand Up @@ -209,7 +232,12 @@ func TestConnectWebHook(t *testing.T) {
type okBuildConfigInstantiator struct{}

func (*okBuildConfigInstantiator) Instantiate(namespace string, request *api.BuildRequest) (*api.Build, error) {
return &api.Build{}, nil
return &api.Build{
ObjectMeta: kapi.ObjectMeta{
Namespace: namespace,
Name: request.Name,
},
}, nil
}

type errorBuildConfigInstantiator struct{}
Expand Down Expand Up @@ -278,7 +306,7 @@ var mockBuildStrategy = api.BuildStrategy{
func TestParseUrlError(t *testing.T) {
bcRegistry := &test.BuildConfigRegistry{BuildConfig: testBuildConfig}
responder := &fakeResponder{}
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, map[string]webhook.Plugin{"github": github.New()}).
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, buildapiv1.SchemeGroupVersion, map[string]webhook.Plugin{"github": github.New()}).
Connect(kapi.NewDefaultContext(), "build100", &kapi.PodProxyOptions{Path: ""}, responder)
server := httptest.NewServer(handler)
defer server.Close()
Expand All @@ -296,7 +324,7 @@ func TestParseUrlError(t *testing.T) {
func TestParseUrlOK(t *testing.T) {
bcRegistry := &test.BuildConfigRegistry{BuildConfig: testBuildConfig}
responder := &fakeResponder{}
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, map[string]webhook.Plugin{"pathplugin": &pathPlugin{}}).
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, buildapiv1.SchemeGroupVersion, map[string]webhook.Plugin{"pathplugin": &pathPlugin{}}).
Connect(kapi.NewDefaultContext(), "build100", &kapi.PodProxyOptions{Path: "secret101/pathplugin"}, responder)
server := httptest.NewServer(handler)
defer server.Close()
Expand All @@ -314,7 +342,7 @@ func TestParseUrlLong(t *testing.T) {
plugin := &pathPlugin{}
bcRegistry := &test.BuildConfigRegistry{BuildConfig: testBuildConfig}
responder := &fakeResponder{}
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, map[string]webhook.Plugin{"pathplugin": plugin}).
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, buildapiv1.SchemeGroupVersion, map[string]webhook.Plugin{"pathplugin": plugin}).
Connect(kapi.NewDefaultContext(), "build100", &kapi.PodProxyOptions{Path: "secret101/pathplugin/some/more/args"}, responder)
server := httptest.NewServer(handler)
defer server.Close()
Expand All @@ -332,7 +360,7 @@ func TestParseUrlLong(t *testing.T) {
func TestInvokeWebhookMissingPlugin(t *testing.T) {
bcRegistry := &test.BuildConfigRegistry{BuildConfig: testBuildConfig}
responder := &fakeResponder{}
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, map[string]webhook.Plugin{"pathplugin": &pathPlugin{}}).
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, buildapiv1.SchemeGroupVersion, map[string]webhook.Plugin{"pathplugin": &pathPlugin{}}).
Connect(kapi.NewDefaultContext(), "build100", &kapi.PodProxyOptions{Path: "secret101/missingplugin"}, responder)
server := httptest.NewServer(handler)
defer server.Close()
Expand All @@ -350,7 +378,7 @@ func TestInvokeWebhookMissingPlugin(t *testing.T) {
func TestInvokeWebhookErrorBuildConfigInstantiate(t *testing.T) {
bcRegistry := &test.BuildConfigRegistry{BuildConfig: testBuildConfig}
responder := &fakeResponder{}
handler, _ := NewWebHookREST(bcRegistry, &errorBuildConfigInstantiator{}, map[string]webhook.Plugin{"pathplugin": &pathPlugin{}}).
handler, _ := NewWebHookREST(bcRegistry, &errorBuildConfigInstantiator{}, buildapiv1.SchemeGroupVersion, map[string]webhook.Plugin{"pathplugin": &pathPlugin{}}).
Connect(kapi.NewDefaultContext(), "build100", &kapi.PodProxyOptions{Path: "secret101/pathplugin"}, responder)
server := httptest.NewServer(handler)
defer server.Close()
Expand All @@ -368,7 +396,7 @@ func TestInvokeWebhookErrorBuildConfigInstantiate(t *testing.T) {
func TestInvokeWebhookErrorGetConfig(t *testing.T) {
bcRegistry := &test.BuildConfigRegistry{BuildConfig: testBuildConfig}
responder := &fakeResponder{}
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, map[string]webhook.Plugin{"pathplugin": &pathPlugin{}}).
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, buildapiv1.SchemeGroupVersion, map[string]webhook.Plugin{"pathplugin": &pathPlugin{}}).
Connect(kapi.NewDefaultContext(), "badbuild100", &kapi.PodProxyOptions{Path: "secret101/pathplugin"}, responder)
server := httptest.NewServer(handler)
defer server.Close()
Expand All @@ -388,7 +416,7 @@ func TestInvokeWebhookErrorGetConfig(t *testing.T) {
func TestInvokeWebhookErrorCreateBuild(t *testing.T) {
bcRegistry := &test.BuildConfigRegistry{BuildConfig: testBuildConfig}
responder := &fakeResponder{}
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, map[string]webhook.Plugin{"errPlugin": &errPlugin{}}).
handler, _ := NewWebHookREST(bcRegistry, &okBuildConfigInstantiator{}, buildapiv1.SchemeGroupVersion, map[string]webhook.Plugin{"errPlugin": &errPlugin{}}).
Connect(kapi.NewDefaultContext(), "build100", &kapi.PodProxyOptions{Path: "secret101/errPlugin"}, responder)
server := httptest.NewServer(handler)
defer server.Close()
Expand Down
16 changes: 16 additions & 0 deletions pkg/cmd/cli/cmd/startbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ import (
kclientcmd "k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
"k8s.io/kubernetes/pkg/fields"
kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/third_party/forked/golang/netutil"

buildapi "github.com/openshift/origin/pkg/build/api"
buildapiv1 "github.com/openshift/origin/pkg/build/api/v1"
osclient "github.com/openshift/origin/pkg/client"
"github.com/openshift/origin/pkg/cmd/templates"
cmdutil "github.com/openshift/origin/pkg/cmd/util"
Expand Down Expand Up @@ -711,6 +713,20 @@ func (o *StartBuildOptions) RunStartBuildWebHook() error {
body, _ := ioutil.ReadAll(resp.Body)
return fmt.Errorf("server rejected our request %d\nremote: %s", resp.StatusCode, string(body))
}

body, _ := ioutil.ReadAll(resp.Body)
if len(body) > 0 {
// In later server versions we return the created Build in the body.
var newBuild buildapi.Build
if err = json.Unmarshal(body, &buildapiv1.Build{}); err == nil {
if err = runtime.DecodeInto(kapi.Codecs.UniversalDecoder(), body, &newBuild); err != nil {
return err
}

kcmdutil.PrintSuccess(o.Mapper, o.ShortOutput, o.Out, "build", newBuild.Name, false, "started")
}
}

return nil
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/server/origin/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,10 @@ func (c *MasterConfig) GetRestStorage() map[unversioned.GroupVersion]map[string]
buildConfigWebHooks := buildconfigregistry.NewWebHookREST(
buildConfigRegistry,
buildclient.NewOSClientBuildConfigInstantiatorClient(bcClient),
// We use the buildapiv1 schemegroup to encode the Build that gets
// returned. As such, we need to make sure that the GroupVersion we use
// is the same API version that the storage is going to be used for.
buildapiv1.SchemeGroupVersion,
map[string]webhook.Plugin{
"generic": generic.New(),
"github": github.New(),
Expand Down
5 changes: 4 additions & 1 deletion test/cmd/builds.sh
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ os::cmd::expect_success_and_text 'oc start-build --list-webhooks=all bc/ruby-sam
os::cmd::expect_success_and_text 'oc start-build --list-webhooks=all ruby-sample-build' 'github'
os::cmd::expect_success_and_text 'oc start-build --list-webhooks=github ruby-sample-build' 'secret101'
os::cmd::expect_failure 'oc start-build --list-webhooks=blah'
os::cmd::expect_success "oc start-build --from-webhook='$(oc start-build --list-webhooks='generic' ruby-sample-build --api-version=v1 | head -n 1)'"
os::cmd::expect_success_and_text "oc start-build --from-webhook='$(oc start-build --list-webhooks='generic' ruby-sample-build --api-version=v1 | head -n 1)'" "build \"ruby-sample-build-[0-9]\" started"
os::cmd::expect_failure_and_text "oc start-build --from-webhook='$(oc start-build --list-webhooks='generic' ruby-sample-build --api-version=v1 | head -n 1)/foo'" "error: server rejected our request"
os::cmd::expect_success "oc patch bc/ruby-sample-build -p '{\"spec\":{\"strategy\":{\"dockerStrategy\":{\"from\":{\"name\":\"asdf:7\"}}}}}'"
os::cmd::expect_failure_and_text "oc start-build --from-webhook='$(oc start-build --list-webhooks='generic' ruby-sample-build --api-version=v1 | head -n 1)'" "Error resolving ImageStreamTag asdf:7"
os::cmd::expect_success 'oc get builds'
os::cmd::expect_success 'oc delete all -l build=docker'
echo "buildConfig: ok"
Expand Down
87 changes: 87 additions & 0 deletions test/integration/webhookgeneric_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package integration

import (
"encoding/json"
"net/http"
"testing"
"time"

kapi "k8s.io/kubernetes/pkg/api"

buildapi "github.com/openshift/origin/pkg/build/api"
buildapiv1 "github.com/openshift/origin/pkg/build/api/v1"
"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
testutil "github.com/openshift/origin/test/util"
testserver "github.com/openshift/origin/test/util/server"
)

func TestWebhookGeneric(t *testing.T) {
testutil.RequireEtcd(t)
defer testutil.DumpEtcdOnFailure(t)
_, clusterAdminKubeConfig, err := testserver.StartTestMaster()
if err != nil {
t.Fatalf("unable to start master: %v", err)
}

kubeClient, err := testutil.GetClusterAdminKubeClient(clusterAdminKubeConfig)
if err != nil {
t.Fatalf("unable to get kubeClient: %v", err)
}
osClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig)
if err != nil {
t.Fatalf("unable to get osClient: %v", err)
}

kubeClient.Core().Namespaces().Create(&kapi.Namespace{
ObjectMeta: kapi.ObjectMeta{Name: testutil.Namespace()},
})

if err := testserver.WaitForServiceAccounts(kubeClient, testutil.Namespace(), []string{bootstrappolicy.BuilderServiceAccountName, bootstrappolicy.DefaultServiceAccountName}); err != nil {
t.Errorf("unexpected error: %v", err)
}

// create buildconfig
buildConfig := mockBuildConfigImageParms("originalimage", "imagestream", "validtag")
if _, err := osClient.BuildConfigs(testutil.Namespace()).Create(buildConfig); err != nil {
t.Fatalf("Unexpected error: %v", err)
}

watch, err := osClient.Builds(testutil.Namespace()).Watch(kapi.ListOptions{})
if err != nil {
t.Fatalf("Couldn't subscribe to builds: %v", err)
}
defer watch.Stop()

for _, s := range []string{
"/oapi/v1/namespaces/" + testutil.Namespace() + "/buildconfigs/pushbuild/webhooks/secret103/generic",
} {
// trigger build event sending push notification
clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

body := postFile(osClient.RESTClient.Client, "", "../../generic/testdata/push-generic.json", clusterAdminClientConfig.Host+s, http.StatusOK, t)
if len(body) == 0 {
t.Fatalf("Webhook did not return expected Build object.")
}

returnedBuild := &buildapiv1.Build{}
err = json.Unmarshal(body, returnedBuild)
if err != nil {
t.Fatalf("Unable to unmarshal returned body into a Build object.")
}

// TODO: improve negative testing
timer := time.NewTimer(time.Minute * 1)
select {
case <-timer.C:
t.Fatalf("Did not receive created build.")
case event := <-watch.ResultChan():
build := event.Object.(*buildapi.Build)
if build.Name != returnedBuild.Name {
t.Fatalf("Webhook returned incorrect build.")
}
}
}
}
Loading

0 comments on commit 769c079

Please sign in to comment.