cleanup: Fix various issues found by code inspection

api/v1beta2/ssp_types.go

@@ -160,10 +160,11 @@ type SSPStatus struct {
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
+// SSP is the Schema for the ssps API
+//
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:storageversion
-// SSP is the Schema for the ssps API
 // +kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase"
 type SSP struct {
 	metav1.TypeMeta   `json:",inline"`

hack/csv-generator.go

@@ -58,7 +58,8 @@ var (
 		Run: func(cmd *cobra.Command, args []string) {
 			err := runGenerator()
 			if err != nil {
-				fmt.Fprintf(os.Stderr, "%v\n", err)
+				// Ignoring returned error: no reasonable way to handle it.
+				_, _ = fmt.Fprintf(os.Stderr, "%v\n", err)
 				os.Exit(1)
 			}
 		},
@@ -67,7 +68,8 @@ var (
 
 func main() {
 	if err := rootCmd.Execute(); err != nil {
-		fmt.Fprintf(os.Stderr, "%v\n", err)
+		// Ignoring returned error: no reasonable way to handle it.
+		_, _ = fmt.Fprintf(os.Stderr, "%v\n", err)
 		os.Exit(1)
 	}
 }

internal/common/labels_test.go

@@ -35,7 +35,7 @@ var _ = Describe("AddAppLabels", func() {
 
 	When("SSP CR has app labels", func() {
 		It("adds app labels from request", func() {
-			obj := AddAppLabels(request.Instance, "test", AppComponent("testing"), &v1.ConfigMap{})
+			obj := AddAppLabels(request.Instance, "test", "testing", &v1.ConfigMap{})
 
 			labels := obj.GetLabels()
 			Expect(labels[AppKubernetesPartOfLabel]).To(Equal("tests"))
@@ -45,15 +45,15 @@ var _ = Describe("AddAppLabels", func() {
 	When("SSP CR does not have app labels", func() {
 		It("does not add app labels on nil", func() {
 			request.Instance.Labels = nil
-			obj := AddAppLabels(request.Instance, "test", AppComponent("testing"), &v1.ConfigMap{})
+			obj := AddAppLabels(request.Instance, "test", "testing", &v1.ConfigMap{})
 
 			labels := obj.GetLabels()
 			Expect(labels[AppKubernetesPartOfLabel]).To(Equal(""))
 			Expect(labels[AppKubernetesVersionLabel]).To(Equal(""))
 		})
 		It("does not add app labels empty map", func() {
 			request.Instance.Labels = map[string]string{}
-			obj := AddAppLabels(request.Instance, "test", AppComponent("testing"), &v1.ConfigMap{})
+			obj := AddAppLabels(request.Instance, "test", "testing", &v1.ConfigMap{})
 
 			labels := obj.GetLabels()
 			Expect(labels[AppKubernetesPartOfLabel]).To(Equal(""))
@@ -62,15 +62,15 @@ var _ = Describe("AddAppLabels", func() {
 	})
 
 	It("adds dynamic app labels", func() {
-		obj := AddAppLabels(request.Instance, "test", AppComponent("testing"), &v1.ConfigMap{})
+		obj := AddAppLabels(request.Instance, "test", "testing", &v1.ConfigMap{})
 
 		labels := obj.GetLabels()
 		Expect(labels[AppKubernetesComponentLabel]).To(Equal("testing"))
 		Expect(labels[AppKubernetesNameLabel]).To(Equal("test"))
 	})
 
 	It("adds managed-by label", func() {
-		obj := AddAppLabels(request.Instance, "test", AppComponent("testing"), &v1.ConfigMap{})
+		obj := AddAppLabels(request.Instance, "test", "testing", &v1.ConfigMap{})
 
 		labels := obj.GetLabels()
 		Expect(labels[AppKubernetesManagedByLabel]).To(Equal("ssp-operator"))

internal/common/resource.go

@@ -537,35 +537,6 @@ func defaultStatusFunc(obj client.Object) ResourceStatus {
 	return status
 }
 
-// AppendDeepCopies appends deep copies of objects from a source slice to a destination slice.
-// This function is useful for creating a new slice containing deep copies of the provided objects.
-//
-// The PT interface {*T ; client.Object } is a trick. It means that type PT satisfies
-// an anonymous typeset defined directly in the function signature.
-// This typeset interface { *T; client.Object } means that it is a pointer to T
-// and implements interface client.Object.
-//
-// Parameters:
-//   - destination: The destination slice where the deep copies of objects will be appended.
-//   - objects: A slice of objects (of type T) to be deep copied and appended to the destination slice.
-//
-// Returns:
-//   - The updated destination slice containing the appended deep copies of objects.
-//
-// Type Parameters:
-//   - PT: A type that is a pointer to the object type (should implement *T and client.Object interfaces).
-//   - T: The actual object type.
-func AppendDeepCopies[PT interface {
-	*T
-	client.Object
-}, T any](destination []client.Object, objects []T) []client.Object {
-	for i := range objects {
-		var object = PT(&objects[i])
-		destination = append(destination, object.DeepCopyObject().(client.Object))
-	}
-	return destination
-}
-
 func CleanupResources[L any, T any, PtrL interface {
 	*L
 	client.ObjectList

internal/common/resource_test.go

@@ -322,7 +322,7 @@ func newTestResource(namespace string) *v1.Service {
 			Ports: []v1.ServicePort{{
 				Name:       "webhook",
 				Port:       443,
-				TargetPort: intstr.FromInt(8443),
+				TargetPort: intstr.FromInt32(8443),
 			}},
 			Selector: map[string]string{
 				"kubevirtIo": "virtTemplateValidator",

internal/common/scheme.go

@@ -30,7 +30,7 @@ func init() {
 	utilruntime.Must(kubevirtv1.AddToScheme(Scheme))
 }
 
-// This function is useful in operand unit tests only
+// AddConversionFunctions is useful in operand unit tests only
 func AddConversionFunctions(s *runtime.Scheme) error {
 	err := s.AddConversionFunc((*extv1.CustomResourceDefinition)(nil), (*metav1.PartialObjectMetadata)(nil), func(a, b interface{}, scope conversion.Scope) error {
 		crd := a.(*extv1.CustomResourceDefinition)

internal/controllers/ssp_controller.go

@@ -122,9 +122,9 @@ func (s *sspController) AddToManager(mgr ctrl.Manager, crdList crd_watch.CrdList
 	return builder.Complete(s)
 }
 
-func (r *sspController) RequiredCrds() []string {
+func (s *sspController) RequiredCrds() []string {
 	var result []string
-	for _, operand := range r.operands {
+	for _, operand := range s.operands {
 		result = append(result, getRequiredCrds(operand)...)
 	}
 	return result

internal/controllers/webhook_controller.go

@@ -23,7 +23,7 @@ const (
 
 // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;update
 
-// CreateWebhookConfigurationController creates a controller
+// NewWebhookConfigurationController creates a controller
 // that watches ValidatingWebhookConfiguration created by OLM,
 // and removes any namespaceSelector defined in it.
 //

internal/env/env_test.go

@@ -12,10 +12,11 @@ import (
 
 var _ = Describe("environments", func() {
 	It("should return correct value for OPERATOR_VERSION when variable is set", func() {
-		os.Setenv(OperatorVersionKey, "v0.0.1")
+		Expect(os.Setenv(OperatorVersionKey, "v0.0.1")).To(Succeed())
+		defer func() { Expect(os.Unsetenv(OperatorVersionKey)).To(Succeed()) }()
+
 		res := GetOperatorVersion()
 		Expect(res).To(Equal("v0.0.1"), "OPERATOR_VERSION should equal")
-		os.Unsetenv(OperatorVersionKey)
 	})
 
 	It("should return correct value for OPERATOR_VERSION when variable is not set", func() {

internal/operands/data-sources/reconcile.go

@@ -37,7 +37,6 @@ const (
 )
 
 const (
-	dataVolumeCrd     = "datavolumes.cdi.kubevirt.io"
 	dataSourceCrd     = "datasources.cdi.kubevirt.io"
 	dataImportCronCrd = "dataimportcrons.cdi.kubevirt.io"
 )

internal/operands/metrics/reconcile_test.go

@@ -63,7 +63,7 @@ var _ = Describe("Metrics operand", func() {
 	})
 
 	AfterEach(func() {
-		os.Unsetenv(runbookURLTemplateEnv)
+		Expect(os.Unsetenv(runbookURLTemplateEnv)).To(Succeed())
 	})
 
 	It("should create metrics resources", func() {
@@ -82,7 +82,7 @@ var _ = Describe("Metrics operand", func() {
 	DescribeTable("runbook URL template",
 		func(template string) {
 			if template != defaultRunbookURLTemplate {
-				os.Setenv(runbookURLTemplateEnv, template)
+				Expect(os.Setenv(runbookURLTemplateEnv, template)).To(Succeed())
 			}
 
 			err := rules.SetupRules()

internal/operands/template-validator/resources.go

@@ -19,7 +19,7 @@ import (
 	"kubevirt.io/ssp-operator/internal/common"
 	"kubevirt.io/ssp-operator/internal/env"
 	common_templates "kubevirt.io/ssp-operator/internal/operands/common-templates"
-	metrics "kubevirt.io/ssp-operator/internal/operands/metrics"
+	"kubevirt.io/ssp-operator/internal/operands/metrics"
 	"kubevirt.io/ssp-operator/internal/template-validator/tlsinfo"
 	webhook "kubevirt.io/ssp-operator/internal/template-validator/webhooks"
 )
@@ -117,7 +117,7 @@ func newService(namespace string) *core.Service {
 			Ports: []core.ServicePort{{
 				Name:       "webhook",
 				Port:       443,
-				TargetPort: intstr.FromInt(ContainerPort),
+				TargetPort: intstr.FromInt32(ContainerPort),
 			}},
 			Selector: CommonLabels(),
 		},
@@ -234,7 +234,7 @@ func newDeployment(namespace string, replicas int32, image string) *apps.Deploym
 							ProbeHandler: core.ProbeHandler{
 								HTTPGet: &core.HTTPGetAction{
 									Path:   "/readyz",
-									Port:   intstr.FromInt(ContainerPort),
+									Port:   intstr.FromInt32(ContainerPort),
 									Scheme: core.URISchemeHTTPS,
 								},
 							},

internal/operands/vm-console-proxy/reconcile_test.go

@@ -514,7 +514,7 @@ func getMockedTestBundle() *vm_console_proxy_bundle.Bundle {
 			Spec: core.ServiceSpec{
 				Ports: []core.ServicePort{{
 					Port:       443,
-					TargetPort: intstr.FromInt(8768),
+					TargetPort: intstr.FromInt32(8768),
 				}},
 				Selector: map[string]string{
 					"vm-console-proxy.kubevirt.io": vmConsoleProxyName,

internal/template-validator/kubevirtjobs/virtualmachine.go

@@ -32,7 +32,7 @@ const (
 	MaxItems         int  = 64
 )
 
-// NewVirtualMachine returns a fully zero-value VirtualMachine with all optional fields
+// NewDefaultVirtualMachine returns a fully zero-value VirtualMachine with all optional fields
 func NewDefaultVirtualMachine() *k6tv1.VirtualMachine {
 	domSpec := k6tv1.DomainSpec{}
 	numItems := NumItems(map[string]int{

internal/template-validator/labels/labels.go

@@ -12,10 +12,11 @@ const (
 	AnnotationTemplateNamespaceOldKey string = "vm.kubevirt.io/template-namespace"
 	AnnotationValidationKey           string = "validations"
 
-	// This is the new annotation we will be using for VirtualMachines that carry their own validation rules
+	// VmValidationAnnotationKey is the new annotation we will be using
+	// for VirtualMachines that carry their own validation rules
 	VmValidationAnnotationKey string = "vm.kubevirt.io/validations"
 
-	// If this annotation exists on a VM, it means that validation should be skipped.
+	// VmSkipValidationAnnotationKey is used to skip validation of a VM.
 	// This annotation is used for troubleshooting, debugging and experimenting with templated VMs.
 	VmSkipValidationAnnotationKey string = "vm.kubevirt.io/skip-validations"
 )

internal/template-validator/tlsinfo/tlsinfo.go

@@ -19,9 +19,6 @@ const (
 	CertFilename       = "tls.crt"
 	KeyFilename        = "tls.key"
 	TLSOptionsFilename = "tls-config.json"
-
-	CiphersEnvName       = "TLS_CIPHERS"
-	TLSMinVersionEnvName = "TLS_MIN_VERSION"
 )
 
 type TLSInfo struct {

internal/template-validator/validation/eval.go

@@ -160,42 +160,49 @@ func (ev *Evaluator) Evaluate(rules []Rule, vm *k6tv1.VirtualMachine) *Result {
 		// Otherwise, if we simply skip the malformed rule, the error can go unnoticed.
 		// IOW, this is a policy decision
 		if _, ok := uniqueNames[r.Name]; ok {
-			fmt.Fprintf(ev.Sink, "%s failed: duplicate name\n", r.Name)
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s failed: duplicate name\n", r.Name)
 			result.Fail(r, ErrDuplicateRuleName)
 			continue
 		}
 		uniqueNames[r.Name] = struct{}{}
 
 		if err := validateRule(r); err != nil {
-			fmt.Fprintf(ev.Sink, "%s failed: %v\n", r.Name, err)
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s failed: %v\n", r.Name, err)
 			result.Fail(r, err)
 			continue
 		}
 
 		// Specialize() may be costly, so we do this before.
 		if !r.IsAppliableOn(vm) {
 			// Legit case. Nothing to do or to complain.
-			fmt.Fprintf(ev.Sink, "%s SKIPPED: not appliable\n", r.Name)
+
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s SKIPPED: not appliable\n", r.Name)
 			result.Skip(r)
 			continue
 		}
 
 		ra, err := r.Specialize(vm, refVm)
 		if err != nil {
-			fmt.Fprintf(ev.Sink, "%s failed: cannot specialize: %v\n", r.Name, err)
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s failed: cannot specialize: %v\n", r.Name, err)
 			result.Fail(r, err)
 			continue
 		}
 
 		satisfied, err := ra.Apply(vm, refVm)
 		if err != nil {
-			fmt.Fprintf(ev.Sink, "%s failed: cannot apply: %v\n", r.Name, err)
+			// Ignoring returned error: This print is used only for logging
+			_, _ = fmt.Fprintf(ev.Sink, "%s failed: cannot apply: %v\n", r.Name, err)
 			result.Fail(r, err)
 			continue
 		}
 
 		applicationText := ra.String()
-		fmt.Fprintf(ev.Sink, "%s applied: %v, %s\n", r.Name, boolAsStatus(satisfied), applicationText)
+		// Ignoring returned error: This print is used only for logging
+		_, _ = fmt.Fprintf(ev.Sink, "%s applied: %v, %s\n", r.Name, boolAsStatus(satisfied), applicationText)
 		result.Applied(r, satisfied, applicationText)
 	}
 

internal/template-validator/validation/eval_test.go

@@ -285,7 +285,8 @@ var _ = Describe("Eval", func() {
 			Expect(res.Succeeded()).To(BeTrue())
 
 			for ix := range res.Status {
-				fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
+				_, err := fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			Expect(res.Status).To(HaveLen(len(rules)))
@@ -342,7 +343,8 @@ var _ = Describe("Eval", func() {
 			res := ev.Evaluate(rules, vmCirros)
 
 			for ix := range res.Status {
-				fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
+				_, err := fmt.Fprintf(GinkgoWriter, "%+#v", res.Status[ix])
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			Expect(res.Succeeded()).To(BeFalse(), "succeeded")

internal/template-validator/validation/path/conv.go

@@ -29,7 +29,7 @@ func toInt64(obj interface{}) (int64, bool) {
 	case int32:
 		return int64(val), true
 	case int64:
-		return int64(val), true
+		return val, true
 	case uint:
 		return int64(val), true
 	case uint32:

internal/template-validator/validator/app.go

@@ -2,7 +2,6 @@ package validator
 
 import (
 	"crypto/tls"
-	"fmt"
 	"net/http"
 	"os"
 
@@ -130,7 +129,9 @@ func (app *App) Run() {
 
 func registerReadinessProbe() {
 	http.HandleFunc("/readyz", func(resp http.ResponseWriter, req *http.Request) {
-		fmt.Fprintf(resp, "ok")
+		if _, err := resp.Write([]byte("ok")); err != nil {
+			logger.Log.Error(err, "Failed to write response to /readyz")
+		}
 	})
 }
 

internal/template-validator/webhooks/utils.go

@@ -12,7 +12,6 @@ import (
 	kubevirt "kubevirt.io/api/core/v1"
 )
 
-// GetAdmissionReview
 func GetAdmissionReview(r *http.Request) (*admissionv1.AdmissionReview, error) {
 	var body []byte
 	if r.Body != nil {
@@ -38,7 +37,6 @@ func ToAdmissionResponseOK() *admissionv1.AdmissionResponse {
 	return &reviewResponse
 }
 
-// ToAdmissionResponseError
 func ToAdmissionResponseError(err error) *admissionv1.AdmissionResponse {
 	return &admissionv1.AdmissionResponse{
 		Result: &metav1.Status{

main.go

@@ -19,6 +19,7 @@ package main
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"flag"
 	"fmt"
 	"net/http"
@@ -164,7 +165,7 @@ func (s *prometheusServer) Start(ctx context.Context) error {
 
 	server.TLSConfig = s.getPrometheusTLSConfig(ctx, certWatcher)
 
-	if err := server.ListenAndServeTLS(s.certPath, s.keyPath); err != nil && err != http.ErrServerClosed {
+	if err := server.ListenAndServeTLS(s.certPath, s.keyPath); err != nil && !errors.Is(err, http.ErrServerClosed) {
 		setupLog.Error(err, "Failed to start Prometheus metrics endpoint server")
 		return err
 	}