Address comments

Signed-off-by: Jason Parraga <[email protected]>
flyteorg · Oct 30, 2024 · 6d7a27f · 6d7a27f
1 parent 0827d74
commit 6d7a27f
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 24 deletions.
diff --git a/charts/flyte-core/README.md b/charts/flyte-core/README.md
@@ -55,24 +55,25 @@ helm install gateway bitnami/contour -n flyte
 
 | Key | Type | Default | Description |
 |-----|------|---------|-------------|
-| cloud_events.aws.region | string | `"us-east-2"` |  |
+| cloud_events.aws | object | `{"region":"us-east-2"}` | Configuration for sending cloud events to AWS SNS |
 | cloud_events.enable | bool | `false` |  |
 | cloud_events.eventsPublisher.eventTypes[0] | string | `"all"` |  |
 | cloud_events.eventsPublisher.topicName | string | `"arn:aws:sns:us-east-2:123456:123-my-topic"` |  |
-| cloud_events.kafka | object | `{"brokers":["mybroker:443"],"saslConfig":{"enabled":false,"handshake":true,"mechanism":"PLAIN","password":"","user":"kafka"},"tlsConfig":{"certPath":"/etc/ssl/certs/kafka-client.crt","enabled":false,"keyPath":"/etc/ssl/certs/kafka-client.key"},"version":"3.7.0"}` | Configuration for sending cloud events to Kafka |
+| cloud_events.gcp | object | `{"region":"us-east1"}` | Configuration for sending cloud events to GCP Pub Sub |
+| cloud_events.kafka | object | `{"brokers":["mybroker:443"],"saslConfig":{"enabled":false,"handshake":true,"mechanism":"PLAIN","password":"","passwordPath":"","user":"kafka"},"tlsConfig":{"certPath":"/etc/ssl/certs/kafka-client.crt","enabled":false,"keyPath":"/etc/ssl/certs/kafka-client.key"},"version":"3.7.0"}` | Configuration for sending cloud events to Kafka |
 | cloud_events.kafka.brokers | list | `["mybroker:443"]` | The kafka brokers to talk to |
-| cloud_events.kafka.saslConfig | object | `{"enabled":false,"handshake":true,"mechanism":"PLAIN","password":"","user":"kafka"}` | SASL based authentication |
+| cloud_events.kafka.saslConfig | object | `{"enabled":false,"handshake":true,"mechanism":"PLAIN","password":"","passwordPath":"","user":"kafka"}` | SASL based authentication |
 | cloud_events.kafka.saslConfig.enabled | bool | `false` | Whether to use SASL authentication |
 | cloud_events.kafka.saslConfig.handshake | bool | `true` | Whether the send the SASL handsahke first |
 | cloud_events.kafka.saslConfig.mechanism | string | `"PLAIN"` | Which SASL mechanism to use. Defaults to PLAIN |
 | cloud_events.kafka.saslConfig.password | string | `""` | The password for the kafka user |
+| cloud_events.kafka.saslConfig.passwordPath | string | `""` | Optional mount path of file containing the kafka password. |
 | cloud_events.kafka.saslConfig.user | string | `"kafka"` | The kafka user |
 | cloud_events.kafka.tlsConfig | object | `{"certPath":"/etc/ssl/certs/kafka-client.crt","enabled":false,"keyPath":"/etc/ssl/certs/kafka-client.key"}` | Certificate based authentication |
 | cloud_events.kafka.tlsConfig.certPath | string | `"/etc/ssl/certs/kafka-client.crt"` | Path to the client certificate |
 | cloud_events.kafka.tlsConfig.enabled | bool | `false` | Whether to use certificate based authentication or TLS |
 | cloud_events.kafka.tlsConfig.keyPath | string | `"/etc/ssl/certs/kafka-client.key"` | Path to the client private key |
 | cloud_events.kafka.version | string | `"3.7.0"` | The version of Kafka |
-| cloud_events.secretName | string | `""` | The name of the secret to use to alternatively load in cloud events configuration via a secret. Useful when the configuration contains secrets. |
 | cloud_events.type | string | `"aws"` |  |
 | cluster_resource_manager | object | `{"config":{"cluster_resources":{"customData":[{"production":[{"projectQuotaCpu":{"value":"5"}},{"projectQuotaMemory":{"value":"4000Mi"}}]},{"staging":[{"projectQuotaCpu":{"value":"2"}},{"projectQuotaMemory":{"value":"3000Mi"}}]},{"development":[{"projectQuotaCpu":{"value":"4"}},{"projectQuotaMemory":{"value":"3000Mi"}}]}],"refreshInterval":"5m","standaloneDeployment":false,"templatePath":"/etc/flyte/clusterresource/templates"}},"enabled":true,"nodeSelector":{},"podAnnotations":{},"podEnv":{},"podLabels":{},"prometheus":{"enabled":false,"path":"/metrics","port":10254},"resources":{},"service_account_name":"flyteadmin","standaloneDeployment":false,"templates":[{"key":"aa_namespace","value":"apiVersion: v1\nkind: Namespace\nmetadata:\n  name: {{ namespace }}\nspec:\n  finalizers:\n  - kubernetes\n"},{"key":"ab_project_resource_quota","value":"apiVersion: v1\nkind: ResourceQuota\nmetadata:\n  name: project-quota\n  namespace: {{ namespace }}\nspec:\n  hard:\n    limits.cpu: {{ projectQuotaCpu }}\n    limits.memory: {{ projectQuotaMemory }}\n"}]}` | Configuration for the Cluster resource manager component. This is an optional component, that enables automatic cluster configuration. This is useful to set default quotas, manage namespaces etc that map to a project/domain |
 | cluster_resource_manager.config | object | `{"cluster_resources":{"customData":[{"production":[{"projectQuotaCpu":{"value":"5"}},{"projectQuotaMemory":{"value":"4000Mi"}}]},{"staging":[{"projectQuotaCpu":{"value":"2"}},{"projectQuotaMemory":{"value":"3000Mi"}}]},{"development":[{"projectQuotaCpu":{"value":"4"}},{"projectQuotaMemory":{"value":"3000Mi"}}]}],"refreshInterval":"5m","standaloneDeployment":false,"templatePath":"/etc/flyte/clusterresource/templates"}}` | Configmap for ClusterResource parameters |

diff --git a/charts/flyte-core/templates/admin/configmap.yaml b/charts/flyte-core/templates/admin/configmap.yaml
@@ -79,7 +79,7 @@ data:
     externalEvents: {{ tpl (toYaml .) $ | nindent 6 }}
 {{- end }}
 {{- end }}
-{{- if and .Values.cloud_events.enable (not .Values.cloud_events.secretName) }}
+{{- if .Values.cloud_events.enable }}
 {{- with .Values.cloud_events }}
   cloud_events.yaml: |
     cloudEvents: {{ tpl (toYaml .) $ | nindent 6 }}

diff --git a/charts/flyte-core/templates/admin/deployment.yaml b/charts/flyte-core/templates/admin/deployment.yaml
@@ -196,10 +196,6 @@ spec:
                 name: flyte-admin-base-config
             - configMap:
                 name: flyte-admin-clusters-config
-            {{- if .Values.cloud_events.secretName }}
-            - secret:
-                name: {{ .Values.cloud_events.secretName }}
-            {{- end }}
         name: clusters-config-volume
       {{- if .Values.cluster_resource_manager.enabled }}
       - configMap:

diff --git a/charts/flyte-core/values.yaml b/charts/flyte-core/values.yaml
@@ -945,19 +945,20 @@ external_events:
 # Cloud events are used to send events (unprocessed, as Admin see them) in cloud event format to
 # an SNS topic (or gcp equivalent)
 cloud_events:
-  # -- The name of the secret to use to alternatively load in cloud events configuration via a secret. Useful when the
-  # configuration contains secrets.
-  secretName: ""
   enable: false
-  type: aws
-  aws:
-    region: us-east-2
   eventsPublisher:
     # Make sure this is not a fifo queue. Admin does not yet support
     # writing to fifo sns topics.
     topicName: "arn:aws:sns:us-east-2:123456:123-my-topic"
     eventTypes:
       - all  # Or workflow, node, task. Or "*"
+  type: aws
+  # -- Configuration for sending cloud events to AWS SNS
+  aws:
+    region: us-east-2
+  # -- Configuration for sending cloud events to GCP Pub Sub
+  gcp:
+    region: us-east1
   # -- Configuration for sending cloud events to Kafka
   kafka:
     # -- The version of Kafka
@@ -973,6 +974,8 @@ cloud_events:
       user: kafka
       # -- The password for the kafka user
       password: ""
+      # -- Optional mount path of file containing the kafka password.
+      passwordPath: ""
       # -- Whether the send the SASL handsahke first
       handshake: true
       # -- Which SASL mechanism to use. Defaults to PLAIN

diff --git a/docker/sandbox-bundled/manifests/complete-agent.yaml b/docker/sandbox-bundled/manifests/complete-agent.yaml
@@ -819,7 +819,7 @@ type: Opaque
 ---
 apiVersion: v1
 data:
-  haSharedSecret: Mm96eUJNNUlWUzB6dG5xag==
+  haSharedSecret: SlI1TDFkTXBMaThuc0hlSQ==
   proxyPassword: ""
   proxyUsername: ""
 kind: Secret
@@ -1416,7 +1416,7 @@ spec:
     metadata:
       annotations:
         checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81
-        checksum/secret: d313245b88895f79af2db62a30442bfaf128d845a6f11fbec7d80e8b342ed247
+        checksum/secret: ffc8aa05a602edd8f9b1d7ef35aa1cc5e383bceb9b91307eef99e86f53e13d4e
       labels:
         app: docker-registry
         release: flyte-sandbox

diff --git a/docker/sandbox-bundled/manifests/complete.yaml b/docker/sandbox-bundled/manifests/complete.yaml
@@ -801,7 +801,7 @@ type: Opaque
 ---
 apiVersion: v1
 data:
-  haSharedSecret: RHRoWE50MnFPOEUxMmZuNA==
+  haSharedSecret: YjdMdE9yejJzZ2xXSDFBRQ==
   proxyPassword: ""
   proxyUsername: ""
 kind: Secret
@@ -1365,7 +1365,7 @@ spec:
     metadata:
       annotations:
         checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81
-        checksum/secret: 3b81a8307491b87f506cfc21f0ba759872ef0b6666427399fa52db75188b6f7c
+        checksum/secret: 956ac1b58c049a630c94605eedaba7ba9de3fc01233701ef403ab4bf24fe2a7a
       labels:
         app: docker-registry
         release: flyte-sandbox

diff --git a/docker/sandbox-bundled/manifests/dev.yaml b/docker/sandbox-bundled/manifests/dev.yaml
@@ -499,7 +499,7 @@ metadata:
 ---
 apiVersion: v1
 data:
-  haSharedSecret: VVF2SndnTXM3cEVGbFM3Mw==
+  haSharedSecret: YUpzb25xNTM1eml3Rmpueg==
   proxyPassword: ""
   proxyUsername: ""
 kind: Secret
@@ -934,7 +934,7 @@ spec:
     metadata:
       annotations:
         checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81
-        checksum/secret: ca5ab8524ec246e8321ad14b55284b4c6f8a488ddfa80377989c1529fa51af45
+        checksum/secret: 2720f13bd64051a7acb512e59e426b9f6c5f6c3c7d1d9a3a423e2df4cf9bab46
       labels:
         app: docker-registry
         release: flyte-sandbox

diff --git a/flyteadmin/pkg/runtime/interfaces/application_configuration.go b/flyteadmin/pkg/runtime/interfaces/application_configuration.go
@@ -3,6 +3,9 @@ package interfaces
 import (
 	"context"
 	"crypto/tls"
+	"fmt"
+	"os"
+	"strings"
 
 	"github.com/Shopify/sarama"
 	"github.com/golang/protobuf/ptypes/wrappers"
@@ -242,8 +245,9 @@ type SASLConfig struct {
 	// The username
 	User string `json:"user"`
 	// The password
-	Password  string `json:"password"`
-	Handshake bool   `json:"handshake"`
+	Password     string `json:"password"`
+	PasswordPath string `json:"passwordPath"`
+	Handshake    bool   `json:"handshake"`
 	// Which SASL Mechanism to use. Defaults to PLAIN
 	Mechanism sarama.SASLMechanism `json:"mechanism"`
 }
@@ -282,7 +286,20 @@ func (k KafkaConfig) UpdateSaramaConfig(ctx context.Context, s *sarama.Config) {
 	if k.SASLConfig.Enabled {
 		s.Net.SASL.Enable = true
 		s.Net.SASL.User = k.SASLConfig.User
-		s.Net.SASL.Password = k.SASLConfig.Password
+
+		if len(k.SASLConfig.PasswordPath) > 0 {
+			if _, err := os.Stat(k.SASLConfig.PasswordPath); os.IsNotExist(err) {
+				panic(fmt.Sprintf("missing kafka password at the specified path [%s]", k.SASLConfig.PasswordPath))
+			}
+			passwordVal, err := os.ReadFile(k.SASLConfig.PasswordPath)
+			if err != nil {
+				panic(fmt.Sprintf("failed to kafka password from path [%s] with err: %v", k.SASLConfig.PasswordPath, err))
+			}
+
+			s.Net.SASL.Password = strings.TrimSpace(string(passwordVal))
+		} else {
+			s.Net.SASL.Password = k.SASLConfig.Password
+		}
 		s.Net.SASL.Handshake = k.SASLConfig.Handshake
 
 		if k.SASLConfig.Mechanism == "" {