From ec0e56c9eec8627fd921f23d43f374e30db694ca Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Mon, 25 Jan 2021 15:21:30 +0200 Subject: [PATCH 01/11] add gosec to verify target this will ensure code is always secured and does not include security vunerabilities. this will be verified by running gosec on all our sources added placeholder for future gosec configs in gosec.conf.json --- Makefile | 7 ++++++- gosec.conf.json | 1 + hack/gosec.sh | 9 +++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 gosec.conf.json create mode 100755 hack/gosec.sh diff --git a/Makefile b/Makefile index fba629c59..80ea3bf4f 100644 --- a/Makefile +++ b/Makefile @@ -283,6 +283,11 @@ render-command-tests: dist unittests: hack/unittests.sh +.PHONY: gosec +gosec: + @echo "Running gosec" + hack/gosec.sh + .PHONY: gofmt gofmt: @echo "Running gofmt" @@ -304,7 +309,7 @@ generate: deps-update gofmt manifests generate-code generate-latest-dev-csv gene @echo .PHONY: verify -verify: golint govet generate +verify: golint govet gosec generate @echo "Verifying that all code is committed after updating deps and formatting and generating code" hack/verify-generated.sh diff --git a/gosec.conf.json b/gosec.conf.json new file mode 100644 index 000000000..0967ef424 --- /dev/null +++ b/gosec.conf.json @@ -0,0 +1 @@ +{} diff --git a/hack/gosec.sh b/hack/gosec.sh new file mode 100755 index 000000000..93ea9fa27 --- /dev/null +++ b/hack/gosec.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +which gosec +if [ $? -ne 0 ]; then + echo "Downloading gosec tool" + go get -u github.com/securego/gosec/v2/cmd/gosec +fi + +time gosec -conf gosec.conf.json ./... From ed45e68394a3b9529172369f6f0a9e6c82179bfe Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Sat, 18 Sep 2021 23:14:13 +0300 Subject: [PATCH 02/11] fix CWE-22 applying filepath.Clean, will make sure filename is not abused --- .../1_performance-profile_creator/ppc.go | 6 +++--- functests/0_config/config.go | 3 ++- pkg/cmd/render/render.go | 2 +- .../components/machineconfig/machineconfig.go | 6 +++--- pkg/controller/performanceprofile/components/tuned/tuned.go | 3 ++- pkg/profilecreator/profilecreator.go | 4 ++-- pkg/utils/csvtools/csvtools.go | 3 ++- tools/csv-processor/csv-processor.go | 2 +- 8 files changed, 16 insertions(+), 13 deletions(-) diff --git a/functests-performance-profile-creator/1_performance-profile_creator/ppc.go b/functests-performance-profile-creator/1_performance-profile_creator/ppc.go index 7b845aa7f..e352c8cdd 100644 --- a/functests-performance-profile-creator/1_performance-profile_creator/ppc.go +++ b/functests-performance-profile-creator/1_performance-profile_creator/ppc.go @@ -77,7 +77,7 @@ var _ = Describe("[rfe_id:OCP-38968][ppc] Performance Profile Creator", func() { err = yaml.Unmarshal(out, profile) Expect(err).To(BeNil(), "failed to unmarshal the output yaml for '%s': %v", expectedProfilePath, err) - bytes, err := ioutil.ReadFile(expectedProfilePath) + bytes, err := ioutil.ReadFile(filepath.Clean(expectedProfilePath)) Expect(err).To(BeNil(), "failed to read the expected yaml for '%s': %v", expectedProfilePath, err) expectedProfile := &performancev2.PerformanceProfile{} @@ -107,7 +107,7 @@ var _ = Describe("[rfe_id:OCP-38968][ppc] Performance Profile Creator", func() { err = json.Unmarshal(out, &cInfo) Expect(err).To(BeNil(), "failed to unmarshal the output json for %q: %v", path, err) expectedClusterInfoPath := filepath.Join(expectedInfoPath, fmt.Sprintf("%s.json", name)) - bytes, err := ioutil.ReadFile(expectedClusterInfoPath) + bytes, err := ioutil.ReadFile(filepath.Clean(expectedClusterInfoPath)) Expect(err).To(BeNil(), "failed to read the expected json for %q: %v", expectedClusterInfoPath, err) var expectedInfo cmd.ClusterInfo @@ -219,7 +219,7 @@ func getExpectedProfiles(expectedProfilesPath string, mustGatherDirs map[string] } fullFilePath := filepath.Join(expectedProfilesPath, file.Name()) - bytes, err := ioutil.ReadFile(fullFilePath) + bytes, err := ioutil.ReadFile(filepath.Clean(fullFilePath)) Expect(err).To(BeNil(), "failed to read the ppc params file for '%s': %v", fullFilePath, err) var ppcArgs cmd.ProfileCreatorArgs diff --git a/functests/0_config/config.go b/functests/0_config/config.go index a2a1babe5..d4cb6a6e0 100644 --- a/functests/0_config/config.go +++ b/functests/0_config/config.go @@ -5,6 +5,7 @@ import ( "fmt" "io/ioutil" "os" + "path/filepath" "time" . "github.com/onsi/ginkgo" @@ -118,7 +119,7 @@ func externalPerformanceProfile(performanceManifest string) (*performancev2.Perf performancev2.AddToScheme(performanceScheme) decode := serializer.NewCodecFactory(performanceScheme).UniversalDeserializer().Decode - manifest, err := ioutil.ReadFile(performanceManifest) + manifest, err := ioutil.ReadFile(filepath.Clean(performanceManifest)) if err != nil { return nil, fmt.Errorf("Failed to read %s file", performanceManifest) } diff --git a/pkg/cmd/render/render.go b/pkg/cmd/render/render.go index e1885c395..e2eac4b4e 100644 --- a/pkg/cmd/render/render.go +++ b/pkg/cmd/render/render.go @@ -121,7 +121,7 @@ func (r *renderOpts) Validate() error { func (r *renderOpts) Run() error { for _, pp := range r.performanceProfileInputFiles { - b, err := ioutil.ReadFile(pp) + b, err := ioutil.ReadFile(filepath.Clean(pp)) if err != nil { return err } diff --git a/pkg/controller/performanceprofile/components/machineconfig/machineconfig.go b/pkg/controller/performanceprofile/components/machineconfig/machineconfig.go index 6c5d95d94..96d98f54e 100644 --- a/pkg/controller/performanceprofile/components/machineconfig/machineconfig.go +++ b/pkg/controller/performanceprofile/components/machineconfig/machineconfig.go @@ -260,7 +260,7 @@ func getSystemdContent(options []*unit.UnitOption) (string, error) { // GetOCIHooksConfigContent reads and returns the content of the OCI hook file func GetOCIHooksConfigContent(configFile string, profile *performancev2.PerformanceProfile) ([]byte, error) { - content, err := ioutil.ReadFile(configFile) + content, err := ioutil.ReadFile(filepath.Clean(configFile)) if err != nil { return nil, err } @@ -355,7 +355,7 @@ func addCrioConfigSnippet(profile *performancev2.PerformanceProfile, src string) templateArgs[templateReservedCpus] = string(*profile.Spec.CPU.Reserved) } - content, err := ioutil.ReadFile(src) + content, err := ioutil.ReadFile(filepath.Clean(src)) if err != nil { return nil, err } @@ -370,7 +370,7 @@ func addCrioConfigSnippet(profile *performancev2.PerformanceProfile, src string) } func addFile(ignitionConfig *igntypes.Config, src string, dst string, mode *int) error { - content, err := ioutil.ReadFile(src) + content, err := ioutil.ReadFile(filepath.Clean(src)) if err != nil { return err } diff --git a/pkg/controller/performanceprofile/components/tuned/tuned.go b/pkg/controller/performanceprofile/components/tuned/tuned.go index a08f5ffcd..397b6f254 100644 --- a/pkg/controller/performanceprofile/components/tuned/tuned.go +++ b/pkg/controller/performanceprofile/components/tuned/tuned.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "io/ioutil" + "path/filepath" "strconv" "strings" "text/template" @@ -195,7 +196,7 @@ func getProfilePath(name string, assetsDir string) string { } func getProfileData(profileOperatorlPath string, data interface{}) (string, error) { - profileContent, err := ioutil.ReadFile(profileOperatorlPath) + profileContent, err := ioutil.ReadFile(filepath.Clean(profileOperatorlPath)) if err != nil { return "", err } diff --git a/pkg/profilecreator/profilecreator.go b/pkg/profilecreator/profilecreator.go index 6a5cbf417..7c7acea94 100644 --- a/pkg/profilecreator/profilecreator.go +++ b/pkg/profilecreator/profilecreator.go @@ -112,7 +112,7 @@ func getNode(mustGatherDirPath, nodeName string) (*v1.Node, error) { return nil, fmt.Errorf("failed to get MachineConfigPool for %s: %v", nodeName, err) } - src, err := os.Open(path) + src, err := os.Open(filepath.Clean(path)) if err != nil { return nil, fmt.Errorf("failed to open %q: %v", path, err) } @@ -196,7 +196,7 @@ func GetMCP(mustGatherDirPath, mcpName string) (*machineconfigv1.MachineConfigPo return nil, fmt.Errorf("failed to obtain MachineConfigPool, mcp:%s does not exist: %v", mcpName, err) } - src, err := os.Open(mcpPath) + src, err := os.Open(filepath.Clean(mcpPath)) if err != nil { return nil, fmt.Errorf("failed to open %q: %v", mcpPath, err) } diff --git a/pkg/utils/csvtools/csvtools.go b/pkg/utils/csvtools/csvtools.go index fbe440974..e678de2fb 100644 --- a/pkg/utils/csvtools/csvtools.go +++ b/pkg/utils/csvtools/csvtools.go @@ -4,6 +4,7 @@ import ( "encoding/json" "io" "io/ioutil" + "path/filepath" "strings" yaml "github.com/ghodss/yaml" @@ -40,7 +41,7 @@ type CSVStrategySpec struct { // UnmarshalCSV decodes a YAML file, by path, and returns a CSV func UnmarshalCSV(filePath string) *csvv1.ClusterServiceVersion { - bytes, err := ioutil.ReadFile(filePath) + bytes, err := ioutil.ReadFile(filepath.Clean(filePath)) if err != nil { panic(err) } diff --git a/tools/csv-processor/csv-processor.go b/tools/csv-processor/csv-processor.go index c5d537e01..051b6c94b 100644 --- a/tools/csv-processor/csv-processor.go +++ b/tools/csv-processor/csv-processor.go @@ -256,7 +256,7 @@ Performance Addon Operator provides the ability to enable advanced node performa } func readFileOrPanic(filename string) []byte { - data, err := ioutil.ReadFile(filename) + data, err := ioutil.ReadFile(filepath.Clean(filename)) if err != nil { panic(err) } From 89412e347a009d06df0db268cc868dda2df9b918 Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Sun, 19 Sep 2021 01:16:34 +0300 Subject: [PATCH 03/11] fix all CWE-118 issues removed all refernces to loop iterator variable --- api/v2/performanceprofile_validation.go | 2 +- controllers/status.go | 4 +- functests/1_performance/netqueues.go | 18 +++-- functests/1_performance/performance.go | 76 +++++++++---------- .../2_performance_update/updating_profile.go | 22 +++--- functests/utils/discovery/discovery.go | 4 +- 6 files changed, 66 insertions(+), 60 deletions(-) diff --git a/api/v2/performanceprofile_validation.go b/api/v2/performanceprofile_validation.go index 94b5d9b6b..d544eeade 100644 --- a/api/v2/performanceprofile_validation.go +++ b/api/v2/performanceprofile_validation.go @@ -201,7 +201,7 @@ func (r *PerformanceProfile) validateHugePages() field.ErrorList { allErrs = append(allErrs, field.Invalid(field.NewPath("spec.hugepages.pages"), r.Spec.HugePages.Pages, fmt.Sprintf("the page size should be equal to %q or %q", hugepagesSize1G, hugepagesSize2M))) } - allErrs = append(allErrs, r.validatePageDuplication(&page, r.Spec.HugePages.Pages[i+1:])...) + allErrs = append(allErrs, r.validatePageDuplication(&r.Spec.HugePages.Pages[i], r.Spec.HugePages.Pages[i+1:])...) } return allErrs diff --git a/controllers/status.go b/controllers/status.go index 2389e8dc1..1f5e2edbb 100644 --- a/controllers/status.go +++ b/controllers/status.go @@ -242,10 +242,10 @@ func (r *PerformanceProfileReconciler) getTunedConditionsByProfile(profile *perf isApplied := true var tunedDegradedCondition *tunedv1.ProfileStatusCondition - for _, condition := range tunedProfile.Status.Conditions { + for i, condition := range tunedProfile.Status.Conditions { if (condition.Type == tunedv1.TunedDegraded) && condition.Status == corev1.ConditionTrue { isDegraded = true - tunedDegradedCondition = &condition + tunedDegradedCondition = &tunedProfile.Status.Conditions[i] } if (condition.Type == tunedv1.TunedProfileApplied) && condition.Status == corev1.ConditionFalse { diff --git a/functests/1_performance/netqueues.go b/functests/1_performance/netqueues.go index 569148ce0..00411b0d9 100644 --- a/functests/1_performance/netqueues.go +++ b/functests/1_performance/netqueues.go @@ -164,8 +164,8 @@ var _ = Describe("[ref_id: 40307][pao]Resizing Network Queues", func() { tunedCmd := []string{"bash", "-c", fmt.Sprintf("cat /etc/tuned/openshift-node-performance-%s/tuned.conf | grep devices_udev_regex", performanceProfileName)} - for _, node := range workerRTNodes { - tunedPod := nodes.TunedForNode(&node, RunningOnSingleNode) + for i := range workerRTNodes { + tunedPod := nodes.TunedForNode(&workerRTNodes[i], RunningOnSingleNode) out, err := pods.WaitForPodOutput(testclient.K8sClient, tunedPod, tunedCmd) deviceExists := strings.ContainsAny(string(out), device) Expect(deviceExists).To(BeTrue()) @@ -214,8 +214,8 @@ var _ = Describe("[ref_id: 40307][pao]Resizing Network Queues", func() { tunedCmd := []string{"bash", "-c", fmt.Sprintf("cat /etc/tuned/openshift-node-performance-%s/tuned.conf | grep devices_udev_regex", performanceProfileName)} - for _, node := range workerRTNodes { - tunedPod := nodes.TunedForNode(&node, RunningOnSingleNode) + for i := range workerRTNodes { + tunedPod := nodes.TunedForNode(&workerRTNodes[i], RunningOnSingleNode) out, err := pods.WaitForPodOutput(testclient.K8sClient, tunedPod, tunedCmd) deviceExists := strings.ContainsAny(string(out), device) Expect(deviceExists).To(BeTrue()) @@ -269,9 +269,15 @@ var _ = Describe("[ref_id: 40307][pao]Resizing Network Queues", func() { } //Verify the tuned profile is created on the worker-cnf nodes: tunedCmd := []string{"bash", "-c", +<<<<<<< HEAD fmt.Sprintf("cat /etc/tuned/openshift-node-performance-%s/tuned.conf | grep devices_udev_regex", performanceProfileName)} for _, node := range workerRTNodes { tunedPod := nodes.TunedForNode(&node, RunningOnSingleNode) +======= + fmt.Sprintf("cat /etc/tuned/openshift-node-performance-performance/tuned.conf | grep devices_udev_regex")} + for i := range workerRTNodes { + tunedPod := nodes.TunedForNode(&workerRTNodes[i], RunningOnSingleNode) +>>>>>>> e00c45ec (fix all CWE-118 issues) out, err := pods.WaitForPodOutput(testclient.K8sClient, tunedPod, tunedCmd) deviceExists := strings.ContainsAny(string(out), device) Expect(deviceExists).To(BeTrue()) @@ -296,8 +302,8 @@ func checkDeviceSupport(workernodes []corev1.Node, devices map[string][]int) err cmdGetPhysicalDevices := []string{"find", "/sys/class/net", "-type", "l", "-not", "-lname", "*virtual*", "-printf", "%f "} var channelCurrentCombined int var err error - for _, node := range workernodes { - tunedPod := nodes.TunedForNode(&node, RunningOnSingleNode) + for i, node := range workernodes { + tunedPod := nodes.TunedForNode(&workernodes[i], RunningOnSingleNode) phyDevs, err := pods.WaitForPodOutput(testclient.K8sClient, tunedPod, cmdGetPhysicalDevices) Expect(err).ToNot(HaveOccurred()) for _, d := range strings.Split(string(phyDevs), " ") { diff --git a/functests/1_performance/performance.go b/functests/1_performance/performance.go index a95b78115..497e028b6 100644 --- a/functests/1_performance/performance.go +++ b/functests/1_performance/performance.go @@ -148,8 +148,8 @@ var _ = Describe("[rfe_id:27368][performance]", func() { }) It("[test_id:37127] Node should point to right tuned profile", func() { - for _, node := range workerRTNodes { - tuned := nodes.TunedForNode(&node, RunningOnSingleNode) + for i := range workerRTNodes { + tuned := nodes.TunedForNode(&workerRTNodes[i], RunningOnSingleNode) activeProfile, err := pods.WaitForPodOutput(testclient.K8sClient, tuned, []string{"cat", "/etc/tuned/active_profile"}) Expect(err).ToNot(HaveOccurred(), "Error getting the tuned active profile") activeProfileName := string(activeProfile) @@ -161,8 +161,8 @@ var _ = Describe("[rfe_id:27368][performance]", func() { Context("Pre boot tuning adjusted by tuned ", func() { It("[test_id:31198] Should set CPU affinity kernel argument", func() { - for _, node := range workerRTNodes { - cmdline, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"cat", "/proc/cmdline"}) + for i := range workerRTNodes { + cmdline, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"cat", "/proc/cmdline"}) Expect(err).ToNot(HaveOccurred()) // since systemd.cpu_affinity is calculated on node level using tuned we can check only the key in this context. Expect(string(cmdline)).To(ContainSubstring("systemd.cpu_affinity=")) @@ -170,8 +170,8 @@ var _ = Describe("[rfe_id:27368][performance]", func() { }) It("[test_id:32702] Should set CPU isolcpu's kernel argument managed_irq flag", func() { - for _, node := range workerRTNodes { - cmdline, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"cat", "/proc/cmdline"}) + for i := range workerRTNodes { + cmdline, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"cat", "/proc/cmdline"}) Expect(err).ToNot(HaveOccurred()) if profile.Spec.CPU.BalanceIsolated != nil && *profile.Spec.CPU.BalanceIsolated == false { Expect(string(cmdline)).To(ContainSubstring("isolcpus=domain,managed_irq,")) @@ -182,9 +182,9 @@ var _ = Describe("[rfe_id:27368][performance]", func() { }) It("[test_id:27081][crit:high][vendor:cnf-qe@redhat.com][level:acceptance] Should set workqueue CPU mask", func() { - for _, node := range workerRTNodes { + for i, node := range workerRTNodes { By(fmt.Sprintf("Getting tuned.non_isolcpus kernel argument on %q", node.Name)) - cmdline, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"cat", "/proc/cmdline"}) + cmdline, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"cat", "/proc/cmdline"}) Expect(err).ToNot(HaveOccurred()) re := regexp.MustCompile(`tuned.non_isolcpus=\S+`) nonIsolcpusFullArgument := re.FindString(string(cmdline)) @@ -205,13 +205,13 @@ var _ = Describe("[rfe_id:27368][performance]", func() { } By(fmt.Sprintf("Getting the virtual workqueue mask (/sys/devices/virtual/workqueue/cpumask) on %q", node.Name)) - workqueueMaskData, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"cat", "/sys/devices/virtual/workqueue/cpumask"}) + workqueueMaskData, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"cat", "/sys/devices/virtual/workqueue/cpumask"}) Expect(err).ToNot(HaveOccurred()) workqueueMask := getTrimmedMaskFromData("virtual", workqueueMaskData) expectMasksEqual(nonIsolcpusMaskNoDelimiters, workqueueMask) By(fmt.Sprintf("Getting the writeback workqueue mask (/sys/bus/workqueue/devices/writeback/cpumask) on %q", node.Name)) - workqueueWritebackMaskData, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"cat", "/sys/bus/workqueue/devices/writeback/cpumask"}) + workqueueWritebackMaskData, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"cat", "/sys/bus/workqueue/devices/writeback/cpumask"}) Expect(err).ToNot(HaveOccurred()) workqueueWritebackMask := getTrimmedMaskFromData("workqueue", workqueueWritebackMaskData) expectMasksEqual(nonIsolcpusMaskNoDelimiters, workqueueWritebackMask) @@ -219,52 +219,52 @@ var _ = Describe("[rfe_id:27368][performance]", func() { }) It("[test_id:32375][crit:high][vendor:cnf-qe@redhat.com][level:acceptance] initramfs should not have injected configuration", func() { - for _, node := range workerRTNodes { - rhcosId, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"awk", "-F", "/", "{printf $3}", "/rootfs/proc/cmdline"}) + for i := range workerRTNodes { + rhcosId, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"awk", "-F", "/", "{printf $3}", "/rootfs/proc/cmdline"}) Expect(err).ToNot(HaveOccurred()) - initramfsImagesPath, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"find", filepath.Join("/rootfs/boot/ostree", string(rhcosId)), "-name", "*.img"}) + initramfsImagesPath, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"find", filepath.Join("/rootfs/boot/ostree", string(rhcosId)), "-name", "*.img"}) Expect(err).ToNot(HaveOccurred()) modifiedImagePath := strings.TrimPrefix(strings.TrimSpace(string(initramfsImagesPath)), "/rootfs") - initrd, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"chroot", "/rootfs", "lsinitrd", modifiedImagePath}) + initrd, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"chroot", "/rootfs", "lsinitrd", modifiedImagePath}) Expect(err).ToNot(HaveOccurred()) Expect(string(initrd)).ShouldNot(ContainSubstring("'/etc/systemd/system.conf /etc/systemd/system.conf.d/setAffinity.conf'")) } }) It("[test_id:35363][crit:high][vendor:cnf-qe@redhat.com][level:acceptance] stalld daemon is running on the host", func() { - for _, node := range workerRTNodes { - tuned := nodes.TunedForNode(&node, RunningOnSingleNode) + for i := range workerRTNodes { + tuned := nodes.TunedForNode(&workerRTNodes[i], RunningOnSingleNode) _, err := pods.WaitForPodOutput(testclient.K8sClient, tuned, []string{"pidof", "stalld"}) Expect(err).ToNot(HaveOccurred()) } }) It("[test_id:42400][crit:medium][vendor:cnf-qe@redhat.com][level:acceptance] stalld daemon is running as sched_fifo", func() { - for _, node := range workerRTNodes { - pid, err := nodes.ExecCommandOnNode([]string{"pidof", "stalld"}, &node) + for i := range workerRTNodes { + pid, err := nodes.ExecCommandOnNode([]string{"pidof", "stalld"}, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) Expect(pid).ToNot(BeEmpty()) - sched_tasks, err := nodes.ExecCommandOnNode([]string{"chrt", "-ap", pid}, &node) + sched_tasks, err := nodes.ExecCommandOnNode([]string{"chrt", "-ap", pid}, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) Expect(sched_tasks).To(ContainSubstring("scheduling policy: SCHED_FIFO")) Expect(sched_tasks).To(ContainSubstring("scheduling priority: 10")) } }) It("[test_id:42696][crit:medium][vendor:cnf-qe@redhat.com][level:acceptance] Stalld runs in higher priority than ksoftirq and rcu{c,b}", func() { - for _, node := range workerRTNodes { - stalld_pid, err := nodes.ExecCommandOnNode([]string{"pidof", "stalld"}, &node) + for i := range workerRTNodes { + stalld_pid, err := nodes.ExecCommandOnNode([]string{"pidof", "stalld"}, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) Expect(stalld_pid).ToNot(BeEmpty()) - sched_tasks, err := nodes.ExecCommandOnNode([]string{"chrt", "-ap", stalld_pid}, &node) + sched_tasks, err := nodes.ExecCommandOnNode([]string{"chrt", "-ap", stalld_pid}, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) re := regexp.MustCompile("scheduling priority: ([0-9]+)") match := re.FindStringSubmatch(sched_tasks) stalld_prio, err := strconv.Atoi(match[1]) Expect(err).ToNot(HaveOccurred()) - ksoftirq_pid, err := nodes.ExecCommandOnNode([]string{"pgrep", "-f", "ksoftirqd", "-n"}, &node) + ksoftirq_pid, err := nodes.ExecCommandOnNode([]string{"pgrep", "-f", "ksoftirqd", "-n"}, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) Expect(ksoftirq_pid).ToNot(BeEmpty()) - sched_tasks, err = nodes.ExecCommandOnNode([]string{"chrt", "-ap", ksoftirq_pid}, &node) + sched_tasks, err = nodes.ExecCommandOnNode([]string{"chrt", "-ap", ksoftirq_pid}, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) match = re.FindStringSubmatch(sched_tasks) ksoftirq_prio, err := strconv.Atoi(match[1]) @@ -279,10 +279,10 @@ var _ = Describe("[rfe_id:27368][performance]", func() { } //rcuc/n : kthreads that are pinned to CPUs & are responsible to execute the callbacks of rcu threads . //rcub/n : are boosting kthreads ,responsible to monitor per-cpu arrays of lists of tasks that were blocked while in an rcu read-side critical sections. - rcu_pid, err := nodes.ExecCommandOnNode([]string{"pgrep", "-f", "rcu[c,b]", "-n"}, &node) + rcu_pid, err := nodes.ExecCommandOnNode([]string{"pgrep", "-f", "rcu[c,b]", "-n"}, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) Expect(rcu_pid).ToNot(BeEmpty()) - sched_tasks, err = nodes.ExecCommandOnNode([]string{"chrt", "-ap", rcu_pid}, &node) + sched_tasks, err = nodes.ExecCommandOnNode([]string{"chrt", "-ap", rcu_pid}, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) match = re.FindStringSubmatch(sched_tasks) rcu_prio, err := strconv.Atoi(match[1]) @@ -298,8 +298,8 @@ var _ = Describe("[rfe_id:27368][performance]", func() { Context("Additional kernel arguments added from perfomance profile", func() { It("[test_id:28611][crit:high][vendor:cnf-qe@redhat.com][level:acceptance] Should set additional kernel arguments on the machine", func() { if profile.Spec.AdditionalKernelArgs != nil { - for _, node := range workerRTNodes { - cmdline, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"cat", "/proc/cmdline"}) + for i := range workerRTNodes { + cmdline, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"cat", "/proc/cmdline"}) Expect(err).ToNot(HaveOccurred()) for _, arg := range profile.Spec.AdditionalKernelArgs { Expect(string(cmdline)).To(ContainSubstring(arg)) @@ -341,9 +341,9 @@ var _ = Describe("[rfe_id:27368][performance]", func() { Expect(err).ToNot(HaveOccurred()) ociHookPath := filepath.Join("/rootfs", machineconfig.OCIHooksConfigDir, machineconfig.OCIHooksConfig+".json") Expect(err).ToNot(HaveOccurred()) - for _, node := range workerRTNodes { + for i, node := range workerRTNodes { // Verify the OCI RPS hook uses the correct RPS mask - hooksConfig, err := nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"cat", ociHookPath}) + hooksConfig, err := nodes.ExecCommandOnMachineConfigDaemon(&workerRTNodes[i], []string{"cat", ociHookPath}) Expect(err).ToNot(HaveOccurred()) var hooks map[string]interface{} @@ -360,7 +360,7 @@ var _ = Describe("[rfe_id:27368][performance]", func() { // Verify the systemd RPS service uses the correct RPS mask cmd := []string{"sed", "-n", "s/^ExecStart=.*echo \\([A-Fa-f0-9]*\\) .*/\\1/p", "/rootfs/etc/systemd/system/update-rps@.service"} - serviceRPSCPUs, err := nodes.ExecCommandOnNode(cmd, &node) + serviceRPSCPUs, err := nodes.ExecCommandOnNode(cmd, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) rpsCPUs, err = components.CPUMaskToCPUSet(serviceRPSCPUs) @@ -369,7 +369,7 @@ var _ = Describe("[rfe_id:27368][performance]", func() { // Verify all host network devices have the correct RPS mask cmd = []string{"find", "/rootfs/sys/devices", "-type", "f", "-name", "rps_cpus", "-exec", "cat", "{}", ";"} - devsRPS, err := nodes.ExecCommandOnNode(cmd, &node) + devsRPS, err := nodes.ExecCommandOnNode(cmd, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) for _, devRPS := range strings.Split(devsRPS, "\n") { @@ -387,9 +387,9 @@ var _ = Describe("[rfe_id:27368][performance]", func() { err = testclient.Client.List(context.TODO(), nodePods, listOptions) Expect(err).ToNot(HaveOccurred()) - for _, pod := range nodePods.Items { + for i, pod := range nodePods.Items { cmd := []string{"find", "/sys/devices", "-type", "f", "-name", "rps_cpus", "-exec", "cat", "{}", ";"} - devsRPS, err := pods.WaitForPodOutput(testclient.K8sClient, &pod, cmd) + devsRPS, err := pods.WaitForPodOutput(testclient.K8sClient, &nodePods.Items[i], cmd) for _, devRPS := range strings.Split(strings.Trim(string(devsRPS), "\n"), "\n") { rpsCPUs, err = components.CPUMaskToCPUSet(devRPS) Expect(err).ToNot(HaveOccurred()) @@ -1331,10 +1331,10 @@ func verifyV2Conversion(v2Profile *performancev2.PerformanceProfile, v1Profile * func execSysctlOnWorkers(workerNodes []corev1.Node, sysctlMap map[string]string) { var err error var out []byte - for _, node := range workerNodes { + for i := range workerNodes { for param, expected := range sysctlMap { By(fmt.Sprintf("executing the command \"sysctl -n %s\"", param)) - out, err = nodes.ExecCommandOnMachineConfigDaemon(&node, []string{"sysctl", "-n", param}) + out, err = nodes.ExecCommandOnMachineConfigDaemon(&workerNodes[i], []string{"sysctl", "-n", param}) Expect(err).ToNot(HaveOccurred()) Expect(strings.TrimSpace(string(out))).Should(Equal(expected), "parameter %s value is not %s.", param, expected) } @@ -1361,8 +1361,8 @@ func validateTunedActiveProfile(wrknodes []corev1.Node) { } } - for _, node := range wrknodes { - tuned := nodes.TunedForNode(&node, RunningOnSingleNode) + for i := range wrknodes { + tuned := nodes.TunedForNode(&wrknodes[i], RunningOnSingleNode) tunedName := tuned.ObjectMeta.Name By(fmt.Sprintf("executing the command cat /etc/tuned/active_profile inside the pod %s", tunedName)) Eventually(func() string { diff --git a/functests/2_performance_update/updating_profile.go b/functests/2_performance_update/updating_profile.go index 34e66dc91..bc633a2a3 100644 --- a/functests/2_performance_update/updating_profile.go +++ b/functests/2_performance_update/updating_profile.go @@ -101,7 +101,7 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance expectedBannedCPUs = cpuset.NewCPUSet() } - for _, node := range workerRTNodes { + for i, node := range workerRTNodes { By(fmt.Sprintf("verifying worker node %q", node.Name)) bannedCPUs, err := nodes.BannedCPUs(node) @@ -112,10 +112,10 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance bannedCPUs, expectedBannedCPUs, node.Name) } - smpAffinitySet, err := nodes.GetDefaultSmpAffinitySet(&node) + smpAffinitySet, err := nodes.GetDefaultSmpAffinitySet(&workerRTNodes[i]) Expect(err).ToNot(HaveOccurred(), "failed to get default smp affinity") - onlineCPUsSet, err := nodes.GetOnlineCPUsSet(&node) + onlineCPUsSet, err := nodes.GetOnlineCPUsSet(&workerRTNodes[i]) Expect(err).ToNot(HaveOccurred(), "failed to get Online CPUs list") if irqLoadBalancingDisabled { @@ -219,9 +219,9 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance }) table.DescribeTable("Verify that profile parameters were updated", func(cmdFn checkFunction, parameter []string, shouldContain bool, useRegex bool) { - for _, node := range workerRTNodes { + for i := range workerRTNodes { for _, param := range parameter { - result, err := cmdFn(&node) + result, err := cmdFn(&workerRTNodes[i]) Expect(err).ToNot(HaveOccurred()) matcher := ContainSubstring(param) if useRegex { @@ -249,15 +249,15 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance ) It("[test_id:27738] should succeed to disable the RT kernel", func() { - for _, node := range workerRTNodes { - err := nodes.HasPreemptRTKernel(&node) + for i := range workerRTNodes { + err := nodes.HasPreemptRTKernel(&workerRTNodes[i]) Expect(err).To(HaveOccurred()) } }) It("[test_id:28612]Verify that Kernel arguments can me updated (added, removed) thru performance profile", func() { - for _, node := range workerRTNodes { - cmdline, err := nodes.ExecCommandOnNode(chkCmdLine, &node) + for i := range workerRTNodes { + cmdline, err := nodes.ExecCommandOnNode(chkCmdLine, &workerRTNodes[i]) Expect(err).ToNot(HaveOccurred(), "failed to execute %s", chkCmdLine) // Verifying that new argument was added @@ -287,8 +287,8 @@ var _ = Describe("[rfe_id:28761][performance] Updating parameters in performance return mcps.GetConditionStatus(performanceMCP, conditionUpdating) }, 30, 5).Should(Equal(corev1.ConditionFalse)) - for _, node := range workerRTNodes { - err := nodes.HasPreemptRTKernel(&node) + for i := range workerRTNodes { + err := nodes.HasPreemptRTKernel(&workerRTNodes[i]) Expect(err).To(HaveOccurred()) } }) diff --git a/functests/utils/discovery/discovery.go b/functests/utils/discovery/discovery.go index 91c717e93..f401283a8 100644 --- a/functests/utils/discovery/discovery.go +++ b/functests/utils/discovery/discovery.go @@ -50,7 +50,7 @@ func GetFilteredDiscoveryPerformanceProfile(iterator ConditionIterator) (*perfor func getDiscoveryPerformanceProfile(performanceProfiles []performancev2.PerformanceProfile, nodesSelector string) (*performancev2.PerformanceProfile, error) { var currentProfile *performancev2.PerformanceProfile = nil maxNodesNumber := 0 - for _, profile := range performanceProfiles { + for i, profile := range performanceProfiles { selector := labels.SelectorFromSet(profile.Spec.NodeSelector) profileNodes := &corev1.NodeList{} @@ -65,7 +65,7 @@ func getDiscoveryPerformanceProfile(performanceProfiles []performancev2.Performa } if len(profileNodes.Items) > maxNodesNumber { - currentProfile = &profile + currentProfile = &performanceProfiles[i] maxNodesNumber = len(profileNodes.Items) } } From 5e530837870e870000992691cce860224e26a501 Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Wed, 22 Sep 2021 15:53:44 +0300 Subject: [PATCH 04/11] fix CWE-276 reducing permissions to 600 --- pkg/cmd/render/render.go | 2 +- tools/csv-processor/csv-processor.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/render/render.go b/pkg/cmd/render/render.go index e2eac4b4e..cc9d2ca8f 100644 --- a/pkg/cmd/render/render.go +++ b/pkg/cmd/render/render.go @@ -154,7 +154,7 @@ func (r *renderOpts) Run() error { } fileName := fmt.Sprintf("%s_%s.yaml", profile.Name, strings.ToLower(kind)) - err = ioutil.WriteFile(filepath.Join(r.assetsOutDir, fileName), b, 0644) + err = ioutil.WriteFile(filepath.Join(r.assetsOutDir, fileName), b, 0600) if err != nil { return err } diff --git a/tools/csv-processor/csv-processor.go b/tools/csv-processor/csv-processor.go index 051b6c94b..7422dd0c9 100644 --- a/tools/csv-processor/csv-processor.go +++ b/tools/csv-processor/csv-processor.go @@ -247,7 +247,7 @@ Performance Addon Operator provides the ability to enable advanced node performa writer := strings.Builder{} csvtools.MarshallObject(operatorCSV, &writer) outputFilename := filepath.Join(*outputDir, finalizedCsvFilename()) - err := ioutil.WriteFile(outputFilename, []byte(writer.String()), 0644) + err := ioutil.WriteFile(outputFilename, []byte(writer.String()), 0600) if err != nil { panic(err) } From 44fc82b78dcc0eb5b5c365815f91b9ec56a31de3 Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Thu, 23 Sep 2021 10:52:03 +0300 Subject: [PATCH 05/11] fix CWE-703 explicitly close the file and handle errors. Close is considered unsafe to defer, because it might fail --- pkg/profilecreator/profilecreator.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/profilecreator/profilecreator.go b/pkg/profilecreator/profilecreator.go index 7c7acea94..9408a0392 100644 --- a/pkg/profilecreator/profilecreator.go +++ b/pkg/profilecreator/profilecreator.go @@ -106,6 +106,7 @@ func getMustGatherFullPaths(mustGatherPath string, suffix string) (string, error func getNode(mustGatherDirPath, nodeName string) (*v1.Node, error) { var node v1.Node + nodePathSuffix := path.Join(ClusterScopedResources, CoreNodes, nodeName) path, err := getMustGatherFullPaths(mustGatherDirPath, nodePathSuffix) if err != nil { @@ -116,12 +117,14 @@ func getNode(mustGatherDirPath, nodeName string) (*v1.Node, error) { if err != nil { return nil, fmt.Errorf("failed to open %q: %v", path, err) } - defer src.Close() dec := k8syaml.NewYAMLOrJSONDecoder(src, 1024) if err := dec.Decode(&node); err != nil { return nil, fmt.Errorf("failed to decode %q: %v", path, err) } + if err := src.Close(); err != nil { + return nil, fmt.Errorf("error closing file %s: %s", src.Name(), err) + } return &node, nil } @@ -200,11 +203,13 @@ func GetMCP(mustGatherDirPath, mcpName string) (*machineconfigv1.MachineConfigPo if err != nil { return nil, fmt.Errorf("failed to open %q: %v", mcpPath, err) } - defer src.Close() dec := k8syaml.NewYAMLOrJSONDecoder(src, 1024) if err := dec.Decode(&mcp); err != nil { return nil, fmt.Errorf("failed to decode %q: %v", mcpPath, err) } + if err := src.Close(); err != nil { + return nil, fmt.Errorf("error closing file %s: %s", src.Name(), err) + } return &mcp, nil } From 23ee18a383c2c2d9ba47280384011414f3a9a665 Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Thu, 23 Sep 2021 13:21:16 +0300 Subject: [PATCH 06/11] ignore unchecked os.Unsetenv from gosec reports it seems that, yes, we dont check for errors, but the function always return nil err. this is to avoid CWE-703 on os.Unsetenv --- gosec.conf.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gosec.conf.json b/gosec.conf.json index 0967ef424..f92cf3f68 100644 --- a/gosec.conf.json +++ b/gosec.conf.json @@ -1 +1,5 @@ -{} +{ + "G104": { + "os": ["Unsetenv"] + } +} From bada12ab0e9eb2295863351e6c5651c0aaf9d587 Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Thu, 23 Sep 2021 11:11:22 +0300 Subject: [PATCH 07/11] fix CWE-703 Handle all error cases. for the future, if error is very rare and unlikely to happen on any normal (supported) user case, it's ok to use `panic(err)` in other cases, it's better to propageate error up back to user and logs --- cmd/performance-profile-creator/cmd/root.go | 8 ++++++-- functests/0_config/config.go | 4 +++- functests/3_performance_status/status.go | 6 +++--- functests/5_latency_testing/latency_testing.go | 4 +++- pkg/cmd/render/render.go | 4 +++- pkg/utils/csvtools/csvtools.go | 5 ++++- tools/csv-processor/csv-processor.go | 17 +++++++++++++---- .../csv-replace-imageref.go | 4 +++- 8 files changed, 38 insertions(+), 14 deletions(-) diff --git a/cmd/performance-profile-creator/cmd/root.go b/cmd/performance-profile-creator/cmd/root.go index dc9abf59d..970a3576c 100644 --- a/cmd/performance-profile-creator/cmd/root.go +++ b/cmd/performance-profile-creator/cmd/root.go @@ -291,7 +291,9 @@ func makeClusterInfoFromClusterData(cluster ClusterData) ClusterInfo { } func showClusterInfoJSON(cInfo ClusterInfo) { - json.NewEncoder(os.Stdout).Encode(cInfo) + if err := json.NewEncoder(os.Stdout).Encode(cInfo); err != nil { + panic(fmt.Errorf("Could not create JSON, err: %s", err)) + } } func showClusterInfoLog(cInfo ClusterInfo) { @@ -506,7 +508,9 @@ func createProfile(profileData ProfileData) { // write CSV to out dir writer := strings.Builder{} - csvtools.MarshallObject(&profile, &writer) + if err := csvtools.MarshallObject(&profile, &writer); err != nil { + panic(fmt.Errorf("Could not marshal profile, err: %s", err)) + } fmt.Printf("%s", writer.String()) } diff --git a/functests/0_config/config.go b/functests/0_config/config.go index d4cb6a6e0..d27b3714f 100644 --- a/functests/0_config/config.go +++ b/functests/0_config/config.go @@ -116,7 +116,9 @@ var _ = Describe("[performance][config] Performance configuration", func() { func externalPerformanceProfile(performanceManifest string) (*performancev2.PerformanceProfile, error) { performanceScheme := runtime.NewScheme() - performancev2.AddToScheme(performanceScheme) + if err := performancev2.AddToScheme(performanceScheme); err != nil { + return nil, fmt.Errorf("Failed to add to scheme, err: %s", err) + } decode := serializer.NewCodecFactory(performanceScheme).UniversalDeserializer().Decode manifest, err := ioutil.ReadFile(filepath.Clean(performanceManifest)) diff --git a/functests/3_performance_status/status.go b/functests/3_performance_status/status.go index 7755cb4a4..55090f718 100644 --- a/functests/3_performance_status/status.go +++ b/functests/3_performance_status/status.go @@ -49,9 +49,9 @@ var _ = Describe("Status testing of performance profile", func() { AfterEach(func() { if clean != nil { - clean() + err := clean() + Expect(err).ToNot(HaveOccurred(), "Failed to clean, err: %s", err) } - }) Context("[rfe_id:28881][performance] Performance Addons detailed status", func() { @@ -135,7 +135,7 @@ var _ = Describe("Status testing of performance profile", func() { err := testclient.Client.Get(context.TODO(), key, runtimeClass) // if err != nil probably the resource were already deleted if err == nil { - testclient.Client.Delete(context.TODO(), runtimeClass) + err = testclient.Client.Delete(context.TODO(), runtimeClass) } return err } diff --git a/functests/5_latency_testing/latency_testing.go b/functests/5_latency_testing/latency_testing.go index 15c61bdbc..cdcdaf42f 100644 --- a/functests/5_latency_testing/latency_testing.go +++ b/functests/5_latency_testing/latency_testing.go @@ -187,7 +187,9 @@ func setEnvAndGetDescription(tst latencyTest) string { } func setEnvWriteDescription(envVar string, val string, sb *bytes.Buffer, flag *bool) { - os.Setenv(envVar, val) + if err := os.Setenv(envVar, val); err != nil { + panic(err) + } fmt.Fprintf(sb, "%s = %s \n", envVar, val) *flag = true } diff --git a/pkg/cmd/render/render.go b/pkg/cmd/render/render.go index cc9d2ca8f..7b2534319 100644 --- a/pkg/cmd/render/render.go +++ b/pkg/cmd/render/render.go @@ -95,7 +95,9 @@ func (r *renderOpts) AddFlags(fs *pflag.FlagSet) { func (r *renderOpts) readFlagsFromEnv() { if ppInFiles := os.Getenv("PERFORMANCE_PROFILE_INPUT_FILES"); len(ppInFiles) > 0 { - r.performanceProfileInputFiles.Set(ppInFiles) + if err := r.performanceProfileInputFiles.Set(ppInFiles); err != nil { + panic(err) + } } if assetInDir := os.Getenv("ASSET_INPUT_DIR"); len(assetInDir) > 0 { diff --git a/pkg/utils/csvtools/csvtools.go b/pkg/utils/csvtools/csvtools.go index e678de2fb..5839a8da6 100644 --- a/pkg/utils/csvtools/csvtools.go +++ b/pkg/utils/csvtools/csvtools.go @@ -81,7 +81,10 @@ func MarshallObject(obj interface{}, writer io.Writer) error { unstructured.RemoveNestedField(deployment, "spec", "template", "metadata", "creationTimestamp") unstructured.RemoveNestedField(deployment, "status") } - unstructured.SetNestedSlice(r.Object, deployments, "spec", "install", "spec", "deployments") + err = unstructured.SetNestedSlice(r.Object, deployments, "spec", "install", "spec", "deployments") + if err != nil { + return err + } } jsonBytes, err = json.Marshal(r.Object) diff --git a/tools/csv-processor/csv-processor.go b/tools/csv-processor/csv-processor.go index 7422dd0c9..c03c7e88a 100644 --- a/tools/csv-processor/csv-processor.go +++ b/tools/csv-processor/csv-processor.go @@ -245,9 +245,12 @@ Performance Addon Operator provides the ability to enable advanced node performa // write CSV to out dir writer := strings.Builder{} - csvtools.MarshallObject(operatorCSV, &writer) + err := csvtools.MarshallObject(operatorCSV, &writer) + if err != nil { + panic(err) + } outputFilename := filepath.Join(*outputDir, finalizedCsvFilename()) - err := ioutil.WriteFile(outputFilename, []byte(writer.String()), 0600) + err = ioutil.WriteFile(outputFilename, []byte(writer.String()), 0600) if err != nil { panic(err) } @@ -318,10 +321,16 @@ Performance Addon Operator provides the ability to enable advanced node performa } // start with a fresh output directory if it already exists - os.RemoveAll(*outputDir) + err = os.RemoveAll(*outputDir) + if err != nil { + panic(err) + } // create output directory - os.MkdirAll(*outputDir, os.FileMode(0775)) + err = os.MkdirAll(*outputDir, os.FileMode(0775)) + if err != nil { + panic(err) + } generateUnifiedCSV(userData) } diff --git a/tools/csv-replace-imageref/csv-replace-imageref.go b/tools/csv-replace-imageref/csv-replace-imageref.go index b2a11dde9..95be16ab1 100644 --- a/tools/csv-replace-imageref/csv-replace-imageref.go +++ b/tools/csv-replace-imageref/csv-replace-imageref.go @@ -29,7 +29,9 @@ func processCSV(operatorImage, csvInput string, dst io.Writer) { operatorCSV.Annotations["containerImage"] = operatorImage - csvtools.MarshallObject(operatorCSV, dst) + if err := csvtools.MarshallObject(operatorCSV, dst); err != nil { + panic(fmt.Errorf("could not marshall CSV, err: %s", err)) + } } func main() { From 6159efa6bf03752c0dbc65f0a4b8267507974008 Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Tue, 28 Sep 2021 16:56:01 +0300 Subject: [PATCH 08/11] ignore G204 in latency_testing This is ignored temporarily as gosec resolver doesnt properly support Call Expressions https://github.com/securego/gosec/blob/master/resolve.go#L70 --- functests/5_latency_testing/latency_testing.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functests/5_latency_testing/latency_testing.go b/functests/5_latency_testing/latency_testing.go index cdcdaf42f..681c8d3c2 100644 --- a/functests/5_latency_testing/latency_testing.go +++ b/functests/5_latency_testing/latency_testing.go @@ -94,7 +94,7 @@ var _ = table.DescribeTable("Test latency measurement tools tests", func(testGro if _, err := os.Stat("../../build/_output/bin/latency-e2e.test"); os.IsNotExist(err) { Skip("The executable test file does not exist , skipping the test.") } - output, err := exec.Command("../../build/_output/bin/latency-e2e.test", "-ginkgo.focus", test.toolToTest).Output() + output, err := exec.Command("../../build/_output/bin/latency-e2e.test", "-ginkgo.focus", test.toolToTest).Output() // #nosec G204 if err != nil { //we don't log Error level here because the test might be a negative check testlog.Info(err.Error()) From 3f7b7fa9424ea0001a1665c630018f3a0e1d90b5 Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Mon, 4 Oct 2021 12:53:27 +0300 Subject: [PATCH 09/11] ignore G204 in ExecAndLogCommandWithStderr Ignored as general purpose command execution function --- functests/utils/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functests/utils/utils.go b/functests/utils/utils.go index f7d5cabe2..d78d849fc 100644 --- a/functests/utils/utils.go +++ b/functests/utils/utils.go @@ -36,7 +36,7 @@ func ExecAndLogCommandWithStderr(name string, arg ...string) ([]byte, []byte, er var stdout bytes.Buffer var stderr bytes.Buffer - cmd := exec.CommandContext(ctx, name, arg...) + cmd := exec.CommandContext(ctx, name, arg...) // #nosec G204 cmd.Stdout = &stdout cmd.Stderr = &stderr From 2df0ea1fab8594b0546e2042a2808a373c4da2d3 Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Mon, 4 Oct 2021 11:12:32 +0300 Subject: [PATCH 10/11] WIP - test gosec with GO111MODULE=off --- hack/gosec.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/gosec.sh b/hack/gosec.sh index 93ea9fa27..191dbe3bb 100755 --- a/hack/gosec.sh +++ b/hack/gosec.sh @@ -6,4 +6,4 @@ if [ $? -ne 0 ]; then go get -u github.com/securego/gosec/v2/cmd/gosec fi -time gosec -conf gosec.conf.json ./... +time GO111MODULE=off gosec -conf gosec.conf.json ./... From 45900bebd1b1b39e9ed1844967da5c2947d6976d Mon Sep 17 00:00:00 2001 From: Yuval Kashtan Date: Mon, 25 Oct 2021 10:50:31 +0300 Subject: [PATCH 11/11] WIP: go get is deprecated.. --- hack/gosec.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/gosec.sh b/hack/gosec.sh index 191dbe3bb..69ef42b0a 100755 --- a/hack/gosec.sh +++ b/hack/gosec.sh @@ -3,7 +3,7 @@ which gosec if [ $? -ne 0 ]; then echo "Downloading gosec tool" - go get -u github.com/securego/gosec/v2/cmd/gosec + go install github.com/securego/gosec/v2/cmd/gosec@v2.9.1 fi -time GO111MODULE=off gosec -conf gosec.conf.json ./... +time gosec -conf gosec.conf.json ./...