Refactor codebase to use inline error checking (#1026)

Problem: There were some places in the codebase where inline error checking wasn't used. Solution: Refactor codebase to use inline error checking.
nginxinc · Sep 1, 2023 · fb81d59 · fb81d59
1 parent f540fca
commit fb81d59
Show file tree

Hide file tree

Showing 15 changed files with 48 additions and 45 deletions.
diff --git a/internal/framework/controller/reconciler.go b/internal/framework/controller/reconciler.go
@@ -74,8 +74,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 	}
 
 	obj := newObject(r.cfg.ObjectType)
-	err := r.cfg.Getter.Get(ctx, req.NamespacedName, obj)
-	if err != nil {
+	if err := r.cfg.Getter.Get(ctx, req.NamespacedName, obj); err != nil {
 		if !apierrors.IsNotFound(err) {
 			logger.Error(err, "Failed to get the resource")
 			return reconcile.Result{}, err

diff --git a/internal/framework/controller/register.go b/internal/framework/controller/register.go
@@ -82,8 +82,13 @@ func Register(
 	}
 
 	for field, indexerFunc := range cfg.fieldIndices {
-		err := addIndex(ctx, mgr.GetFieldIndexer(), objectType, field, indexerFunc)
-		if err != nil {
+		if err := addIndex(
+			ctx,
+			mgr.GetFieldIndexer(),
+			objectType,
+			field,
+			indexerFunc,
+		); err != nil {
 			return err
 		}
 	}
@@ -101,8 +106,7 @@ func Register(
 		NamespacedNameFilter: cfg.namespacedNameFilter,
 	}
 
-	err := builder.Complete(cfg.newReconciler(recCfg))
-	if err != nil {
+	if err := builder.Complete(cfg.newReconciler(recCfg)); err != nil {
 		return fmt.Errorf("cannot build a controller for %T: %w", objectType, err)
 	}
 
@@ -119,8 +123,7 @@ func addIndex(
 	c, cancel := context.WithTimeout(ctx, addIndexFieldTimeout)
 	defer cancel()
 
-	err := indexer.IndexField(c, objectType, field, indexerFunc)
-	if err != nil {
+	if err := indexer.IndexField(c, objectType, field, indexerFunc); err != nil {
 		return fmt.Errorf("failed to add index for %T for field %s: %w", objectType, field, err)
 	}
 

diff --git a/internal/framework/events/first_eventbatch_preparer.go b/internal/framework/events/first_eventbatch_preparer.go
@@ -70,8 +70,7 @@ func (p *FirstEventBatchPreparerImpl) Prepare(ctx context.Context) (EventBatch,
 	total := 0
 
 	for _, list := range p.objectLists {
-		err := p.reader.List(ctx, list)
-		if err != nil {
+		if err := p.reader.List(ctx, list); err != nil {
 			return nil, err
 		}
 
@@ -85,8 +84,7 @@ func (p *FirstEventBatchPreparerImpl) Prepare(ctx context.Context) (EventBatch,
 	for _, obj := range p.objects {
 		key := types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}
 
-		err := p.reader.Get(ctx, key, obj)
-		if err != nil {
+		if err := p.reader.Get(ctx, key, obj); err != nil {
 			if !apierrors.IsNotFound(err) {
 				return nil, err
 			}

diff --git a/internal/framework/helpers/helpers.go b/internal/framework/helpers/helpers.go
@@ -35,8 +35,7 @@ func PrepareTimeForFakeClient(t metav1.Time) metav1.Time {
 		panic(fmt.Errorf("failed to marshal time: %w", err))
 	}
 
-	err = t.Unmarshal(bytes)
-	if err != nil {
+	if err = t.Unmarshal(bytes); err != nil {
 		panic(fmt.Errorf("failed to unmarshal time: %w", err))
 	}
 

diff --git a/internal/framework/status/updater.go b/internal/framework/status/updater.go
@@ -146,8 +146,7 @@ func (upd *updaterImpl) update(
 	// Otherwise, the Update status API call can fail.
 	// Note: the default client uses a cache for reads, so we're not making an unnecessary API call here.
 	// the default is configurable in the Manager options.
-	err := upd.cfg.Client.Get(ctx, nsname, obj)
-	if err != nil {
+	if err := upd.cfg.Client.Get(ctx, nsname, obj); err != nil {
 		if !apierrors.IsNotFound(err) {
 			upd.cfg.Logger.Error(
 				err,
@@ -161,8 +160,7 @@ func (upd *updaterImpl) update(
 
 	statusSetter(obj)
 
-	err = upd.cfg.Client.Status().Update(ctx, obj)
-	if err != nil {
+	if err := upd.cfg.Client.Status().Update(ctx, obj); err != nil {
 		upd.cfg.Logger.Error(
 			err,
 			"Failed to update status",

diff --git a/internal/mode/provisioner/deployment.go b/internal/mode/provisioner/deployment.go
@@ -14,8 +14,7 @@ import (
 // It will configure the Deployment to use the Gateway with the given NamespacedName.
 func prepareDeployment(depYAML []byte, id string, gwNsName types.NamespacedName) (*v1.Deployment, error) {
 	dep := &v1.Deployment{}
-	err := yaml.Unmarshal(depYAML, dep)
-	if err != nil {
+	if err := yaml.Unmarshal(depYAML, dep); err != nil {
 		return nil, fmt.Errorf("failed to unmarshal deployment: %w", err)
 	}
 

diff --git a/internal/mode/provisioner/handler.go b/internal/mode/provisioner/handler.go
@@ -110,8 +110,7 @@ func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context) {
 			panic(fmt.Errorf("failed to prepare deployment: %w", err))
 		}
 
-		err = h.k8sClient.Create(ctx, deployment)
-		if err != nil {
+		if err = h.k8sClient.Create(ctx, deployment); err != nil {
 			panic(fmt.Errorf("failed to create deployment: %w", err))
 		}
 
@@ -129,8 +128,7 @@ func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context) {
 	for _, nsname := range removedGwsWithDeps {
 		deployment := h.provisions[nsname]
 
-		err := h.k8sClient.Delete(ctx, deployment)
-		if err != nil {
+		if err := h.k8sClient.Delete(ctx, deployment); err != nil {
 			panic(fmt.Errorf("failed to delete deployment: %w", err))
 		}
 

diff --git a/internal/mode/provisioner/manager.go b/internal/mode/provisioner/manager.go
@@ -72,8 +72,13 @@ func StartManager(cfg Config) error {
 	eventCh := make(chan interface{})
 
 	for _, regCfg := range controllerRegCfgs {
-		err := controller.Register(ctx, regCfg.objectType, mgr, eventCh, regCfg.options...)
-		if err != nil {
+		if err := controller.Register(
+			ctx,
+			regCfg.objectType,
+			mgr,
+			eventCh,
+			regCfg.options...,
+		); err != nil {
 			return fmt.Errorf("cannot register controller for %T: %w", regCfg.objectType, err)
 		}
 	}
@@ -113,8 +118,7 @@ func StartManager(cfg Config) error {
 		firstBatchPreparer,
 	)
 
-	err = mgr.Add(eventLoop)
-	if err != nil {
+	if err := mgr.Add(eventLoop); err != nil {
 		return fmt.Errorf("cannot register event loop: %w", err)
 	}
 

diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go
@@ -90,8 +90,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, batch events.Ev
 	}
 
 	var nginxReloadRes nginxReloadResult
-	err := h.updateNginx(ctx, dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver))
-	if err != nil {
+	if err := h.updateNginx(ctx, dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver)); err != nil {
 		h.cfg.logger.Error(err, "Failed to update NGINX configuration")
 		nginxReloadRes.error = err
 	} else {

diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go
@@ -156,8 +156,13 @@ func StartManager(cfg config.Config) error {
 	ctx := ctlr.SetupSignalHandler()
 
 	for _, regCfg := range controllerRegCfgs {
-		err := controller.Register(ctx, regCfg.objectType, mgr, eventCh, regCfg.options...)
-		if err != nil {
+		if err := controller.Register(
+			ctx,
+			regCfg.objectType,
+			mgr,
+			eventCh,
+			regCfg.options...,
+		); err != nil {
 			return fmt.Errorf("cannot register controller for %T: %w", regCfg.objectType, err)
 		}
 	}
@@ -226,8 +231,7 @@ func StartManager(cfg config.Config) error {
 		eventHandler,
 		firstBatchPreparer)
 
-	err = mgr.Add(eventLoop)
-	if err != nil {
+	if err = mgr.Add(eventLoop); err != nil {
 		return fmt.Errorf("cannot register event loop: %w", err)
 	}
 

diff --git a/internal/mode/static/nginx/config/execute.go b/internal/mode/static/nginx/config/execute.go
@@ -9,8 +9,7 @@ import (
 func execute(template *template.Template, data interface{}) []byte {
 	var buf bytes.Buffer
 
-	err := template.Execute(&buf, data)
-	if err != nil {
+	if err := template.Execute(&buf, data); err != nil {
 		panic(err)
 	}
 

diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go
@@ -49,8 +49,7 @@ func (m *ManagerImpl) Reload(ctx context.Context) error {
 
 	// send HUP signal to the NGINX main process reload configuration
 	// See https://nginx.org/en/docs/control.html
-	err = syscall.Kill(pid, syscall.SIGHUP)
-	if err != nil {
+	if err := syscall.Kill(pid, syscall.SIGHUP); err != nil {
 		return fmt.Errorf("failed to send the HUP signal to NGINX main: %w", err)
 	}
 

diff --git a/internal/mode/static/state/graph/gateway_listener.go b/internal/mode/static/state/graph/gateway_listener.go
@@ -206,8 +206,7 @@ func validateListenerHostname(listener v1beta1.Listener) []conditions.Condition
 		return nil
 	}
 
-	err := validateHostname(h)
-	if err != nil {
+	if err := validateHostname(h); err != nil {
 		path := field.NewPath("hostname")
 		valErr := field.Invalid(path, listener.Hostname, err.Error())
 		return staticConds.NewListenerUnsupportedValue(valErr.Error())

diff --git a/internal/mode/static/state/graph/httproute.go b/internal/mode/static/state/graph/httproute.go
@@ -180,8 +180,10 @@ func buildRoute(
 		ParentRefs: sectionNameRefs,
 	}
 
-	err := validateHostnames(ghr.Spec.Hostnames, field.NewPath("spec").Child("hostnames"))
-	if err != nil {
+	if err := validateHostnames(
+		ghr.Spec.Hostnames,
+		field.NewPath("spec").Child("hostnames"),
+	); err != nil {
 		r.Valid = false
 		r.Conditions = append(r.Conditions, staticConds.NewRouteUnsupportedValue(err.Error()))
 
@@ -550,7 +552,11 @@ func validateMatch(
 		allErrs = append(allErrs, validateQueryParamMatch(validator, q, queryParamPath)...)
 	}
 
-	if err := validateMethodMatch(validator, match.Method, matchPath.Child("method")); err != nil {
+	if err := validateMethodMatch(
+		validator,
+		match.Method,
+		matchPath.Child("method"),
+	); err != nil {
 		allErrs = append(allErrs, err)
 	}
 

diff --git a/internal/mode/static/state/resolver/service_resolver_test.go b/internal/mode/static/state/resolver/service_resolver_test.go
@@ -72,8 +72,7 @@ func createSlice(
 
 func createFakeK8sClient(initObjs ...client.Object) (client.Client, error) {
 	scheme := runtime.NewScheme()
-	err := discoveryV1.AddToScheme(scheme)
-	if err != nil {
+	if err := discoveryV1.AddToScheme(scheme); err != nil {
 		return nil, err
 	}