-
Notifications
You must be signed in to change notification settings - Fork 630
Make Actions dynamc and add sample. #50
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,9 +35,15 @@ type Bind struct { | |
| Status BindStatus `json:"status"` | ||
| } | ||
|
|
||
| // BindAction describes where events should be delivered. | ||
| type BindAction struct { | ||
| // RouteName specifies Elafros route as a target. | ||
| RouteName string `json:"routeName,omitempty"` | ||
| // Processor dictates the kind of service that will handle the Event. | ||
| // For example "elafros.dev/Route" | ||
| Processor string `json:"processor"` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Processor field gives us some extra flexibility in configuring different types of actions. Even if all the actions expect HTTP POSTs, a same-cluster Elafros action might have different requirements than an external action (e.g. different auth headers). Also conceivably we might want to allow non-HTTP actions. So I think there is value in keeping the Processor field.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't the internal vs. external header requirements best be handled via Istio/Envoy? And is there an example of a non-HTTP action that's more than "conceivable" at this point? If not, it seems premature to add a strategy pattern. Also since the different action types in this PR are hardcoded, it's not yet designed in a way to be open for extension. |
||
|
|
||
| // Name dicates the resource exposed by Processor that will handle the event. | ||
| // The semantics of Name is determined by Processor. | ||
| Name string `json:"name"` | ||
| } | ||
|
|
||
| // EventTrigger describes when an Event should be delivered. | ||
|
|
@@ -119,13 +125,22 @@ type EventTrigger struct { | |
| ParametersFrom []ParametersFromSource `json:"parametersFrom,omitempty"` | ||
| } | ||
|
|
||
| // Zero determines whether an EventTrigger is fully empty. If a Bind is set up | ||
| // without an EventTrigger, it is assumed that the real event source is incompatible | ||
| // with our event framework's control plane and will be set up manually through | ||
| // side channels. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any example or doc that describes the event framework control plane compatibility, and what this "side channels" concept means? Why would a bind be created if it's not establishing a connection to a trigger? |
||
| func (t *EventTrigger) Zero() bool { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, I think I'd prefer something like isZero or isEmpty. Zero to me indicates you're zeroing something. Might be good to say something about why the go default zero value is not enough in the comments also. |
||
| return t.EventType == "" && t.Resource == "" && t.Service == "" && | ||
| t.Parameters == nil && t.ParametersFrom == nil | ||
| } | ||
|
|
||
| // BindSpec is the spec for a Bind resource | ||
| type BindSpec struct { | ||
| // Action specifies the target handler for the events | ||
| Action BindAction `json:"action"` | ||
|
|
||
| // Trigger specifies the trigger we're binding to | ||
| Trigger EventTrigger `json:trigger"` | ||
| Trigger EventTrigger `json:"trigger"` | ||
| } | ||
|
|
||
| // ParametersFromSource represents the source of a set of Parameters | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ package bind | |
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/ghodss/yaml" | ||
|
|
@@ -48,6 +49,7 @@ import ( | |
| bindscheme "github.com/elafros/eventing/pkg/client/clientset/versioned/scheme" | ||
| informers "github.com/elafros/eventing/pkg/client/informers/externalversions" | ||
| listers "github.com/elafros/eventing/pkg/client/listers/bind/v1alpha1" | ||
| "github.com/elafros/eventing/pkg/delivery/action" | ||
| ) | ||
|
|
||
| const controllerAgentName = "bind-controller" | ||
|
|
@@ -133,7 +135,7 @@ func NewController( | |
| } | ||
|
|
||
| controller.sources = make(map[string]sources.EventSource) | ||
| controller.sources["github.com"] = sources.NewGithubEventSource() | ||
| controller.sources["github.com"] = sources.NewGithubEventSource(kubeclientset) | ||
| glog.Info("Setting up event handlers") | ||
| // Set up an event handler for when Bind resources change | ||
| bindInformer.Binds().Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ | ||
|
|
@@ -288,76 +290,40 @@ func (c *Controller) syncHandler(key string) error { | |
| bind = bind.DeepCopy() | ||
|
|
||
| // Find the Route that they want. | ||
| routeName := bind.Spec.Action.RouteName | ||
| route, err := c.routesLister.Routes(namespace).Get(routeName) | ||
| if err != nil { | ||
| actionName := bind.Spec.Action.Name | ||
| actionType := bind.Spec.Action.Processor | ||
| if err := c.validateAction(namespace, actionName, actionType); err != nil { | ||
| if errors.IsNotFound(err) { | ||
| runtime.HandleError(fmt.Errorf("Route %q in namespace %q does not exist", routeName, namespace)) | ||
| runtime.HandleError(fmt.Errorf("Action %q of type %q in namespace %q does not exist", actionName, actionType, namespace)) | ||
| } | ||
| glog.Warningf("Bind %q rejected for invalid action: %s", name, err) | ||
| return err | ||
| } | ||
|
|
||
| domainSuffix, err := getDomainSuffixFromElaConfig(c.kubeclientset) | ||
| if err != nil { | ||
| if errors.IsNotFound(err) { | ||
| runtime.HandleError(fmt.Errorf("Can't find ela ConfigMap")) | ||
| var condition *v1alpha1.BindCondition | ||
| if bind.Spec.Trigger.Zero() { | ||
| glog.Warningf("Bind %q has no Spec.Trigger", name) | ||
| if bind.Status.Conditions != nil { | ||
| condition = &v1alpha1.BindCondition{ | ||
| Type: v1alpha1.BindComplete, | ||
| Status: corev1.ConditionTrue, | ||
| Reason: fmt.Sprintf("BindSuccess; no Trigger to manage"), | ||
| Message: "Bind successful", | ||
| } | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| functionDNS := fmt.Sprintf("%s.%s.%s", route.Name, route.Namespace, domainSuffix) | ||
| glog.Infof("Found route DNS as '%q'", functionDNS) | ||
|
|
||
| es, err := c.eventSourcesLister.EventSources(namespace).Get(bind.Spec.Trigger.Service) | ||
| if err != nil { | ||
| if errors.IsNotFound(err) { | ||
| runtime.HandleError(fmt.Errorf("EventSource %q in namespace %q does not exist", bind.Spec.Trigger.Service, namespace)) | ||
| } else { | ||
| if condition, err = c.bindTrigger(namespace, bind); err != nil { | ||
| return err | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| et, err := c.eventTypesLister.EventTypes(namespace).Get(bind.Spec.Trigger.EventType) | ||
| if err != nil { | ||
| if errors.IsNotFound(err) { | ||
| runtime.HandleError(fmt.Errorf("EventType %q in namespace %q does not exist", bind.Spec.Trigger.Service, namespace)) | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| trigger, err := resolveTrigger(c.kubeclientset, namespace, bind.Spec.Trigger) | ||
| if err != nil { | ||
| glog.Warningf("Failed to process parameters: %s", err) | ||
| return err | ||
| } | ||
|
|
||
| if bind.Status.Conditions != nil { | ||
| if condition == nil { | ||
| glog.Infof("Already has status, skipping") | ||
| return nil | ||
| } | ||
|
|
||
| glog.Infof("Creating a subscription to %q : %q with Trigger %+v", es.Name, et.Name, trigger) | ||
| if val, ok := c.sources[es.Name]; ok { | ||
| r, err := val.Bind(trigger, functionDNS) | ||
| if err != nil { | ||
| glog.Warningf("BIND failed: %s", err) | ||
| msg := fmt.Sprintf("Bind failed with : %s", r) | ||
| bind.Status.SetCondition(&v1alpha1.BindCondition{ | ||
| Type: v1alpha1.BindFailed, | ||
| Status: corev1.ConditionTrue, | ||
| Reason: "BindFailed", | ||
| Message: msg, | ||
| }) | ||
| } else { | ||
| bind.Status.SetCondition(&v1alpha1.BindCondition{ | ||
| Type: v1alpha1.BindComplete, | ||
| Status: corev1.ConditionTrue, | ||
| Reason: fmt.Sprintf("BindSuccess: Hook: %s", r["ID"].(string)), | ||
| Message: "Bind successful", | ||
| }) | ||
| } | ||
| } | ||
| _, err = c.updateStatus(bind) | ||
| if err != nil { | ||
| bind.Status.SetCondition(condition) | ||
| if _, err = c.updateStatus(bind); err != nil { | ||
| glog.Warningf("Failed to update status: %s", err) | ||
| return err | ||
| } | ||
|
|
@@ -381,19 +347,69 @@ func (c *Controller) updateStatus(u *v1alpha1.Bind) (*v1alpha1.Bind, error) { | |
| return bindClient.Update(newu) | ||
| } | ||
|
|
||
| func resolveTrigger(kubeClient kubernetes.Interface, namespace string, trigger v1alpha1.EventTrigger) (sources.EventTrigger, error) { | ||
| r := sources.EventTrigger{ | ||
| Resource: trigger.Resource, | ||
| EventType: trigger.EventType, | ||
| Parameters: make(map[string]interface{}), | ||
| func (c *Controller) bindTrigger(namespace string, bind *v1alpha1.Bind) (*v1alpha1.BindCondition, error) { | ||
| es, err := c.eventSourcesLister.EventSources(namespace).Get(bind.Spec.Trigger.Service) | ||
| if err != nil { | ||
| if errors.IsNotFound(err) { | ||
| runtime.HandleError(fmt.Errorf("EventSource %q in namespace %q does not exist", bind.Spec.Trigger.Service, namespace)) | ||
| } | ||
| return nil, err | ||
| } | ||
|
|
||
| et, err := c.eventTypesLister.EventTypes(namespace).Get(bind.Spec.Trigger.EventType) | ||
| if err != nil { | ||
| if errors.IsNotFound(err) { | ||
| runtime.HandleError(fmt.Errorf("EventType %q in namespace %q does not exist", bind.Spec.Trigger.Service, namespace)) | ||
| } | ||
| return nil, err | ||
| } | ||
|
|
||
| parameters, err := resolveParameters(c.kubeclientset, namespace, bind.Spec.Trigger) | ||
| if err != nil { | ||
| glog.Warningf("Failed to process parameters: %s", err) | ||
| return nil, err | ||
| } | ||
|
|
||
| if bind.Status.Conditions != nil { | ||
| return nil, nil | ||
| } | ||
|
|
||
| glog.Infof("Creating a subscription to %q : %q with Trigger %+v", es.Name, et.Name, bind.Spec.Trigger) | ||
| eventSource, ok := c.sources[es.Name] | ||
| if !ok { | ||
| return nil, fmt.Errorf("Do not know how to deploy EventTrigger to source with name %s", es.Name) | ||
| } | ||
|
|
||
| r, err := eventSource.Bind(bind, parameters) | ||
| if err != nil { | ||
| glog.Warningf("BIND failed: %s", err) | ||
| msg := fmt.Sprintf("Bind failed with : %s", r) | ||
| return &v1alpha1.BindCondition{ | ||
| Type: v1alpha1.BindFailed, | ||
| Status: corev1.ConditionTrue, | ||
| Reason: "BindFailed", | ||
| Message: msg, | ||
| }, nil | ||
| } | ||
| return &v1alpha1.BindCondition{ | ||
| Type: v1alpha1.BindComplete, | ||
| Status: corev1.ConditionTrue, | ||
| // BUG(43) ID is a feature of the GitHub event source. If it is to become standardized, | ||
| // we should not be using a map[string]interface{} result. | ||
| Reason: fmt.Sprintf("BindSuccess: Hook: %s", r["ID"].(string)), | ||
| Message: "Bind successful", | ||
| }, nil | ||
| } | ||
|
|
||
| func resolveParameters(kubeClient kubernetes.Interface, namespace string, trigger v1alpha1.EventTrigger) (map[string]interface{}, error) { | ||
| r := make(map[string]interface{}) | ||
| if trigger.Parameters != nil && trigger.Parameters.Raw != nil && len(trigger.Parameters.Raw) > 0 { | ||
| p := make(map[string]interface{}) | ||
| if err := yaml.Unmarshal(trigger.Parameters.Raw, &p); err != nil { | ||
| return r, err | ||
| } | ||
| for k, v := range p { | ||
| r.Parameters[k] = v | ||
| r[k] = v | ||
| } | ||
| } | ||
| if trigger.ParametersFrom != nil { | ||
|
|
@@ -404,7 +420,7 @@ func resolveTrigger(kubeClient kubernetes.Interface, namespace string, trigger v | |
| return r, err | ||
| } | ||
| for k, v := range pfs { | ||
| r.Parameters[k] = v | ||
| r[k] = v | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -446,16 +462,20 @@ func unmarshalJSON(in []byte) (map[string]interface{}, error) { | |
| return parameters, nil | ||
| } | ||
|
|
||
| func getDomainSuffixFromElaConfig(cl kubernetes.Interface) (string, error) { | ||
| const name = "ela-config" | ||
| const namespace = "ela-system" | ||
| c, err := cl.CoreV1().ConfigMaps(namespace).Get(name, metav1.GetOptions{}) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| domainSuffix, ok := c.Data["domainSuffix"] | ||
| if !ok { | ||
| return "", fmt.Errorf("cannot find domainSuffix in %v: ConfigMap.Data is %#v", name, c.Data) | ||
| func (c *Controller) validateAction(namespace string, name string, actionType string) error { | ||
| switch actionType { | ||
| case action.ElafrosActionType: | ||
| parts := strings.Split(name, "/") | ||
| if len(parts) == 2 { | ||
| namespace, name = parts[0], parts[1] | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if name does not contain namespace? Seems like we should accept one or the other (namespace, name OR name=namespace/name) but not both here? |
||
| _, err := c.routesLister.Routes(namespace).Get(name) | ||
| return err | ||
| case action.LoggingActionType: | ||
| return nil | ||
| default: | ||
| // Should this be an errors.NewNotFound? How does the action struct | ||
| // fit into the schema.GroupResource idea? | ||
| return fmt.Errorf("Unknown Action.Processor %q", actionType) | ||
| } | ||
| return domainSuffix, nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious if you considered naming this "Handler". When it comes to naming, I tend to look for clues in the description, and in this case you wrote: