konstructio / kubefirst

The Kubefirst Open Source Platform
https://kubefirst.konstruct.io/docs
MIT License
1.78k stars 139 forks source link

AwaitHostNTimes needs to use https client #193

Closed converge closed 2 years ago

converge commented 2 years ago

AwaitHostNTimes (internal/gitlab) uses http.Get() that accepts HTTP or HTTPS when requesting HTTP. We should only accept valid HTTPS responses adding Go http default client: http.DefaultClient like we do on getArgoCDToken function.

6za commented 2 years ago

As acceptance criteria for this: to ensure we only set as valid HTTPS on the final step of install, once we have the real certificate not the self-signed one.

So, it should only pass the test, and return true, if the certificate match on HTTPS calls.

Used on the last check of installer and other places.

6za commented 2 years ago

For clarification:

This code is used here: https://github.com/kubefirst/kubefirst/blob/main/cmd/createGitlab.go#L312

        // Wait argocd cert to work, or force restart
        if !globalFlags.DryRun {
            argocdPodClient := clientset.CoreV1().Pods("argocd")
            for i := 1; i < 15; i++ {
                argoCDHostReady := gitlab.AwaitHostNTimes("argocd", globalFlags.DryRun, 20)
                if argoCDHostReady {
                    informUser("ArgoCD DNS is ready", globalFlags.SilentMode)
                    break
                } else {
                    k8s.DeletePodByLabel(argocdPodClient, "app.kubernetes.io/name=argocd-server")
                }
            }
        }

This loop is meant to solve a known issue that sometimes takes some reboots to load the right certificate.

It AwaitHostNTimes should only be a success(return true) if it has the letsencrypt certificate or any other trusted certificate. If it has a self-signed one the broken lock, it should return false. So, the loop work as desired.

Example: image

I believe to achieve that we discussed before to create a custom http client in go to proper control its flags, as it seems it is setting globally to accept insecureCerts..

6za commented 2 years ago

Most recent PR is not executing what is expected, and remove some sleeps. Waiting a new window to try to change this client.

converge commented 2 years ago

most recent PR commit was this one https://github.com/kubefirst/kubefirst/pull/357/commits/95107efe8c93019a600f5f63b8be2257e4df9b2a

was it tested with that?

6za commented 2 years ago

Installer seems to works as expected, this may not be needed anymore.