diff --git a/.golangci.yaml b/.golangci.yaml index 657f5897..a73e634e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -4,12 +4,12 @@ linters: - asciicheck - bodyclose - containedctx - - deadcode - - depguard +# - deadcode +# - depguard - dogsled - errcheck - exportloopref - - gci +# - gci - goconst - gocritic - gofmt @@ -20,24 +20,24 @@ linters: - govet - importas - ineffassign - - misspell +# - misspell - nakedret - nilerr - noctx - nolintlint - prealloc - predeclared - - revive +# - revive - rowserrcheck - staticcheck - - structcheck +# - structcheck - stylecheck - thelper - typecheck - unconvert - - unparam +# - unparam - unused - - varcheck +# - varcheck - whitespace linters-settings: @@ -49,11 +49,6 @@ linters-settings: exclude: - '^ \+.*' - '^ ANCHOR.*' - ifshort: - # Maximum length of variable declaration measured in number of characters, after which linter won't suggest using short syntax. - max-decl-chars: 50 - gci: - local-prefixes: "github.com/kubesphere/kubekey" importas: no-unaliased: true alias: @@ -71,10 +66,12 @@ linters-settings: alias: metav1 - pkg: "k8s.io/apimachinery/pkg/api/errors" alias: apierrors + - pkg: "k8s.io/apimachinery/pkg/util/runtime" + alias: utilruntime - pkg: "k8s.io/client-go/tools/cache" - alias: cgcache + alias: cgtoolscache - pkg: "k8s.io/client-go/util/cert" - alias: certutil + alias: cgutilcert - pkg: "k8s.io/component-base/cli/flag" alias: cliflag - pkg: "k8s.io/utils/exec/testing" @@ -101,7 +98,6 @@ linters-settings: alias: kkcorev1 nolintlint: allow-unused: false - allow-leading-space: false require-specific: true revive: rules: @@ -137,6 +133,10 @@ linters-settings: - name: constant-logical-expr gosec: excludes: + - G106 # Deferring unsafe method "InsecureIgnoreHostKey" on type "\*ssh" + - G301 # Deferring unsafe method "MkdirAll" on type "\*os.File" + - G304 # Deferring unsafe method "Create" or "Open" on type "\*os.File" + - G306 # Deferring unsafe method "WriteFile" on type "\*os.File" - G307 # Deferring unsafe method "Close" on type "\*os.File" - G108 # Profiling endpoint is automatically exposed on /debug/pprof gocritic: @@ -160,6 +160,9 @@ linters-settings: - filepathJoin - rangeValCopy - hugeParam + stylecheck: + checks: + - -ST1000 # ignore package comment issues: max-same-issues: 0 max-issues-per-linter: 0 @@ -245,15 +248,7 @@ issues: run: timeout: 10m - go: "1.19" + go: "1.22" build-tags: - - tools - - e2e - - containers_image_openpgp - - exclude_graphdriver_devicemapper - - exclude_graphdriver_btrfs - skip-files: - - "zz_generated.*\\.go$" - - "vendored_openapi\\.go$" - - "cmd" + - builtin allow-parallel-runners: true diff --git a/cmd/controller-manager/app/options/common.go b/cmd/controller-manager/app/options/common.go index 7ad49c5c..2f14c67c 100644 --- a/cmd/controller-manager/app/options/common.go +++ b/cmd/controller-manager/app/options/common.go @@ -80,8 +80,12 @@ func InitProfiling() error { signal.Notify(c, os.Interrupt) go func() { <-c - f.Close() - FlushProfiling() + if err := f.Close(); err != nil { + fmt.Printf("failed to close file. file: %v. error: %v \n", profileOutput, err) + } + if err := FlushProfiling(); err != nil { + fmt.Printf("failed to FlushProfiling. file: %v. error: %v \n", profileOutput, err) + } os.Exit(0) }() @@ -107,7 +111,9 @@ func FlushProfiling() error { return err } defer f.Close() - profile.WriteTo(f, 0) + if err := profile.WriteTo(f, 0); err != nil { + return err + } } return nil diff --git a/cmd/kk/app/options/common.go b/cmd/kk/app/options/common.go index 3b3bf471..2f14c67c 100644 --- a/cmd/kk/app/options/common.go +++ b/cmd/kk/app/options/common.go @@ -80,8 +80,12 @@ func InitProfiling() error { signal.Notify(c, os.Interrupt) go func() { <-c - f.Close() - FlushProfiling() + if err := f.Close(); err != nil { + fmt.Printf("failed to close file. file: %v. error: %v \n", profileOutput, err) + } + if err := FlushProfiling(); err != nil { + fmt.Printf("failed to FlushProfiling. file: %v. error: %v \n", profileOutput, err) + } os.Exit(0) }() @@ -107,7 +111,9 @@ func FlushProfiling() error { return err } defer f.Close() - profile.WriteTo(f, 0) + if err := profile.WriteTo(f, 0); err != nil { + return err + } } return nil @@ -121,7 +127,7 @@ var gops bool func AddGOPSFlags(flags *pflag.FlagSet) { flags.BoolVar(&gops, "gops", false, "Whether to enable gops or not. When enabled this option, "+ - "kk will listen on a random port on 127.0.0.1, then you can use the gops tool to list and diagnose the kk currently running.") + "controller-manager will listen on a random port on 127.0.0.1, then you can use the gops tool to list and diagnose the controller-manager currently running.") } func InitGOPS() error { diff --git a/cmd/kk/app/options/option.go b/cmd/kk/app/options/option.go index b73943d2..ca46bd60 100644 --- a/cmd/kk/app/options/option.go +++ b/cmd/kk/app/options/option.go @@ -203,9 +203,9 @@ func setValue(config *kubekeyv1.Config, key, val string) error { return err } return config.SetValue(key, value) - case strings.ToUpper(val) == "TRUE" || strings.ToUpper(val) == "YES" || strings.ToUpper(val) == "Y": + case strings.EqualFold(val, "TRUE") || strings.EqualFold(val, "YES") || strings.EqualFold(val, "Y"): return config.SetValue(key, true) - case strings.ToUpper(val) == "FALSE" || strings.ToUpper(val) == "NO" || strings.ToUpper(val) == "N": + case strings.EqualFold(val, "FALSE") || strings.EqualFold(val, "NO") || strings.EqualFold(val, "N"): return config.SetValue(key, false) default: return config.SetValue(key, val) diff --git a/cmd/kk/app/pipeline.go b/cmd/kk/app/pipeline.go index 6825d873..886f87b0 100644 --- a/cmd/kk/app/pipeline.go +++ b/cmd/kk/app/pipeline.go @@ -37,6 +37,9 @@ func newPipelineCommand() *cobra.Command { client, err := ctrlclient.New(restconfig, ctrlclient.Options{ Scheme: _const.Scheme, }) + if err != nil { + return fmt.Errorf("could not create client: %w", err) + } ctx := signals.SetupSignalHandler() var pipeline = new(kubekeyv1.Pipeline) var config = new(kubekeyv1.Config) diff --git a/hack/update-goimports.sh b/hack/update-goimports.sh index 3e3df807..ff621100 100755 --- a/hack/update-goimports.sh +++ b/hack/update-goimports.sh @@ -34,9 +34,7 @@ export GO111MODULE=on if ! command -v goimports ; then # Install goimports echo 'installing goimports' - pushd "${KUBE_ROOT}/hack/tools" >/dev/null - GO111MODULE=auto go install -mod=mod golang.org/x/tools/cmd/goimports@v0.7.0 - popd >/dev/null + GO111MODULE=auto go install -mod=mod golang.org/x/tools/cmd/goimports@v0.7.0 fi cd "${KUBE_ROOT}" || exit 1 diff --git a/hack/verify-goimports.sh b/hack/verify-goimports.sh index 87075dd7..60c74510 100755 --- a/hack/verify-goimports.sh +++ b/hack/verify-goimports.sh @@ -34,16 +34,14 @@ export GO111MODULE=on if ! command -v goimports ; then # Install goimports echo 'installing goimports' - pushd "${KUBE_ROOT}/hack/tools" >/dev/null - GO111MODULE=auto go install -mod=mod golang.org/x/tools/cmd/goimports@v0.7.0 - popd >/dev/null + GO111MODULE=auto go install -mod=mod golang.org/x/tools/cmd/goimports@v0.7.0 fi cd "${KUBE_ROOT}" || exit 1 IFS=$'\n' read -r -d '' -a files < <( find . -type f -name '*.go' -not -path "./vendor/*" -not -path "./pkg/apis/*" -not -path "./pkg/client/*" -not -name "zz_generated.deepcopy.go" && printf '\0' ) -output=$(goimports -local github.com/kubesphere/kubekey -l "${files[@]}") +output=$(goimports -local kubesphere.io/kubekey -l "${files[@]}") if [ "${output}" != "" ]; then echo "The following files are not import formatted" diff --git a/pkg/apis/core/v1/play.go b/pkg/apis/core/v1/play.go index f1f3e4da..396f02d2 100644 --- a/pkg/apis/core/v1/play.go +++ b/pkg/apis/core/v1/play.go @@ -72,7 +72,6 @@ func (s *PlaySerial) UnmarshalYAML(unmarshal func(interface{}) error) error { return nil } return fmt.Errorf("unsupported type, excepted any or array") - } type PlayHost struct { @@ -91,5 +90,4 @@ func (p *PlayHost) UnmarshalYAML(unmarshal func(interface{}) error) error { return nil } return fmt.Errorf("unsupported type, excepted string or string array") - } diff --git a/pkg/apis/core/v1/playbook.go b/pkg/apis/core/v1/playbook.go index 49019e90..af41848e 100644 --- a/pkg/apis/core/v1/playbook.go +++ b/pkg/apis/core/v1/playbook.go @@ -23,8 +23,8 @@ type Playbook struct { } func (p *Playbook) Validate() error { - var newPlay []Play - for _, play := range p.Play { + var newPlay = make([]Play, len(p.Play)) + for i, play := range p.Play { // delete import_playbook import_playbook is a link, should be ignored. if play.ImportPlaybook != "" { continue @@ -32,7 +32,7 @@ func (p *Playbook) Validate() error { if len(play.PlayHost.Hosts) == 0 { return fmt.Errorf("playbook's hosts must not be empty") } - newPlay = append(newPlay, play) + newPlay[i] = play } p.Play = newPlay return nil diff --git a/pkg/apis/kubekey/v1alpha1/task_types.go b/pkg/apis/kubekey/v1alpha1/task_types.go index ecd2edfc..b6b9b71c 100644 --- a/pkg/apis/kubekey/v1alpha1/task_types.go +++ b/pkg/apis/kubekey/v1alpha1/task_types.go @@ -56,10 +56,10 @@ type Module struct { } type TaskStatus struct { - RestartCount int `json:"restartCount,omitempty"` - Phase TaskPhase `json:"phase,omitempty"` - Conditions []TaskCondition `json:"conditions,omitempty"` - FailedDetail []TaskFailedDetail `json:"failedDetail,omitempty"` + RestartCount int `json:"restartCount,omitempty"` + Phase TaskPhase `json:"phase,omitempty"` + Conditions []TaskCondition `json:"conditions,omitempty"` + HostResults []TaskHostResult `json:"failedDetail,omitempty"` } type TaskCondition struct { @@ -69,12 +69,6 @@ type TaskCondition struct { HostResults []TaskHostResult `json:"hostResults,omitempty"` } -type TaskFailedDetail struct { - Host string `json:"host,omitempty"` - Stdout string `json:"stdout,omitempty"` - StdErr string `json:"stdErr,omitempty"` -} - type TaskHostResult struct { Host string `json:"host,omitempty"` Stdout string `json:"stdout,omitempty"` diff --git a/pkg/apis/kubekey/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/kubekey/v1alpha1/zz_generated.deepcopy.go index 32e717b3..cd386bca 100644 --- a/pkg/apis/kubekey/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/kubekey/v1alpha1/zz_generated.deepcopy.go @@ -32,6 +32,11 @@ func (in *KubeKeyTaskSpec) DeepCopyInto(out *KubeKeyTaskSpec) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.IgnoreError != nil { + in, out := &in.IgnoreError, &out.IgnoreError + *out = new(bool) + **out = **in + } if in.When != nil { in, out := &in.When, &out.When *out = make([]string, len(*in)) @@ -121,21 +126,6 @@ func (in *TaskCondition) DeepCopy() *TaskCondition { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *TaskFailedDetail) DeepCopyInto(out *TaskFailedDetail) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TaskFailedDetail. -func (in *TaskFailedDetail) DeepCopy() *TaskFailedDetail { - if in == nil { - return nil - } - out := new(TaskFailedDetail) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskHostResult) DeepCopyInto(out *TaskHostResult) { *out = *in @@ -193,9 +183,9 @@ func (in *TaskStatus) DeepCopyInto(out *TaskStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.FailedDetail != nil { - in, out := &in.FailedDetail, &out.FailedDetail - *out = make([]TaskFailedDetail, len(*in)) + if in.HostResults != nil { + in, out := &in.HostResults, &out.HostResults + *out = make([]TaskHostResult, len(*in)) copy(*out, *in) } } diff --git a/pkg/connector/helper.go b/pkg/connector/helper.go index b33e9121..848a7da4 100644 --- a/pkg/connector/helper.go +++ b/pkg/connector/helper.go @@ -49,19 +49,17 @@ func convertBytesToSlice(bs []byte, split string) []map[string]string { line := scanner.Text() line = strings.TrimSpace(line) - if len(line) > 0 { + if line != "" { parts := strings.SplitN(line, split, 2) if len(parts) == 2 { key := strings.TrimSpace(parts[0]) value := strings.TrimSpace(parts[1]) currentMap[key] = value } - } else { + } else if len(currentMap) > 0 { // If encountering an empty line, add the current map to config and create a new map - if len(currentMap) > 0 { - config = append(config, currentMap) - currentMap = make(map[string]string) - } + config = append(config, currentMap) + currentMap = make(map[string]string) } } diff --git a/pkg/const/scheme.go b/pkg/const/scheme.go index 672ccb93..986a0460 100644 --- a/pkg/const/scheme.go +++ b/pkg/const/scheme.go @@ -22,6 +22,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" kubekeyv1 "github.com/kubesphere/kubekey/v4/pkg/apis/kubekey/v1" kubekeyv1alpha1 "github.com/kubesphere/kubekey/v4/pkg/apis/kubekey/v1alpha1" @@ -44,11 +45,11 @@ var ( func newScheme() *runtime.Scheme { s := runtime.NewScheme() - batchv1.AddToScheme(s) - corev1.AddToScheme(s) - rbacv1.AddToScheme(s) - kubekeyv1.AddToScheme(s) - kubekeyv1alpha1.AddToScheme(s) - kubekeyv1alpha1.AddConversionFuncs(s) + utilruntime.Must(batchv1.AddToScheme(s)) + utilruntime.Must(corev1.AddToScheme(s)) + utilruntime.Must(rbacv1.AddToScheme(s)) + utilruntime.Must(kubekeyv1.AddToScheme(s)) + utilruntime.Must(kubekeyv1alpha1.AddToScheme(s)) + utilruntime.Must(kubekeyv1alpha1.AddConversionFuncs(s)) return s } diff --git a/pkg/controllers/pipeline_controller.go b/pkg/controllers/pipeline_controller.go index ae87af5c..b5425d30 100644 --- a/pkg/controllers/pipeline_controller.go +++ b/pkg/controllers/pipeline_controller.go @@ -139,7 +139,6 @@ func (r *PipelineReconciler) dealRunningPipeline(ctx context.Context, pipeline * jobLabel: pipeline.Name, }); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err - } else if len(jobs.Items) != 0 { // could find exist cronJob for _, job := range jobs.Items { diff --git a/pkg/converter/converter.go b/pkg/converter/converter.go index 1646378d..8c467762 100644 --- a/pkg/converter/converter.go +++ b/pkg/converter/converter.go @@ -77,18 +77,18 @@ func GroupHostBySerial(hosts []string, serial []any) ([][]string, error) { // the count for sis var count int for i, a := range serial { - switch a.(type) { + switch val := a.(type) { case int: - sis[i] = a.(int) + sis[i] = val case string: - if strings.HasSuffix(a.(string), "%") { - b, err := strconv.ParseFloat(a.(string)[:len(a.(string))-1], 64) + if strings.HasSuffix(val, "%") { + b, err := strconv.ParseFloat(val[:len(val)-1], 64) if err != nil { return nil, fmt.Errorf("convert serial %v to float error: %w", a, err) } sis[i] = int(math.Ceil(float64(len(hosts)) * b / 100.0)) } else { - b, err := strconv.Atoi(a.(string)) + b, err := strconv.Atoi(val) if err != nil { return nil, fmt.Errorf("convert serial %v to int error: %w", a, err) } diff --git a/pkg/converter/tmpl/filter_extension.go b/pkg/converter/tmpl/filter_extension.go index 06819df5..5252e2a4 100644 --- a/pkg/converter/tmpl/filter_extension.go +++ b/pkg/converter/tmpl/filter_extension.go @@ -25,18 +25,19 @@ import ( "github.com/flosch/pongo2/v6" "gopkg.in/yaml.v3" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/version" ) func init() { - pongo2.RegisterFilter("defined", filterDefined) - pongo2.RegisterFilter("version", filterVersion) - pongo2.RegisterFilter("pow", filterPow) - pongo2.RegisterFilter("match", filterMatch) - pongo2.RegisterFilter("to_json", filterToJson) - pongo2.RegisterFilter("to_yaml", filterToYaml) - pongo2.RegisterFilter("ip_range", filterIpRange) - pongo2.RegisterFilter("get", filterGet) + utilruntime.Must(pongo2.RegisterFilter("defined", filterDefined)) + utilruntime.Must(pongo2.RegisterFilter("version", filterVersion)) + utilruntime.Must(pongo2.RegisterFilter("pow", filterPow)) + utilruntime.Must(pongo2.RegisterFilter("match", filterMatch)) + utilruntime.Must(pongo2.RegisterFilter("to_json", filterToJson)) + utilruntime.Must(pongo2.RegisterFilter("to_yaml", filterToYaml)) + utilruntime.Must(pongo2.RegisterFilter("ip_range", filterIpRange)) + utilruntime.Must(pongo2.RegisterFilter("get", filterGet)) } func filterDefined(in *pongo2.Value, param *pongo2.Value) (*pongo2.Value, *pongo2.Error) { diff --git a/pkg/converter/tmpl/helper.go b/pkg/converter/tmpl/helper.go index a1e9f43c..4cf9790e 100644 --- a/pkg/converter/tmpl/helper.go +++ b/pkg/converter/tmpl/helper.go @@ -25,14 +25,14 @@ func ParseIp(ip string) []string { var availableIPs []string // if ip is "1.1.1.1/",trim / ip = strings.TrimRight(ip, "/") - if strings.Contains(ip, "/") == true { - if strings.Contains(ip, "/32") == true { + if strings.Contains(ip, "/") { + if strings.Contains(ip, "/32") { aip := strings.Replace(ip, "/32", "", -1) availableIPs = append(availableIPs, aip) } else { availableIPs = GetAvailableIP(ip) } - } else if strings.Contains(ip, "-") == true { + } else if strings.Contains(ip, "-") { ipRange := strings.SplitN(ip, "-", 2) availableIPs = GetAvailableIPRange(ipRange[0], ipRange[1]) } else { @@ -57,7 +57,7 @@ func GetAvailableIPRange(ipStart, ipEnd string) []string { for newNum <= EndIPNum { availableIPs = append(availableIPs, intToIP(newNum).String()) - newNum = newNum + pos + newNum += pos } return availableIPs } @@ -95,11 +95,11 @@ func intToIP(n int32) net.IP { } func IPAddressToCIDR(ipAddress string) string { - if strings.Contains(ipAddress, "/") == true { + if strings.Contains(ipAddress, "/") { ipAndMask := strings.Split(ipAddress, "/") ip := ipAndMask[0] mask := ipAndMask[1] - if strings.Contains(mask, ".") == true { + if strings.Contains(mask, ".") { mask = IPMaskStringToCIDR(mask) } return ip + "/" + mask @@ -110,10 +110,9 @@ func IPAddressToCIDR(ipAddress string) string { func IPMaskStringToCIDR(netmask string) string { netmaskList := strings.Split(netmask, ".") - var mint []int - for _, v := range netmaskList { - strv, _ := strconv.Atoi(v) - mint = append(mint, strv) + var mint = make([]int, len(netmaskList)) + for i, v := range netmaskList { + mint[i], _ = strconv.Atoi(v) } myIPMask := net.IPv4Mask(byte(mint[0]), byte(mint[1]), byte(mint[2]), byte(mint[3])) ones, _ := myIPMask.Size() diff --git a/pkg/executor/executor.go b/pkg/executor/executor.go index 0fa1aebf..4b01b8e4 100644 --- a/pkg/executor/executor.go +++ b/pkg/executor/executor.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "os" - "path" "github.com/schollz/progressbar/v3" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -87,10 +86,6 @@ func (e executor) Exec(ctx context.Context) error { } // convert to transfer.Playbook struct - playbookPath := e.pipeline.Spec.Playbook - if path.IsAbs(playbookPath) { - playbookPath = playbookPath[1:] - } pb, err := pj.MarshalPlaybook() if err != nil { return fmt.Errorf("convert playbook error: %w", err) @@ -360,7 +355,7 @@ func (e executor) execBlock(ctx context.Context, options execBlockOptions) error // exit when task run failed if task.IsFailed() { var hostReason []kubekeyv1.PipelineFailedDetailHost - for _, tr := range task.Status.FailedDetail { + for _, tr := range task.Status.HostResults { hostReason = append(hostReason, kubekeyv1.PipelineFailedDetailHost{ Host: tr.Host, Stdout: tr.Stdout, @@ -401,7 +396,9 @@ func (e executor) executeTask(ctx context.Context, task *kubekeyv1alpha1.Task, o progressbar.OptionEnableColorCodes(true), progressbar.OptionSetDescription(fmt.Sprintf("[%s] running...", h)), progressbar.OptionOnCompletion(func() { - os.Stdout.Write([]byte("\n")) + if _, err := os.Stdout.WriteString("\n"); err != nil { + klog.ErrorS(err, "failed to write output", "host", h) + } }), progressbar.OptionShowElapsedTimeOnFinish(), progressbar.OptionSetPredictTime(false), @@ -411,13 +408,9 @@ func (e executor) executeTask(ctx context.Context, task *kubekeyv1alpha1.Task, o var stdoutResult any = stdout var stderrResult any = stderr // try to convert by json - if err := json.Unmarshal([]byte(stdout), &stdoutResult); err != nil { - // dothing - } + _ = json.Unmarshal([]byte(stdout), &stdoutResult) // try to convert by json - if err := json.Unmarshal([]byte(stderr), &stderrResult); err != nil { - // dothing - } + _ = json.Unmarshal([]byte(stderr), &stderrResult) // set variable to parent location if err := e.variable.Merge(variable.MergeRuntimeVariable(host, map[string]any{ task.Spec.Register: map[string]any{ @@ -433,14 +426,20 @@ func (e executor) executeTask(ctx context.Context, task *kubekeyv1alpha1.Task, o switch { case stderr != "": // failed bar.Describe(fmt.Sprintf("[%s] failed", h)) - bar.Finish() + if err := bar.Finish(); err != nil { + klog.ErrorS(err, "fail to finish bar") + } klog.Errorf("[Task %s] run failed: %s", ctrlclient.ObjectKeyFromObject(task), stderr) case stdout == "skip": // skip bar.Describe(fmt.Sprintf("[%s] skip", h)) - bar.Finish() + if err := bar.Finish(); err != nil { + klog.ErrorS(err, "fail to finish bar") + } default: //success bar.Describe(fmt.Sprintf("[%s] success", h)) - bar.Finish() + if err := bar.Finish(); err != nil { + klog.ErrorS(err, "fail to finish bar") + } } // fill result @@ -485,7 +484,9 @@ func (e executor) executeTask(ctx context.Context, task *kubekeyv1alpha1.Task, o stderr = fmt.Sprintf("set loop item to variable error: %v", err) return } - bar.Add(1) + if err := bar.Add(1); err != nil { + klog.ErrorS(err, "fail to add bar") + } stdout, stderr = e.executeModule(ctx, task, modules.ExecOptions{ Args: task.Spec.Module.Args, Host: host, @@ -493,7 +494,9 @@ func (e executor) executeTask(ctx context.Context, task *kubekeyv1alpha1.Task, o Task: *task, Pipeline: *e.pipeline, }) - bar.Add(1) + if err := bar.Add(1); err != nil { + klog.ErrorS(err, "fail to add bar") + } // delete item if err := e.variable.Merge(variable.MergeRuntimeVariable(host, map[string]any{ "item": nil, @@ -501,7 +504,9 @@ func (e executor) executeTask(ctx context.Context, task *kubekeyv1alpha1.Task, o stderr = fmt.Sprintf("clean loop item to variable error: %v", err) return } - bar.Add(1) + if err := bar.Add(1); err != nil { + klog.ErrorS(err, "fail to add bar") + } } }) } @@ -517,7 +522,7 @@ func (e executor) executeTask(ctx context.Context, task *kubekeyv1alpha1.Task, o task.Status.Phase = kubekeyv1alpha1.TaskPhaseIgnored } else { task.Status.Phase = kubekeyv1alpha1.TaskPhaseFailed - task.Status.FailedDetail = append(task.Status.FailedDetail, kubekeyv1alpha1.TaskFailedDetail{ + task.Status.HostResults = append(task.Status.HostResults, kubekeyv1alpha1.TaskHostResult{ Host: data.Host, Stdout: data.Stdout, StdErr: data.StdErr, @@ -569,7 +574,6 @@ func (e executor) mergeVariable(ctx context.Context, v variable.Variable, vd map return nil } for _, host := range hosts { - if err := v.Merge(variable.MergeRuntimeVariable(host, vd)); err != nil { return err } diff --git a/pkg/manager/command_manager.go b/pkg/manager/command_manager.go index fa4404e2..53f4af13 100644 --- a/pkg/manager/command_manager.go +++ b/pkg/manager/command_manager.go @@ -57,7 +57,6 @@ func (m *commandManager) Run(ctx context.Context) error { if err := m.Client.Status().Patch(ctx, m.Pipeline, ctrlclient.MergeFrom(cp)); err != nil { klog.ErrorS(err, "update pipeline error", "pipeline", ctrlclient.ObjectKeyFromObject(m.Pipeline)) } - }() klog.Infof("[Pipeline %s] start task controller", ctrlclient.ObjectKeyFromObject(m.Pipeline)) diff --git a/pkg/modules/assert.go b/pkg/modules/assert.go index 56079a4b..36194cc6 100644 --- a/pkg/modules/assert.go +++ b/pkg/modules/assert.go @@ -18,8 +18,7 @@ package modules import ( "context" - - "k8s.io/klog/v2" + "fmt" "github.com/kubesphere/kubekey/v4/pkg/converter/tmpl" "github.com/kubesphere/kubekey/v4/pkg/variable" @@ -29,8 +28,7 @@ func ModuleAssert(ctx context.Context, options ExecOptions) (string, string) { // get host variable ha, err := options.Variable.Get(variable.GetAllVariable(options.Host)) if err != nil { - klog.V(4).ErrorS(err, "failed to get host variable", "hostname", options.Host) - return "", err.Error() + return "", fmt.Sprintf("failed to get host variable: %v", err) } args := variable.Extension2Variables(options.Args) @@ -41,33 +39,33 @@ func ModuleAssert(ctx context.Context, options ExecOptions) (string, string) { ok, err := tmpl.ParseBool(ha.(map[string]any), thatParam) if err != nil { - return "", err.Error() + return "", fmt.Sprintf("parse \"that\" error: %v", err) } if ok { if successMsgParam, err := variable.StringVar(ha.(map[string]any), args, "success_msg"); err == nil { if r, err := tmpl.ParseString(ha.(map[string]any), successMsgParam); err != nil { - return "", err.Error() + return "", fmt.Sprintf("parse \"success_msg\" error: %v", err) } else { return r, "" } } - return "True", "" + return stdoutTrue, "" } else { if failMsgParam, err := variable.StringVar(ha.(map[string]any), args, "fail_msg"); err == nil { if r, err := tmpl.ParseString(ha.(map[string]any), failMsgParam); err != nil { - return "", err.Error() + return "", fmt.Sprintf("parse \"fail_msg\" error: %v", err) } else { - return "False", r + return stdoutFalse, r } } if msgParam, err := variable.StringVar(ha.(map[string]any), args, "msg"); err == nil { if r, err := tmpl.ParseString(ha.(map[string]any), msgParam); err != nil { - return "", err.Error() + return "", fmt.Sprintf("parse \"msg\" error: %v", err) } else { - return "False", r + return stdoutFalse, r } } - return "False", "False" + return stdoutFalse, "False" } } diff --git a/pkg/modules/assert_test.go b/pkg/modules/assert_test.go index fe946113..109b3ca5 100644 --- a/pkg/modules/assert_test.go +++ b/pkg/modules/assert_test.go @@ -54,7 +54,7 @@ func TestAssert(t *testing.T) { }, }, }, - exceptStdout: "True", + exceptStdout: stdoutTrue, }, { name: "success with success_msg", @@ -86,7 +86,7 @@ func TestAssert(t *testing.T) { }, }, }, - exceptStdout: "False", + exceptStdout: stdoutFalse, exceptStderr: "False", }, { @@ -103,7 +103,7 @@ func TestAssert(t *testing.T) { }, }, }, - exceptStdout: "False", + exceptStdout: stdoutFalse, exceptStderr: "failed v2", }, } diff --git a/pkg/modules/command.go b/pkg/modules/command.go index 024d870f..8af610a1 100644 --- a/pkg/modules/command.go +++ b/pkg/modules/command.go @@ -18,10 +18,9 @@ package modules import ( "context" + "fmt" "strings" - "k8s.io/klog/v2" - "github.com/kubesphere/kubekey/v4/pkg/variable" ) @@ -29,9 +28,9 @@ func ModuleCommand(ctx context.Context, options ExecOptions) (string, string) { // get host variable ha, err := options.Variable.Get(variable.GetAllVariable(options.Host)) if err != nil { - klog.V(4).ErrorS(err, "failed to get host variable", "hostname", options.Host) - return "", err.Error() + return "", fmt.Sprintf("failed to get host variable: %v", err) } + // get connector conn, err := getConnector(ctx, options.Host, ha.(map[string]any)) if err != nil { diff --git a/pkg/modules/command_test.go b/pkg/modules/command_test.go index 36cb0fee..9499be19 100644 --- a/pkg/modules/command_test.go +++ b/pkg/modules/command_test.go @@ -18,7 +18,6 @@ package modules import ( "context" - "fmt" "testing" "time" @@ -30,7 +29,7 @@ func TestCommand(t *testing.T) { testcases := []struct { name string opt ExecOptions - ctx context.Context + ctxFunc func() context.Context exceptStdout string exceptStderr string }{ @@ -39,14 +38,14 @@ func TestCommand(t *testing.T) { opt: ExecOptions{ Variable: &testVariable{}, }, - ctx: context.Background(), + ctxFunc: context.Background, exceptStderr: "cannot find variable \"ssh_host\"", }, { name: "exec command success", - ctx: context.WithValue(context.Background(), ConnKey, &testConnector{ - output: []byte("success"), - }), + ctxFunc: func() context.Context { + return context.WithValue(context.Background(), ConnKey, successConnector) + }, opt: ExecOptions{ Host: "test", Args: runtime.RawExtension{Raw: []byte("echo success")}, @@ -55,10 +54,8 @@ func TestCommand(t *testing.T) { exceptStdout: "success", }, { - name: "exec command failed", - ctx: context.WithValue(context.Background(), ConnKey, &testConnector{ - commandErr: fmt.Errorf("failed"), - }), + name: "exec command failed", + ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, failedConnector) }, opt: ExecOptions{ Host: "test", Args: runtime.RawExtension{Raw: []byte("echo success")}, @@ -70,7 +67,7 @@ func TestCommand(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx, cancel := context.WithTimeout(tc.ctx, time.Second*5) + ctx, cancel := context.WithTimeout(tc.ctxFunc(), time.Second*5) defer cancel() acStdout, acStderr := ModuleCommand(ctx, tc.opt) assert.Equal(t, tc.exceptStdout, acStdout) diff --git a/pkg/modules/copy.go b/pkg/modules/copy.go index 39f658cc..604b437e 100644 --- a/pkg/modules/copy.go +++ b/pkg/modules/copy.go @@ -24,8 +24,6 @@ import ( "path/filepath" "strings" - "k8s.io/klog/v2" - kubekeyv1alpha1 "github.com/kubesphere/kubekey/v4/pkg/apis/kubekey/v1alpha1" "github.com/kubesphere/kubekey/v4/pkg/project" "github.com/kubesphere/kubekey/v4/pkg/variable" @@ -35,8 +33,7 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { // get host variable ha, err := options.Variable.Get(variable.GetAllVariable(options.Host)) if err != nil { - klog.V(4).ErrorS(err, "failed to get host variable", "hostname", options.Host) - return "", err.Error() + return "", fmt.Sprintf("failed to get host variable: %v", err) } // check args @@ -44,9 +41,6 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { args := variable.Extension2Variables(options.Args) srcParam, _ := variable.StringVar(ha.(map[string]any), args, "src") contentParam, _ := variable.StringVar(ha.(map[string]any), args, "content") - if srcParam == "" && contentParam == "" { - return "", "\"src\" or \"content\" in args should be string" - } destParam, err := variable.StringVar(ha.(map[string]any), args, "dest") if err != nil { return "", "\"dest\" in args should be string" @@ -55,7 +49,7 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { // get connector conn, err := getConnector(ctx, options.Host, ha.(map[string]any)) if err != nil { - return "", err.Error() + return "", fmt.Sprintf("get connector error: %v", err) } defer conn.Close(ctx) @@ -112,7 +106,7 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { return "", fmt.Sprintf("read file error: %v", err) } if strings.HasSuffix(destParam, "/") { - destParam = destParam + filepath.Base(srcParam) + destParam += filepath.Base(srcParam) } mode := fileInfo.Mode() if modeParam, err := variable.IntVar(ha.(map[string]any), args, "mode"); err == nil { @@ -143,7 +137,7 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { info, err := d.Info() if err != nil { - return fmt.Errorf("get file info error: %v", err) + return fmt.Errorf("get file info error: %w", err) } mode := info.Mode() if modeParam, err := variable.IntVar(ha.(map[string]any), args, "mode"); err == nil { @@ -151,7 +145,7 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { } data, err := pj.ReadFile(path, project.GetFileOption{Role: options.Task.Annotations[kubekeyv1alpha1.TaskAnnotationRole]}) if err != nil { - return fmt.Errorf("read file error: %v", err) + return fmt.Errorf("read file error: %w", err) } var destFilename = destParam if strings.HasSuffix(destParam, "/") { @@ -162,11 +156,11 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { destFilename = filepath.Join(destParam, rel) } if err := conn.PutFile(ctx, data, destFilename, mode); err != nil { - return fmt.Errorf("copy file error: %v", err) + return fmt.Errorf("copy file error: %w", err) } return nil }); err != nil { - return "", fmt.Sprintf("") + return "", fmt.Sprintf(" walk dir %s in local path error: %v", srcParam, err) } } else { data, err := pj.ReadFile(srcParam, project.GetFileOption{IsFile: true, Role: options.Task.Annotations[kubekeyv1alpha1.TaskAnnotationRole]}) @@ -174,7 +168,7 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { return "", fmt.Sprintf("read file error: %v", err) } if strings.HasSuffix(destParam, "/") { - destParam = destParam + filepath.Base(srcParam) + destParam += filepath.Base(srcParam) } mode := fileInfo.Mode() if modeParam, err := variable.IntVar(ha.(map[string]any), args, "mode"); err == nil { @@ -185,6 +179,7 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { } } } + return stdoutSuccess, "" case contentParam != "": // convert content param and copy to remote if strings.HasSuffix(destParam, "/") { @@ -196,9 +191,10 @@ func ModuleCopy(ctx context.Context, options ExecOptions) (string, string) { } if err := conn.PutFile(ctx, []byte(contentParam), destParam, mode); err != nil { - return "", err.Error() + return "", fmt.Sprintf("copy file error: %v", err) } + return stdoutSuccess, "" + default: + return "", "either \"src\" or \"content\" must be provided." } - return "success", "" - } diff --git a/pkg/modules/copy_test.go b/pkg/modules/copy_test.go index 074f462c..f006d907 100644 --- a/pkg/modules/copy_test.go +++ b/pkg/modules/copy_test.go @@ -18,7 +18,6 @@ package modules import ( "context" - "fmt" "testing" "time" @@ -30,21 +29,23 @@ func TestCopy(t *testing.T) { testcases := []struct { name string opt ExecOptions - ctx context.Context + ctxFunc func() context.Context exceptStdout string exceptStderr string }{ { name: "src and content is empty", opt: ExecOptions{ - Args: runtime.RawExtension{}, + Args: runtime.RawExtension{ + Raw: []byte(`{"dest": "hello world"}`), + }, Host: "local", Variable: &testVariable{}, }, - ctx: context.WithValue(context.Background(), ConnKey, &testConnector{ - output: []byte("success"), - }), - exceptStderr: "\"src\" or \"content\" in args should be string", + ctxFunc: func() context.Context { + return context.WithValue(context.Background(), ConnKey, successConnector) + }, + exceptStderr: "either \"src\" or \"content\" must be provided.", }, { name: "dest is empty", @@ -55,9 +56,9 @@ func TestCopy(t *testing.T) { Host: "local", Variable: &testVariable{}, }, - ctx: context.WithValue(context.Background(), ConnKey, &testConnector{ - output: []byte("success"), - }), + ctxFunc: func() context.Context { + return context.WithValue(context.Background(), ConnKey, successConnector) + }, exceptStderr: "\"dest\" in args should be string", }, { @@ -69,9 +70,9 @@ func TestCopy(t *testing.T) { Host: "local", Variable: &testVariable{}, }, - ctx: context.WithValue(context.Background(), ConnKey, &testConnector{ - output: []byte("success"), - }), + ctxFunc: func() context.Context { + return context.WithValue(context.Background(), ConnKey, successConnector) + }, exceptStderr: "\"content\" should copy to a file", }, { @@ -83,10 +84,10 @@ func TestCopy(t *testing.T) { Host: "local", Variable: &testVariable{}, }, - ctx: context.WithValue(context.Background(), ConnKey, &testConnector{ - output: []byte("success"), - }), - exceptStdout: "success", + ctxFunc: func() context.Context { + return context.WithValue(context.Background(), ConnKey, successConnector) + }, + exceptStdout: stdoutSuccess, }, { name: "copy failed", @@ -97,16 +98,16 @@ func TestCopy(t *testing.T) { Host: "local", Variable: &testVariable{}, }, - ctx: context.WithValue(context.Background(), ConnKey, &testConnector{ - copyErr: fmt.Errorf("copy failed"), - }), - exceptStderr: "copy failed", + ctxFunc: func() context.Context { + return context.WithValue(context.Background(), ConnKey, failedConnector) + }, + exceptStderr: "copy file error: failed", }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx, cancel := context.WithTimeout(tc.ctx, time.Second*5) + ctx, cancel := context.WithTimeout(tc.ctxFunc(), time.Second*5) defer cancel() acStdout, acStderr := ModuleCopy(ctx, tc.opt) assert.Equal(t, tc.exceptStdout, acStdout) diff --git a/pkg/modules/debug.go b/pkg/modules/debug.go index 79b77690..e6544724 100644 --- a/pkg/modules/debug.go +++ b/pkg/modules/debug.go @@ -20,8 +20,6 @@ import ( "context" "fmt" - "k8s.io/klog/v2" - "github.com/kubesphere/kubekey/v4/pkg/converter/tmpl" "github.com/kubesphere/kubekey/v4/pkg/variable" ) @@ -30,8 +28,7 @@ func ModuleDebug(ctx context.Context, options ExecOptions) (string, string) { // get host variable ha, err := options.Variable.Get(variable.GetAllVariable(options.Host)) if err != nil { - klog.V(4).ErrorS(err, "failed to get host variable", "hostname", options.Host) - return "", err.Error() + return "", fmt.Sprintf("failed to get host variable: %v", err) } args := variable.Extension2Variables(options.Args) @@ -39,8 +36,7 @@ func ModuleDebug(ctx context.Context, options ExecOptions) (string, string) { if varParam, err := variable.StringVar(ha.(map[string]any), args, "var"); err == nil { result, err := tmpl.ParseString(ha.(map[string]any), fmt.Sprintf("{{ %s }}", varParam)) if err != nil { - klog.V(4).ErrorS(err, "Failed to parse var") - return "", err.Error() + return "", fmt.Sprintf("failed to parse var: %v", err) } return result, "" } @@ -48,9 +44,6 @@ func ModuleDebug(ctx context.Context, options ExecOptions) (string, string) { if msgParam, err := variable.StringVar(ha.(map[string]any), args, "msg"); err == nil { return msgParam, "" } - if err != nil { - return "", err.Error() - } return "", "unknown args for debug. only support var or msg" } diff --git a/pkg/modules/fetch.go b/pkg/modules/fetch.go index 7194e6b6..e7ad8a31 100644 --- a/pkg/modules/fetch.go +++ b/pkg/modules/fetch.go @@ -18,6 +18,7 @@ package modules import ( "context" + "fmt" "os" "path/filepath" @@ -30,9 +31,9 @@ func ModuleFetch(ctx context.Context, options ExecOptions) (string, string) { // get host variable ha, err := options.Variable.Get(variable.GetAllVariable(options.Host)) if err != nil { - klog.V(4).ErrorS(err, "failed to get host variable", "hostname", options.Host) - return "", err.Error() + return "", fmt.Sprintf("failed to get host variable: %v", err) } + // check args args := variable.Extension2Variables(options.Args) srcParam, err := variable.StringVar(ha.(map[string]any), args, "src") @@ -47,15 +48,14 @@ func ModuleFetch(ctx context.Context, options ExecOptions) (string, string) { // get connector conn, err := getConnector(ctx, options.Host, ha.(map[string]any)) if err != nil { - return "", err.Error() + return "", fmt.Sprintf("get connector error: %v", err) } defer conn.Close(ctx) // fetch file if _, err := os.Stat(filepath.Dir(destParam)); os.IsNotExist(err) { if err := os.MkdirAll(filepath.Dir(destParam), 0755); err != nil { - klog.V(4).ErrorS(err, "failed to create dest dir") - return "", err.Error() + return "", fmt.Sprintf("failed to create dest dir: %v", err) } } destFile, err := os.Create(destParam) @@ -66,8 +66,7 @@ func ModuleFetch(ctx context.Context, options ExecOptions) (string, string) { defer destFile.Close() if err := conn.FetchFile(ctx, srcParam, destFile); err != nil { - klog.V(4).ErrorS(err, "failed to fetch file") - return "", err.Error() + return "", fmt.Sprintf("failed to fetch file: %v", err) } - return "success", "" + return stdoutSuccess, "" } diff --git a/pkg/modules/fetch_test.go b/pkg/modules/fetch_test.go index ced8ebf0..7c4ae63b 100644 --- a/pkg/modules/fetch_test.go +++ b/pkg/modules/fetch_test.go @@ -29,7 +29,7 @@ func TestFetch(t *testing.T) { testcases := []struct { name string opt ExecOptions - ctx context.Context + ctxFunc func() context.Context exceptStdout string exceptStderr string }{ @@ -40,9 +40,10 @@ func TestFetch(t *testing.T) { Host: "local", Variable: &testVariable{}, }, - ctx: context.WithValue(context.Background(), "connector", &testConnector{ - output: []byte("success"), - }), exceptStderr: "\"src\" in args should be string", + ctxFunc: func() context.Context { + return context.WithValue(context.Background(), ConnKey, successConnector) + }, + exceptStderr: "\"src\" in args should be string", }, { name: "dest is empty", @@ -53,16 +54,16 @@ func TestFetch(t *testing.T) { Host: "local", Variable: &testVariable{}, }, - ctx: context.WithValue(context.Background(), "connector", &testConnector{ - output: []byte("success"), - }), + ctxFunc: func() context.Context { + return context.WithValue(context.Background(), ConnKey, successConnector) + }, exceptStderr: "\"dest\" in args should be string", }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx, cancel := context.WithTimeout(tc.ctx, time.Second*5) + ctx, cancel := context.WithTimeout(tc.ctxFunc(), time.Second*5) defer cancel() acStdout, acStderr := ModuleFetch(ctx, tc.opt) assert.Equal(t, tc.exceptStdout, acStdout) diff --git a/pkg/modules/gen_cert.go b/pkg/modules/gen_cert.go index 7efd7ca3..25191830 100644 --- a/pkg/modules/gen_cert.go +++ b/pkg/modules/gen_cert.go @@ -19,7 +19,7 @@ import ( "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" - certutil "k8s.io/client-go/util/cert" + cgutilcert "k8s.io/client-go/util/cert" "k8s.io/client-go/util/keyutil" "k8s.io/klog/v2" netutils "k8s.io/utils/net" @@ -47,9 +47,9 @@ func ModuleGenCert(ctx context.Context, options ExecOptions) (stdout string, std // get host variable ha, err := options.Variable.Get(variable.GetAllVariable(options.Host)) if err != nil { - klog.V(4).ErrorS(err, "failed to get host variable", "hostname", options.Host) - return "", err.Error() + return "", fmt.Sprintf("failed to get host variable: %v", err) } + // args args := variable.Extension2Variables(options.Args) rootKeyParam, _ := variable.StringVar(ha.(map[string]any), args, "root_key") @@ -71,12 +71,12 @@ func ModuleGenCert(ctx context.Context, options ExecOptions) (stdout string, std return "", "\"cn\" in args should be string" } - altName := &certutil.AltNames{ + altName := &cgutilcert.AltNames{ DNSNames: []string{"localhost"}, IPs: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, } appendSANsToAltNames(altName, sansParam, outCertParam) - cfg := &certutil.Config{ + cfg := &cgutilcert.Config{ CommonName: cnParam, Organization: []string{"kubekey"}, AltNames: *altName, @@ -86,7 +86,7 @@ func ModuleGenCert(ctx context.Context, options ExecOptions) (stdout string, std var newCert *x509.Certificate newKey, err = rsa.GenerateKey(cryptorand.Reader, rsaKeySize) if err != nil { - return "", err.Error() + return "", fmt.Sprintf("generate rsa key error: %v", err) } var after time.Duration @@ -94,8 +94,7 @@ func ModuleGenCert(ctx context.Context, options ExecOptions) (stdout string, std if dateParam != "" { dur, err := time.ParseDuration(dateParam) if err != nil { - klog.V(4).ErrorS(err, "Failed to parse duration") - return "", err.Error() + return "", fmt.Sprintf("failed to parse duration: %v", err) } after = dur } @@ -104,19 +103,16 @@ func ModuleGenCert(ctx context.Context, options ExecOptions) (stdout string, std case rootKeyParam == "" || rootCertParam == "": // generate Self-signed certificate newCert, err = NewSelfSignedCACert(*cfg, after, newKey) if err != nil { - klog.V(4).ErrorS(err, "Failed to generate Self-signed certificate") - return "", err.Error() + return "", fmt.Sprintf("failed to generate Self-signed certificate: %v", err) } default: // generate certificate signed by root certificate parentKey, err := TryLoadKeyFromDisk(rootKeyParam) if err != nil { - klog.V(4).ErrorS(err, "Failed to load root key") - return "", err.Error() + return "", fmt.Sprintf("failed to load root key: %v", err) } parentCert, _, err := TryLoadCertChainFromDisk(rootCertParam) if err != nil { - klog.V(4).ErrorS(err, "Failed to load root certificate") - return "", err.Error() + return "", fmt.Sprintf("failed to load root certificate: %v", err) } if policyParam == policyIfNotPresent { if _, err := TryLoadKeyFromDisk(outKeyParam); err != nil { @@ -130,37 +126,31 @@ func ModuleGenCert(ctx context.Context, options ExecOptions) (stdout string, std } // check if the existing key and cert match the root key and cert if err := ValidateCertPeriod(existCert, 0); err != nil { - klog.V(4).ErrorS(err, "Failed to ValidateCertPeriod", "out_cert", outCertParam) - return "", err.Error() + return "", fmt.Sprintf("failed to ValidateCertPeriod: %v", err) } if err := VerifyCertChain(existCert, intermediates, parentCert); err != nil { - klog.V(4).ErrorS(err, "Failed to VerifyCertChain", "out_cert", outCertParam) - return "", err.Error() + return "", fmt.Sprintf("failed to VerifyCertChain: %v", err) } if err := validateCertificateWithConfig(existCert, outCertParam, cfg); err != nil { - klog.V(4).ErrorS(err, "Failed to validateCertificateWithConfig", "out_cert", outCertParam) - return "", err.Error() + return "", fmt.Sprintf("failed to validateCertificateWithConfig: %v", err) } - return "skip", "" + return stdoutSkip, "" } NEW: newCert, err = NewSignedCert(*cfg, after, newKey, parentCert, parentKey, true) if err != nil { - klog.V(4).ErrorS(err, "Failed to generate certificate") - return "", err.Error() + return "", fmt.Sprintf("failed to generate certificate: %v", err) } } // write key and cert to file if err := WriteKey(outKeyParam, newKey, policyParam); err != nil { - klog.V(4).ErrorS(err, "Failed to write key") - return "", err.Error() + return "", fmt.Sprintf("failed to write key: %v", err) } if err := WriteCert(outCertParam, newCert, policyParam); err != nil { - klog.V(4).ErrorS(err, "Failed to write certificate") - return "", err.Error() + return "", fmt.Sprintf("failed to write certificate: %v", err) } - return "success", "" + return stdoutSuccess, "" } // WriteKey stores the given key at the given location @@ -175,10 +165,10 @@ func WriteKey(outKey string, key crypto.Signer, policy string) error { encoded, err := keyutil.MarshalPrivateKeyToPEM(key) if err != nil { - return errors.Wrapf(err, "unable to marshal private key to PEM") + return fmt.Errorf("unable to marshal private key to PEM, error: %w", err) } if err := keyutil.WriteKey(outKey, encoded); err != nil { - return errors.Wrapf(err, "unable to write private key to file %s", outKey) + return fmt.Errorf("unable to write private key to file %s, error: %w", outKey, err) } return nil @@ -194,8 +184,8 @@ func WriteCert(outCert string, cert *x509.Certificate, policy string) error { return errors.New("certificate cannot be nil when writing to file") } - if err := certutil.WriteCert(outCert, EncodeCertPEM(cert)); err != nil { - return errors.Wrapf(err, "unable to write certificate to file %s", outCert) + if err := cgutilcert.WriteCert(outCert, EncodeCertPEM(cert)); err != nil { + return fmt.Errorf("unable to write certificate to file %s, error: %w", outCert, err) } return nil @@ -215,7 +205,7 @@ func TryLoadKeyFromDisk(rootKey string) (crypto.Signer, error) { // Parse the private key from a file privKey, err := keyutil.PrivateKeyFromFile(rootKey) if err != nil { - return nil, errors.Wrapf(err, "couldn't load the private key file %s", rootKey) + return nil, fmt.Errorf("couldn't load the private key file %s, error: %w", rootKey, err) } // Allow RSA and ECDSA formats only @@ -226,7 +216,7 @@ func TryLoadKeyFromDisk(rootKey string) (crypto.Signer, error) { case *ecdsa.PrivateKey: key = k default: - return nil, errors.Errorf("the private key file %s is neither in RSA nor ECDSA format", rootKey) + return nil, fmt.Errorf("the private key file %s is neither in RSA nor ECDSA format", rootKey) } return key, nil @@ -234,10 +224,9 @@ func TryLoadKeyFromDisk(rootKey string) (crypto.Signer, error) { // TryLoadCertChainFromDisk tries to load the cert chain from the disk func TryLoadCertChainFromDisk(rootCert string) (*x509.Certificate, []*x509.Certificate, error) { - - certs, err := certutil.CertsFromFile(rootCert) + certs, err := cgutilcert.CertsFromFile(rootCert) if err != nil { - return nil, nil, errors.Wrapf(err, "couldn't load the certificate file %s", rootCert) + return nil, nil, fmt.Errorf("couldn't load the certificate file %s, error: %w", rootCert, err) } cert := certs[0] @@ -252,8 +241,8 @@ func TryLoadCertChainFromDisk(rootCert string) (*x509.Certificate, []*x509.Certi // RFC-1123 compliant DNS strings are added to altNames.DNSNames as strings // RFC-1123 compliant wildcard DNS strings are added to altNames.DNSNames as strings // certNames is used to print user facing warnings and should be the name of the cert the altNames will be used for -func appendSANsToAltNames(altNames *certutil.AltNames, SANs []string, certName string) { - for _, altname := range SANs { +func appendSANsToAltNames(altNames *cgutilcert.AltNames, sans []string, certName string) { + for _, altname := range sans { if ip := netutils.ParseIPSloppy(altname); ip != nil { altNames.IPs = append(altNames.IPs, ip) } else if len(validation.IsDNS1123Subdomain(altname)) == 0 { @@ -271,7 +260,7 @@ func appendSANsToAltNames(altNames *certutil.AltNames, SANs []string, certName s } // NewSelfSignedCACert creates a CA certificate -func NewSelfSignedCACert(cfg certutil.Config, after time.Duration, key crypto.Signer) (*x509.Certificate, error) { +func NewSelfSignedCACert(cfg cgutilcert.Config, after time.Duration, key crypto.Signer) (*x509.Certificate, error) { now := time.Now() // returns a uniform random value in [0, max-1), then add 1 to serial to make it a uniform random value in [1, max). serial, err := cryptorand.Int(cryptorand.Reader, new(big.Int).SetInt64(math.MaxInt64-1)) @@ -309,15 +298,15 @@ func NewSelfSignedCACert(cfg certutil.Config, after time.Duration, key crypto.Si } // NewSignedCert creates a signed certificate using the given CA certificate and key -func NewSignedCert(cfg certutil.Config, after time.Duration, key crypto.Signer, caCert *x509.Certificate, caKey crypto.Signer, isCA bool) (*x509.Certificate, error) { +func NewSignedCert(cfg cgutilcert.Config, after time.Duration, key crypto.Signer, caCert *x509.Certificate, caKey crypto.Signer, isCA bool) (*x509.Certificate, error) { // returns a uniform random value in [0, max-1), then add 1 to serial to make it a uniform random value in [1, max). serial, err := cryptorand.Int(cryptorand.Reader, new(big.Int).SetInt64(math.MaxInt64-1)) if err != nil { return nil, err } serial = new(big.Int).Add(serial, big.NewInt(1)) - if len(cfg.CommonName) == 0 { - return nil, errors.New("must specify a CommonName") + if cfg.CommonName == "" { + return nil, fmt.Errorf("must specify a CommonName") } keyUsage := x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature @@ -354,7 +343,7 @@ func NewSignedCert(cfg certutil.Config, after time.Duration, key crypto.Signer, } // RemoveDuplicateAltNames removes duplicate items in altNames. -func RemoveDuplicateAltNames(altNames *certutil.AltNames) { +func RemoveDuplicateAltNames(altNames *cgutilcert.AltNames) { if altNames == nil { return } @@ -380,10 +369,10 @@ func ValidateCertPeriod(cert *x509.Certificate, offset time.Duration) error { period := fmt.Sprintf("NotBefore: %v, NotAfter: %v", cert.NotBefore, cert.NotAfter) now := time.Now().Add(offset) if now.Before(cert.NotBefore) { - return errors.Errorf("the certificate is not valid yet: %s", period) + return fmt.Errorf("the certificate is not valid yet: %s", period) } if now.After(cert.NotAfter) { - return errors.Errorf("the certificate has expired: %s", period) + return fmt.Errorf("the certificate has expired: %s", period) } return nil } @@ -414,15 +403,15 @@ func VerifyCertChain(cert *x509.Certificate, intermediates []*x509.Certificate, // validateCertificateWithConfig makes sure that a given certificate is valid at // least for the SANs defined in the configuration. -func validateCertificateWithConfig(cert *x509.Certificate, baseName string, cfg *certutil.Config) error { +func validateCertificateWithConfig(cert *x509.Certificate, baseName string, cfg *cgutilcert.Config) error { for _, dnsName := range cfg.AltNames.DNSNames { if err := cert.VerifyHostname(dnsName); err != nil { - return errors.Wrapf(err, "certificate %s is invalid", baseName) + return fmt.Errorf("certificate %s is invalid, error: %w", baseName, err) } } for _, ipAddress := range cfg.AltNames.IPs { if err := cert.VerifyHostname(ipAddress.String()); err != nil { - return errors.Wrapf(err, "certificate %s is invalid", baseName) + return fmt.Errorf("certificate %s is invalid, error: %w", baseName, err) } } return nil diff --git a/pkg/modules/image.go b/pkg/modules/image.go index 7526ebd5..c21d5c75 100644 --- a/pkg/modules/image.go +++ b/pkg/modules/image.go @@ -29,7 +29,6 @@ import ( "strings" imagev1 "github.com/opencontainers/image-spec/specs-go/v1" - "k8s.io/klog/v2" "oras.land/oras-go/v2" "oras.land/oras-go/v2/registry" "oras.land/oras-go/v2/registry/remote" @@ -44,8 +43,7 @@ func ModuleImage(ctx context.Context, options ExecOptions) (stdout string, stder // get host variable ha, err := options.Variable.Get(variable.GetAllVariable(options.Host)) if err != nil { - klog.V(4).ErrorS(err, "failed to get host variable", "hostname", options.Host) - return "", err.Error() + return "", fmt.Sprintf("failed to get host variable: %v", err) } // check args @@ -109,7 +107,7 @@ func ModuleImage(ctx context.Context, options ExecOptions) (stdout string, stder } } - return "", "" + return stdoutSuccess, "" } func findLocalImageManifests(localDir string) ([]string, error) { diff --git a/pkg/modules/module.go b/pkg/modules/module.go index 8aabf63a..469cd8c6 100644 --- a/pkg/modules/module.go +++ b/pkg/modules/module.go @@ -21,6 +21,7 @@ import ( "fmt" "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/klog/v2" kubekeyv1 "github.com/kubesphere/kubekey/v4/pkg/apis/kubekey/v1" @@ -29,6 +30,18 @@ import ( "github.com/kubesphere/kubekey/v4/pkg/variable" ) +// message for stdout +const ( + // stdoutSuccess message for common module + stdoutSuccess = "success" + stdoutSkip = "skip" + + // stdoutTrue for bool module + stdoutTrue = "True" + // stdoutFalse for bool module + stdoutFalse = "False" +) + type ModuleExecFunc func(ctx context.Context, options ExecOptions) (stdout string, stderr string) type ExecOptions struct { @@ -39,9 +52,9 @@ type ExecOptions struct { // the variable module need variable.Variable // the task to be executed - kubekeyv1alpha1.Task + Task kubekeyv1alpha1.Task // the pipeline to be executed - kubekeyv1.Pipeline + Pipeline kubekeyv1.Pipeline } var module = make(map[string]ModuleExecFunc) @@ -59,16 +72,16 @@ func FindModule(moduleName string) ModuleExecFunc { } func init() { - RegisterModule("assert", ModuleAssert) - RegisterModule("command", ModuleCommand) - RegisterModule("shell", ModuleCommand) - RegisterModule("copy", ModuleCopy) - RegisterModule("fetch", ModuleFetch) - RegisterModule("debug", ModuleDebug) - RegisterModule("template", ModuleTemplate) - RegisterModule("set_fact", ModuleSetFact) - RegisterModule("gen_cert", ModuleGenCert) - RegisterModule("image", ModuleImage) + utilruntime.Must(RegisterModule("assert", ModuleAssert)) + utilruntime.Must(RegisterModule("command", ModuleCommand)) + utilruntime.Must(RegisterModule("shell", ModuleCommand)) + utilruntime.Must(RegisterModule("copy", ModuleCopy)) + utilruntime.Must(RegisterModule("fetch", ModuleFetch)) + utilruntime.Must(RegisterModule("debug", ModuleDebug)) + utilruntime.Must(RegisterModule("template", ModuleTemplate)) + utilruntime.Must(RegisterModule("set_fact", ModuleSetFact)) + utilruntime.Must(RegisterModule("gen_cert", ModuleGenCert)) + utilruntime.Must(RegisterModule("image", ModuleImage)) } // ConnKey for connector which store in context diff --git a/pkg/modules/module_test.go b/pkg/modules/module_test.go index 1198a694..0da2fc60 100644 --- a/pkg/modules/module_test.go +++ b/pkg/modules/module_test.go @@ -18,6 +18,7 @@ package modules import ( "context" + "fmt" "io" "io/fs" @@ -37,13 +38,20 @@ func (v testVariable) Get(f variable.GetFunc) (any, error) { return v.value, v.err } -func (v testVariable) Merge(f variable.MergeFunc) error { +func (v *testVariable) Merge(f variable.MergeFunc) error { v.value = map[string]any{ "k": "v", } return nil } +var successConnector = &testConnector{output: []byte("success")} +var failedConnector = &testConnector{ + copyErr: fmt.Errorf("failed"), + fetchErr: fmt.Errorf("failed"), + commandErr: fmt.Errorf("failed"), +} + type testConnector struct { // return for init initErr error diff --git a/pkg/modules/set_fact.go b/pkg/modules/set_fact.go index e25f5399..69f460ef 100644 --- a/pkg/modules/set_fact.go +++ b/pkg/modules/set_fact.go @@ -30,5 +30,5 @@ func ModuleSetFact(ctx context.Context, options ExecOptions) (string, string) { if err := options.Variable.Merge(variable.MergeAllRuntimeVariable(options.Host, args)); err != nil { return "", fmt.Sprintf("set_fact error: %v", err) } - return "success", "" + return stdoutSuccess, "" } diff --git a/pkg/modules/template.go b/pkg/modules/template.go index 8e353404..c89f2df5 100644 --- a/pkg/modules/template.go +++ b/pkg/modules/template.go @@ -24,8 +24,6 @@ import ( "path/filepath" "strings" - "k8s.io/klog/v2" - kubekeyv1alpha1 "github.com/kubesphere/kubekey/v4/pkg/apis/kubekey/v1alpha1" "github.com/kubesphere/kubekey/v4/pkg/converter/tmpl" "github.com/kubesphere/kubekey/v4/pkg/project" @@ -36,9 +34,9 @@ func ModuleTemplate(ctx context.Context, options ExecOptions) (string, string) { // get host variable ha, err := options.Variable.Get(variable.GetAllVariable(options.Host)) if err != nil { - klog.V(4).ErrorS(err, "failed to get host variable", "hostname", options.Host) - return "", err.Error() + return "", fmt.Sprintf("failed to get host variable: %v", err) } + // check args args := variable.Extension2Variables(options.Args) srcParam, err := variable.StringVar(ha.(map[string]any), args, "src") @@ -142,12 +140,12 @@ func ModuleTemplate(ctx context.Context, options ExecOptions) (string, string) { return nil } if err != nil { - return fmt.Errorf("walk dir %s error: %v", srcParam, err) + return fmt.Errorf("walk dir %s error: %w", srcParam, err) } info, err := d.Info() if err != nil { - return fmt.Errorf("get file info error: %v", err) + return fmt.Errorf("get file info error: %w", err) } mode := info.Mode() if modeParam, err := variable.IntVar(ha.(map[string]any), args, "mode"); err == nil { @@ -155,11 +153,11 @@ func ModuleTemplate(ctx context.Context, options ExecOptions) (string, string) { } data, err := pj.ReadFile(path, project.GetFileOption{IsTemplate: true, Role: options.Task.Annotations[kubekeyv1alpha1.TaskAnnotationRole]}) if err != nil { - return fmt.Errorf("read file error: %v", err) + return fmt.Errorf("read file error: %w", err) } result, err := tmpl.ParseFile(ha.(map[string]any), data) if err != nil { - return fmt.Errorf("parse file error: %v", err) + return fmt.Errorf("parse file error: %w", err) } var destFilename = destParam if strings.HasSuffix(destParam, "/") { @@ -170,7 +168,7 @@ func ModuleTemplate(ctx context.Context, options ExecOptions) (string, string) { destFilename = filepath.Join(destParam, rel) } if err := conn.PutFile(ctx, []byte(result), destFilename, mode); err != nil { - return fmt.Errorf("copy file error: %v", err) + return fmt.Errorf("copy file error: %w", err) } return nil }); err != nil { @@ -186,7 +184,7 @@ func ModuleTemplate(ctx context.Context, options ExecOptions) (string, string) { return "", fmt.Sprintf("parse file error: %v", err) } if strings.HasSuffix(destParam, "/") { - destParam = destParam + filepath.Base(srcParam) + destParam += filepath.Base(srcParam) } mode := fileInfo.Mode() if modeParam, err := variable.IntVar(ha.(map[string]any), args, "mode"); err == nil { @@ -197,5 +195,5 @@ func ModuleTemplate(ctx context.Context, options ExecOptions) (string, string) { } } } - return "success", "" + return stdoutSuccess, "" } diff --git a/pkg/modules/template_test.go b/pkg/modules/template_test.go index 005c6a2b..4f1f9b6c 100644 --- a/pkg/modules/template_test.go +++ b/pkg/modules/template_test.go @@ -38,7 +38,7 @@ func TestTemplate(t *testing.T) { testcases := []struct { name string opt ExecOptions - ctx context.Context + ctxFunc func() context.Context exceptStdout string exceptStderr string }{ @@ -49,7 +49,7 @@ func TestTemplate(t *testing.T) { Host: "local", Variable: &testVariable{}, }, - ctx: context.Background(), + ctxFunc: context.Background, exceptStderr: "\"src\" should be string", }, { @@ -61,14 +61,14 @@ func TestTemplate(t *testing.T) { Host: "local", Variable: &testVariable{}, }, - ctx: context.Background(), + ctxFunc: context.Background, exceptStderr: "\"dest\" should be string", }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx, cancel := context.WithTimeout(tc.ctx, time.Second*5) + ctx, cancel := context.WithTimeout(tc.ctxFunc(), time.Second*5) defer cancel() acStdout, acStderr := ModuleTemplate(ctx, tc.opt) assert.Equal(t, tc.exceptStdout, acStdout) diff --git a/pkg/project/builtin.go b/pkg/project/builtin.go index eeff422f..f767601f 100644 --- a/pkg/project/builtin.go +++ b/pkg/project/builtin.go @@ -71,7 +71,7 @@ func (p builtinProject) getFilePath(path string, o GetFileOption) string { } find = append(find, filepath.Join(_const.ProjectRolesTemplateDir, path)) default: - find = append(find, filepath.Join(path)) + find = append(find, path) } for _, s := range find { if _, err := fs.Stat(p.FS, s); err == nil { diff --git a/pkg/project/git.go b/pkg/project/git.go index 6ce11386..02c41aac 100644 --- a/pkg/project/git.go +++ b/pkg/project/git.go @@ -18,6 +18,7 @@ package project import ( "context" + "errors" "fmt" "io/fs" "os" @@ -126,7 +127,7 @@ func (p gitProject) gitClone(ctx context.Context) error { Progress: nil, ReferenceName: plumbing.NewBranchReferenceName(p.Pipeline.Spec.Project.Branch), SingleBranch: true, - Auth: &http.TokenAuth{p.Pipeline.Spec.Project.Token}, + Auth: &http.TokenAuth{Token: p.Pipeline.Spec.Project.Token}, InsecureSkipTLS: false, }); err != nil { klog.Errorf("clone project %s failed: %v", p.Pipeline.Spec.Project.Addr, err) @@ -150,9 +151,9 @@ func (p gitProject) gitPull(ctx context.Context) error { RemoteURL: p.Pipeline.Spec.Project.Addr, ReferenceName: plumbing.NewBranchReferenceName(p.Pipeline.Spec.Project.Branch), SingleBranch: true, - Auth: &http.TokenAuth{p.Pipeline.Spec.Project.Token}, + Auth: &http.TokenAuth{Token: p.Pipeline.Spec.Project.Token}, InsecureSkipTLS: false, - }); err != nil && err != git.NoErrAlreadyUpToDate { + }); err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) { klog.V(4).ErrorS(err, "git pull error", "local_dir", p.projectDir) return err } diff --git a/pkg/project/helper.go b/pkg/project/helper.go index f9366911..35ed29d1 100644 --- a/pkg/project/helper.go +++ b/pkg/project/helper.go @@ -187,7 +187,6 @@ func convertRoles(baseFS fs.FS, pbPath string, pb *kkcorev1.Playbook) error { if err != nil { return err } - } } pb.Play[i] = p diff --git a/pkg/project/helper_test.go b/pkg/project/helper_test.go index 23eba15c..b977678e 100644 --- a/pkg/project/helper_test.go +++ b/pkg/project/helper_test.go @@ -133,24 +133,26 @@ func TestMarshalPlaybook(t *testing.T) { { name: "marshal playbook", file: "playbooks/playbook1.yaml", - except: &kkcorev1.Playbook{[]kkcorev1.Play{ + except: &kkcorev1.Playbook{Play: []kkcorev1.Play{ { Base: kkcorev1.Base{Name: "play1"}, PlayHost: kkcorev1.PlayHost{Hosts: []string{"localhost"}}, Roles: []kkcorev1.Role{ - {kkcorev1.RoleInfo{ - Role: "role1", - Block: []kkcorev1.Block{ - { - BlockBase: kkcorev1.BlockBase{Base: kkcorev1.Base{Name: "role1 | block1"}}, - Task: kkcorev1.Task{UnknownFiled: map[string]any{ - "debug": map[string]any{ - "msg": "echo \"hello world\"", - }, - }}, + { + RoleInfo: kkcorev1.RoleInfo{ + Role: "role1", + Block: []kkcorev1.Block{ + { + BlockBase: kkcorev1.BlockBase{Base: kkcorev1.Base{Name: "role1 | block1"}}, + Task: kkcorev1.Task{UnknownFiled: map[string]any{ + "debug": map[string]any{ + "msg": "echo \"hello world\"", + }, + }}, + }, }, }, - }}, + }, }, Handlers: nil, PreTasks: []kkcorev1.Block{ @@ -281,7 +283,6 @@ func TestCombineMaps(t *testing.T) { } else { assert.Equal(t, tc.except, maps) } - }) } } diff --git a/pkg/project/project.go b/pkg/project/project.go index 1efe358b..d909ed3c 100644 --- a/pkg/project/project.go +++ b/pkg/project/project.go @@ -47,7 +47,6 @@ func New(pipeline kubekeyv1.Pipeline, update bool) (Project, error) { if strings.HasPrefix(pipeline.Spec.Project.Addr, "https://") || strings.HasPrefix(pipeline.Spec.Project.Addr, "http://") || strings.HasPrefix(pipeline.Spec.Project.Addr, "git@") { - return newGitProject(pipeline, update) } diff --git a/pkg/proxy/api_resources.go b/pkg/proxy/api_resources.go index e8b1b6b9..e7a42c58 100644 --- a/pkg/proxy/api_resources.go +++ b/pkg/proxy/api_resources.go @@ -102,7 +102,7 @@ func (r *apiResources) AddResource(o resourceOptions) error { if err != nil { return err } - if len(subResource) == 0 { + if subResource == "" { singularNameProvider, ok := o.storage.(apirest.SingularNameProvider) if !ok { return fmt.Errorf("resource %s must implement SingularNameProvider", o.path) diff --git a/pkg/proxy/internal/file_storage.go b/pkg/proxy/internal/file_storage.go index 28f07314..bc228f50 100644 --- a/pkg/proxy/internal/file_storage.go +++ b/pkg/proxy/internal/file_storage.go @@ -184,7 +184,7 @@ func (s fileStorage) GetList(ctx context.Context, key string, opts apistorage.Li } switch { - case opts.Recursive && len(opts.Predicate.Continue) > 0: + case opts.Recursive && opts.Predicate.Continue != "": // The format of continueKey is: namespace/resourceName/name.yaml // continueKey is localPath which resources store. continueKey, _, err := apistorage.DecodeContinue(opts.Predicate.Continue, key) @@ -203,7 +203,7 @@ func (s fileStorage) GetList(ctx context.Context, key string, opts apistorage.Li // start read after continueKey (not contain). Because it has read in last result. return startRead && key != continueKey } - case len(opts.ResourceVersion) > 0: + case opts.ResourceVersion != "": parsedRV, err := s.versioner.ParseResourceVersion(opts.ResourceVersion) if err != nil { return fmt.Errorf("invalid resource version: %w", err) @@ -364,7 +364,7 @@ RESULT: // we never return a key that the client wouldn't be allowed to see if hasMore { // we want to start immediately after the last key - next, err := apistorage.EncodeContinue(string(lastKey)+"\x00", key, 0) + next, err := apistorage.EncodeContinue(lastKey+"\x00", key, 0) if err != nil { return err } @@ -499,7 +499,7 @@ func decode(codec runtime.Codec, value []byte, objPtr runtime.Object) error { func getNewItem(listObj runtime.Object, v reflect.Value) runtime.Object { // For unstructured lists with a target group/version, preserve the group/version in the instantiated list items if unstructuredList, isUnstructured := listObj.(*unstructured.UnstructuredList); isUnstructured { - if apiVersion := unstructuredList.GetAPIVersion(); len(apiVersion) > 0 { + if apiVersion := unstructuredList.GetAPIVersion(); apiVersion != "" { return &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": apiVersion}} } } diff --git a/pkg/proxy/internal/rest_option.go b/pkg/proxy/internal/rest_option.go index 5ce3c90b..e16b0971 100644 --- a/pkg/proxy/internal/rest_option.go +++ b/pkg/proxy/internal/rest_option.go @@ -29,7 +29,7 @@ import ( cacherstorage "k8s.io/apiserver/pkg/storage/cacher" "k8s.io/apiserver/pkg/storage/storagebackend" "k8s.io/apiserver/pkg/storage/storagebackend/factory" - cgcache "k8s.io/client-go/tools/cache" + cgtoolscache "k8s.io/client-go/tools/cache" _const "github.com/kubesphere/kubekey/v4/pkg/const" ) @@ -48,7 +48,7 @@ func NewFileRESTOptionsGetter(gv schema.GroupVersion) apigeneric.RESTOptionsGett } func newYamlCodec(gv schema.GroupVersion) runtime.Codec { - yamlSerializer := json.NewSerializerWithOptions(json.DefaultMetaFactory, _const.Scheme, _const.Scheme, json.SerializerOptions{true, false, false}) + yamlSerializer := json.NewSerializerWithOptions(json.DefaultMetaFactory, _const.Scheme, _const.Scheme, json.SerializerOptions{Yaml: true}) return versioning.NewDefaultingCodecForScheme( _const.Scheme, yamlSerializer, @@ -60,7 +60,6 @@ func newYamlCodec(gv schema.GroupVersion) runtime.Codec { type fileRESTOptionsGetter struct { gv schema.GroupVersion - allowGroups []string storageConfig *storagebackend.Config } @@ -74,7 +73,7 @@ func (f fileRESTOptionsGetter) GetRESTOptions(resource schema.GroupResource) (ap newListFunc func() runtime.Object, getAttrsFunc apistorage.AttrFunc, triggerFuncs apistorage.IndexerFuncs, - indexers *cgcache.Indexers) (apistorage.Interface, factory.DestroyFunc, error) { + indexers *cgtoolscache.Indexers) (apistorage.Interface, factory.DestroyFunc, error) { s, d, err := newFileStorage(prefix, resource, storageConfig.Codec, newFunc) if err != nil { return s, d, err diff --git a/pkg/proxy/path_expression.go b/pkg/proxy/path_expression.go index 2c7f327b..70e2fc21 100644 --- a/pkg/proxy/path_expression.go +++ b/pkg/proxy/path_expression.go @@ -53,7 +53,6 @@ func newPathExpression(path string) (*pathExpression, error) { func templateToRegularExpression(template string) (expression string, literalCount int, varNames []string, varCount int, tokens []string) { var buffer bytes.Buffer buffer.WriteString("^") - //tokens = strings.Split(template, "/") tokens = tokenizePath(template) for _, each := range tokens { if each == "" { @@ -91,7 +90,7 @@ func templateToRegularExpression(template string) (expression string, literalCou // Tokenize an URL path using the slash separator ; the result does not have empty tokens func tokenizePath(path string) []string { - if "/" == path { + if path == "/" { return nil } if TrimRightSlashEnabled { @@ -101,7 +100,6 @@ func tokenizePath(path string) []string { // 3.10.2 return strings.Split(strings.TrimLeft(path, "/"), "/") } - } // TrimRightSlashEnabled controls whether diff --git a/pkg/proxy/resources/task/strategy.go b/pkg/proxy/resources/task/strategy.go index c187b58f..468ad719 100644 --- a/pkg/proxy/resources/task/strategy.go +++ b/pkg/proxy/resources/task/strategy.go @@ -29,13 +29,15 @@ import ( apigeneric "k8s.io/apiserver/pkg/registry/generic" apistorage "k8s.io/apiserver/pkg/storage" apinames "k8s.io/apiserver/pkg/storage/names" - "k8s.io/client-go/tools/cache" + cgtoolscache "k8s.io/client-go/tools/cache" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" kubekeyv1alpha1 "github.com/kubesphere/kubekey/v4/pkg/apis/kubekey/v1alpha1" _const "github.com/kubesphere/kubekey/v4/pkg/const" ) +const pipelineKind = "Pipeline" + // taskStrategy implements behavior for Pods type taskStrategy struct { runtime.ObjectTyper @@ -118,7 +120,7 @@ func OwnerPipelineIndexFunc(obj interface{}) ([]string, error) { var index string for _, reference := range task.OwnerReferences { - if reference.Kind == "Pipeline" { + if reference.Kind == pipelineKind { index = types.NamespacedName{ Namespace: task.Namespace, Name: reference.Name, @@ -134,8 +136,8 @@ func OwnerPipelineIndexFunc(obj interface{}) ([]string, error) { } // Indexers returns the indexers for pod storage. -func Indexers() *cache.Indexers { - return &cache.Indexers{ +func Indexers() *cgtoolscache.Indexers { + return &cgtoolscache.Indexers{ apistorage.FieldIndex(kubekeyv1alpha1.TaskOwnerField): OwnerPipelineIndexFunc, } } @@ -168,7 +170,7 @@ func ToSelectableFields(task *kubekeyv1alpha1.Task) fields.Set { // be adjusted. taskSpecificFieldsSet := make(fields.Set, 10) for _, reference := range task.OwnerReferences { - if reference.Kind == "Pipeline" { + if reference.Kind == pipelineKind { taskSpecificFieldsSet[kubekeyv1alpha1.TaskOwnerField] = types.NamespacedName{ Namespace: task.Namespace, Name: reference.Name, @@ -183,7 +185,7 @@ func ToSelectableFields(task *kubekeyv1alpha1.Task) fields.Set { func OwnerPipelineTriggerFunc(obj runtime.Object) string { task := obj.(*kubekeyv1alpha1.Task) for _, reference := range task.OwnerReferences { - if reference.Kind == "Pipeline" { + if reference.Kind == pipelineKind { return types.NamespacedName{ Namespace: task.Namespace, Name: reference.Name, diff --git a/pkg/proxy/router.go b/pkg/proxy/router.go index b92b7c60..5ac89a2d 100644 --- a/pkg/proxy/router.go +++ b/pkg/proxy/router.go @@ -24,10 +24,6 @@ type router struct { handlers map[string]http.HandlerFunc } -func (r router) matchPath(url string) bool { - return r.pathExpr.Matcher.MatchString(url) -} - // Types and functions to support the sorting of Dispatchers type dispatcherCandidate struct { diff --git a/pkg/proxy/transport.go b/pkg/proxy/transport.go index 207dc169..3b048754 100644 --- a/pkg/proxy/transport.go +++ b/pkg/proxy/transport.go @@ -280,7 +280,7 @@ func (l *transport) registerResources(resources *apiResources) error { if err != nil { return err } - isSubresource := len(subresource) > 0 + isSubresource := subresource != "" scoper, ok := o.storage.(apirest.Scoper) if !ok { return fmt.Errorf("%q must implement scoper", o.path) diff --git a/pkg/variable/helper.go b/pkg/variable/helper.go index 089105cf..7fc60c80 100644 --- a/pkg/variable/helper.go +++ b/pkg/variable/helper.go @@ -58,7 +58,6 @@ func combineVariables(v1, v2 map[string]any) map[string]any { mv[k] = f(mv[k], v) } return mv - } func convertGroup(inv kubekeyv1.Inventory) map[string]any { @@ -129,10 +128,10 @@ func StringSliceVar(d map[string]any, vars map[string]any, key string) ([]string if !ok { return nil, fmt.Errorf("cannot find variable \"%s\"", key) } - switch val.(type) { + switch valv := val.(type) { case []any: var ss []string - for _, a := range val.([]any) { + for _, a := range valv { av, ok := a.(string) if !ok { klog.V(6).InfoS("variable is not string", "key", key) @@ -146,7 +145,7 @@ func StringSliceVar(d map[string]any, vars map[string]any, key string) ([]string } return ss, nil case string: - as, err := tmpl.ParseString(d, val.(string)) + as, err := tmpl.ParseString(d, valv) if err != nil { return nil, err } @@ -156,7 +155,7 @@ func StringSliceVar(d map[string]any, vars map[string]any, key string) ([]string // in pongo2 cannot get slice value. add extension filter value. var input = val.(string) // try to escape string - if ns, err := strconv.Unquote(val.(string)); err == nil { + if ns, err := strconv.Unquote(valv); err == nil { input = ns } vv := GetValue(d, input) @@ -170,7 +169,7 @@ func StringSliceVar(d map[string]any, vars map[string]any, key string) ([]string // value is simple string if err := json.Unmarshal([]byte(as), &ss); err != nil { // if is not json format. only return a value contains this - return []string{as}, nil + return []string{as}, nil //nolint:nilerr } } return ss, nil @@ -186,11 +185,11 @@ func IntVar(d map[string]any, vars map[string]any, key string) (int, error) { return 0, fmt.Errorf("cannot find variable \"%s\"", key) } // default convert to float64 - switch val.(type) { + switch valv := val.(type) { case float64: - return int(val.(float64)), nil + return int(valv), nil case string: - vs, err := tmpl.ParseString(d, val.(string)) + vs, err := tmpl.ParseString(d, valv) if err != nil { return 0, err } @@ -295,9 +294,9 @@ func parseVariable(v any, parseTmplFunc func(string) (string, error)) error { return err } switch { - case strings.ToUpper(newValue) == "TRUE": + case strings.EqualFold(newValue, "TRUE"): reflect.ValueOf(v).SetMapIndex(kv, reflect.ValueOf(true)) - case strings.ToUpper(newValue) == "FALSE": + case strings.EqualFold(newValue, "FALSE"): reflect.ValueOf(v).SetMapIndex(kv, reflect.ValueOf(false)) default: reflect.ValueOf(v).SetMapIndex(kv, reflect.ValueOf(newValue)) @@ -319,10 +318,10 @@ func parseVariable(v any, parseTmplFunc func(string) (string, error)) error { return err } switch { - case strings.ToUpper(newValue) == "TRUE": + case strings.EqualFold(newValue, "TRUE"): val.Set(reflect.ValueOf(true)) - case strings.ToUpper(newValue) == "FALSE": + case strings.EqualFold(newValue, "FALSE"): val.Set(reflect.ValueOf(false)) default: val.Set(reflect.ValueOf(newValue)) diff --git a/pkg/variable/internal.go b/pkg/variable/internal.go index 1f3c509c..1f831b49 100644 --- a/pkg/variable/internal.go +++ b/pkg/variable/internal.go @@ -89,7 +89,6 @@ func (v value) getParameterVariable() map[string]any { // merge config vars to host vars hostVars = combineVariables(hostVars, Extension2Variables(v.Config.Spec)) globalHosts[hostname] = hostVars - } var externalVal = make(map[string]any) // external vars