feat: Add external link annotation support. #3487

[carsonoid]

Example implementation of #3487

Using the current node.Info list is a bit clunky but requires minimal code change for the feature. I'm open to other ideas, maybe a second info list that is not automatically attached to nodes. That would allow the ui to get bits of metadata that it can use more selectively.

It's a simple interface for the end-user. Just add the annotation to any resource:

And then get a link in the interface. This was typically only done for ingress resources or other Argo links. But it can now be done on any resource. Providing a handy way to make Argo-CD the jumping-off point for any application. I think this could be extended even further with a CRD as defined in the issue. But this is a good start and would be enough for most users.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• Optional. My organization is added to USERS.md.
• I've signed the CLA and my build is green (troubleshooting builds).

[codecov-commenter]

Codecov Report

Merging #4380 into master will decrease coverage by 0.12%.
The diff coverage is 14.28%.

@@            Coverage Diff             @@
##           master    #4380      +/-   ##
==========================================
- Coverage   41.29%   41.16%   -0.13%     
==========================================
  Files         122      122              
  Lines       16584    16590       +6     
==========================================
- Hits         6848     6830      -18     
- Misses       8750     8777      +27     
+ Partials      986      983       -3     

Impacted Files	Coverage Δ
controller/cache/info.go	58.70% <14.28%> (-2.37%)	⬇️
controller/appcontroller.go	48.10% <0.00%> (-2.07%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b2e05c...d1acc77. Read the comment docs.

[jannfis]

Thanks for this contribution, I think it's a very handy feature to have!

But I'm unsure whether it's a good idea to create a link from arbitrary input such as the value of an annotation. This might easily lead to XSS and other kinds of attacks. I think we should make sure that the supplied value is indeed a valid URL or at least escape all control characters before rendering it out.

I'm not sure what React makes out of the value passed to rendering the a tag, but I feel we should put in a little more safety measures here. What do you think?

[alexmt]

We already have a field to store external URLs:

argo-cd/pkg/apis/application/v1alpha1/types.go

Line 895 in 9f47a11

ExternalURLs []string `json:"externalURLs,omitempty" protobuf:"bytes,5,opt,name=externalURLs"`

Here is example where it is populated:

argo-cd/controller/cache/info.go

Line 149 in 9f47a11

res.NetworkingInfo = &v1alpha1.ResourceNetworkingInfo{TargetRefs: targets, Ingress: ingress, ExternalURLs: urls}

It is possible to keep using it?

[carsonoid]

@alexmt great catch! That's much better. That list has the added advantage of being able to present a drop-down for multiple urls.

With that in mind, I would propose that we actually tweak the feature a bit:

• Use a prefix match for annotations to support any number of links. Each with the same title|url format as the original feature (title optional)

    link.argocd.argoproj.io/dashboard: 'Open Dashboard|https://www.google.com'
    link.argocd.argoproj.io/traces: 'Show Traces|https://www.google.com'
  

Then tweak the dropdown code just a bit to make it render the links with titles when present.

I have a rough version running locally. Here the annotations:

And this is how the dropdown looks when you have more than one matching annotation. Or when you put the annotation on an ingress resource that already has data in the externalUrls list:

If we are good with that, then I'm happy to clean up the code and submit. I would also love advice on how to best sanitize urls like @jannfis suggested.

[jannfis]

I did perform some research, and came to the conclusion that JSX expressions within curly braces {} are automatically sanitized. At least according to https://reactjs.org/docs/introducing-jsx.html#jsx-prevents-injection-attacks

So, my request for change is void :)

[alexmt]

@carsonoid I agree with your proposal. Both prefix and optional title sound good to me.

[alexmt]

It is intentional? I think we still need to render node tags.

[carsonoid]

@alexmt updated as requested.

[alexmt]

LGTM. thank you @carsonoid !

[alexmt]

@jannfis , looks like review comments have been addressed. Can you please approve so we can merge it ?

[jannfis]

LGTM