Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unchecked type assertions #2580

Merged
merged 5 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -70,6 +71,7 @@ linters:
- errname
- errorlint
- fatcontext
- forcetypeassert
- ginkgolinter
- gocheckcompilerdirectives
- gochecksumtype
Expand Down
9 changes: 8 additions & 1 deletion internal/framework/status/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
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{
Expand Down Expand Up @@ -87,7 +89,12 @@
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

Check warning on line 96 in internal/framework/status/updater.go

View check run for this annotation

Codecov / codecov/patch

internal/framework/status/updater.go#L95-L96

Added lines #L95 - L96 were not covered by tests
salonichf5 marked this conversation as resolved.
Show resolved Hide resolved
}

err := wait.ExponentialBackoffWithContext(
ctx,
Expand Down
14 changes: 12 additions & 2 deletions internal/mode/static/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,20 @@

ngxruntimeCollector = collectors.NewManagerMetricsCollector(constLabels)
handlerCollector = collectors.NewControllerCollector(constLabels)

ngxruntimeCollector, ok := ngxruntimeCollector.(prometheus.Collector)
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
return fmt.Errorf("ngxruntimeCollector is not a prometheus.Collector: %w", status.ErrFailedAssert)

Check warning on line 206 in internal/mode/static/manager.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/manager.go#L204-L206

Added lines #L204 - L206 were not covered by tests
salonichf5 marked this conversation as resolved.
Show resolved Hide resolved
}
handlerCollector, ok := handlerCollector.(prometheus.Collector)
if !ok {
return fmt.Errorf("handlerCollector is not a prometheus.Collector: %w", status.ErrFailedAssert)

Check warning on line 210 in internal/mode/static/manager.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/manager.go#L208-L210

Added lines #L208 - L210 were not covered by tests
}

metrics.Registry.MustRegister(
ngxCollector,
ngxruntimeCollector.(prometheus.Collector),
handlerCollector.(prometheus.Collector),
ngxruntimeCollector,
handlerCollector,

Check warning on line 216 in internal/mode/static/manager.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/manager.go#L215-L216

Added lines #L215 - L216 were not covered by tests
)
}

Expand Down
6 changes: 3 additions & 3 deletions internal/mode/static/status/status_setters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
15 changes: 13 additions & 2 deletions tests/framework/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"crypto/tls"
"errors"
"fmt"
"io"
"net"
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
Loading