From b6c89f9d0950d62a17ddc43cae997f1f54f825ad Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Wed, 31 Jul 2024 12:12:35 +0200 Subject: [PATCH] Enable and fix `goconst` linter Signed-off-by: Sascha Grunert --- .golangci.yml | 2 +- cmd/crictl/container.go | 16 +++++++++------- cmd/crictl/container_stats.go | 4 ++-- cmd/crictl/events.go | 6 +++--- cmd/crictl/image.go | 12 ++++++------ cmd/crictl/info.go | 2 +- cmd/crictl/main.go | 3 ++- cmd/crictl/pod_metrics.go | 6 +++--- cmd/crictl/pod_stats.go | 4 ++-- cmd/crictl/sandbox.go | 12 ++++++------ cmd/crictl/templates.go | 8 ++++---- cmd/crictl/util.go | 19 ++++++++++++------- cmd/crictl/util_test.go | 10 +++++----- pkg/common/file.go | 18 ++++++++++++------ pkg/framework/test_context.go | 6 ++++-- pkg/framework/util.go | 2 +- pkg/validate/consts.go | 8 ++++---- pkg/validate/image.go | 2 +- pkg/validate/security_context_linux.go | 6 +++--- 19 files changed, 81 insertions(+), 65 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 59899c68d5..f17ab46b5b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -32,6 +32,7 @@ linters: - gocheckcompilerdirectives - gochecksumtype - gocognit + - goconst - gocritic - gocyclo - godot @@ -99,7 +100,6 @@ linters: # - funlen # - gochecknoglobals # - gochecknoinits - # - goconst # - gosec # - ireturn # - lll diff --git a/cmd/crictl/container.go b/cmd/crictl/container.go index b3074d216c..3d3e79fbf6 100644 --- a/cmd/crictl/container.go +++ b/cmd/crictl/container.go @@ -37,6 +37,8 @@ import ( internalapi "k8s.io/cri-api/pkg/apis" pb "k8s.io/cri-api/pkg/apis/runtime/v1" "k8s.io/kubelet/pkg/types" + + "sigs.k8s.io/cri-tools/pkg/framework" ) type containerByCreated []*pb.Container @@ -580,7 +582,7 @@ var listContainersCommand = &cli.Command{ Name: "output", Aliases: []string{"o"}, Usage: "Output format, One of: json|yaml|table", - Value: "table", + Value: outputTypeTable, }, &cli.BoolFlag{ Name: "all", @@ -889,7 +891,7 @@ func UpdateContainerResources(client internalapi.RuntimeService, id string, opts request := &pb.UpdateContainerResourcesRequest{ ContainerId: id, } - if goruntime.GOOS != "windows" { + if goruntime.GOOS != framework.OSWindows { request.Linux = &pb.LinuxContainerResources{ CpuPeriod: opts.CPUPeriod, CpuQuota: opts.CPUQuota, @@ -1010,7 +1012,7 @@ func marshalContainerStatus(cs *pb.ContainerStatus) (string, error) { func containerStatus(client internalapi.RuntimeService, ids []string, output, tmplStr string, quiet bool) error { verbose := !(quiet) if output == "" { // default to json output - output = "json" + output = outputTypeJSON } if len(ids) == 0 { return errors.New("ID cannot be empty") @@ -1036,7 +1038,7 @@ func containerStatus(client internalapi.RuntimeService, ids []string, output, tm return fmt.Errorf("marshal container status: %w", err) } - if output == "table" { + if output == outputTypeTable { outputContainerStatusTable(r, verbose) } else { statuses = append(statuses, statusData{json: statusJSON, info: r.Info}) @@ -1138,11 +1140,11 @@ func ListContainers(runtimeClient internalapi.RuntimeService, imageClient intern r = getContainersList(r, opts) switch opts.output { - case "json": + case outputTypeJSON: return outputProtobufObjAsJSON(&pb.ListContainersResponse{Containers: r}) - case "yaml": + case outputTypeYAML: return outputProtobufObjAsYAML(&pb.ListContainersResponse{Containers: r}) - case "table": + case outputTypeTable: // continue; output will be generated after the switch block ends. default: return fmt.Errorf("unsupported output format %q", opts.output) diff --git a/cmd/crictl/container_stats.go b/cmd/crictl/container_stats.go index fe416acdb0..dff28e6324 100644 --- a/cmd/crictl/container_stats.go +++ b/cmd/crictl/container_stats.go @@ -164,9 +164,9 @@ func (d containerStatsDisplayer) displayStats(ctx context.Context, client intern return err } switch d.opts.output { - case "json": + case outputTypeJSON: return outputProtobufObjAsJSON(r) - case "yaml": + case outputTypeYAML: return outputProtobufObjAsYAML(r) } oldStats := make(map[string]*pb.ContainerStats) diff --git a/cmd/crictl/events.go b/cmd/crictl/events.go index 46c2ee00f7..4cf0df70d9 100644 --- a/cmd/crictl/events.go +++ b/cmd/crictl/events.go @@ -37,7 +37,7 @@ var eventsCommand = &cli.Command{ &cli.StringFlag{ Name: "output", Aliases: []string{"o"}, - Value: "json", + Value: outputTypeJSON, Usage: "Output format, One of: json|yaml|go-template", }, &cli.StringFlag{ @@ -51,11 +51,11 @@ var eventsCommand = &cli.Command{ } switch format := c.String("output"); format { - case "json", "yaml": + case outputTypeJSON, outputTypeYAML: if c.String("template") != "" { return fmt.Errorf("template can't be used with %q format", format) } - case "go-template": + case outputTypeGoTemplate: if err := validateTemplate(c.String(("template"))); err != nil { return fmt.Errorf("failed to parse go-template: %w", err) } diff --git a/cmd/crictl/image.go b/cmd/crictl/image.go index 738f53a05a..55a20712a6 100644 --- a/cmd/crictl/image.go +++ b/cmd/crictl/image.go @@ -210,9 +210,9 @@ var listImageCommand = &cli.Command{ } switch c.String("output") { - case "json": + case outputTypeJSON: return outputProtobufObjAsJSON(r) - case "yaml": + case outputTypeYAML: return outputProtobufObjAsYAML(r) } @@ -320,7 +320,7 @@ var imageStatusCommand = &cli.Command{ verbose := !(c.Bool("quiet")) output := c.String("output") if output == "" { // default to json output - output = "json" + output = outputTypeJSON } tmplStr := c.String("template") @@ -342,7 +342,7 @@ var imageStatusCommand = &cli.Command{ return fmt.Errorf("marshal status to JSON for %q: %w", id, err) } - if output == "table" { + if output == outputTypeTable { outputImageStatusTable(r, verbose) } else { statuses = append(statuses, statusData{json: statusJSON, info: r.Info}) @@ -528,7 +528,7 @@ var imageFsInfoCommand = &cli.Command{ output := c.String("output") if output == "" { // default to json output - output = "json" + output = outputTypeJSON } tmplStr := c.String("template") @@ -541,7 +541,7 @@ var imageFsInfoCommand = &cli.Command{ return fmt.Errorf("marshal filesystem info to json: %w", err) } - if output == "table" { + if output == outputTypeTable { ouputImageFsInfoTable(r) } else { return outputStatusData([]statusData{{json: status}}, output, tmplStr) diff --git a/cmd/crictl/info.go b/cmd/crictl/info.go index 1d3d460f9e..38066a11e3 100644 --- a/cmd/crictl/info.go +++ b/cmd/crictl/info.go @@ -36,7 +36,7 @@ var runtimeStatusCommand = &cli.Command{ &cli.StringFlag{ Name: "output", Aliases: []string{"o"}, - Value: "json", + Value: outputTypeJSON, Usage: "Output format, One of: json|yaml|go-template", }, &cli.BoolFlag{ diff --git a/cmd/crictl/main.go b/cmd/crictl/main.go index 434a05d91b..aa9d218df3 100644 --- a/cmd/crictl/main.go +++ b/cmd/crictl/main.go @@ -37,6 +37,7 @@ import ( "k8s.io/klog/v2" "sigs.k8s.io/cri-tools/pkg/common" + "sigs.k8s.io/cri-tools/pkg/framework" "sigs.k8s.io/cri-tools/pkg/tracing" "sigs.k8s.io/cri-tools/pkg/version" ) @@ -165,7 +166,7 @@ func getTimeout(timeDuration time.Duration) time.Duration { return timeDuration } - if runtime.GOOS == "windows" { + if runtime.GOOS == framework.OSWindows { return defaultTimeoutWindows } diff --git a/cmd/crictl/pod_metrics.go b/cmd/crictl/pod_metrics.go index 6aba475e81..df418685b4 100644 --- a/cmd/crictl/pod_metrics.go +++ b/cmd/crictl/pod_metrics.go @@ -66,7 +66,7 @@ var podMetricsCommand = &cli.Command{ } switch opts.output { - case "json", "yaml", "": + case outputTypeJSON, outputTypeYAML, "": default: return cli.ShowSubcommandHelp(c) } @@ -103,9 +103,9 @@ func (p *podMetricsDisplayer) displayPodMetrics( response := &pb.ListPodSandboxMetricsResponse{PodMetrics: metrics} switch p.opts.output { - case "json", "": + case outputTypeJSON, "": return outputProtobufObjAsJSON(response) - case "yaml": + case outputTypeYAML: return outputProtobufObjAsYAML(response) } return nil diff --git a/cmd/crictl/pod_stats.go b/cmd/crictl/pod_stats.go index 1058740cd8..6c0599c987 100644 --- a/cmd/crictl/pod_stats.go +++ b/cmd/crictl/pod_stats.go @@ -158,9 +158,9 @@ func (d *podStatsDisplayer) displayPodStats( response := &pb.ListPodSandboxStatsResponse{Stats: stats} switch d.opts.output { - case "json": + case outputTypeJSON: return outputProtobufObjAsJSON(response) - case "yaml": + case outputTypeYAML: return outputProtobufObjAsYAML(response) } diff --git a/cmd/crictl/sandbox.go b/cmd/crictl/sandbox.go index 264b113985..80bc0799c4 100644 --- a/cmd/crictl/sandbox.go +++ b/cmd/crictl/sandbox.go @@ -287,7 +287,7 @@ var listPodCommand = &cli.Command{ Name: "output", Aliases: []string{"o"}, Usage: "Output format, One of: json|yaml|table", - Value: "table", + Value: outputTypeTable, }, &cli.BoolFlag{ Name: "latest", @@ -406,7 +406,7 @@ func marshalPodSandboxStatus(ps *pb.PodSandboxStatus) (string, error) { func podSandboxStatus(client internalapi.RuntimeService, ids []string, output string, quiet bool, tmplStr string) error { verbose := !(quiet) if output == "" { // default to json output - output = "json" + output = outputTypeJSON } if len(ids) == 0 { return errors.New("ID cannot be empty") @@ -433,7 +433,7 @@ func podSandboxStatus(client internalapi.RuntimeService, ids []string, output st return fmt.Errorf("marshal pod sandbox status: %w", err) } - if output == "table" { + if output == outputTypeTable { outputPodSandboxStatusTable(r, verbose) } else { statuses = append(statuses, statusData{json: statusJSON, info: r.Info}) @@ -523,11 +523,11 @@ func ListPodSandboxes(client internalapi.RuntimeService, opts *listOptions) erro r = getSandboxesList(r, opts) switch opts.output { - case "json": + case outputTypeJSON: return outputProtobufObjAsJSON(&pb.ListPodSandboxResponse{Items: r}) - case "yaml": + case outputTypeYAML: return outputProtobufObjAsYAML(&pb.ListPodSandboxResponse{Items: r}) - case "table": + case outputTypeTable: // continue; output will be generated after the switch block ends. default: return fmt.Errorf("unsupported output format %q", opts.output) diff --git a/cmd/crictl/templates.go b/cmd/crictl/templates.go index 81b2f7ee72..b8ed4491b9 100644 --- a/cmd/crictl/templates.go +++ b/cmd/crictl/templates.go @@ -32,10 +32,10 @@ func builtinTmplFuncs() template.FuncMap { l := cases.Lower(language.Und) u := cases.Upper(language.Und) return template.FuncMap{ - "json": jsonBuiltinTmplFunc, - "title": t.String, - "lower": l.String, - "upper": u.String, + outputTypeJSON: jsonBuiltinTmplFunc, + "title": t.String, + "lower": l.String, + "upper": u.String, } } diff --git a/cmd/crictl/util.go b/cmd/crictl/util.go index 75a1e9df6f..18f15febd0 100644 --- a/cmd/crictl/util.go +++ b/cmd/crictl/util.go @@ -44,6 +44,11 @@ import ( const ( // truncatedImageIDLen is the truncated length of imageID. truncatedIDLen = 13 + + outputTypeJSON = "json" + outputTypeYAML = "yaml" + outputTypeTable = "table" + outputTypeGoTemplate = "go-template" ) var ( @@ -348,19 +353,19 @@ func outputStatusData(statuses []statusData, format, tmplStr string) (err error) } switch format { - case "yaml": + case outputTypeYAML: yamlInfo, err := yaml.JSONToYAML(jsonResult) if err != nil { return fmt.Errorf("JSON result to YAML: %w", err) } fmt.Println(string(yamlInfo)) - case "json": + case outputTypeJSON: var output bytes.Buffer if err := json.Indent(&output, jsonResult, "", " "); err != nil { return fmt.Errorf("indent JSON result: %w", err) } fmt.Println(output.String()) - case "go-template": + case outputTypeGoTemplate: output, err := tmplExecuteRawJSON(tmplStr, string(jsonResult)) if err != nil { return fmt.Errorf("execute template: %w", err) @@ -375,17 +380,17 @@ func outputStatusData(statuses []statusData, format, tmplStr string) (err error) func outputEvent(event protoiface.MessageV1, format, tmplStr string) error { switch format { - case "yaml": + case outputTypeYAML: err := outputProtobufObjAsYAML(event) if err != nil { return err } - case "json": + case outputTypeJSON: err := outputProtobufObjAsJSON(event) if err != nil { return err } - case "go-template": + case outputTypeGoTemplate: jsonEvent, err := protobufObjectToJSON(event) if err != nil { return err @@ -440,7 +445,7 @@ func marshalMapInOrder(m map[string]interface{}, t interface{}) (string, error) // jsonFieldFromTag gets json field name from field tag. func jsonFieldFromTag(tag reflect.StructTag) string { - field := strings.Split(tag.Get("json"), ",")[0] + field := strings.Split(tag.Get(outputTypeJSON), ",")[0] for _, f := range strings.Split(tag.Get("protobuf"), ",") { if !strings.HasPrefix(f, "json=") { continue diff --git a/cmd/crictl/util_test.go b/cmd/crictl/util_test.go index 9a28c86c07..b771d0479f 100644 --- a/cmd/crictl/util_test.go +++ b/cmd/crictl/util_test.go @@ -117,7 +117,7 @@ func TestOutputStatusData(t *testing.T) { status: statusResponse, handlers: handlerResponse, info: map[string]string{"key1": `{"foo": "bar"}`, "key2": `{"bar": "baz"}`}, - format: "yaml", + format: outputTypeYAML, tmplStr: "", expectedOut: "key1:\n foo: bar\nkey2:\n bar: baz\nruntimeHandlers:\n- features:\n recursive_read_only_mounts: true\n name: runc\n- features:\n recursive_read_only_mounts: true\n user_namespaces: true\n name: crun\nstatus:\n conditions:\n - message: no network config found in C:\\Program Files\n reason: NetworkPluginNotReady\n status: false\n type: NetworkReady", }, @@ -125,7 +125,7 @@ func TestOutputStatusData(t *testing.T) { name: "YAML format with empty status response", status: emptyResponse, handlers: handlerResponse, - format: "yaml", + format: outputTypeYAML, tmplStr: "", expectedOut: "runtimeHandlers:\n- features:\n recursive_read_only_mounts: true\n name: runc\n- features:\n recursive_read_only_mounts: true\n user_namespaces: true\n name: crun", }, @@ -133,7 +133,7 @@ func TestOutputStatusData(t *testing.T) { name: "YAML format with empty handlers response", status: statusResponse, handlers: emptyResponse, - format: "yaml", + format: outputTypeYAML, tmplStr: "", expectedOut: "status:\n conditions:\n - message: no network config found in C:\\Program Files\n reason: NetworkPluginNotReady\n status: false\n type: NetworkReady", }, @@ -141,7 +141,7 @@ func TestOutputStatusData(t *testing.T) { name: "JSON format", status: statusResponse, handlers: handlerResponse, - format: "json", + format: outputTypeJSON, tmplStr: "", expectedOut: "{\n \"runtimeHandlers\": [\n {\n \"features\": {\n \"recursive_read_only_mounts\": true\n },\n \"name\": \"runc\"\n },\n {\n \"features\": {\n \"recursive_read_only_mounts\": true,\n \"user_namespaces\": true\n },\n \"name\": \"crun\"\n }\n ],\n \"status\": {\n \"conditions\": [\n {\n \"message\": \"no network config found in C:\\\\Program Files\",\n \"reason\": \"NetworkPluginNotReady\",\n \"status\": false,\n \"type\": \"NetworkReady\"\n }\n ]\n }\n}", }, @@ -149,7 +149,7 @@ func TestOutputStatusData(t *testing.T) { name: "Go template format", status: statusResponse, handlers: handlerResponse, - format: "go-template", + format: outputTypeGoTemplate, tmplStr: `NetworkReady: {{ (index .status.conditions 0).status }}`, expectedOut: "NetworkReady: false", }, diff --git a/pkg/common/file.go b/pkg/common/file.go index eed96ddc27..cc6f78eef4 100644 --- a/pkg/common/file.go +++ b/pkg/common/file.go @@ -173,23 +173,29 @@ func setConfigOption(configName, configValue string, yamlData *yaml.Node) { // These ScalarNodes help preserve comments associated with // the YAML entry if !foundOption { + const ( + tagPrefix = "!!" + tagStr = tagPrefix + "str" + tagBool = tagPrefix + "bool" + tagInt = tagPrefix + "int" + ) name := &yaml.Node{ Kind: yaml.ScalarNode, Value: configName, - Tag: "!!str", + Tag: tagStr, } var tagType string switch configName { case "timeout": - tagType = "!!int" + tagType = tagInt case "debug": - tagType = "!!bool" + tagType = tagBool case "pull-image-on-create": - tagType = "!!bool" + tagType = tagBool case "disable-pull-on-run": - tagType = "!!bool" + tagType = tagBool default: - tagType = "!!str" + tagType = tagStr } value := &yaml.Node{ diff --git a/pkg/framework/test_context.go b/pkg/framework/test_context.go index 28f05df46f..3190ffa032 100644 --- a/pkg/framework/test_context.go +++ b/pkg/framework/test_context.go @@ -126,6 +126,8 @@ const ( ContainerdSockPathUnix = "unix:///run/containerd/containerd.sock" ContainerdSockPathWindows = "npipe:////./pipe/containerd-containerd" + + OSWindows = "windows" ) // RegisterFlags registers flags to e2e test suites. @@ -149,7 +151,7 @@ func RegisterFlags() { svcaddr := ContainerdSockPathUnix defaultConfigPath := "/etc/crictl.yaml" - if runtime.GOOS == "windows" { + if runtime.GOOS == OSWindows { svcaddr = ContainerdSockPathWindows defaultConfigPath = filepath.Join(os.Getenv("USERPROFILE"), ".crictl", "crictl.yaml") } @@ -161,7 +163,7 @@ func RegisterFlags() { flag.StringVar(&benchamrkSettingFilePath, "benchmarking-params-file", "", "Optional path to a YAML file specifying benchmarking configuration options.") flag.StringVar(&TestContext.BenchmarkingOutputDir, "benchmarking-output-dir", "", "Optional path to a directory in which benchmarking data should be placed.") - if runtime.GOOS == "windows" { + if runtime.GOOS == OSWindows { flag.BoolVar(&TestContext.IsLcow, "lcow", false, "Run Linux container on Windows tests instead of Windows container tests") } else { TestContext.IsLcow = false diff --git a/pkg/framework/util.go b/pkg/framework/util.go index f5bac2ffc4..15fd542329 100644 --- a/pkg/framework/util.go +++ b/pkg/framework/util.go @@ -96,7 +96,7 @@ const ( // Set the constants based on operating system and flags. var _ = BeforeSuite(func() { - if runtime.GOOS != "windows" || TestContext.IsLcow { + if runtime.GOOS != OSWindows || TestContext.IsLcow { DefaultPodLabels = DefaultLinuxPodLabels DefaultContainerCommand = DefaultLinuxContainerCommand DefaultPauseCommand = DefaultLinuxPauseCommand diff --git a/pkg/validate/consts.go b/pkg/validate/consts.go index b4eca7e064..eec004add6 100644 --- a/pkg/validate/consts.go +++ b/pkg/validate/consts.go @@ -61,7 +61,7 @@ var ( ) var _ = framework.AddBeforeSuiteCallback(func() { - if runtime.GOOS != "windows" || framework.TestContext.IsLcow { + if runtime.GOOS != framework.OSWindows || framework.TestContext.IsLcow { echoHelloCmd = echoHelloLinuxCmd sleepCmd = sleepLinuxCmd checkSleepCmd = checkSleepLinuxCmd @@ -162,7 +162,7 @@ var ( ) var _ = framework.AddBeforeSuiteCallback(func() { - if runtime.GOOS != "windows" || framework.TestContext.IsLcow { + if runtime.GOOS != framework.OSWindows || framework.TestContext.IsLcow { testImageWithoutTag = testLinuxImageWithoutTag testImageWithTag = testLinuxImageWithTag testImageWithDigest = testLinuxImageWithDigest @@ -234,7 +234,7 @@ var ( ) var _ = framework.AddBeforeSuiteCallback(func() { - if runtime.GOOS != "windows" || framework.TestContext.IsLcow { + if runtime.GOOS != framework.OSWindows || framework.TestContext.IsLcow { webServerImage = webServerLinuxImage hostNetWebServerImage = hostNetWebServerLinuxImage getDNSConfigCmd = getDNSConfigLinuxCmd @@ -270,7 +270,7 @@ const ( var attachEchoHelloOutput string var _ = framework.AddBeforeSuiteCallback(func() { - if runtime.GOOS != "windows" || framework.TestContext.IsLcow { + if runtime.GOOS != framework.OSWindows || framework.TestContext.IsLcow { attachEchoHelloOutput = attachEchoHelloLinuxOutput } else { attachEchoHelloOutput = attachEchoHelloWindowsOutput diff --git a/pkg/validate/image.go b/pkg/validate/image.go index 1b3687bec0..a520d3998b 100644 --- a/pkg/validate/image.go +++ b/pkg/validate/image.go @@ -99,7 +99,7 @@ var _ = framework.KubeDescribe("Image Manager", func() { testRemoveImage(c, imageName) }) - if runtime.GOOS != "windows" || framework.TestContext.IsLcow { + if runtime.GOOS != framework.OSWindows || framework.TestContext.IsLcow { It("image status get image fields should not have Uid|Username empty [Conformance]", func() { for _, item := range []struct { description string diff --git a/pkg/validate/security_context_linux.go b/pkg/validate/security_context_linux.go index 3b43c085c8..defee3468c 100644 --- a/pkg/validate/security_context_linux.go +++ b/pkg/validate/security_context_linux.go @@ -89,7 +89,7 @@ var _ = framework.KubeDescribe("Security Context", func() { podID, podConfig = createNamespacePodSandbox(rc, namespaceOption, podSandboxName, "") By("create nginx container") - prefix := "nginx-container-" + prefix := "nginx-container-hostpid-" nginxContainerName := prefix + framework.NewUUID() containerID, _ := createNamespaceContainer(rc, ic, podID, podConfig, nginxContainerName, nginxContainerImage, namespaceOption, nil, "") @@ -190,7 +190,7 @@ var _ = framework.KubeDescribe("Security Context", func() { podID, podConfig = createNamespacePodSandbox(rc, namespaceOption, podSandboxName, "") By("create nginx container") - prefix := "nginx-container-" + prefix := "nginx-container-process-namespace-" containerName := prefix + framework.NewUUID() containerID, _ := createNamespaceContainer(rc, ic, podID, podConfig, containerName, nginxContainerImage, namespaceOption, nil, "") @@ -216,7 +216,7 @@ var _ = framework.KubeDescribe("Security Context", func() { podID, podConfig = createNamespacePodSandbox(rc, namespaceOption, podSandboxName, "") By("create nginx container") - prefix := "nginx-container-" + prefix := "nginx-container-pid-" containerName := prefix + framework.NewUUID() containerID, _ := createNamespaceContainer(rc, ic, podID, podConfig, containerName, nginxContainerImage, namespaceOption, nil, "")