Merge pull request #4895 from sbueringer/pr-fix-unhandled-errors

🐛  fix unhandled errors

.golangci.yml

@@ -74,7 +74,7 @@ issues:
   # The following are being worked on to remove their exclusion. This list should be reduced or go away all together over time.
   # If it is decided they will not be addressed they should be moved above this comment.
   - Subprocess launch(ed with variable|ing should be audited)
-  - (G104|G307)
+  - 'G307: Deferring unsafe method "Close" on type "\*os.File"'
   exclude-rules:
   - linters:
     - gosec

cmd/clusterctl/client/config/reader_viper_test.go

@@ -125,7 +125,7 @@ func Test_viperReader_Get(t *testing.T) {
 	g.Expect(err).NotTo(HaveOccurred())
 	defer os.RemoveAll(dir)
 
-	os.Setenv("FOO", "foo")
+	_ = os.Setenv("FOO", "foo")
 
 	configFile := filepath.Join(dir, "clusterctl.yaml")
 	g.Expect(os.WriteFile(configFile, []byte("bar: bar"), 0600)).To(Succeed())
@@ -190,7 +190,7 @@ func Test_viperReader_GetWithoutDefaultConfig(t *testing.T) {
 	g.Expect(err).NotTo(HaveOccurred())
 	defer os.RemoveAll(dir)
 
-	os.Setenv("FOO_FOO", "bar")
+	_ = os.Setenv("FOO_FOO", "bar")
 
 	v := newViperReader(injectConfigPaths([]string{dir}))
 	g.Expect(v.Init("")).To(Succeed())
@@ -207,7 +207,7 @@ func Test_viperReader_Set(t *testing.T) {
 	g.Expect(err).NotTo(HaveOccurred())
 	defer os.RemoveAll(dir)
 
-	os.Setenv("FOO", "foo")
+	_ = os.Setenv("FOO", "foo")
 
 	configFile := filepath.Join(dir, "clusterctl.yaml")
 

cmd/clusterctl/cmd/config_provider.go

@@ -187,8 +187,10 @@ func printComponents(c client.Components, output string) error {
 		if _, err := os.Stdout.Write(yaml); err != nil {
 			return errors.Wrap(err, "failed to write yaml to Stdout")
 		}
-		os.Stdout.WriteString("\n")
-		return err
+		if _, err := os.Stdout.WriteString("\n"); err != nil {
+			return errors.Wrap(err, "failed to write trailing new line of yaml to Stdout")
+		}
+		return nil
 	}
 	return nil
 }

cmd/clusterctl/cmd/config_repositories.go

@@ -110,6 +110,5 @@ func runGetRepositories(cfgFile string, out io.Writer) error {
 		}
 		fmt.Fprint(w, string(y))
 	}
-	w.Flush()
-	return nil
+	return w.Flush()
 }

cmd/clusterctl/cmd/generate_yaml_test.go

@@ -124,6 +124,7 @@ func createTempFile(g *WithT, contents string) (string, func()) {
 	g.Expect(os.WriteFile(templateFile, []byte(contents), 0600)).To(Succeed())
 
 	return templateFile, func() {
-		os.RemoveAll(dir)
+		// We don't want to fail if the deletion of the temp file fails, so we ignore the error here
+		_ = os.RemoveAll(dir)
 	}
 }

cmd/clusterctl/cmd/upgrade_plan.go

@@ -116,7 +116,9 @@ func runUpgradePlan() error {
 				upgradeAvailable = true
 			}
 		}
-		w.Flush()
+		if err := w.Flush(); err != nil {
+			return err
+		}
 		fmt.Println("")
 
 		if upgradeAvailable {

cmd/clusterctl/cmd/util.go

@@ -129,7 +129,9 @@ func printVariablesOutput(template client.Template, options client.GetClusterTem
 		for _, v := range optionalVariables {
 			fmt.Fprintf(w, "  - %s\t(defaults to %s)\n", v, *variableMap[v])
 		}
-		w.Flush()
+		if err := w.Flush(); err != nil {
+			return err
+		}
 	}
 
 	fmt.Println()

cmd/clusterctl/cmd/version_checker_test.go

@@ -380,6 +380,7 @@ func generateTempVersionFilePath(g *WithT) (string, func()) {
 	tmpVersionFile := filepath.Join(dir, "clusterctl", "state.yaml")
 
 	return tmpVersionFile, func() {
-		os.RemoveAll(dir)
+		// We don't want to fail if the deletion of the temp file fails, so we ignore the error here
+		_ = os.RemoveAll(dir)
 	}
 }

controllers/machinedeployment_sync.go

@@ -137,7 +137,11 @@ func (r *MachineDeploymentReconciler) getNewMachineSet(ctx context.Context, d *c
 
 	// new MachineSet does not exist, create one.
 	newMSTemplate := *d.Spec.Template.DeepCopy()
-	machineTemplateSpecHash := fmt.Sprintf("%d", mdutil.ComputeHash(&newMSTemplate))
+	hash, err := mdutil.ComputeSpewHash(&newMSTemplate)
+	if err != nil {
+		return nil, err
+	}
+	machineTemplateSpecHash := fmt.Sprintf("%d", hash)
 	newMSTemplate.Labels = mdutil.CloneAndAddLabel(d.Spec.Template.Labels,
 		mdutil.DefaultMachineDeploymentUniqueLabelKey, machineTemplateSpecHash)
 

controllers/mdutil/util.go

@@ -693,6 +693,7 @@ func CloneSelectorAndAddLabel(selector *metav1.LabelSelector, labelKey, labelVal
 // DeepHashObject writes specified object to hash using the spew library
 // which follows pointers and prints actual values of the nested objects
 // ensuring the hash does not change when a pointer changes.
+// Deprecated: Please use controllers/mdutil SpewHashObject(hasher, objectToWrite).
 func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) {
 	hasher.Reset()
 	printer := spew.ConfigState{
@@ -701,16 +702,46 @@ func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) {
 		DisableMethods: true,
 		SpewKeys:       true,
 	}
-	printer.Fprintf(hasher, "%#v", objectToWrite)
+	// We ignore the returned error because there is no way to return the error without
+	// breaking API compatibility. Please use SpewHashObject instead.
+	_, _ = printer.Fprintf(hasher, "%#v", objectToWrite)
 }
 
-// ComputeHash computes the has of a MachineTemplateSpec.
+// SpewHashObject writes specified object to hash using the spew library
+// which follows pointers and prints actual values of the nested objects
+// ensuring the hash does not change when a pointer changes.
+func SpewHashObject(hasher hash.Hash, objectToWrite interface{}) error {
+	hasher.Reset()
+	printer := spew.ConfigState{
+		Indent:         " ",
+		SortKeys:       true,
+		DisableMethods: true,
+		SpewKeys:       true,
+	}
+
+	if _, err := printer.Fprintf(hasher, "%#v", objectToWrite); err != nil {
+		return fmt.Errorf("failed to write object to hasher")
+	}
+	return nil
+}
+
+// ComputeHash computes the hash of a MachineTemplateSpec using the spew library.
+// Deprecated: Please use controllers/mdutil ComputeSpewHash(template).
 func ComputeHash(template *clusterv1.MachineTemplateSpec) uint32 {
 	machineTemplateSpecHasher := fnv.New32a()
 	DeepHashObject(machineTemplateSpecHasher, *template)
 	return machineTemplateSpecHasher.Sum32()
 }
 
+// ComputeSpewHash computes the hash of a MachineTemplateSpec using the spew library.
+func ComputeSpewHash(template *clusterv1.MachineTemplateSpec) (uint32, error) {
+	machineTemplateSpecHasher := fnv.New32a()
+	if err := SpewHashObject(machineTemplateSpecHasher, *template); err != nil {
+		return 0, err
+	}
+	return machineTemplateSpecHasher.Sum32(), nil
+}
+
 // GetDeletingMachineCount gets the number of machines that are in the process of being deleted
 // in a machineList.
 func GetDeletingMachineCount(machineList *clusterv1.MachineList) int32 {

controllers/remote/cluster_cache_healthcheck_test.go

@@ -168,7 +168,7 @@ func TestClusterCacheHealthCheck(t *testing.T) {
 			g.Expect(err).NotTo(HaveOccurred())
 			l, err := net.ListenTCP("tcp", addr)
 			g.Expect(err).NotTo(HaveOccurred())
-			l.Close()
+			g.Expect(l.Close()).To(Succeed())
 
 			config := rest.CopyConfig(env.Config)
 			config.Host = fmt.Sprintf("http://127.0.0.1:%d", l.Addr().(*net.TCPAddr).Port)

internal/envtest/environment.go

@@ -234,7 +234,9 @@ func (e *Environment) WaitForWebhooks() {
 			klog.V(2).Infof("Webhook port is not ready, will retry in %v: %s", timeout, err)
 			continue
 		}
-		conn.Close()
+		if err := conn.Close(); err != nil {
+			klog.V(2).Infof("Closing connection when testing if webhook port is ready failed: %v", err)
+		}
 		klog.V(2).Info("Webhook port is now open. Continuing with tests...")
 		return
 	}