Skip to content

Commit

Permalink
Adds appProtocol values of tcp on services
Browse files Browse the repository at this point in the history
 * flyteadmin http port
 * flyteadmin grpc port
 * flyteconsole grpc port

   This is necessary because the ingress may be configured in a way that
   it sends TLS traffic to internal Flyte services. Istio will use port
   names to determine traffic - and may therefore assume the appProtocol
   of http, even though traffic from ingress -> flyteadmin is actually
   https. This misconfiguration prevents any traffic from flowing
   through the ingress to the service.

   Flyteadmin http and grcp ports *are* accessible using `http` and
   `grpc` values for appProtocol respectively within the cluster, but as
   soon as traffic travels between the ingress and the service those settings
   will not work. The most "compatible" setting is `tcp` which works for
   any network stream.

 - Adds the nginx.ingress.kubernetes.io/service-upstream: "true"

   Nginx Controller using endpoints instead of Services kubernetes/ingress-nginx#257
   kubernetes/ingress-nginx@main/docs/user-guide/nginx-configuration/annotations.md#service-upstream

Signed-off-by: noahjax <[email protected]>
Signed-off-by: ddl-ebrown <[email protected]>
  • Loading branch information
noahjax authored and ddl-ebrown committed Jun 25, 2024
1 parent 37fe0a3 commit 8590993
Show file tree
Hide file tree
Showing 17 changed files with 70 additions and 9 deletions.
4 changes: 2 additions & 2 deletions charts/flyte-core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ helm install gateway bitnami/contour -n flyte
| cluster_resource_manager.service_account_name | string | `"flyteadmin"` | Service account name to run with |
| cluster_resource_manager.templates | list | `[{"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"}]` | Resource templates that should be applied |
| cluster_resource_manager.templates[0] | object | `{"key":"aa_namespace","value":"apiVersion: v1\nkind: Namespace\nmetadata:\n name: {{ namespace }}\nspec:\n finalizers:\n - kubernetes\n"}` | Template for namespaces resources |
| common | object | `{"databaseSecret":{"name":"","secretManifest":{}},"flyteNamespaceTemplate":{"enabled":false},"ingress":{"albSSLRedirect":false,"annotations":{"nginx.ingress.kubernetes.io/app-root":"/console"},"enabled":true,"ingressClassName":null,"separateGrpcIngress":false,"separateGrpcIngressAnnotations":{"nginx.ingress.kubernetes.io/backend-protocol":"GRPC"},"tls":{"enabled":false},"webpackHMR":false}}` | ---------------------------------------------- COMMON SETTINGS |
| common | object | `{"databaseSecret":{"name":"","secretManifest":{}},"flyteNamespaceTemplate":{"enabled":false},"ingress":{"albSSLRedirect":false,"annotations":{"nginx.ingress.kubernetes.io/app-root":"/console","nginx.ingress.kubernetes.io/service-upstream":"true"},"enabled":true,"ingressClassName":null,"separateGrpcIngress":false,"separateGrpcIngressAnnotations":{"nginx.ingress.kubernetes.io/backend-protocol":"GRPC"},"tls":{"enabled":false},"webpackHMR":false}}` | ---------------------------------------------- COMMON SETTINGS |
| common.databaseSecret.name | string | `""` | Specify name of K8s Secret which contains Database password. Leave it empty if you don't need this Secret |
| common.databaseSecret.secretManifest | object | `{}` | Specify your Secret (with sensitive data) or pseudo-manifest (without sensitive data). See https://github.com/godaddy/kubernetes-external-secrets |
| common.flyteNamespaceTemplate.enabled | bool | `false` | - Enable or disable creating Flyte namespace in template. Enable when using helm as template-engine only. Disable when using `helm install ...`. |
| common.ingress.albSSLRedirect | bool | `false` | - albSSLRedirect adds a special route for ssl redirect. Only useful in combination with the AWS LoadBalancer Controller. |
| common.ingress.annotations | object | `{"nginx.ingress.kubernetes.io/app-root":"/console"}` | - Ingress annotations applied to both HTTP and GRPC ingresses. |
| common.ingress.annotations | object | `{"nginx.ingress.kubernetes.io/app-root":"/console","nginx.ingress.kubernetes.io/service-upstream":"true"}` | - Ingress annotations applied to both HTTP and GRPC ingresses. |
| common.ingress.enabled | bool | `true` | - Enable or disable creating Ingress for Flyte. Relevant to disable when using e.g. Istio as ingress controller. |
| common.ingress.ingressClassName | string | `nil` | - Sets the ingressClassName |
| common.ingress.separateGrpcIngress | bool | `false` | - separateGrpcIngress puts GRPC routes into a separate ingress if true. Required for certain ingress controllers like nginx. |
Expand Down
5 changes: 5 additions & 0 deletions charts/flyte-core/templates/admin/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,22 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8088
- name: grpc
port: 81
protocol: TCP
# intentionally set to TCP instead of grpc
appProtocol: TCP
targetPort: 8089
- name: redoc
protocol: TCP
appProtocol: TCP
port: 87
targetPort: 8087
- name: http-metrics
protocol: TCP
appProtocol: TCP
port: 10254
{{- with .Values.flyteadmin.service.additionalPorts -}}
{{ tpl (toYaml .) $ | nindent 4 }}
Expand Down
1 change: 1 addition & 0 deletions charts/flyte-core/templates/console/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8080
{{- if .Values.flyteconsole.serviceMonitor.enabled }}
- name: http-metrics
Expand Down
1 change: 1 addition & 0 deletions charts/flyte-core/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ common:
# --- Ingress annotations applied to both HTTP and GRPC ingresses.
annotations:
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/service-upstream: "true"
# --- albSSLRedirect adds a special route for ssl redirect. Only useful in combination with the AWS LoadBalancer Controller.
albSSLRedirect: false
# --- Ingress hostname
Expand Down
1 change: 1 addition & 0 deletions charts/flyteagent/templates/agent/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ spec:
- name: {{ .Values.ports.name }}
port: {{ .Values.ports.containerPort }}
protocol: TCP
appProtocol: TCP
targetPort: {{ .Values.ports.name }}
selector: {{ include "flyteagent.selectorLabels" . | nindent 4 }}
1 change: 1 addition & 0 deletions deployment/agent/flyte_agent_helm_generated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ spec:
- name: agent-grpc
port: 8000
protocol: TCP
appProtocol: TCP
targetPort: agent-grpc
selector:
app.kubernetes.io/name: flyteagent
Expand Down
8 changes: 8 additions & 0 deletions deployment/eks/flyte_aws_scheduler_helm_generated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -745,17 +745,22 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8088
- name: grpc
port: 81
protocol: TCP
# intentionally set to TCP instead of grpc
appProtocol: TCP
targetPort: 8089
- name: redoc
protocol: TCP
appProtocol: TCP
port: 87
targetPort: 8087
- name: http-metrics
protocol: TCP
appProtocol: TCP
port: 10254
selector:
app.kubernetes.io/name: flyteadmin
Expand All @@ -778,6 +783,7 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8080
selector:
app.kubernetes.io/name: flyteconsole
Expand Down Expand Up @@ -1456,6 +1462,7 @@ metadata:
alb.ingress.kubernetes.io/target-type: ip
kubernetes.io/ingress.class: alb
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/service-upstream: "true"
spec:
ingressClassName:
rules:
Expand Down Expand Up @@ -1626,6 +1633,7 @@ metadata:
kubernetes.io/ingress.class: alb
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/backend-protocol: GRPC
nginx.ingress.kubernetes.io/service-upstream: "true"
spec:
ingressClassName:
rules:
Expand Down
8 changes: 8 additions & 0 deletions deployment/eks/flyte_helm_controlplane_generated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -468,17 +468,22 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8088
- name: grpc
port: 81
protocol: TCP
# intentionally set to TCP instead of grpc
appProtocol: TCP
targetPort: 8089
- name: redoc
protocol: TCP
appProtocol: TCP
port: 87
targetPort: 8087
- name: http-metrics
protocol: TCP
appProtocol: TCP
port: 10254
selector:
app.kubernetes.io/name: flyteadmin
Expand All @@ -501,6 +506,7 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8080
selector:
app.kubernetes.io/name: flyteconsole
Expand Down Expand Up @@ -1072,6 +1078,7 @@ metadata:
alb.ingress.kubernetes.io/target-type: ip
kubernetes.io/ingress.class: alb
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/service-upstream: "true"
spec:
ingressClassName:
rules:
Expand Down Expand Up @@ -1242,6 +1249,7 @@ metadata:
kubernetes.io/ingress.class: alb
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/backend-protocol: GRPC
nginx.ingress.kubernetes.io/service-upstream: "true"
spec:
ingressClassName:
rules:
Expand Down
2 changes: 2 additions & 0 deletions deployment/eks/flyte_helm_dataplane_generated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,7 @@ metadata:
alb.ingress.kubernetes.io/target-type: ip
kubernetes.io/ingress.class: alb
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/service-upstream: "true"
spec:
ingressClassName:
rules:
Expand Down Expand Up @@ -783,6 +784,7 @@ metadata:
kubernetes.io/ingress.class: alb
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/backend-protocol: GRPC
nginx.ingress.kubernetes.io/service-upstream: "true"
spec:
ingressClassName:
rules:
Expand Down
8 changes: 8 additions & 0 deletions deployment/eks/flyte_helm_generated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -776,17 +776,22 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8088
- name: grpc
port: 81
protocol: TCP
# intentionally set to TCP instead of grpc
appProtocol: TCP
targetPort: 8089
- name: redoc
protocol: TCP
appProtocol: TCP
port: 87
targetPort: 8087
- name: http-metrics
protocol: TCP
appProtocol: TCP
port: 10254
selector:
app.kubernetes.io/name: flyteadmin
Expand All @@ -809,6 +814,7 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8080
selector:
app.kubernetes.io/name: flyteconsole
Expand Down Expand Up @@ -1586,6 +1592,7 @@ metadata:
alb.ingress.kubernetes.io/target-type: ip
kubernetes.io/ingress.class: alb
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/service-upstream: "true"
spec:
ingressClassName:
rules:
Expand Down Expand Up @@ -1756,6 +1763,7 @@ metadata:
kubernetes.io/ingress.class: alb
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/backend-protocol: GRPC
nginx.ingress.kubernetes.io/service-upstream: "true"
spec:
ingressClassName:
rules:
Expand Down
8 changes: 8 additions & 0 deletions deployment/gcp/flyte_helm_controlplane_generated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -482,17 +482,22 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8088
- name: grpc
port: 81
protocol: TCP
# intentionally set to TCP instead of grpc
appProtocol: TCP
targetPort: 8089
- name: redoc
protocol: TCP
appProtocol: TCP
port: 87
targetPort: 8087
- name: http-metrics
protocol: TCP
appProtocol: TCP
port: 10254
selector:
app.kubernetes.io/name: flyteadmin
Expand All @@ -515,6 +520,7 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8080
selector:
app.kubernetes.io/name: flyteconsole
Expand Down Expand Up @@ -1080,6 +1086,7 @@ metadata:
cert-manager.io/issuer: letsencrypt-production
kubernetes.io/ingress.class: nginx
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/service-upstream: "true"
nginx.ingress.kubernetes.io/ssl-redirect: "true"
spec:
ingressClassName:
Expand Down Expand Up @@ -1241,6 +1248,7 @@ metadata:
kubernetes.io/ingress.class: nginx
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/backend-protocol: GRPC
nginx.ingress.kubernetes.io/service-upstream: "true"
nginx.ingress.kubernetes.io/ssl-redirect: "true"
spec:
ingressClassName:
Expand Down
2 changes: 2 additions & 0 deletions deployment/gcp/flyte_helm_dataplane_generated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,7 @@ metadata:
cert-manager.io/issuer: letsencrypt-production
kubernetes.io/ingress.class: nginx
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/service-upstream: "true"
nginx.ingress.kubernetes.io/ssl-redirect: "true"
spec:
ingressClassName:
Expand Down Expand Up @@ -774,6 +775,7 @@ metadata:
kubernetes.io/ingress.class: nginx
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/backend-protocol: GRPC
nginx.ingress.kubernetes.io/service-upstream: "true"
nginx.ingress.kubernetes.io/ssl-redirect: "true"
spec:
ingressClassName:
Expand Down
8 changes: 8 additions & 0 deletions deployment/gcp/flyte_helm_generated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -798,17 +798,22 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8088
- name: grpc
port: 81
protocol: TCP
# intentionally set to TCP instead of grpc
appProtocol: TCP
targetPort: 8089
- name: redoc
protocol: TCP
appProtocol: TCP
port: 87
targetPort: 8087
- name: http-metrics
protocol: TCP
appProtocol: TCP
port: 10254
selector:
app.kubernetes.io/name: flyteadmin
Expand All @@ -831,6 +836,7 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8080
selector:
app.kubernetes.io/name: flyteconsole
Expand Down Expand Up @@ -1601,6 +1607,7 @@ metadata:
cert-manager.io/issuer: letsencrypt-production
kubernetes.io/ingress.class: nginx
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/service-upstream: "true"
nginx.ingress.kubernetes.io/ssl-redirect: "true"
spec:
ingressClassName:
Expand Down Expand Up @@ -1762,6 +1769,7 @@ metadata:
kubernetes.io/ingress.class: nginx
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/backend-protocol: GRPC
nginx.ingress.kubernetes.io/service-upstream: "true"
nginx.ingress.kubernetes.io/ssl-redirect: "true"
spec:
ingressClassName:
Expand Down
7 changes: 7 additions & 0 deletions deployment/sandbox/flyte_helm_generated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6139,17 +6139,22 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8088
- name: grpc
port: 81
protocol: TCP
# intentionally set to TCP instead of grpc
appProtocol: TCP
targetPort: 8089
- name: redoc
protocol: TCP
appProtocol: TCP
port: 87
targetPort: 8087
- name: http-metrics
protocol: TCP
appProtocol: TCP
port: 10254
selector:
app.kubernetes.io/name: flyteadmin
Expand All @@ -6172,6 +6177,7 @@ spec:
- name: http
port: 80
protocol: TCP
appProtocol: TCP
targetPort: 8080
selector:
app.kubernetes.io/name: flyteconsole
Expand Down Expand Up @@ -7612,6 +7618,7 @@ metadata:
namespace: flyte
annotations:
nginx.ingress.kubernetes.io/app-root: /console
nginx.ingress.kubernetes.io/service-upstream: "true"
spec:
ingressClassName:
rules:
Expand Down
Loading

0 comments on commit 8590993

Please sign in to comment.