From ec515595de5bba8dd162c844b3528d1ac68daa90 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Wed, 13 Mar 2019 20:34:49 +0900 Subject: [PATCH] Handle empty namespace on trigger resources (#224) * Set sensor namespace on trigger resources by default * Update OpenAPI gen * Update Gopkg.lock * Fix tests * Make condition easier to read --- Gopkg.lock | 5 +- pkg/apis/sensor/v1alpha1/openapi_generated.go | 9 ++- pkg/apis/sensor/v1alpha1/types.go | 2 +- sensors/event-handler_test.go | 17 ++++- sensors/trigger.go | 12 ++- sensors/trigger_test.go | 75 +++++++++++++++---- 6 files changed, 94 insertions(+), 26 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index ba8c8e324d..cdf68c5578 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1290,12 +1290,13 @@ [[projects]] branch = "release-7.0" - digest = "1:6e37d35d68d68b1368b803617d346633830eb13b50120f68ead2c029c05eeea0" + digest = "1:7336dcb25cac9940ac84ea6c59c4c36d1b6b6ca4a3aba38d6d8c74bc53699a40" name = "k8s.io/client-go" packages = [ "discovery", "discovery/fake", "dynamic", + "dynamic/fake", "informers", "informers/admissionregistration", "informers/admissionregistration/v1alpha1", @@ -1552,6 +1553,7 @@ "github.com/nats-io/go-nats", "github.com/nats-io/go-nats-streaming", "github.com/nlopes/slack/slackevents", + "github.com/pkg/errors", "github.com/robfig/cron", "github.com/rs/zerolog", "github.com/satori/go.uuid", @@ -1594,6 +1596,7 @@ "k8s.io/client-go/discovery", "k8s.io/client-go/discovery/fake", "k8s.io/client-go/dynamic", + "k8s.io/client-go/dynamic/fake", "k8s.io/client-go/informers", "k8s.io/client-go/informers/core/v1", "k8s.io/client-go/kubernetes", diff --git a/pkg/apis/sensor/v1alpha1/openapi_generated.go b/pkg/apis/sensor/v1alpha1/openapi_generated.go index 250dbcfdba..71540cf4cd 100644 --- a/pkg/apis/sensor/v1alpha1/openapi_generated.go +++ b/pkg/apis/sensor/v1alpha1/openapi_generated.go @@ -159,7 +159,7 @@ func schema_pkg_apis_sensor_v1alpha1_DataFilter(ref common.ReferenceCallback) co }, "value": { SchemaProps: spec.SchemaProps{ - Description: "Value is the allowed string values for this key Booleans are pased using strconv.ParseBool() Numbers are parsed using as float64 using strconv.ParseFloat() Strings are taken as is Nils this value is ignored", + Description: "Value is the allowed string values for this key Booleans are passed using strconv.ParseBool() Numbers are parsed using as float64 using strconv.ParseFloat() Strings are taken as is Nils this value is ignored", Type: []string{"array"}, Items: &spec.SchemaOrArray{ Schema: &spec.Schema{ @@ -387,7 +387,7 @@ func schema_pkg_apis_sensor_v1alpha1_GitArtifact(ref common.ReferenceCallback) c }, "remote": { SchemaProps: spec.SchemaProps{ - Description: "Git remote to manage set of tracked repositories. Defaults to \"origin\". Refer https://git-scm.com/docs/git-remote", + Description: "Remote to manage set of tracked repositories. Defaults to \"origin\". Refer https://git-scm.com/docs/git-remote", Ref: ref("github.com/argoproj/argo-events/pkg/apis/sensor/v1alpha1.GitRemoteConfig"), }, }, @@ -430,7 +430,8 @@ func schema_pkg_apis_sensor_v1alpha1_GitRemoteConfig(ref common.ReferenceCallbac return common.OpenAPIDefinition{ Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, + Description: "GitRemoteConfig contains the configuration of a Git remote", + Type: []string{"object"}, Properties: map[string]spec.Schema{ "name": { SchemaProps: spec.SchemaProps{ @@ -595,7 +596,7 @@ func schema_pkg_apis_sensor_v1alpha1_ResourceObject(ref common.ReferenceCallback }, "namespace": { SchemaProps: spec.SchemaProps{ - Description: "Namespace in which to create this object defaults to the service account namespace", + Description: "Namespace in which to create this object defaults to the sensor namespace", Type: []string{"string"}, Format: "", }, diff --git a/pkg/apis/sensor/v1alpha1/types.go b/pkg/apis/sensor/v1alpha1/types.go index 664a416204..5a8963fc08 100644 --- a/pkg/apis/sensor/v1alpha1/types.go +++ b/pkg/apis/sensor/v1alpha1/types.go @@ -261,7 +261,7 @@ type ResourceObject struct { GroupVersionKind `json:",inline" protobuf:"bytes,5,opt,name=groupVersionKind"` // Namespace in which to create this object - // defaults to the service account namespace + // defaults to the sensor namespace Namespace string `json:"namespace" protobuf:"bytes,1,opt,name=namespace"` // Source of the K8 resource file(s) diff --git a/sensors/event-handler_test.go b/sensors/event-handler_test.go index fb8e23a71d..e22fa1b3d0 100644 --- a/sensors/event-handler_test.go +++ b/sensors/event-handler_test.go @@ -37,6 +37,7 @@ import ( var sensorStr = `apiVersion: argoproj.io/v1alpha1 kind: Sensor metadata: + namespace: argo-events name: test-sensor labels: sensors.argoproj.io/sensor-controller-instanceid: argo-events @@ -78,6 +79,13 @@ spec: image: "docker/whalesay:latest" name: whalesay` +var podResourceList = metav1.APIResourceList{ + GroupVersion: metav1.GroupVersion{Group: "", Version: "v1"}.String(), + APIResources: []metav1.APIResource{ + {Kind: "Pod", Namespaced: true, Name: "pods", SingularName: "pod", Group: "", Version: "v1", Verbs: []string{"create", "get"}}, + }, +} + func getSensor() (*v1alpha1.Sensor, error) { var sensor v1alpha1.Sensor err := yaml.Unmarshal([]byte(sensorStr), &sensor) @@ -100,10 +108,15 @@ func (m *mockHttpWriter) WriteHeader(statusCode int) { func getsensorExecutionCtx(sensor *v1alpha1.Sensor) *sensorExecutionCtx { kubeClientset := fake.NewSimpleClientset() + fakeDiscoveryClient := kubeClientset.Discovery().(*discoveryFake.FakeDiscovery) + clientPool := &FakeClientPool{ + Fake: kubeClientset.Fake, + } + fakeDiscoveryClient.Resources = append(fakeDiscoveryClient.Resources, &podResourceList) return &sensorExecutionCtx{ kubeClient: kubeClientset, - discoveryClient: kubeClientset.Discovery().(*discoveryFake.FakeDiscovery), - clientPool: NewFakeClientPool(), + discoveryClient: fakeDiscoveryClient, + clientPool: clientPool, log: common.GetLoggerContext(common.LoggerConf()).Logger(), sensorClient: sensorFake.NewSimpleClientset(), sensor: sensor, diff --git a/sensors/trigger.go b/sensors/trigger.go index 760e219970..9d9a1fdfcb 100644 --- a/sensors/trigger.go +++ b/sensors/trigger.go @@ -178,9 +178,13 @@ func (sec *sensorExecutionCtx) executeTrigger(trigger v1alpha1.Trigger) error { // createResourceObject creates K8s object for trigger func (sec *sensorExecutionCtx) createResourceObject(resource *v1alpha1.ResourceObject, obj *unstructured.Unstructured) error { - if resource.Namespace != "" { - obj.SetNamespace(resource.Namespace) + namespace := resource.Namespace + // Defaults to sensor's namespace + if namespace == "" { + namespace = sec.sensor.Namespace } + obj.SetNamespace(namespace) + if resource.Labels != nil { labels := obj.GetLabels() if labels != nil { @@ -225,13 +229,13 @@ func (sec *sensorExecutionCtx) createResourceObject(resource *v1alpha1.ResourceO } sec.log.Info().Str("api", apiResource.Name).Str("group-version", gvk.Version).Msg("created api resource") - reIf := client.Resource(apiResource, resource.Namespace) + reIf := client.Resource(apiResource, namespace) liveObj, err := reIf.Create(obj) if err != nil { return fmt.Errorf("failed to create resource object. err: %+v", err) } sec.log.Info().Str("kind", liveObj.GetKind()).Str("name", liveObj.GetName()).Msg("created object") - if !errors.IsAlreadyExists(err) { + if err == nil || !errors.IsAlreadyExists(err) { return err } liveObj, err = reIf.Get(obj.GetName(), metav1.GetOptions{}) diff --git a/sensors/trigger_test.go b/sensors/trigger_test.go index 1af95d1599..b52fd6dd8d 100644 --- a/sensors/trigger_test.go +++ b/sensors/trigger_test.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/dynamic" + dynamicfake "k8s.io/client-go/dynamic/fake" "k8s.io/client-go/kubernetes/fake" kTesting "k8s.io/client-go/testing" "k8s.io/client-go/util/flowcontrol" @@ -197,20 +198,14 @@ func (p *FakeClientPool) ClientForGroupVersionKind(kind schema.GroupVersionKind) } var testWf = ` -apiVersion: argoproj.io/v1alpha1 -kind: Workflow +apiVersion: v1 +kind: Pod metadata: generateName: hello-world- spec: -entrypoint: whalesay -templates: -- name: whalesay -container: - args: - - "hello world" - command: - - cowsay - image: "docker/whalesay:latest" + containers: + - name: whalesay + image: "docker/whalesay:latest" ` var testTrigger = v1alpha1.Trigger{ @@ -218,9 +213,8 @@ var testTrigger = v1alpha1.Trigger{ Resource: &v1alpha1.ResourceObject{ Namespace: corev1.NamespaceDefault, GroupVersionKind: v1alpha1.GroupVersionKind{ - Group: "argoproj.io", - Version: "v1alpha1", - Kind: "workflow", + Version: "v1", + Kind: "Pod", }, Source: v1alpha1.ArtifactLocation{ Inline: &testWf, @@ -241,3 +235,56 @@ func TestProcessTrigger(t *testing.T) { convey.So(err, convey.ShouldNotBeNil) }) } + +func TestCreateResourceObject(t *testing.T) { + convey.Convey("Given a resource object", t, func() { + testSensor, err := getSensor() + convey.So(err, convey.ShouldBeNil) + soc := getsensorExecutionCtx(testSensor) + fakeclient := soc.clientPool.(*FakeClientPool).Fake + dynamicClient := dynamicfake.FakeResourceClient{Resource: schema.GroupVersionResource{Version: "v1", Resource: "pods"}, Fake: &fakeclient} + + convey.Convey("Given a pod", func() { + rObj := testTrigger.Resource.DeepCopy() + rObj.Namespace = "foo" + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: rObj.Namespace, Name: "my-pod"}, + } + uObj, err := getUnstructuredPod(pod) + convey.So(err, convey.ShouldBeNil) + + err = soc.createResourceObject(rObj, uObj) + convey.So(err, convey.ShouldBeNil) + + unstructuredPod, err := dynamicClient.Get(pod.Name, metav1.GetOptions{}) + convey.So(err, convey.ShouldBeNil) + convey.So(unstructuredPod.GetNamespace(), convey.ShouldEqual, rObj.Namespace) + }) + convey.Convey("Given a pod without namespace, use sensor namespace", func() { + rObj := testTrigger.Resource.DeepCopy() + rObj.Namespace = "" + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "my-pod-without-namespace"}, + } + uObj, err := getUnstructuredPod(pod) + convey.So(err, convey.ShouldBeNil) + + err = soc.createResourceObject(rObj, uObj) + convey.So(err, convey.ShouldBeNil) + + unstructuredPod, err := dynamicClient.Get(pod.Name, metav1.GetOptions{}) + convey.So(err, convey.ShouldBeNil) + convey.So(unstructuredPod.GetNamespace(), convey.ShouldEqual, testSensor.Namespace) + }) + }) +} + +func getUnstructuredPod(pod *corev1.Pod) (*unstructured.Unstructured, error) { + obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) + if err != nil { + return nil, err + } + return &unstructured.Unstructured{Object: obj}, nil +}