From f52164d51338cbbcb2b29c9ecfba1dc461ec5f80 Mon Sep 17 00:00:00 2001 From: "Fabio M. Graetz, Ph.D" Date: Fri, 8 Mar 2024 19:22:37 +0100 Subject: [PATCH 1/5] Fix: Sanitize user identity before injecting into task pod as K8s label (#5023) * Fix: Sanitize user identity before injecting into task pod as K8s label Signed-off-by: Fabio Graetz * Lint Signed-off-by: Fabio Graetz --------- Signed-off-by: Fabio Graetz --- .../nodes/task/k8s/task_exec_context.go | 4 +++- .../nodes/task/k8s/task_exec_context_test.go | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/flytepropeller/pkg/controller/nodes/task/k8s/task_exec_context.go b/flytepropeller/pkg/controller/nodes/task/k8s/task_exec_context.go index 17bbce5398..bb987acbc2 100644 --- a/flytepropeller/pkg/controller/nodes/task/k8s/task_exec_context.go +++ b/flytepropeller/pkg/controller/nodes/task/k8s/task_exec_context.go @@ -5,6 +5,7 @@ import ( pluginsCore "github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/core" "github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/utils" "github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/utils/secrets" + k8sUtils "github.com/flyteorg/flyte/flytepropeller/pkg/utils" ) const executionIdentityVariable = "execution-identity" @@ -60,7 +61,8 @@ func newTaskExecutionMetadata(tCtx pluginsCore.TaskExecutionMetadata, taskTmpl * id := tCtx.GetSecurityContext().RunAs.ExecutionIdentity if len(id) > 0 { - injectLabels[executionIdentityVariable] = id + sanitizedID := k8sUtils.SanitizeLabelValue(id) + injectLabels[executionIdentityVariable] = sanitizedID } return TaskExecutionMetadata{ diff --git a/flytepropeller/pkg/controller/nodes/task/k8s/task_exec_context_test.go b/flytepropeller/pkg/controller/nodes/task/k8s/task_exec_context_test.go index bf9ca1eadb..e3c6f10ab6 100644 --- a/flytepropeller/pkg/controller/nodes/task/k8s/task_exec_context_test.go +++ b/flytepropeller/pkg/controller/nodes/task/k8s/task_exec_context_test.go @@ -86,6 +86,25 @@ func Test_newTaskExecutionMetadata(t *testing.T) { assert.Equal(t, 2, len(actual.GetLabels())) assert.Equal(t, "test-exec-identity", actual.GetLabels()[executionIdentityVariable]) }) + t.Run("Inject exec identity K8s label sanitation", func(t *testing.T) { + + existingMetadata := &mocks.TaskExecutionMetadata{} + existingAnnotations := map[string]string{} + existingMetadata.OnGetAnnotations().Return(existingAnnotations) + + existingMetadata.OnGetSecurityContext().Return(core.SecurityContext{RunAs: &core.Identity{ExecutionIdentity: "name@company.com"}}) + + existingLabels := map[string]string{ + "existingLabel": "existingLabelValue", + } + existingMetadata.OnGetLabels().Return(existingLabels) + + actual, err := newTaskExecutionMetadata(existingMetadata, &core.TaskTemplate{}) + assert.NoError(t, err) + + assert.Equal(t, 2, len(actual.GetLabels())) + assert.Equal(t, "name-company-com", actual.GetLabels()[executionIdentityVariable]) + }) } func Test_newTaskExecutionContext(t *testing.T) { From 028ff987cb45478e07474953528f947ad6132e48 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com> Date: Fri, 8 Mar 2024 11:19:03 -0800 Subject: [PATCH 2/5] Hide generated proto stubs by default (#4947) * Add flyteidl/gen to .gitattributes Signed-off-by: Eduardo Apolinario * make generate to show a change Signed-off-by: Eduardo Apolinario * Prepend forward slash to rule Signed-off-by: Eduardo Apolinario * Hide copied swagger.json Signed-off-by: Eduardo Apolinario * Move .gitattributes from flyteidl to root Signed-off-by: Eduardo Apolinario * Revert test attribute change Signed-off-by: Eduardo Apolinario --------- Signed-off-by: Eduardo Apolinario Co-authored-by: Eduardo Apolinario --- .gitattributes | 4 ++++ flyteidl/.gitattributes | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) delete mode 100644 flyteidl/.gitattributes diff --git a/.gitattributes b/.gitattributes index e4b260b693..cbaca9f35c 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,5 @@ docs/**/*html linguist-generated=true +flyteidl/gen/** linguist-generated=true +flyteidl/protos/**/*.rst linguist-generated=true +flyteidl/clients/go/assets/admin.swagger.json linguist-generated=true + diff --git a/flyteidl/.gitattributes b/flyteidl/.gitattributes deleted file mode 100644 index a2236d5f13..0000000000 --- a/flyteidl/.gitattributes +++ /dev/null @@ -1,2 +0,0 @@ -gen/** linguist-generated=true -protos/**/*.rst linguist-generated=true From d0245d6d0d343382d4b1915cacb43145eb8e6b98 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com> Date: Fri, 8 Mar 2024 14:00:21 -0800 Subject: [PATCH 3/5] Fix yaml comment in pod template examples (#5024) * Replace character to denote a comment in podtemplate examples Signed-off-by: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com> * Update the other example Signed-off-by: Eduardo Apolinario --------- Signed-off-by: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com> Signed-off-by: Eduardo Apolinario Co-authored-by: Eduardo Apolinario --- docs/deployment/configuration/general.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/deployment/configuration/general.rst b/docs/deployment/configuration/general.rst index b5278365d2..0ce9d259a4 100644 --- a/docs/deployment/configuration/general.rst +++ b/docs/deployment/configuration/general.rst @@ -241,19 +241,19 @@ The resultant Pod using the above default PodTemplate and K8s Plugin configurati name: example-pod namespace: flytesnacks-development labels: - - foo // maintained initial value - - bar // value appended by k8s plugin configuration + - foo # maintained initial value + - bar # value appended by k8s plugin configuration annotations: - - foo: overridden-value // value overridden by k8s plugin configuration - - bar: initial-value // maintained initial value - - baz: non-overridden-value // value added by k8s plugin configuration + - foo: overridden-value # value overridden by k8s plugin configuration + - bar: initial-value # maintained initial value + - baz: non-overridden-value # value added by k8s plugin configuration spec: containers: - name: ax9kd5xb4p8r45bpdv7v-n0-0 image: ghcr.io/flyteorg/flytecookbook:core-bfee7e549ad749bfb55922e130f4330a0ebc25b0 terminationMessagePath: "/dev/foo" - // remaining container configuration omitted - hostNetwork: true // overridden by the k8s plugin configuration + # remaining container configuration omitted + hostNetwork: true # overridden by the k8s plugin configuration The last step in constructing a Pod is to apply any task-specific configuration. These options follow the same rules as merging the default PodTemplate and K8s @@ -343,7 +343,7 @@ The resultant Pod is as follows: image: a.b.c/image:v1 command: cmd args: [] - // remaining container configuration omitted + # remaining container configuration omitted Notice how options follow the same merging rules, i.e. lists append and maps override. @@ -450,4 +450,4 @@ The resultant pod for that task is as follows: image: a.b.c/image:v1 command: cmd args: [] - // remaining container configuration omitted + # remaining container configuration omitted From 3ff007a69b02b489d77b83856dd92d55b9808616 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com> Date: Fri, 8 Mar 2024 15:17:24 -0800 Subject: [PATCH 4/5] Fix use of labels and annotations in pod template examples (#5025) Signed-off-by: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com> --- docs/deployment/configuration/general.rst | 66 +++++++++++------------ 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/docs/deployment/configuration/general.rst b/docs/deployment/configuration/general.rst index 0ce9d259a4..0b97f8b8ff 100644 --- a/docs/deployment/configuration/general.rst +++ b/docs/deployment/configuration/general.rst @@ -202,10 +202,10 @@ An example PodTemplate is shown: template: metadata: labels: - - foo + foo: from-pod-template annotations: - - foo: initial-value - - bar: initial-value + foo: initial-value + bar: initial-value spec: containers: - name: default @@ -221,10 +221,10 @@ Pod Labels, Annotations, and enables the host networking. plugins: k8s: default-labels: - - bar + bar: from-default-label default-annotations: - - foo: overridden-value - - baz: non-overridden-value + foo: overridden-value + baz: non-overridden-value enable-host-networking-pod: true To construct a Pod, FlytePropeller initializes a Pod definition using the default @@ -241,12 +241,12 @@ The resultant Pod using the above default PodTemplate and K8s Plugin configurati name: example-pod namespace: flytesnacks-development labels: - - foo # maintained initial value - - bar # value appended by k8s plugin configuration + foo: from-pod-template # maintained initial value + bar: from-default-label # value appended by k8s plugin configuration annotations: - - foo: overridden-value # value overridden by k8s plugin configuration - - bar: initial-value # maintained initial value - - baz: non-overridden-value # value added by k8s plugin configuration + foo: overridden-value # value overridden by k8s plugin configuration + bar: initial-value # maintained initial value + baz: non-overridden-value # value added by k8s plugin configuration spec: containers: - name: ax9kd5xb4p8r45bpdv7v-n0-0 @@ -280,8 +280,8 @@ of the task. For example: template: metadata: annotations: - - annotation_1: initial-value - - bar: initial-value + annotation_1: initial-value + bar: initial-value spec: containers: - name: default @@ -328,12 +328,12 @@ The resultant Pod is as follows: name: example-pod namespace: flytesnacks-development labels: - - label_1: value-1 # from Compile-time value - - label_2: value-2 # from Compile-time value + label_1: value-1 # from Compile-time value + label_2: value-2 # from Compile-time value annotations: - - annotation_1: value-1 # value overridden by Compile-time PodTemplate - - annotation_2: value-2 # from Compile-time PodTemplate - - bar: initial-value # from Runtime PodTemplate + annotation_1: value-1 # value overridden by Compile-time PodTemplate + annotation_2: value-2 # from Compile-time PodTemplate + bar: initial-value # from Runtime PodTemplate spec: containers: - name: default @@ -398,12 +398,12 @@ And a Runtime PodTemplate: template: metadata: labels: - - label_1: value-runtime - - label_2: value-runtime - - label_3: value-runtime + label_1: value-runtime + label_2: value-runtime + label_3: value-runtime annotations: - - foo: value-runtime - - bar: value-runtime + foo: value-runtime + bar: value-runtime spec: containers: - name: default @@ -418,10 +418,10 @@ And the following K8s Plugin Configuration: plugins: k8s: default-labels: - - label_1: value-plugin + label_1: value-plugin default-annotations: - - annotation_1: value-plugin - - baz: value-plugin + annotation_1: value-plugin + baz: value-plugin The resultant pod for that task is as follows: @@ -433,14 +433,14 @@ The resultant pod for that task is as follows: name: example-pod namespace: flytesnacks-development labels: - - label_1: value-plugin - - label_2: value-compile + label_1: value-plugin + label_2: value-compile annotations: - - annotation_1: value-plugin - - annotation_2: value-compile - - foo: value-runtime - - bar: value-runtime - - baz: value-plugin + annotation_1: value-plugin + annotation_2: value-compile + foo: value-runtime + bar: value-runtime + baz: value-plugin spec: containers: - name: default From a49284e94e156f10a1f1b4a91c825cb70aa70ea0 Mon Sep 17 00:00:00 2001 From: Kevin Su Date: Fri, 8 Mar 2024 17:11:44 -0800 Subject: [PATCH 5/5] Add gRPC probe for agent (#4990) Signed-off-by: Kevin Su --- charts/flyteagent/README.md | 3 ++- charts/flyteagent/templates/agent/deployment.yaml | 4 ++++ charts/flyteagent/values.yaml | 9 ++++++++- deployment/agent/flyte_agent_helm_generated.yaml | 7 ++++++- docker/sandbox-bundled/manifests/complete-agent.yaml | 11 ++++++++--- docker/sandbox-bundled/manifests/complete.yaml | 4 ++-- docker/sandbox-bundled/manifests/dev.yaml | 4 ++-- 7 files changed, 32 insertions(+), 10 deletions(-) diff --git a/charts/flyteagent/README.md b/charts/flyteagent/README.md index f889c095bd..d862716673 100644 --- a/charts/flyteagent/README.md +++ b/charts/flyteagent/README.md @@ -20,7 +20,7 @@ A Helm chart for Flyte agent | fullnameOverride | string | `""` | | | image.pullPolicy | string | `"IfNotPresent"` | Docker image pull policy | | image.repository | string | `"ghcr.io/flyteorg/flyteagent"` | Docker image for flyteagent deployment | -| image.tag | string | `"1.10.7"` | Docker image tag | +| image.tag | string | `"1.10.8b4"` | Docker image tag | | nameOverride | string | `""` | | | nodeSelector | object | `{}` | nodeSelector for flyteagent deployment | | podAnnotations | object | `{}` | Annotations for flyteagent pods | @@ -30,6 +30,7 @@ A Helm chart for Flyte agent | ports.containerPort | int | `8000` | | | ports.name | string | `"agent-grpc"` | | | priorityClassName | string | `""` | Sets priorityClassName for datacatalog pod(s). | +| readinessProbe | object | `{"grpc":{"port":8000},"initialDelaySeconds":1,"periodSeconds":3}` | https://kubernetes.io/blog/2022/05/13/grpc-probes-now-in-beta/#trying-the-feature-out | | replicaCount | int | `1` | Replicas count for flyteagent deployment | | resources | object | `{"limits":{"cpu":"500m","ephemeral-storage":"200Mi","memory":"200Mi"},"requests":{"cpu":"500m","ephemeral-storage":"200Mi","memory":"200Mi"}}` | Default resources requests and limits for flyteagent deployment | | securityContext | object | `{"allowPrivilegeEscalation":false}` | Security context for container | diff --git a/charts/flyteagent/templates/agent/deployment.yaml b/charts/flyteagent/templates/agent/deployment.yaml index 2c6a903f8f..caad9ca44a 100644 --- a/charts/flyteagent/templates/agent/deployment.yaml +++ b/charts/flyteagent/templates/agent/deployment.yaml @@ -39,6 +39,10 @@ spec: ports: - containerPort: {{ .Values.ports.containerPort }} name: {{ .Values.ports.name }} + readinessProbe: + {{- with .Values.readinessProbe -}} + {{ tpl (toYaml .) $ | nindent 10 }} + {{- end }} securityContext: {{- toYaml .Values.securityContext | nindent 12 }} resources: {{- toYaml .Values.resources | nindent 10 }} diff --git a/charts/flyteagent/values.yaml b/charts/flyteagent/values.yaml index aee84dc2b2..266ec9c0eb 100755 --- a/charts/flyteagent/values.yaml +++ b/charts/flyteagent/values.yaml @@ -23,7 +23,7 @@ image: # -- Docker image for flyteagent deployment repository: ghcr.io/flyteorg/flyteagent # -- Docker image tag - tag: 1.10.7 # FLYTEAGENT_TAG + tag: 1.10.8b4 # FLYTEAGENT_TAG # -- Docker image pull policy pullPolicy: IfNotPresent ports: @@ -56,6 +56,13 @@ serviceAccount: imagePullSecrets: [] # -- Security context for pod podSecurityContext: {} +# -- Readiness probe for flyteagent. Use readinessProbe: {} if agent doesn't implement grpc-health-checking service. +# -- https://kubernetes.io/blog/2022/05/13/grpc-probes-now-in-beta/#trying-the-feature-out +readinessProbe: + grpc: + port: 8000 + initialDelaySeconds: 1 + periodSeconds: 3 # -- Security context for container securityContext: allowPrivilegeEscalation: false diff --git a/deployment/agent/flyte_agent_helm_generated.yaml b/deployment/agent/flyte_agent_helm_generated.yaml index 46762b4cff..4e78b991ac 100644 --- a/deployment/agent/flyte_agent_helm_generated.yaml +++ b/deployment/agent/flyte_agent_helm_generated.yaml @@ -78,7 +78,7 @@ spec: - pyflyte - serve - agent - image: "ghcr.io/flyteorg/flyteagent:1.10.7" + image: "ghcr.io/flyteorg/flyteagent:1.10.8b4" imagePullPolicy: "IfNotPresent" name: flyteagent volumeMounts: @@ -87,6 +87,11 @@ spec: ports: - containerPort: 8000 name: agent-grpc + readinessProbe: + grpc: + port: 8000 + initialDelaySeconds: 1 + periodSeconds: 3 securityContext: allowPrivilegeEscalation: false resources: diff --git a/docker/sandbox-bundled/manifests/complete-agent.yaml b/docker/sandbox-bundled/manifests/complete-agent.yaml index 5f72d69c20..60577c5716 100644 --- a/docker/sandbox-bundled/manifests/complete-agent.yaml +++ b/docker/sandbox-bundled/manifests/complete-agent.yaml @@ -816,7 +816,7 @@ type: Opaque --- apiVersion: v1 data: - haSharedSecret: NTRkQThQbW1FNDlVQ1B4Rw== + haSharedSecret: UVdVTnB4cXBMVXMyRjhGUw== proxyPassword: "" proxyUsername: "" kind: Secret @@ -1412,7 +1412,7 @@ spec: metadata: annotations: checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81 - checksum/secret: 7ce58af0efd945f657619718c017f528d7055b8069cab01aed85e29639bd6d6a + checksum/secret: 7dd7be244652f0c2d4d5651db7e9d879fb74fb3a4407a80c48cba3dcd7b74e53 labels: app: docker-registry release: flyte-sandbox @@ -1755,12 +1755,17 @@ spec: value: minio - name: FLYTE_AWS_SECRET_ACCESS_KEY value: miniostorage - image: ghcr.io/flyteorg/flyteagent:1.10.7 + image: ghcr.io/flyteorg/flyteagent:1.10.8b4 imagePullPolicy: IfNotPresent name: flyteagent ports: - containerPort: 8000 name: agent-grpc + readinessProbe: + grpc: + port: 8000 + initialDelaySeconds: 1 + periodSeconds: 3 resources: limits: cpu: 500m diff --git a/docker/sandbox-bundled/manifests/complete.yaml b/docker/sandbox-bundled/manifests/complete.yaml index cacefc4730..f148ebf1c8 100644 --- a/docker/sandbox-bundled/manifests/complete.yaml +++ b/docker/sandbox-bundled/manifests/complete.yaml @@ -796,7 +796,7 @@ type: Opaque --- apiVersion: v1 data: - haSharedSecret: c21mSkU1U0c0S0J0Q2lRRg== + haSharedSecret: cnBScDd2Y3Y0a2JlcHlzVQ== proxyPassword: "" proxyUsername: "" kind: Secret @@ -1360,7 +1360,7 @@ spec: metadata: annotations: checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81 - checksum/secret: 67f0b60e6520f3ae46a9f396411386d473441e7733c2544f6bface0f3b8475d9 + checksum/secret: 459fe43b90e7be7fce10bc94020ba94037c1a21501b61d4e7121c84251f1b198 labels: app: docker-registry release: flyte-sandbox diff --git a/docker/sandbox-bundled/manifests/dev.yaml b/docker/sandbox-bundled/manifests/dev.yaml index 5e0017253e..84e16dcaba 100644 --- a/docker/sandbox-bundled/manifests/dev.yaml +++ b/docker/sandbox-bundled/manifests/dev.yaml @@ -499,7 +499,7 @@ metadata: --- apiVersion: v1 data: - haSharedSecret: YjFqbVE2UE05QnpwMVVVeg== + haSharedSecret: NlJkVzYyZ1MzZmZIRE1nOA== proxyPassword: "" proxyUsername: "" kind: Secret @@ -934,7 +934,7 @@ spec: metadata: annotations: checksum/config: 8f50e768255a87f078ba8b9879a0c174c3e045ffb46ac8723d2eedbe293c8d81 - checksum/secret: ec699a25c688a15358899da82f9fa9bb199fe085f200f00facc09b448ad80369 + checksum/secret: 2e417b7dd337346d064d1cd4edf6b5624d335fce92ccfe22acd20485be450ec9 labels: app: docker-registry release: flyte-sandbox