From d137e777c6db9dd282d0f2187766f27532c1e6e0 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Tue, 23 Apr 2024 22:24:11 -0400 Subject: [PATCH 01/34] chore!: remove legacy `patch` `pods` fallback - the fallback is old and insecure, and the error confuses users as it's not mentioned in the docs (as it's legacy and a fallback) - it's also tech debt that we have to write code around specifically right now - it's no longer needed and hasn't been the main RBAC in a few versions, so remove it in the next minor - remove the Executor code that patches pods - remove the operator code that reads the patched annotations Signed-off-by: Anton Gilgur --- workflow/common/common.go | 9 --------- workflow/controller/controller_test.go | 14 -------------- workflow/controller/exit_handler_test.go | 5 ++--- workflow/controller/operator.go | 3 +++ workflow/controller/operator_test.go | 17 ++++------------- workflow/executor/executor.go | 24 +++--------------------- 6 files changed, 12 insertions(+), 60 deletions(-) diff --git a/workflow/common/common.go b/workflow/common/common.go index b4f174263eaa..429972ab4477 100644 --- a/workflow/common/common.go +++ b/workflow/common/common.go @@ -34,8 +34,6 @@ const ( AnnotationKeyRBACRule = workflow.WorkflowFullName + "/rbac-rule" AnnotationKeyRBACRulePrecedence = workflow.WorkflowFullName + "/rbac-rule-precedence" - // AnnotationKeyOutputs is the pod metadata annotation key containing the container outputs - AnnotationKeyOutputs = workflow.WorkflowFullName + "/outputs" // AnnotationKeyCronWfScheduledTime is the workflow metadata annotation key containing the time when the workflow // was scheduled to run by CronWorkflow. AnnotationKeyCronWfScheduledTime = workflow.WorkflowFullName + "/scheduled-time" @@ -48,13 +46,6 @@ const ( // AnnotationKeyPodNameVersion stores the pod naming convention version AnnotationKeyPodNameVersion = workflow.WorkflowFullName + "/pod-name-format" - // AnnotationKeyProgress is N/M progress for the node - AnnotationKeyProgress = workflow.WorkflowFullName + "/progress" - - // AnnotationKeyReportOutputsCompleted is an annotation on a workflow pod indicating outputs have completed. - // Only used as a backup in case LabelKeyReportOutputsCompleted can't be added to WorkflowTaskResult. - AnnotationKeyReportOutputsCompleted = workflow.WorkflowFullName + "/report-outputs-completed" - // AnnotationKeyArtifactGCStrategy is listed as an annotation on the Artifact GC Pod to identify // the strategy whose artifacts are being deleted AnnotationKeyArtifactGCStrategy = workflow.WorkflowFullName + "/artifact-gc-strategy" diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index e7c018d92857..ad269703dcc5 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -449,16 +449,6 @@ func listPods(woc *wfOperationCtx) (*apiv1.PodList, error) { type with func(pod *apiv1.Pod) -func withOutputs(v interface{}) with { - switch x := v.(type) { - case string: - return withAnnotation(common.AnnotationKeyOutputs, x) - default: - return withOutputs(wfv1.MustMarshallJSON(x)) - } -} -func withProgress(v string) with { return withAnnotation(common.AnnotationKeyProgress, v) } - func withExitCode(v int32) with { return func(pod *apiv1.Pod) { for _, c := range pod.Spec.Containers { @@ -474,10 +464,6 @@ func withExitCode(v int32) with { } } -func withAnnotation(key, val string) with { - return func(pod *apiv1.Pod) { pod.Annotations[key] = val } -} - // createRunningPods creates the pods that are marked as running in a given test so that they can be accessed by the // pod assessor func createRunningPods(ctx context.Context, woc *wfOperationCtx) { diff --git a/workflow/controller/exit_handler_test.go b/workflow/controller/exit_handler_test.go index b55923c95d3d..5091fae33a4c 100644 --- a/workflow/controller/exit_handler_test.go +++ b/workflow/controller/exit_handler_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/require" apiv1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" ) @@ -357,7 +356,7 @@ func TestStepsTmplOnExit(t *testing.T) { woc := newWorkflowOperationCtx(wf, controller) woc.operate(ctx) assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase) - makePodsPhase(ctx, woc, apiv1.PodSucceeded, withOutputs(wfv1.Outputs{Result: pointer.String("ok"), Parameters: []wfv1.Parameter{{}}})) + makePodsPhase(ctx, woc, apiv1.PodSucceeded) woc1 := newWorkflowOperationCtx(woc.wf, controller) woc1.operate(ctx) assert.Equal(t, wfv1.WorkflowRunning, woc1.wf.Status.Phase) @@ -462,7 +461,7 @@ func TestDAGOnExit(t *testing.T) { woc := newWorkflowOperationCtx(wf, controller) woc.operate(ctx) assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase) - makePodsPhase(ctx, woc, apiv1.PodSucceeded, withOutputs(wfv1.Outputs{Parameters: []wfv1.Parameter{{}}})) + makePodsPhase(ctx, woc, apiv1.PodSucceeded) woc1 := newWorkflowOperationCtx(woc.wf, controller) woc1.operate(ctx) assert.Equal(t, wfv1.WorkflowRunning, woc1.wf.Status.Phase) diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index 2ca55ca833a3..da578cf603cf 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -1427,6 +1427,7 @@ func (woc *wfOperationCtx) assessNodeStatus(ctx context.Context, pod *apiv1.Pod, new.PodIP = pod.Status.PodIP } +<<<<<<< HEAD // If `AnnotationKeyReportOutputsCompleted` is set, it means RBAC prevented WorkflowTaskResult from being written. if x, ok := pod.Annotations[common.AnnotationKeyReportOutputsCompleted]; ok { woc.log.Warn("workflow uses legacy/insecure pod patch, see https://argo-workflows.readthedocs.io/en/latest/workflow-rbac/") @@ -1456,6 +1457,8 @@ func (woc *wfOperationCtx) assessNodeStatus(ctx context.Context, pod *apiv1.Pod, } } +======= +>>>>>>> dc7699c91 (chore!: remove legacy `patch` `pods` fallback) new.HostNodeName = pod.Spec.NodeName if !new.Progress.IsValid() { diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index 7ac9e7809112..c953054bb0d3 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -291,14 +291,7 @@ spec: woc := newWorkflowOperationCtx(wf, controller) woc.operate(ctx) - assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase) - assert.Equal(t, wfv1.Progress("0/100"), woc.wf.Status.Progress) - assert.Equal(t, wfv1.Progress("0/100"), woc.wf.Status.Nodes[woc.wf.Name].Progress) - pod := woc.wf.Status.Nodes.FindByDisplayName("pod") - assert.Equal(t, wfv1.Progress("0/100"), pod.Progress) - - // mock workflow uses legacy/insecure pod patch - makePodsPhase(ctx, woc, apiv1.PodRunning, withAnnotation(common.AnnotationKeyReportOutputsCompleted, "false"), withProgress("50/100")) + makePodsPhase(ctx, woc, apiv1.PodRunning) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) @@ -308,8 +301,7 @@ spec: pod = woc.wf.Status.Nodes.FindByDisplayName("pod") assert.Equal(t, wfv1.Progress("50/100"), pod.Progress) - // mock workflow uses legacy/insecure pod patch - makePodsPhase(ctx, woc, apiv1.PodSucceeded, withAnnotation(common.AnnotationKeyReportOutputsCompleted, "true"), withProgress("100/100")) + makePodsPhase(ctx, woc, apiv1.PodSucceeded) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) @@ -6254,7 +6246,7 @@ func TestConfigMapCacheSaveOperate(t *testing.T) { ctx := context.Background() woc.operate(ctx) - makePodsPhase(ctx, woc, apiv1.PodSucceeded, withExitCode(0), withAnnotation(common.AnnotationKeyReportOutputsCompleted, "true"), withOutputs(wfv1.MustMarshallJSON(sampleOutputs))) + makePodsPhase(ctx, woc, apiv1.PodSucceeded, withExitCode(0)) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) @@ -6537,7 +6529,7 @@ spec: assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase) // make all created pods as successful - makePodsPhase(ctx, woc, apiv1.PodSucceeded, withAnnotation(common.AnnotationKeyReportOutputsCompleted, "true"), withOutputs(`{"parameters": [{"name": "my-param"}]}`)) + makePodsPhase(ctx, woc, apiv1.PodSucceeded) // reconcile woc = newWorkflowOperationCtx(woc.wf, controller) @@ -8601,7 +8593,6 @@ func TestWFGlobalArtifactNil(t *testing.T) { makePodsPhase(ctx, woc, apiv1.PodRunning) woc.operate(ctx) makePodsPhase(ctx, woc, apiv1.PodFailed, func(pod *apiv1.Pod) { - pod.Annotations[common.AnnotationKeyOutputs] = string("{\"parameters\":[{\"name\":\"hello-param\",\"valueFrom\":{\"path\":\"/tmp/hello_world.txt\"},\"globalName\":\"my-global-param\"}],\"artifacts\":[{\"name\":\"hello-art\",\"path\":\"/tmp/hello_world.txt\",\"globalName\":\"my-global-art\"}]}") pod.Status.ContainerStatuses = []apiv1.ContainerStatus{ { Name: "main", diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index 4f0b52875ac9..82648858126a 100644 --- a/workflow/executor/executor.go +++ b/workflow/executor/executor.go @@ -802,9 +802,7 @@ func (we *WorkflowExecutor) FinalizeOutput(ctx context.Context) { common.LabelKeyReportOutputsCompleted: "true", }) if apierr.IsForbidden(err) || apierr.IsNotFound(err) { - log.WithError(err).Warn("failed to patch task result, falling back to legacy/insecure pod patch, see https://argo-workflows.readthedocs.io/en/latest/workflow-rbac/") - // Only added as a backup in case LabelKeyReportOutputsCompleted could not be set - err = we.AddAnnotation(ctx, common.AnnotationKeyReportOutputsCompleted, "true") + log.WithError(err).Warn("failed to patch task result, see https://argo-workflows.readthedocs.io/en/latest/workflow-rbac/") } return err }) @@ -823,9 +821,7 @@ func (we *WorkflowExecutor) InitializeOutput(ctx context.Context) { }, errorsutil.IsTransientErr, func() error { err := we.upsertTaskResult(ctx, wfv1.NodeResult{}) if apierr.IsForbidden(err) { - log.WithError(err).Warn("failed to patch task result, falling back to legacy/insecure pod patch, see https://argo-workflows.readthedocs.io/en/latest/workflow-rbac/") - // Only added as a backup in case LabelKeyReportOutputsCompleted could not be set - err = we.AddAnnotation(ctx, common.AnnotationKeyReportOutputsCompleted, "false") + log.WithError(err).Warn("failed to patch task result, see https://argo-workflows.readthedocs.io/en/latest/workflow-rbac/") } return err }) @@ -851,22 +847,8 @@ func (we *WorkflowExecutor) reportResult(ctx context.Context, result wfv1.NodeRe }, errorsutil.IsTransientErr, func() error { err := we.upsertTaskResult(ctx, result) if apierr.IsForbidden(err) { - log.WithError(err).Warn("failed to patch task result, falling back to legacy/insecure pod patch, see https://argo-workflows.readthedocs.io/en/latest/workflow-rbac/") - if result.Outputs.HasOutputs() { - value, err := json.Marshal(result.Outputs) - if err != nil { - return err - } - - return we.AddAnnotation(ctx, common.AnnotationKeyOutputs, string(value)) - } - if result.Progress.IsValid() { // this may result in occasionally two patches - return we.AddAnnotation(ctx, common.AnnotationKeyProgress, string(result.Progress)) - } - // Only added as a backup in case LabelKeyReportOutputsCompleted could not be set - return we.AddAnnotation(ctx, common.AnnotationKeyReportOutputsCompleted, "false") + log.WithError(err).Warn("failed to patch task result, see https://argo-workflows.readthedocs.io/en/latest/workflow-rbac/") } - return err }) } From ae066c7d3d85dbc7a6cfc8646a11b355e27011d0 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 26 May 2024 23:50:15 -0400 Subject: [PATCH 02/34] fix: WIP test fixes. Signed-off-by: Garett MacGowan --- .../arguments-parameters-from-configmap.yaml | 3 +- test/e2e/resource_template_test.go | 2 -- .../workflow_configmap_substitution_test.go | 1 - workflow/controller/controller_test.go | 24 ++++++++++++-- workflow/controller/operator.go | 32 ------------------- .../controller/operator_concurrency_test.go | 2 +- workflow/controller/operator_test.go | 18 +++++------ 7 files changed, 32 insertions(+), 50 deletions(-) diff --git a/examples/arguments-parameters-from-configmap.yaml b/examples/arguments-parameters-from-configmap.yaml index 39339ff41b7f..fd6ebbb095d5 100644 --- a/examples/arguments-parameters-from-configmap.yaml +++ b/examples/arguments-parameters-from-configmap.yaml @@ -11,8 +11,7 @@ metadata: workflows.argoproj.io/verify.py: | assert status["phase"] == "Succeeded" spec: - serviceAccountName: argo - entrypoint: print-message-from-configmap + entrypoint: whalesay templates: - name: print-message-from-configmap diff --git a/test/e2e/resource_template_test.go b/test/e2e/resource_template_test.go index f2f319920be0..01b312340673 100644 --- a/test/e2e/resource_template_test.go +++ b/test/e2e/resource_template_test.go @@ -104,11 +104,9 @@ kind: Workflow metadata: generateName: k8s-resource-tmpl-with-artifact- spec: - serviceAccount: argo entrypoint: main templates: - name: main - serviceAccountName: argo inputs: artifacts: - name: manifest diff --git a/test/e2e/workflow_configmap_substitution_test.go b/test/e2e/workflow_configmap_substitution_test.go index cf0dd6e8a0f9..25655bebfbde 100644 --- a/test/e2e/workflow_configmap_substitution_test.go +++ b/test/e2e/workflow_configmap_substitution_test.go @@ -194,7 +194,6 @@ metadata: label: workflows.argoproj.io/test: "true" spec: - serviceAccountName: argo entrypoint: whalesay arguments: parameters: diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index ad269703dcc5..aa4e375f0cc9 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -447,10 +447,10 @@ func listPods(woc *wfOperationCtx) (*apiv1.PodList, error) { return woc.controller.kubeclientset.CoreV1().Pods(woc.wf.Namespace).List(context.Background(), metav1.ListOptions{}) } -type with func(pod *apiv1.Pod) +type with func(pod *apiv1.Pod, woc *wfOperationCtx) func withExitCode(v int32) with { - return func(pod *apiv1.Pod) { + return func(pod *apiv1.Pod, _ *wfOperationCtx) { for _, c := range pod.Spec.Containers { pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, apiv1.ContainerStatus{ Name: c.Name, @@ -504,6 +504,24 @@ func syncPodsInformer(ctx context.Context, woc *wfOperationCtx, podObjs ...apiv1 } } +func withOutputs(outputs wfv1.Outputs) with { + return func(pod *apiv1.Pod, woc *wfOperationCtx) { + nodeId := woc.nodeID(pod) + woc.controller.taskResultInformer.GetStore().Add(&wfv1.WorkflowTaskResult{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeId, + Labels: map[string]string{ + common.LabelKeyWorkflow: woc.wf.Name, + }, + }, + NodeResult: wfv1.NodeResult{ + Phase: wfv1.NodeSucceeded, + Outputs: &outputs, + }, + }) + } +} + // makePodsPhase acts like a pod controller and simulates the transition of pods transitioning into a specified state func makePodsPhase(ctx context.Context, woc *wfOperationCtx, phase apiv1.PodPhase, with ...with) { podcs := woc.controller.kubeclientset.CoreV1().Pods(woc.wf.GetNamespace()) @@ -518,7 +536,7 @@ func makePodsPhase(ctx context.Context, woc *wfOperationCtx, phase apiv1.PodPhas pod.Status.Message = "Pod failed" } for _, w := range with { - w(&pod) + w(&pod, woc) } updatedPod, err := podcs.Update(ctx, &pod, metav1.UpdateOptions{}) if err != nil { diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index da578cf603cf..42709564ddd7 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -1427,38 +1427,6 @@ func (woc *wfOperationCtx) assessNodeStatus(ctx context.Context, pod *apiv1.Pod, new.PodIP = pod.Status.PodIP } -<<<<<<< HEAD - // If `AnnotationKeyReportOutputsCompleted` is set, it means RBAC prevented WorkflowTaskResult from being written. - if x, ok := pod.Annotations[common.AnnotationKeyReportOutputsCompleted]; ok { - woc.log.Warn("workflow uses legacy/insecure pod patch, see https://argo-workflows.readthedocs.io/en/latest/workflow-rbac/") - resultName := woc.nodeID(pod) - if x == "true" { - woc.wf.Status.MarkTaskResultComplete(resultName) - } else { - woc.wf.Status.MarkTaskResultIncomplete(resultName) - } - - // Get node outputs from pod annotations instead if RBAC prevented WorkflowTaskResult from being written. - if x, ok = pod.Annotations[common.AnnotationKeyOutputs]; ok { - if new.Outputs == nil { - new.Outputs = &wfv1.Outputs{} - } - if err := json.Unmarshal([]byte(x), new.Outputs); err != nil { - new.Phase = wfv1.NodeError - new.Message = err.Error() - } - } - - // Get node progress from pod annotations instead if RBAC prevented WorkflowTaskResult from being written. - if x, ok = pod.Annotations[common.AnnotationKeyProgress]; ok { - if p, ok := wfv1.ParseProgress(x); ok { - new.Progress = p - } - } - } - -======= ->>>>>>> dc7699c91 (chore!: remove legacy `patch` `pods` fallback) new.HostNodeName = pod.Spec.NodeName if !new.Progress.IsValid() { diff --git a/workflow/controller/operator_concurrency_test.go b/workflow/controller/operator_concurrency_test.go index a0e842516b7a..a7d52a0218c5 100644 --- a/workflow/controller/operator_concurrency_test.go +++ b/workflow/controller/operator_concurrency_test.go @@ -1118,7 +1118,7 @@ spec: assert.True(t, job1AcquiredLock) // Make job-1's pod succeed - makePodsPhase(ctx, woc, apiv1.PodSucceeded, func(pod *apiv1.Pod) { + makePodsPhase(ctx, woc, apiv1.PodSucceeded, func(pod *apiv1.Pod, _ *wfOperationCtx) { if pod.ObjectMeta.Name == "job-1" { pod.Status.Phase = apiv1.PodSucceeded } diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index c953054bb0d3..c8fa735079e4 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -296,20 +296,20 @@ spec: woc.operate(ctx) assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase) - assert.Equal(t, wfv1.Progress("50/100"), woc.wf.Status.Progress) - assert.Equal(t, wfv1.Progress("50/100"), woc.wf.Status.Nodes[woc.wf.Name].Progress) - pod = woc.wf.Status.Nodes.FindByDisplayName("pod") - assert.Equal(t, wfv1.Progress("50/100"), pod.Progress) + assert.Equal(t, wfv1.Progress("0/1"), woc.wf.Status.Progress) + assert.Equal(t, wfv1.Progress("0/1"), woc.wf.Status.Nodes[woc.wf.Name].Progress) + pod := woc.wf.Status.Nodes.FindByDisplayName("pod") + assert.Equal(t, wfv1.Progress("0/1"), pod.Progress) makePodsPhase(ctx, woc, apiv1.PodSucceeded) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) assert.Equal(t, wfv1.WorkflowSucceeded, woc.wf.Status.Phase) - assert.Equal(t, wfv1.Progress("100/100"), woc.wf.Status.Progress) - assert.Equal(t, wfv1.Progress("100/100"), woc.wf.Status.Nodes[woc.wf.Name].Progress) + assert.Equal(t, wfv1.Progress("1/1"), woc.wf.Status.Progress) + assert.Equal(t, wfv1.Progress("1/1"), woc.wf.Status.Nodes[woc.wf.Name].Progress) pod = woc.wf.Status.Nodes.FindByDisplayName("pod") - assert.Equal(t, wfv1.Progress("100/100"), pod.Progress) + assert.Equal(t, wfv1.Progress("1/1"), pod.Progress) } var sidecarWithVol = ` @@ -6246,7 +6246,7 @@ func TestConfigMapCacheSaveOperate(t *testing.T) { ctx := context.Background() woc.operate(ctx) - makePodsPhase(ctx, woc, apiv1.PodSucceeded, withExitCode(0)) + makePodsPhase(ctx, woc, apiv1.PodSucceeded, withOutputs(sampleOutputs)) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) @@ -8592,7 +8592,7 @@ func TestWFGlobalArtifactNil(t *testing.T) { woc.operate(ctx) makePodsPhase(ctx, woc, apiv1.PodRunning) woc.operate(ctx) - makePodsPhase(ctx, woc, apiv1.PodFailed, func(pod *apiv1.Pod) { + makePodsPhase(ctx, woc, apiv1.PodFailed, func(pod *apiv1.Pod, _ *wfOperationCtx) { pod.Status.ContainerStatuses = []apiv1.ContainerStatus{ { Name: "main", From c1c348175eb00dc4c796217018f672b9a39d4422 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Fri, 7 Jun 2024 19:03:51 -0400 Subject: [PATCH 03/34] test: test fixes Signed-off-by: Garett MacGowan --- test/e2e/resource_template_test.go | 2 +- workflow/controller/controller_test.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/test/e2e/resource_template_test.go b/test/e2e/resource_template_test.go index 01b312340673..e8cd68834ddb 100644 --- a/test/e2e/resource_template_test.go +++ b/test/e2e/resource_template_test.go @@ -80,7 +80,7 @@ spec: metadata: generateName: k8s-pod-resource- spec: - serviceAccountName: argo + serviceAccountName: default containers: - name: argosay-container image: argoproj/argosay:v2 diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index aa4e375f0cc9..0f4daf0d3dfa 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -507,7 +507,7 @@ func syncPodsInformer(ctx context.Context, woc *wfOperationCtx, podObjs ...apiv1 func withOutputs(outputs wfv1.Outputs) with { return func(pod *apiv1.Pod, woc *wfOperationCtx) { nodeId := woc.nodeID(pod) - woc.controller.taskResultInformer.GetStore().Add(&wfv1.WorkflowTaskResult{ + err := woc.controller.taskResultInformer.GetStore().Add(&wfv1.WorkflowTaskResult{ ObjectMeta: metav1.ObjectMeta{ Name: nodeId, Labels: map[string]string{ @@ -519,6 +519,9 @@ func withOutputs(outputs wfv1.Outputs) with { Outputs: &outputs, }, }) + if err != nil { + panic(err) + } } } From e1ec574d093f639307af70500a40beb27f4c6428 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Fri, 7 Jun 2024 20:04:35 -0400 Subject: [PATCH 04/34] test: test fixes Signed-off-by: Garett MacGowan --- .../workflow-controller-role.yaml | 2 ++ workflow/controller/controller_test.go | 15 --------------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/manifests/namespace-install/workflow-controller-rbac/workflow-controller-role.yaml b/manifests/namespace-install/workflow-controller-rbac/workflow-controller-role.yaml index 4e8df30fa5f2..08ab06a4a6f7 100644 --- a/manifests/namespace-install/workflow-controller-rbac/workflow-controller-role.yaml +++ b/manifests/namespace-install/workflow-controller-rbac/workflow-controller-role.yaml @@ -74,6 +74,8 @@ rules: verbs: - list - watch + - patch + - create - deletecollection - apiGroups: - "" diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index 0f4daf0d3dfa..1204cb5f0379 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -449,21 +449,6 @@ func listPods(woc *wfOperationCtx) (*apiv1.PodList, error) { type with func(pod *apiv1.Pod, woc *wfOperationCtx) -func withExitCode(v int32) with { - return func(pod *apiv1.Pod, _ *wfOperationCtx) { - for _, c := range pod.Spec.Containers { - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, apiv1.ContainerStatus{ - Name: c.Name, - State: apiv1.ContainerState{ - Terminated: &apiv1.ContainerStateTerminated{ - ExitCode: v, - }, - }, - }) - } - } -} - // createRunningPods creates the pods that are marked as running in a given test so that they can be accessed by the // pod assessor func createRunningPods(ctx context.Context, woc *wfOperationCtx) { From 80fcc9aec012062ce13f7dac4b1c3adbd6dd3e1d Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Fri, 7 Jun 2024 20:25:38 -0400 Subject: [PATCH 05/34] test: test fixes Signed-off-by: Garett MacGowan --- .../workflow-controller-clusterrole.yaml | 2 ++ workflow/controller/controller_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml b/manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml index b66dbc948afc..93ccafad54bf 100644 --- a/manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml +++ b/manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml @@ -68,6 +68,8 @@ rules: verbs: - list - watch + - patch + - create - deletecollection - apiGroups: - "" diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index 1204cb5f0379..49416444519b 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -500,7 +500,7 @@ func withOutputs(outputs wfv1.Outputs) with { }, }, NodeResult: wfv1.NodeResult{ - Phase: wfv1.NodeSucceeded, + Phase: wfv1.NodeSucceeded, Outputs: &outputs, }, }) From fa9fc8eb7e2bfee5f1adf16e08d1b06284ebcea4 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Fri, 7 Jun 2024 21:03:29 -0400 Subject: [PATCH 06/34] test: test fixes Signed-off-by: Garett MacGowan --- .../workflow-controller-rolebinding.yaml | 4 ++-- manifests/quick-start-minimal.yaml | 2 ++ manifests/quick-start-mysql.yaml | 2 ++ manifests/quick-start-postgres.yaml | 2 ++ test/e2e/resource_template_test.go | 2 +- 5 files changed, 9 insertions(+), 3 deletions(-) diff --git a/manifests/namespace-install/workflow-controller-rbac/workflow-controller-rolebinding.yaml b/manifests/namespace-install/workflow-controller-rbac/workflow-controller-rolebinding.yaml index f8ac7ec0d606..720de37c553c 100644 --- a/manifests/namespace-install/workflow-controller-rbac/workflow-controller-rolebinding.yaml +++ b/manifests/namespace-install/workflow-controller-rbac/workflow-controller-rolebinding.yaml @@ -7,6 +7,6 @@ roleRef: kind: Role name: argo-role subjects: -- kind: ServiceAccount - name: argo + - kind: ServiceAccount + name: argo diff --git a/manifests/quick-start-minimal.yaml b/manifests/quick-start-minimal.yaml index 84b7d22ad581..9cdb4dc9ff92 100644 --- a/manifests/quick-start-minimal.yaml +++ b/manifests/quick-start-minimal.yaml @@ -1233,6 +1233,8 @@ rules: verbs: - list - watch + - patch + - create - deletecollection - apiGroups: - "" diff --git a/manifests/quick-start-mysql.yaml b/manifests/quick-start-mysql.yaml index 5bdf3f13ba62..40612fe93894 100644 --- a/manifests/quick-start-mysql.yaml +++ b/manifests/quick-start-mysql.yaml @@ -1233,6 +1233,8 @@ rules: verbs: - list - watch + - patch + - create - deletecollection - apiGroups: - "" diff --git a/manifests/quick-start-postgres.yaml b/manifests/quick-start-postgres.yaml index b0603cbe3130..ca77b2ba4159 100644 --- a/manifests/quick-start-postgres.yaml +++ b/manifests/quick-start-postgres.yaml @@ -1233,6 +1233,8 @@ rules: verbs: - list - watch + - patch + - create - deletecollection - apiGroups: - "" diff --git a/test/e2e/resource_template_test.go b/test/e2e/resource_template_test.go index e8cd68834ddb..01b312340673 100644 --- a/test/e2e/resource_template_test.go +++ b/test/e2e/resource_template_test.go @@ -80,7 +80,7 @@ spec: metadata: generateName: k8s-pod-resource- spec: - serviceAccountName: default + serviceAccountName: argo containers: - name: argosay-container image: argoproj/argosay:v2 From f996a750c463ec1c50cdb4b12e6daba38fd7318c Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Fri, 7 Jun 2024 21:54:08 -0400 Subject: [PATCH 07/34] test: test fixes Signed-off-by: Garett MacGowan --- workflow/controller/operator_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index c8fa735079e4..9228e5496555 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -6246,9 +6246,12 @@ func TestConfigMapCacheSaveOperate(t *testing.T) { ctx := context.Background() woc.operate(ctx) - makePodsPhase(ctx, woc, apiv1.PodSucceeded, withOutputs(sampleOutputs)) + makePodsPhase(ctx, woc, apiv1.PodRunning, withOutputs(sampleOutputs)) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) + makePodsPhase(ctx, woc, apiv1.PodSucceeded) + woc = newWorkflowOperationCtx(woc.wf, controller) + woc.operate(ctx) cm, err := controller.kubeclientset.CoreV1().ConfigMaps("default").Get(ctx, "whalesay-cache", metav1.GetOptions{}) require.NoError(t, err) From 3b2d66edc22efb95d15a8ddc2ba9666b0c1b249f Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Fri, 7 Jun 2024 22:05:09 -0400 Subject: [PATCH 08/34] test: test fixes Signed-off-by: Garett MacGowan --- workflow/controller/operator_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index 9228e5496555..f43f66a4e066 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -6246,12 +6246,12 @@ func TestConfigMapCacheSaveOperate(t *testing.T) { ctx := context.Background() woc.operate(ctx) - makePodsPhase(ctx, woc, apiv1.PodRunning, withOutputs(sampleOutputs)) + makePodsPhase(ctx, woc, apiv1.PodSucceeded, withOutputs(sampleOutputs)) + woc = newWorkflowOperationCtx(woc.wf, controller) + woc.operate(ctx) + makePodsPhase(ctx, woc, apiv1.PodSucceeded) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) - makePodsPhase(ctx, woc, apiv1.PodSucceeded) - woc = newWorkflowOperationCtx(woc.wf, controller) - woc.operate(ctx) cm, err := controller.kubeclientset.CoreV1().ConfigMaps("default").Get(ctx, "whalesay-cache", metav1.GetOptions{}) require.NoError(t, err) From 2cf3d6c965fd71dcdb1e665009985a6819bb1444 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sat, 8 Jun 2024 00:05:44 -0400 Subject: [PATCH 09/34] test: test fixes Signed-off-by: Garett MacGowan --- workflow/controller/controller_test.go | 1 + workflow/controller/operator_test.go | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index 49416444519b..b693715aa270 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -497,6 +497,7 @@ func withOutputs(outputs wfv1.Outputs) with { Name: nodeId, Labels: map[string]string{ common.LabelKeyWorkflow: woc.wf.Name, + common.LabelKeyReportOutputsCompleted: "true", }, }, NodeResult: wfv1.NodeResult{ diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index f43f66a4e066..c8fa735079e4 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -6249,9 +6249,6 @@ func TestConfigMapCacheSaveOperate(t *testing.T) { makePodsPhase(ctx, woc, apiv1.PodSucceeded, withOutputs(sampleOutputs)) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) - makePodsPhase(ctx, woc, apiv1.PodSucceeded) - woc = newWorkflowOperationCtx(woc.wf, controller) - woc.operate(ctx) cm, err := controller.kubeclientset.CoreV1().ConfigMaps("default").Get(ctx, "whalesay-cache", metav1.GetOptions{}) require.NoError(t, err) From eebacf7b7ecc2423d5b468e0ebd548a3c6f62c63 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sat, 8 Jun 2024 02:05:03 -0400 Subject: [PATCH 10/34] test: test fixes Signed-off-by: Garett MacGowan --- workflow/controller/controller_test.go | 17 ++++++++++++++++- workflow/controller/operator_test.go | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index b693715aa270..08fd6595bb32 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -496,7 +496,7 @@ func withOutputs(outputs wfv1.Outputs) with { ObjectMeta: metav1.ObjectMeta{ Name: nodeId, Labels: map[string]string{ - common.LabelKeyWorkflow: woc.wf.Name, + common.LabelKeyWorkflow: woc.wf.Name, common.LabelKeyReportOutputsCompleted: "true", }, }, @@ -511,6 +511,21 @@ func withOutputs(outputs wfv1.Outputs) with { } } +func withExitCode(v int32) with { + return func(pod *apiv1.Pod, woc *wfOperationCtx) { + for _, c := range pod.Spec.Containers { + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, apiv1.ContainerStatus{ + Name: c.Name, + State: apiv1.ContainerState{ + Terminated: &apiv1.ContainerStateTerminated{ + ExitCode: v, + }, + }, + }) + } + } +} + // makePodsPhase acts like a pod controller and simulates the transition of pods transitioning into a specified state func makePodsPhase(ctx context.Context, woc *wfOperationCtx, phase apiv1.PodPhase, with ...with) { podcs := woc.controller.kubeclientset.CoreV1().Pods(woc.wf.GetNamespace()) diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index c8fa735079e4..a1c03b6aa98c 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -6246,7 +6246,7 @@ func TestConfigMapCacheSaveOperate(t *testing.T) { ctx := context.Background() woc.operate(ctx) - makePodsPhase(ctx, woc, apiv1.PodSucceeded, withOutputs(sampleOutputs)) + makePodsPhase(ctx, woc, apiv1.PodSucceeded, withExitCode(0), withOutputs(sampleOutputs)) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) From dcd550316bfde09c763e13425ca0e48fca8bb13a Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 13:15:26 -0400 Subject: [PATCH 11/34] test: test fixes Signed-off-by: Garett MacGowan --- .../workflow-controller-clusterrole.yaml | 2 - .../workflow-controller-role.yaml | 2 - manifests/quick-start-minimal.yaml | 2 - manifests/quick-start-mysql.yaml | 2 - manifests/quick-start-postgres.yaml | 2 - test/e2e/manifests/minimal/kustomization.yaml | 1 + .../resource-permissioned-executor-sa.yaml | 33 +++++++++ test/e2e/resource_template_test.go | 4 +- workflow/controller/controller_test.go | 74 +++++++++---------- 9 files changed, 72 insertions(+), 50 deletions(-) create mode 100644 test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml diff --git a/manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml b/manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml index 93ccafad54bf..b66dbc948afc 100644 --- a/manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml +++ b/manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml @@ -68,8 +68,6 @@ rules: verbs: - list - watch - - patch - - create - deletecollection - apiGroups: - "" diff --git a/manifests/namespace-install/workflow-controller-rbac/workflow-controller-role.yaml b/manifests/namespace-install/workflow-controller-rbac/workflow-controller-role.yaml index 08ab06a4a6f7..4e8df30fa5f2 100644 --- a/manifests/namespace-install/workflow-controller-rbac/workflow-controller-role.yaml +++ b/manifests/namespace-install/workflow-controller-rbac/workflow-controller-role.yaml @@ -74,8 +74,6 @@ rules: verbs: - list - watch - - patch - - create - deletecollection - apiGroups: - "" diff --git a/manifests/quick-start-minimal.yaml b/manifests/quick-start-minimal.yaml index 9cdb4dc9ff92..84b7d22ad581 100644 --- a/manifests/quick-start-minimal.yaml +++ b/manifests/quick-start-minimal.yaml @@ -1233,8 +1233,6 @@ rules: verbs: - list - watch - - patch - - create - deletecollection - apiGroups: - "" diff --git a/manifests/quick-start-mysql.yaml b/manifests/quick-start-mysql.yaml index 40612fe93894..5bdf3f13ba62 100644 --- a/manifests/quick-start-mysql.yaml +++ b/manifests/quick-start-mysql.yaml @@ -1233,8 +1233,6 @@ rules: verbs: - list - watch - - patch - - create - deletecollection - apiGroups: - "" diff --git a/manifests/quick-start-postgres.yaml b/manifests/quick-start-postgres.yaml index ca77b2ba4159..b0603cbe3130 100644 --- a/manifests/quick-start-postgres.yaml +++ b/manifests/quick-start-postgres.yaml @@ -1233,8 +1233,6 @@ rules: verbs: - list - watch - - patch - - create - deletecollection - apiGroups: - "" diff --git a/test/e2e/manifests/minimal/kustomization.yaml b/test/e2e/manifests/minimal/kustomization.yaml index 46ffc3d94741..f9472822cead 100644 --- a/test/e2e/manifests/minimal/kustomization.yaml +++ b/test/e2e/manifests/minimal/kustomization.yaml @@ -9,6 +9,7 @@ resources: - ../mixins/argo-workflows-agent-ca-certificates.yaml - ../mixins/argo-server.service-account-token-secret.yaml - ../mixins/argo.service-account-token-secret.yaml +- ../mixins/resource-permissioned-executor-sa.yaml patches: - path: ../mixins/argo-server-deployment.yaml diff --git a/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml b/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml new file mode 100644 index 000000000000..93633d3e9f69 --- /dev/null +++ b/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml @@ -0,0 +1,33 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ServiceAccount +metadata: + name: resource-permissioned-executor + namespace: default +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: executor-resource-permissioned-executor + namespace: default +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: executor +subjects: +- kind: ServiceAccount + name: resource-permissioned-executor + namespace: default +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: pod-manager-resource-permissioned-executor + namespace: default +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: pod-manager +subjects: +- kind: ServiceAccount + name: resource-permissioned-executor + namespace: default diff --git a/test/e2e/resource_template_test.go b/test/e2e/resource_template_test.go index 01b312340673..32ba07af6416 100644 --- a/test/e2e/resource_template_test.go +++ b/test/e2e/resource_template_test.go @@ -64,11 +64,10 @@ kind: Workflow metadata: generateName: k8s-resource-tmpl-with-pod- spec: - serviceAccount: argo entrypoint: main templates: - name: main - serviceAccountName: argo + serviceAccountName: resource-permissioned-executor resource: action: create setOwnerReference: true @@ -80,7 +79,6 @@ spec: metadata: generateName: k8s-pod-resource- spec: - serviceAccountName: argo containers: - name: argosay-container image: argoproj/argosay:v2 diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index 08fd6595bb32..b68b9f6740ac 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -449,6 +449,43 @@ func listPods(woc *wfOperationCtx) (*apiv1.PodList, error) { type with func(pod *apiv1.Pod, woc *wfOperationCtx) +func withOutputs(outputs wfv1.Outputs) with { + return func(pod *apiv1.Pod, woc *wfOperationCtx) { + nodeId := woc.nodeID(pod) + err := woc.controller.taskResultInformer.GetStore().Add(&wfv1.WorkflowTaskResult{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeId, + Labels: map[string]string{ + common.LabelKeyWorkflow: woc.wf.Name, + common.LabelKeyReportOutputsCompleted: "true", + }, + }, + NodeResult: wfv1.NodeResult{ + Phase: wfv1.NodeSucceeded, + Outputs: &outputs, + }, + }) + if err != nil { + panic(err) + } + } +} + +func withExitCode(v int32) with { + return func(pod *apiv1.Pod, woc *wfOperationCtx) { + for _, c := range pod.Spec.Containers { + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, apiv1.ContainerStatus{ + Name: c.Name, + State: apiv1.ContainerState{ + Terminated: &apiv1.ContainerStateTerminated{ + ExitCode: v, + }, + }, + }) + } + } +} + // createRunningPods creates the pods that are marked as running in a given test so that they can be accessed by the // pod assessor func createRunningPods(ctx context.Context, woc *wfOperationCtx) { @@ -489,43 +526,6 @@ func syncPodsInformer(ctx context.Context, woc *wfOperationCtx, podObjs ...apiv1 } } -func withOutputs(outputs wfv1.Outputs) with { - return func(pod *apiv1.Pod, woc *wfOperationCtx) { - nodeId := woc.nodeID(pod) - err := woc.controller.taskResultInformer.GetStore().Add(&wfv1.WorkflowTaskResult{ - ObjectMeta: metav1.ObjectMeta{ - Name: nodeId, - Labels: map[string]string{ - common.LabelKeyWorkflow: woc.wf.Name, - common.LabelKeyReportOutputsCompleted: "true", - }, - }, - NodeResult: wfv1.NodeResult{ - Phase: wfv1.NodeSucceeded, - Outputs: &outputs, - }, - }) - if err != nil { - panic(err) - } - } -} - -func withExitCode(v int32) with { - return func(pod *apiv1.Pod, woc *wfOperationCtx) { - for _, c := range pod.Spec.Containers { - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, apiv1.ContainerStatus{ - Name: c.Name, - State: apiv1.ContainerState{ - Terminated: &apiv1.ContainerStateTerminated{ - ExitCode: v, - }, - }, - }) - } - } -} - // makePodsPhase acts like a pod controller and simulates the transition of pods transitioning into a specified state func makePodsPhase(ctx context.Context, woc *wfOperationCtx, phase apiv1.PodPhase, with ...with) { podcs := woc.controller.kubeclientset.CoreV1().Pods(woc.wf.GetNamespace()) From 3274435b0403c4c183ab8e8c77409a84668b2be9 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 13:21:16 -0400 Subject: [PATCH 12/34] test: test fixes Signed-off-by: Garett MacGowan --- .../e2e/manifests/mixins/resource-permissioned-executor-sa.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml b/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml index 93633d3e9f69..7248554aa659 100644 --- a/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml +++ b/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml @@ -1,4 +1,4 @@ -apiVersion: rbac.authorization.k8s.io/v1 +apiVersion: v1 kind: ServiceAccount metadata: name: resource-permissioned-executor From 23f781b195ec21606c8d247c3b170c269c5319a2 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 14:39:26 -0400 Subject: [PATCH 13/34] test: test fixes Signed-off-by: Garett MacGowan --- test/e2e/workflow_inputs_orverridable_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/e2e/workflow_inputs_orverridable_test.go b/test/e2e/workflow_inputs_orverridable_test.go index 3005bc4e4690..a5a99f9505ab 100644 --- a/test/e2e/workflow_inputs_orverridable_test.go +++ b/test/e2e/workflow_inputs_orverridable_test.go @@ -27,7 +27,6 @@ metadata: label: workflows.argoproj.io/test: "true" spec: - serviceAccountName: argo entrypoint: whalesay arguments: parameters: @@ -73,7 +72,6 @@ metadata: label: workflows.argoproj.io/test: "true" spec: - serviceAccountName: argo entrypoint: whalesay arguments: parameters: @@ -127,7 +125,6 @@ metadata: label: workflows.argoproj.io/test: "true" spec: - serviceAccountName: argo entrypoint: whalesay arguments: parameters: @@ -164,7 +161,6 @@ metadata: label: workflows.argoproj.io/test: "true" spec: - serviceAccountName: argo entrypoint: whalesay arguments: parameters: From 15c840c96b3fa08b25b34f911dbd067e1067ec6a Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 15:31:51 -0400 Subject: [PATCH 14/34] test: test fixes Signed-off-by: Garett MacGowan --- docs/workflow-templates.md | 4 +-- ...onfigmap-referenced-as-local-variable.yaml | 3 +- .../global-parameters-from-configmap.yaml | 3 +- test/e2e/manifests/minimal/kustomization.yaml | 1 - .../resource-permissioned-executor-sa.yaml | 33 ------------------- test/e2e/resource_template_test.go | 2 -- test/e2e/workflow_test.go | 4 +-- 7 files changed, 5 insertions(+), 45 deletions(-) delete mode 100644 test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml diff --git a/docs/workflow-templates.md b/docs/workflow-templates.md index 66de9c71dd21..fb30b8307af2 100644 --- a/docs/workflow-templates.md +++ b/docs/workflow-templates.md @@ -138,7 +138,6 @@ kind: WorkflowTemplate metadata: name: hello-world-template-global-arg spec: - serviceAccountName: argo templates: - name: hello-world container: @@ -151,8 +150,7 @@ kind: Workflow metadata: generateName: hello-world-wf-global-arg- spec: - serviceAccountName: argo - entrypoint: print-message + entrypoint: whalesay arguments: parameters: - name: global-parameter diff --git a/examples/global-parameters-from-configmap-referenced-as-local-variable.yaml b/examples/global-parameters-from-configmap-referenced-as-local-variable.yaml index 701ebd1c1caa..ac2976be730d 100644 --- a/examples/global-parameters-from-configmap-referenced-as-local-variable.yaml +++ b/examples/global-parameters-from-configmap-referenced-as-local-variable.yaml @@ -9,8 +9,7 @@ metadata: This example demonstrates how a global parameter from a ConfigMap can be referenced as a template local variable. Note that the "simple-parameters" ConfigMap (defined in `examples/configmaps/simple-parameters-configmap.yaml`) needs to be created first before submitting this workflow. spec: - serviceAccountName: argo - entrypoint: print-message + entrypoint: whalesay arguments: parameters: - name: message diff --git a/examples/global-parameters-from-configmap.yaml b/examples/global-parameters-from-configmap.yaml index 4b8dc78114a2..ddf353310d26 100644 --- a/examples/global-parameters-from-configmap.yaml +++ b/examples/global-parameters-from-configmap.yaml @@ -9,8 +9,7 @@ metadata: This example demonstrates loading global parameter values from a ConfigMap. Note that the "simple-parameters" ConfigMap (defined in `examples/configmaps/simple-parameters-configmap.yaml`) needs to be created first before submitting this workflow. spec: - serviceAccountName: argo - entrypoint: print-message + entrypoint: whalesay # Parameters can also be passed via configmap reference. arguments: parameters: diff --git a/test/e2e/manifests/minimal/kustomization.yaml b/test/e2e/manifests/minimal/kustomization.yaml index f9472822cead..46ffc3d94741 100644 --- a/test/e2e/manifests/minimal/kustomization.yaml +++ b/test/e2e/manifests/minimal/kustomization.yaml @@ -9,7 +9,6 @@ resources: - ../mixins/argo-workflows-agent-ca-certificates.yaml - ../mixins/argo-server.service-account-token-secret.yaml - ../mixins/argo.service-account-token-secret.yaml -- ../mixins/resource-permissioned-executor-sa.yaml patches: - path: ../mixins/argo-server-deployment.yaml diff --git a/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml b/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml deleted file mode 100644 index 7248554aa659..000000000000 --- a/test/e2e/manifests/mixins/resource-permissioned-executor-sa.yaml +++ /dev/null @@ -1,33 +0,0 @@ -apiVersion: v1 -kind: ServiceAccount -metadata: - name: resource-permissioned-executor - namespace: default ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: executor-resource-permissioned-executor - namespace: default -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: executor -subjects: -- kind: ServiceAccount - name: resource-permissioned-executor - namespace: default ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: pod-manager-resource-permissioned-executor - namespace: default -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: pod-manager -subjects: -- kind: ServiceAccount - name: resource-permissioned-executor - namespace: default diff --git a/test/e2e/resource_template_test.go b/test/e2e/resource_template_test.go index 32ba07af6416..4582bde44d63 100644 --- a/test/e2e/resource_template_test.go +++ b/test/e2e/resource_template_test.go @@ -67,7 +67,6 @@ spec: entrypoint: main templates: - name: main - serviceAccountName: resource-permissioned-executor resource: action: create setOwnerReference: true @@ -116,7 +115,6 @@ spec: metadata: generateName: k8s-pod-resource- spec: - serviceAccountName: argo containers: - name: argosay-container image: argoproj/argosay:v2 diff --git a/test/e2e/workflow_test.go b/test/e2e/workflow_test.go index 6d988b02d044..6d21262f5069 100644 --- a/test/e2e/workflow_test.go +++ b/test/e2e/workflow_test.go @@ -33,7 +33,7 @@ spec: serviceAccountName: argo automountServiceAccountToken: false executor: - serviceAccountName: argo + serviceAccountName: default entrypoint: main templates: - name: main @@ -67,7 +67,7 @@ spec: serviceAccountName: argo automountServiceAccountToken: false executor: - serviceAccountName: argo + serviceAccountName: default entrypoint: main templates: - name: main From c10ec1a65d1dad6e2831dde2210401650e3d07dc Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 16:10:09 -0400 Subject: [PATCH 15/34] test: test fixes Signed-off-by: Garett MacGowan --- test/e2e/testdata/artifact-workflow-stopped.yaml | 2 +- test/e2e/testdata/artifactgc/artgc-dag-wf-self-delete.yaml | 2 +- test/e2e/testdata/retry-on-stopped.yaml | 2 +- test/e2e/workflow_test.go | 4 ++-- workflow/controller/operator_test.go | 3 +-- workflow/controller/operator_wfdefault_test.go | 4 ++-- workflow/util/merge_test.go | 6 +++--- workflow/validate/validate_dag_test.go | 2 +- workflow/validate/validate_test.go | 3 +-- 9 files changed, 13 insertions(+), 15 deletions(-) diff --git a/test/e2e/testdata/artifact-workflow-stopped.yaml b/test/e2e/testdata/artifact-workflow-stopped.yaml index ae85aa207a3f..05032054e7a5 100644 --- a/test/e2e/testdata/artifact-workflow-stopped.yaml +++ b/test/e2e/testdata/artifact-workflow-stopped.yaml @@ -8,7 +8,7 @@ spec: workflows.argoproj.io/test: "true" workflows.argoproj.io/workflow: "wf-stopped" entrypoint: wf-stopped-main - serviceAccountName: argo + serviceAccountName: default executor: serviceAccountName: default volumeClaimTemplates: diff --git a/test/e2e/testdata/artifactgc/artgc-dag-wf-self-delete.yaml b/test/e2e/testdata/artifactgc/artgc-dag-wf-self-delete.yaml index ddce1199d629..ce6aa466e5da 100644 --- a/test/e2e/testdata/artifactgc/artgc-dag-wf-self-delete.yaml +++ b/test/e2e/testdata/artifactgc/artgc-dag-wf-self-delete.yaml @@ -13,7 +13,7 @@ spec: serviceAccountName: default strategy: OnWorkflowDeletion entrypoint: artgc-dag-wf-self-delete-main - serviceAccountName: argo + serviceAccountName: default executor: serviceAccountName: default volumeClaimTemplates: diff --git a/test/e2e/testdata/retry-on-stopped.yaml b/test/e2e/testdata/retry-on-stopped.yaml index 6b7276f570cb..59a8749a340e 100644 --- a/test/e2e/testdata/retry-on-stopped.yaml +++ b/test/e2e/testdata/retry-on-stopped.yaml @@ -8,7 +8,7 @@ spec: workflows.argoproj.io/test: "true" workflows.argoproj.io/workflow: "wf-retry-stopped" entrypoint: wf-retry-stopped-main - serviceAccountName: argo + serviceAccountName: default executor: serviceAccountName: default templates: diff --git a/test/e2e/workflow_test.go b/test/e2e/workflow_test.go index 6d21262f5069..4d7c4ddb293e 100644 --- a/test/e2e/workflow_test.go +++ b/test/e2e/workflow_test.go @@ -30,7 +30,7 @@ metadata: generateName: get-resources-via-container-template- namespace: argo spec: - serviceAccountName: argo + serviceAccountName: default automountServiceAccountToken: false executor: serviceAccountName: default @@ -64,7 +64,7 @@ metadata: generateName: get-resources-via-script-template- namespace: argo spec: - serviceAccountName: argo + serviceAccountName: default automountServiceAccountToken: false executor: serviceAccountName: default diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index a1c03b6aa98c..7818ee410f42 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -9328,7 +9328,6 @@ metadata: spec: arguments: {} entrypoint: main - serviceAccountName: argo templates: - inputs: {} metadata: {} @@ -10902,7 +10901,7 @@ spec: serviceAccountName: default podSpecPatch: | terminationGracePeriodSeconds: 3 - serviceAccountName: argo + serviceAccountName: default templates: - inputs: {} metadata: {} diff --git a/workflow/controller/operator_wfdefault_test.go b/workflow/controller/operator_wfdefault_test.go index 608d1af9692c..69ba9da11150 100644 --- a/workflow/controller/operator_wfdefault_test.go +++ b/workflow/controller/operator_wfdefault_test.go @@ -26,7 +26,7 @@ var wfDefaults = ` name: message value: "hello world" onExit: whalesay-exit - serviceAccountName: argo + serviceAccountName: default templates: - container: @@ -81,7 +81,7 @@ spec: value: "hello world" entrypoint: whalesay onExit: whalesay-exit - serviceAccountName: argo + serviceAccountName: default templates: - container: diff --git a/workflow/util/merge_test.go b/workflow/util/merge_test.go index 6b1839c6af1f..2306dd8f3dde 100644 --- a/workflow/util/merge_test.go +++ b/workflow/util/merge_test.go @@ -22,7 +22,7 @@ spec: value: original entrypoint: start onExit: end - serviceAccountName: argo + serviceAccountName: default workflowTemplateRef: name: workflow-template-submittable ` @@ -97,7 +97,7 @@ spec: name: message value: "hello world" onExit: whalesay-exit - serviceAccountName: argo + serviceAccountName: default templates: - container: @@ -185,7 +185,7 @@ spec: value: "hello world" entrypoint: whalesay onExit: whalesay-exit - serviceAccountName: argo + serviceAccountName: default templates: - container: diff --git a/workflow/validate/validate_dag_test.go b/workflow/validate/validate_dag_test.go index fbb885edee53..e0c1c00f93a2 100644 --- a/workflow/validate/validate_dag_test.go +++ b/workflow/validate/validate_dag_test.go @@ -937,7 +937,7 @@ kind: Workflow metadata: generateName: loops- spec: - serviceAccountName: argo + serviceAccountName: default entrypoint: dag templates: - name: dag diff --git a/workflow/validate/validate_test.go b/workflow/validate/validate_test.go index 68498eae9e9c..e5a597d86a43 100644 --- a/workflow/validate/validate_test.go +++ b/workflow/validate/validate_test.go @@ -2151,7 +2151,7 @@ metadata: generateName: hello-world- spec: entrypoint: A - serviceAccountName: argo + serviceAccountName: default parallelism: 1 volumes: - name: workdir @@ -2995,7 +2995,6 @@ metadata: generateName: arguments-parameters-from-configmap- spec: entrypoint: whalesay - serviceAccountName: argo arguments: parameters: - name: message From d36a0853df9fce2a254a8a83a14842be400531f5 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 17:03:36 -0400 Subject: [PATCH 16/34] test: test fixes Signed-off-by: Garett MacGowan --- test/e2e/manifests/minimal/kustomization.yaml | 1 + .../pod-manager-full-permissions-rbac.yaml | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 test/e2e/manifests/mixins/pod-manager-full-permissions-rbac.yaml diff --git a/test/e2e/manifests/minimal/kustomization.yaml b/test/e2e/manifests/minimal/kustomization.yaml index 46ffc3d94741..d0f0542ffc2e 100644 --- a/test/e2e/manifests/minimal/kustomization.yaml +++ b/test/e2e/manifests/minimal/kustomization.yaml @@ -16,6 +16,7 @@ patches: - path: ../mixins/workflow-controller-configmap.yaml - path: ../mixins/workflow-controller-deployment.yaml - path: ../mixins/workflow-controller-cluster-workflow-template-rbac.yaml +- path: ../mixins/pod-manager-full-permissions-rbac.yaml - path: ../mixins/minio-deployment.yaml labels: diff --git a/test/e2e/manifests/mixins/pod-manager-full-permissions-rbac.yaml b/test/e2e/manifests/mixins/pod-manager-full-permissions-rbac.yaml new file mode 100644 index 000000000000..db63ec24ca9f --- /dev/null +++ b/test/e2e/manifests/mixins/pod-manager-full-permissions-rbac.yaml @@ -0,0 +1,16 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: pod-manager +rules: +- apiGroups: + - "" + resources: + - pods + verbs: + - create + - get + - patch + - apply + - delete + - replace From 5a075caf7662d9d4b5a500794770a9c00ffb7489 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 17:23:26 -0400 Subject: [PATCH 17/34] test: test fixes Signed-off-by: Garett MacGowan --- test/e2e/manifests/minimal/kustomization.yaml | 1 - .../pod-manager-full-permissions-rbac.yaml | 16 ---------------- test/e2e/testdata/artifact-workflow-stopped.yaml | 2 +- .../artifactgc/artgc-dag-wf-self-delete.yaml | 2 +- test/e2e/testdata/retry-on-stopped.yaml | 2 +- 5 files changed, 3 insertions(+), 20 deletions(-) delete mode 100644 test/e2e/manifests/mixins/pod-manager-full-permissions-rbac.yaml diff --git a/test/e2e/manifests/minimal/kustomization.yaml b/test/e2e/manifests/minimal/kustomization.yaml index d0f0542ffc2e..46ffc3d94741 100644 --- a/test/e2e/manifests/minimal/kustomization.yaml +++ b/test/e2e/manifests/minimal/kustomization.yaml @@ -16,7 +16,6 @@ patches: - path: ../mixins/workflow-controller-configmap.yaml - path: ../mixins/workflow-controller-deployment.yaml - path: ../mixins/workflow-controller-cluster-workflow-template-rbac.yaml -- path: ../mixins/pod-manager-full-permissions-rbac.yaml - path: ../mixins/minio-deployment.yaml labels: diff --git a/test/e2e/manifests/mixins/pod-manager-full-permissions-rbac.yaml b/test/e2e/manifests/mixins/pod-manager-full-permissions-rbac.yaml deleted file mode 100644 index db63ec24ca9f..000000000000 --- a/test/e2e/manifests/mixins/pod-manager-full-permissions-rbac.yaml +++ /dev/null @@ -1,16 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: pod-manager -rules: -- apiGroups: - - "" - resources: - - pods - verbs: - - create - - get - - patch - - apply - - delete - - replace diff --git a/test/e2e/testdata/artifact-workflow-stopped.yaml b/test/e2e/testdata/artifact-workflow-stopped.yaml index 05032054e7a5..ae85aa207a3f 100644 --- a/test/e2e/testdata/artifact-workflow-stopped.yaml +++ b/test/e2e/testdata/artifact-workflow-stopped.yaml @@ -8,7 +8,7 @@ spec: workflows.argoproj.io/test: "true" workflows.argoproj.io/workflow: "wf-stopped" entrypoint: wf-stopped-main - serviceAccountName: default + serviceAccountName: argo executor: serviceAccountName: default volumeClaimTemplates: diff --git a/test/e2e/testdata/artifactgc/artgc-dag-wf-self-delete.yaml b/test/e2e/testdata/artifactgc/artgc-dag-wf-self-delete.yaml index ce6aa466e5da..ddce1199d629 100644 --- a/test/e2e/testdata/artifactgc/artgc-dag-wf-self-delete.yaml +++ b/test/e2e/testdata/artifactgc/artgc-dag-wf-self-delete.yaml @@ -13,7 +13,7 @@ spec: serviceAccountName: default strategy: OnWorkflowDeletion entrypoint: artgc-dag-wf-self-delete-main - serviceAccountName: default + serviceAccountName: argo executor: serviceAccountName: default volumeClaimTemplates: diff --git a/test/e2e/testdata/retry-on-stopped.yaml b/test/e2e/testdata/retry-on-stopped.yaml index 59a8749a340e..6b7276f570cb 100644 --- a/test/e2e/testdata/retry-on-stopped.yaml +++ b/test/e2e/testdata/retry-on-stopped.yaml @@ -8,7 +8,7 @@ spec: workflows.argoproj.io/test: "true" workflows.argoproj.io/workflow: "wf-retry-stopped" entrypoint: wf-retry-stopped-main - serviceAccountName: default + serviceAccountName: argo executor: serviceAccountName: default templates: From 2fb2bd47c7a3457bdb4ead159020276afa913c73 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 17:48:34 -0400 Subject: [PATCH 18/34] test: test fixes Signed-off-by: Garett MacGowan --- workflow/controller/operator_wfdefault_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/controller/operator_wfdefault_test.go b/workflow/controller/operator_wfdefault_test.go index 69ba9da11150..289e7322e751 100644 --- a/workflow/controller/operator_wfdefault_test.go +++ b/workflow/controller/operator_wfdefault_test.go @@ -144,7 +144,7 @@ var storedSpecResult = ` }, "entrypoint": "whalesay-template", "onExit": "whalesay-exit", - "serviceAccountName": "argo", + "serviceAccountName": "default", "templates": [ { "container": { From 378ae92b85fc365bc04a9002b5b17fc26fd13d56 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 20:22:07 -0400 Subject: [PATCH 19/34] test: test fixes Signed-off-by: Garett MacGowan --- test/e2e/workflow_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/workflow_test.go b/test/e2e/workflow_test.go index 4d7c4ddb293e..05ab383aae0f 100644 --- a/test/e2e/workflow_test.go +++ b/test/e2e/workflow_test.go @@ -30,7 +30,7 @@ metadata: generateName: get-resources-via-container-template- namespace: argo spec: - serviceAccountName: default + serviceAccountName: argo automountServiceAccountToken: false executor: serviceAccountName: default From 5432fbb46e44fb3f55a96fc2886211e401fbb7dd Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Sun, 9 Jun 2024 23:49:53 -0400 Subject: [PATCH 20/34] test: test fixes Signed-off-by: Garett MacGowan --- test/e2e/workflow_test.go | 2 +- workflow/controller/controller_test.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/e2e/workflow_test.go b/test/e2e/workflow_test.go index 05ab383aae0f..6d21262f5069 100644 --- a/test/e2e/workflow_test.go +++ b/test/e2e/workflow_test.go @@ -64,7 +64,7 @@ metadata: generateName: get-resources-via-script-template- namespace: argo spec: - serviceAccountName: default + serviceAccountName: argo automountServiceAccountToken: false executor: serviceAccountName: default diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index b68b9f6740ac..02e66e0d5558 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -27,6 +27,7 @@ import ( "github.com/argoproj/argo-workflows/v3/config" "github.com/argoproj/argo-workflows/v3/persist/sqldb" + "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" fakewfclientset "github.com/argoproj/argo-workflows/v3/pkg/client/clientset/versioned/fake" "github.com/argoproj/argo-workflows/v3/pkg/client/clientset/versioned/scheme" @@ -453,6 +454,10 @@ func withOutputs(outputs wfv1.Outputs) with { return func(pod *apiv1.Pod, woc *wfOperationCtx) { nodeId := woc.nodeID(pod) err := woc.controller.taskResultInformer.GetStore().Add(&wfv1.WorkflowTaskResult{ + TypeMeta: metav1.TypeMeta{ + APIVersion: workflow.APIVersion, + Kind: workflow.WorkflowTaskResultKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: nodeId, Labels: map[string]string{ From 681b0bedd14579d6cbdea2397983dd64f3c97096 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Wed, 12 Jun 2024 16:54:21 -0400 Subject: [PATCH 21/34] test: test fixes. Signed-off-by: Garett MacGowan --- .../base/workflow-default-rolebinding.yaml | 11 ----------- test/e2e/workflow_test.go | 1 + 2 files changed, 1 insertion(+), 11 deletions(-) delete mode 100644 manifests/quick-start/base/workflow-default-rolebinding.yaml diff --git a/manifests/quick-start/base/workflow-default-rolebinding.yaml b/manifests/quick-start/base/workflow-default-rolebinding.yaml deleted file mode 100644 index cb930cfb27a7..000000000000 --- a/manifests/quick-start/base/workflow-default-rolebinding.yaml +++ /dev/null @@ -1,11 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: workflow-default-binding -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: workflow -subjects: - - kind: ServiceAccount - name: default diff --git a/test/e2e/workflow_test.go b/test/e2e/workflow_test.go index 6d21262f5069..81c551d7d58a 100644 --- a/test/e2e/workflow_test.go +++ b/test/e2e/workflow_test.go @@ -37,6 +37,7 @@ spec: entrypoint: main templates: - name: main + serviceAccountName: argo container: name: main image: bitnami/kubectl From e04935db8ffb3ecc2372bc3955ef71dd656d15fd Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Wed, 12 Jun 2024 21:48:42 -0400 Subject: [PATCH 22/34] test: test fixes. Signed-off-by: Garett MacGowan --- test/e2e/workflow_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/workflow_test.go b/test/e2e/workflow_test.go index 81c551d7d58a..8dee50a2f3e1 100644 --- a/test/e2e/workflow_test.go +++ b/test/e2e/workflow_test.go @@ -37,7 +37,7 @@ spec: entrypoint: main templates: - name: main - serviceAccountName: argo + serviceAccountName: argo container: name: main image: bitnami/kubectl From db334cb74c33a2388435382ca8f55174aff5eed4 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Thu, 13 Jun 2024 14:05:37 -0400 Subject: [PATCH 23/34] test: test fixes. Signed-off-by: Garett MacGowan --- test/e2e/manifests/minimal/kustomization.yaml | 1 + test/e2e/manifests/mixins/get-cm-rbac.yaml | 40 +++++++++++++++++++ test/e2e/workflow_test.go | 3 +- 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 test/e2e/manifests/mixins/get-cm-rbac.yaml diff --git a/test/e2e/manifests/minimal/kustomization.yaml b/test/e2e/manifests/minimal/kustomization.yaml index 46ffc3d94741..70e6d3618a2e 100644 --- a/test/e2e/manifests/minimal/kustomization.yaml +++ b/test/e2e/manifests/minimal/kustomization.yaml @@ -9,6 +9,7 @@ resources: - ../mixins/argo-workflows-agent-ca-certificates.yaml - ../mixins/argo-server.service-account-token-secret.yaml - ../mixins/argo.service-account-token-secret.yaml +- ../mixins/get-cm-rbac.yaml patches: - path: ../mixins/argo-server-deployment.yaml diff --git a/test/e2e/manifests/mixins/get-cm-rbac.yaml b/test/e2e/manifests/mixins/get-cm-rbac.yaml new file mode 100644 index 000000000000..880c723cf526 --- /dev/null +++ b/test/e2e/manifests/mixins/get-cm-rbac.yaml @@ -0,0 +1,40 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: get-cm +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: get-cm +rules: +- apiGroups: + - "" + resources: + - configmaps + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: get-cm-get-cm +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: get-cm +subjects: + - kind: ServiceAccount + name: get-cm +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: executor-get-cm +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: executor +subjects: +- kind: ServiceAccount + name: get-cm \ No newline at end of file diff --git a/test/e2e/workflow_test.go b/test/e2e/workflow_test.go index 8dee50a2f3e1..4f7a5d0ea76c 100644 --- a/test/e2e/workflow_test.go +++ b/test/e2e/workflow_test.go @@ -33,11 +33,10 @@ spec: serviceAccountName: argo automountServiceAccountToken: false executor: - serviceAccountName: default + serviceAccountName: get-cm entrypoint: main templates: - name: main - serviceAccountName: argo container: name: main image: bitnami/kubectl From f592ea469380b4acedd30494197bc3b971315b3e Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Thu, 13 Jun 2024 14:06:00 -0400 Subject: [PATCH 24/34] test: test fixes. Signed-off-by: Garett MacGowan --- test/e2e/workflow_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/workflow_test.go b/test/e2e/workflow_test.go index 4f7a5d0ea76c..76a0b6dc6a2b 100644 --- a/test/e2e/workflow_test.go +++ b/test/e2e/workflow_test.go @@ -67,7 +67,7 @@ spec: serviceAccountName: argo automountServiceAccountToken: false executor: - serviceAccountName: default + serviceAccountName: get-cm entrypoint: main templates: - name: main From 1cb41da040c93cabb98f4488b12552fa000880ef Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Thu, 13 Jun 2024 14:38:05 -0400 Subject: [PATCH 25/34] test: test fixes. Signed-off-by: Garett MacGowan --- test/e2e/manifests/mixins/get-cm-rbac.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/manifests/mixins/get-cm-rbac.yaml b/test/e2e/manifests/mixins/get-cm-rbac.yaml index 880c723cf526..4b1fda9256ac 100644 --- a/test/e2e/manifests/mixins/get-cm-rbac.yaml +++ b/test/e2e/manifests/mixins/get-cm-rbac.yaml @@ -21,7 +21,7 @@ metadata: name: get-cm-get-cm roleRef: apiGroup: rbac.authorization.k8s.io - kind: Role + kind: ClusterRole name: get-cm subjects: - kind: ServiceAccount From d59b97286233f594e4d532f40ef6de865dcd7b3c Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Thu, 13 Jun 2024 15:51:47 -0400 Subject: [PATCH 26/34] test: test fixes. Signed-off-by: Garett MacGowan --- test/e2e/manifests/mixins/get-cm-rbac.yaml | 8 ++++++++ workflow/controller/controller_test.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/test/e2e/manifests/mixins/get-cm-rbac.yaml b/test/e2e/manifests/mixins/get-cm-rbac.yaml index 4b1fda9256ac..875ac3c0a27f 100644 --- a/test/e2e/manifests/mixins/get-cm-rbac.yaml +++ b/test/e2e/manifests/mixins/get-cm-rbac.yaml @@ -3,6 +3,14 @@ kind: ServiceAccount metadata: name: get-cm --- +apiVersion: v1 +kind: Secret +metadata: + name: get-cm.service-account-token + annotations: + kubernetes.io/service-account.name: get-cm +type: kubernetes.io/service-account-token +--- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index 02e66e0d5558..250d57774362 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -453,7 +453,7 @@ type with func(pod *apiv1.Pod, woc *wfOperationCtx) func withOutputs(outputs wfv1.Outputs) with { return func(pod *apiv1.Pod, woc *wfOperationCtx) { nodeId := woc.nodeID(pod) - err := woc.controller.taskResultInformer.GetStore().Add(&wfv1.WorkflowTaskResult{ + err := woc.controller.taskResultInformer.GetIndexer().Add(&wfv1.WorkflowTaskResult{ TypeMeta: metav1.TypeMeta{ APIVersion: workflow.APIVersion, Kind: workflow.WorkflowTaskResultKind, From f881fc28d097115151593159dfba86ca55c38763 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Thu, 13 Jun 2024 17:15:52 -0400 Subject: [PATCH 27/34] test: test fixes. Signed-off-by: Garett MacGowan --- test/e2e/manifests/mixins/get-cm-rbac.yaml | 2 +- workflow/controller/controller_test.go | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/e2e/manifests/mixins/get-cm-rbac.yaml b/test/e2e/manifests/mixins/get-cm-rbac.yaml index 875ac3c0a27f..efd7a5108ba0 100644 --- a/test/e2e/manifests/mixins/get-cm-rbac.yaml +++ b/test/e2e/manifests/mixins/get-cm-rbac.yaml @@ -24,7 +24,7 @@ rules: - get --- apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding +kind: ClusterRoleBinding metadata: name: get-cm-get-cm roleRef: diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index 250d57774362..14925ee0615d 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -453,7 +453,7 @@ type with func(pod *apiv1.Pod, woc *wfOperationCtx) func withOutputs(outputs wfv1.Outputs) with { return func(pod *apiv1.Pod, woc *wfOperationCtx) { nodeId := woc.nodeID(pod) - err := woc.controller.taskResultInformer.GetIndexer().Add(&wfv1.WorkflowTaskResult{ + taskResult := &wfv1.WorkflowTaskResult{ TypeMeta: metav1.TypeMeta{ APIVersion: workflow.APIVersion, Kind: workflow.WorkflowTaskResultKind, @@ -469,7 +469,10 @@ func withOutputs(outputs wfv1.Outputs) with { Phase: wfv1.NodeSucceeded, Outputs: &outputs, }, - }) + } + _, err := woc.controller.wfclientset.ArgoprojV1alpha1().WorkflowTaskResults(woc.controller.GetManagedNamespace()).Create( + context.Background(), taskResult, metav1.CreateOptions{}, + ) if err != nil { panic(err) } From 05618dfc22eb4f79416271c1c5fe0fa96263aa26 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Thu, 13 Jun 2024 17:38:00 -0400 Subject: [PATCH 28/34] test: test fixes. Signed-off-by: Garett MacGowan --- workflow/controller/controller_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index 14925ee0615d..bcc5c5cb05ae 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -470,9 +470,12 @@ func withOutputs(outputs wfv1.Outputs) with { Outputs: &outputs, }, } - _, err := woc.controller.wfclientset.ArgoprojV1alpha1().WorkflowTaskResults(woc.controller.GetManagedNamespace()).Create( - context.Background(), taskResult, metav1.CreateOptions{}, - ) + _, err := woc.controller.wfclientset.ArgoprojV1alpha1().WorkflowTaskResults(woc.wf.Namespace). + Create( + context.Background(), + taskResult, + metav1.CreateOptions{}, + ) if err != nil { panic(err) } From 9c7e2f6e2dd271a0fd352e53a102a98605253f52 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Fri, 14 Jun 2024 01:45:54 -0400 Subject: [PATCH 29/34] test: test fixes. Signed-off-by: Garett MacGowan --- test/e2e/manifests/mixins/get-cm-rbac.yaml | 1 + workflow/controller/workflowpod.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/manifests/mixins/get-cm-rbac.yaml b/test/e2e/manifests/mixins/get-cm-rbac.yaml index efd7a5108ba0..f22befaa898c 100644 --- a/test/e2e/manifests/mixins/get-cm-rbac.yaml +++ b/test/e2e/manifests/mixins/get-cm-rbac.yaml @@ -22,6 +22,7 @@ rules: - configmaps verbs: - get + - list --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go index 267ff2d35514..3d0810a635e5 100644 --- a/workflow/controller/workflowpod.go +++ b/workflow/controller/workflowpod.go @@ -240,7 +240,7 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin } // Configuring default container to be used with commands like "kubectl exec/logs". - // Select "main" container if it's available. In other case use the last container (can happent when pod created from ContainerSet). + // Select "main" container if it's available. In other case use the last container (can happen when pod created from ContainerSet). defaultContainer := pod.Spec.Containers[len(pod.Spec.Containers)-1].Name for _, c := range pod.Spec.Containers { if c.Name == common.MainContainerName { From 49cb84b40ce43064fae239f540b68ebee8566609 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Thu, 27 Jun 2024 16:50:58 -0400 Subject: [PATCH 30/34] docs: Add note to v3.6 upgrade docs regarding required RBAC. Signed-off-by: Garett MacGowan --- docs/upgrading.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/upgrading.md b/docs/upgrading.md index 6ddcf6e55499..59b6fd03fc4f 100644 --- a/docs/upgrading.md +++ b/docs/upgrading.md @@ -15,6 +15,10 @@ Previously it was `--basehref` (no dash in between) and `ARGO_BASEHREF` (no unde `ALLOWED_LINK_PROTOCOL` and `BASE_HREF` have been removed as redundant. Use `ARGO_ALLOWED_LINK_PROTOCOL` and `ARGO_BASE_HREF` instead. +### Legacy insecure pod patch fallback removed. ([#13100](https://github.com/argoproj/argo-workflows/pull/13100)) + +For the Emissary executor to work properly, you must set up RBAC. See [workflow RBAC](workflow-rbac.md) + ### Metrics changes You can now retrieve metrics using the OpenTelemetry Protocol using the [OpenTelemetry collector](https://opentelemetry.io/docs/collector/), and this is the recommended mechanism. From ac3d68ff248cea0ff932a92fa67aa30126c01a66 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Mon, 9 Sep 2024 00:37:23 -0400 Subject: [PATCH 31/34] fix: Add AnnotationKeyProgress to support N/M initial node progress. Fixes #13100 Signed-off-by: Garett MacGowan --- workflow/common/common.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/workflow/common/common.go b/workflow/common/common.go index 429972ab4477..8c353b3219ae 100644 --- a/workflow/common/common.go +++ b/workflow/common/common.go @@ -46,6 +46,9 @@ const ( // AnnotationKeyPodNameVersion stores the pod naming convention version AnnotationKeyPodNameVersion = workflow.WorkflowFullName + "/pod-name-format" + // AnnotationKeyProgress is N/M progress for the node + AnnotationKeyProgress = workflow.WorkflowFullName + "/progress" + // AnnotationKeyArtifactGCStrategy is listed as an annotation on the Artifact GC Pod to identify // the strategy whose artifacts are being deleted AnnotationKeyArtifactGCStrategy = workflow.WorkflowFullName + "/artifact-gc-strategy" From c8b06b7c1ea884a0e29cb8fc77820fe2a428c7a8 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Mon, 9 Sep 2024 09:47:23 -0400 Subject: [PATCH 32/34] fix: Fix rebase regressions. fixes #13100 Signed-off-by: Garett MacGowan --- docs/workflow-templates.md | 2 +- examples/arguments-parameters-from-configmap.yaml | 2 +- ...-parameters-from-configmap-referenced-as-local-variable.yaml | 2 +- examples/global-parameters-from-configmap.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/workflow-templates.md b/docs/workflow-templates.md index fb30b8307af2..bb52956fab80 100644 --- a/docs/workflow-templates.md +++ b/docs/workflow-templates.md @@ -150,7 +150,7 @@ kind: Workflow metadata: generateName: hello-world-wf-global-arg- spec: - entrypoint: whalesay + entrypoint: print-message arguments: parameters: - name: global-parameter diff --git a/examples/arguments-parameters-from-configmap.yaml b/examples/arguments-parameters-from-configmap.yaml index fd6ebbb095d5..7e03b9bf70e4 100644 --- a/examples/arguments-parameters-from-configmap.yaml +++ b/examples/arguments-parameters-from-configmap.yaml @@ -11,7 +11,7 @@ metadata: workflows.argoproj.io/verify.py: | assert status["phase"] == "Succeeded" spec: - entrypoint: whalesay + entrypoint: print-message-from-configmap templates: - name: print-message-from-configmap diff --git a/examples/global-parameters-from-configmap-referenced-as-local-variable.yaml b/examples/global-parameters-from-configmap-referenced-as-local-variable.yaml index ac2976be730d..72e25bd8918f 100644 --- a/examples/global-parameters-from-configmap-referenced-as-local-variable.yaml +++ b/examples/global-parameters-from-configmap-referenced-as-local-variable.yaml @@ -9,7 +9,7 @@ metadata: This example demonstrates how a global parameter from a ConfigMap can be referenced as a template local variable. Note that the "simple-parameters" ConfigMap (defined in `examples/configmaps/simple-parameters-configmap.yaml`) needs to be created first before submitting this workflow. spec: - entrypoint: whalesay + entrypoint: print-message arguments: parameters: - name: message diff --git a/examples/global-parameters-from-configmap.yaml b/examples/global-parameters-from-configmap.yaml index ddf353310d26..349cbb83a405 100644 --- a/examples/global-parameters-from-configmap.yaml +++ b/examples/global-parameters-from-configmap.yaml @@ -9,7 +9,7 @@ metadata: This example demonstrates loading global parameter values from a ConfigMap. Note that the "simple-parameters" ConfigMap (defined in `examples/configmaps/simple-parameters-configmap.yaml`) needs to be created first before submitting this workflow. spec: - entrypoint: whalesay + entrypoint: print-message # Parameters can also be passed via configmap reference. arguments: parameters: From b4b95a895fc693cd1d452a2ed2dd0608aaffedca Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Mon, 9 Sep 2024 10:22:27 -0400 Subject: [PATCH 33/34] test: fix TestLoggedProgress. fixes #13100 Signed-off-by: Garett MacGowan --- workflow/controller/operator_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index 7818ee410f42..9339c0bc2550 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -296,20 +296,20 @@ spec: woc.operate(ctx) assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase) - assert.Equal(t, wfv1.Progress("0/1"), woc.wf.Status.Progress) - assert.Equal(t, wfv1.Progress("0/1"), woc.wf.Status.Nodes[woc.wf.Name].Progress) + assert.Equal(t, wfv1.Progress("0/100"), woc.wf.Status.Progress) + assert.Equal(t, wfv1.Progress("0/100"), woc.wf.Status.Nodes[woc.wf.Name].Progress) pod := woc.wf.Status.Nodes.FindByDisplayName("pod") - assert.Equal(t, wfv1.Progress("0/1"), pod.Progress) + assert.Equal(t, wfv1.Progress("0/100"), pod.Progress) makePodsPhase(ctx, woc, apiv1.PodSucceeded) woc = newWorkflowOperationCtx(woc.wf, controller) woc.operate(ctx) assert.Equal(t, wfv1.WorkflowSucceeded, woc.wf.Status.Phase) - assert.Equal(t, wfv1.Progress("1/1"), woc.wf.Status.Progress) - assert.Equal(t, wfv1.Progress("1/1"), woc.wf.Status.Nodes[woc.wf.Name].Progress) + assert.Equal(t, wfv1.Progress("100/100"), woc.wf.Status.Progress) + assert.Equal(t, wfv1.Progress("100/100"), woc.wf.Status.Nodes[woc.wf.Name].Progress) pod = woc.wf.Status.Nodes.FindByDisplayName("pod") - assert.Equal(t, wfv1.Progress("1/1"), pod.Progress) + assert.Equal(t, wfv1.Progress("100/100"), pod.Progress) } var sidecarWithVol = ` From c92fe1e458d3d20aa096d2caccfb799410dfd6c0 Mon Sep 17 00:00:00 2001 From: Garett MacGowan Date: Fri, 13 Sep 2024 15:59:13 -0400 Subject: [PATCH 34/34] Update workflow/controller/controller_test.go Co-authored-by: Alan Clucas Signed-off-by: Garett MacGowan --- workflow/controller/controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index bcc5c5cb05ae..3032bfcec9b7 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -483,7 +483,7 @@ func withOutputs(outputs wfv1.Outputs) with { } func withExitCode(v int32) with { - return func(pod *apiv1.Pod, woc *wfOperationCtx) { + return func(pod *apiv1.Pod, _ *wfOperationCtx) { for _, c := range pod.Spec.Containers { pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, apiv1.ContainerStatus{ Name: c.Name,