diff --git a/server/workflow/workflow_server.go b/server/workflow/workflow_server.go index ef4a5b17cc60..066ac274abbf 100644 --- a/server/workflow/workflow_server.go +++ b/server/workflow/workflow_server.go @@ -377,7 +377,7 @@ func (s *workflowServer) ResubmitWorkflow(ctx context.Context, req *workflowpkg. return nil, sutils.ToStatusError(err, codes.InvalidArgument) } - newWF, err := util.FormulateResubmitWorkflow(wf, req.Memoized, req.Parameters) + newWF, err := util.FormulateResubmitWorkflow(ctx, wf, req.Memoized, req.Parameters) if err != nil { return nil, sutils.ToStatusError(err, codes.Internal) } diff --git a/server/workflowarchive/archived_workflow_server.go b/server/workflowarchive/archived_workflow_server.go index 3e0450ca9c58..11678228c025 100644 --- a/server/workflowarchive/archived_workflow_server.go +++ b/server/workflowarchive/archived_workflow_server.go @@ -262,7 +262,7 @@ func (w *archivedWorkflowServer) ResubmitArchivedWorkflow(ctx context.Context, r return nil, sutils.ToStatusError(err, codes.Internal) } - newWF, err := util.FormulateResubmitWorkflow(wf, req.Memoized, req.Parameters) + newWF, err := util.FormulateResubmitWorkflow(ctx, wf, req.Memoized, req.Parameters) if err != nil { return nil, sutils.ToStatusError(err, codes.Internal) } diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index e6281e3e2af3..d2905529579f 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -5502,12 +5502,12 @@ status: name: my-wf phase: Failed `) - wf, err := util.FormulateResubmitWorkflow(wf, true, nil) + ctx := context.Background() + wf, err := util.FormulateResubmitWorkflow(ctx, wf, true, nil) if assert.NoError(t, err) { cancel, controller := newController(wf) defer cancel() - ctx := context.Background() woc := newWorkflowOperationCtx(wf, controller) woc.operate(ctx) assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase) @@ -5551,12 +5551,12 @@ status: name: my-wf phase: Failed `) - wf, err := util.FormulateResubmitWorkflow(wf, true, []string{"message=modified"}) + ctx := context.Background() + wf, err := util.FormulateResubmitWorkflow(ctx, wf, true, []string{"message=modified"}) if assert.NoError(t, err) { cancel, controller := newController(wf) defer cancel() - ctx := context.Background() woc := newWorkflowOperationCtx(wf, controller) woc.operate(ctx) assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase) diff --git a/workflow/creator/creator.go b/workflow/creator/creator.go index 91895bb473c8..b52f7dcd9e3c 100644 --- a/workflow/creator/creator.go +++ b/workflow/creator/creator.go @@ -15,13 +15,27 @@ import ( func Label(ctx context.Context, obj metav1.Object) { claims := auth.GetClaims(ctx) if claims != nil { - labels.Label(obj, common.LabelKeyCreator, dnsFriendly(claims.Subject)) + if claims.Subject != "" { + labels.Label(obj, common.LabelKeyCreator, dnsFriendly(claims.Subject)) + } else { + labels.UnLabel(obj, common.LabelKeyCreator) + } if claims.Email != "" { labels.Label(obj, common.LabelKeyCreatorEmail, dnsFriendly(strings.Replace(claims.Email, "@", ".at.", 1))) + } else { + labels.UnLabel(obj, common.LabelKeyCreatorEmail) } if claims.PreferredUsername != "" { labels.Label(obj, common.LabelKeyCreatorPreferredUsername, dnsFriendly(claims.PreferredUsername)) + } else { + labels.UnLabel(obj, common.LabelKeyCreatorPreferredUsername) } + } else { + // If the object already has creator-related labels, but the actual request lacks auth information, + // remove the creator-related labels from the object. + labels.UnLabel(obj, common.LabelKeyCreator) + labels.UnLabel(obj, common.LabelKeyCreatorEmail) + labels.UnLabel(obj, common.LabelKeyCreatorPreferredUsername) } } diff --git a/workflow/creator/creator_test.go b/workflow/creator/creator_test.go index 945d02bc92f7..2796ab873e9a 100644 --- a/workflow/creator/creator_test.go +++ b/workflow/creator/creator_test.go @@ -7,6 +7,7 @@ import ( "github.com/go-jose/go-jose/v3/jwt" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" "github.com/argoproj/argo-workflows/v3/server/auth" @@ -52,4 +53,80 @@ func TestLabel(t *testing.T) { assert.Equal(t, strings.Repeat("x", 20), wf.Labels[common.LabelKeyCreator]) } }) + t.Run("DifferentUsersFromCreatorLabels", func(t *testing.T) { + type input struct { + claims *types.Claims + wf *wfv1.Workflow + } + type output struct { + creatorLabelsToHave map[string]string + creatorLabelsNotToHave []string + } + for _, testCase := range []struct { + name string + input *input + output *output + }{ + { + name: "when claims are empty", + input: &input{ + claims: &types.Claims{Claims: jwt.Claims{}}, + wf: &wfv1.Workflow{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + common.LabelKeyCreator: "xxxx-xxxx-xxxx-xxxx", + common.LabelKeyCreatorEmail: "foo.at.example.com", + common.LabelKeyCreatorPreferredUsername: "foo", + }}}, + }, + output: &output{ + creatorLabelsToHave: nil, + creatorLabelsNotToHave: []string{common.LabelKeyCreator, common.LabelKeyCreatorEmail, common.LabelKeyCreatorPreferredUsername}, + }, + }, { + name: "when claims are nil", + input: &input{ + claims: nil, + wf: &wfv1.Workflow{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + common.LabelKeyCreator: "xxxx-xxxx-xxxx-xxxx", + common.LabelKeyCreatorEmail: "foo.at.example.com", + common.LabelKeyCreatorPreferredUsername: "foo", + }}}, + }, + output: &output{ + creatorLabelsToHave: nil, + creatorLabelsNotToHave: []string{common.LabelKeyCreator, common.LabelKeyCreatorEmail, common.LabelKeyCreatorPreferredUsername}, + }, + }, { + name: "when user information in claim is different from the existing labels of a Workflow", + input: &input{ + claims: &types.Claims{Claims: jwt.Claims{Subject: "yyyy-yyyy-yyyy-yyyy"}, Email: "bar.at.example.com", PreferredUsername: "bar"}, + wf: &wfv1.Workflow{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + common.LabelKeyCreator: "xxxx-xxxx-xxxx-xxxx", + common.LabelKeyCreatorEmail: "foo.at.example.com", + common.LabelKeyCreatorPreferredUsername: "foo", + }}}, + }, + output: &output{ + creatorLabelsToHave: map[string]string{ + common.LabelKeyCreator: "yyyy-yyyy-yyyy-yyyy", + common.LabelKeyCreatorEmail: "bar.at.example.com", + common.LabelKeyCreatorPreferredUsername: "bar", + }, + creatorLabelsNotToHave: nil, + }, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + Label(context.WithValue(context.TODO(), auth.ClaimsKey, testCase.input.claims), testCase.input.wf) + labels := testCase.input.wf.GetLabels() + for k, expectedValue := range testCase.output.creatorLabelsToHave { + assert.Equal(t, expectedValue, labels[k]) + } + for _, creatorLabelKey := range testCase.output.creatorLabelsNotToHave { + _, ok := labels[creatorLabelKey] + assert.Falsef(t, ok, "should not have the creator label, \"%s\"", creatorLabelKey) + } + }) + + } + }) } diff --git a/workflow/util/util.go b/workflow/util/util.go index 6543e92c3ca1..dd8b9cea79e0 100644 --- a/workflow/util/util.go +++ b/workflow/util/util.go @@ -35,6 +35,8 @@ import ( "k8s.io/utils/pointer" "sigs.k8s.io/yaml" + "github.com/argoproj/argo-workflows/v3/workflow/creator" + "github.com/argoproj/argo-workflows/v3/errors" "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" @@ -606,7 +608,7 @@ func RandSuffix() string { } // FormulateResubmitWorkflow formulate a new workflow from a previous workflow, optionally re-using successful nodes -func FormulateResubmitWorkflow(wf *wfv1.Workflow, memoized bool, parameters []string) (*wfv1.Workflow, error) { +func FormulateResubmitWorkflow(ctx context.Context, wf *wfv1.Workflow, memoized bool, parameters []string) (*wfv1.Workflow, error) { newWF := wfv1.Workflow{} newWF.TypeMeta = wf.TypeMeta @@ -644,12 +646,16 @@ func FormulateResubmitWorkflow(wf *wfv1.Workflow, memoized bool, parameters []st } for key, val := range wf.ObjectMeta.Labels { switch key { - case common.LabelKeyCreator, common.LabelKeyPhase, common.LabelKeyCompleted, common.LabelKeyWorkflowArchivingStatus: + case common.LabelKeyCreator, common.LabelKeyCreatorEmail, common.LabelKeyCreatorPreferredUsername, + common.LabelKeyPhase, common.LabelKeyCompleted, common.LabelKeyWorkflowArchivingStatus: // ignore default: newWF.ObjectMeta.Labels[key] = val } } + // Apply creator labels based on the authentication information of the current request, + // regardless of the creator labels of the original Workflow. + creator.Label(ctx, &newWF) // Append an additional label so it's easy for user to see the // name of the original workflow that has been resubmitted. newWF.ObjectMeta.Labels[common.LabelKeyPreviousWorkflowName] = wf.ObjectMeta.Name diff --git a/workflow/util/util_test.go b/workflow/util/util_test.go index 7cc4ea8e81ed..1c66429b5f79 100644 --- a/workflow/util/util_test.go +++ b/workflow/util/util_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/go-jose/go-jose/v3/jwt" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -15,6 +16,9 @@ import ( "k8s.io/apimachinery/pkg/fields" "sigs.k8s.io/yaml" + "github.com/argoproj/argo-workflows/v3/server/auth" + "github.com/argoproj/argo-workflows/v3/server/auth/types" + "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" argofake "github.com/argoproj/argo-workflows/v3/pkg/client/clientset/versioned/fake" @@ -67,7 +71,7 @@ func TestResubmitWorkflowWithOnExit(t *testing.T) { Name: onExitName, Phase: wfv1.NodeSucceeded, } - newWF, err := FormulateResubmitWorkflow(&wf, true, nil) + newWF, err := FormulateResubmitWorkflow(context.Background(), &wf, true, nil) assert.NoError(t, err) newWFOnExitName := newWF.ObjectMeta.Name + ".onExit" newWFOneExitID := newWF.NodeID(newWFOnExitName) @@ -640,7 +644,7 @@ func TestFormulateResubmitWorkflow(t *testing.T) { }, }, } - wf, err := FormulateResubmitWorkflow(wf, false, nil) + wf, err := FormulateResubmitWorkflow(context.Background(), wf, false, nil) if assert.NoError(t, err) { assert.Contains(t, wf.GetLabels(), common.LabelKeyControllerInstanceID) assert.Contains(t, wf.GetLabels(), common.LabelKeyClusterWorkflowTemplate) @@ -656,6 +660,57 @@ func TestFormulateResubmitWorkflow(t *testing.T) { assert.Equal(t, "testObj", wf.OwnerReferences[0].Name) } }) + t.Run("OverrideCreatorLabels", func(t *testing.T) { + wf := &wfv1.Workflow{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + common.LabelKeyCreator: "xxxx-xxxx-xxxx", + common.LabelKeyCreatorEmail: "foo.at.example.com", + common.LabelKeyCreatorPreferredUsername: "foo", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "test", + Name: "testObj", + }, + }, + }, + } + ctx := context.WithValue(context.Background(), auth.ClaimsKey, &types.Claims{ + Claims: jwt.Claims{Subject: "yyyy-yyyy-yyyy-yyyy"}, + Email: "bar.at.example.com", + PreferredUsername: "bar", + }) + wf, err := FormulateResubmitWorkflow(ctx, wf, false, nil) + if assert.NoError(t, err) { + assert.Equal(t, "yyyy-yyyy-yyyy-yyyy", wf.Labels[common.LabelKeyCreator]) + assert.Equal(t, "bar.at.example.com", wf.Labels[common.LabelKeyCreatorEmail]) + assert.Equal(t, "bar", wf.Labels[common.LabelKeyCreatorPreferredUsername]) + } + }) + t.Run("UnlabelCreatorLabels", func(t *testing.T) { + wf := &wfv1.Workflow{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + common.LabelKeyCreator: "xxxx-xxxx-xxxx", + common.LabelKeyCreatorEmail: "foo.at.example.com", + common.LabelKeyCreatorPreferredUsername: "foo", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "test", + Name: "testObj", + }, + }, + }, + } + wf, err := FormulateResubmitWorkflow(context.Background(), wf, false, nil) + if assert.NoError(t, err) { + assert.Emptyf(t, wf.Labels[common.LabelKeyCreator], "should not %s label when a workflow is resubmitted by an unauthenticated request", common.LabelKeyCreator) + assert.Emptyf(t, wf.Labels[common.LabelKeyCreatorEmail], "should not %s label when a workflow is resubmitted by an unauthenticated request", common.LabelKeyCreatorEmail) + assert.Emptyf(t, wf.Labels[common.LabelKeyCreatorPreferredUsername], "should not %s label when a workflow is resubmitted by an unauthenticated request", common.LabelKeyCreatorPreferredUsername) + } + }) t.Run("OverrideParams", func(t *testing.T) { wf := &wfv1.Workflow{ Spec: wfv1.WorkflowSpec{Arguments: wfv1.Arguments{ @@ -664,7 +719,7 @@ func TestFormulateResubmitWorkflow(t *testing.T) { }, }}, } - wf, err := FormulateResubmitWorkflow(wf, false, []string{"message=modified"}) + wf, err := FormulateResubmitWorkflow(context.Background(), wf, false, []string{"message=modified"}) if assert.NoError(t, err) { assert.Equal(t, "modified", wf.Spec.Arguments.Parameters[0].Value.String()) }