Skip to content

Commit

Permalink
fix(server&ui): can't fetch inline artifact. Fixes #9817 (#9853)
Browse files Browse the repository at this point in the history
Signed-off-by: ibotdotout <tkroputa@gmail.com>
Signed-off-by: botbotbot <tkroputa@gmail.com>
  • Loading branch information
ibotdotout committed Oct 18, 2022
1 parent ce31728 commit 05e1425
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 7 deletions.
16 changes: 10 additions & 6 deletions server/artifacts/artifact_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,16 +392,20 @@ func (a *ArtifactServer) getArtifactAndDriver(ctx context.Context, nodeId, artif
// 2. Defined by Controller configmap
// 3. Workflow spec defines artifactRepositoryRef which is a ConfigMap which defines the location
// 4. Template defines ArchiveLocation
// 5. Inline Template

var archiveLocation *wfv1.ArtifactLocation
templateName := util.GetTemplateFromNode(wf.Status.Nodes[nodeId])
template := wf.GetTemplateByName(templateName)
if template == nil {
return nil, nil, fmt.Errorf("no template found by the name of '%s' (which is the template associated with nodeId '%s'??", templateName, nodeId)
if templateName != "" {
template := wf.GetTemplateByName(templateName)
if template == nil {
return nil, nil, fmt.Errorf("no template found by the name of '%s' (which is the template associated with nodeId '%s'??", templateName, nodeId)
}
archiveLocation = template.ArchiveLocation // this is case 4
}

archiveLocation := template.ArchiveLocation // this is case 4
if !archiveLocation.HasLocation() {
ar, err := a.artifactRepositories.Get(ctx, wf.Status.ArtifactRepositoryRef) // this should handle cases 2 and 3
if templateName == "" || !archiveLocation.HasLocation() {
ar, err := a.artifactRepositories.Get(ctx, wf.Status.ArtifactRepositoryRef) // this should handle cases 2, 3 and 5
if err != nil {
return art, nil, err
}
Expand Down
49 changes: 49 additions & 0 deletions server/artifacts/artifact_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ var bucketsOfKeys = map[string][]string{
"my-wf/my-node-1/my-gcs-artifact.tgz",
"my-wf/my-node-1/my-oss-artifact.zip",
"my-wf/my-node-1/my-s3-artifact.tgz",
"my-wf/my-node-inline/main.log",
},
"my-bucket-2": {
"my-wf/my-node-2/my-s3-artifact-bucket-2",
Expand Down Expand Up @@ -305,6 +306,22 @@ func newServer() *ArtifactServer {
},
},
},
"my-node-inline": wfv1.NodeStatus{
TemplateName: "",
Outputs: &wfv1.Outputs{
Artifacts: wfv1.Artifacts{
{
Name: "my-s3-artifact-inline",
ArtifactLocation: wfv1.ArtifactLocation{
S3: &wfv1.S3Artifact{
// S3 is a configured artifact repo, so does not need key
Key: "my-wf/my-node-inline/main.log",
},
},
},
},
},
},
// a node without input/output artifacts
"my-node-no-artifacts": wfv1.NodeStatus{},
},
Expand Down Expand Up @@ -520,6 +537,38 @@ func TestArtifactServer_GetOutputArtifactWithTemplate(t *testing.T) {
}
}

func TestArtifactServer_GetOutputArtifactWithInlineTemplate(t *testing.T) {
s := newServer()

tests := []struct {
fileName string
artifactName string
}{
{
fileName: "main.log",
artifactName: "my-s3-artifact-inline",
},
}

for _, tt := range tests {
t.Run(tt.artifactName, func(t *testing.T) {
r := &http.Request{}
r.URL = mustParse(fmt.Sprintf("/artifacts/my-ns/my-wf/my-node-inline/%s", tt.artifactName))
recorder := httptest.NewRecorder()

s.GetOutputArtifact(recorder, r)
if assert.Equal(t, 200, recorder.Result().StatusCode) {
assert.Equal(t, fmt.Sprintf(`filename="%s"`, tt.fileName), recorder.Header().Get("Content-Disposition"))
all, err := io.ReadAll(recorder.Result().Body)
if err != nil {
panic(fmt.Sprintf("failed to read http body: %v", err))
}
assert.Equal(t, "my-data", string(all))
}
})
}
}

func TestArtifactServer_GetInputArtifact(t *testing.T) {
s := newServer()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,11 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps<
// (Note that individual Artifacts may also override whatever this gets set to)
if (workflow && workflow.status && workflow.status.nodes && selectedArtifact) {
const template = getResolvedTemplates(workflow, workflow.status.nodes[selectedArtifact.nodeId]);
const artifactRepo = template.archiveLocation;
let artifactRepo;
if (template) {
artifactRepo = template.archiveLocation;
}

if (artifactRepo && artifactRepoHasLocation(artifactRepo)) {
setSelectedTemplateArtifactRepo(artifactRepo);
} else {
Expand Down

0 comments on commit 05e1425

Please sign in to comment.