[BUG] Missing GCP field in notifications configuration

[ariefrahmansyah]

Describe the bug

When trying to update to Flyte 1.2.0, helm diff command shows notifications.gcp field in flyte-admin-base-config would be deleted:

-       gcp:
-         projectId: gcp-project-id

This gcp field got removed on Helm templates in #2781. However, the gcp field seems still needed as shown in flyteadmin config

Expected behavior

The notifications.gcp field should not be deleted.

Additional context to reproduce

In values.yaml, you must set notifications.gcp:

workflow_notifications:
  config:
    notifications:
      gcp:
        projectId: gcp-project-id

Screenshots

No response

Are you sure this issue hasn't been raised already?

• Yes

Have you read the Code of Conduct?

• Yes

[github-actions]

Hello 👋, This issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will close the issue if we detect no activity in the next 7 days. Thank you for your contribution and understanding! 🙏

[dyu-bot]

Hi there, also wanted to bump this. We're currently blocked on rolling out GCP based notifications due to this

"level":"warning","msg":"Failed [project id is required] on attempt 0 of 0","ts":"2023-09-19T15:23:18Z"}
{"json":{"src":"base.go:75"},"level":"fatal","msg":"caught panic: project id is required [goroutine 1 [running]:\nruntime/debug.Stack()\n\t/usr/local/go/src/runtime/debug/stack.go:24 +0x65\ngithub.com/flyteorg/flyteadmin/pkg/rpc/adminservice.NewAdminServer.func1()\n\t/go/src/github.com/flyteorg/flyteadmin/pkg/rpc/adminservice/base.go:75 +0x88\npanic({0x237cf40, 0xc000addad0})\n\t/usr/local/go/src/runtime/panic.go:884 +0x212\ngithub.com/flyteorg/flyteadmin/pkg/async/notifications.NewNotificationsPublisher({{0xc000c9d068, 0x3}, {0xc000c9d080, 0x3}, {{0x0, 0x0}}, {{0x0, 0x0}}, {{0xc000c9aeb8, 0x13}}, ...}, ...)\n\t/go/src/github.com/flyteorg/flyteadmin/pkg/async/notifications/factory.go:185

[dyu-bot]

Adding something like this should fix:

      {{- if eq .Values.workflow_notifications.config.notifications.type "aws" }}
      {{- with .Values.workflow_notifications.config.notifications.aws }}
      aws: {{ tpl (toYaml .) $ | nindent 8 }}
      {{- end }}
      {{- end }}
      {{- if eq .Values.workflow_notifications.config.notifications.type "gcp" }}
      {{- with .Values.workflow_notifications.config.notifications.gcp }}
      gcp: {{ tpl (toYaml .) $ | nindent 8 }}
      {{- end }}
      {{- end }}

Also region is deprecated I think? So can be removed

[dyu-bot]

Btw, fix PR is up, see below