Skip to content

Commit 5fc8d4f

Browse files
eduardodbrgithub-actions[bot]
authored andcommitted
fix: Add optional UID query parameter to GetWorkflow (#15251)
Signed-off-by: Eduardo Rodrigues <eduardodbr@hotmail.com>
1 parent 9e69e21 commit 5fc8d4f

File tree

9 files changed

+280
-120
lines changed

9 files changed

+280
-120
lines changed

api/openapi-spec/swagger.json

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/apiclient/workflow/workflow.pb.go

Lines changed: 147 additions & 94 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/apiclient/workflow/workflow.proto

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ message WorkflowGetRequest {
2626
k8s.io.apimachinery.pkg.apis.meta.v1.GetOptions getOptions = 3;
2727
// Fields to be included or excluded in the response. e.g. "spec,status.phase", "-status.nodes"
2828
string fields = 4;
29+
// Optional UID to retrieve a specific workflow (useful for archived workflows with the same name)
30+
string uid = 5;
2931
}
3032

3133
message WorkflowListRequest {

sdks/java/client/docs/WorkflowServiceApi.md

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

server/workflow/workflow_server.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func (s *workflowServer) GetWorkflow(ctx context.Context, req *workflowpkg.Workf
150150
wfGetOption = *req.GetOptions
151151
}
152152
wfClient := auth.GetWfClient(ctx)
153-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, wfGetOption)
153+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, req.Uid, wfGetOption)
154154
if err != nil {
155155
return nil, sutils.ToStatusError(err, codes.Internal)
156156
}
@@ -311,7 +311,7 @@ func (s *workflowServer) WatchWorkflows(req *workflowpkg.WatchWorkflowsRequest,
311311
if wfName != "" {
312312
// If we are using an alias (such as `@latest`) we need to dereference it.
313313
// s.getWorkflow does that for us
314-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, wfName, metav1.GetOptions{})
314+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, wfName, "", metav1.GetOptions{})
315315
if err != nil {
316316
return sutils.ToStatusError(err, codes.Internal)
317317
}
@@ -432,7 +432,7 @@ func (s *workflowServer) WatchEvents(req *workflowpkg.WatchEventsRequest, ws wor
432432

433433
func (s *workflowServer) DeleteWorkflow(ctx context.Context, req *workflowpkg.WorkflowDeleteRequest) (*workflowpkg.WorkflowDeleteResponse, error) {
434434
wfClient := auth.GetWfClient(ctx)
435-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, metav1.GetOptions{})
435+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, "", metav1.GetOptions{})
436436
if err != nil {
437437
return nil, sutils.ToStatusError(err, codes.Internal)
438438
}
@@ -467,7 +467,7 @@ func (s *workflowServer) RetryWorkflow(ctx context.Context, req *workflowpkg.Wor
467467
wfClient := auth.GetWfClient(ctx)
468468
kubeClient := auth.GetKubeClient(ctx)
469469

470-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, metav1.GetOptions{})
470+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, "", metav1.GetOptions{})
471471
if err != nil {
472472
return nil, sutils.ToStatusError(err, codes.Internal)
473473
}
@@ -525,7 +525,7 @@ func (s *workflowServer) RetryWorkflow(ctx context.Context, req *workflowpkg.Wor
525525

526526
func (s *workflowServer) ResubmitWorkflow(ctx context.Context, req *workflowpkg.WorkflowResubmitRequest) (*wfv1.Workflow, error) {
527527
wfClient := auth.GetWfClient(ctx)
528-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, metav1.GetOptions{})
528+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, "", metav1.GetOptions{})
529529
if err != nil {
530530
return nil, sutils.ToStatusError(err, codes.Internal)
531531
}
@@ -550,7 +550,7 @@ func (s *workflowServer) ResubmitWorkflow(ctx context.Context, req *workflowpkg.
550550

551551
func (s *workflowServer) ResumeWorkflow(ctx context.Context, req *workflowpkg.WorkflowResumeRequest) (*wfv1.Workflow, error) {
552552
wfClient := auth.GetWfClient(ctx)
553-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, metav1.GetOptions{})
553+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, "", metav1.GetOptions{})
554554
if err != nil {
555555
return nil, sutils.ToStatusError(err, codes.Internal)
556556
}
@@ -579,7 +579,7 @@ func (s *workflowServer) ResumeWorkflow(ctx context.Context, req *workflowpkg.Wo
579579
func (s *workflowServer) SuspendWorkflow(ctx context.Context, req *workflowpkg.WorkflowSuspendRequest) (*wfv1.Workflow, error) {
580580
wfClient := auth.GetWfClient(ctx)
581581

582-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, metav1.GetOptions{})
582+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, "", metav1.GetOptions{})
583583
if err != nil {
584584
return nil, sutils.ToStatusError(err, codes.Internal)
585585
}
@@ -605,7 +605,7 @@ func (s *workflowServer) SuspendWorkflow(ctx context.Context, req *workflowpkg.W
605605
func (s *workflowServer) TerminateWorkflow(ctx context.Context, req *workflowpkg.WorkflowTerminateRequest) (*wfv1.Workflow, error) {
606606
wfClient := auth.GetWfClient(ctx)
607607

608-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, metav1.GetOptions{})
608+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, "", metav1.GetOptions{})
609609
if err != nil {
610610
return nil, sutils.ToStatusError(err, codes.Internal)
611611
}
@@ -629,7 +629,7 @@ func (s *workflowServer) TerminateWorkflow(ctx context.Context, req *workflowpkg
629629

630630
func (s *workflowServer) StopWorkflow(ctx context.Context, req *workflowpkg.WorkflowStopRequest) (*wfv1.Workflow, error) {
631631
wfClient := auth.GetWfClient(ctx)
632-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, metav1.GetOptions{})
632+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, "", metav1.GetOptions{})
633633
if err != nil {
634634
return nil, sutils.ToStatusError(err, codes.Internal)
635635
}
@@ -651,7 +651,7 @@ func (s *workflowServer) StopWorkflow(ctx context.Context, req *workflowpkg.Work
651651

652652
func (s *workflowServer) SetWorkflow(ctx context.Context, req *workflowpkg.WorkflowSetRequest) (*wfv1.Workflow, error) {
653653
wfClient := auth.GetWfClient(ctx)
654-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, metav1.GetOptions{})
654+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, "", metav1.GetOptions{})
655655
if err != nil {
656656
return nil, sutils.ToStatusError(err, codes.Internal)
657657
}
@@ -715,7 +715,7 @@ func (s *workflowServer) PodLogs(req *workflowpkg.WorkflowLogRequest, ws workflo
715715
ctx := ws.Context()
716716
wfClient := auth.GetWfClient(ctx)
717717
kubeClient := auth.GetKubeClient(ctx)
718-
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, metav1.GetOptions{})
718+
wf, err := s.getWorkflow(ctx, wfClient, req.Namespace, req.Name, "", metav1.GetOptions{})
719719
if err != nil {
720720
return sutils.ToStatusError(err, codes.Internal)
721721
}
@@ -738,7 +738,7 @@ func (s *workflowServer) WorkflowLogs(req *workflowpkg.WorkflowLogRequest, ws wo
738738
return sutils.ToStatusError(s.PodLogs(req, ws), codes.Internal)
739739
}
740740

741-
func (s *workflowServer) getWorkflow(ctx context.Context, wfClient versioned.Interface, namespace string, name string, options metav1.GetOptions) (*wfv1.Workflow, error) {
741+
func (s *workflowServer) getWorkflow(ctx context.Context, wfClient versioned.Interface, namespace string, name string, uid string, options metav1.GetOptions) (*wfv1.Workflow, error) {
742742
logger := logging.RequireLoggerFromContext(ctx)
743743
if name == latestAlias {
744744
latest, err := getLatestWorkflow(ctx, wfClient, namespace)
@@ -751,7 +751,7 @@ func (s *workflowServer) getWorkflow(ctx context.Context, wfClient versioned.Int
751751

752752
wf, origErr := wfClient.ArgoprojV1alpha1().Workflows(namespace).Get(ctx, name, options)
753753
// fallback to retrieve from archived workflows
754-
if wf == nil || origErr != nil {
754+
if wf == nil || origErr != nil || (uid != "" && string(wf.UID) != uid) {
755755
allowed, err := auth.CanI(ctx, "get", workflow.WorkflowPlural, namespace, name)
756756
if err != nil {
757757
return nil, getWorkflowOrigErr(ctx, origErr, err)
@@ -761,7 +761,7 @@ func (s *workflowServer) getWorkflow(ctx context.Context, wfClient versioned.Int
761761
return nil, getWorkflowOrigErr(ctx, origErr, err)
762762
}
763763

764-
wf, err = s.wfArchive.GetWorkflow(ctx, "", namespace, name)
764+
wf, err = s.wfArchive.GetWorkflow(ctx, uid, namespace, name)
765765
if wf == nil || err != nil {
766766
return nil, getWorkflowOrigErr(ctx, origErr, err)
767767
}

server/workflow/workflow_server_test.go

Lines changed: 84 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,17 @@ func getWorkflowServer(t *testing.T) (workflowpkg.WorkflowServiceServer, context
611611
archivedRepo.On("GetWorkflow", mock.Anything, "", "test", "unlabelled").Return(nil, nil)
612612
archivedRepo.On("GetWorkflow", mock.Anything, "", "workflows", "latest").Return(nil, nil)
613613
archivedRepo.On("GetWorkflow", mock.Anything, "", "workflows", "hello-world-9tql2-not").Return(nil, nil)
614+
// Mock for UID-based lookups on archive
615+
archivedRepo.On("GetWorkflow", mock.Anything, "91066a6c-1ddc-11ea-b443-42010aa80099", "test", "hello-world-9tql2-test").Return(&v1alpha1.Workflow{
616+
ObjectMeta: metav1.ObjectMeta{
617+
Name: "hello-world-9tql2-test",
618+
Namespace: "test",
619+
UID: "91066a6c-1ddc-11ea-b443-42010aa80099",
620+
Labels: map[string]string{
621+
common.LabelKeyControllerInstanceID: "my-instanceid", // necessary to pass validation
622+
},
623+
},
624+
}, nil)
614625
r, err := labels.ParseToRequirements("workflows.argoproj.io/controller-instanceid=my-instanceid")
615626
if err != nil {
616627
panic(err)
@@ -750,10 +761,10 @@ func TestGetWorkflow(t *testing.T) {
750761
server, ctx := getWorkflowServer(t)
751762
s := server.(*workflowServer)
752763
wfClient := auth.GetWfClient(ctx)
753-
wf, err := s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", metav1.GetOptions{})
764+
wf, err := s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", "", metav1.GetOptions{})
754765
require.NoError(t, err)
755766
assert.NotNil(t, wf)
756-
wf, err = s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", metav1.GetOptions{})
767+
wf, err = s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", "", metav1.GetOptions{})
757768
require.NoError(t, err)
758769
assert.NotNil(t, wf)
759770
}
@@ -762,11 +773,81 @@ func TestValidateWorkflow(t *testing.T) {
762773
server, ctx := getWorkflowServer(t)
763774
s := server.(*workflowServer)
764775
wfClient := auth.GetWfClient(ctx)
765-
wf, err := s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", metav1.GetOptions{})
776+
wf, err := s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", "", metav1.GetOptions{})
766777
require.NoError(t, err)
767778
require.NoError(t, s.validateWorkflow(wf))
768779
}
769780

781+
func TestGetWorkflowByUID(t *testing.T) {
782+
server, ctx := getWorkflowServer(t)
783+
s := server.(*workflowServer)
784+
wfClient := auth.GetWfClient(ctx)
785+
786+
t.Run("GetWorkflowWithValidUID", func(t *testing.T) {
787+
wf, err := s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", "6522aff1-1e01-11ea-b443-42010aa80074", metav1.GetOptions{})
788+
require.NoError(t, err)
789+
assert.NotNil(t, wf)
790+
assert.Equal(t, "hello-world-9tql2-test", wf.Name)
791+
assert.Equal(t, "6522aff1-1e01-11ea-b443-42010aa80074", string(wf.UID))
792+
})
793+
794+
t.Run("GetWorkflowWithUIDMatchesClusterUID", func(t *testing.T) {
795+
wfWithUID, err := s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", "6522aff1-1e01-11ea-b443-42010aa80074", metav1.GetOptions{})
796+
require.NoError(t, err)
797+
assert.NotNil(t, wfWithUID)
798+
wfWithoutUID, err := s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", "", metav1.GetOptions{})
799+
require.NoError(t, err)
800+
assert.NotNil(t, wfWithoutUID)
801+
assert.Equal(t, wfWithoutUID, wfWithUID)
802+
})
803+
804+
t.Run("GetWorkflowWithMismatchedUID", func(t *testing.T) {
805+
// The cluster has uid "6522aff1-1e01-11ea-b443-42010aa80074"
806+
// archive has uid "91066a6c-1ddc-11ea-b443-42010aa80099"
807+
wf, err := s.getWorkflow(ctx, wfClient, "test", "hello-world-9tql2-test", "91066a6c-1ddc-11ea-b443-42010aa80099", metav1.GetOptions{})
808+
require.NoError(t, err)
809+
assert.NotNil(t, wf)
810+
assert.Equal(t, "91066a6c-1ddc-11ea-b443-42010aa80099", string(wf.UID))
811+
})
812+
813+
t.Run("GetClusterWorkflowViaAPI", func(t *testing.T) {
814+
// Test the full API with UID parameter
815+
wf, err := server.GetWorkflow(ctx, &workflowpkg.WorkflowGetRequest{
816+
Name: "hello-world-9tql2-test",
817+
Namespace: "test",
818+
Uid: "6522aff1-1e01-11ea-b443-42010aa80074",
819+
})
820+
require.NoError(t, err)
821+
assert.NotNil(t, wf)
822+
assert.Equal(t, "hello-world-9tql2-test", wf.Name)
823+
assert.Equal(t, "6522aff1-1e01-11ea-b443-42010aa80074", string(wf.UID))
824+
})
825+
826+
t.Run("GetClusterWorkflowViaAPIWithoutUID", func(t *testing.T) {
827+
// Test the full API without UID parameter
828+
wf, err := server.GetWorkflow(ctx, &workflowpkg.WorkflowGetRequest{
829+
Name: "hello-world-9tql2-test",
830+
Namespace: "test",
831+
Uid: "",
832+
})
833+
require.NoError(t, err)
834+
assert.NotNil(t, wf)
835+
assert.Equal(t, "hello-world-9tql2-test", wf.Name)
836+
assert.Equal(t, "6522aff1-1e01-11ea-b443-42010aa80074", string(wf.UID))
837+
})
838+
839+
t.Run("GetArchivedWorkflowViaAPI", func(t *testing.T) {
840+
wf, err := server.GetWorkflow(ctx, &workflowpkg.WorkflowGetRequest{
841+
Name: "hello-world-9tql2-test",
842+
Namespace: "test",
843+
Uid: "91066a6c-1ddc-11ea-b443-42010aa80099",
844+
})
845+
require.NoError(t, err)
846+
assert.NotNil(t, wf)
847+
assert.Equal(t, "91066a6c-1ddc-11ea-b443-42010aa80099", string(wf.UID))
848+
})
849+
}
850+
770851
func TestListWorkflow(t *testing.T) {
771852
server, ctx := getWorkflowServer(t)
772853
wfl, err := getWorkflowList(ctx, server, "workflows")

test/e2e/argo_server_test.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -809,9 +809,9 @@ func (s *ArgoServerSuite) TestCreateWorkflowDryRun() {
809809
}
810810

811811
func (s *ArgoServerSuite) TestWorkflowService() {
812-
var name string
812+
var name, uid string
813813
s.Run("Create", func() {
814-
name = s.e().POST("/api/v1/workflows/argo").
814+
jsonResp := s.e().POST("/api/v1/workflows/argo").
815815
WithBytes([]byte(`{
816816
"workflow": {
817817
"metadata": {
@@ -836,8 +836,12 @@ func (s *ArgoServerSuite) TestWorkflowService() {
836836
}`)).
837837
Expect().
838838
Status(200).
839-
JSON().
840-
Path("$.metadata.name").
839+
JSON()
840+
name = jsonResp.Path("$.metadata.name").
841+
NotNull().
842+
String().
843+
Raw()
844+
uid = jsonResp.Path("$.metadata.uid").
841845
NotNull().
842846
String().
843847
Raw()
@@ -891,6 +895,16 @@ func (s *ArgoServerSuite) TestWorkflowService() {
891895
Status(404)
892896
})
893897

898+
s.Run("GetByUid", func() {
899+
j := s.e().GET("/api/v1/workflows/argo/"+name).
900+
WithQuery("uid", uid).
901+
Expect().
902+
Status(200).
903+
JSON()
904+
j.Path("$.status.nodes").
905+
NotNull()
906+
})
907+
894908
s.Run("GetWithFields", func() {
895909
j := s.e().GET("/api/v1/workflows/argo/"+name).
896910
WithQuery("fields", "status.phase").

ui/src/shared/services/workflows-service.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@ export const WorkflowsService = {
6464
return requests.get(`api/v1/workflows/${namespace}?${params.join('&')}`).then(res => res.body as WorkflowList);
6565
},
6666

67-
get(namespace: string, name: string) {
68-
return requests.get(`api/v1/workflows/${namespace}/${name}`).then(res => res.body as Workflow);
67+
get(namespace: string, name: string, uid?: string) {
68+
const params = uid ? `?uid=${uid}` : '';
69+
return requests.get(`api/v1/workflows/${namespace}/${name}${params}`).then(res => res.body as Workflow);
6970
},
7071

7172
getArchived(namespace: string, uid: string) {

ui/src/workflows/components/workflow-details/workflow-details.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,8 @@ export function WorkflowDetails({history, location, match}: RouteComponentProps<
394394
useEffect(() => {
395395
(async () => {
396396
try {
397-
const wf = await services.workflows.get(namespace, name);
397+
// Pass uid if available from URL query params
398+
const wf = await services.workflows.get(namespace, name, uid || undefined);
398399
setUid(wf.metadata.uid);
399400
setWorkflow(wf);
400401
setError(null);

0 commit comments

Comments
 (0)