Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to use in-cluster kube client for "test" command #288

Closed
janpieper opened this issue Nov 3, 2021 · 2 comments
Closed

Unable to use in-cluster kube client for "test" command #288

janpieper opened this issue Nov 3, 2021 · 2 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@janpieper
Copy link
Contributor

janpieper commented Nov 3, 2021

Describe the bug

Running argocd-image-updater test [...] with credentials in a kubernetes Secret fails when running inside a kubernetes cluster.

To Reproduce

  1. Add a custom registry and use a kubernetes Secret for credentials.
registries:
  - name: myfancyregistry
    ping: yes
    api_url: https://myfancyregistry.azurecr.io
    prefix: myfancyregistry.azurecr.io
    credentials: secret:myfancyns/myfancysecret#auth
  1. Exec into the kubernetes pod
kubectl exec -it -n argocd-image-updater argocd-image-updater-7ff85b79cd-lmcjs -- /bin/sh
  1. Run argocd-image-updater test [...]
$ argocd-image-updater --registries-conf /app/config/registries.conf test myfancyregistry.azurecr.io/example

Expected behavior

Use in-cluster kube client.

Additional context

I think I already tracked down the error. When running argocd-image-updater test [...] inside a kubernetes cluster, I get the following warning, because kubeClient is nil:

if (credSrc.Type == image.CredentialSourcePullSecret || credSrc.Type == image.CredentialSourceSecret) && kubeClient == nil {
log.WithContext().
AddField("registry", ep.RegistryAPI).
Warnf("cannot user K8s credentials without Kubernetes client")
return fmt.Errorf("could not fetch image tags")
}

The kubeClient is nil, because I do not specify --kubeconfig, so it skips creating a kube client:

if kubeConfig != "" {
kubeClient, err = getKubeConfig(ctx, "", kubeConfig)
if err != nil {
log.Fatalf("could not create K8s client: %v", err)
}
}

Checking for kubeConfig != "" is wrong because "" (empty string) tells getKubeConfig to return an in-cluster kube client (see line 661):

func getKubeConfig(ctx context.Context, namespace string, kubeConfig string) (*kube.KubernetesClient, error) {
var fullKubeConfigPath string
var kubeClient *kube.KubernetesClient
var err error
if kubeConfig != "" {
fullKubeConfigPath, err = filepath.Abs(kubeConfig)
if err != nil {
return nil, fmt.Errorf("cannot expand path %s: %v", kubeConfig, err)
}
}
if fullKubeConfigPath != "" {
log.Debugf("Creating Kubernetes client from %s", fullKubeConfigPath)
} else {
log.Debugf("Creating in-cluster Kubernetes client")
}

I had a look into what argocd-image-updater run [...] does in this case and found out it does not check for kubeConfig != "". Instead if checks for !disableKubernetes:

if !disableKubernetes {
ctx := context.Background()
cfg.KubeClient, err = getKubeConfig(ctx, cfg.ArgocdNamespace, kubeConfig)
if err != nil {
log.Fatalf("could not create K8s client: %v", err)
}
if cfg.ClientOpts.ServerAddr == "" {
cfg.ClientOpts.ServerAddr = fmt.Sprintf("argocd-server.%s", cfg.KubeClient.Namespace)
}
}

I think using !disableKubernetes would be the correct check for argocd-image-updater test [...] too.

Version

0.10.1 (via Helm chart)

Logs

INFO[0000] getting image                                 image_name=example registry=myfancyregistry.azurecr.io
DEBU[0000] rate limit for https://myfancyregistry.azurecr.io is 2147483647 
INFO[0000] Loaded 1 registry configurations from /app/config/registries.conf 
WARN[0000] cannot user K8s credentials without Kubernetes client  registry="https://myfancyregistry.azurecr.io"
FATA[0000] could not set registry credentials: could not fetch image tags
@janpieper janpieper added the bug Something isn't working label Nov 3, 2021
@jannfis
Copy link
Contributor

jannfis commented Nov 5, 2021

As opposed to the run command, the test command was intended to be run from your workstation, and not from within a cluster. So that's the reason why the test command expects a kube config for building the actual K8s client.

But I agree that there might be a valid use case for running test from within your cluster, and thus requiring an in-cluster K8s client.

So probably, we should mimic the run command in that behaviour:

  • Use Kubernetes client by default for the test command
  • If --kubeconfig is given, use that configuration for building the K8s client. Otherwise, build an in-cluser K8s client.
  • Similar to run command, provide a --disable-kubernetes option for the test command that disables the creation and usage of a K8s client

@jannfis jannfis added the good first issue Good for newcomers label Nov 5, 2021
janpieper added a commit to janpieper/argocd-image-updater that referenced this issue Nov 5, 2021
…abs#288)

* Use in-cluster kube client unless k8s is disabled

* Add `--argocd-namespace` option to test-command

* Assign `ctx` only if needed

Signed-off-by: Jan Pieper <kontakt@jan-pieper.info>
janpieper added a commit to janpieper/argocd-image-updater that referenced this issue Nov 5, 2021
…abs#288)

* Use in-cluster kube client unless k8s is disabled

* Assign `ctx` only if needed

Signed-off-by: Jan Pieper <kontakt@jan-pieper.info>
jannfis pushed a commit that referenced this issue Nov 6, 2021
* Use in-cluster kube client unless k8s is disabled

* Assign `ctx` only if needed

Signed-off-by: Jan Pieper <kontakt@jan-pieper.info>

Co-authored-by: Jan Pieper <kontakt@jan-pieper.info>
@jannfis
Copy link
Contributor

jannfis commented Nov 6, 2021

Closed by #293

@jannfis jannfis closed this as completed Nov 6, 2021
tuananh pushed a commit to tuananh/argocd-image-updater that referenced this issue Nov 11, 2021
…abs#288) (argoproj-labs#293)

* Use in-cluster kube client unless k8s is disabled

* Assign `ctx` only if needed

Signed-off-by: Jan Pieper <kontakt@jan-pieper.info>

Co-authored-by: Jan Pieper <kontakt@jan-pieper.info>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants