ArgoCD UI Ingress go-to links use wrong URL scheme (HTTP)

[brsolomon-deloitte]

Checklist:

• I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
• I've included steps to reproduce the bug.
• I've pasted the output of argocd version.

Describe the bug

In the ArgoCD UI, the external link button for Ingress resources points to http rather than https. Our load balancer is only exposed over https/443 (TLS termination occurs there), so the links from ArgoCD are effectively dead links with the wrong scheme. Is there a way to configure this to generate HTTPS links?

To Reproduce

Deploy ArgoCD, nginx-ingress-controller, and any Ingress resource.

Expected behavior

Link uses HTTPS scheme or is configurable in some way to use HTTPS by default, not HTTP.

Screenshots

The generated HTML looks like:

<div class="applications-list__external-links-icon-container">
  <a title="http://jupyter.example.com/">
    <i class="fa fa-external-link-alt"></i>
  </a>
</div>

Whereas the link should be "https://jupyter.example.com/"

Version

argocd: v2.1.3+d855831
  BuildDate: 2021-09-29T21:51:21Z
  GitCommit: d855831540e51d8a90b1006d2eb9f49ab1b088af
  GitTreeState: clean
  GoVersion: go1.16.5
  Compiler: gc
  Platform: linux/amd64

[brsolomon-deloitte]

Found some relevant snippets here:

• argo-cd/ui/src/app/applications/components/applications-list/applications-tiles.tsx

  Line 114 in 6f5f7ff

  <ApplicationURLs urls={app.status.summary.externalURLs} />

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

  Line 1057 in 3bd8688

  // ExternalURLs holds list of URLs which should be available externally. List is populated for ingress resources using rules hostnames.

• argo-cd/controller/cache/info.go

  Line 109 in a940cb5

  func populateIngressInfo(un *unstructured.Unstructured, res *ResourceInfo) {

[brsolomon-deloitte]

It looks like the crux of this happens here:

argo-cd/controller/cache/info.go

Line 163 in a940cb5

stringPort := "http"

with the determination of stringPort that follows this line.

The defect of how stringPort is configured is that it is entirely dependent on whether TLS is configured within the Ingress definition. This is not the only want to enforce/induce HTTPS. Another very common way, for instance, is to add annotations to the ingress-nginx Service to specify the AWS Cert. Using this method, there is zero specification of a tls Secret required for each Ingress.

So, it seems like:

• ArgoCD's method for determining stringPort is too narrow; it makes the assumption that HTTPS should only be decided based on whether a TLS Secret (Ingress.spec.tls) is specified in the Ingress resource
• ArgoCD could, but does not, offer configurability to override this behavior. (For instance, a Helm chart parameter that dictates that all Ingress links should go to HTTPS without any of the stringPort formula.

[brsolomon-deloitte]

On 2nd bullet above, perhaps the ArgoCD controller could listen to an environment variable such as ARGO_INGRESS_LINK_PROTO, which accepts a string pointing to a protocol to use.

[jannfis]

I think that using an environment variable for this would be too narrow as well

An idea would be to use an annotation on the resource itself to specify the URL for the link-out directly, either an URL or just the protocol, and use this value for rendering the link. We do have a mechanism to add links to the Application resource itself using the link.argocd.argoproj.io/<linkname>: <url> annotation, so we may just reuse that, but set it on the Ingress (or Route, or Service) resource:

For example:

# will use the URL from the Ingress, and will use https as protocol scheme without applying logic to determine the protocol
link.argocd.argoproj.io/external-protocol: https

# will use the complete URL instead of the one specified in the Ingress
link.argocd.argoproj.io/external-url: https://some.rand.om/link/to/the/app

/cc @alexmt @jessesuen

[brsolomon-deloitte]

so we may just reuse that, but set it on the Ingress (or Route, or Service) resource:

From a k8s admin perspective, my only gripe with this is it now introduces some necessary duplication, it seems. Let's say I have 10 different Ingress resources deployed, and they all sit behind a reverse proxy with TLS terminated at the load balancer; this would necessitate an annotation on each one of the Ingress specs, right?

[jannfis]

Actually, there is already some progress on this: #3487 and #7923

[brsolomon-deloitte]

A workaround we've found for this is to have the ingress controller just issue an 80->443 redirect rather than removing the port 80 listener entirely. I realize that setup doesn't describe everyone's situation, though.

[eburnette]

#6901 uses https if you have any tls hosts, which is good enough for me.

[maxnitze]

I have a similar problem. And I'm definitely on @brsolomon-deloitte s side with the duplication: I have multiple clusters with hundreds of applications. I would need to add the annotation to all of their ingress resources.

I'm using Traefik as my ingress controller and in most of my clusters Traefik is configured with a wildcard certificate. So no Ingress resource has any TLS configuration on its own.

My idea would be a config key for the cluster like:

apiVersion: v1
kind: Secret
metadata:
  name: mycluster-secret
  labels:
    argocd.argoproj.io/secret-type: cluster
type: Opaque
stringData:
  name: mycluster.com
  server: https://mycluster.com
  config: |
    {
      ...
      "defaultIngressProtocol": "https", # optional, one of null or "", "http", "https"
      ...
    }

see https://github.com/argoproj/argo-cd/blob/master/docs/operator-manual/declarative-setup.md#clusters

argo-cd/controller/cache/info.go

Lines 163 to 187 in a940cb5

	stringPort := "http"
	if tls, ok, err := unstructured.NestedSlice(un.Object, "spec", "tls"); ok && err == nil {
	for i := range tls {
	tlsline, ok := tls[i].(map[string]interface{})
	secretName := tlsline["secretName"]
	if ok && secretName != nil {
	stringPort = "https"
	}
	tlshost := tlsline["host"]
	if tlshost == host {
	stringPort = "https"
	continue
	}
	if hosts := tlsline["hosts"]; hosts != nil {
	tlshosts, ok := tlsline["hosts"].(map[string]interface{})
	if ok {
	for j := range tlshosts {
	if tlshosts[j] == host {
	stringPort = "https"
	}
	}
	}
	}
	}
	}

And then in this controller we could do something like:

if clusterConfig.defaultIngressProtocol AND clusterConfig.defaultIngressProtocol not in [null, ""]
    stringPort = clusterConfig.defaultIngressProtocol
else
    // check, if there is a TLS config
    // fallback to http otherwise
fi

What do you think?