argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.32k stars 5.26k forks source link

TLS certs not passed correctly if helm repo contains path in repository URL #19138

Closed svghadi closed 3 weeks ago

svghadi commented 1 month ago

Describe the bug

I am try to deploy a chart from a private OCI Helm registry with self-signed certificates. I have added the tls certificate for my domain my-registry.default in argocd-tls-certs-cm configmap.

$ kubctl get cm argocd-tls-certs-cm -o yaml -n default
apiVersion: v1
kind: ConfigMap
metadata:
  name: argocd-tls-certs-cm
data:
  my-registry.default: |-
    -----BEGIN CERTIFICATE-----
    <cert data>
    -----END CERTIFICATE-----

When I create a Helm repository with my-registry.default repository URL everything works as expected. However, if the repository URL contains a path eg: my-registry.default/helm-charts, the tls certificate for the domain i.e my-registry.default is not picked up by Argo CD and results into x509: certificate signed by unknown authority errors.

To Reproduce

  1. Add OCI registry certs into Argo CD via UI or CLI
  2. Create a Helm OCI repository with path in repo url.
  3. Observe repo connection status and repo server logs

Expected behavior

Argo CD should connect successfully to the registry

Screenshots

image

Version

v2.10.11

Didn't test with master but I think should be reproducible with it.

Logs Repo server logs


# no path in url
time="2024-07-22T11:14:17Z" level=info msg=Trace args="[helm registry login my-registry --username ****** --password ****** --ca-file /app/config/tls/my-registry]" dir=/tmp/helm2649027701 operation_name="exec helm" time_ms=62.63742499999999
time="2024-07-22T11:14:17Z" level=info msg="took to test helm oci repository" seconds=0.062829967

# path in url
time="2024-07-22T11:15:34Z" level=info msg="helm registry login my-registry/helm-charts --username ****** --password ******" dir=/tmp/helm1150378190 execID=75776
time="2024-07-22T11:15:34Z" level=error msg="`helm registry login my-registry/helm-charts --username ****** --password ******` failed exit status 1: WARNING: Using --password ****** the CLI is insecure. Use --password-stdin.\ntime=\"2024-07-22T11:15:34Z\" level=info msg=\"Error logging in to endpoint, trying next endpoint\" error=\"Get \\\"https://my-registry/v2/\\\": tls: failed to verify certificate: x509: certificate signed by unknown authority\"\nError: Get \"https://my-registry/v2/\": tls: failed to verify certificate: x509: certificate signed by unknown authority" execID=75776```
svghadi commented 1 month ago

I did some digging, if the the repoURL contains oci scheme, the getCAPath func will return the correct CA cert even if the url contains path. https://github.com/argoproj/argo-cd/blob/3d77d9ced03c1e619df05c319a8257db5d98a47d/pkg/apis/application/v1alpha1/repository_types.go#L239-L240

However, when I try to set the oci scheme in the url, eg. oci://my-registry.default/helm-charts, the creation of the repo fails with OCI Helm repository URL should include hostname and port only from cli. From UI, the repo is created but oci:// is truncated.

PR #5888 introduced this change to disallow oci scheme in URL. @alexmt, I noticed that you implemented this change. Do you think this can be reverted as helm now supports oci:// prefix? https://helm.sh/docs/topics/registries/#other-subcommands

Support for the oci:// protocol is also available in various other subcommands. Here is a complete list:

An alternative simple solution to fix this bug could be to explicitly add oci:// scheme during fetching of the CA certs if the repo has EnableOCI set to true so that the url parsing correctly detects the hostname.

svghadi commented 3 weeks ago

Another case(https://github.com/argoproj/argo-cd/pull/8508#issuecomment-1630996731) of incorrect hostname parsing when repoURL contains port.