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/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/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/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-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..d27b3714f 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" @@ -115,10 +116,12 @@ 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(performanceManifest) + manifest, err := ioutil.ReadFile(filepath.Clean(performanceManifest)) if err != nil { return nil, fmt.Errorf("Failed to read %s file", performanceManifest) } 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/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..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()) @@ -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/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) } } 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 diff --git a/gosec.conf.json b/gosec.conf.json new file mode 100644 index 000000000..f92cf3f68 --- /dev/null +++ b/gosec.conf.json @@ -0,0 +1,5 @@ +{ + "G104": { + "os": ["Unsetenv"] + } +} diff --git a/hack/gosec.sh b/hack/gosec.sh new file mode 100755 index 000000000..69ef42b0a --- /dev/null +++ b/hack/gosec.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +which gosec +if [ $? -ne 0 ]; then + echo "Downloading gosec tool" + go install github.com/securego/gosec/v2/cmd/gosec@v2.9.1 +fi + +time gosec -conf gosec.conf.json ./... diff --git a/pkg/cmd/render/render.go b/pkg/cmd/render/render.go index e1885c395..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 { @@ -121,7 +123,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 } @@ -154,7 +156,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/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..9408a0392 100644 --- a/pkg/profilecreator/profilecreator.go +++ b/pkg/profilecreator/profilecreator.go @@ -106,22 +106,25 @@ 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 { 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) } - 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 } @@ -196,15 +199,17 @@ 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) } - 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 } diff --git a/pkg/utils/csvtools/csvtools.go b/pkg/utils/csvtools/csvtools.go index fbe440974..5839a8da6 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) } @@ -80,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 c5d537e01..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()), 0644) + err = ioutil.WriteFile(outputFilename, []byte(writer.String()), 0600) if err != nil { panic(err) } @@ -256,7 +259,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) } @@ -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() {