Skip to content

Enable linkify in the Log component#873

Closed
AlanGreene wants to merge 1 commit into
tektoncd:masterfrom
AlanGreene:linkify
Closed

Enable linkify in the Log component#873
AlanGreene wants to merge 1 commit into
tektoncd:masterfrom
AlanGreene:linkify

Conversation

@AlanGreene

Copy link
Copy Markdown
Member

Changes

Convert URLs to clickable links by setting the linkify prop
on ansi-to-react.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide
for more details.

Convert URLs to clickable links by setting the `linkify` prop
on ansi-to-react.
@tekton-robot tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 7, 2020
@tekton-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign alangreene
You can assign the PR to them by writing /assign @alangreene in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 7, 2020
@AlanGreene
AlanGreene marked this pull request as ready for review January 8, 2020 15:05
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 8, 2020
@AlanGreene
AlanGreene requested review from a-roberts and steveodonovan and removed request for CarolynMabbott January 8, 2020 15:05

@Megan-Wright Megan-Wright left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlanGreene Adam asked me to have a look at this since he's pretty busy.... code looks ok but tested out your branch and a link in the logs to github.ibm.com took me to this address:

Screenshot 2020-01-08 at 16 54 56

....so not sure it's working as intended? Thoughts?

@AlanGreene

Copy link
Copy Markdown
Member Author

hrmm that's definitely unexpected... can you share the raw log content (i.e. from the API response)?

@Megan-Wright

Copy link
Copy Markdown
Member

@AlanGreene
{"apiVersion":"tekton.dev/v1alpha1","kind":"TaskRun","metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"tekton.dev/v1alpha1\",\"kind\":\"Task\",\"metadata\":{\"annotations\":{},\"name\":\"build-push\",\"namespace\":\"tekton-pipelines\"},\"spec\":{\"inputs\":{\"params\":[{\"default\":\"./git-source/Dockerfile\",\"description\":\"The path to the dockerfile to build\",\"name\":\"pathToDockerFile\"},{\"default\":\"./git-source\",\"description\":\"The build context used by Kaniko (https://github.com/GoogleContainerTools/kaniko#kaniko-build-contexts)\",\"name\":\"pathToContext\"}],\"resources\":[{\"name\":\"git-source\",\"type\":\"git\"}]},\"outputs\":{\"resources\":[{\"name\":\"builtImage\",\"type\":\"image\"}]},\"steps\":[{\"args\":[\"build\",\"-f\",\"$(inputs.params.pathToDockerFile)\",\"-t\",\"$(outputs.resources.builtImage.url)\",\"$(inputs.params.pathToContext)\"],\"command\":[\"docker\"],\"image\":\"docker\",\"name\":\"build\",\"volumeMounts\":[{\"mountPath\":\"/var/run/docker.sock\",\"name\":\"docker-socket\"}]},{\"args\":[\"push\",\"$(outputs.resources.builtImage.url)\"],\"command\":[\"docker\"],\"image\":\"docker\",\"name\":\"push\",\"volumeMounts\":[{\"mountPath\":\"/var/run/docker.sock\",\"name\":\"docker-socket\"}]}],\"volumes\":[{\"hostPath\":{\"path\":\"/var/run/docker.sock\",\"type\":\"Socket\"},\"name\":\"docker-socket\"}]}}\n"},"creationTimestamp":"2020-01-08T13:15:00Z","generation":1,"labels":{"tekton.dev/eventlistener":"tekton-webhooks-eventlistener","tekton.dev/pipeline":"simple-pipeline","tekton.dev/pipelineRun":"simple-pipeline-run-h25g2","tekton.dev/pipelineTask":"build-simple","tekton.dev/task":"build-push","tekton.dev/triggers-eventid":"9npvc","webhooks.tekton.dev/gitBranch":"foo","webhooks.tekton.dev/gitOrg":"megan-wright","webhooks.tekton.dev/gitRepo":"knative-helloworld","webhooks.tekton.dev/gitServer":"github.ibm.com"},"name":"simple-pipeline-run-h25g2-build-simple-bnfh4","namespace":"tekton-pipelines","ownerReferences":[{"apiVersion":"tekton.dev/v1alpha1","blockOwnerDeletion":true,"controller":true,"kind":"PipelineRun","name":"simple-pipeline-run-h25g2","uid":"b33c46ed-67e2-4ed0-98ec-b580952d324a"}],"resourceVersion":"19277","selfLink":"/apis/tekton.dev/v1alpha1/namespaces/tekton-pipelines/taskruns/simple-pipeline-run-h25g2-build-simple-bnfh4","uid":"039f746d-49b1-4e9f-8ad2-4028012a62c8"},"spec":{"inputs":{"resources":[{"name":"git-source","resourceRef":{"name":"git-source-wqmfd"}}]},"outputs":{"resources":[{"name":"builtImage","paths":["build-simple/builtImage"],"resourceRef":{"name":"docker-image-wqmfd"}}]},"podTemplate":{},"serviceAccountName":"tekton-dashboard","taskRef":{"kind":"Task","name":"build-push"},"timeout":"1h0m0s"},"status":{"completionTime":"2020-01-08T13:15:37Z","conditions":[{"lastTransitionTime":"2020-01-08T13:15:37Z","message":"\"step-build\" exited with code 1 (image: \"docker-pullable://docker@sha256:f01089cb69ada00d5d27e3b77c7b0703974c48881abf16a548cbd7f5cfbb5df0\"); for logs run: kubectl -n tekton-pipelines logs simple-pipeline-run-h25g2-build-simple-bnfh4-pod-da3bed -c step-build","reason":"Failed","status":"False","type":"Succeeded"}],"podName":"simple-pipeline-run-h25g2-build-simple-bnfh4-pod-da3bed","startTime":"2020-01-08T13:15:00Z","steps":[{"container":"step-build","imageID":"docker-pullable://docker@sha256:f01089cb69ada00d5d27e3b77c7b0703974c48881abf16a548cbd7f5cfbb5df0","name":"build","terminated":{"containerID":"docker://7b4b6dd684d88799c70bde8bc02733a200b0fb64abfc1759d7275d6e40c52398","exitCode":1,"finishedAt":"2020-01-08T13:15:35Z","reason":"Error","startedAt":"2020-01-08T13:15:25Z"}},{"container":"step-push","imageID":"docker-pullable://docker@sha256:f01089cb69ada00d5d27e3b77c7b0703974c48881abf16a548cbd7f5cfbb5df0","name":"push","terminated":{"containerID":"docker://a9fbfa1f23b9778c35b93477d9944de6dccf473e60849b4187afb427ec85704b","exitCode":1,"finishedAt":"2020-01-08T13:15:36Z","reason":"Error","startedAt":"2020-01-08T13:15:28Z"}},{"container":"step-git-source-git-source-wqmfd-z74p6","imageID":"docker-pullable://gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/git-init@sha256:ce917a4a8f41a811c485fafec0f92774df3e09316da1d35e5a01e9e95a313f1e","name":"git-source-git-source-wqmfd-z74p6","terminated":{"containerID":"docker://3dec85e2aab096ff7792bb7f4e31372e93750745299727f6332b2ba2e6d23ef7","exitCode":1,"finishedAt":"2020-01-08T13:15:35Z","reason":"Error","startedAt":"2020-01-08T13:15:06Z"}},{"container":"step-image-digest-exporter-57whd","imageID":"docker-pullable://gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/imagedigestexporter@sha256:10cc6e64fbb28ad87c1a95d7300caa4545fba7903996128198077cd65ca45f0e","name":"image-digest-exporter-57whd","terminated":{"containerID":"docker://8f247d6801850a45e36a76b6d8a18181cd3c410db3bbf3a21c11f1c3040ff75e","exitCode":1,"finishedAt":"2020-01-08T13:15:37Z","message":"2020/01/08 13:15:37 Skipping step because a previous step failed\n","reason":"Error","startedAt":"2020-01-08T13:15:33Z"}},{"container":"step-create-dir-builtimage-9g2hk","imageID":"docker-pullable://busybox@sha256:6915be4043561d64e0ab0f8f098dc2ac48e077fe23f488ac24b665166898115a","name":"create-dir-builtimage-9g2hk","terminated":{"containerID":"docker://c5ca3b7ddfff48437eaac9176406cf300a79a4f56217eb33e63375b6ea9fac20","exitCode":0,"finishedAt":"2020-01-08T13:15:34Z","reason":"Completed","startedAt":"2020-01-08T13:15:06Z"}}]}}

@AlanGreene

Copy link
Copy Markdown
Member Author

@Megan-Wright can you provide the raw log content (or a stripped down version showing the affected lines) where you're seeing this issue? i.e. the response to /proxy/api/v1/namespaces/<namespace>/pods/<pod_name>/log?container=<container_name>

@Megan-Wright

Copy link
Copy Markdown
Member

Hi Alan
Screenshot 2020-01-10 at 16 38 09
I've got a URL in the logs here that isn't converted into a link, when I ran the triggers example simple-pipeline. It's not what you were asking about but thought I should mention it

@AlanGreene

Copy link
Copy Markdown
Member Author

Thanks for providing the logs @Megan-Wright, they confirmed my suspicions.

When using ansi-to-react with the linkify prop it expects URLs to be surrounded by whitespace. If the URL is surrounded by single quotes for example, it renders an anchor tag with href relative to the current page's URL as it's including the quotes, e.g. the http://localhost:5000/'https://github.ibm.com': example above

Looking at the ansi-to-react code, it appears they're doing the link parsing themselves rather than using a well-tested library such as https://www.npmjs.com/package/linkify-it. They're using a regex and some simple string splitting to attempt to extract URLs. Even with support for non-whitespace surround chars this results in a less than ideal outcome.

I've tested a number of samples that break when using the current linkify prop against linkify-it and a few other libraries, all produce far better results.

Closing this PR for now, but I'll try to put together a prototype using linkify-it in ansi-to-react instead of their current approach to confirm it works and passes their existing tests. Once we have that we can see if it's something they'd be willing to accept as a PR, or if we need to fork / look for an alternative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants