From 9445586cf7036e83fbdc13f551d74305f878f4a1 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Wed, 18 Sep 2024 14:51:55 -0700 Subject: [PATCH] Fix unchecked assertions --- .golangci.yml | 2 ++ internal/framework/status/updater.go | 9 ++++++++- internal/mode/static/manager.go | 14 ++++++++++++-- internal/mode/static/status/status_setters.go | 6 +++--- tests/framework/request.go | 15 +++++++++++++-- 5 files changed, 38 insertions(+), 8 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 600610203..1b496f032 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -37,6 +37,7 @@ linters-settings: - name: string-of-int - name: superfluous-else - name: time-naming + - name: unchecked-type-assertion - name: unexported-return - name: unnecessary-stmt - name: unreachable-code @@ -70,6 +71,7 @@ linters: - errname - errorlint - fatcontext + - forcetypeassert - ginkgolinter - gocheckcompilerdirectives - gochecksumtype diff --git a/internal/framework/status/updater.go b/internal/framework/status/updater.go index 903975d80..c07ad7a04 100644 --- a/internal/framework/status/updater.go +++ b/internal/framework/status/updater.go @@ -53,6 +53,8 @@ type Updater struct { logger logr.Logger } +var ErrFailedAssert = errors.New("type assertion failed") + // NewUpdater creates a new Updater. func NewUpdater(c client.Client, logger logr.Logger) *Updater { return &Updater{ @@ -87,7 +89,12 @@ func (u *Updater) writeStatuses( resourceType ngftypes.ObjectType, statusSetter Setter, ) { - obj := resourceType.DeepCopyObject().(client.Object) + copiedObject := resourceType.DeepCopyObject() + obj, ok := copiedObject.(client.Object) + if !ok { + u.logger.Error(ErrFailedAssert, "object is not a client.Object") + return + } err := wait.ExponentialBackoffWithContext( ctx, diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index d4671e246..f24682185 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -200,10 +200,20 @@ func StartManager(cfg config.Config) error { ngxruntimeCollector = collectors.NewManagerMetricsCollector(constLabels) handlerCollector = collectors.NewControllerCollector(constLabels) + + ngxruntimeCollector, ok := ngxruntimeCollector.(prometheus.Collector) + if !ok { + return fmt.Errorf("ngxruntimeCollector is not a prometheus.Collector: %w", status.ErrFailedAssert) + } + handlerCollector, ok := handlerCollector.(prometheus.Collector) + if !ok { + return fmt.Errorf("handlerCollector is not a prometheus.Collector: %w", status.ErrFailedAssert) + } + metrics.Registry.MustRegister( ngxCollector, - ngxruntimeCollector.(prometheus.Collector), - handlerCollector.(prometheus.Collector), + ngxruntimeCollector, + handlerCollector, ) } diff --git a/internal/mode/static/status/status_setters.go b/internal/mode/static/status/status_setters.go index dc6449050..2c4a6c2ac 100644 --- a/internal/mode/static/status/status_setters.go +++ b/internal/mode/static/status/status_setters.go @@ -82,7 +82,7 @@ func gwStatusEqual(prev, cur gatewayv1.GatewayStatus) bool { func newHTTPRouteStatusSetter(status gatewayv1.HTTPRouteStatus, gatewayCtlrName string) frameworkStatus.Setter { return func(object client.Object) (wasSet bool) { - hr := object.(*gatewayv1.HTTPRoute) + hr := helpers.MustCastObject[*gatewayv1.HTTPRoute](object) // keep all the parent statuses that belong to other controllers for _, os := range hr.Status.Parents { @@ -103,7 +103,7 @@ func newHTTPRouteStatusSetter(status gatewayv1.HTTPRouteStatus, gatewayCtlrName func newTLSRouteStatusSetter(status v1alpha2.TLSRouteStatus, gatewayCtlrName string) frameworkStatus.Setter { return func(object client.Object) (wasSet bool) { - tr := object.(*v1alpha2.TLSRoute) + tr := helpers.MustCastObject[*v1alpha2.TLSRoute](object) // keep all the parent statuses that belong to other controllers for _, os := range tr.Status.Parents { @@ -124,7 +124,7 @@ func newTLSRouteStatusSetter(status v1alpha2.TLSRouteStatus, gatewayCtlrName str func newGRPCRouteStatusSetter(status gatewayv1.GRPCRouteStatus, gatewayCtlrName string) frameworkStatus.Setter { return func(object client.Object) (wasSet bool) { - gr := object.(*gatewayv1.GRPCRoute) + gr := helpers.MustCastObject[*gatewayv1.GRPCRoute](object) // keep all the parent statuses that belong to other controllers for _, os := range gr.Status.Parents { diff --git a/tests/framework/request.go b/tests/framework/request.go index 48705321a..add4c9995 100644 --- a/tests/framework/request.go +++ b/tests/framework/request.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "crypto/tls" + "errors" "fmt" "io" "net" @@ -41,7 +42,12 @@ func Post(url, address string, body io.Reader, timeout time.Duration) (*http.Res func makeRequest(method, url, address string, body io.Reader, timeout time.Duration) (*http.Response, error) { dialer := &net.Dialer{} - http.DefaultTransport.(*http.Transport).DialContext = func( + transport, ok := http.DefaultTransport.(*http.Transport) + if !ok { + return nil, errors.New("transport is not of type *http.Transport") + } + + transport.DialContext = func( ctx context.Context, network, addr string, @@ -61,7 +67,12 @@ func makeRequest(method, url, address string, body io.Reader, timeout time.Durat var resp *http.Response if strings.HasPrefix(url, "https") { - customTransport := http.DefaultTransport.(*http.Transport).Clone() + transport, ok := http.DefaultTransport.(*http.Transport) + if !ok { + return nil, errors.New("transport is not of type *http.Transport") + } + + customTransport := transport.Clone() // similar to how in our examples with https requests we run our curl command // we turn off verification of the certificate, we do the same here customTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} //nolint:gosec // for https test traffic