add image pullSecrets for cfssl and kubectl in charts

[dzcvxe]

What type of PR is this? /kind feature

What this PR does / why we need it: add imagePullSecret for cfssl and kubectl in case of private registry.

Which issue(s) this PR fixes: Fixes #4666

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[karmada-bot]

Welcome @dzcvxe! It looks like this is your first PR to karmada-io/karmada 🎉

[RainbowMango]

@dzcvxe Thank you for doing this!

cc @warjiang Could you please take a look? Is this what you want?

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 53.11%. Comparing base (fdad87e) to head (9e4e6bf).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #4785 +/- ## ========================================== - Coverage 53.11% 53.11% -0.01% ========================================== Files 250 250 Lines 20351 20351 ========================================== - Hits 10810 10809 -1 - Misses 8824 8825 +1 Partials 717 717 ``` | [Flag](https://app.codecov.io/gh/karmada-io/karmada/pull/4785/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=karmada-io) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/karmada-io/karmada/pull/4785/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=karmada-io) | `53.11% <ø> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=karmada-io#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[RainbowMango]

@dzcvxe The chart lint test is failing, you can get the logs here: https://github.com/karmada-io/karmada/actions/runs/8497892686/job/23288292073?pr=4785#step:9:45

Could you please take a look?

[warjiang]

@dzcvxe in fact, only add pullSecrets for cfssl and kubectl in values.yaml not really work

take cfssl in pre-init-job as an example

https://github.com/karmada-io/karmada/blob/c81649a423165bd660a3bf0ee3b1abd13b0f90e3/charts/karmada/templates/pre-install-job.yaml#L357C7-L360C31

with the input of

image:
  registry: {your-private-registry}
  repository: cfssl/cfssl
  tag: latest
  pullPolicy: IfNotPresent
  pullSecrets: 
  - {your-private-registry-secret-name}

it will output like following:

initContainers:
- name: init
  image: {your-private-registry}/cfssl/cfssl:latest
  imagePullPolicy: IfNotPresent
  workingDir: /opt/mount

[dzcvxe]

@RainbowMango Hi! I have made changes to this pull request. Could you please take a look?

[RainbowMango]

Sure, thanks @dzcvxe . Just triggered the test. But I'm not good at helm chart, @warjiang Can you help to take another look?

[warjiang]

@RainbowMango Hi! I have made changes to this pull request. Could you please take a look?

@dzcvxe combine all the image secrets is good which will decrease similar code obviously like following 👍

{{/*
Return the proper Docker Image Registry Secret Names
*/}}
{{- define "karmada.agent.imagePullSecrets" -}}
{{ include "common.images.pullSecrets" (dict "images" (list .Values.agent.image) "global" .Values.global) }}
{{- end -}}

But there is somthing wrong with common.images.pullSecrets, a helper method which introduced by common dependency in bitnami repo, if you combine all the image secrets:

For example, if the values.yaml is as following:

cfssl:
  image:
    # ...
    pullSecrets:
      - duplicate-secret-name
kubectl:
  image:
    # ...
    pullSecrets:
      - duplicate-secret-name

you'll finally get the output as following:

# Source: karmada/templates/post-delete-job.yaml
apiVersion: batch/v1
kind: Job
metadata:
  name: "karmada-post-delete"
  namespace: karmada-system
  labels:
    helm.sh/chart: "karmada-0.0.0"

  annotations:
    # This is what defines this resource as a hook. Without this line, the
    # job is considered part of the release.
    "helm.sh/hook": post-delete
    "helm.sh/hook-weight": "0"
    "helm.sh/hook-delete-policy": hook-succeeded
spec:
  parallelism: 1
  completions: 1
  template:
    metadata:
      name: karmada
      labels:
        helm.sh/chart: "karmada-0.0.0"
    spec:

      imagePullSecrets:
        - name: duplicate-secret-name
        - name: duplicate-secret-name
      serviceAccountName: karmada-pre-job
      restartPolicy: Never
      containers:
        - name: post-delete

the duplicate-secret-name is duplicated in imagePullSecrets field.

this is a bug of common dependency, maybe we need to upgrade common dependency cc @RainbowMango

For detailed information, you can follow this link 👉 : https://github.com/bitnami/charts/commit/ddfea70831875639cb298a555ad6dd5e68f059e4

[RainbowMango]

this is a bug of common dependency, maybe we need to upgrade common dependency cc @RainbowMango

Could you please remind me how to upgrade the common dependency? I'm curious why the version of common is 1.x.x?

[warjiang]

this is a bug of common dependency, maybe we need to upgrade common dependency cc @RainbowMango

Could you please remind me how to upgrade the common dependency? I'm curious why the version of common is 1.x.x?

@RainbowMango 💡 In short 1.x.x is a valid constraint in helm, which has the same effect as ^1.0.0. as for how to upgrade thecommon dependency, you can execute following instruction:

// the following two step is optionnal if you have not configure  bitnami repo on your local machine
helm repo add bitnami https://charts.bitnami.com/bitnami
helm repo update bitnami

// and then edit `charts/karmada/Chart.yaml`, update the version of `common` to `2.x.x` or `^2.0.0`
// after update dependency,  `charts/karmada/Chart.lock` will update as well
helm dependency update

the semver lib used in helm is github.com/Masterminds/semver/v3 https://github.com/helm/helm/blob/14d0c13e9eefff5b4a1b511cf50643529692ec94/go.mod#L8C1-L8C41

An simple example with github.com/Masterminds/semver/v3 lib:

package main

import (
    "fmt"
    "github.com/Masterminds/semver/v3"
)

func main() {
    /*
        // we can get all chart version by executing
        // `helm search repo bitnami/common -l | grep -v 'CHART VERSION' | awk '{print $2}'`
    */
    versions := []string{
        "2.19.1", "2.19.0",
        "2.18.0",
        "2.16.1", "2.16.0",
        "2.15.3", "2.15.2", "2.15.1", "2.15.0",
        "2.14.1",
        "2.13.4", "2.13.3", "2.13.2", "2.13.1", "2.13.0",
        "2.12.1", "2.12.0",
        "2.11.1", "2.11.0",
        "2.10.1", "2.10.0",
        "2.9.2", "2.9.1", "2.9.0",
        "2.8.0",
        "2.6.0",
        "2.5.0",
        "2.4.0",
        "2.3.0",
        "2.2.5", "2.2.4", "2.2.3", "2.2.2", "2.2.1", "2.2.0",
        "2.1.2", "2.1.1",
        "2.0.4", "2.0.3", "2.0.2", "2.0.1", "2.0.0",
        "1.17.1", "1.17.0",
        "1.16.1", "1.16.0",
        "1.15.2", "1.15.1",
        "1.14.2", "1.14.1", "1.14.0",
        "1.13.1", "1.13.0",
        "1.12.0",
        "1.11.3", "1.11.2", "1.11.1", "1.11.0",
        "1.10.4",
    }
    semvers := make([]*semver.Version, 0, len(versions))
    for _, version := range versions {
        v, err := semver.NewVersion(version)
        if err == nil {
            semvers = append(semvers, v)
        }
    }

    constraint, err := semver.NewConstraint("1.x.x")
    // validVersions is => [1.17.1 1.17.0 1.16.1 1.16.0 1.15.2 1.15.1 1.14.2 1.14.1 1.14.0 1.13.1 1.13.0 1.12.0 1.11.3 1.11.2 1.11.1 1.11.0 1.10.4]
    //constraint, err := semver.NewConstraint("^1.0.0")
    // validVersions is => [1.17.1 1.17.0 1.16.1 1.16.0 1.15.2 1.15.1 1.14.2 1.14.1 1.14.0 1.13.1 1.13.0 1.12.0 1.11.3 1.11.2 1.11.1 1.11.0 1.10.4]
    if err != nil {
        panic(err)
    }

    validVersions := make([]string, 0)
    for _, version := range semvers {
        if constraint.Check(version) {
            validVersions = append(validVersions, version.String())
        }
    }
    fmt.Println(validVersions)
}

[RainbowMango]

💡 In short 1.x.x is a valid constraint in helm, which has the same effect as ^1.0.0. as for how to upgrade thecommon dependency, you can execute following instruction:

Good to know, thanks.

@warjiang Would you like to send a PR for the common dependency?

[warjiang]

sorry late for the reply, i've already made a pr for upgrading bitnami/common dependency in karmada chart cc @dzcvxe @RainbowMango

https://github.com/karmada-io/karmada/pull/4829

[dzcvxe]

@RainbowMango @warjiang I have matched pullSercret in readme.md and values.yaml. Could you please take a look if it's correct？

[warjiang]

I've tested it locally, and the dry-run output remove duplicate secrect name as expected cc @RainbowMango

[warjiang]

/lgtm

[karmada-bot]

@warjiang: changing LGTM is restricted to collaborators

In response to [this](https://github.com/karmada-io/karmada/pull/4785#issuecomment-2060320067): >/lgtm Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[karmada-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[charts/OWNERS](https://github.com/karmada-io/karmada/blob/master/charts/OWNERS)~~ [RainbowMango] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment