feat: Removed usage of redundant GVR fields in k8s/workflow triggers (#…

…1333) * removed usage of redundent group,version,resource fields in k8s/workflow triggers Signed-off-by: Daniel Soifer <[email protected]> * fix Signed-off-by: Daniel Soifer <[email protected]>
argoproj · Sep 5, 2021 · 24ef7c6 · 24ef7c6
1 parent f763e68
commit 24ef7c6
Show file tree

Hide file tree

Showing 9 changed files with 230 additions and 36 deletions.
diff --git a/controllers/sensor/validate.go b/controllers/sensor/validate.go
@@ -163,9 +163,7 @@ func validateK8STrigger(trigger *v1alpha1.StandardK8STrigger) error {
 	if trigger.Source == nil {
 		return errors.New("k8s trigger does not contain an absolute action")
 	}
-	if trigger.GroupVersionResource.Size() == 0 {
-		return errors.New("must provide group, version and resource for the resource")
-	}
+
 	switch trigger.Operation {
 	case "", v1alpha1.Create, v1alpha1.Patch, v1alpha1.Update, v1alpha1.Delete:
 
@@ -190,9 +188,7 @@ func validateArgoWorkflowTrigger(trigger *v1alpha1.ArgoWorkflowTrigger) error {
 	if trigger.Source == nil {
 		return errors.New("argoWorkflow trigger does not contain an absolute action")
 	}
-	if trigger.GroupVersionResource.Size() == 0 {
-		return errors.New("must provide group, version and resource for the resource")
-	}
+
 	switch trigger.Operation {
 	case v1alpha1.Submit, v1alpha1.Suspend, v1alpha1.Retry, v1alpha1.Resume, v1alpha1.Resubmit, v1alpha1.Terminate:
 	default:

diff --git a/docs/quick_start.md b/docs/quick_start.md
@@ -25,7 +25,7 @@ Note: You will need to have [Argo Workflows](https://argoproj.github.io/argo-wor
 
 1. Create webhook sensor.
 
-        kubectl apply -n argo-events -f https://raw.githubusercontent.com/argoproj/argo-events/stable/examples/sensors/webhook.yaml
+        kubectl apply -n argo-events -f https://raw.githubusercontent.com/argoproj/argo-events/master/examples/sensors/webhook-shortened.yaml
 
    Once the sensor object is created, sensor controller will create corresponding pod and a service. 
 

diff --git a/examples/sensors/special-workflow-trigger-shortened.yaml b/examples/sensors/special-workflow-trigger-shortened.yaml
@@ -0,0 +1,43 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Sensor
+metadata:
+  name: webhook
+spec:
+  template:
+    serviceAccountName: operate-workflow-sa
+  dependencies:
+    - name: test-dep
+      eventSourceName: webhook
+      eventName: example
+  triggers:
+    - template:
+        name: argo-workflow-trigger
+        argoWorkflow:
+          operation: submit
+          source:
+            resource:
+              apiVersion: argoproj.io/v1alpha1
+              kind: Workflow
+              metadata:
+                name: special-trigger
+              spec:
+                entrypoint: whalesay
+                arguments:
+                  parameters:
+                    - name: message
+                      # the value will get overridden by event payload from test-dep
+                      value: hello world
+                templates:
+                  - name: whalesay
+                    inputs:
+                      parameters:
+                        - name: message
+                    container:
+                      image: docker/whalesay:latest
+                      command: [cowsay]
+                      args: ["{{inputs.parameters.message}}"]
+          parameters:
+            - src:
+                dependencyName: test-dep
+                dataKey: body
+              dest: spec.arguments.parameters.0.value
diff --git a/examples/sensors/trigger-standard-k8s-resource-shortened.yaml b/examples/sensors/trigger-standard-k8s-resource-shortened.yaml
@@ -0,0 +1,62 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Sensor
+metadata:
+  name: webhook
+spec:
+  template:
+    serviceAccountName: operate-workflow-sa
+  dependencies:
+    - name: test-dep
+      eventSourceName: webhook
+      eventName: example
+  triggers:
+    - template:
+        name: webhook-pod-trigger
+        k8s:
+          operation: create
+          source:
+            resource:
+              apiVersion: v1
+              kind: Pod
+              metadata:
+                generateName: hello-world-
+              spec:
+                containers:
+                  - name: hello-container
+                    args:
+                      - "hello-world"
+                    command:
+                      - cowsay
+                    image: "docker/whalesay:latest"
+          parameters:
+            - src:
+                dependencyName: test-dep
+                dataKey: body
+              dest: spec.containers.0.args.0
+#    - template:
+#        name: webhook-deployment-trigger
+#        k8s:
+#          operation: create
+#          source:
+#            resource:
+#              apiVersion: apps/v1
+#              kind: Deployment
+#              metadata:
+#                generateName: hello-world-
+#              spec:
+#                replicas: 1
+#                selector:
+#                  matchLabels:
+#                    app: mydeploy
+#                template:
+#                  metadata:
+#                    labels:
+#                      app: mydeploy
+#                  spec:
+#                    containers:
+#                      - name: hello-container
+#                        args:
+#                         - "hello world"
+#                        command:
+#                         - cowsay
+#                        image: "docker/whalesay:latest"
diff --git a/examples/sensors/webhook-shortened.yaml b/examples/sensors/webhook-shortened.yaml
@@ -0,0 +1,43 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Sensor
+metadata:
+  name: webhook
+spec:
+  template:
+    serviceAccountName: operate-workflow-sa
+  dependencies:
+    - name: test-dep
+      eventSourceName: webhook
+      eventName: example
+  triggers:
+    - template:
+        name: webhook-workflow-trigger
+        k8s:
+          operation: create
+          source:
+            resource:
+              apiVersion: argoproj.io/v1alpha1
+              kind: Workflow
+              metadata:
+                generateName: webhook-
+              spec:
+                entrypoint: whalesay
+                arguments:
+                  parameters:
+                  - name: message
+                    # the value will get overridden by event payload from test-dep
+                    value: hello world
+                templates:
+                - name: whalesay
+                  inputs:
+                    parameters:
+                    - name: message
+                  container:
+                    image: docker/whalesay:latest
+                    command: [cowsay]
+                    args: ["{{inputs.parameters.message}}"]
+          parameters:
+            - src:
+                dependencyName: test-dep
+                dataKey: body
+              dest: spec.arguments.parameters.0.value
diff --git a/sensors/triggers/fetch.go b/sensors/triggers/fetch.go
@@ -36,6 +36,9 @@ func FetchKubernetesResource(source *v1alpha1.ArtifactLocation) (*unstructured.U
 	if err != nil {
 		return nil, err
 	}
+
+	// uObj will either hold the resource definition stored in the trigger or just
+	// a stub to provide enough information to fetch the object from K8s cluster
 	uObj, err := artifacts.FetchArtifact(reader)
 	if err != nil {
 		return nil, err

diff --git a/sensors/triggers/standard-k8s/standar-k8s.go b/sensors/triggers/standard-k8s/standar-k8s.go
@@ -29,7 +29,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/dynamic"
@@ -38,7 +37,6 @@ import (
 	"github.com/argoproj/argo-events/common/logging"
 	apicommon "github.com/argoproj/argo-events/pkg/apis/common"
 	"github.com/argoproj/argo-events/pkg/apis/sensor/v1alpha1"
-	"github.com/argoproj/argo-events/sensors/artifacts"
 	"github.com/argoproj/argo-events/sensors/policy"
 	"github.com/argoproj/argo-events/sensors/triggers"
 )
@@ -83,32 +81,15 @@ func (k8sTrigger *StandardK8sTrigger) GetTriggerType() apicommon.TriggerType {
 // FetchResource fetches the trigger resource from external source
 func (k8sTrigger *StandardK8sTrigger) FetchResource(ctx context.Context) (interface{}, error) {
 	trigger := k8sTrigger.Trigger
-	if trigger.Template.K8s.Source == nil {
-		return nil, errors.Errorf("trigger source for k8s is empty")
-	}
-	creds, err := artifacts.GetCredentials(trigger.Template.K8s.Source)
-	if err != nil {
-		return nil, err
-	}
-	reader, err := artifacts.GetArtifactReader(trigger.Template.K8s.Source, creds)
-	if err != nil {
-		return nil, err
-	}
-
 	var rObj runtime.Object
 
-	// uObj will either hold the resource definition stored in the trigger or just
-	// a stub to provide enough information to fetch the object from K8s cluster
-	uObj, err := artifacts.FetchArtifact(reader)
+	uObj, err := triggers.FetchKubernetesResource(trigger.Template.K8s.Source)
 	if err != nil {
 		return nil, err
 	}
 
-	k8sTrigger.namespableDynamicClient = k8sTrigger.DynamicClient.Resource(schema.GroupVersionResource{
-		Group:    trigger.Template.K8s.GroupVersionResource.Group,
-		Version:  trigger.Template.K8s.GroupVersionResource.Version,
-		Resource: trigger.Template.K8s.GroupVersionResource.Resource,
-	})
+	gvr := triggers.GetGroupVersionResource(uObj)
+	k8sTrigger.namespableDynamicClient = k8sTrigger.DynamicClient.Resource(gvr)
 
 	if trigger.Template.K8s.LiveObject && trigger.Template.K8s.Operation == v1alpha1.Update {
 		objName := uObj.GetName()
@@ -117,7 +98,7 @@ func (k8sTrigger *StandardK8sTrigger) FetchResource(ctx context.Context) (interf
 		}
 
 		objNamespace := uObj.GetNamespace()
-		_, isClusterResource := clusterResources[trigger.Template.K8s.GroupVersionResource.Resource]
+		_, isClusterResource := clusterResources[gvr.Resource]
 		if objNamespace == "" && !isClusterResource {
 			return nil, fmt.Errorf("resource namespace must be specified for fetching live object")
 		}
@@ -152,8 +133,9 @@ func (k8sTrigger *StandardK8sTrigger) Execute(ctx context.Context, events map[st
 		return nil, errors.New("failed to interpret the trigger resource")
 	}
 
+	gvr := triggers.GetGroupVersionResource(obj)
 	namespace := ""
-	if _, isClusterResource := clusterResources[trigger.Template.K8s.GroupVersionResource.Resource]; !isClusterResource {
+	if _, isClusterResource := clusterResources[gvr.Resource]; !isClusterResource {
 		namespace = obj.GetNamespace()
 		// Defaults to sensor's namespace
 		if namespace == "" {
@@ -169,11 +151,7 @@ func (k8sTrigger *StandardK8sTrigger) Execute(ctx context.Context, events map[st
 
 	// We might have a client from FetchResource() already, or we might not have one yet.
 	if k8sTrigger.namespableDynamicClient == nil {
-		k8sTrigger.namespableDynamicClient = k8sTrigger.DynamicClient.Resource(schema.GroupVersionResource{
-			Group:    trigger.Template.K8s.GroupVersionResource.Group,
-			Version:  trigger.Template.K8s.GroupVersionResource.Version,
-			Resource: trigger.Template.K8s.GroupVersionResource.Resource,
-		})
+		k8sTrigger.namespableDynamicClient = k8sTrigger.DynamicClient.Resource(gvr)
 	}
 
 	switch op {

diff --git a/sensors/triggers/triggers.go b/sensors/triggers/triggers.go
@@ -0,0 +1,34 @@
+/*
+Copyright 2020 BlackRock, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package triggers
+
+import (
+	"fmt"
+	"strings"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+func GetGroupVersionResource(obj *unstructured.Unstructured) schema.GroupVersionResource {
+	gvk := obj.GroupVersionKind()
+	return schema.GroupVersionResource{
+		Group:    gvk.Group,
+		Version:  gvk.Version,
+		Resource: fmt.Sprintf("%ss", strings.ToLower(gvk.Kind)),
+	}
+}
diff --git a/sensors/triggers/triggers_test.go b/sensors/triggers/triggers_test.go
@@ -0,0 +1,35 @@
+/*
+Copyright 2020 BlackRock, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package triggers
+
+import (
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetGroupVersionResource(t *testing.T) {
+	deployment := newUnstructured("apps/v1", "Deployment", "fake", "test")
+	expectedGVR := schema.GroupVersionResource{
+		Group:    "apps",
+		Version:  "v1",
+		Resource: "deployments",
+	}
+	result := GetGroupVersionResource(deployment)
+	assert.Equal(t, expectedGVR, result)
+}