From 1fb5f4b2a62ecdb012e52960300e1f797865d20c Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Fri, 19 May 2023 16:14:28 -0700 Subject: [PATCH 01/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/async/webhook/factory.go | 15 +++++++ .../webhook/implementations/slack_webhook.go | 45 +++++++++++++++++++ .../implementations/webhook_metrics.go | 22 +++++++++ .../pkg/async/webhook/interfaces/webhook.go | 15 +++++++ .../pkg/manager/impl/execution_manager.go | 15 ++++++- flyteadmin/pkg/rpc/adminservice/base.go | 4 +- .../interfaces/application_configuration.go | 7 +++ 7 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 flyteadmin/pkg/async/webhook/factory.go create mode 100644 flyteadmin/pkg/async/webhook/implementations/slack_webhook.go create mode 100644 flyteadmin/pkg/async/webhook/implementations/webhook_metrics.go create mode 100644 flyteadmin/pkg/async/webhook/interfaces/webhook.go diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go new file mode 100644 index 0000000000..0eeaea0f08 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -0,0 +1,15 @@ +package webhook + +import ( + "context" + + "github.com/flyteorg/flyteadmin/pkg/async/webhook/implementations" + "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" + runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" + "github.com/flyteorg/flytestdlib/promutils" +) + +func NewWebhooks(ctx context.Context, config runtimeInterfaces.WebhookConfig, scope promutils.Scope) []interfaces.Webhook { + + return []interfaces.Webhook{implementations.NewSlackWebhook(config, scope)} +} diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go new file mode 100644 index 0000000000..7028343f82 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -0,0 +1,45 @@ +package implementations + +import ( + "bytes" + "context" + "fmt" + "net/http" + + "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" + runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" + "github.com/flyteorg/flytestdlib/promutils" + "github.com/golang/protobuf/proto" +) + +type SlackWebhook struct { + config runtimeInterfaces.WebhookConfig + systemMetrics webhookMetrics +} + +func (s *SlackWebhook) Post(ctx context.Context, notificationType string, msg proto.Message) error { + // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 + webhookURL := "https://hooks.slack.com/services/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" + data := []byte(fmt.Sprintf("{ hello: world }")) + request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) + if err != nil { + return err + } + request.Header.Add("Content-Type", "application/json") + client := &http.Client{} + resp, err := client.Do(request) + if err != nil { + return err + } + defer resp.Body.Close() + // TODO: Check response status code + return nil +} + +func NewSlackWebhook(config runtimeInterfaces.WebhookConfig, scope promutils.Scope) interfaces.Webhook { + + return &SlackWebhook{ + config: config, + systemMetrics: newWebhookMetrics(scope.NewSubScope("slack_webhook")), + } +} diff --git a/flyteadmin/pkg/async/webhook/implementations/webhook_metrics.go b/flyteadmin/pkg/async/webhook/implementations/webhook_metrics.go new file mode 100644 index 0000000000..72be315eff --- /dev/null +++ b/flyteadmin/pkg/async/webhook/implementations/webhook_metrics.go @@ -0,0 +1,22 @@ +package implementations + +import ( + "github.com/flyteorg/flytestdlib/promutils" + "github.com/prometheus/client_golang/prometheus" +) + +type webhookMetrics struct { + Scope promutils.Scope + SendSuccess prometheus.Counter + SendError prometheus.Counter + SendTotal prometheus.Counter +} + +func newWebhookMetrics(scope promutils.Scope) webhookMetrics { + return webhookMetrics{ + Scope: scope, + SendSuccess: scope.MustNewCounter("send_success", "Number of successful emails sent via Emailer."), + SendError: scope.MustNewCounter("send_error", "Number of errors when sending email via Emailer"), + SendTotal: scope.MustNewCounter("send_total", "Total number of emails attempted to be sent"), + } +} diff --git a/flyteadmin/pkg/async/webhook/interfaces/webhook.go b/flyteadmin/pkg/async/webhook/interfaces/webhook.go new file mode 100644 index 0000000000..ad4f9e95ba --- /dev/null +++ b/flyteadmin/pkg/async/webhook/interfaces/webhook.go @@ -0,0 +1,15 @@ +package interfaces + +import ( + "context" + + "github.com/golang/protobuf/proto" +) + +//go:generate mockery -name=Webhook -output=../mocks -case=underscore + +// Webhook Defines the interface for Publishing execution event to other services (Slack). +type Webhook interface { + // Post The notificationType is inferred from the Notification object in the Execution Spec. + Post(ctx context.Context, notificationType string, msg proto.Message) error +} diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index bbcc45a4a1..4facb7c3f5 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -32,6 +32,7 @@ import ( eventWriter "github.com/flyteorg/flyteadmin/pkg/async/events/interfaces" "github.com/flyteorg/flyteadmin/pkg/async/notifications" notificationInterfaces "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" + webhookInterface "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" "github.com/flyteorg/flyteadmin/pkg/errors" "github.com/flyteorg/flyteadmin/pkg/manager/impl/executions" "github.com/flyteorg/flyteadmin/pkg/manager/impl/util" @@ -90,6 +91,7 @@ type ExecutionManager struct { systemMetrics executionSystemMetrics userMetrics executionUserMetrics notificationClient notificationInterfaces.Publisher + webhooks []webhookInterface.Webhook urlData dataInterfaces.RemoteURLInterface workflowManager interfaces.WorkflowInterface namedEntityManager interfaces.NamedEntityInterface @@ -1294,6 +1296,15 @@ func (m *ExecutionManager) CreateWorkflowEvent(ctx context.Context, request admi logger.Infof(ctx, "error publishing event [%+v] with err: [%v]", request.RequestId, err) } + go func() { + for _, client := range m.webhooks { + if err := client.Post(ctx, proto.MessageName(&request), &request); err != nil { + m.systemMetrics.PublishEventError.Inc() + logger.Infof(ctx, "error publishing webhook event [%+v] with err: [%v]", request.RequestId, err) + } + } + }() + go func() { if err := m.cloudEventPublisher.Publish(ctx, proto.MessageName(&request), &request); err != nil { m.systemMetrics.PublishEventError.Inc() @@ -1625,7 +1636,8 @@ func newExecutionSystemMetrics(scope promutils.Scope) executionSystemMetrics { func NewExecutionManager(db repositoryInterfaces.Repository, pluginRegistry *plugins.Registry, config runtimeInterfaces.Configuration, storageClient *storage.DataStore, systemScope promutils.Scope, userScope promutils.Scope, - publisher notificationInterfaces.Publisher, urlData dataInterfaces.RemoteURLInterface, + publisher notificationInterfaces.Publisher, webhooks []webhookInterface.Webhook, + urlData dataInterfaces.RemoteURLInterface, workflowManager interfaces.WorkflowInterface, namedEntityManager interfaces.NamedEntityInterface, eventPublisher notificationInterfaces.Publisher, cloudEventPublisher cloudeventInterfaces.Publisher, eventWriter eventWriter.WorkflowExecutionEventWriter) interfaces.ExecutionInterface { @@ -1652,6 +1664,7 @@ func NewExecutionManager(db repositoryInterfaces.Repository, pluginRegistry *plu systemMetrics: systemMetrics, userMetrics: userMetrics, notificationClient: publisher, + webhooks: webhooks, urlData: urlData, workflowManager: workflowManager, namedEntityManager: namedEntityManager, diff --git a/flyteadmin/pkg/rpc/adminservice/base.go b/flyteadmin/pkg/rpc/adminservice/base.go index 77c78f480e..2ac6d91b14 100644 --- a/flyteadmin/pkg/rpc/adminservice/base.go +++ b/flyteadmin/pkg/rpc/adminservice/base.go @@ -20,6 +20,7 @@ import ( "github.com/flyteorg/flyteadmin/pkg/async/notifications" "github.com/flyteorg/flyteadmin/pkg/async/schedule" + "github.com/flyteorg/flyteadmin/pkg/async/webhook" "github.com/flyteorg/flyteadmin/pkg/data" executionCluster "github.com/flyteorg/flyteadmin/pkg/executioncluster/impl" manager "github.com/flyteorg/flyteadmin/pkg/manager/impl" @@ -102,6 +103,7 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi processor := notifications.NewNotificationsProcessor(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) eventPublisher := notifications.NewEventsPublisher(*configuration.ApplicationConfiguration().GetExternalEventsConfig(), adminScope) cloudEventPublisher := cloudevent.NewCloudEventsPublisher(ctx, *configuration.ApplicationConfiguration().GetCloudEventsConfig(), adminScope) + webhooks := webhook.NewWebhooks(ctx, *configuration.ApplicationConfiguration().GetWebhookConfig(), adminScope) go func() { logger.Info(ctx, "Started processing notifications.") processor.StartProcessing() @@ -143,7 +145,7 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi executionManager := manager.NewExecutionManager(repo, pluginRegistry, configuration, dataStorageClient, adminScope.NewSubScope("execution_manager"), adminScope.NewSubScope("user_execution_metrics"), - publisher, urlData, workflowManager, namedEntityManager, eventPublisher, cloudEventPublisher, executionEventWriter) + publisher, webhooks, urlData, workflowManager, namedEntityManager, eventPublisher, cloudEventPublisher, executionEventWriter) versionManager := manager.NewVersionManager() scheduledWorkflowExecutor := workflowScheduler.GetWorkflowExecutor(executionManager, launchPlanManager) diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index cf9bf2e9ed..dfebcef7e6 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -552,6 +552,12 @@ type NotificationsConfig struct { ReconnectDelaySeconds int `json:"reconnectDelaySeconds"` } +// WebhookConfig defines the configuration for the webhook service. +type WebhookConfig struct { + Type string `json:"type"` + URL string `json:"url"` +} + // Domains are always globally set in the application config, whereas individual projects can be individually registered. type Domain struct { // Unique identifier for a domain. @@ -572,4 +578,5 @@ type ApplicationConfiguration interface { GetDomainsConfig() *DomainsConfig GetExternalEventsConfig() *ExternalEventsConfig GetCloudEventsConfig() *CloudEventsConfig + GetWebhookConfig() *WebhookConfig } From 0d4bfa65b51dcfa8531174d69f884ad6d44e4a5d Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Fri, 19 May 2023 16:21:23 -0700 Subject: [PATCH 02/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/runtime/application_config_provider.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/flyteadmin/pkg/runtime/application_config_provider.go b/flyteadmin/pkg/runtime/application_config_provider.go index 3b8b0a2707..fe7f0c86ef 100644 --- a/flyteadmin/pkg/runtime/application_config_provider.go +++ b/flyteadmin/pkg/runtime/application_config_provider.go @@ -84,6 +84,10 @@ var cloudEventsConfig = config.MustRegisterSection(cloudEvents, &interfaces.Clou Type: common.Local, }) +var webhooksConfig = config.MustRegisterSection("webhooks", &interfaces.WebhookConfig{ + Type: "slack", +}) + // Implementation of an interfaces.ApplicationConfiguration type ApplicationConfigurationProvider struct{} @@ -119,6 +123,10 @@ func (p *ApplicationConfigurationProvider) GetCloudEventsConfig() *interfaces.Cl return cloudEventsConfig.GetConfig().(*interfaces.CloudEventsConfig) } +func (p *ApplicationConfigurationProvider) GetWebhookConfig() *interfaces.WebhookConfig { + return webhooksConfig.GetConfig().(*interfaces.WebhookConfig) +} + func NewApplicationConfigurationProvider() interfaces.ApplicationConfiguration { return &ApplicationConfigurationProvider{} } From 513fbad7121a6e3b53284046af48e31cf63d48f7 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Mon, 22 May 2023 15:20:13 -0700 Subject: [PATCH 03/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/async/webhook/factory.go | 1 - .../webhook/implementations/slack_webhook.go | 16 ++++++++++++++-- .../implementations/slack_webhook_test.go | 15 +++++++++++++++ flyteadmin/pkg/rpc/adminservice/base.go | 3 ++- .../pkg/runtime/application_config_provider.go | 3 ++- 5 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index 0eeaea0f08..3fe524cb8c 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -10,6 +10,5 @@ import ( ) func NewWebhooks(ctx context.Context, config runtimeInterfaces.WebhookConfig, scope promutils.Scope) []interfaces.Webhook { - return []interfaces.Webhook{implementations.NewSlackWebhook(config, scope)} } diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index 7028343f82..a2316891ae 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "fmt" + "github.com/flyteorg/flytestdlib/logger" + "io/ioutil" "net/http" "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" @@ -19,20 +21,30 @@ type SlackWebhook struct { func (s *SlackWebhook) Post(ctx context.Context, notificationType string, msg proto.Message) error { // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 + logger.Info(ctx, "Posting to Slack") webhookURL := "https://hooks.slack.com/services/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" - data := []byte(fmt.Sprintf("{ hello: world }")) + data := []byte(fmt.Sprintf("{'text':'Hello, flyte!'}")) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) if err != nil { + logger.Errorf(ctx, "Failed to create request to Slack webhook with error: %v", err) return err } request.Header.Add("Content-Type", "application/json") client := &http.Client{} resp, err := client.Do(request) if err != nil { + logger.Errorf(ctx, "Failed to post to Slack webhook with error: %v", err) return err } defer resp.Body.Close() - // TODO: Check response status code + if resp.StatusCode >= 400 { + respBody, _ := ioutil.ReadAll(resp.Body) + return fmt.Errorf("received an error response (%d): %s", + resp.StatusCode, + string(respBody), + ) + } + return nil } diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go new file mode 100644 index 0000000000..d2b6bf5788 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go @@ -0,0 +1,15 @@ +package implementations + +import ( + "context" + runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" + "github.com/flyteorg/flytestdlib/promutils" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestSlackWebhook(t *testing.T) { + webhook := NewSlackWebhook(runtimeInterfaces.WebhookConfig{}, promutils.NewTestScope()) + err := webhook.Post(context.Background(), "workflowExecution", nil) + assert.Nil(t, err) +} diff --git a/flyteadmin/pkg/rpc/adminservice/base.go b/flyteadmin/pkg/rpc/adminservice/base.go index 2ac6d91b14..028bfeb74a 100644 --- a/flyteadmin/pkg/rpc/adminservice/base.go +++ b/flyteadmin/pkg/rpc/adminservice/base.go @@ -3,6 +3,7 @@ package adminservice import ( "context" "fmt" + "github.com/flyteorg/flyteadmin/pkg/async/notifications/implementations" "runtime/debug" "github.com/flyteorg/flyteadmin/plugins" @@ -100,7 +101,7 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi pluginRegistry.RegisterDefault(plugins.PluginIDWorkflowExecutor, workflowExecutor) publisher := notifications.NewNotificationsPublisher(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) - processor := notifications.NewNotificationsProcessor(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) + processor := implementations.NewNoopProcess() eventPublisher := notifications.NewEventsPublisher(*configuration.ApplicationConfiguration().GetExternalEventsConfig(), adminScope) cloudEventPublisher := cloudevent.NewCloudEventsPublisher(ctx, *configuration.ApplicationConfiguration().GetCloudEventsConfig(), adminScope) webhooks := webhook.NewWebhooks(ctx, *configuration.ApplicationConfiguration().GetWebhookConfig(), adminScope) diff --git a/flyteadmin/pkg/runtime/application_config_provider.go b/flyteadmin/pkg/runtime/application_config_provider.go index fe7f0c86ef..1382db7d4e 100644 --- a/flyteadmin/pkg/runtime/application_config_provider.go +++ b/flyteadmin/pkg/runtime/application_config_provider.go @@ -15,6 +15,7 @@ const notifications = "notifications" const domains = "domains" const externalEvents = "externalEvents" const cloudEvents = "cloudEvents" +const webhook = "webhook" const metricPort = 10254 const KB = 1024 @@ -84,7 +85,7 @@ var cloudEventsConfig = config.MustRegisterSection(cloudEvents, &interfaces.Clou Type: common.Local, }) -var webhooksConfig = config.MustRegisterSection("webhooks", &interfaces.WebhookConfig{ +var webhooksConfig = config.MustRegisterSection(webhook, &interfaces.WebhookConfig{ Type: "slack", }) From d334e492de82ee537a29ebd130c410d2664cc8fe Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Mon, 22 May 2023 17:26:34 -0700 Subject: [PATCH 04/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/async/notifications/factory.go | 10 +++++++++- .../implementations/aws_processor.go | 18 +++++++++++++++--- flyteadmin/pkg/async/webhook/factory.go | 5 ++--- .../webhook/implementations/slack_webhook.go | 11 +++++------ .../implementations/slack_webhook_test.go | 4 ++-- .../implementations/webhook_processer.go | 1 + .../pkg/async/webhook/interfaces/webhook.go | 4 +--- .../pkg/manager/impl/execution_manager.go | 16 ++++++++-------- flyteadmin/pkg/rpc/adminservice/base.go | 7 +++++-- .../pkg/runtime/application_config_provider.go | 6 +++--- .../interfaces/application_configuration.go | 15 ++++++++++----- 11 files changed, 61 insertions(+), 36 deletions(-) create mode 100644 flyteadmin/pkg/async/webhook/implementations/webhook_processer.go diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index 836c1ff2fd..110bbb29de 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -9,6 +9,8 @@ import ( "github.com/flyteorg/flyteadmin/pkg/async/notifications/implementations" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" + webhookImplementations "github.com/flyteorg/flyteadmin/pkg/async/webhook/implementations" + webhookInterface "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flytestdlib/logger" @@ -74,11 +76,16 @@ func GetEmailer(config runtimeInterfaces.NotificationsConfig, scope promutils.Sc } } +func GetWebhook(config runtimeInterfaces.WebhooksConfig, scope promutils.Scope) webhookInterface.Webhook { + return webhookImplementations.NewSlackWebhook(config, scope) +} + func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, scope promutils.Scope) interfaces.Processor { reconnectAttempts := config.ReconnectAttempts reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second var sub pubsub.Subscriber var emailer interfaces.Emailer + var webhook webhookInterface.Webhook switch config.Type { case common.AWS: sqsConfig := gizmoAWS.SQSConfig{ @@ -103,7 +110,8 @@ func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, sco panic(err) } emailer = GetEmailer(config, scope) - return implementations.NewProcessor(sub, emailer, scope) + webhook = GetWebhook(runtimeInterfaces.WebhooksConfig{}, scope) + return implementations.NewProcessor(sub, emailer, webhook, scope) case common.GCP: projectID := config.GCPConfig.ProjectID subscription := config.NotificationsProcessorConfig.QueueName diff --git a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go index 2cbf6d971a..b7936320ca 100644 --- a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go +++ b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go @@ -9,6 +9,7 @@ import ( "github.com/NYTimes/gizmo/pubsub" "github.com/flyteorg/flyteadmin/pkg/async" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" + webhookInterfaces "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flytestdlib/logger" "github.com/flyteorg/flytestdlib/promutils" @@ -19,6 +20,7 @@ import ( type Processor struct { sub pubsub.Subscriber email interfaces.Emailer + webhook webhookInterfaces.Webhook systemMetrics processorSystemMetrics } @@ -89,9 +91,18 @@ func (p *Processor) run() error { continue } - if err = p.email.SendEmail(context.Background(), emailMessage); err != nil { + //if err = p.email.SendEmail(context.Background(), emailMessage); err != nil { + // p.systemMetrics.MessageProcessorError.Inc() + // logger.Errorf(context.Background(), "Error sending an email message for message [%s] with emailM with err: %v", emailMessage.String(), err) + //} else { + // p.systemMetrics.MessageSuccess.Inc() + //} + + logger.Info(context.Background(), "Processor is sending message to webhook endpoint") + // Send message to webhook + if err = p.webhook.Post(context.Background(), emailMessage.Body); err != nil { p.systemMetrics.MessageProcessorError.Inc() - logger.Errorf(context.Background(), "Error sending an email message for message [%s] with emailM with err: %v", emailMessage.String(), err) + logger.Errorf(context.Background(), "Error sending an message [%s] to webhook endpoint with err: %v", emailMessage.Body, err) } else { p.systemMetrics.MessageSuccess.Inc() } @@ -129,10 +140,11 @@ func (p *Processor) StopProcessing() error { return err } -func NewProcessor(sub pubsub.Subscriber, emailer interfaces.Emailer, scope promutils.Scope) interfaces.Processor { +func NewProcessor(sub pubsub.Subscriber, emailer interfaces.Emailer, webhook webhookInterfaces.Webhook, scope promutils.Scope) interfaces.Processor { return &Processor{ sub: sub, email: emailer, + webhook: webhook, systemMetrics: newProcessorSystemMetrics(scope.NewSubScope("processor")), } } diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index 3fe524cb8c..e52acdbdcb 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -3,12 +3,11 @@ package webhook import ( "context" - "github.com/flyteorg/flyteadmin/pkg/async/webhook/implementations" "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flytestdlib/promutils" ) -func NewWebhooks(ctx context.Context, config runtimeInterfaces.WebhookConfig, scope promutils.Scope) []interfaces.Webhook { - return []interfaces.Webhook{implementations.NewSlackWebhook(config, scope)} +func NewWebhooks(ctx context.Context, config runtimeInterfaces.WebhooksConfig, scope promutils.Scope) []interfaces.Webhook { + return []interfaces.Webhook{} } diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index a2316891ae..89bb40ed37 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -11,19 +11,18 @@ import ( "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flytestdlib/promutils" - "github.com/golang/protobuf/proto" ) type SlackWebhook struct { - config runtimeInterfaces.WebhookConfig + config runtimeInterfaces.WebhooksConfig systemMetrics webhookMetrics } -func (s *SlackWebhook) Post(ctx context.Context, notificationType string, msg proto.Message) error { +func (s *SlackWebhook) Post(ctx context.Context, message string) error { // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 - logger.Info(ctx, "Posting to Slack") + logger.Infof(ctx, "Posting to Slack with message: [%v]", message) webhookURL := "https://hooks.slack.com/services/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" - data := []byte(fmt.Sprintf("{'text':'Hello, flyte!'}")) + data := []byte(fmt.Sprintf("{'text': %s}", message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) if err != nil { logger.Errorf(ctx, "Failed to create request to Slack webhook with error: %v", err) @@ -48,7 +47,7 @@ func (s *SlackWebhook) Post(ctx context.Context, notificationType string, msg pr return nil } -func NewSlackWebhook(config runtimeInterfaces.WebhookConfig, scope promutils.Scope) interfaces.Webhook { +func NewSlackWebhook(config runtimeInterfaces.WebhooksConfig, scope promutils.Scope) interfaces.Webhook { return &SlackWebhook{ config: config, diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go index d2b6bf5788..929dea12e9 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go @@ -9,7 +9,7 @@ import ( ) func TestSlackWebhook(t *testing.T) { - webhook := NewSlackWebhook(runtimeInterfaces.WebhookConfig{}, promutils.NewTestScope()) - err := webhook.Post(context.Background(), "workflowExecution", nil) + webhook := NewSlackWebhook(runtimeInterfaces.WebhooksConfig{}, promutils.NewTestScope()) + err := webhook.Post(context.Background(), "message") assert.Nil(t, err) } diff --git a/flyteadmin/pkg/async/webhook/implementations/webhook_processer.go b/flyteadmin/pkg/async/webhook/implementations/webhook_processer.go new file mode 100644 index 0000000000..89926c85e1 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/implementations/webhook_processer.go @@ -0,0 +1 @@ +package implementations diff --git a/flyteadmin/pkg/async/webhook/interfaces/webhook.go b/flyteadmin/pkg/async/webhook/interfaces/webhook.go index ad4f9e95ba..984f916d09 100644 --- a/flyteadmin/pkg/async/webhook/interfaces/webhook.go +++ b/flyteadmin/pkg/async/webhook/interfaces/webhook.go @@ -2,8 +2,6 @@ package interfaces import ( "context" - - "github.com/golang/protobuf/proto" ) //go:generate mockery -name=Webhook -output=../mocks -case=underscore @@ -11,5 +9,5 @@ import ( // Webhook Defines the interface for Publishing execution event to other services (Slack). type Webhook interface { // Post The notificationType is inferred from the Notification object in the Execution Spec. - Post(ctx context.Context, notificationType string, msg proto.Message) error + Post(ctx context.Context, message string) error } diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index 4facb7c3f5..8d64d21149 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -1296,14 +1296,14 @@ func (m *ExecutionManager) CreateWorkflowEvent(ctx context.Context, request admi logger.Infof(ctx, "error publishing event [%+v] with err: [%v]", request.RequestId, err) } - go func() { - for _, client := range m.webhooks { - if err := client.Post(ctx, proto.MessageName(&request), &request); err != nil { - m.systemMetrics.PublishEventError.Inc() - logger.Infof(ctx, "error publishing webhook event [%+v] with err: [%v]", request.RequestId, err) - } - } - }() + //go func() { + // for _, client := range m.webhooks { + // if err := client.Post(ctx, proto.MessageName(&request), &request); err != nil { + // m.systemMetrics.PublishEventError.Inc() + // logger.Infof(ctx, "error publishing webhook event [%+v] with err: [%v]", request.RequestId, err) + // } + // } + //}() go func() { if err := m.cloudEventPublisher.Publish(ctx, proto.MessageName(&request), &request); err != nil { diff --git a/flyteadmin/pkg/rpc/adminservice/base.go b/flyteadmin/pkg/rpc/adminservice/base.go index 028bfeb74a..898a0ff63f 100644 --- a/flyteadmin/pkg/rpc/adminservice/base.go +++ b/flyteadmin/pkg/rpc/adminservice/base.go @@ -3,7 +3,7 @@ package adminservice import ( "context" "fmt" - "github.com/flyteorg/flyteadmin/pkg/async/notifications/implementations" + "reflect" "runtime/debug" "github.com/flyteorg/flyteadmin/plugins" @@ -100,8 +100,11 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi logger.Info(ctx, "Successfully created a workflow executor engine") pluginRegistry.RegisterDefault(plugins.PluginIDWorkflowExecutor, workflowExecutor) + logger.Infof(ctx, "notifier config: %v", *configuration.ApplicationConfiguration().GetNotificationsConfig()) publisher := notifications.NewNotificationsPublisher(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) - processor := implementations.NewNoopProcess() + logger.Infof(ctx, "publisher: %v", reflect.TypeOf(publisher)) + processor := notifications.NewNotificationsProcessor(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) + logger.Infof(ctx, "processor: %v", reflect.TypeOf(processor)) eventPublisher := notifications.NewEventsPublisher(*configuration.ApplicationConfiguration().GetExternalEventsConfig(), adminScope) cloudEventPublisher := cloudevent.NewCloudEventsPublisher(ctx, *configuration.ApplicationConfiguration().GetCloudEventsConfig(), adminScope) webhooks := webhook.NewWebhooks(ctx, *configuration.ApplicationConfiguration().GetWebhookConfig(), adminScope) diff --git a/flyteadmin/pkg/runtime/application_config_provider.go b/flyteadmin/pkg/runtime/application_config_provider.go index 1382db7d4e..c0e92a008a 100644 --- a/flyteadmin/pkg/runtime/application_config_provider.go +++ b/flyteadmin/pkg/runtime/application_config_provider.go @@ -85,7 +85,7 @@ var cloudEventsConfig = config.MustRegisterSection(cloudEvents, &interfaces.Clou Type: common.Local, }) -var webhooksConfig = config.MustRegisterSection(webhook, &interfaces.WebhookConfig{ +var webhooksConfig = config.MustRegisterSection(webhook, &interfaces.WebhooksConfig{ Type: "slack", }) @@ -124,8 +124,8 @@ func (p *ApplicationConfigurationProvider) GetCloudEventsConfig() *interfaces.Cl return cloudEventsConfig.GetConfig().(*interfaces.CloudEventsConfig) } -func (p *ApplicationConfigurationProvider) GetWebhookConfig() *interfaces.WebhookConfig { - return webhooksConfig.GetConfig().(*interfaces.WebhookConfig) +func (p *ApplicationConfigurationProvider) GetWebhookConfig() *interfaces.WebhooksConfig { + return webhooksConfig.GetConfig().(*interfaces.WebhooksConfig) } func NewApplicationConfigurationProvider() interfaces.ApplicationConfiguration { diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index dfebcef7e6..28071e9894 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -552,10 +552,15 @@ type NotificationsConfig struct { ReconnectDelaySeconds int `json:"reconnectDelaySeconds"` } -// WebhookConfig defines the configuration for the webhook service. -type WebhookConfig struct { - Type string `json:"type"` - URL string `json:"url"` +// WebhooksConfig defines the configuration for the webhook service. +type WebhooksConfig struct { + // Defines the cloud provider that backs the scheduler. In the absence of a specification the no-op, 'local' + // scheme is used. + Type string `json:"type"` + Region string `json:"region"` + AWSConfig AWSConfig `json:"aws"` + GCPConfig GCPConfig `json:"gcp"` + URL string `json:"url"` } // Domains are always globally set in the application config, whereas individual projects can be individually registered. @@ -578,5 +583,5 @@ type ApplicationConfiguration interface { GetDomainsConfig() *DomainsConfig GetExternalEventsConfig() *ExternalEventsConfig GetCloudEventsConfig() *CloudEventsConfig - GetWebhookConfig() *WebhookConfig + GetWebhookConfig() *WebhooksConfig } From d5a48f73417f1ff657e9df21ed17058376f0001b Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 23 May 2023 12:40:45 -0700 Subject: [PATCH 05/35] add log Signed-off-by: Kevin Su --- flyteadmin/pkg/async/notifications/factory.go | 1 + 1 file changed, 1 insertion(+) diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index 110bbb29de..6d7f48517b 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -155,6 +155,7 @@ func NewNotificationsPublisher(config runtimeInterfaces.NotificationsConfig, sco var err error err = async.Retry(reconnectAttempts, reconnectDelay, func() error { publisher, err = gizmoAWS.NewPublisher(snsConfig) + logger.Errorf(context.Background(), "Failed to initialize aws publisher with config [%+v] and err: %v", snsConfig, err) return err }) From 126f77759ef1fa00ccd6515cdef73aa7fefd20d6 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 23 May 2023 13:12:52 -0700 Subject: [PATCH 06/35] update webhook url Signed-off-by: Kevin Su --- flyteadmin/pkg/async/notifications/factory.go | 1 + flyteadmin/pkg/async/webhook/implementations/slack_webhook.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index 6d7f48517b..0da9c41ba8 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -163,6 +163,7 @@ func NewNotificationsPublisher(config runtimeInterfaces.NotificationsConfig, sco if err != nil { panic(err) } + logger.Infof(context.Background(), "Using aws publisher implementation for config type [%s]", config.Type) return implementations.NewPublisher(publisher, scope) case common.GCP: pubsubConfig := gizmoGCP.Config{ diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index 89bb40ed37..ac1054e484 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -21,7 +21,7 @@ type SlackWebhook struct { func (s *SlackWebhook) Post(ctx context.Context, message string) error { // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 logger.Infof(ctx, "Posting to Slack with message: [%v]", message) - webhookURL := "https://hooks.slack.com/services/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" + webhookURL := "https://hooks.slack.com/services/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" data := []byte(fmt.Sprintf("{'text': %s}", message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) if err != nil { From cfc33fce1678318b0878f6110246194d01e6edb1 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 23 May 2023 15:01:49 -0700 Subject: [PATCH 07/35] wip Signed-off-by: Kevin Su --- .../pkg/async/notifications/factory_test.go | 46 +++++++++++++++++++ .../webhook/implementations/slack_webhook.go | 2 +- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/flyteadmin/pkg/async/notifications/factory_test.go b/flyteadmin/pkg/async/notifications/factory_test.go index 280f383f2c..0c4bcc2da2 100644 --- a/flyteadmin/pkg/async/notifications/factory_test.go +++ b/flyteadmin/pkg/async/notifications/factory_test.go @@ -1,6 +1,12 @@ package notifications import ( + "github.com/NYTimes/gizmo/pubsub" + gizmoAWS "github.com/NYTimes/gizmo/pubsub/aws" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/event" + "golang.org/x/net/context" "testing" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" @@ -23,3 +29,43 @@ func TestGetEmailer(t *testing.T) { // shouldn't reach here t.Errorf("did not panic") } + +func TestAWSNotificationsPublisher(t *testing.T) { + cfg := runtimeInterfaces.NotificationsConfig{ + Type: "aws", + Region: "us-west-2", + NotificationsPublisherConfig: runtimeInterfaces.NotificationsPublisherConfig{ + TopicName: "arn:aws:sns:us-west-2:590375264460:webhook", + }, + } + var publisher pubsub.Publisher + var err error + snsConfig := gizmoAWS.SNSConfig{ + Topic: cfg.NotificationsPublisherConfig.TopicName, + } + if cfg.AWSConfig.Region != "" { + snsConfig.Region = cfg.AWSConfig.Region + } else { + snsConfig.Region = cfg.Region + } + publisher, err = gizmoAWS.NewPublisher(snsConfig) + assert.Nil(t, err) + + var executionID = core.WorkflowExecutionIdentifier{ + Project: "project", + Domain: "domain", + Name: "name", + } + + var workflowRequest = &admin.WorkflowExecutionEventRequest{ + Event: &event.WorkflowExecutionEvent{ + Phase: core.WorkflowExecution_SUCCEEDED, + OutputResult: &event.WorkflowExecutionEvent_OutputUri{ + OutputUri: "somestring", + }, + ExecutionId: &executionID, + }, + } + err = publisher.Publish(context.Background(), "test", workflowRequest) + assert.Nil(t, err) +} diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index ac1054e484..1cdd759f0b 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -21,7 +21,7 @@ type SlackWebhook struct { func (s *SlackWebhook) Post(ctx context.Context, message string) error { // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 logger.Infof(ctx, "Posting to Slack with message: [%v]", message) - webhookURL := "https://hooks.slack.com/services/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" + webhookURL := "https://hooks.slack.com/services/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" data := []byte(fmt.Sprintf("{'text': %s}", message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) if err != nil { From c5442ddef57ff122837548371b4122572ff9ca98 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 23 May 2023 15:10:08 -0700 Subject: [PATCH 08/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/async/webhook/implementations/slack_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index 1cdd759f0b..7c8ada99b4 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -21,7 +21,7 @@ type SlackWebhook struct { func (s *SlackWebhook) Post(ctx context.Context, message string) error { // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 logger.Infof(ctx, "Posting to Slack with message: [%v]", message) - webhookURL := "https://hooks.slack.com/services/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" + webhookURL := "https://hooks.slack.com/services/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" data := []byte(fmt.Sprintf("{'text': %s}", message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) if err != nil { From c75707a8b7edb4705556347a9b25c8959fd15d26 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 23 May 2023 16:23:05 -0700 Subject: [PATCH 09/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/async/notifications/factory.go | 10 +- .../pkg/async/notifications/factory_test.go | 3 +- .../implementations/aws_processor.go | 14 +- flyteadmin/pkg/async/webhook/factory.go | 53 +++++++- .../webhook/implementations/processer.go | 123 ++++++++++++++++++ .../implementations/processor_metrics.go | 32 +++++ .../webhook/implementations/slack_webhook.go | 5 +- .../implementations/slack_webhook_test.go | 3 +- .../implementations/webhook_processer.go | 1 - .../pkg/manager/impl/execution_manager.go | 5 +- flyteadmin/pkg/rpc/adminservice/base.go | 18 ++- .../interfaces/application_configuration.go | 18 ++- 12 files changed, 237 insertions(+), 48 deletions(-) create mode 100644 flyteadmin/pkg/async/webhook/implementations/processer.go create mode 100644 flyteadmin/pkg/async/webhook/implementations/processor_metrics.go delete mode 100644 flyteadmin/pkg/async/webhook/implementations/webhook_processer.go diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index 0da9c41ba8..a971d19e7e 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -9,8 +9,6 @@ import ( "github.com/flyteorg/flyteadmin/pkg/async/notifications/implementations" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" - webhookImplementations "github.com/flyteorg/flyteadmin/pkg/async/webhook/implementations" - webhookInterface "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flytestdlib/logger" @@ -76,16 +74,11 @@ func GetEmailer(config runtimeInterfaces.NotificationsConfig, scope promutils.Sc } } -func GetWebhook(config runtimeInterfaces.WebhooksConfig, scope promutils.Scope) webhookInterface.Webhook { - return webhookImplementations.NewSlackWebhook(config, scope) -} - func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, scope promutils.Scope) interfaces.Processor { reconnectAttempts := config.ReconnectAttempts reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second var sub pubsub.Subscriber var emailer interfaces.Emailer - var webhook webhookInterface.Webhook switch config.Type { case common.AWS: sqsConfig := gizmoAWS.SQSConfig{ @@ -110,8 +103,7 @@ func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, sco panic(err) } emailer = GetEmailer(config, scope) - webhook = GetWebhook(runtimeInterfaces.WebhooksConfig{}, scope) - return implementations.NewProcessor(sub, emailer, webhook, scope) + return implementations.NewProcessor(sub, emailer, scope) case common.GCP: projectID := config.GCPConfig.ProjectID subscription := config.NotificationsProcessorConfig.QueueName diff --git a/flyteadmin/pkg/async/notifications/factory_test.go b/flyteadmin/pkg/async/notifications/factory_test.go index 0c4bcc2da2..dd3433c0de 100644 --- a/flyteadmin/pkg/async/notifications/factory_test.go +++ b/flyteadmin/pkg/async/notifications/factory_test.go @@ -1,13 +1,14 @@ package notifications import ( + "testing" + "github.com/NYTimes/gizmo/pubsub" gizmoAWS "github.com/NYTimes/gizmo/pubsub/aws" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/event" "golang.org/x/net/context" - "testing" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flytestdlib/promutils" diff --git a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go index b7936320ca..8701c8ef2f 100644 --- a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go +++ b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go @@ -9,7 +9,6 @@ import ( "github.com/NYTimes/gizmo/pubsub" "github.com/flyteorg/flyteadmin/pkg/async" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" - webhookInterfaces "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flytestdlib/logger" "github.com/flyteorg/flytestdlib/promutils" @@ -20,7 +19,6 @@ import ( type Processor struct { sub pubsub.Subscriber email interfaces.Emailer - webhook webhookInterfaces.Webhook systemMetrics processorSystemMetrics } @@ -98,15 +96,6 @@ func (p *Processor) run() error { // p.systemMetrics.MessageSuccess.Inc() //} - logger.Info(context.Background(), "Processor is sending message to webhook endpoint") - // Send message to webhook - if err = p.webhook.Post(context.Background(), emailMessage.Body); err != nil { - p.systemMetrics.MessageProcessorError.Inc() - logger.Errorf(context.Background(), "Error sending an message [%s] to webhook endpoint with err: %v", emailMessage.Body, err) - } else { - p.systemMetrics.MessageSuccess.Inc() - } - p.markMessageDone(msg) } @@ -140,11 +129,10 @@ func (p *Processor) StopProcessing() error { return err } -func NewProcessor(sub pubsub.Subscriber, emailer interfaces.Emailer, webhook webhookInterfaces.Webhook, scope promutils.Scope) interfaces.Processor { +func NewProcessor(sub pubsub.Subscriber, emailer interfaces.Emailer, scope promutils.Scope) interfaces.Processor { return &Processor{ sub: sub, email: emailer, - webhook: webhook, systemMetrics: newProcessorSystemMetrics(scope.NewSubScope("processor")), } } diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index e52acdbdcb..fd1f55551a 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -2,12 +2,59 @@ package webhook import ( "context" + "time" - "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" + "github.com/NYTimes/gizmo/pubsub" + gizmoAWS "github.com/NYTimes/gizmo/pubsub/aws" + "github.com/flyteorg/flyteadmin/pkg/async" + "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" + "github.com/flyteorg/flyteadmin/pkg/async/webhook/implementations" + "github.com/flyteorg/flytestdlib/logger" + + webhookInterfaces "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flytestdlib/promutils" ) -func NewWebhooks(ctx context.Context, config runtimeInterfaces.WebhooksConfig, scope promutils.Scope) []interfaces.Webhook { - return []interfaces.Webhook{} +var enable64decoding = false + +func GetWebhook(config runtimeInterfaces.WebhooksConfig, scope promutils.Scope) webhookInterfaces.Webhook { + // TODO: Get others webhooks + return implementations.NewSlackWebhook(config, scope) +} + +func NewWebhookProcessors(config runtimeInterfaces.NotificationsConfig, scope promutils.Scope) []interfaces.Processor { + reconnectAttempts := config.ReconnectAttempts + reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second + var sub pubsub.Subscriber + var processors []interfaces.Processor + + for _, cfg := range config.WebhooksConfig { + if len(cfg.AWSConfig.Region) != 0 { + sqsConfig := gizmoAWS.SQSConfig{ + QueueName: cfg.NotificationsProcessorConfig.QueueName, + QueueOwnerAccountID: cfg.NotificationsProcessorConfig.AccountID, + // The AWS configuration type uses SNS to SQS for notifications. + // Gizmo by default will decode the SQS message using Base64 decoding. + // However, the message body of SQS is the SNS message format which isn't Base64 encoded. + ConsumeBase64: &enable64decoding, + } + sqsConfig.Region = cfg.AWSConfig.Region + var err error + err = async.Retry(reconnectAttempts, reconnectDelay, func() error { + sub, err = gizmoAWS.NewSubscriber(sqsConfig) + if err != nil { + logger.Warnf(context.TODO(), "Failed to initialize new gizmo aws subscriber with config [%+v] and err: %v", sqsConfig, err) + } + return err + }) + if err != nil { + panic(err) + } + } + + webhook := GetWebhook(cfg, scope) + processors = append(processors, implementations.NewWebhookProcessor(sub, webhook, scope)) + } + return processors } diff --git a/flyteadmin/pkg/async/webhook/implementations/processer.go b/flyteadmin/pkg/async/webhook/implementations/processer.go new file mode 100644 index 0000000000..cd5110007e --- /dev/null +++ b/flyteadmin/pkg/async/webhook/implementations/processer.go @@ -0,0 +1,123 @@ +package implementations + +import ( + "context" + "encoding/base64" + "encoding/json" + "time" + + "github.com/NYTimes/gizmo/pubsub" + "github.com/flyteorg/flyteadmin/pkg/async" + "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" + webhookInterfaces "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" + "github.com/flyteorg/flytestdlib/logger" + "github.com/flyteorg/flytestdlib/promutils" +) + +type Processor struct { + sub pubsub.Subscriber + webhook webhookInterfaces.Webhook + systemMetrics processorSystemMetrics +} + +func (p *Processor) StartProcessing() { + for { + logger.Warningf(context.Background(), "Starting webhook processor") + err := p.run() + logger.Errorf(context.Background(), "error with running processor err: [%v] ", err) + time.Sleep(async.RetryDelay) + } +} + +func (p *Processor) run() error { + var err error + + for msg := range p.sub.Start() { + p.systemMetrics.MessageTotal.Inc() + // Currently this is safe because Gizmo takes a string and casts it to a byte array. + stringMsg := string(msg.Message()) + + var snsJSONFormat map[string]interface{} + + // At Lyft, SNS populates SQS. This results in the message body of SQS having the SNS message format. + // The message format is documented here: https://docs.aws.amazon.com/sns/latest/dg/sns-message-and-json-formats.html + // The notification published is stored in the message field after unmarshalling the SQS message. + if err := json.Unmarshal(msg.Message(), &snsJSONFormat); err != nil { + p.systemMetrics.MessageDecodingError.Inc() + logger.Errorf(context.Background(), "failed to unmarshall JSON message [%s] from processor with err: %v", stringMsg, err) + p.markMessageDone(msg) + continue + } + + var value interface{} + var ok bool + var valueString string + + if value, ok = snsJSONFormat["Message"]; !ok { + logger.Errorf(context.Background(), "failed to retrieve message from unmarshalled JSON object [%s]", stringMsg) + p.systemMetrics.MessageDataError.Inc() + p.markMessageDone(msg) + continue + } + + if valueString, ok = value.(string); !ok { + p.systemMetrics.MessageDataError.Inc() + logger.Errorf(context.Background(), "failed to retrieve notification message (in string format) from unmarshalled JSON object for message [%s]", stringMsg) + p.markMessageDone(msg) + continue + } + + // The Publish method for SNS Encodes the notification using Base64 then stringifies it before + // setting that as the message body for SNS. Do the inverse to retrieve the notification. + messageBytes, err := base64.StdEncoding.DecodeString(valueString) + if err != nil { + logger.Errorf(context.Background(), "failed to Base64 decode from message string [%s] from message [%s] with err: %v", valueString, stringMsg, err) + p.systemMetrics.MessageDecodingError.Inc() + p.markMessageDone(msg) + continue + } + + logger.Info(context.Background(), "Processor is sending message to webhook endpoint") + // Send message to webhook + if err = p.webhook.Post(context.Background(), string(messageBytes)); err != nil { + p.systemMetrics.MessageProcessorError.Inc() + logger.Errorf(context.Background(), "Error sending an message [%s] to webhook endpoint with err: %v", string(messageBytes), err) + } else { + p.systemMetrics.MessageSuccess.Inc() + } + + p.markMessageDone(msg) + } + + if err = p.sub.Err(); err != nil { + p.systemMetrics.ChannelClosedError.Inc() + logger.Warningf(context.Background(), "The stream for the subscriber channel closed with err: %v", err) + } + + return err +} + +func (p *Processor) markMessageDone(message pubsub.SubscriberMessage) { + if err := message.Done(); err != nil { + p.systemMetrics.MessageDoneError.Inc() + logger.Errorf(context.Background(), "failed to mark message as Done() in processor with err: %v", err) + } +} + +func (p *Processor) StopProcessing() error { + // Note: If the underlying channel is already closed, then Stop() will return an error. + err := p.sub.Stop() + if err != nil { + p.systemMetrics.StopError.Inc() + logger.Errorf(context.Background(), "Failed to stop the subscriber channel gracefully with err: %v", err) + } + return err +} + +func NewWebhookProcessor(sub pubsub.Subscriber, webhook webhookInterfaces.Webhook, scope promutils.Scope) interfaces.Processor { + return &Processor{ + sub: sub, + webhook: webhook, + systemMetrics: newProcessorSystemMetrics(scope.NewSubScope("webhook_processor")), + } +} diff --git a/flyteadmin/pkg/async/webhook/implementations/processor_metrics.go b/flyteadmin/pkg/async/webhook/implementations/processor_metrics.go new file mode 100644 index 0000000000..adc4219f96 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/implementations/processor_metrics.go @@ -0,0 +1,32 @@ +package implementations + +import ( + "github.com/flyteorg/flytestdlib/promutils" + "github.com/prometheus/client_golang/prometheus" +) + +type processorSystemMetrics struct { + Scope promutils.Scope + MessageTotal prometheus.Counter + MessageDoneError prometheus.Counter + MessageDecodingError prometheus.Counter + MessageDataError prometheus.Counter + MessageProcessorError prometheus.Counter + MessageSuccess prometheus.Counter + ChannelClosedError prometheus.Counter + StopError prometheus.Counter +} + +func newProcessorSystemMetrics(scope promutils.Scope) processorSystemMetrics { + return processorSystemMetrics{ + Scope: scope, + MessageTotal: scope.MustNewCounter("message_total", "overall count of messages processed"), + MessageDecodingError: scope.MustNewCounter("message_decoding_error", "count of messages with decoding errors"), + MessageDataError: scope.MustNewCounter("message_data_error", "count of message data processing errors experience when preparing the message to be notified."), + MessageDoneError: scope.MustNewCounter("message_done_error", "count of message errors when marking it as done with underlying processor"), + MessageProcessorError: scope.MustNewCounter("message_processing_error", "count of errors when interacting with notification processor"), + MessageSuccess: scope.MustNewCounter("message_ok", "count of messages successfully processed by underlying notification mechanism"), + ChannelClosedError: scope.MustNewCounter("channel_closed_error", "count of channel closing errors"), + StopError: scope.MustNewCounter("stop_error", "count of errors in Stop() method"), + } +} diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index 7c8ada99b4..e31207e1bd 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -4,10 +4,11 @@ import ( "bytes" "context" "fmt" - "github.com/flyteorg/flytestdlib/logger" "io/ioutil" "net/http" + "github.com/flyteorg/flytestdlib/logger" + "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flytestdlib/promutils" @@ -21,7 +22,7 @@ type SlackWebhook struct { func (s *SlackWebhook) Post(ctx context.Context, message string) error { // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 logger.Infof(ctx, "Posting to Slack with message: [%v]", message) - webhookURL := "https://hooks.slack.com/services/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" + webhookURL := s.config.URL data := []byte(fmt.Sprintf("{'text': %s}", message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) if err != nil { diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go index 929dea12e9..f69472f704 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go @@ -2,10 +2,11 @@ package implementations import ( "context" + "testing" + runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flytestdlib/promutils" "github.com/stretchr/testify/assert" - "testing" ) func TestSlackWebhook(t *testing.T) { diff --git a/flyteadmin/pkg/async/webhook/implementations/webhook_processer.go b/flyteadmin/pkg/async/webhook/implementations/webhook_processer.go deleted file mode 100644 index 89926c85e1..0000000000 --- a/flyteadmin/pkg/async/webhook/implementations/webhook_processer.go +++ /dev/null @@ -1 +0,0 @@ -package implementations diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index 8d64d21149..80c4783714 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -32,7 +32,6 @@ import ( eventWriter "github.com/flyteorg/flyteadmin/pkg/async/events/interfaces" "github.com/flyteorg/flyteadmin/pkg/async/notifications" notificationInterfaces "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" - webhookInterface "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" "github.com/flyteorg/flyteadmin/pkg/errors" "github.com/flyteorg/flyteadmin/pkg/manager/impl/executions" "github.com/flyteorg/flyteadmin/pkg/manager/impl/util" @@ -91,7 +90,6 @@ type ExecutionManager struct { systemMetrics executionSystemMetrics userMetrics executionUserMetrics notificationClient notificationInterfaces.Publisher - webhooks []webhookInterface.Webhook urlData dataInterfaces.RemoteURLInterface workflowManager interfaces.WorkflowInterface namedEntityManager interfaces.NamedEntityInterface @@ -1636,7 +1634,7 @@ func newExecutionSystemMetrics(scope promutils.Scope) executionSystemMetrics { func NewExecutionManager(db repositoryInterfaces.Repository, pluginRegistry *plugins.Registry, config runtimeInterfaces.Configuration, storageClient *storage.DataStore, systemScope promutils.Scope, userScope promutils.Scope, - publisher notificationInterfaces.Publisher, webhooks []webhookInterface.Webhook, + publisher notificationInterfaces.Publisher, urlData dataInterfaces.RemoteURLInterface, workflowManager interfaces.WorkflowInterface, namedEntityManager interfaces.NamedEntityInterface, eventPublisher notificationInterfaces.Publisher, cloudEventPublisher cloudeventInterfaces.Publisher, @@ -1664,7 +1662,6 @@ func NewExecutionManager(db repositoryInterfaces.Repository, pluginRegistry *plu systemMetrics: systemMetrics, userMetrics: userMetrics, notificationClient: publisher, - webhooks: webhooks, urlData: urlData, workflowManager: workflowManager, namedEntityManager: namedEntityManager, diff --git a/flyteadmin/pkg/rpc/adminservice/base.go b/flyteadmin/pkg/rpc/adminservice/base.go index 898a0ff63f..9d478e7c77 100644 --- a/flyteadmin/pkg/rpc/adminservice/base.go +++ b/flyteadmin/pkg/rpc/adminservice/base.go @@ -3,9 +3,10 @@ package adminservice import ( "context" "fmt" - "reflect" "runtime/debug" + "github.com/flyteorg/flyteadmin/pkg/async/webhook" + "github.com/flyteorg/flyteadmin/plugins" "github.com/flyteorg/flyteadmin/pkg/async/cloudevent" @@ -21,7 +22,6 @@ import ( "github.com/flyteorg/flyteadmin/pkg/async/notifications" "github.com/flyteorg/flyteadmin/pkg/async/schedule" - "github.com/flyteorg/flyteadmin/pkg/async/webhook" "github.com/flyteorg/flyteadmin/pkg/data" executionCluster "github.com/flyteorg/flyteadmin/pkg/executioncluster/impl" manager "github.com/flyteorg/flyteadmin/pkg/manager/impl" @@ -100,19 +100,23 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi logger.Info(ctx, "Successfully created a workflow executor engine") pluginRegistry.RegisterDefault(plugins.PluginIDWorkflowExecutor, workflowExecutor) - logger.Infof(ctx, "notifier config: %v", *configuration.ApplicationConfiguration().GetNotificationsConfig()) publisher := notifications.NewNotificationsPublisher(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) - logger.Infof(ctx, "publisher: %v", reflect.TypeOf(publisher)) processor := notifications.NewNotificationsProcessor(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) - logger.Infof(ctx, "processor: %v", reflect.TypeOf(processor)) + webhookProcessors := webhook.NewWebhookProcessors(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) + eventPublisher := notifications.NewEventsPublisher(*configuration.ApplicationConfiguration().GetExternalEventsConfig(), adminScope) cloudEventPublisher := cloudevent.NewCloudEventsPublisher(ctx, *configuration.ApplicationConfiguration().GetCloudEventsConfig(), adminScope) - webhooks := webhook.NewWebhooks(ctx, *configuration.ApplicationConfiguration().GetWebhookConfig(), adminScope) + go func() { logger.Info(ctx, "Started processing notifications.") processor.StartProcessing() }() + go func() { + logger.Info(ctx, "Started processing webhook events.") + webhookProcessors[0].StartProcessing() + }() + // Configure workflow scheduler async processes. schedulerConfig := configuration.ApplicationConfiguration().GetSchedulerConfig() workflowScheduler := schedule.NewWorkflowScheduler(repo, schedule.WorkflowSchedulerConfig{ @@ -149,7 +153,7 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi executionManager := manager.NewExecutionManager(repo, pluginRegistry, configuration, dataStorageClient, adminScope.NewSubScope("execution_manager"), adminScope.NewSubScope("user_execution_metrics"), - publisher, webhooks, urlData, workflowManager, namedEntityManager, eventPublisher, cloudEventPublisher, executionEventWriter) + publisher, urlData, workflowManager, namedEntityManager, eventPublisher, cloudEventPublisher, executionEventWriter) versionManager := manager.NewVersionManager() scheduledWorkflowExecutor := workflowScheduler.GetWorkflowExecutor(executionManager, launchPlanManager) diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index 28071e9894..c6a13715d8 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -546,6 +546,7 @@ type NotificationsConfig struct { NotificationsPublisherConfig NotificationsPublisherConfig `json:"publisher"` NotificationsProcessorConfig NotificationsProcessorConfig `json:"processor"` NotificationsEmailerConfig NotificationsEmailerConfig `json:"emailer"` + WebhooksConfig []WebhooksConfig `json:"webhook"` // Number of times to attempt recreating a notifications processor client should there be any disruptions. ReconnectAttempts int `json:"reconnectAttempts"` // Specifies the time interval to wait before attempting to reconnect the notifications processor client. @@ -554,13 +555,16 @@ type NotificationsConfig struct { // WebhooksConfig defines the configuration for the webhook service. type WebhooksConfig struct { - // Defines the cloud provider that backs the scheduler. In the absence of a specification the no-op, 'local' - // scheme is used. - Type string `json:"type"` - Region string `json:"region"` - AWSConfig AWSConfig `json:"aws"` - GCPConfig GCPConfig `json:"gcp"` - URL string `json:"url"` + // Type of webhook service to use. Currently only "slack" is supported. + Type string `json:"type"` + URL string `json:"url"` + AWSConfig AWSConfig `json:"aws"` + GCPConfig GCPConfig `json:"gcp"` + NotificationsProcessorConfig NotificationsProcessorConfig `json:"processor"` + // Number of times to attempt recreating a notifications processor client should there be any disruptions. + ReconnectAttempts int `json:"reconnectAttempts"` + // Specifies the time interval to wait before attempting to reconnect the notifications processor client. + ReconnectDelaySeconds int `json:"reconnectDelaySeconds"` } // Domains are always globally set in the application config, whereas individual projects can be individually registered. From a646c2ffc4f6db8cc427e21d380e775d98ff50f1 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 23 May 2023 16:25:28 -0700 Subject: [PATCH 10/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/async/notifications/factory.go | 2 -- .../notifications/implementations/aws_processor.go | 12 ++++++------ flyteadmin/pkg/manager/impl/execution_manager.go | 11 +---------- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index a971d19e7e..836c1ff2fd 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -147,7 +147,6 @@ func NewNotificationsPublisher(config runtimeInterfaces.NotificationsConfig, sco var err error err = async.Retry(reconnectAttempts, reconnectDelay, func() error { publisher, err = gizmoAWS.NewPublisher(snsConfig) - logger.Errorf(context.Background(), "Failed to initialize aws publisher with config [%+v] and err: %v", snsConfig, err) return err }) @@ -155,7 +154,6 @@ func NewNotificationsPublisher(config runtimeInterfaces.NotificationsConfig, sco if err != nil { panic(err) } - logger.Infof(context.Background(), "Using aws publisher implementation for config type [%s]", config.Type) return implementations.NewPublisher(publisher, scope) case common.GCP: pubsubConfig := gizmoGCP.Config{ diff --git a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go index 8701c8ef2f..2cbf6d971a 100644 --- a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go +++ b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go @@ -89,12 +89,12 @@ func (p *Processor) run() error { continue } - //if err = p.email.SendEmail(context.Background(), emailMessage); err != nil { - // p.systemMetrics.MessageProcessorError.Inc() - // logger.Errorf(context.Background(), "Error sending an email message for message [%s] with emailM with err: %v", emailMessage.String(), err) - //} else { - // p.systemMetrics.MessageSuccess.Inc() - //} + if err = p.email.SendEmail(context.Background(), emailMessage); err != nil { + p.systemMetrics.MessageProcessorError.Inc() + logger.Errorf(context.Background(), "Error sending an email message for message [%s] with emailM with err: %v", emailMessage.String(), err) + } else { + p.systemMetrics.MessageSuccess.Inc() + } p.markMessageDone(msg) diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index 80c4783714..b51f8a8912 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -1293,16 +1293,7 @@ func (m *ExecutionManager) CreateWorkflowEvent(ctx context.Context, request admi m.systemMetrics.PublishEventError.Inc() logger.Infof(ctx, "error publishing event [%+v] with err: [%v]", request.RequestId, err) } - - //go func() { - // for _, client := range m.webhooks { - // if err := client.Post(ctx, proto.MessageName(&request), &request); err != nil { - // m.systemMetrics.PublishEventError.Inc() - // logger.Infof(ctx, "error publishing webhook event [%+v] with err: [%v]", request.RequestId, err) - // } - // } - //}() - + go func() { if err := m.cloudEventPublisher.Publish(ctx, proto.MessageName(&request), &request); err != nil { m.systemMetrics.PublishEventError.Inc() From d071e7530a852c64b9e45e16fc395f84b95cb6b0 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 23 May 2023 16:44:48 -0700 Subject: [PATCH 11/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/async/notifications/factory.go | 4 ++++ flyteadmin/pkg/manager/impl/execution_manager.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index 836c1ff2fd..2cf25f4764 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -75,6 +75,10 @@ func GetEmailer(config runtimeInterfaces.NotificationsConfig, scope promutils.Sc } func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, scope promutils.Scope) interfaces.Processor { + if len(config.NotificationsProcessorConfig.QueueName) == 0 { + return implementations.NewNoopProcess() + } + reconnectAttempts := config.ReconnectAttempts reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second var sub pubsub.Subscriber diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index b51f8a8912..04b51aa603 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -1293,7 +1293,7 @@ func (m *ExecutionManager) CreateWorkflowEvent(ctx context.Context, request admi m.systemMetrics.PublishEventError.Inc() logger.Infof(ctx, "error publishing event [%+v] with err: [%v]", request.RequestId, err) } - + go func() { if err := m.cloudEventPublisher.Publish(ctx, proto.MessageName(&request), &request); err != nil { m.systemMetrics.PublishEventError.Inc() From a37209ffcef08ec4d961ac814dc287ea50d3af74 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 23 May 2023 22:37:06 -0700 Subject: [PATCH 12/35] wip Signed-off-by: Kevin Su --- flyteadmin/go.mod | 4 +- flyteadmin/go.sum | 4 +- flyteadmin/pkg/async/notifications/email.go | 6 +- .../pkg/async/webhook/interfaces/webhook.go | 19 +++++++ .../pkg/manager/impl/execution_manager.go | 56 +++++++++++-------- 5 files changed, 61 insertions(+), 28 deletions(-) diff --git a/flyteadmin/go.mod b/flyteadmin/go.mod index ddef8ccff6..14e4f9ee01 100644 --- a/flyteadmin/go.mod +++ b/flyteadmin/go.mod @@ -46,6 +46,7 @@ require ( github.com/spf13/cobra v1.4.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.0 + golang.org/x/net v0.7.0 golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 google.golang.org/api v0.76.0 @@ -169,7 +170,6 @@ require ( go.opencensus.io v0.23.0 // indirect golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa // indirect golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect - golang.org/x/net v0.7.0 // indirect golang.org/x/sync v0.1.0 // indirect golang.org/x/sys v0.5.0 // indirect golang.org/x/term v0.5.0 // indirect @@ -213,3 +213,5 @@ replace github.com/robfig/cron/v3 => github.com/unionai/cron/v3 v3.0.2-0.2022091 // Retracted versions // This was published in error when attempting to create 1.5.1 Flyte release. retract v1.1.94 + +replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.5.8-0.20230524044929-7350f5703352 diff --git a/flyteadmin/go.sum b/flyteadmin/go.sum index 91807b3aaa..d0d44a94b3 100644 --- a/flyteadmin/go.sum +++ b/flyteadmin/go.sum @@ -312,8 +312,8 @@ github.com/fatih/structs v1.0.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ= github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= -github.com/flyteorg/flyteidl v1.5.5 h1:tNNhuXPog4atAMSGE2kyAg6JzYy1TvjqrrQeh1EZVHs= -github.com/flyteorg/flyteidl v1.5.5/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= +github.com/flyteorg/flyteidl v1.5.8-0.20230524044929-7350f5703352 h1:PaefUuLQ/yNM9Gy7/Nmzn0Dy1zDpJWzCA4CkfchgbU0= +github.com/flyteorg/flyteidl v1.5.8-0.20230524044929-7350f5703352/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= github.com/flyteorg/flyteplugins v1.0.56 h1:kBTDgTpdSi7wcptk4cMwz5vfh1MU82VaUMMboe1InXw= github.com/flyteorg/flyteplugins v1.0.56/go.mod h1:aFCKSn8TPzxSAILIiogHtUnHlUCN9+y6Vf+r9R4KZDU= github.com/flyteorg/flytepropeller v1.1.87 h1:Px7ASDjrWyeVrUb15qXmhw9QK7xPcFjL5Yetr2P6iGM= diff --git a/flyteadmin/pkg/async/notifications/email.go b/flyteadmin/pkg/async/notifications/email.go index 063acf335b..7abdcb507e 100644 --- a/flyteadmin/pkg/async/notifications/email.go +++ b/flyteadmin/pkg/async/notifications/email.go @@ -101,7 +101,7 @@ var getTemplateValueFuncs = map[string]GetTemplateValue{ launchPlanVersion: getLaunchPlanVersion, } -func substituteEmailParameters(message string, request admin.WorkflowExecutionEventRequest, execution *admin.Execution) string { +func SubstituteParameters(message string, request admin.WorkflowExecutionEventRequest, execution *admin.Execution) string { for template, function := range getTemplateValueFuncs { message = strings.Replace(message, fmt.Sprintf(substitutionParam, template), function(request, execution), replaceAllInstances) message = strings.Replace(message, fmt.Sprintf(substitutionParamNoSpaces, template), function(request, execution), replaceAllInstances) @@ -118,9 +118,9 @@ func ToEmailMessageFromWorkflowExecutionEvent( execution *admin.Execution) *admin.EmailMessage { return &admin.EmailMessage{ - SubjectLine: substituteEmailParameters(config.NotificationsEmailerConfig.Subject, request, execution), + SubjectLine: SubstituteParameters(config.NotificationsEmailerConfig.Subject, request, execution), SenderEmail: config.NotificationsEmailerConfig.Sender, RecipientsEmail: emailNotification.GetRecipientsEmail(), - Body: substituteEmailParameters(config.NotificationsEmailerConfig.Body, request, execution), + Body: SubstituteParameters(config.NotificationsEmailerConfig.Body, request, execution), } } diff --git a/flyteadmin/pkg/async/webhook/interfaces/webhook.go b/flyteadmin/pkg/async/webhook/interfaces/webhook.go index 984f916d09..153308edff 100644 --- a/flyteadmin/pkg/async/webhook/interfaces/webhook.go +++ b/flyteadmin/pkg/async/webhook/interfaces/webhook.go @@ -6,6 +6,25 @@ import ( //go:generate mockery -name=Webhook -output=../mocks -case=underscore +type Payload struct { + Value string `protobuf:"bytes,1,opt,value=value"` +} + +func (p Payload) Reset() { + //TODO implement me + panic("implement me") +} + +func (p Payload) String() string { + //TODO implement me + panic("implement me") +} + +func (p Payload) ProtoMessage() { + //TODO implement me + panic("implement me") +} + // Webhook Defines the interface for Publishing execution event to other services (Slack). type Webhook interface { // Post The notificationType is inferred from the Notification object in the Execution Spec. diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index 04b51aa603..09c0593abb 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -1512,31 +1512,11 @@ func (m *ExecutionManager) publishNotifications(ctx context.Context, request adm continue } - // Currently all three supported notifications use email underneath to send the notification. - // Convert Slack and PagerDuty into an EmailNotification type. - var emailNotification admin.EmailNotification - if notification.GetEmail() != nil { - emailNotification.RecipientsEmail = notification.GetEmail().GetRecipientsEmail() - } else if notification.GetPagerDuty() != nil { - emailNotification.RecipientsEmail = notification.GetPagerDuty().GetRecipientsEmail() - } else if notification.GetSlack() != nil { - emailNotification.RecipientsEmail = notification.GetSlack().GetRecipientsEmail() - } else { - logger.Debugf(ctx, "failed to publish notification, encountered unrecognized type: %v", notification.Type) - m.systemMetrics.UnexpectedDataError.Inc() - // Unsupported notification types should have been caught when the launch plan was being created. - return errors.NewFlyteAdminErrorf(codes.Internal, "Unsupported notification type [%v] for execution [%+v]", - notification.Type, request.Event.ExecutionId) - } + payload, err := m.getPayload(ctx, request, notification, adminExecution) - // Convert the email Notification into an email message to be published. - // Currently there are no possible errors while creating an email message. - // Once customizable content is specified, errors are possible. - email := notifications.ToEmailMessageFromWorkflowExecutionEvent( - *m.config.ApplicationConfiguration().GetNotificationsConfig(), emailNotification, request, adminExecution) // Errors seen while publishing a message are considered non-fatal to the method and will not result // in the method returning an error. - if err = m.notificationClient.Publish(ctx, proto.MessageName(&emailNotification), email); err != nil { + if err = m.notificationClient.Publish(ctx, proto.MessageName(payload), payload); err != nil { m.systemMetrics.PublishNotificationError.Inc() logger.Infof(ctx, "error publishing email notification [%+v] with err: [%v]", notification, err) } @@ -1544,6 +1524,38 @@ func (m *ExecutionManager) publishNotifications(ctx context.Context, request adm return nil } +func (m *ExecutionManager) getPayload(ctx context.Context, request admin.WorkflowExecutionEventRequest, + notification *admin.Notification, adminExecution *admin.Execution) (proto.Message, error) { + // Currently all three supported notifications use email underneath to send the notification. + // Convert Slack and PagerDuty into an EmailNotification type. + var emailNotification admin.EmailNotification + if notification.GetEmail() != nil { + emailNotification.RecipientsEmail = notification.GetEmail().GetRecipientsEmail() + } else if notification.GetPagerDuty() != nil { + emailNotification.RecipientsEmail = notification.GetPagerDuty().GetRecipientsEmail() + } else if notification.GetSlack() != nil { + emailNotification.RecipientsEmail = notification.GetSlack().GetRecipientsEmail() + } else if notification.GetWebhook() != nil { + var msg proto.Message + payload := notifications.SubstituteParameters(notification.GetWebhook().Payload, request, adminExecution) + err := proto.UnmarshalText(payload, msg) + return msg, err + } else { + logger.Debugf(ctx, "failed to publish notification, encountered unrecognized type: %v", notification.Type) + m.systemMetrics.UnexpectedDataError.Inc() + // Unsupported notification types should have been caught when the launch plan was being created. + return nil, errors.NewFlyteAdminErrorf(codes.Internal, "Unsupported notification type [%v] for execution [%+v]", + notification.Type, request.Event.ExecutionId) + } + + // Convert the email Notification into an email message to be published. + // Currently, there are no possible errors while creating an email message. + // Once customizable content is specified, errors are possible. + email := notifications.ToEmailMessageFromWorkflowExecutionEvent( + *m.config.ApplicationConfiguration().GetNotificationsConfig(), emailNotification, request, adminExecution) + return email, nil +} + func (m *ExecutionManager) TerminateExecution( ctx context.Context, request admin.ExecutionTerminateRequest) (*admin.ExecutionTerminateResponse, error) { if err := validation.ValidateWorkflowExecutionIdentifier(request.Id); err != nil { From e498539938b0c6fa03c11a794381f557129727f3 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 24 May 2023 10:28:24 -0700 Subject: [PATCH 13/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/async/notifications/factory.go | 14 +++ flyteadmin/pkg/async/webhook/factory.go | 2 +- .../webhook/implementations/slack_webhook.go | 4 +- .../implementations/slack_webhook_test.go | 2 +- .../pkg/manager/impl/execution_manager.go | 100 ++++++++++++------ flyteadmin/pkg/rpc/adminservice/base.go | 14 +-- .../runtime/application_config_provider.go | 6 +- .../interfaces/application_configuration.go | 21 ++-- 8 files changed, 112 insertions(+), 51 deletions(-) diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index 2cf25f4764..3bfeca008b 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -133,6 +133,20 @@ func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, sco } } +func NewWebhookNotificationsPublisher(config runtimeInterfaces.WebhooksNotificationConfig, scope promutils.Scope) interfaces.Publisher { + cfg := runtimeInterfaces.NotificationsConfig{ + Type: config.Type, + NotificationsPublisherConfig: runtimeInterfaces.NotificationsPublisherConfig{ + TopicName: config.NotificationsPublisherConfig.TopicName, + }, + AWSConfig: config.AWSConfig, + GCPConfig: config.GCPConfig, + ReconnectAttempts: config.ReconnectAttempts, + ReconnectDelaySeconds: config.ReconnectDelaySeconds, + } + return NewNotificationsPublisher(cfg, scope) +} + func NewNotificationsPublisher(config runtimeInterfaces.NotificationsConfig, scope promutils.Scope) interfaces.Publisher { reconnectAttempts := config.ReconnectAttempts reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index fd1f55551a..5f20b4c789 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -18,7 +18,7 @@ import ( var enable64decoding = false -func GetWebhook(config runtimeInterfaces.WebhooksConfig, scope promutils.Scope) webhookInterfaces.Webhook { +func GetWebhook(config runtimeInterfaces.WebhooksNotificationConfig, scope promutils.Scope) webhookInterfaces.Webhook { // TODO: Get others webhooks return implementations.NewSlackWebhook(config, scope) } diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index e31207e1bd..2372047575 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -15,7 +15,7 @@ import ( ) type SlackWebhook struct { - config runtimeInterfaces.WebhooksConfig + config runtimeInterfaces.WebhooksNotificationConfig systemMetrics webhookMetrics } @@ -48,7 +48,7 @@ func (s *SlackWebhook) Post(ctx context.Context, message string) error { return nil } -func NewSlackWebhook(config runtimeInterfaces.WebhooksConfig, scope promutils.Scope) interfaces.Webhook { +func NewSlackWebhook(config runtimeInterfaces.WebhooksNotificationConfig, scope promutils.Scope) interfaces.Webhook { return &SlackWebhook{ config: config, diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go index f69472f704..8c70d2286d 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go @@ -10,7 +10,7 @@ import ( ) func TestSlackWebhook(t *testing.T) { - webhook := NewSlackWebhook(runtimeInterfaces.WebhooksConfig{}, promutils.NewTestScope()) + webhook := NewSlackWebhook(runtimeInterfaces.WebhooksNotificationConfig{}, promutils.NewTestScope()) err := webhook.Post(context.Background(), "message") assert.Nil(t, err) } diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index 09c0593abb..84a181581f 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -90,6 +90,7 @@ type ExecutionManager struct { systemMetrics executionSystemMetrics userMetrics executionUserMetrics notificationClient notificationInterfaces.Publisher + webhookClient notificationInterfaces.Publisher urlData dataInterfaces.RemoteURLInterface workflowManager interfaces.WorkflowInterface namedEntityManager interfaces.NamedEntityInterface @@ -1287,6 +1288,14 @@ func (m *ExecutionManager) CreateWorkflowEvent(ctx context.Context, request admi request, err) return nil, err } + err = m.publishWebhookNotifications(ctx, request, *executionModel) + if err != nil { + // The only errors that publishNotifications will forward are those related + // to unexpected data and transformation errors. + logger.Debugf(ctx, "failed to publish notifications for CreateWorkflowEvent [%+v] due to err: %v", + request, err) + return nil, err + } } if err := m.eventPublisher.Publish(ctx, proto.MessageName(&request), &request); err != nil { @@ -1512,11 +1521,32 @@ func (m *ExecutionManager) publishNotifications(ctx context.Context, request adm continue } - payload, err := m.getPayload(ctx, request, notification, adminExecution) + // Currently all three supported notifications use email underneath to send the notification. + // Convert Slack and PagerDuty into an EmailNotification type. + var emailNotification admin.EmailNotification + if notification.GetEmail() != nil { + emailNotification.RecipientsEmail = notification.GetEmail().GetRecipientsEmail() + } else if notification.GetPagerDuty() != nil { + emailNotification.RecipientsEmail = notification.GetPagerDuty().GetRecipientsEmail() + } else if notification.GetSlack() != nil { + emailNotification.RecipientsEmail = notification.GetSlack().GetRecipientsEmail() + } else { + logger.Debugf(ctx, "failed to publish notification, encountered unrecognized type: %v", notification.Type) + m.systemMetrics.UnexpectedDataError.Inc() + // Unsupported notification types should have been caught when the launch plan was being created. + return errors.NewFlyteAdminErrorf(codes.Internal, "Unsupported notification type [%v] for execution [%+v]", + notification.Type, request.Event.ExecutionId) + } + + // Convert the email Notification into an email message to be published. + // Currently, there are no possible errors while creating an email message. + // Once customizable content is specified, errors are possible. + email := notifications.ToEmailMessageFromWorkflowExecutionEvent( + *m.config.ApplicationConfiguration().GetNotificationsConfig(), emailNotification, request, adminExecution) // Errors seen while publishing a message are considered non-fatal to the method and will not result // in the method returning an error. - if err = m.notificationClient.Publish(ctx, proto.MessageName(payload), payload); err != nil { + if err = m.notificationClient.Publish(ctx, proto.MessageName(email), email); err != nil { m.systemMetrics.PublishNotificationError.Inc() logger.Infof(ctx, "error publishing email notification [%+v] with err: [%v]", notification, err) } @@ -1524,36 +1554,43 @@ func (m *ExecutionManager) publishNotifications(ctx context.Context, request adm return nil } -func (m *ExecutionManager) getPayload(ctx context.Context, request admin.WorkflowExecutionEventRequest, - notification *admin.Notification, adminExecution *admin.Execution) (proto.Message, error) { - // Currently all three supported notifications use email underneath to send the notification. - // Convert Slack and PagerDuty into an EmailNotification type. - var emailNotification admin.EmailNotification - if notification.GetEmail() != nil { - emailNotification.RecipientsEmail = notification.GetEmail().GetRecipientsEmail() - } else if notification.GetPagerDuty() != nil { - emailNotification.RecipientsEmail = notification.GetPagerDuty().GetRecipientsEmail() - } else if notification.GetSlack() != nil { - emailNotification.RecipientsEmail = notification.GetSlack().GetRecipientsEmail() - } else if notification.GetWebhook() != nil { +func (m *ExecutionManager) publishWebhookNotifications(ctx context.Context, request admin.WorkflowExecutionEventRequest, + execution models.Execution) error { + adminExecution, err := transformers.FromExecutionModel(execution, transformers.DefaultExecutionTransformerOptions) + if err != nil { + // This shouldn't happen because execution manager marshaled the data into models.Execution. + m.systemMetrics.TransformerError.Inc() + return errors.NewFlyteAdminErrorf(codes.Internal, "Failed to transform execution [%+v] with err: %v", request.Event.ExecutionId, err) + } + var notificationsList = adminExecution.Closure.Notifications + logger.Debugf(ctx, "publishing notifications for execution [%+v] in state [%+v] for notifications [%+v]", + request.Event.ExecutionId, request.Event.Phase, notificationsList) + for _, notification := range notificationsList { + // Check if the notification phase matches the current one. + var matchPhase = false + for _, phase := range notification.Phases { + if phase == request.Event.Phase { + matchPhase = true + } + } + + // The current phase doesn't match; no notifications will be sent for the current notification option. + if !matchPhase { + continue + } + var msg proto.Message payload := notifications.SubstituteParameters(notification.GetWebhook().Payload, request, adminExecution) err := proto.UnmarshalText(payload, msg) - return msg, err - } else { - logger.Debugf(ctx, "failed to publish notification, encountered unrecognized type: %v", notification.Type) - m.systemMetrics.UnexpectedDataError.Inc() - // Unsupported notification types should have been caught when the launch plan was being created. - return nil, errors.NewFlyteAdminErrorf(codes.Internal, "Unsupported notification type [%v] for execution [%+v]", - notification.Type, request.Event.ExecutionId) - } - - // Convert the email Notification into an email message to be published. - // Currently, there are no possible errors while creating an email message. - // Once customizable content is specified, errors are possible. - email := notifications.ToEmailMessageFromWorkflowExecutionEvent( - *m.config.ApplicationConfiguration().GetNotificationsConfig(), emailNotification, request, adminExecution) - return email, nil + + // Errors seen while publishing a message are considered non-fatal to the method and will not result + // in the method returning an error. + if err = m.webhookClient.Publish(ctx, proto.MessageName(msg), msg); err != nil { + m.systemMetrics.PublishNotificationError.Inc() + logger.Infof(ctx, "error publishing email notification [%+v] with err: [%v]", notification, err) + } + } + return nil } func (m *ExecutionManager) TerminateExecution( @@ -1637,7 +1674,7 @@ func newExecutionSystemMetrics(scope promutils.Scope) executionSystemMetrics { func NewExecutionManager(db repositoryInterfaces.Repository, pluginRegistry *plugins.Registry, config runtimeInterfaces.Configuration, storageClient *storage.DataStore, systemScope promutils.Scope, userScope promutils.Scope, - publisher notificationInterfaces.Publisher, + notificationPublisher notificationInterfaces.Publisher, publisher notificationInterfaces.Publisher, urlData dataInterfaces.RemoteURLInterface, workflowManager interfaces.WorkflowInterface, namedEntityManager interfaces.NamedEntityInterface, eventPublisher notificationInterfaces.Publisher, cloudEventPublisher cloudeventInterfaces.Publisher, @@ -1664,7 +1701,8 @@ func NewExecutionManager(db repositoryInterfaces.Repository, pluginRegistry *plu _clock: clock.New(), systemMetrics: systemMetrics, userMetrics: userMetrics, - notificationClient: publisher, + notificationClient: notificationPublisher, + webhookClient: publisher, urlData: urlData, workflowManager: workflowManager, namedEntityManager: namedEntityManager, diff --git a/flyteadmin/pkg/rpc/adminservice/base.go b/flyteadmin/pkg/rpc/adminservice/base.go index 9d478e7c77..e3dddff43f 100644 --- a/flyteadmin/pkg/rpc/adminservice/base.go +++ b/flyteadmin/pkg/rpc/adminservice/base.go @@ -100,18 +100,18 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi logger.Info(ctx, "Successfully created a workflow executor engine") pluginRegistry.RegisterDefault(plugins.PluginIDWorkflowExecutor, workflowExecutor) - publisher := notifications.NewNotificationsPublisher(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) + notificationPublisher := notifications.NewNotificationsPublisher(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) processor := notifications.NewNotificationsProcessor(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) - webhookProcessors := webhook.NewWebhookProcessors(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) - - eventPublisher := notifications.NewEventsPublisher(*configuration.ApplicationConfiguration().GetExternalEventsConfig(), adminScope) - cloudEventPublisher := cloudevent.NewCloudEventsPublisher(ctx, *configuration.ApplicationConfiguration().GetCloudEventsConfig(), adminScope) - go func() { logger.Info(ctx, "Started processing notifications.") processor.StartProcessing() }() + eventPublisher := notifications.NewEventsPublisher(*configuration.ApplicationConfiguration().GetExternalEventsConfig(), adminScope) + cloudEventPublisher := cloudevent.NewCloudEventsPublisher(ctx, *configuration.ApplicationConfiguration().GetCloudEventsConfig(), adminScope) + + publisher := notifications.NewWebhookNotificationsPublisher(*configuration.ApplicationConfiguration().GetWebhookNotificationConfig(), adminScope) + webhookProcessors := webhook.NewWebhookProcessors(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) go func() { logger.Info(ctx, "Started processing webhook events.") webhookProcessors[0].StartProcessing() @@ -153,7 +153,7 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi executionManager := manager.NewExecutionManager(repo, pluginRegistry, configuration, dataStorageClient, adminScope.NewSubScope("execution_manager"), adminScope.NewSubScope("user_execution_metrics"), - publisher, urlData, workflowManager, namedEntityManager, eventPublisher, cloudEventPublisher, executionEventWriter) + notificationPublisher, publisher, urlData, workflowManager, namedEntityManager, eventPublisher, cloudEventPublisher, executionEventWriter) versionManager := manager.NewVersionManager() scheduledWorkflowExecutor := workflowScheduler.GetWorkflowExecutor(executionManager, launchPlanManager) diff --git a/flyteadmin/pkg/runtime/application_config_provider.go b/flyteadmin/pkg/runtime/application_config_provider.go index c0e92a008a..79cbe061d6 100644 --- a/flyteadmin/pkg/runtime/application_config_provider.go +++ b/flyteadmin/pkg/runtime/application_config_provider.go @@ -85,7 +85,7 @@ var cloudEventsConfig = config.MustRegisterSection(cloudEvents, &interfaces.Clou Type: common.Local, }) -var webhooksConfig = config.MustRegisterSection(webhook, &interfaces.WebhooksConfig{ +var webhooksConfig = config.MustRegisterSection(webhook, &interfaces.WebhooksNotificationConfig{ Type: "slack", }) @@ -124,8 +124,8 @@ func (p *ApplicationConfigurationProvider) GetCloudEventsConfig() *interfaces.Cl return cloudEventsConfig.GetConfig().(*interfaces.CloudEventsConfig) } -func (p *ApplicationConfigurationProvider) GetWebhookConfig() *interfaces.WebhooksConfig { - return webhooksConfig.GetConfig().(*interfaces.WebhooksConfig) +func (p *ApplicationConfigurationProvider) GetWebhookNotificationConfig() *interfaces.WebhooksNotificationConfig { + return webhooksConfig.GetConfig().(*interfaces.WebhooksNotificationConfig) } func NewApplicationConfigurationProvider() interfaces.ApplicationConfiguration { diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index c6a13715d8..08ed8c303f 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -546,21 +546,30 @@ type NotificationsConfig struct { NotificationsPublisherConfig NotificationsPublisherConfig `json:"publisher"` NotificationsProcessorConfig NotificationsProcessorConfig `json:"processor"` NotificationsEmailerConfig NotificationsEmailerConfig `json:"emailer"` - WebhooksConfig []WebhooksConfig `json:"webhook"` // Number of times to attempt recreating a notifications processor client should there be any disruptions. ReconnectAttempts int `json:"reconnectAttempts"` // Specifies the time interval to wait before attempting to reconnect the notifications processor client. ReconnectDelaySeconds int `json:"reconnectDelaySeconds"` } -// WebhooksConfig defines the configuration for the webhook service. -type WebhooksConfig struct { +type WebHookConfig struct { // Type of webhook service to use. Currently only "slack" is supported. - Type string `json:"type"` + Name string `json:"name"` URL string `json:"url"` + Payload string `json:"payload"` + SecretName string `json:"secret"` + NotificationsProcessorConfig NotificationsProcessorConfig `json:"processor"` +} + +// WebhooksNotificationConfig defines the configuration for the webhook service. +type WebhooksNotificationConfig struct { + // Defines the cloud provider that backs the scheduler. In the absence of a specification the no-op, 'local' + // scheme is used. + Type string `json:"type"` AWSConfig AWSConfig `json:"aws"` GCPConfig GCPConfig `json:"gcp"` - NotificationsProcessorConfig NotificationsProcessorConfig `json:"processor"` + NotificationsPublisherConfig NotificationsPublisherConfig `json:"publisher"` + WebhooksConfig []WebHookConfig `json:"webhooks"` // Number of times to attempt recreating a notifications processor client should there be any disruptions. ReconnectAttempts int `json:"reconnectAttempts"` // Specifies the time interval to wait before attempting to reconnect the notifications processor client. @@ -587,5 +596,5 @@ type ApplicationConfiguration interface { GetDomainsConfig() *DomainsConfig GetExternalEventsConfig() *ExternalEventsConfig GetCloudEventsConfig() *CloudEventsConfig - GetWebhookConfig() *WebhooksConfig + GetWebhookNotificationConfig() *WebhooksNotificationConfig } From bb05d3c671715836466427d51a695c3467ed5a75 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 24 May 2023 15:02:06 -0700 Subject: [PATCH 14/35] wip Signed-off-by: Kevin Su --- .../pkg/async/notifications/implementations/publisher.go | 1 - flyteadmin/pkg/async/webhook/factory.go | 8 ++++---- .../pkg/async/webhook/implementations/slack_webhook.go | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/flyteadmin/pkg/async/notifications/implementations/publisher.go b/flyteadmin/pkg/async/notifications/implementations/publisher.go index b8ec7047c5..deadc82639 100644 --- a/flyteadmin/pkg/async/notifications/implementations/publisher.go +++ b/flyteadmin/pkg/async/notifications/implementations/publisher.go @@ -2,7 +2,6 @@ package implementations import ( "context" - "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" "github.com/NYTimes/gizmo/pubsub" diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index 5f20b4c789..57b7f3e6ea 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -18,19 +18,19 @@ import ( var enable64decoding = false -func GetWebhook(config runtimeInterfaces.WebhooksNotificationConfig, scope promutils.Scope) webhookInterfaces.Webhook { +func GetWebhook(config runtimeInterfaces.WebHookConfig, scope promutils.Scope) webhookInterfaces.Webhook { // TODO: Get others webhooks return implementations.NewSlackWebhook(config, scope) } -func NewWebhookProcessors(config runtimeInterfaces.NotificationsConfig, scope promutils.Scope) []interfaces.Processor { +func NewWebhookProcessors(config runtimeInterfaces.WebhooksNotificationConfig, scope promutils.Scope) []interfaces.Processor { reconnectAttempts := config.ReconnectAttempts reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second var sub pubsub.Subscriber var processors []interfaces.Processor for _, cfg := range config.WebhooksConfig { - if len(cfg.AWSConfig.Region) != 0 { + if len(config.AWSConfig.Region) != 0 { sqsConfig := gizmoAWS.SQSConfig{ QueueName: cfg.NotificationsProcessorConfig.QueueName, QueueOwnerAccountID: cfg.NotificationsProcessorConfig.AccountID, @@ -39,7 +39,7 @@ func NewWebhookProcessors(config runtimeInterfaces.NotificationsConfig, scope pr // However, the message body of SQS is the SNS message format which isn't Base64 encoded. ConsumeBase64: &enable64decoding, } - sqsConfig.Region = cfg.AWSConfig.Region + sqsConfig.Region = config.AWSConfig.Region var err error err = async.Retry(reconnectAttempts, reconnectDelay, func() error { sub, err = gizmoAWS.NewSubscriber(sqsConfig) diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index 2372047575..de4d431ded 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -15,7 +15,7 @@ import ( ) type SlackWebhook struct { - config runtimeInterfaces.WebhooksNotificationConfig + config runtimeInterfaces.WebHookConfig systemMetrics webhookMetrics } @@ -48,7 +48,7 @@ func (s *SlackWebhook) Post(ctx context.Context, message string) error { return nil } -func NewSlackWebhook(config runtimeInterfaces.WebhooksNotificationConfig, scope promutils.Scope) interfaces.Webhook { +func NewSlackWebhook(config runtimeInterfaces.WebHookConfig, scope promutils.Scope) interfaces.Webhook { return &SlackWebhook{ config: config, From eccd859c50c018b676e9b3cf45f25edd56a34641 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 24 May 2023 15:03:15 -0700 Subject: [PATCH 15/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/rpc/adminservice/base.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flyteadmin/pkg/rpc/adminservice/base.go b/flyteadmin/pkg/rpc/adminservice/base.go index e3dddff43f..d4ccf95d5d 100644 --- a/flyteadmin/pkg/rpc/adminservice/base.go +++ b/flyteadmin/pkg/rpc/adminservice/base.go @@ -111,7 +111,7 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi cloudEventPublisher := cloudevent.NewCloudEventsPublisher(ctx, *configuration.ApplicationConfiguration().GetCloudEventsConfig(), adminScope) publisher := notifications.NewWebhookNotificationsPublisher(*configuration.ApplicationConfiguration().GetWebhookNotificationConfig(), adminScope) - webhookProcessors := webhook.NewWebhookProcessors(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) + webhookProcessors := webhook.NewWebhookProcessors(*configuration.ApplicationConfiguration().GetWebhookNotificationConfig(), adminScope) go func() { logger.Info(ctx, "Started processing webhook events.") webhookProcessors[0].StartProcessing() From 31d11a46b7186699096e7c7484b954453f1ce361 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 24 May 2023 15:39:55 -0700 Subject: [PATCH 16/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/runtime/application_config_provider.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/flyteadmin/pkg/runtime/application_config_provider.go b/flyteadmin/pkg/runtime/application_config_provider.go index 79cbe061d6..824312995b 100644 --- a/flyteadmin/pkg/runtime/application_config_provider.go +++ b/flyteadmin/pkg/runtime/application_config_provider.go @@ -12,6 +12,7 @@ const flyteAdmin = "flyteadmin" const scheduler = "scheduler" const remoteData = "remoteData" const notifications = "notifications" +const webhookNotifications = "webhookNotifications" const domains = "domains" const externalEvents = "externalEvents" const cloudEvents = "cloudEvents" @@ -63,6 +64,9 @@ var remoteDataConfig = config.MustRegisterSection(remoteData, &interfaces.Remote var notificationsConfig = config.MustRegisterSection(notifications, &interfaces.NotificationsConfig{ Type: common.Local, }) +var webhookNotificationsConfig = config.MustRegisterSection(webhookNotifications, &interfaces.WebhooksNotificationConfig{ + Type: common.Local, +}) var domainsConfig = config.MustRegisterSection(domains, &interfaces.DomainsConfig{ { ID: "development", @@ -85,10 +89,6 @@ var cloudEventsConfig = config.MustRegisterSection(cloudEvents, &interfaces.Clou Type: common.Local, }) -var webhooksConfig = config.MustRegisterSection(webhook, &interfaces.WebhooksNotificationConfig{ - Type: "slack", -}) - // Implementation of an interfaces.ApplicationConfiguration type ApplicationConfigurationProvider struct{} @@ -125,7 +125,7 @@ func (p *ApplicationConfigurationProvider) GetCloudEventsConfig() *interfaces.Cl } func (p *ApplicationConfigurationProvider) GetWebhookNotificationConfig() *interfaces.WebhooksNotificationConfig { - return webhooksConfig.GetConfig().(*interfaces.WebhooksNotificationConfig) + return webhookNotificationsConfig.GetConfig().(*interfaces.WebhooksNotificationConfig) } func NewApplicationConfigurationProvider() interfaces.ApplicationConfiguration { From c966a24d6c424a9fef03a4df3f8f120b38586e50 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 24 May 2023 16:23:42 -0700 Subject: [PATCH 17/35] wip Signed-off-by: Kevin Su --- flyteadmin/pkg/async/notifications/factory.go | 5 +---- flyteadmin/pkg/manager/impl/execution_manager.go | 3 ++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index 3bfeca008b..82abd97bde 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -75,10 +75,6 @@ func GetEmailer(config runtimeInterfaces.NotificationsConfig, scope promutils.Sc } func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, scope promutils.Scope) interfaces.Processor { - if len(config.NotificationsProcessorConfig.QueueName) == 0 { - return implementations.NewNoopProcess() - } - reconnectAttempts := config.ReconnectAttempts reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second var sub pubsub.Subscriber @@ -134,6 +130,7 @@ func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, sco } func NewWebhookNotificationsPublisher(config runtimeInterfaces.WebhooksNotificationConfig, scope promutils.Scope) interfaces.Publisher { + logger.Infof(context.Background(), "Starting webhook notification publisher [%s]", config.Type) cfg := runtimeInterfaces.NotificationsConfig{ Type: config.Type, NotificationsPublisherConfig: runtimeInterfaces.NotificationsPublisherConfig{ diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index 84a181581f..86ae3d55be 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -1580,7 +1580,8 @@ func (m *ExecutionManager) publishWebhookNotifications(ctx context.Context, requ } var msg proto.Message - payload := notifications.SubstituteParameters(notification.GetWebhook().Payload, request, adminExecution) + // TODO: Add message attributes + payload := notifications.SubstituteParameters("hello world", request, adminExecution) err := proto.UnmarshalText(payload, msg) // Errors seen while publishing a message are considered non-fatal to the method and will not result From e0b14d2ee27613fa31eb460283b128508b6823be Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Thu, 25 May 2023 17:37:30 -0700 Subject: [PATCH 18/35] webhook poc Signed-off-by: Kevin Su --- flyteadmin/go.mod | 2 +- flyteadmin/go.sum | 4 ++-- flyteadmin/pkg/async/notifications/factory.go | 2 +- flyteadmin/pkg/async/webhook/factory.go | 2 +- .../{processer.go => aws_processer.go} | 17 +++++++++++++---- .../webhook/implementations/slack_webhook.go | 7 ++++--- .../implementations/slack_webhook_test.go | 5 +++-- .../pkg/async/webhook/interfaces/webhook.go | 3 ++- .../pkg/manager/impl/execution_manager.go | 12 ++++++++---- .../pkg/runtime/application_config_provider.go | 6 +++--- .../interfaces/application_configuration.go | 6 +++--- 11 files changed, 41 insertions(+), 25 deletions(-) rename flyteadmin/pkg/async/webhook/implementations/{processer.go => aws_processer.go} (86%) diff --git a/flyteadmin/go.mod b/flyteadmin/go.mod index 14e4f9ee01..10753d139d 100644 --- a/flyteadmin/go.mod +++ b/flyteadmin/go.mod @@ -214,4 +214,4 @@ replace github.com/robfig/cron/v3 => github.com/unionai/cron/v3 v3.0.2-0.2022091 // This was published in error when attempting to create 1.5.1 Flyte release. retract v1.1.94 -replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.5.8-0.20230524044929-7350f5703352 +replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.5.8-0.20230525232442-053ca98a8450 diff --git a/flyteadmin/go.sum b/flyteadmin/go.sum index d0d44a94b3..264e4fc4ee 100644 --- a/flyteadmin/go.sum +++ b/flyteadmin/go.sum @@ -312,8 +312,8 @@ github.com/fatih/structs v1.0.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ= github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= -github.com/flyteorg/flyteidl v1.5.8-0.20230524044929-7350f5703352 h1:PaefUuLQ/yNM9Gy7/Nmzn0Dy1zDpJWzCA4CkfchgbU0= -github.com/flyteorg/flyteidl v1.5.8-0.20230524044929-7350f5703352/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= +github.com/flyteorg/flyteidl v1.5.8-0.20230525232442-053ca98a8450 h1:mlu/s3sCGf3/3diKrQn06OvWdroM2ErhqnXm17I8vLY= +github.com/flyteorg/flyteidl v1.5.8-0.20230525232442-053ca98a8450/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= github.com/flyteorg/flyteplugins v1.0.56 h1:kBTDgTpdSi7wcptk4cMwz5vfh1MU82VaUMMboe1InXw= github.com/flyteorg/flyteplugins v1.0.56/go.mod h1:aFCKSn8TPzxSAILIiogHtUnHlUCN9+y6Vf+r9R4KZDU= github.com/flyteorg/flytepropeller v1.1.87 h1:Px7ASDjrWyeVrUb15qXmhw9QK7xPcFjL5Yetr2P6iGM= diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index 82abd97bde..8febfb731b 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -129,7 +129,7 @@ func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, sco } } -func NewWebhookNotificationsPublisher(config runtimeInterfaces.WebhooksNotificationConfig, scope promutils.Scope) interfaces.Publisher { +func NewWebhookNotificationsPublisher(config runtimeInterfaces.WebhookNotificationsConfig, scope promutils.Scope) interfaces.Publisher { logger.Infof(context.Background(), "Starting webhook notification publisher [%s]", config.Type) cfg := runtimeInterfaces.NotificationsConfig{ Type: config.Type, diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index 57b7f3e6ea..7b75d58b0d 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -23,7 +23,7 @@ func GetWebhook(config runtimeInterfaces.WebHookConfig, scope promutils.Scope) w return implementations.NewSlackWebhook(config, scope) } -func NewWebhookProcessors(config runtimeInterfaces.WebhooksNotificationConfig, scope promutils.Scope) []interfaces.Processor { +func NewWebhookProcessors(config runtimeInterfaces.WebhookNotificationsConfig, scope promutils.Scope) []interfaces.Processor { reconnectAttempts := config.ReconnectAttempts reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second var sub pubsub.Subscriber diff --git a/flyteadmin/pkg/async/webhook/implementations/processer.go b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go similarity index 86% rename from flyteadmin/pkg/async/webhook/implementations/processer.go rename to flyteadmin/pkg/async/webhook/implementations/aws_processer.go index cd5110007e..2228508451 100644 --- a/flyteadmin/pkg/async/webhook/implementations/processer.go +++ b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go @@ -4,6 +4,8 @@ import ( "context" "encoding/base64" "encoding/json" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" + "github.com/golang/protobuf/proto" "time" "github.com/NYTimes/gizmo/pubsub" @@ -30,8 +32,8 @@ func (p *Processor) StartProcessing() { } func (p *Processor) run() error { + var payload admin.WebhookPayload var err error - for msg := range p.sub.Start() { p.systemMetrics.MessageTotal.Inc() // Currently this is safe because Gizmo takes a string and casts it to a byte array. @@ -69,7 +71,7 @@ func (p *Processor) run() error { // The Publish method for SNS Encodes the notification using Base64 then stringifies it before // setting that as the message body for SNS. Do the inverse to retrieve the notification. - messageBytes, err := base64.StdEncoding.DecodeString(valueString) + notificationBytes, err := base64.StdEncoding.DecodeString(valueString) if err != nil { logger.Errorf(context.Background(), "failed to Base64 decode from message string [%s] from message [%s] with err: %v", valueString, stringMsg, err) p.systemMetrics.MessageDecodingError.Inc() @@ -77,11 +79,18 @@ func (p *Processor) run() error { continue } + if err = proto.Unmarshal(notificationBytes, &payload); err != nil { + logger.Debugf(context.Background(), "failed to unmarshal to notification object from decoded string[%s] from message [%s] with err: %v", valueString, stringMsg, err) + p.systemMetrics.MessageDecodingError.Inc() + p.markMessageDone(msg) + continue + } + logger.Info(context.Background(), "Processor is sending message to webhook endpoint") // Send message to webhook - if err = p.webhook.Post(context.Background(), string(messageBytes)); err != nil { + if err = p.webhook.Post(context.Background(), payload); err != nil { p.systemMetrics.MessageProcessorError.Inc() - logger.Errorf(context.Background(), "Error sending an message [%s] to webhook endpoint with err: %v", string(messageBytes), err) + logger.Errorf(context.Background(), "Error sending an message [%v] to webhook endpoint with err: %v", payload, err) } else { p.systemMetrics.MessageSuccess.Inc() } diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index de4d431ded..ab261807c0 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "io/ioutil" "net/http" @@ -19,11 +20,11 @@ type SlackWebhook struct { systemMetrics webhookMetrics } -func (s *SlackWebhook) Post(ctx context.Context, message string) error { +func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) error { // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 - logger.Infof(ctx, "Posting to Slack with message: [%v]", message) + logger.Infof(ctx, "Posting to Slack with message: [%v]", payload.Message) webhookURL := s.config.URL - data := []byte(fmt.Sprintf("{'text': %s}", message)) + data := []byte(fmt.Sprintf("{'text': '%s'}", payload.Message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) if err != nil { logger.Errorf(ctx, "Failed to create request to Slack webhook with error: %v", err) diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go index 8c70d2286d..f3d460c297 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go @@ -2,6 +2,7 @@ package implementations import ( "context" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "testing" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" @@ -10,7 +11,7 @@ import ( ) func TestSlackWebhook(t *testing.T) { - webhook := NewSlackWebhook(runtimeInterfaces.WebhooksNotificationConfig{}, promutils.NewTestScope()) - err := webhook.Post(context.Background(), "message") + webhook := NewSlackWebhook(runtimeInterfaces.WebHookConfig{}, promutils.NewTestScope()) + err := webhook.Post(context.Background(), admin.WebhookPayload{Message: "hello world"}) assert.Nil(t, err) } diff --git a/flyteadmin/pkg/async/webhook/interfaces/webhook.go b/flyteadmin/pkg/async/webhook/interfaces/webhook.go index 153308edff..c84ff46cc2 100644 --- a/flyteadmin/pkg/async/webhook/interfaces/webhook.go +++ b/flyteadmin/pkg/async/webhook/interfaces/webhook.go @@ -2,6 +2,7 @@ package interfaces import ( "context" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" ) //go:generate mockery -name=Webhook -output=../mocks -case=underscore @@ -28,5 +29,5 @@ func (p Payload) ProtoMessage() { // Webhook Defines the interface for Publishing execution event to other services (Slack). type Webhook interface { // Post The notificationType is inferred from the Notification object in the Execution Spec. - Post(ctx context.Context, message string) error + Post(ctx context.Context, payload admin.WebhookPayload) error } diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index 86ae3d55be..026a061e06 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -1565,6 +1565,12 @@ func (m *ExecutionManager) publishWebhookNotifications(ctx context.Context, requ var notificationsList = adminExecution.Closure.Notifications logger.Debugf(ctx, "publishing notifications for execution [%+v] in state [%+v] for notifications [%+v]", request.Event.ExecutionId, request.Event.Phase, notificationsList) + // payloads[phase][name] = body + payloads := make(map[string]string) + for _, w := range m.config.ApplicationConfiguration().GetWebhookNotificationConfig().WebhooksConfig { + payloads[w.Name] = w.Payload + } + for _, notification := range notificationsList { // Check if the notification phase matches the current one. var matchPhase = false @@ -1579,14 +1585,12 @@ func (m *ExecutionManager) publishWebhookNotifications(ctx context.Context, requ continue } - var msg proto.Message + payload := &admin.WebhookPayload{Message: notifications.SubstituteParameters(payloads["slack"], request, adminExecution)} // TODO: Add message attributes - payload := notifications.SubstituteParameters("hello world", request, adminExecution) - err := proto.UnmarshalText(payload, msg) // Errors seen while publishing a message are considered non-fatal to the method and will not result // in the method returning an error. - if err = m.webhookClient.Publish(ctx, proto.MessageName(msg), msg); err != nil { + if err = m.webhookClient.Publish(ctx, "slack ", payload); err != nil { m.systemMetrics.PublishNotificationError.Inc() logger.Infof(ctx, "error publishing email notification [%+v] with err: [%v]", notification, err) } diff --git a/flyteadmin/pkg/runtime/application_config_provider.go b/flyteadmin/pkg/runtime/application_config_provider.go index 824312995b..d4dadffa65 100644 --- a/flyteadmin/pkg/runtime/application_config_provider.go +++ b/flyteadmin/pkg/runtime/application_config_provider.go @@ -64,7 +64,7 @@ var remoteDataConfig = config.MustRegisterSection(remoteData, &interfaces.Remote var notificationsConfig = config.MustRegisterSection(notifications, &interfaces.NotificationsConfig{ Type: common.Local, }) -var webhookNotificationsConfig = config.MustRegisterSection(webhookNotifications, &interfaces.WebhooksNotificationConfig{ +var webhookNotificationsConfig = config.MustRegisterSection(webhookNotifications, &interfaces.WebhookNotificationsConfig{ Type: common.Local, }) var domainsConfig = config.MustRegisterSection(domains, &interfaces.DomainsConfig{ @@ -124,8 +124,8 @@ func (p *ApplicationConfigurationProvider) GetCloudEventsConfig() *interfaces.Cl return cloudEventsConfig.GetConfig().(*interfaces.CloudEventsConfig) } -func (p *ApplicationConfigurationProvider) GetWebhookNotificationConfig() *interfaces.WebhooksNotificationConfig { - return webhookNotificationsConfig.GetConfig().(*interfaces.WebhooksNotificationConfig) +func (p *ApplicationConfigurationProvider) GetWebhookNotificationConfig() *interfaces.WebhookNotificationsConfig { + return webhookNotificationsConfig.GetConfig().(*interfaces.WebhookNotificationsConfig) } func NewApplicationConfigurationProvider() interfaces.ApplicationConfiguration { diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index 08ed8c303f..e03ae347db 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -561,8 +561,8 @@ type WebHookConfig struct { NotificationsProcessorConfig NotificationsProcessorConfig `json:"processor"` } -// WebhooksNotificationConfig defines the configuration for the webhook service. -type WebhooksNotificationConfig struct { +// WebhookNotificationsConfig defines the configuration for the webhook service. +type WebhookNotificationsConfig struct { // Defines the cloud provider that backs the scheduler. In the absence of a specification the no-op, 'local' // scheme is used. Type string `json:"type"` @@ -596,5 +596,5 @@ type ApplicationConfiguration interface { GetDomainsConfig() *DomainsConfig GetExternalEventsConfig() *ExternalEventsConfig GetCloudEventsConfig() *CloudEventsConfig - GetWebhookNotificationConfig() *WebhooksNotificationConfig + GetWebhookNotificationConfig() *WebhookNotificationsConfig } From 699adce22e202268c9e2c9d48e6fa3a381895cf2 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 20 Jun 2023 17:46:00 -0700 Subject: [PATCH 19/35] init Signed-off-by: Kevin Su --- .../pkg/async/notifications/email_test.go | 8 +- flyteadmin/pkg/async/notifications/factory.go | 15 ---- .../pkg/async/notifications/factory_test.go | 47 ------------ .../implementations/event_publisher.go | 1 - .../implementations/publisher.go | 1 + flyteadmin/pkg/async/webhook/factory.go | 10 ++- .../webhook/implementations/aws_processer.go | 51 +++++++++++-- .../webhook/implementations/slack_webhook.go | 22 +++++- .../pkg/async/webhook/interfaces/webhook.go | 17 +---- .../pkg/manager/impl/execution_manager.go | 76 +++---------------- flyteadmin/pkg/rpc/adminservice/base.go | 16 ++-- .../runtime/application_config_provider.go | 4 +- .../interfaces/application_configuration.go | 11 +-- 13 files changed, 102 insertions(+), 177 deletions(-) diff --git a/flyteadmin/pkg/async/notifications/email_test.go b/flyteadmin/pkg/async/notifications/email_test.go index 612cc4adfb..0829c5e330 100644 --- a/flyteadmin/pkg/async/notifications/email_test.go +++ b/flyteadmin/pkg/async/notifications/email_test.go @@ -59,14 +59,14 @@ func TestSubstituteEmailParameters(t *testing.T) { }, } assert.Equal(t, "{{ unused }}. {{project }} and prod and e124 ended up in succeeded.", - substituteEmailParameters(message, request, workflowExecution)) + SubstituteParameters(message, request, workflowExecution)) request.Event.OutputResult = &event.WorkflowExecutionEvent_Error{ Error: &core.ExecutionError{ Message: "uh-oh", }, } assert.Equal(t, "{{ unused }}. {{project }} and prod and e124 ended up in succeeded. The execution failed with error: [uh-oh].", - substituteEmailParameters(message, request, workflowExecution)) + SubstituteParameters(message, request, workflowExecution)) } func TestSubstituteAllTemplates(t *testing.T) { @@ -95,7 +95,7 @@ func TestSubstituteAllTemplates(t *testing.T) { }, } assert.Equal(t, strings.Join(desiredResult, ","), - substituteEmailParameters(strings.Join(messageTemplate, ","), request, workflowExecution)) + SubstituteParameters(strings.Join(messageTemplate, ","), request, workflowExecution)) } func TestSubstituteAllTemplatesNoSpaces(t *testing.T) { @@ -124,7 +124,7 @@ func TestSubstituteAllTemplatesNoSpaces(t *testing.T) { }, } assert.Equal(t, strings.Join(desiredResult, ","), - substituteEmailParameters(strings.Join(messageTemplate, ","), request, workflowExecution)) + SubstituteParameters(strings.Join(messageTemplate, ","), request, workflowExecution)) } func TestToEmailMessageFromWorkflowExecutionEvent(t *testing.T) { diff --git a/flyteadmin/pkg/async/notifications/factory.go b/flyteadmin/pkg/async/notifications/factory.go index 8febfb731b..836c1ff2fd 100644 --- a/flyteadmin/pkg/async/notifications/factory.go +++ b/flyteadmin/pkg/async/notifications/factory.go @@ -129,21 +129,6 @@ func NewNotificationsProcessor(config runtimeInterfaces.NotificationsConfig, sco } } -func NewWebhookNotificationsPublisher(config runtimeInterfaces.WebhookNotificationsConfig, scope promutils.Scope) interfaces.Publisher { - logger.Infof(context.Background(), "Starting webhook notification publisher [%s]", config.Type) - cfg := runtimeInterfaces.NotificationsConfig{ - Type: config.Type, - NotificationsPublisherConfig: runtimeInterfaces.NotificationsPublisherConfig{ - TopicName: config.NotificationsPublisherConfig.TopicName, - }, - AWSConfig: config.AWSConfig, - GCPConfig: config.GCPConfig, - ReconnectAttempts: config.ReconnectAttempts, - ReconnectDelaySeconds: config.ReconnectDelaySeconds, - } - return NewNotificationsPublisher(cfg, scope) -} - func NewNotificationsPublisher(config runtimeInterfaces.NotificationsConfig, scope promutils.Scope) interfaces.Publisher { reconnectAttempts := config.ReconnectAttempts reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second diff --git a/flyteadmin/pkg/async/notifications/factory_test.go b/flyteadmin/pkg/async/notifications/factory_test.go index dd3433c0de..280f383f2c 100644 --- a/flyteadmin/pkg/async/notifications/factory_test.go +++ b/flyteadmin/pkg/async/notifications/factory_test.go @@ -3,13 +3,6 @@ package notifications import ( "testing" - "github.com/NYTimes/gizmo/pubsub" - gizmoAWS "github.com/NYTimes/gizmo/pubsub/aws" - "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" - "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" - "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/event" - "golang.org/x/net/context" - runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flytestdlib/promutils" "github.com/stretchr/testify/assert" @@ -30,43 +23,3 @@ func TestGetEmailer(t *testing.T) { // shouldn't reach here t.Errorf("did not panic") } - -func TestAWSNotificationsPublisher(t *testing.T) { - cfg := runtimeInterfaces.NotificationsConfig{ - Type: "aws", - Region: "us-west-2", - NotificationsPublisherConfig: runtimeInterfaces.NotificationsPublisherConfig{ - TopicName: "arn:aws:sns:us-west-2:590375264460:webhook", - }, - } - var publisher pubsub.Publisher - var err error - snsConfig := gizmoAWS.SNSConfig{ - Topic: cfg.NotificationsPublisherConfig.TopicName, - } - if cfg.AWSConfig.Region != "" { - snsConfig.Region = cfg.AWSConfig.Region - } else { - snsConfig.Region = cfg.Region - } - publisher, err = gizmoAWS.NewPublisher(snsConfig) - assert.Nil(t, err) - - var executionID = core.WorkflowExecutionIdentifier{ - Project: "project", - Domain: "domain", - Name: "name", - } - - var workflowRequest = &admin.WorkflowExecutionEventRequest{ - Event: &event.WorkflowExecutionEvent{ - Phase: core.WorkflowExecution_SUCCEEDED, - OutputResult: &event.WorkflowExecutionEvent_OutputUri{ - OutputUri: "somestring", - }, - ExecutionId: &executionID, - }, - } - err = publisher.Publish(context.Background(), "test", workflowRequest) - assert.Nil(t, err) -} diff --git a/flyteadmin/pkg/async/notifications/implementations/event_publisher.go b/flyteadmin/pkg/async/notifications/implementations/event_publisher.go index 69d61c32a3..c505634354 100644 --- a/flyteadmin/pkg/async/notifications/implementations/event_publisher.go +++ b/flyteadmin/pkg/async/notifications/implementations/event_publisher.go @@ -55,7 +55,6 @@ func (p *EventPublisher) Publish(ctx context.Context, notificationType string, m } p.systemMetrics.PublishTotal.Inc() logger.Debugf(ctx, "Publishing the following message [%+v]", msg) - err := p.pub.Publish(ctx, notificationType, msg) if err != nil { p.systemMetrics.PublishError.Inc() diff --git a/flyteadmin/pkg/async/notifications/implementations/publisher.go b/flyteadmin/pkg/async/notifications/implementations/publisher.go index deadc82639..b8ec7047c5 100644 --- a/flyteadmin/pkg/async/notifications/implementations/publisher.go +++ b/flyteadmin/pkg/async/notifications/implementations/publisher.go @@ -2,6 +2,7 @@ package implementations import ( "context" + "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" "github.com/NYTimes/gizmo/pubsub" diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index 7b75d58b0d..f23e947da1 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -2,6 +2,7 @@ package webhook import ( "context" + repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" "time" "github.com/NYTimes/gizmo/pubsub" @@ -20,10 +21,13 @@ var enable64decoding = false func GetWebhook(config runtimeInterfaces.WebHookConfig, scope promutils.Scope) webhookInterfaces.Webhook { // TODO: Get others webhooks + if config.Name == "slack" { + return implementations.NewSlackWebhook(config, scope) + } return implementations.NewSlackWebhook(config, scope) } -func NewWebhookProcessors(config runtimeInterfaces.WebhookNotificationsConfig, scope promutils.Scope) []interfaces.Processor { +func NewWebhookProcessors(db repoInterfaces.Repository, config runtimeInterfaces.WebhookNotificationsConfig, scope promutils.Scope) []interfaces.Processor { reconnectAttempts := config.ReconnectAttempts reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second var sub pubsub.Subscriber @@ -44,7 +48,7 @@ func NewWebhookProcessors(config runtimeInterfaces.WebhookNotificationsConfig, s err = async.Retry(reconnectAttempts, reconnectDelay, func() error { sub, err = gizmoAWS.NewSubscriber(sqsConfig) if err != nil { - logger.Warnf(context.TODO(), "Failed to initialize new gizmo aws subscriber with config [%+v] and err: %v", sqsConfig, err) + logger.Errorf(context.TODO(), "Failed to initialize new gizmo aws subscriber with config [%+v] and err: %v", sqsConfig, err) } return err }) @@ -54,7 +58,7 @@ func NewWebhookProcessors(config runtimeInterfaces.WebhookNotificationsConfig, s } webhook := GetWebhook(cfg, scope) - processors = append(processors, implementations.NewWebhookProcessor(sub, webhook, scope)) + processors = append(processors, implementations.NewWebhookProcessor(sub, webhook, db, scope)) } return processors } diff --git a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go index 2228508451..d1ce1e3bf5 100644 --- a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go +++ b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go @@ -4,12 +4,17 @@ import ( "context" "encoding/base64" "encoding/json" + "github.com/flyteorg/flyteadmin/pkg/common" + "github.com/flyteorg/flyteadmin/pkg/manager/impl/util" + repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" + "github.com/flyteorg/flyteadmin/pkg/repositories/transformers" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/golang/protobuf/proto" "time" "github.com/NYTimes/gizmo/pubsub" "github.com/flyteorg/flyteadmin/pkg/async" + "github.com/flyteorg/flyteadmin/pkg/async/notifications" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" webhookInterfaces "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" "github.com/flyteorg/flytestdlib/logger" @@ -19,6 +24,7 @@ import ( type Processor struct { sub pubsub.Subscriber webhook webhookInterfaces.Webhook + db repoInterfaces.Repository systemMetrics processorSystemMetrics } @@ -33,12 +39,12 @@ func (p *Processor) StartProcessing() { func (p *Processor) run() error { var payload admin.WebhookPayload + var request admin.WorkflowExecutionEventRequest var err error for msg := range p.sub.Start() { p.systemMetrics.MessageTotal.Inc() // Currently this is safe because Gizmo takes a string and casts it to a byte array. stringMsg := string(msg.Message()) - var snsJSONFormat map[string]interface{} // At Lyft, SNS populates SQS. This results in the message body of SQS having the SNS message format. @@ -54,6 +60,9 @@ func (p *Processor) run() error { var value interface{} var ok bool var valueString string + var messageType string + + logger.Warningf(context.Background(), "snsJSONFormat: [%v]", snsJSONFormat) if value, ok = snsJSONFormat["Message"]; !ok { logger.Errorf(context.Background(), "failed to retrieve message from unmarshalled JSON object [%s]", stringMsg) @@ -69,9 +78,31 @@ func (p *Processor) run() error { continue } + if value, ok = snsJSONFormat["Subject"]; !ok { + logger.Errorf(context.Background(), "failed to retrieve message type from unmarshalled JSON object [%s]", stringMsg) + p.systemMetrics.MessageDataError.Inc() + p.markMessageDone(msg) + continue + } + + if messageType, ok = value.(string); !ok { + p.systemMetrics.MessageDataError.Inc() + logger.Errorf(context.Background(), "failed to retrieve notification message type (in string format) from unmarshalled JSON object for message [%s]", stringMsg) + p.markMessageDone(msg) + continue + } + logger.Warningf(context.Background(), "Message Subject is [%s]", messageType) + + // flyteidl.admin.WorkflowExecutionEventRequest + if messageType != proto.MessageName(&admin.WorkflowExecutionEventRequest{}) { + p.markMessageDone(msg) + logger.Warningf(context.Background(), "Message Subject [%v] is not flyteidl.admin.WorkflowExecutionEventRequest", messageType) + continue + } + // The Publish method for SNS Encodes the notification using Base64 then stringifies it before // setting that as the message body for SNS. Do the inverse to retrieve the notification. - notificationBytes, err := base64.StdEncoding.DecodeString(valueString) + requestBytes, err := base64.StdEncoding.DecodeString(valueString) if err != nil { logger.Errorf(context.Background(), "failed to Base64 decode from message string [%s] from message [%s] with err: %v", valueString, stringMsg, err) p.systemMetrics.MessageDecodingError.Inc() @@ -79,15 +110,22 @@ func (p *Processor) run() error { continue } - if err = proto.Unmarshal(notificationBytes, &payload); err != nil { - logger.Debugf(context.Background(), "failed to unmarshal to notification object from decoded string[%s] from message [%s] with err: %v", valueString, stringMsg, err) + if err = proto.Unmarshal(requestBytes, &request); err != nil { + logger.Errorf(context.Background(), "failed to unmarshal to notification object from decoded string[%s] from message [%s] with err: %v", valueString, stringMsg, err) p.systemMetrics.MessageDecodingError.Inc() p.markMessageDone(msg) continue } + if common.IsExecutionTerminal(request.Event.Phase) == false { + p.markMessageDone(msg) + continue + } + + executionModel, err := util.GetExecutionModel(context.Background(), p.db, *request.Event.ExecutionId) + adminExecution, err := transformers.FromExecutionModel(*executionModel, transformers.DefaultExecutionTransformerOptions) + payload.Message = notifications.SubstituteParameters(p.webhook.GetConfig().Payload, request, adminExecution) logger.Info(context.Background(), "Processor is sending message to webhook endpoint") - // Send message to webhook if err = p.webhook.Post(context.Background(), payload); err != nil { p.systemMetrics.MessageProcessorError.Inc() logger.Errorf(context.Background(), "Error sending an message [%v] to webhook endpoint with err: %v", payload, err) @@ -123,10 +161,11 @@ func (p *Processor) StopProcessing() error { return err } -func NewWebhookProcessor(sub pubsub.Subscriber, webhook webhookInterfaces.Webhook, scope promutils.Scope) interfaces.Processor { +func NewWebhookProcessor(sub pubsub.Subscriber, webhook webhookInterfaces.Webhook, db repoInterfaces.Repository, scope promutils.Scope) interfaces.Processor { return &Processor{ sub: sub, webhook: webhook, + db: db, systemMetrics: newProcessorSystemMetrics(scope.NewSubScope("webhook_processor")), } } diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index ab261807c0..e8badbb175 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" + "github.com/flyteorg/flytepropeller/pkg/controller/nodes/task/secretmanager" "io/ioutil" "net/http" @@ -16,14 +17,19 @@ import ( ) type SlackWebhook struct { - config runtimeInterfaces.WebHookConfig + Config runtimeInterfaces.WebHookConfig systemMetrics webhookMetrics } +func (s *SlackWebhook) GetConfig() runtimeInterfaces.WebHookConfig { + //TODO implement me + return s.Config +} + func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) error { // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 logger.Infof(ctx, "Posting to Slack with message: [%v]", payload.Message) - webhookURL := s.config.URL + webhookURL := s.Config.URL data := []byte(fmt.Sprintf("{'text': '%s'}", payload.Message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) if err != nil { @@ -31,6 +37,16 @@ func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) e return err } request.Header.Add("Content-Type", "application/json") + if len(s.Config.SecretName) != 0 { + sm := secretmanager.NewFileEnvSecretManager(secretmanager.GetConfig()) + token, err := sm.Get(ctx, s.Config.SecretName) + if err != nil { + logger.Errorf(ctx, "Failed to get secret from secret manager with error: %v", err) + return err + } + request.Header.Add("Authorization", "Bearer "+token) + } + client := &http.Client{} resp, err := client.Do(request) if err != nil { @@ -52,7 +68,7 @@ func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) e func NewSlackWebhook(config runtimeInterfaces.WebHookConfig, scope promutils.Scope) interfaces.Webhook { return &SlackWebhook{ - config: config, + Config: config, systemMetrics: newWebhookMetrics(scope.NewSubScope("slack_webhook")), } } diff --git a/flyteadmin/pkg/async/webhook/interfaces/webhook.go b/flyteadmin/pkg/async/webhook/interfaces/webhook.go index c84ff46cc2..04dfae7cdc 100644 --- a/flyteadmin/pkg/async/webhook/interfaces/webhook.go +++ b/flyteadmin/pkg/async/webhook/interfaces/webhook.go @@ -2,6 +2,7 @@ package interfaces import ( "context" + runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" ) @@ -11,23 +12,9 @@ type Payload struct { Value string `protobuf:"bytes,1,opt,value=value"` } -func (p Payload) Reset() { - //TODO implement me - panic("implement me") -} - -func (p Payload) String() string { - //TODO implement me - panic("implement me") -} - -func (p Payload) ProtoMessage() { - //TODO implement me - panic("implement me") -} - // Webhook Defines the interface for Publishing execution event to other services (Slack). type Webhook interface { // Post The notificationType is inferred from the Notification object in the Execution Spec. Post(ctx context.Context, payload admin.WebhookPayload) error + GetConfig() runtimeInterfaces.WebHookConfig } diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index 026a061e06..5cb5ea3429 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -90,7 +90,6 @@ type ExecutionManager struct { systemMetrics executionSystemMetrics userMetrics executionUserMetrics notificationClient notificationInterfaces.Publisher - webhookClient notificationInterfaces.Publisher urlData dataInterfaces.RemoteURLInterface workflowManager interfaces.WorkflowInterface namedEntityManager interfaces.NamedEntityInterface @@ -1284,24 +1283,18 @@ func (m *ExecutionManager) CreateWorkflowEvent(ctx context.Context, request admi if err != nil { // The only errors that publishNotifications will forward are those related // to unexpected data and transformation errors. - logger.Debugf(ctx, "failed to publish notifications for CreateWorkflowEvent [%+v] due to err: %v", - request, err) - return nil, err - } - err = m.publishWebhookNotifications(ctx, request, *executionModel) - if err != nil { - // The only errors that publishNotifications will forward are those related - // to unexpected data and transformation errors. - logger.Debugf(ctx, "failed to publish notifications for CreateWorkflowEvent [%+v] due to err: %v", + logger.Errorf(ctx, "failed to publish notifications for CreateWorkflowEvent [%+v] due to err: %v", request, err) return nil, err } } - if err := m.eventPublisher.Publish(ctx, proto.MessageName(&request), &request); err != nil { - m.systemMetrics.PublishEventError.Inc() - logger.Infof(ctx, "error publishing event [%+v] with err: [%v]", request.RequestId, err) - } + go func() { + if err := m.eventPublisher.Publish(ctx, proto.MessageName(&request), &request); err != nil { + m.systemMetrics.PublishEventError.Inc() + logger.Infof(ctx, "error publishing event [%+v] with err: [%v]", request.RequestId, err) + } + }() go func() { if err := m.cloudEventPublisher.Publish(ctx, proto.MessageName(&request), &request); err != nil { @@ -1539,58 +1532,13 @@ func (m *ExecutionManager) publishNotifications(ctx context.Context, request adm } // Convert the email Notification into an email message to be published. - // Currently, there are no possible errors while creating an email message. + // Currently there are no possible errors while creating an email message. // Once customizable content is specified, errors are possible. email := notifications.ToEmailMessageFromWorkflowExecutionEvent( *m.config.ApplicationConfiguration().GetNotificationsConfig(), emailNotification, request, adminExecution) - - // Errors seen while publishing a message are considered non-fatal to the method and will not result - // in the method returning an error. - if err = m.notificationClient.Publish(ctx, proto.MessageName(email), email); err != nil { - m.systemMetrics.PublishNotificationError.Inc() - logger.Infof(ctx, "error publishing email notification [%+v] with err: [%v]", notification, err) - } - } - return nil -} - -func (m *ExecutionManager) publishWebhookNotifications(ctx context.Context, request admin.WorkflowExecutionEventRequest, - execution models.Execution) error { - adminExecution, err := transformers.FromExecutionModel(execution, transformers.DefaultExecutionTransformerOptions) - if err != nil { - // This shouldn't happen because execution manager marshaled the data into models.Execution. - m.systemMetrics.TransformerError.Inc() - return errors.NewFlyteAdminErrorf(codes.Internal, "Failed to transform execution [%+v] with err: %v", request.Event.ExecutionId, err) - } - var notificationsList = adminExecution.Closure.Notifications - logger.Debugf(ctx, "publishing notifications for execution [%+v] in state [%+v] for notifications [%+v]", - request.Event.ExecutionId, request.Event.Phase, notificationsList) - // payloads[phase][name] = body - payloads := make(map[string]string) - for _, w := range m.config.ApplicationConfiguration().GetWebhookNotificationConfig().WebhooksConfig { - payloads[w.Name] = w.Payload - } - - for _, notification := range notificationsList { - // Check if the notification phase matches the current one. - var matchPhase = false - for _, phase := range notification.Phases { - if phase == request.Event.Phase { - matchPhase = true - } - } - - // The current phase doesn't match; no notifications will be sent for the current notification option. - if !matchPhase { - continue - } - - payload := &admin.WebhookPayload{Message: notifications.SubstituteParameters(payloads["slack"], request, adminExecution)} - // TODO: Add message attributes - // Errors seen while publishing a message are considered non-fatal to the method and will not result // in the method returning an error. - if err = m.webhookClient.Publish(ctx, "slack ", payload); err != nil { + if err = m.notificationClient.Publish(ctx, proto.MessageName(&emailNotification), email); err != nil { m.systemMetrics.PublishNotificationError.Inc() logger.Infof(ctx, "error publishing email notification [%+v] with err: [%v]", notification, err) } @@ -1679,8 +1627,7 @@ func newExecutionSystemMetrics(scope promutils.Scope) executionSystemMetrics { func NewExecutionManager(db repositoryInterfaces.Repository, pluginRegistry *plugins.Registry, config runtimeInterfaces.Configuration, storageClient *storage.DataStore, systemScope promutils.Scope, userScope promutils.Scope, - notificationPublisher notificationInterfaces.Publisher, publisher notificationInterfaces.Publisher, - urlData dataInterfaces.RemoteURLInterface, + publisher notificationInterfaces.Publisher, urlData dataInterfaces.RemoteURLInterface, workflowManager interfaces.WorkflowInterface, namedEntityManager interfaces.NamedEntityInterface, eventPublisher notificationInterfaces.Publisher, cloudEventPublisher cloudeventInterfaces.Publisher, eventWriter eventWriter.WorkflowExecutionEventWriter) interfaces.ExecutionInterface { @@ -1706,8 +1653,7 @@ func NewExecutionManager(db repositoryInterfaces.Repository, pluginRegistry *plu _clock: clock.New(), systemMetrics: systemMetrics, userMetrics: userMetrics, - notificationClient: notificationPublisher, - webhookClient: publisher, + notificationClient: publisher, urlData: urlData, workflowManager: workflowManager, namedEntityManager: namedEntityManager, diff --git a/flyteadmin/pkg/rpc/adminservice/base.go b/flyteadmin/pkg/rpc/adminservice/base.go index d4ccf95d5d..4046ea1a39 100644 --- a/flyteadmin/pkg/rpc/adminservice/base.go +++ b/flyteadmin/pkg/rpc/adminservice/base.go @@ -100,21 +100,21 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi logger.Info(ctx, "Successfully created a workflow executor engine") pluginRegistry.RegisterDefault(plugins.PluginIDWorkflowExecutor, workflowExecutor) - notificationPublisher := notifications.NewNotificationsPublisher(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) + publisher := notifications.NewNotificationsPublisher(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) processor := notifications.NewNotificationsProcessor(*configuration.ApplicationConfiguration().GetNotificationsConfig(), adminScope) + eventPublisher := notifications.NewEventsPublisher(*configuration.ApplicationConfiguration().GetExternalEventsConfig(), adminScope) + cloudEventPublisher := cloudevent.NewCloudEventsPublisher(ctx, *configuration.ApplicationConfiguration().GetCloudEventsConfig(), adminScope) go func() { logger.Info(ctx, "Started processing notifications.") processor.StartProcessing() }() - eventPublisher := notifications.NewEventsPublisher(*configuration.ApplicationConfiguration().GetExternalEventsConfig(), adminScope) - cloudEventPublisher := cloudevent.NewCloudEventsPublisher(ctx, *configuration.ApplicationConfiguration().GetCloudEventsConfig(), adminScope) - - publisher := notifications.NewWebhookNotificationsPublisher(*configuration.ApplicationConfiguration().GetWebhookNotificationConfig(), adminScope) - webhookProcessors := webhook.NewWebhookProcessors(*configuration.ApplicationConfiguration().GetWebhookNotificationConfig(), adminScope) + webhookProcessors := webhook.NewWebhookProcessors(repo, *configuration.ApplicationConfiguration().GetWebhookNotificationConfig(), adminScope) go func() { logger.Info(ctx, "Started processing webhook events.") - webhookProcessors[0].StartProcessing() + for _, webhookProcessor := range webhookProcessors { + webhookProcessor.StartProcessing() + } }() // Configure workflow scheduler async processes. @@ -153,7 +153,7 @@ func NewAdminServer(ctx context.Context, pluginRegistry *plugins.Registry, confi executionManager := manager.NewExecutionManager(repo, pluginRegistry, configuration, dataStorageClient, adminScope.NewSubScope("execution_manager"), adminScope.NewSubScope("user_execution_metrics"), - notificationPublisher, publisher, urlData, workflowManager, namedEntityManager, eventPublisher, cloudEventPublisher, executionEventWriter) + publisher, urlData, workflowManager, namedEntityManager, eventPublisher, cloudEventPublisher, executionEventWriter) versionManager := manager.NewVersionManager() scheduledWorkflowExecutor := workflowScheduler.GetWorkflowExecutor(executionManager, launchPlanManager) diff --git a/flyteadmin/pkg/runtime/application_config_provider.go b/flyteadmin/pkg/runtime/application_config_provider.go index d4dadffa65..a6694fbf9b 100644 --- a/flyteadmin/pkg/runtime/application_config_provider.go +++ b/flyteadmin/pkg/runtime/application_config_provider.go @@ -64,9 +64,7 @@ var remoteDataConfig = config.MustRegisterSection(remoteData, &interfaces.Remote var notificationsConfig = config.MustRegisterSection(notifications, &interfaces.NotificationsConfig{ Type: common.Local, }) -var webhookNotificationsConfig = config.MustRegisterSection(webhookNotifications, &interfaces.WebhookNotificationsConfig{ - Type: common.Local, -}) +var webhookNotificationsConfig = config.MustRegisterSection(webhookNotifications, &interfaces.WebhookNotificationsConfig{}) var domainsConfig = config.MustRegisterSection(domains, &interfaces.DomainsConfig{ { ID: "development", diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index e03ae347db..fb624578df 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -563,13 +563,10 @@ type WebHookConfig struct { // WebhookNotificationsConfig defines the configuration for the webhook service. type WebhookNotificationsConfig struct { - // Defines the cloud provider that backs the scheduler. In the absence of a specification the no-op, 'local' - // scheme is used. - Type string `json:"type"` - AWSConfig AWSConfig `json:"aws"` - GCPConfig GCPConfig `json:"gcp"` - NotificationsPublisherConfig NotificationsPublisherConfig `json:"publisher"` - WebhooksConfig []WebHookConfig `json:"webhooks"` + AWSConfig AWSConfig `json:"aws"` + GCPConfig GCPConfig `json:"gcp"` + // Defines the list of webhooks to be configured. + WebhooksConfig []WebHookConfig `json:"webhooks"` // Number of times to attempt recreating a notifications processor client should there be any disruptions. ReconnectAttempts int `json:"reconnectAttempts"` // Specifies the time interval to wait before attempting to reconnect the notifications processor client. From 1b3e8962610795e2a13c5eb3fde8227c6b0b7dde Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 20 Jun 2023 19:44:56 -0700 Subject: [PATCH 20/35] nit Signed-off-by: Kevin Su --- .../notifications/implementations/event_publisher.go | 1 + .../pkg/async/webhook/implementations/aws_processer.go | 10 +--------- .../pkg/async/webhook/implementations/slack_webhook.go | 2 -- flyteadmin/pkg/async/webhook/interfaces/webhook.go | 2 +- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/flyteadmin/pkg/async/notifications/implementations/event_publisher.go b/flyteadmin/pkg/async/notifications/implementations/event_publisher.go index c505634354..84d46ef4ff 100644 --- a/flyteadmin/pkg/async/notifications/implementations/event_publisher.go +++ b/flyteadmin/pkg/async/notifications/implementations/event_publisher.go @@ -54,6 +54,7 @@ func (p *EventPublisher) Publish(ctx context.Context, notificationType string, m return nil } p.systemMetrics.PublishTotal.Inc() + logger.Debugf(ctx, "Publishing the following message [%+v]", msg) err := p.pub.Publish(ctx, notificationType, msg) if err != nil { diff --git a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go index d1ce1e3bf5..45b60f9882 100644 --- a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go +++ b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go @@ -47,9 +47,6 @@ func (p *Processor) run() error { stringMsg := string(msg.Message()) var snsJSONFormat map[string]interface{} - // At Lyft, SNS populates SQS. This results in the message body of SQS having the SNS message format. - // The message format is documented here: https://docs.aws.amazon.com/sns/latest/dg/sns-message-and-json-formats.html - // The notification published is stored in the message field after unmarshalling the SQS message. if err := json.Unmarshal(msg.Message(), &snsJSONFormat); err != nil { p.systemMetrics.MessageDecodingError.Inc() logger.Errorf(context.Background(), "failed to unmarshall JSON message [%s] from processor with err: %v", stringMsg, err) @@ -62,8 +59,6 @@ func (p *Processor) run() error { var valueString string var messageType string - logger.Warningf(context.Background(), "snsJSONFormat: [%v]", snsJSONFormat) - if value, ok = snsJSONFormat["Message"]; !ok { logger.Errorf(context.Background(), "failed to retrieve message from unmarshalled JSON object [%s]", stringMsg) p.systemMetrics.MessageDataError.Inc() @@ -91,12 +86,9 @@ func (p *Processor) run() error { p.markMessageDone(msg) continue } - logger.Warningf(context.Background(), "Message Subject is [%s]", messageType) - // flyteidl.admin.WorkflowExecutionEventRequest if messageType != proto.MessageName(&admin.WorkflowExecutionEventRequest{}) { p.markMessageDone(msg) - logger.Warningf(context.Background(), "Message Subject [%v] is not flyteidl.admin.WorkflowExecutionEventRequest", messageType) continue } @@ -138,7 +130,7 @@ func (p *Processor) run() error { if err = p.sub.Err(); err != nil { p.systemMetrics.ChannelClosedError.Inc() - logger.Warningf(context.Background(), "The stream for the subscriber channel closed with err: %v", err) + logger.Errorf(context.Background(), "The stream for the subscriber channel closed with err: %v", err) } return err diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index e8badbb175..6aa073ce1b 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -27,8 +27,6 @@ func (s *SlackWebhook) GetConfig() runtimeInterfaces.WebHookConfig { } func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) error { - // curl -X POST -H 'Content-type: application/json' --data '{"text":"Hello, World!"}' https://hooks.slack.com/services/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 - logger.Infof(ctx, "Posting to Slack with message: [%v]", payload.Message) webhookURL := s.Config.URL data := []byte(fmt.Sprintf("{'text': '%s'}", payload.Message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) diff --git a/flyteadmin/pkg/async/webhook/interfaces/webhook.go b/flyteadmin/pkg/async/webhook/interfaces/webhook.go index 04dfae7cdc..1179c6cf50 100644 --- a/flyteadmin/pkg/async/webhook/interfaces/webhook.go +++ b/flyteadmin/pkg/async/webhook/interfaces/webhook.go @@ -12,7 +12,7 @@ type Payload struct { Value string `protobuf:"bytes,1,opt,value=value"` } -// Webhook Defines the interface for Publishing execution event to other services (Slack). +// Webhook Defines the interface for Publishing execution event to other services, such as slack. type Webhook interface { // Post The notificationType is inferred from the Notification object in the Execution Spec. Post(ctx context.Context, payload admin.WebhookPayload) error From 206cefc6232d006040cec13c081d3317525ffc07 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Tue, 20 Jun 2023 22:49:34 -0700 Subject: [PATCH 21/35] More tests Signed-off-by: Kevin Su --- .../implementations/event_publisher.go | 2 +- flyteadmin/pkg/async/webhook/factory.go | 10 +- flyteadmin/pkg/async/webhook/factory_test.go | 17 ++ .../implementations/aws_processor_test.go | 211 ++++++++++++++++++ .../implementations/slack_webhook_test.go | 8 +- .../pkg/async/webhook/mocks/processor.go | 50 +++++ .../pkg/async/webhook/mocks/publisher.go | 24 ++ flyteadmin/pkg/async/webhook/mocks/webhook.go | 23 ++ .../mocks/mock_application_provider.go | 21 +- 9 files changed, 349 insertions(+), 17 deletions(-) create mode 100644 flyteadmin/pkg/async/webhook/factory_test.go create mode 100644 flyteadmin/pkg/async/webhook/implementations/aws_processor_test.go create mode 100644 flyteadmin/pkg/async/webhook/mocks/processor.go create mode 100644 flyteadmin/pkg/async/webhook/mocks/publisher.go create mode 100644 flyteadmin/pkg/async/webhook/mocks/webhook.go diff --git a/flyteadmin/pkg/async/notifications/implementations/event_publisher.go b/flyteadmin/pkg/async/notifications/implementations/event_publisher.go index 84d46ef4ff..69d61c32a3 100644 --- a/flyteadmin/pkg/async/notifications/implementations/event_publisher.go +++ b/flyteadmin/pkg/async/notifications/implementations/event_publisher.go @@ -54,8 +54,8 @@ func (p *EventPublisher) Publish(ctx context.Context, notificationType string, m return nil } p.systemMetrics.PublishTotal.Inc() - logger.Debugf(ctx, "Publishing the following message [%+v]", msg) + err := p.pub.Publish(ctx, notificationType, msg) if err != nil { p.systemMetrics.PublishError.Inc() diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index f23e947da1..69db100bbf 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -17,14 +17,16 @@ import ( "github.com/flyteorg/flytestdlib/promutils" ) +const Slack = "slack" + var enable64decoding = false func GetWebhook(config runtimeInterfaces.WebHookConfig, scope promutils.Scope) webhookInterfaces.Webhook { // TODO: Get others webhooks - if config.Name == "slack" { + if config.Name == Slack { return implementations.NewSlackWebhook(config, scope) } - return implementations.NewSlackWebhook(config, scope) + return nil } func NewWebhookProcessors(db repoInterfaces.Repository, config runtimeInterfaces.WebhookNotificationsConfig, scope promutils.Scope) []interfaces.Processor { @@ -58,7 +60,9 @@ func NewWebhookProcessors(db repoInterfaces.Repository, config runtimeInterfaces } webhook := GetWebhook(cfg, scope) - processors = append(processors, implementations.NewWebhookProcessor(sub, webhook, db, scope)) + if webhook != nil { + processors = append(processors, implementations.NewWebhookProcessor(sub, webhook, db, scope)) + } } return processors } diff --git a/flyteadmin/pkg/async/webhook/factory_test.go b/flyteadmin/pkg/async/webhook/factory_test.go new file mode 100644 index 0000000000..eb45697075 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/factory_test.go @@ -0,0 +1,17 @@ +package webhook + +import ( + "testing" + + runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" + "github.com/flyteorg/flytestdlib/promutils" +) + +func TestGetWebhook(t *testing.T) { + cfg := runtimeInterfaces.WebHookConfig{ + Name: "unsupported", + } + GetWebhook(cfg, promutils.NewTestScope()) + cfg.Name = Slack + GetWebhook(cfg, promutils.NewTestScope()) +} diff --git a/flyteadmin/pkg/async/webhook/implementations/aws_processor_test.go b/flyteadmin/pkg/async/webhook/implementations/aws_processor_test.go new file mode 100644 index 0000000000..32d795f8e1 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/implementations/aws_processor_test.go @@ -0,0 +1,211 @@ +package implementations + +import ( + "context" + "encoding/base64" + "errors" + "testing" + "time" + + "github.com/NYTimes/gizmo/pubsub" + "github.com/NYTimes/gizmo/pubsub/pubsubtest" + "github.com/aws/aws-sdk-go/aws" + "github.com/flyteorg/flyteadmin/pkg/async/webhook/mocks" + "github.com/flyteorg/flyteadmin/pkg/manager/impl/testutils" + "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" + "github.com/flyteorg/flyteadmin/pkg/repositories/models" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/event" + "github.com/flyteorg/flytestdlib/promutils" + "github.com/golang/protobuf/proto" + "github.com/stretchr/testify/assert" + + repositoryMocks "github.com/flyteorg/flyteadmin/pkg/repositories/mocks" +) + +var ( + mockWebhook = mocks.MockWebhook{} + repo = repositoryMocks.NewMockRepository() + testWebhook = admin.WebhookPayload{Message: "hello world"} + workflowRequest = &admin.WorkflowExecutionEventRequest{ + Event: &event.WorkflowExecutionEvent{ + Phase: core.WorkflowExecution_FAILED, + ExecutionId: &core.WorkflowExecutionIdentifier{ + Project: "project", + Domain: "domain", + Name: "name", + }, + }, + } + msg, _ = proto.Marshal(workflowRequest) + testSubscriberMessage = map[string]interface{}{ + "Type": "Notification", + "MessageId": "1-a-3-c", + "TopicArn": "arn:aws:sns:my-region:123:flyte-test-notifications", + "Subject": "flyteidl.admin.WorkflowExecutionEventRequest", + "Message": aws.String(base64.StdEncoding.EncodeToString(msg)), + "Timestamp": "2019-01-04T22:59:32.849Z", + "SignatureVersion": "1", + "Signature": "some&ignature==", + "SigningCertURL": "https://sns.my-region.amazonaws.com/afdaf", + "UnsubscribeURL": "https://sns.my-region.amazonaws.com/sns:my-region:123:flyte-test-notifications:1-2-3-4-5"} + testSubscriber pubsubtest.TestSubscriber + mockSub pubsub.Subscriber = &testSubscriber +) + +// This method should be invoked before every test to Subscriber. +func initializeProcessor() { + testSubscriber.GivenStopError = nil + testSubscriber.GivenErrError = nil + testSubscriber.FoundError = nil + testSubscriber.ProtoMessages = nil + testSubscriber.JSONMessages = nil +} + +func TestProcessor_StartProcessing(t *testing.T) { + initializeProcessor() + testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testSubscriberMessage) + + sendWebhookValidationFunc := func(ctx context.Context, payload admin.WebhookPayload) error { + assert.Equal(t, payload.Message, testWebhook.Message) + return nil + } + mockWebhook.SetWebhookPostFunc(sendWebhookValidationFunc) + occurredAt := time.Now().UTC() + closure := &admin.ExecutionClosure{ + Phase: core.WorkflowExecution_RUNNING, + StateChangeDetails: &admin.ExecutionStateChangeDetails{ + State: admin.ExecutionState_EXECUTION_ACTIVE, + OccurredAt: testutils.MockCreatedAtProto, + }, + WorkflowId: &core.Identifier{ + Project: "project", + Domain: "domain", + Name: "name", + Version: "version", + }, + } + closureBytes, err := proto.Marshal(closure) + assert.Nil(t, err) + spec := &admin.ExecutionSpec{ + LaunchPlan: &core.Identifier{ + Project: "project", + Domain: "domain", + Name: "name", + Version: "version", + }, + } + specBytes, err := proto.Marshal(spec) + assert.Nil(t, err) + repo.ExecutionRepo().(*repositoryMocks.MockExecutionRepo).SetGetCallback( + func(ctx context.Context, input interfaces.Identifier) (models.Execution, error) { + return models.Execution{ + ExecutionKey: models.ExecutionKey{ + Project: "project", + Domain: "domain", + Name: "name", + }, + BaseModel: models.BaseModel{ + ID: uint(8), + }, + Spec: specBytes, + Phase: core.WorkflowExecution_SUCCEEDED.String(), + Closure: closureBytes, + LaunchPlanID: uint(1), + WorkflowID: uint(2), + StartedAt: &occurredAt, + }, nil + }, + ) + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Nil(t, testProcessor.(*Processor).run()) +} + +func TestProcessor_StartProcessingNoMessages(t *testing.T) { + initializeProcessor() + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Nil(t, testProcessor.(*Processor).run()) +} + +func TestProcessor_StartProcessingNoNotificationMessage(t *testing.T) { + var testMessage = map[string]interface{}{ + "Type": "Not a real notification", + "MessageId": "1234", + } + initializeProcessor() + testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testMessage) + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Nil(t, testProcessor.(*Processor).run()) +} + +func TestProcessor_StartProcessingMessageWrongDataType(t *testing.T) { + var testMessage = map[string]interface{}{ + "Type": "Not a real notification", + "MessageId": "1234", + "Message": 12, + } + initializeProcessor() + testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testMessage) + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Nil(t, testProcessor.(*Processor).run()) +} + +func TestProcessor_StartProcessingBase64DecodeError(t *testing.T) { + var testMessage = map[string]interface{}{ + "Type": "Not a real notification", + "MessageId": "1234", + "Message": "NotBase64encoded", + } + initializeProcessor() + testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testMessage) + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Nil(t, testProcessor.(*Processor).run()) +} + +func TestProcessor_StartProcessingProtoMarshallError(t *testing.T) { + var badByte = []byte("atreyu") + var testMessage = map[string]interface{}{ + "Type": "Not a real notification", + "MessageId": "1234", + "Message": aws.String(base64.StdEncoding.EncodeToString(badByte)), + } + initializeProcessor() + testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testMessage) + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Nil(t, testProcessor.(*Processor).run()) +} + +func TestProcessor_StartProcessingError(t *testing.T) { + initializeProcessor() + var ret = errors.New("err() returned an error") + testSubscriber.GivenErrError = ret + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Equal(t, ret, testProcessor.(*Processor).run()) +} + +func TestProcessor_StartProcessingWebhookError(t *testing.T) { + initializeProcessor() + webhookError := errors.New("webhook error") + sendWebhookErrorFunc := func(ctx context.Context, payload admin.WebhookPayload) error { + return webhookError + } + mockWebhook.SetWebhookPostFunc(sendWebhookErrorFunc) + testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testSubscriberMessage) + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Nil(t, testProcessor.(*Processor).run()) +} + +func TestProcessor_StopProcessing(t *testing.T) { + initializeProcessor() + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Nil(t, testProcessor.StopProcessing()) +} + +func TestProcessor_StopProcessingError(t *testing.T) { + initializeProcessor() + var stopError = errors.New("stop() returns an error") + testSubscriber.GivenStopError = stopError + testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + assert.Equal(t, stopError, testProcessor.StopProcessing()) +} diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go index f3d460c297..c5e61a3b52 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go @@ -1,8 +1,6 @@ package implementations import ( - "context" - "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "testing" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" @@ -11,7 +9,7 @@ import ( ) func TestSlackWebhook(t *testing.T) { - webhook := NewSlackWebhook(runtimeInterfaces.WebHookConfig{}, promutils.NewTestScope()) - err := webhook.Post(context.Background(), admin.WebhookPayload{Message: "hello world"}) - assert.Nil(t, err) + cfg := runtimeInterfaces.WebHookConfig{Name: "slack"} + webhook := NewSlackWebhook(cfg, promutils.NewTestScope()) + assert.Equal(t, webhook.GetConfig().Name, cfg.Name) } diff --git a/flyteadmin/pkg/async/webhook/mocks/processor.go b/flyteadmin/pkg/async/webhook/mocks/processor.go new file mode 100644 index 0000000000..0a46b19c85 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/mocks/processor.go @@ -0,0 +1,50 @@ +package mocks + +import ( + "context" + runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" + + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" +) + +type RunFunc func() error + +type StopFunc func() error + +type MockSubscriber struct { + runFunc RunFunc + stopFunc StopFunc +} + +func (m *MockSubscriber) Run() error { + if m.runFunc != nil { + return m.runFunc() + } + return nil +} + +func (m *MockSubscriber) Stop() error { + if m.stopFunc != nil { + return m.stopFunc() + } + return nil +} + +type MockWebhook struct { + post WebhookPostFunc +} + +func (m *MockWebhook) GetConfig() runtimeInterfaces.WebHookConfig { + return runtimeInterfaces.WebHookConfig{Payload: "hello world"} +} + +func (m *MockWebhook) SetWebhookPostFunc(webhookPostFunc WebhookPostFunc) { + m.post = webhookPostFunc +} + +func (m *MockWebhook) Post(ctx context.Context, payload admin.WebhookPayload) error { + if m.post != nil { + return m.post(ctx, payload) + } + return nil +} diff --git a/flyteadmin/pkg/async/webhook/mocks/publisher.go b/flyteadmin/pkg/async/webhook/mocks/publisher.go new file mode 100644 index 0000000000..ccfa041eb1 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/mocks/publisher.go @@ -0,0 +1,24 @@ +package mocks + +import ( + "context" + + "github.com/golang/protobuf/proto" +) + +type PublishFunc func(ctx context.Context, key string, msg proto.Message) error + +type MockPublisher struct { + publishFunc PublishFunc +} + +func (m *MockPublisher) SetPublishCallback(publishFunction PublishFunc) { + m.publishFunc = publishFunction +} + +func (m *MockPublisher) Publish(ctx context.Context, notificationType string, msg proto.Message) error { + if m.publishFunc != nil { + return m.publishFunc(ctx, notificationType, msg) + } + return nil +} diff --git a/flyteadmin/pkg/async/webhook/mocks/webhook.go b/flyteadmin/pkg/async/webhook/mocks/webhook.go new file mode 100644 index 0000000000..a128cb07a9 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/mocks/webhook.go @@ -0,0 +1,23 @@ +package mocks + +import ( + "context" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" +) + +type WebhookPostFunc func(ctx context.Context, payload admin.WebhookPayload) error + +type Webhook struct { + post WebhookPostFunc +} + +func (m *Webhook) SetWebhookPostFunc(webhookPostFunc WebhookPostFunc) { + m.post = webhookPostFunc +} + +func (m *Webhook) Post(ctx context.Context, payload admin.WebhookPayload) error { + if m.post != nil { + return m.post(ctx, payload) + } + return nil +} diff --git a/flyteadmin/pkg/runtime/mocks/mock_application_provider.go b/flyteadmin/pkg/runtime/mocks/mock_application_provider.go index 13079619c1..f32c13b495 100644 --- a/flyteadmin/pkg/runtime/mocks/mock_application_provider.go +++ b/flyteadmin/pkg/runtime/mocks/mock_application_provider.go @@ -6,14 +6,19 @@ import ( ) type MockApplicationProvider struct { - dbConfig database.DbConfig - topLevelConfig interfaces.ApplicationConfig - schedulerConfig interfaces.SchedulerConfig - remoteDataConfig interfaces.RemoteDataConfig - notificationsConfig interfaces.NotificationsConfig - domainsConfig interfaces.DomainsConfig - externalEventsConfig interfaces.ExternalEventsConfig - cloudEventConfig interfaces.CloudEventsConfig + dbConfig database.DbConfig + topLevelConfig interfaces.ApplicationConfig + schedulerConfig interfaces.SchedulerConfig + remoteDataConfig interfaces.RemoteDataConfig + notificationsConfig interfaces.NotificationsConfig + webhookNotificationsConfig interfaces.WebhookNotificationsConfig + domainsConfig interfaces.DomainsConfig + externalEventsConfig interfaces.ExternalEventsConfig + cloudEventConfig interfaces.CloudEventsConfig +} + +func (p *MockApplicationProvider) GetWebhookNotificationConfig() *interfaces.WebhookNotificationsConfig { + return &p.webhookNotificationsConfig } func (p *MockApplicationProvider) GetDbConfig() *database.DbConfig { From 83d7c3ae8e90afc874bb0c02687248599150f0db Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 5 Jul 2023 07:02:32 -0700 Subject: [PATCH 22/35] bump idl Signed-off-by: Kevin Su --- flyteadmin/go.mod | 4 ++-- flyteadmin/go.sum | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/flyteadmin/go.mod b/flyteadmin/go.mod index 10753d139d..fbfb21c532 100644 --- a/flyteadmin/go.mod +++ b/flyteadmin/go.mod @@ -46,7 +46,6 @@ require ( github.com/spf13/cobra v1.4.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.0 - golang.org/x/net v0.7.0 golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 google.golang.org/api v0.76.0 @@ -170,6 +169,7 @@ require ( go.opencensus.io v0.23.0 // indirect golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa // indirect golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect + golang.org/x/net v0.7.0 // indirect golang.org/x/sync v0.1.0 // indirect golang.org/x/sys v0.5.0 // indirect golang.org/x/term v0.5.0 // indirect @@ -214,4 +214,4 @@ replace github.com/robfig/cron/v3 => github.com/unionai/cron/v3 v3.0.2-0.2022091 // This was published in error when attempting to create 1.5.1 Flyte release. retract v1.1.94 -replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.5.8-0.20230525232442-053ca98a8450 +replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.5.13-0.20230705131852-ba27680aa8b3 diff --git a/flyteadmin/go.sum b/flyteadmin/go.sum index 264e4fc4ee..5e065a2e86 100644 --- a/flyteadmin/go.sum +++ b/flyteadmin/go.sum @@ -312,8 +312,8 @@ github.com/fatih/structs v1.0.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ= github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= -github.com/flyteorg/flyteidl v1.5.8-0.20230525232442-053ca98a8450 h1:mlu/s3sCGf3/3diKrQn06OvWdroM2ErhqnXm17I8vLY= -github.com/flyteorg/flyteidl v1.5.8-0.20230525232442-053ca98a8450/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= +github.com/flyteorg/flyteidl v1.5.13-0.20230705131852-ba27680aa8b3 h1:UmrrH3OVeyiCuE+R6v+Ka8mDOuyfTlnajf2TYWS+9UM= +github.com/flyteorg/flyteidl v1.5.13-0.20230705131852-ba27680aa8b3/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= github.com/flyteorg/flyteplugins v1.0.56 h1:kBTDgTpdSi7wcptk4cMwz5vfh1MU82VaUMMboe1InXw= github.com/flyteorg/flyteplugins v1.0.56/go.mod h1:aFCKSn8TPzxSAILIiogHtUnHlUCN9+y6Vf+r9R4KZDU= github.com/flyteorg/flytepropeller v1.1.87 h1:Px7ASDjrWyeVrUb15qXmhw9QK7xPcFjL5Yetr2P6iGM= From cd07b2a2cca92dea0a342e21859cffd7b8f25e44 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 5 Jul 2023 10:32:57 -0700 Subject: [PATCH 23/35] nit Signed-off-by: Kevin Su --- flyteadmin/pkg/async/webhook/factory.go | 9 +++++---- .../pkg/async/webhook/implementations/aws_processer.go | 2 +- .../pkg/async/webhook/implementations/slack_webhook.go | 4 +++- .../async/webhook/implementations/slack_webhook_test.go | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index 69db100bbf..634b0b37f3 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -2,6 +2,7 @@ package webhook import ( "context" + "fmt" repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" "time" @@ -17,14 +18,14 @@ import ( "github.com/flyteorg/flytestdlib/promutils" ) -const Slack = "slack" - var enable64decoding = false func GetWebhook(config runtimeInterfaces.WebHookConfig, scope promutils.Scope) webhookInterfaces.Webhook { - // TODO: Get others webhooks - if config.Name == Slack { + switch config.Name { + case implementations.Slack: return implementations.NewSlackWebhook(config, scope) + default: + panic(fmt.Errorf("no matching webhook implementation for %s", config.Name)) } return nil } diff --git a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go index 45b60f9882..03f92855ee 100644 --- a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go +++ b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go @@ -115,7 +115,7 @@ func (p *Processor) run() error { } executionModel, err := util.GetExecutionModel(context.Background(), p.db, *request.Event.ExecutionId) - adminExecution, err := transformers.FromExecutionModel(*executionModel, transformers.DefaultExecutionTransformerOptions) + adminExecution, err := transformers.FromExecutionModel(context.Background(), *executionModel, transformers.DefaultExecutionTransformerOptions) payload.Message = notifications.SubstituteParameters(p.webhook.GetConfig().Payload, request, adminExecution) logger.Info(context.Background(), "Processor is sending message to webhook endpoint") if err = p.webhook.Post(context.Background(), payload); err != nil { diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index 6aa073ce1b..ef6fc198f5 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -16,17 +16,19 @@ import ( "github.com/flyteorg/flytestdlib/promutils" ) +const Slack = "slack" + type SlackWebhook struct { Config runtimeInterfaces.WebHookConfig systemMetrics webhookMetrics } func (s *SlackWebhook) GetConfig() runtimeInterfaces.WebHookConfig { - //TODO implement me return s.Config } func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) error { + // TODO: we should read the webhook URL from the secret webhookURL := s.Config.URL data := []byte(fmt.Sprintf("{'text': '%s'}", payload.Message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go index c5e61a3b52..ab21da36d0 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook_test.go @@ -9,7 +9,7 @@ import ( ) func TestSlackWebhook(t *testing.T) { - cfg := runtimeInterfaces.WebHookConfig{Name: "slack"} + cfg := runtimeInterfaces.WebHookConfig{Name: Slack} webhook := NewSlackWebhook(cfg, promutils.NewTestScope()) assert.Equal(t, webhook.GetConfig().Name, cfg.Name) } From 4343654fc33dc491086ecc4a179f5692b2c0584a Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 5 Jul 2023 23:17:19 -0700 Subject: [PATCH 24/35] nit Signed-off-by: Kevin Su --- flyteadmin/pkg/async/webhook/factory_test.go | 6 +++--- .../webhook/implementations/aws_processer.go | 15 ++++++++++++++- .../pkg/runtime/application_config_provider.go | 1 - 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/flyteadmin/pkg/async/webhook/factory_test.go b/flyteadmin/pkg/async/webhook/factory_test.go index eb45697075..be078e35a9 100644 --- a/flyteadmin/pkg/async/webhook/factory_test.go +++ b/flyteadmin/pkg/async/webhook/factory_test.go @@ -4,14 +4,14 @@ import ( "testing" runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" + + "github.com/flyteorg/flyteadmin/pkg/async/webhook/implementations" "github.com/flyteorg/flytestdlib/promutils" ) func TestGetWebhook(t *testing.T) { cfg := runtimeInterfaces.WebHookConfig{ - Name: "unsupported", + Name: implementations.Slack, } GetWebhook(cfg, promutils.NewTestScope()) - cfg.Name = Slack - GetWebhook(cfg, promutils.NewTestScope()) } diff --git a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go index 03f92855ee..cad281809e 100644 --- a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go +++ b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go @@ -109,13 +109,26 @@ func (p *Processor) run() error { continue } - if common.IsExecutionTerminal(request.Event.Phase) == false { + if !common.IsExecutionTerminal(request.Event.Phase) { p.markMessageDone(msg) continue } executionModel, err := util.GetExecutionModel(context.Background(), p.db, *request.Event.ExecutionId) + if err != nil { + p.systemMetrics.MessageProcessorError.Inc() + logger.Errorf(context.Background(), "failed to retrieve execution model for execution [%+v] from message [%s] with err: %v", request.Event.ExecutionId, stringMsg, err) + p.markMessageDone(msg) + continue + } adminExecution, err := transformers.FromExecutionModel(context.Background(), *executionModel, transformers.DefaultExecutionTransformerOptions) + if err != nil { + p.systemMetrics.MessageProcessorError.Inc() + logger.Errorf(context.Background(), "failed to transform execution model [%+v] from message [%s] with err: %v", executionModel, stringMsg, err) + p.markMessageDone(msg) + continue + } + payload.Message = notifications.SubstituteParameters(p.webhook.GetConfig().Payload, request, adminExecution) logger.Info(context.Background(), "Processor is sending message to webhook endpoint") if err = p.webhook.Post(context.Background(), payload); err != nil { diff --git a/flyteadmin/pkg/runtime/application_config_provider.go b/flyteadmin/pkg/runtime/application_config_provider.go index a6694fbf9b..578f072079 100644 --- a/flyteadmin/pkg/runtime/application_config_provider.go +++ b/flyteadmin/pkg/runtime/application_config_provider.go @@ -16,7 +16,6 @@ const webhookNotifications = "webhookNotifications" const domains = "domains" const externalEvents = "externalEvents" const cloudEvents = "cloudEvents" -const webhook = "webhook" const metricPort = 10254 const KB = 1024 From 96e663d470c010cd416edec0c061599f58bcbcdf Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 5 Jul 2023 23:20:59 -0700 Subject: [PATCH 25/35] make generate Signed-off-by: Kevin Su --- flyteadmin/pkg/async/webhook/mocks/webhook.go | 81 ++++++++++++++++--- 1 file changed, 70 insertions(+), 11 deletions(-) diff --git a/flyteadmin/pkg/async/webhook/mocks/webhook.go b/flyteadmin/pkg/async/webhook/mocks/webhook.go index a128cb07a9..b0f84498cd 100644 --- a/flyteadmin/pkg/async/webhook/mocks/webhook.go +++ b/flyteadmin/pkg/async/webhook/mocks/webhook.go @@ -1,23 +1,82 @@ +// Code generated by mockery v1.0.1. DO NOT EDIT. + package mocks import ( - "context" - "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" -) + context "context" -type WebhookPostFunc func(ctx context.Context, payload admin.WebhookPayload) error + admin "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" + interfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" + + mock "github.com/stretchr/testify/mock" +) + +// Webhook is an autogenerated mock type for the Webhook type type Webhook struct { - post WebhookPostFunc + mock.Mock +} + +type Webhook_GetConfig struct { + *mock.Call +} + +func (_m Webhook_GetConfig) Return(_a0 interfaces.WebHookConfig) *Webhook_GetConfig { + return &Webhook_GetConfig{Call: _m.Call.Return(_a0)} +} + +func (_m *Webhook) OnGetConfig() *Webhook_GetConfig { + c_call := _m.On("GetConfig") + return &Webhook_GetConfig{Call: c_call} +} + +func (_m *Webhook) OnGetConfigMatch(matchers ...interface{}) *Webhook_GetConfig { + c_call := _m.On("GetConfig", matchers...) + return &Webhook_GetConfig{Call: c_call} } -func (m *Webhook) SetWebhookPostFunc(webhookPostFunc WebhookPostFunc) { - m.post = webhookPostFunc +// GetConfig provides a mock function with given fields: +func (_m *Webhook) GetConfig() interfaces.WebHookConfig { + ret := _m.Called() + + var r0 interfaces.WebHookConfig + if rf, ok := ret.Get(0).(func() interfaces.WebHookConfig); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(interfaces.WebHookConfig) + } + + return r0 +} + +type Webhook_Post struct { + *mock.Call +} + +func (_m Webhook_Post) Return(_a0 error) *Webhook_Post { + return &Webhook_Post{Call: _m.Call.Return(_a0)} } -func (m *Webhook) Post(ctx context.Context, payload admin.WebhookPayload) error { - if m.post != nil { - return m.post(ctx, payload) +func (_m *Webhook) OnPost(ctx context.Context, payload admin.WebhookPayload) *Webhook_Post { + c_call := _m.On("Post", ctx, payload) + return &Webhook_Post{Call: c_call} +} + +func (_m *Webhook) OnPostMatch(matchers ...interface{}) *Webhook_Post { + c_call := _m.On("Post", matchers...) + return &Webhook_Post{Call: c_call} +} + +// Post provides a mock function with given fields: ctx, payload +func (_m *Webhook) Post(ctx context.Context, payload admin.WebhookPayload) error { + ret := _m.Called(ctx, payload) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, admin.WebhookPayload) error); ok { + r0 = rf(ctx, payload) + } else { + r0 = ret.Error(0) } - return nil + + return r0 } From 6cee8cb83700c1b7c9de104e5a902625f3be061e Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Thu, 6 Jul 2023 00:07:58 -0700 Subject: [PATCH 26/35] lint Signed-off-by: Kevin Su --- flyteadmin/pkg/async/webhook/factory.go | 3 ++- .../pkg/async/webhook/implementations/aws_processer.go | 3 ++- .../pkg/async/webhook/implementations/slack_webhook.go | 5 +++-- flyteadmin/pkg/async/webhook/interfaces/webhook.go | 1 + flyteadmin/pkg/async/webhook/mocks/processor.go | 3 +++ 5 files changed, 11 insertions(+), 4 deletions(-) diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index 634b0b37f3..14842fc3a3 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -3,9 +3,10 @@ package webhook import ( "context" "fmt" - repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" "time" + repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" + "github.com/NYTimes/gizmo/pubsub" gizmoAWS "github.com/NYTimes/gizmo/pubsub/aws" "github.com/flyteorg/flyteadmin/pkg/async" diff --git a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go index cad281809e..1d363c0953 100644 --- a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go +++ b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go @@ -4,13 +4,14 @@ import ( "context" "encoding/base64" "encoding/json" + "time" + "github.com/flyteorg/flyteadmin/pkg/common" "github.com/flyteorg/flyteadmin/pkg/manager/impl/util" repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" "github.com/flyteorg/flyteadmin/pkg/repositories/transformers" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/golang/protobuf/proto" - "time" "github.com/NYTimes/gizmo/pubsub" "github.com/flyteorg/flyteadmin/pkg/async" diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index ef6fc198f5..6ad9466144 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -4,11 +4,12 @@ import ( "bytes" "context" "fmt" - "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" - "github.com/flyteorg/flytepropeller/pkg/controller/nodes/task/secretmanager" "io/ioutil" "net/http" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" + "github.com/flyteorg/flytepropeller/pkg/controller/nodes/task/secretmanager" + "github.com/flyteorg/flytestdlib/logger" "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" diff --git a/flyteadmin/pkg/async/webhook/interfaces/webhook.go b/flyteadmin/pkg/async/webhook/interfaces/webhook.go index 1179c6cf50..95deca0505 100644 --- a/flyteadmin/pkg/async/webhook/interfaces/webhook.go +++ b/flyteadmin/pkg/async/webhook/interfaces/webhook.go @@ -2,6 +2,7 @@ package interfaces import ( "context" + runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" ) diff --git a/flyteadmin/pkg/async/webhook/mocks/processor.go b/flyteadmin/pkg/async/webhook/mocks/processor.go index 0a46b19c85..dbc6713b04 100644 --- a/flyteadmin/pkg/async/webhook/mocks/processor.go +++ b/flyteadmin/pkg/async/webhook/mocks/processor.go @@ -2,6 +2,7 @@ package mocks import ( "context" + runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" @@ -11,6 +12,8 @@ type RunFunc func() error type StopFunc func() error +type WebhookPostFunc func(ctx context.Context, payload admin.WebhookPayload) error + type MockSubscriber struct { runFunc RunFunc stopFunc StopFunc From 412f8cc61d5156a9ce80660674f79ff0ba446482 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Thu, 6 Jul 2023 02:26:14 -0700 Subject: [PATCH 27/35] lint Signed-off-by: Kevin Su --- flyteadmin/pkg/async/webhook/factory.go | 1 - 1 file changed, 1 deletion(-) diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index 14842fc3a3..a527f73f98 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -28,7 +28,6 @@ func GetWebhook(config runtimeInterfaces.WebHookConfig, scope promutils.Scope) w default: panic(fmt.Errorf("no matching webhook implementation for %s", config.Name)) } - return nil } func NewWebhookProcessors(db repoInterfaces.Repository, config runtimeInterfaces.WebhookNotificationsConfig, scope promutils.Scope) []interfaces.Processor { From 99c39c0aa96e2e9893c06af0eb94d661fce36694 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Wed, 19 Jul 2023 11:24:16 -0700 Subject: [PATCH 28/35] Read webhook url from secret Signed-off-by: Kevin Su --- .../webhook/implementations/slack_webhook.go | 15 +++++++++------ .../interfaces/application_configuration.go | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index 6ad9466144..be4f13e726 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -29,8 +29,12 @@ func (s *SlackWebhook) GetConfig() runtimeInterfaces.WebHookConfig { } func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) error { - // TODO: we should read the webhook URL from the secret - webhookURL := s.Config.URL + sm := secretmanager.NewFileEnvSecretManager(secretmanager.GetConfig()) + webhookURL, err := sm.Get(ctx, s.Config.URL) + if err != nil { + logger.Errorf(ctx, "Failed to get url from secret manager with error: %v", err) + return err + } data := []byte(fmt.Sprintf("{'text': '%s'}", payload.Message)) request, err := http.NewRequest("POST", webhookURL, bytes.NewBuffer(data)) if err != nil { @@ -38,11 +42,10 @@ func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) e return err } request.Header.Add("Content-Type", "application/json") - if len(s.Config.SecretName) != 0 { - sm := secretmanager.NewFileEnvSecretManager(secretmanager.GetConfig()) - token, err := sm.Get(ctx, s.Config.SecretName) + if len(s.Config.Token) != 0 { + token, err := sm.Get(ctx, s.Config.Token) if err != nil { - logger.Errorf(ctx, "Failed to get secret from secret manager with error: %v", err) + logger.Errorf(ctx, "Failed to get bearer token from secret manager with error: %v", err) return err } request.Header.Add("Authorization", "Bearer "+token) diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index fb624578df..55979a8d29 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -555,9 +555,9 @@ type NotificationsConfig struct { type WebHookConfig struct { // Type of webhook service to use. Currently only "slack" is supported. Name string `json:"name"` - URL string `json:"url"` + URL string `json:"url" pflag:",Secret that contains the webhook URL"` Payload string `json:"payload"` - SecretName string `json:"secret"` + Token string `json:"token" pflag:",Secret that contains the bearer token"` NotificationsProcessorConfig NotificationsProcessorConfig `json:"processor"` } From 868ce14cb2cd7bb493cdc7d42aeefb87d0000d37 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Thu, 20 Jul 2023 12:46:48 -0700 Subject: [PATCH 29/35] add gcp processor Signed-off-by: Kevin Su --- .../implementations/aws_processor.go | 111 +++-------- .../implementations/gcp_processor.go | 61 ++---- .../notifications/interfaces/processor.go | 123 ++++++++++++ .../processor_metrics.go | 8 +- flyteadmin/pkg/async/webhook/factory.go | 36 +++- .../webhook/implementations/aws_processer.go | 177 ------------------ .../webhook/implementations/processer.go | 125 +++++++++++++ .../implementations/processor_metrics.go | 32 ---- ...ws_processor_test.go => processor_test.go} | 21 ++- .../runtime/application_config_provider.go | 4 +- .../interfaces/application_configuration.go | 3 + 11 files changed, 335 insertions(+), 366 deletions(-) rename flyteadmin/pkg/async/notifications/{implementations => interfaces}/processor_metrics.go (90%) delete mode 100644 flyteadmin/pkg/async/webhook/implementations/aws_processer.go create mode 100644 flyteadmin/pkg/async/webhook/implementations/processer.go delete mode 100644 flyteadmin/pkg/async/webhook/implementations/processor_metrics.go rename flyteadmin/pkg/async/webhook/implementations/{aws_processor_test.go => processor_test.go} (85%) diff --git a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go index 2cbf6d971a..4d3475b968 100644 --- a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go +++ b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go @@ -2,12 +2,7 @@ package implementations import ( "context" - "encoding/base64" - "encoding/json" - "time" - "github.com/NYTimes/gizmo/pubsub" - "github.com/flyteorg/flyteadmin/pkg/async" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flytestdlib/logger" @@ -17,94 +12,45 @@ import ( // TODO: Add a counter that encompasses the publisher stats grouped by project and domain. type Processor struct { - sub pubsub.Subscriber - email interfaces.Emailer - systemMetrics processorSystemMetrics -} - -// Currently only email is the supported notification because slack and pagerduty both use -// email client to trigger those notifications. -// When Pagerduty and other notifications are supported, a publisher per type should be created. -func (p *Processor) StartProcessing() { - for { - logger.Warningf(context.Background(), "Starting notifications processor") - err := p.run() - logger.Errorf(context.Background(), "error with running processor err: [%v] ", err) - time.Sleep(async.RetryDelay) - } + email interfaces.Emailer + interfaces.BaseProcessor } func (p *Processor) run() error { var emailMessage admin.EmailMessage var err error - for msg := range p.sub.Start() { - p.systemMetrics.MessageTotal.Inc() - // Currently this is safe because Gizmo takes a string and casts it to a byte array. + p.BaseProcessor.StartProcessing() + for msg := range p.Sub.Start() { + p.SystemMetrics.MessageTotal.Inc() stringMsg := string(msg.Message()) - var snsJSONFormat map[string]interface{} - - // At Lyft, SNS populates SQS. This results in the message body of SQS having the SNS message format. - // The message format is documented here: https://docs.aws.amazon.com/sns/latest/dg/sns-message-and-json-formats.html - // The notification published is stored in the message field after unmarshalling the SQS message. - if err := json.Unmarshal(msg.Message(), &snsJSONFormat); err != nil { - p.systemMetrics.MessageDecodingError.Inc() - logger.Errorf(context.Background(), "failed to unmarshall JSON message [%s] from processor with err: %v", stringMsg, err) - p.markMessageDone(msg) - continue - } - - var value interface{} - var ok bool - var valueString string - - if value, ok = snsJSONFormat["Message"]; !ok { - logger.Errorf(context.Background(), "failed to retrieve message from unmarshalled JSON object [%s]", stringMsg) - p.systemMetrics.MessageDataError.Inc() - p.markMessageDone(msg) - continue - } - - if valueString, ok = value.(string); !ok { - p.systemMetrics.MessageDataError.Inc() - logger.Errorf(context.Background(), "failed to retrieve notification message (in string format) from unmarshalled JSON object for message [%s]", stringMsg) - p.markMessageDone(msg) + _, messageByte, ok := p.FromSQSMessage(msg) + if !ok { continue } - // The Publish method for SNS Encodes the notification using Base64 then stringifies it before - // setting that as the message body for SNS. Do the inverse to retrieve the notification. - notificationBytes, err := base64.StdEncoding.DecodeString(valueString) - if err != nil { - logger.Errorf(context.Background(), "failed to Base64 decode from message string [%s] from message [%s] with err: %v", valueString, stringMsg, err) - p.systemMetrics.MessageDecodingError.Inc() - p.markMessageDone(msg) - continue - } - - if err = proto.Unmarshal(notificationBytes, &emailMessage); err != nil { - logger.Debugf(context.Background(), "failed to unmarshal to notification object from decoded string[%s] from message [%s] with err: %v", valueString, stringMsg, err) - p.systemMetrics.MessageDecodingError.Inc() - p.markMessageDone(msg) + if err = proto.Unmarshal(messageByte, &emailMessage); err != nil { + logger.Debugf(context.Background(), "failed to unmarshal to notification object from decoded string from message [%s] with err: %v", stringMsg, err) + p.SystemMetrics.MessageDecodingError.Inc() + p.MarkMessageDone(msg) continue } if err = p.email.SendEmail(context.Background(), emailMessage); err != nil { - p.systemMetrics.MessageProcessorError.Inc() + p.SystemMetrics.MessageProcessorError.Inc() logger.Errorf(context.Background(), "Error sending an email message for message [%s] with emailM with err: %v", emailMessage.String(), err) } else { - p.systemMetrics.MessageSuccess.Inc() + p.SystemMetrics.MessageSuccess.Inc() } - p.markMessageDone(msg) - + p.MarkMessageDone(msg) } // According to https://github.com/NYTimes/gizmo/blob/f2b3deec03175b11cdfb6642245a49722751357f/pubsub/pubsub.go#L36-L39, // the channel backing the subscriber will just close if there is an error. The call to Err() is needed to identify // there was an error in the channel or there are no more messages left (resulting in no errors when calling Err()). - if err = p.sub.Err(); err != nil { - p.systemMetrics.ChannelClosedError.Inc() + if err = p.Sub.Err(); err != nil { + p.SystemMetrics.ChannelClosedError.Inc() logger.Warningf(context.Background(), "The stream for the subscriber channel closed with err: %v", err) } @@ -112,27 +58,12 @@ func (p *Processor) run() error { return err } -func (p *Processor) markMessageDone(message pubsub.SubscriberMessage) { - if err := message.Done(); err != nil { - p.systemMetrics.MessageDoneError.Inc() - logger.Errorf(context.Background(), "failed to mark message as Done() in processor with err: %v", err) - } -} - -func (p *Processor) StopProcessing() error { - // Note: If the underlying channel is already closed, then Stop() will return an error. - err := p.sub.Stop() - if err != nil { - p.systemMetrics.StopError.Inc() - logger.Errorf(context.Background(), "Failed to stop the subscriber channel gracefully with err: %v", err) - } - return err -} - func NewProcessor(sub pubsub.Subscriber, emailer interfaces.Emailer, scope promutils.Scope) interfaces.Processor { return &Processor{ - sub: sub, - email: emailer, - systemMetrics: newProcessorSystemMetrics(scope.NewSubScope("processor")), + email: emailer, + BaseProcessor: interfaces.BaseProcessor{ + Sub: sub, + SystemMetrics: interfaces.NewProcessorSystemMetrics(scope.NewSubScope("processor")), + }, } } diff --git a/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go b/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go index 0ce49371e3..f4c888ef01 100644 --- a/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go +++ b/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go @@ -2,10 +2,7 @@ package implementations import ( "context" - "time" - "github.com/NYTimes/gizmo/pubsub" - "github.com/flyteorg/flyteadmin/pkg/async" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flytestdlib/logger" @@ -15,77 +12,51 @@ import ( // TODO: Add a counter that encompasses the publisher stats grouped by project and domain. type GcpProcessor struct { - sub pubsub.Subscriber - email interfaces.Emailer - systemMetrics processorSystemMetrics + email interfaces.Emailer + interfaces.BaseProcessor } func NewGcpProcessor(sub pubsub.Subscriber, emailer interfaces.Emailer, scope promutils.Scope) interfaces.Processor { return &GcpProcessor{ - sub: sub, - email: emailer, - systemMetrics: newProcessorSystemMetrics(scope.NewSubScope("gcp_processor")), - } -} - -func (p *GcpProcessor) StartProcessing() { - for { - logger.Warningf(context.Background(), "Starting GCP notifications processor") - err := p.run() - logger.Errorf(context.Background(), "error with running GCP processor err: [%v] ", err) - time.Sleep(async.RetryDelay) + email: emailer, + BaseProcessor: interfaces.BaseProcessor{ + Sub: sub, + SystemMetrics: interfaces.NewProcessorSystemMetrics(scope.NewSubScope("processor")), + }, } } func (p *GcpProcessor) run() error { var emailMessage admin.EmailMessage - for msg := range p.sub.Start() { - p.systemMetrics.MessageTotal.Inc() + for msg := range p.Sub.Start() { + p.SystemMetrics.MessageTotal.Inc() if err := proto.Unmarshal(msg.Message(), &emailMessage); err != nil { logger.Debugf(context.Background(), "failed to unmarshal to notification object message [%s] with err: %v", string(msg.Message()), err) - p.systemMetrics.MessageDecodingError.Inc() - p.markMessageDone(msg) + p.SystemMetrics.MessageDecodingError.Inc() + p.MarkMessageDone(msg) continue } if err := p.email.SendEmail(context.Background(), emailMessage); err != nil { - p.systemMetrics.MessageProcessorError.Inc() + p.SystemMetrics.MessageProcessorError.Inc() logger.Errorf(context.Background(), "Error sending an email message for message [%s] with emailM with err: %v", emailMessage.String(), err) } else { - p.systemMetrics.MessageSuccess.Inc() + p.SystemMetrics.MessageSuccess.Inc() } - p.markMessageDone(msg) + p.MarkMessageDone(msg) } // According to https://github.com/NYTimes/gizmo/blob/f2b3deec03175b11cdfb6642245a49722751357f/pubsub/pubsub.go#L36-L39, // the channel backing the subscriber will just close if there is an error. The call to Err() is needed to identify // there was an error in the channel or there are no more messages left (resulting in no errors when calling Err()). - if err := p.sub.Err(); err != nil { - p.systemMetrics.ChannelClosedError.Inc() + if err := p.Sub.Err(); err != nil { + p.SystemMetrics.ChannelClosedError.Inc() logger.Warningf(context.Background(), "The stream for the subscriber channel closed with err: %v", err) return err } return nil } - -func (p *GcpProcessor) markMessageDone(message pubsub.SubscriberMessage) { - if err := message.Done(); err != nil { - p.systemMetrics.MessageDoneError.Inc() - logger.Errorf(context.Background(), "failed to mark message as Done() in processor with err: %v", err) - } -} - -func (p *GcpProcessor) StopProcessing() error { - // Note: If the underlying channel is already closed, then Stop() will return an error. - if err := p.sub.Stop(); err != nil { - p.systemMetrics.StopError.Inc() - logger.Errorf(context.Background(), "Failed to stop the subscriber channel gracefully with err: %v", err) - return err - } - - return nil -} diff --git a/flyteadmin/pkg/async/notifications/interfaces/processor.go b/flyteadmin/pkg/async/notifications/interfaces/processor.go index b7ce30d56e..08fe15dffa 100644 --- a/flyteadmin/pkg/async/notifications/interfaces/processor.go +++ b/flyteadmin/pkg/async/notifications/interfaces/processor.go @@ -1,5 +1,17 @@ package interfaces +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "github.com/NYTimes/gizmo/pubsub" + gizmoGCP "github.com/NYTimes/gizmo/pubsub/gcp" + "github.com/flyteorg/flyteadmin/pkg/async" + "github.com/flyteorg/flytestdlib/logger" + "time" +) + // Exposes the common methods required for a subscriber. // There is one ProcessNotification per type. type Processor interface { @@ -15,3 +27,114 @@ type Processor interface { // the channel was already closed. StopProcessing() error } + +type BaseProcessor struct { + Sub pubsub.Subscriber + SystemMetrics ProcessorSystemMetrics +} + +// StartProcessing Currently only email is the supported notification because slack and pagerduty both use +// email client to trigger those notifications. +// When Pagerduty and other notifications are supported, a publisher per type should be created. +func (p *BaseProcessor) StartProcessing() { + for { + logger.Warningf(context.Background(), "Starting notifications processor") + err := p.run() + logger.Errorf(context.Background(), "error with running processor err: [%v] ", err) + time.Sleep(async.RetryDelay) + } +} + +func (p *BaseProcessor) run() error { + return errors.New("run() is not implemented") +} + +// FromPubSubMessage Parse the message from GCP PubSub and return the message subject and the message body. +func (p *BaseProcessor) FromPubSubMessage(msg pubsub.SubscriberMessage) (string, []byte, bool) { + var gcpMsg *gizmoGCP.SubMessage + var ok bool + p.SystemMetrics.MessageTotal.Inc() + if gcpMsg, ok = msg.(*gizmoGCP.SubMessage); !ok { + logger.Errorf(context.Background(), "failed to cast message [%v] to gizmoGCP.SubMessage", msg) + p.SystemMetrics.MessageDataError.Inc() + p.MarkMessageDone(msg) + return "", nil, false + } + subject := gcpMsg.Attributes["key"] + return subject, gcpMsg.Message(), true +} + +// FromSQSMessage Parse the message from AWS SQS and return the message subject and the message body. +func (p *BaseProcessor) FromSQSMessage(msg pubsub.SubscriberMessage) (string, []byte, bool) { + // Currently this is safe because Gizmo takes a string and casts it to a byte array. + stringMsg := string(msg.Message()) + var snsJSONFormat map[string]interface{} + + if err := json.Unmarshal(msg.Message(), &snsJSONFormat); err != nil { + p.SystemMetrics.MessageDecodingError.Inc() + logger.Errorf(context.Background(), "failed to unmarshall JSON message [%s] from processor with err: %v", stringMsg, err) + p.MarkMessageDone(msg) + return "", nil, false + } + + var value interface{} + var ok bool + var valueString string + var subject string + + if value, ok = snsJSONFormat["Message"]; !ok { + logger.Errorf(context.Background(), "failed to retrieve message from unmarshalled JSON object [%s]", stringMsg) + p.SystemMetrics.MessageDataError.Inc() + p.MarkMessageDone(msg) + return "", nil, false + } + + if valueString, ok = value.(string); !ok { + p.SystemMetrics.MessageDataError.Inc() + logger.Errorf(context.Background(), "failed to retrieve notification message (in string format) from unmarshalled JSON object for message [%s]", stringMsg) + p.MarkMessageDone(msg) + return "", nil, false + } + + if value, ok = snsJSONFormat["Subject"]; !ok { + logger.Errorf(context.Background(), "failed to retrieve message type from unmarshalled JSON object [%s]", stringMsg) + p.SystemMetrics.MessageDataError.Inc() + p.MarkMessageDone(msg) + return "", nil, false + } + + if subject, ok = value.(string); !ok { + p.SystemMetrics.MessageDataError.Inc() + logger.Errorf(context.Background(), "failed to retrieve notification message type (in string format) from unmarshalled JSON object for message [%s]", stringMsg) + p.MarkMessageDone(msg) + return "", nil, false + } + + // The Publish method for SNS Encodes the notification using Base64 then stringifies it before + // setting that as the message body for SNS. Do the inverse to retrieve the notification. + messageBytes, err := base64.StdEncoding.DecodeString(valueString) + if err != nil { + logger.Errorf(context.Background(), "failed to Base64 decode from message string [%s] from message [%s] with err: %v", valueString, stringMsg, err) + p.SystemMetrics.MessageDecodingError.Inc() + p.MarkMessageDone(msg) + return "", nil, false + } + return subject, messageBytes, true +} + +func (p *BaseProcessor) StopProcessing() error { + // Note: If the underlying channel is already closed, then Stop() will return an error. + err := p.Sub.Stop() + if err != nil { + p.SystemMetrics.StopError.Inc() + logger.Errorf(context.Background(), "Failed to stop the subscriber channel gracefully with err: %v", err) + } + return err +} + +func (p *BaseProcessor) MarkMessageDone(message pubsub.SubscriberMessage) { + if err := message.Done(); err != nil { + p.SystemMetrics.MessageDoneError.Inc() + logger.Errorf(context.Background(), "failed to mark message as Done() in processor with err: %v", err) + } +} diff --git a/flyteadmin/pkg/async/notifications/implementations/processor_metrics.go b/flyteadmin/pkg/async/notifications/interfaces/processor_metrics.go similarity index 90% rename from flyteadmin/pkg/async/notifications/implementations/processor_metrics.go rename to flyteadmin/pkg/async/notifications/interfaces/processor_metrics.go index adc4219f96..ab796a2ca1 100644 --- a/flyteadmin/pkg/async/notifications/implementations/processor_metrics.go +++ b/flyteadmin/pkg/async/notifications/interfaces/processor_metrics.go @@ -1,11 +1,11 @@ -package implementations +package interfaces import ( "github.com/flyteorg/flytestdlib/promutils" "github.com/prometheus/client_golang/prometheus" ) -type processorSystemMetrics struct { +type ProcessorSystemMetrics struct { Scope promutils.Scope MessageTotal prometheus.Counter MessageDoneError prometheus.Counter @@ -17,8 +17,8 @@ type processorSystemMetrics struct { StopError prometheus.Counter } -func newProcessorSystemMetrics(scope promutils.Scope) processorSystemMetrics { - return processorSystemMetrics{ +func NewProcessorSystemMetrics(scope promutils.Scope) ProcessorSystemMetrics { + return ProcessorSystemMetrics{ Scope: scope, MessageTotal: scope.MustNewCounter("message_total", "overall count of messages processed"), MessageDecodingError: scope.MustNewCounter("message_decoding_error", "count of messages with decoding errors"), diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index a527f73f98..fae0f9ca3b 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -3,6 +3,8 @@ package webhook import ( "context" "fmt" + gizmoGCP "github.com/NYTimes/gizmo/pubsub/gcp" + "github.com/flyteorg/flyteadmin/pkg/common" "time" repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" @@ -10,6 +12,7 @@ import ( "github.com/NYTimes/gizmo/pubsub" gizmoAWS "github.com/NYTimes/gizmo/pubsub/aws" "github.com/flyteorg/flyteadmin/pkg/async" + notificationsImplementations "github.com/flyteorg/flyteadmin/pkg/async/notifications/implementations" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" "github.com/flyteorg/flyteadmin/pkg/async/webhook/implementations" "github.com/flyteorg/flytestdlib/logger" @@ -35,9 +38,12 @@ func NewWebhookProcessors(db repoInterfaces.Repository, config runtimeInterfaces reconnectDelay := time.Duration(config.ReconnectDelaySeconds) * time.Second var sub pubsub.Subscriber var processors []interfaces.Processor + var err error for _, cfg := range config.WebhooksConfig { - if len(config.AWSConfig.Region) != 0 { + var processor interfaces.Processor + switch config.Type { + case common.AWS: sqsConfig := gizmoAWS.SQSConfig{ QueueName: cfg.NotificationsProcessorConfig.QueueName, QueueOwnerAccountID: cfg.NotificationsProcessorConfig.AccountID, @@ -47,7 +53,6 @@ func NewWebhookProcessors(db repoInterfaces.Repository, config runtimeInterfaces ConsumeBase64: &enable64decoding, } sqsConfig.Region = config.AWSConfig.Region - var err error err = async.Retry(reconnectAttempts, reconnectDelay, func() error { sub, err = gizmoAWS.NewSubscriber(sqsConfig) if err != nil { @@ -58,12 +63,29 @@ func NewWebhookProcessors(db repoInterfaces.Repository, config runtimeInterfaces if err != nil { panic(err) } + processor = implementations.NewWebhookProcessor(common.AWS, sub, GetWebhook(cfg, scope), db, scope) + case common.GCP: + projectID := config.GCPConfig.ProjectID + subscription := cfg.NotificationsProcessorConfig.QueueName + err = async.Retry(reconnectAttempts, reconnectDelay, func() error { + sub, err = gizmoGCP.NewSubscriber(context.TODO(), projectID, subscription) + if err != nil { + logger.Warnf(context.TODO(), "Failed to initialize new gizmo gcp subscriber with config [ProjectID: %s, Subscription: %s] and err: %v", projectID, subscription, err) + } + return err + }) + if err != nil { + panic(err) + } + processor = implementations.NewWebhookProcessor(common.GCP, sub, GetWebhook(cfg, scope), db, scope) + case common.Local: + fallthrough + default: + logger.Infof(context.Background(), + "Using default noop notifications processor implementation for config type [%s]", config.Type) + processor = notificationsImplementations.NewNoopProcess() } - - webhook := GetWebhook(cfg, scope) - if webhook != nil { - processors = append(processors, implementations.NewWebhookProcessor(sub, webhook, db, scope)) - } + processors = append(processors, processor) } return processors } diff --git a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go b/flyteadmin/pkg/async/webhook/implementations/aws_processer.go deleted file mode 100644 index 1d363c0953..0000000000 --- a/flyteadmin/pkg/async/webhook/implementations/aws_processer.go +++ /dev/null @@ -1,177 +0,0 @@ -package implementations - -import ( - "context" - "encoding/base64" - "encoding/json" - "time" - - "github.com/flyteorg/flyteadmin/pkg/common" - "github.com/flyteorg/flyteadmin/pkg/manager/impl/util" - repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" - "github.com/flyteorg/flyteadmin/pkg/repositories/transformers" - "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" - "github.com/golang/protobuf/proto" - - "github.com/NYTimes/gizmo/pubsub" - "github.com/flyteorg/flyteadmin/pkg/async" - "github.com/flyteorg/flyteadmin/pkg/async/notifications" - "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" - webhookInterfaces "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" - "github.com/flyteorg/flytestdlib/logger" - "github.com/flyteorg/flytestdlib/promutils" -) - -type Processor struct { - sub pubsub.Subscriber - webhook webhookInterfaces.Webhook - db repoInterfaces.Repository - systemMetrics processorSystemMetrics -} - -func (p *Processor) StartProcessing() { - for { - logger.Warningf(context.Background(), "Starting webhook processor") - err := p.run() - logger.Errorf(context.Background(), "error with running processor err: [%v] ", err) - time.Sleep(async.RetryDelay) - } -} - -func (p *Processor) run() error { - var payload admin.WebhookPayload - var request admin.WorkflowExecutionEventRequest - var err error - for msg := range p.sub.Start() { - p.systemMetrics.MessageTotal.Inc() - // Currently this is safe because Gizmo takes a string and casts it to a byte array. - stringMsg := string(msg.Message()) - var snsJSONFormat map[string]interface{} - - if err := json.Unmarshal(msg.Message(), &snsJSONFormat); err != nil { - p.systemMetrics.MessageDecodingError.Inc() - logger.Errorf(context.Background(), "failed to unmarshall JSON message [%s] from processor with err: %v", stringMsg, err) - p.markMessageDone(msg) - continue - } - - var value interface{} - var ok bool - var valueString string - var messageType string - - if value, ok = snsJSONFormat["Message"]; !ok { - logger.Errorf(context.Background(), "failed to retrieve message from unmarshalled JSON object [%s]", stringMsg) - p.systemMetrics.MessageDataError.Inc() - p.markMessageDone(msg) - continue - } - - if valueString, ok = value.(string); !ok { - p.systemMetrics.MessageDataError.Inc() - logger.Errorf(context.Background(), "failed to retrieve notification message (in string format) from unmarshalled JSON object for message [%s]", stringMsg) - p.markMessageDone(msg) - continue - } - - if value, ok = snsJSONFormat["Subject"]; !ok { - logger.Errorf(context.Background(), "failed to retrieve message type from unmarshalled JSON object [%s]", stringMsg) - p.systemMetrics.MessageDataError.Inc() - p.markMessageDone(msg) - continue - } - - if messageType, ok = value.(string); !ok { - p.systemMetrics.MessageDataError.Inc() - logger.Errorf(context.Background(), "failed to retrieve notification message type (in string format) from unmarshalled JSON object for message [%s]", stringMsg) - p.markMessageDone(msg) - continue - } - - if messageType != proto.MessageName(&admin.WorkflowExecutionEventRequest{}) { - p.markMessageDone(msg) - continue - } - - // The Publish method for SNS Encodes the notification using Base64 then stringifies it before - // setting that as the message body for SNS. Do the inverse to retrieve the notification. - requestBytes, err := base64.StdEncoding.DecodeString(valueString) - if err != nil { - logger.Errorf(context.Background(), "failed to Base64 decode from message string [%s] from message [%s] with err: %v", valueString, stringMsg, err) - p.systemMetrics.MessageDecodingError.Inc() - p.markMessageDone(msg) - continue - } - - if err = proto.Unmarshal(requestBytes, &request); err != nil { - logger.Errorf(context.Background(), "failed to unmarshal to notification object from decoded string[%s] from message [%s] with err: %v", valueString, stringMsg, err) - p.systemMetrics.MessageDecodingError.Inc() - p.markMessageDone(msg) - continue - } - - if !common.IsExecutionTerminal(request.Event.Phase) { - p.markMessageDone(msg) - continue - } - - executionModel, err := util.GetExecutionModel(context.Background(), p.db, *request.Event.ExecutionId) - if err != nil { - p.systemMetrics.MessageProcessorError.Inc() - logger.Errorf(context.Background(), "failed to retrieve execution model for execution [%+v] from message [%s] with err: %v", request.Event.ExecutionId, stringMsg, err) - p.markMessageDone(msg) - continue - } - adminExecution, err := transformers.FromExecutionModel(context.Background(), *executionModel, transformers.DefaultExecutionTransformerOptions) - if err != nil { - p.systemMetrics.MessageProcessorError.Inc() - logger.Errorf(context.Background(), "failed to transform execution model [%+v] from message [%s] with err: %v", executionModel, stringMsg, err) - p.markMessageDone(msg) - continue - } - - payload.Message = notifications.SubstituteParameters(p.webhook.GetConfig().Payload, request, adminExecution) - logger.Info(context.Background(), "Processor is sending message to webhook endpoint") - if err = p.webhook.Post(context.Background(), payload); err != nil { - p.systemMetrics.MessageProcessorError.Inc() - logger.Errorf(context.Background(), "Error sending an message [%v] to webhook endpoint with err: %v", payload, err) - } else { - p.systemMetrics.MessageSuccess.Inc() - } - - p.markMessageDone(msg) - } - - if err = p.sub.Err(); err != nil { - p.systemMetrics.ChannelClosedError.Inc() - logger.Errorf(context.Background(), "The stream for the subscriber channel closed with err: %v", err) - } - - return err -} - -func (p *Processor) markMessageDone(message pubsub.SubscriberMessage) { - if err := message.Done(); err != nil { - p.systemMetrics.MessageDoneError.Inc() - logger.Errorf(context.Background(), "failed to mark message as Done() in processor with err: %v", err) - } -} - -func (p *Processor) StopProcessing() error { - // Note: If the underlying channel is already closed, then Stop() will return an error. - err := p.sub.Stop() - if err != nil { - p.systemMetrics.StopError.Inc() - logger.Errorf(context.Background(), "Failed to stop the subscriber channel gracefully with err: %v", err) - } - return err -} - -func NewWebhookProcessor(sub pubsub.Subscriber, webhook webhookInterfaces.Webhook, db repoInterfaces.Repository, scope promutils.Scope) interfaces.Processor { - return &Processor{ - sub: sub, - webhook: webhook, - db: db, - systemMetrics: newProcessorSystemMetrics(scope.NewSubScope("webhook_processor")), - } -} diff --git a/flyteadmin/pkg/async/webhook/implementations/processer.go b/flyteadmin/pkg/async/webhook/implementations/processer.go new file mode 100644 index 0000000000..6a347e4cb7 --- /dev/null +++ b/flyteadmin/pkg/async/webhook/implementations/processer.go @@ -0,0 +1,125 @@ +package implementations + +import ( + "context" + "time" + + "github.com/flyteorg/flyteadmin/pkg/common" + "github.com/flyteorg/flyteadmin/pkg/manager/impl/util" + repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" + "github.com/flyteorg/flyteadmin/pkg/repositories/transformers" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" + "github.com/golang/protobuf/proto" + + "github.com/NYTimes/gizmo/pubsub" + "github.com/flyteorg/flyteadmin/pkg/async" + "github.com/flyteorg/flyteadmin/pkg/async/notifications" + "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" + webhookInterfaces "github.com/flyteorg/flyteadmin/pkg/async/webhook/interfaces" + "github.com/flyteorg/flytestdlib/logger" + "github.com/flyteorg/flytestdlib/promutils" +) + +type Processor struct { + subType string + webhook webhookInterfaces.Webhook + db repoInterfaces.Repository + interfaces.BaseProcessor +} + +func (p *Processor) StartProcessing() { + for { + logger.Warningf(context.Background(), "Starting webhook processor") + err := p.run() + logger.Errorf(context.Background(), "error with running processor err: [%v] ", err) + time.Sleep(async.RetryDelay) + } +} + +func (p *Processor) run() error { + var payload admin.WebhookPayload + var request admin.WorkflowExecutionEventRequest + var err error + var subject string + var messageByte []byte + var ok bool + + for msg := range p.Sub.Start() { + p.SystemMetrics.MessageTotal.Inc() + stringMsg := string(msg.Message()) + if p.subType == common.AWS { + subject, messageByte, ok = p.FromSQSMessage(msg) + } else { + subject, messageByte, ok = p.FromPubSubMessage(msg) + } + if !ok { + continue + } + + if subject != proto.MessageName(&admin.WorkflowExecutionEventRequest{}) { + p.MarkMessageDone(msg) + continue + } + + if err = proto.Unmarshal(messageByte, &request); err != nil { + logger.Errorf(context.Background(), "failed to unmarshal to notification object from decoded string from message [%s] with err: %v", stringMsg, err) + p.SystemMetrics.MessageDecodingError.Inc() + p.MarkMessageDone(msg) + continue + } + + if !common.IsExecutionTerminal(request.Event.Phase) { + p.MarkMessageDone(msg) + continue + } + + executionModel, err := util.GetExecutionModel(context.Background(), p.db, *request.Event.ExecutionId) + if err != nil { + p.SystemMetrics.MessageProcessorError.Inc() + logger.Errorf(context.Background(), "failed to retrieve execution model for execution [%+v] from message [%s] with err: %v", request.Event.ExecutionId, stringMsg, err) + p.MarkMessageDone(msg) + continue + } + adminExecution, err := transformers.FromExecutionModel(context.Background(), *executionModel, transformers.DefaultExecutionTransformerOptions) + if err != nil { + p.SystemMetrics.MessageProcessorError.Inc() + logger.Errorf(context.Background(), "failed to transform execution model [%+v] from message [%s] with err: %v", executionModel, stringMsg, err) + p.MarkMessageDone(msg) + continue + } + + payload.Message = notifications.SubstituteParameters(p.webhook.GetConfig().Payload, request, adminExecution) + logger.Info(context.Background(), "Processor is sending message to webhook endpoint") + if err = p.webhook.Post(context.Background(), payload); err != nil { + p.SystemMetrics.MessageProcessorError.Inc() + logger.Errorf(context.Background(), "Error sending an message [%v] to webhook endpoint with err: %v", payload, err) + } else { + p.SystemMetrics.MessageSuccess.Inc() + } + + p.MarkMessageDone(msg) + } + + if err = p.Sub.Err(); err != nil { + p.SystemMetrics.ChannelClosedError.Inc() + logger.Errorf(context.Background(), "The stream for the subscriber channel closed with err: %v", err) + } + + return err +} + +func NewWebhookProcessor(subType string, sub pubsub.Subscriber, webhook webhookInterfaces.Webhook, db repoInterfaces.Repository, scope promutils.Scope) interfaces.Processor { + if subType != common.AWS && subType != common.GCP { + panic("unknown subscriber type [" + subType + "]") + } + + return &Processor{ + subType: subType, + webhook: webhook, + db: db, + BaseProcessor: interfaces.BaseProcessor{ + Sub: sub, + SystemMetrics: interfaces.NewProcessorSystemMetrics(scope.NewSubScope("webhook_processor")), + }, + } +} diff --git a/flyteadmin/pkg/async/webhook/implementations/processor_metrics.go b/flyteadmin/pkg/async/webhook/implementations/processor_metrics.go deleted file mode 100644 index adc4219f96..0000000000 --- a/flyteadmin/pkg/async/webhook/implementations/processor_metrics.go +++ /dev/null @@ -1,32 +0,0 @@ -package implementations - -import ( - "github.com/flyteorg/flytestdlib/promutils" - "github.com/prometheus/client_golang/prometheus" -) - -type processorSystemMetrics struct { - Scope promutils.Scope - MessageTotal prometheus.Counter - MessageDoneError prometheus.Counter - MessageDecodingError prometheus.Counter - MessageDataError prometheus.Counter - MessageProcessorError prometheus.Counter - MessageSuccess prometheus.Counter - ChannelClosedError prometheus.Counter - StopError prometheus.Counter -} - -func newProcessorSystemMetrics(scope promutils.Scope) processorSystemMetrics { - return processorSystemMetrics{ - Scope: scope, - MessageTotal: scope.MustNewCounter("message_total", "overall count of messages processed"), - MessageDecodingError: scope.MustNewCounter("message_decoding_error", "count of messages with decoding errors"), - MessageDataError: scope.MustNewCounter("message_data_error", "count of message data processing errors experience when preparing the message to be notified."), - MessageDoneError: scope.MustNewCounter("message_done_error", "count of message errors when marking it as done with underlying processor"), - MessageProcessorError: scope.MustNewCounter("message_processing_error", "count of errors when interacting with notification processor"), - MessageSuccess: scope.MustNewCounter("message_ok", "count of messages successfully processed by underlying notification mechanism"), - ChannelClosedError: scope.MustNewCounter("channel_closed_error", "count of channel closing errors"), - StopError: scope.MustNewCounter("stop_error", "count of errors in Stop() method"), - } -} diff --git a/flyteadmin/pkg/async/webhook/implementations/aws_processor_test.go b/flyteadmin/pkg/async/webhook/implementations/processor_test.go similarity index 85% rename from flyteadmin/pkg/async/webhook/implementations/aws_processor_test.go rename to flyteadmin/pkg/async/webhook/implementations/processor_test.go index 32d795f8e1..56c9b3848d 100644 --- a/flyteadmin/pkg/async/webhook/implementations/aws_processor_test.go +++ b/flyteadmin/pkg/async/webhook/implementations/processor_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "errors" + "github.com/flyteorg/flyteadmin/pkg/common" "testing" "time" @@ -118,13 +119,13 @@ func TestProcessor_StartProcessing(t *testing.T) { }, nil }, ) - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Nil(t, testProcessor.(*Processor).run()) } func TestProcessor_StartProcessingNoMessages(t *testing.T) { initializeProcessor() - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Nil(t, testProcessor.(*Processor).run()) } @@ -135,7 +136,7 @@ func TestProcessor_StartProcessingNoNotificationMessage(t *testing.T) { } initializeProcessor() testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testMessage) - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Nil(t, testProcessor.(*Processor).run()) } @@ -147,7 +148,7 @@ func TestProcessor_StartProcessingMessageWrongDataType(t *testing.T) { } initializeProcessor() testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testMessage) - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Nil(t, testProcessor.(*Processor).run()) } @@ -159,7 +160,7 @@ func TestProcessor_StartProcessingBase64DecodeError(t *testing.T) { } initializeProcessor() testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testMessage) - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Nil(t, testProcessor.(*Processor).run()) } @@ -172,7 +173,7 @@ func TestProcessor_StartProcessingProtoMarshallError(t *testing.T) { } initializeProcessor() testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testMessage) - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Nil(t, testProcessor.(*Processor).run()) } @@ -180,7 +181,7 @@ func TestProcessor_StartProcessingError(t *testing.T) { initializeProcessor() var ret = errors.New("err() returned an error") testSubscriber.GivenErrError = ret - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Equal(t, ret, testProcessor.(*Processor).run()) } @@ -192,13 +193,13 @@ func TestProcessor_StartProcessingWebhookError(t *testing.T) { } mockWebhook.SetWebhookPostFunc(sendWebhookErrorFunc) testSubscriber.JSONMessages = append(testSubscriber.JSONMessages, testSubscriberMessage) - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Nil(t, testProcessor.(*Processor).run()) } func TestProcessor_StopProcessing(t *testing.T) { initializeProcessor() - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Nil(t, testProcessor.StopProcessing()) } @@ -206,6 +207,6 @@ func TestProcessor_StopProcessingError(t *testing.T) { initializeProcessor() var stopError = errors.New("stop() returns an error") testSubscriber.GivenStopError = stopError - testProcessor := NewWebhookProcessor(mockSub, &mockWebhook, repo, promutils.NewTestScope()) + testProcessor := NewWebhookProcessor(common.AWS, mockSub, &mockWebhook, repo, promutils.NewTestScope()) assert.Equal(t, stopError, testProcessor.StopProcessing()) } diff --git a/flyteadmin/pkg/runtime/application_config_provider.go b/flyteadmin/pkg/runtime/application_config_provider.go index 578f072079..91aa8dda97 100644 --- a/flyteadmin/pkg/runtime/application_config_provider.go +++ b/flyteadmin/pkg/runtime/application_config_provider.go @@ -63,7 +63,9 @@ var remoteDataConfig = config.MustRegisterSection(remoteData, &interfaces.Remote var notificationsConfig = config.MustRegisterSection(notifications, &interfaces.NotificationsConfig{ Type: common.Local, }) -var webhookNotificationsConfig = config.MustRegisterSection(webhookNotifications, &interfaces.WebhookNotificationsConfig{}) +var webhookNotificationsConfig = config.MustRegisterSection(webhookNotifications, &interfaces.WebhookNotificationsConfig{ + Type: common.Local, +}) var domainsConfig = config.MustRegisterSection(domains, &interfaces.DomainsConfig{ { ID: "development", diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index 55979a8d29..4e2ed92df6 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -563,6 +563,9 @@ type WebHookConfig struct { // WebhookNotificationsConfig defines the configuration for the webhook service. type WebhookNotificationsConfig struct { + // Defines the cloud provider that backs the scheduler. In the absence of a specification the no-op, 'local' + // scheme is used. + Type string `json:"type"` AWSConfig AWSConfig `json:"aws"` GCPConfig GCPConfig `json:"gcp"` // Defines the list of webhooks to be configured. From fcfe2028586258f9b53b5802b3b20ad9c1ba2ca0 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Thu, 20 Jul 2023 17:43:35 -0700 Subject: [PATCH 30/35] lint Signed-off-by: Kevin Su --- .../notifications/implementations/gcp_processor_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flyteadmin/pkg/async/notifications/implementations/gcp_processor_test.go b/flyteadmin/pkg/async/notifications/implementations/gcp_processor_test.go index 2aa216d296..92658c93be 100644 --- a/flyteadmin/pkg/async/notifications/implementations/gcp_processor_test.go +++ b/flyteadmin/pkg/async/notifications/implementations/gcp_processor_test.go @@ -45,7 +45,7 @@ func TestGcpProcessor_StartProcessing(t *testing.T) { // Check fornumber of messages processed. m := &dto.Metric{} - err := testGcpProcessor.(*GcpProcessor).systemMetrics.MessageSuccess.Write(m) + err := testGcpProcessor.(*GcpProcessor).SystemMetrics.MessageSuccess.Write(m) assert.Nil(t, err) assert.Equal(t, "counter: ", m.String()) } @@ -60,7 +60,7 @@ func TestGcpProcessor_StartProcessingNoMessages(t *testing.T) { // Check fornumber of messages processed. m := &dto.Metric{} - err := testGcpProcessor.(*GcpProcessor).systemMetrics.MessageSuccess.Write(m) + err := testGcpProcessor.(*GcpProcessor).SystemMetrics.MessageSuccess.Write(m) assert.Nil(t, err) assert.Equal(t, "counter: ", m.String()) } @@ -93,7 +93,7 @@ func TestGcpProcessor_StartProcessingEmailError(t *testing.T) { // Check for an email error stat. m := &dto.Metric{} - err := testGcpProcessor.(*GcpProcessor).systemMetrics.MessageProcessorError.Write(m) + err := testGcpProcessor.(*GcpProcessor).SystemMetrics.MessageProcessorError.Write(m) assert.Nil(t, err) assert.Equal(t, "counter: ", m.String()) } From 633031407f88e8d277e378709285dcacb84d5b50 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Thu, 20 Jul 2023 22:43:27 -0700 Subject: [PATCH 31/35] Fixed tests Signed-off-by: Kevin Su --- .../implementations/aws_processor.go | 15 ++++++++++++++- .../implementations/gcp_processor.go | 11 +++++++++++ .../notifications/interfaces/processor.go | 19 ------------------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go index 4d3475b968..f88404b6ca 100644 --- a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go +++ b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go @@ -3,11 +3,13 @@ package implementations import ( "context" "github.com/NYTimes/gizmo/pubsub" + "github.com/flyteorg/flyteadmin/pkg/async" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flytestdlib/logger" "github.com/flyteorg/flytestdlib/promutils" "github.com/golang/protobuf/proto" + "time" ) // TODO: Add a counter that encompasses the publisher stats grouped by project and domain. @@ -16,10 +18,21 @@ type Processor struct { interfaces.BaseProcessor } +// StartProcessing Currently only email is the supported notification because slack and pagerduty both use +// email client to trigger those notifications. +// When Pagerduty and other notifications are supported, a publisher per type should be created. +func (p *Processor) StartProcessing() { + for { + logger.Warningf(context.Background(), "Starting notifications processor") + err := p.run() + logger.Errorf(context.Background(), "error with running processor err: [%v] ", err) + time.Sleep(async.RetryDelay) + } +} + func (p *Processor) run() error { var emailMessage admin.EmailMessage var err error - p.BaseProcessor.StartProcessing() for msg := range p.Sub.Start() { p.SystemMetrics.MessageTotal.Inc() stringMsg := string(msg.Message()) diff --git a/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go b/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go index f4c888ef01..bbbd490439 100644 --- a/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go +++ b/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go @@ -3,11 +3,13 @@ package implementations import ( "context" "github.com/NYTimes/gizmo/pubsub" + "github.com/flyteorg/flyteadmin/pkg/async" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flytestdlib/logger" "github.com/flyteorg/flytestdlib/promutils" "github.com/golang/protobuf/proto" + "time" ) // TODO: Add a counter that encompasses the publisher stats grouped by project and domain. @@ -26,6 +28,15 @@ func NewGcpProcessor(sub pubsub.Subscriber, emailer interfaces.Emailer, scope pr } } +func (p *GcpProcessor) StartProcessing() { + for { + logger.Warningf(context.Background(), "Starting GCP notifications processor") + err := p.run() + logger.Errorf(context.Background(), "error with running GCP processor err: [%v] ", err) + time.Sleep(async.RetryDelay) + } +} + func (p *GcpProcessor) run() error { var emailMessage admin.EmailMessage diff --git a/flyteadmin/pkg/async/notifications/interfaces/processor.go b/flyteadmin/pkg/async/notifications/interfaces/processor.go index 08fe15dffa..f0356b8a57 100644 --- a/flyteadmin/pkg/async/notifications/interfaces/processor.go +++ b/flyteadmin/pkg/async/notifications/interfaces/processor.go @@ -4,12 +4,9 @@ import ( "context" "encoding/base64" "encoding/json" - "errors" "github.com/NYTimes/gizmo/pubsub" gizmoGCP "github.com/NYTimes/gizmo/pubsub/gcp" - "github.com/flyteorg/flyteadmin/pkg/async" "github.com/flyteorg/flytestdlib/logger" - "time" ) // Exposes the common methods required for a subscriber. @@ -33,22 +30,6 @@ type BaseProcessor struct { SystemMetrics ProcessorSystemMetrics } -// StartProcessing Currently only email is the supported notification because slack and pagerduty both use -// email client to trigger those notifications. -// When Pagerduty and other notifications are supported, a publisher per type should be created. -func (p *BaseProcessor) StartProcessing() { - for { - logger.Warningf(context.Background(), "Starting notifications processor") - err := p.run() - logger.Errorf(context.Background(), "error with running processor err: [%v] ", err) - time.Sleep(async.RetryDelay) - } -} - -func (p *BaseProcessor) run() error { - return errors.New("run() is not implemented") -} - // FromPubSubMessage Parse the message from GCP PubSub and return the message subject and the message body. func (p *BaseProcessor) FromPubSubMessage(msg pubsub.SubscriberMessage) (string, []byte, bool) { var gcpMsg *gizmoGCP.SubMessage From d6d534341d6cc8421ce976e7afc5fcc67f099286 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Thu, 20 Jul 2023 23:06:49 -0700 Subject: [PATCH 32/35] nit Signed-off-by: Kevin Su --- .../pkg/async/webhook/implementations/slack_webhook.go | 6 +++--- .../pkg/runtime/interfaces/application_configuration.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go index be4f13e726..2a0d87b99a 100644 --- a/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go +++ b/flyteadmin/pkg/async/webhook/implementations/slack_webhook.go @@ -30,7 +30,7 @@ func (s *SlackWebhook) GetConfig() runtimeInterfaces.WebHookConfig { func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) error { sm := secretmanager.NewFileEnvSecretManager(secretmanager.GetConfig()) - webhookURL, err := sm.Get(ctx, s.Config.URL) + webhookURL, err := sm.Get(ctx, s.Config.URLSecretName) if err != nil { logger.Errorf(ctx, "Failed to get url from secret manager with error: %v", err) return err @@ -42,8 +42,8 @@ func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookPayload) e return err } request.Header.Add("Content-Type", "application/json") - if len(s.Config.Token) != 0 { - token, err := sm.Get(ctx, s.Config.Token) + if len(s.Config.TokenSecretName) != 0 { + token, err := sm.Get(ctx, s.Config.TokenSecretName) if err != nil { logger.Errorf(ctx, "Failed to get bearer token from secret manager with error: %v", err) return err diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go index 4e2ed92df6..d02ef27a99 100644 --- a/flyteadmin/pkg/runtime/interfaces/application_configuration.go +++ b/flyteadmin/pkg/runtime/interfaces/application_configuration.go @@ -555,9 +555,9 @@ type NotificationsConfig struct { type WebHookConfig struct { // Type of webhook service to use. Currently only "slack" is supported. Name string `json:"name"` - URL string `json:"url" pflag:",Secret that contains the webhook URL"` + URLSecretName string `json:"urlSecretName" pflag:",Secret name to use for the webhook URL"` Payload string `json:"payload"` - Token string `json:"token" pflag:",Secret that contains the bearer token"` + TokenSecretName string `json:"tokenSecretName" pflag:",Secret name to use for the barer token"` NotificationsProcessorConfig NotificationsProcessorConfig `json:"processor"` } From a3828c8749379b521e3ab8f5e0eeaf1809a1b314 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Thu, 20 Jul 2023 23:08:48 -0700 Subject: [PATCH 33/35] lint Signed-off-by: Kevin Su --- .../pkg/async/notifications/implementations/aws_processor.go | 3 ++- .../pkg/async/notifications/implementations/gcp_processor.go | 3 ++- flyteadmin/pkg/async/notifications/interfaces/processor.go | 1 + flyteadmin/pkg/async/webhook/factory.go | 3 ++- flyteadmin/pkg/async/webhook/implementations/processor_test.go | 3 ++- 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go index f88404b6ca..1800c854ee 100644 --- a/flyteadmin/pkg/async/notifications/implementations/aws_processor.go +++ b/flyteadmin/pkg/async/notifications/implementations/aws_processor.go @@ -2,6 +2,8 @@ package implementations import ( "context" + "time" + "github.com/NYTimes/gizmo/pubsub" "github.com/flyteorg/flyteadmin/pkg/async" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" @@ -9,7 +11,6 @@ import ( "github.com/flyteorg/flytestdlib/logger" "github.com/flyteorg/flytestdlib/promutils" "github.com/golang/protobuf/proto" - "time" ) // TODO: Add a counter that encompasses the publisher stats grouped by project and domain. diff --git a/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go b/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go index bbbd490439..a07e30e1cd 100644 --- a/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go +++ b/flyteadmin/pkg/async/notifications/implementations/gcp_processor.go @@ -2,6 +2,8 @@ package implementations import ( "context" + "time" + "github.com/NYTimes/gizmo/pubsub" "github.com/flyteorg/flyteadmin/pkg/async" "github.com/flyteorg/flyteadmin/pkg/async/notifications/interfaces" @@ -9,7 +11,6 @@ import ( "github.com/flyteorg/flytestdlib/logger" "github.com/flyteorg/flytestdlib/promutils" "github.com/golang/protobuf/proto" - "time" ) // TODO: Add a counter that encompasses the publisher stats grouped by project and domain. diff --git a/flyteadmin/pkg/async/notifications/interfaces/processor.go b/flyteadmin/pkg/async/notifications/interfaces/processor.go index f0356b8a57..27041998dc 100644 --- a/flyteadmin/pkg/async/notifications/interfaces/processor.go +++ b/flyteadmin/pkg/async/notifications/interfaces/processor.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/json" + "github.com/NYTimes/gizmo/pubsub" gizmoGCP "github.com/NYTimes/gizmo/pubsub/gcp" "github.com/flyteorg/flytestdlib/logger" diff --git a/flyteadmin/pkg/async/webhook/factory.go b/flyteadmin/pkg/async/webhook/factory.go index fae0f9ca3b..fb38d35c0a 100644 --- a/flyteadmin/pkg/async/webhook/factory.go +++ b/flyteadmin/pkg/async/webhook/factory.go @@ -3,9 +3,10 @@ package webhook import ( "context" "fmt" + "time" + gizmoGCP "github.com/NYTimes/gizmo/pubsub/gcp" "github.com/flyteorg/flyteadmin/pkg/common" - "time" repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces" diff --git a/flyteadmin/pkg/async/webhook/implementations/processor_test.go b/flyteadmin/pkg/async/webhook/implementations/processor_test.go index 56c9b3848d..f3b959dbfb 100644 --- a/flyteadmin/pkg/async/webhook/implementations/processor_test.go +++ b/flyteadmin/pkg/async/webhook/implementations/processor_test.go @@ -4,10 +4,11 @@ import ( "context" "encoding/base64" "errors" - "github.com/flyteorg/flyteadmin/pkg/common" "testing" "time" + "github.com/flyteorg/flyteadmin/pkg/common" + "github.com/NYTimes/gizmo/pubsub" "github.com/NYTimes/gizmo/pubsub/pubsubtest" "github.com/aws/aws-sdk-go/aws" From b754cf82ce456664ea62485bd640a1218d261469 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Mon, 21 Aug 2023 11:45:16 -0700 Subject: [PATCH 34/35] bump idl Signed-off-by: Kevin Su --- flyteadmin/go.mod | 2 +- flyteadmin/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/flyteadmin/go.mod b/flyteadmin/go.mod index 55804117c1..4e23a0ba96 100644 --- a/flyteadmin/go.mod +++ b/flyteadmin/go.mod @@ -214,4 +214,4 @@ replace github.com/robfig/cron/v3 => github.com/unionai/cron/v3 v3.0.2-0.2022091 // This was published in error when attempting to create 1.5.1 Flyte release. retract v1.1.94 -replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.5.13-0.20230705131852-ba27680aa8b3 +replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.5.17-0.20230821184213-8fa0999bf29f diff --git a/flyteadmin/go.sum b/flyteadmin/go.sum index ba20ba08c3..fbb42df9fd 100644 --- a/flyteadmin/go.sum +++ b/flyteadmin/go.sum @@ -293,8 +293,8 @@ github.com/fatih/structs v1.0.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ= github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= -github.com/flyteorg/flyteidl v1.5.13-0.20230705131852-ba27680aa8b3 h1:UmrrH3OVeyiCuE+R6v+Ka8mDOuyfTlnajf2TYWS+9UM= -github.com/flyteorg/flyteidl v1.5.13-0.20230705131852-ba27680aa8b3/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= +github.com/flyteorg/flyteidl v1.5.17-0.20230821184213-8fa0999bf29f h1:bFNEJZPQ3g/7AW2HEZL9eimfxxuro0smGnTCnCLKjos= +github.com/flyteorg/flyteidl v1.5.17-0.20230821184213-8fa0999bf29f/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= github.com/flyteorg/flyteplugins v1.0.67 h1:d2FXpwxQwX/k4YdmhuusykOemHb/cUTPEob4WBmdpjE= github.com/flyteorg/flyteplugins v1.0.67/go.mod h1:HHt4nKDKVwrZPKDsj99dNtDSIJL378xNotYMA3a/TFA= github.com/flyteorg/flytepropeller v1.1.98 h1:Zk2ENYB9VZRT5tFUIFjm+aCkr0TU2EuyJ5gh52fpLoA= From 25c3a0f854828687d8b08a49f6c9c596ec4dce85 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Mon, 21 Aug 2023 15:29:51 -0700 Subject: [PATCH 35/35] bump idl Signed-off-by: Kevin Su --- flyteadmin/go.mod | 2 +- flyteadmin/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/flyteadmin/go.mod b/flyteadmin/go.mod index 4e23a0ba96..38070c77fa 100644 --- a/flyteadmin/go.mod +++ b/flyteadmin/go.mod @@ -214,4 +214,4 @@ replace github.com/robfig/cron/v3 => github.com/unionai/cron/v3 v3.0.2-0.2022091 // This was published in error when attempting to create 1.5.1 Flyte release. retract v1.1.94 -replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.5.17-0.20230821184213-8fa0999bf29f +replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.5.17-0.20230821222808-485ae223c7f1 diff --git a/flyteadmin/go.sum b/flyteadmin/go.sum index fbb42df9fd..1300e184bd 100644 --- a/flyteadmin/go.sum +++ b/flyteadmin/go.sum @@ -293,8 +293,8 @@ github.com/fatih/structs v1.0.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ= github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= -github.com/flyteorg/flyteidl v1.5.17-0.20230821184213-8fa0999bf29f h1:bFNEJZPQ3g/7AW2HEZL9eimfxxuro0smGnTCnCLKjos= -github.com/flyteorg/flyteidl v1.5.17-0.20230821184213-8fa0999bf29f/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= +github.com/flyteorg/flyteidl v1.5.17-0.20230821222808-485ae223c7f1 h1:B0OlEJujyYKWVkgJ7cqsIVbhaRGwMgdToCIAau8JmAY= +github.com/flyteorg/flyteidl v1.5.17-0.20230821222808-485ae223c7f1/go.mod h1:EtE/muM2lHHgBabjYcxqe9TWeJSL0kXwbI0RgVwI4Og= github.com/flyteorg/flyteplugins v1.0.67 h1:d2FXpwxQwX/k4YdmhuusykOemHb/cUTPEob4WBmdpjE= github.com/flyteorg/flyteplugins v1.0.67/go.mod h1:HHt4nKDKVwrZPKDsj99dNtDSIJL378xNotYMA3a/TFA= github.com/flyteorg/flytepropeller v1.1.98 h1:Zk2ENYB9VZRT5tFUIFjm+aCkr0TU2EuyJ5gh52fpLoA=