Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
105 changes: 104 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,108 @@
version: 2
# HyperFleet Standard golangci-lint Configuration
# Reference: https://golangci-lint.run/usage/configuration
#
# This is the baseline configuration for all HyperFleet Go repositories.
# Copy this file to your repository root as .golangci.yml
#
# golangci-lint should be installed and versioned via bingo in each repository.
# See linting.md for full documentation of this standard.

version: "2"

run:
timeout: 5m
tests: true
modules-download-mode: readonly

linters-settings:
errcheck:
check-type-assertions: true
check-blank: true

govet:
enable-all: true

goconst:
min-len: 3
min-occurrences: 3

misspell:
locale: US

lll:
line-length: 120

gofmt:
simplify: true

revive:
rules:
- name: exported
severity: warning
disabled: true
- name: unexported-return
severity: warning
disabled: false
- name: var-naming
severity: warning
disabled: false

unused:
check-exported: false

unparam:
check-exported: false

exhaustive:
check-generated: false
default-signifies-exhaustive: true

linters:
enable:
# Code Quality
- errcheck
- govet
- staticcheck
- ineffassign
- unused
- unconvert
- unparam
- goconst
- exhaustive

# Code Style
- misspell
- lll
# - revive
- gocritic

# Security
- gosec

issues:
# Add repository-specific generated code directories here
exclude-dirs:
- pkg/api/openapi # Generated code
- data/generated/openapi # Generated file

exclude-rules:
# Relaxed rules for test files
- path: _test\.go
linters:
- gosec
- errcheck
- unparam

# Exclude generated code (backup pattern if exclude-dirs doesn't catch all)
- path: pkg/api/openapi/
linters:
- all

max-issues-per-linter: 0
max-same-issues: 0
new: false

output:
format: colored-line-number
print-issued-lines: true
print-linter-name: true
4 changes: 3 additions & 1 deletion cmd/hyperfleet-api/servecmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ func runServe(cmd *cobra.Command, args []string) {
}

if tp != nil {
shutdownCtx, cancel := context.WithTimeout(context.Background(), environments.Environment().Config.Health.ShutdownTimeout)
shutdownCtx, cancel := context.WithTimeout(
context.Background(), environments.Environment().Config.Health.ShutdownTimeout,
)
defer cancel()
if err := telemetry.Shutdown(shutdownCtx, tp); err != nil {
logger.WithError(ctx, err).Error("Failed to shutdown OpenTelemetry")
Expand Down
8 changes: 5 additions & 3 deletions cmd/hyperfleet-api/server/api_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,9 @@ func NewAPIServer() Server {
mainHandler = removeTrailingSlash(mainHandler)

s.httpServer = &http.Server{
Addr: env().Config.Server.BindAddress,
Handler: mainHandler,
Addr: env().Config.Server.BindAddress,
Handler: mainHandler,
ReadHeaderTimeout: 10 * time.Second,
}

return s
Expand Down Expand Up @@ -136,7 +137,8 @@ func (s apiServer) Listen() (listener net.Listener, err error) {
return net.Listen("tcp", env().Config.Server.BindAddress)
}

// Start listening on the configured port and start the server. This is a convenience wrapper for Listen() and Serve(listener Listener)
// Start listening on the configured port and start the server.
// This is a convenience wrapper for Listen() and Serve(listener Listener)
func (s apiServer) Start() {
ctx := context.Background()
listener, err := s.Listen()
Expand Down
5 changes: 3 additions & 2 deletions cmd/hyperfleet-api/server/health_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ func NewHealthServer() Server {
listening: make(chan struct{}),
}
s.httpServer = &http.Server{
Addr: env().Config.Health.BindAddress,
Handler: mainHandler,
Addr: env().Config.Health.BindAddress,
Handler: mainHandler,
ReadHeaderTimeout: 10 * time.Second,
}
return s
}
Expand Down
6 changes: 4 additions & 2 deletions cmd/hyperfleet-api/server/metrics_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net"
"net/http"
"time"

"github.com/gorilla/mux"

Expand All @@ -25,8 +26,9 @@ func NewMetricsServer() Server {

s := &metricsServer{}
s.httpServer = &http.Server{
Addr: env().Config.Metrics.BindAddress,
Handler: mainHandler,
Addr: env().Config.Metrics.BindAddress,
Handler: mainHandler,
ReadHeaderTimeout: 10 * time.Second,
}
return s
}
Expand Down
14 changes: 12 additions & 2 deletions cmd/hyperfleet-api/server/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,25 @@ type ServicesInterface interface {
GetService(name string) interface{}
}

type RouteRegistrationFunc func(apiV1Router *mux.Router, services ServicesInterface, authMiddleware auth.JWTMiddleware, authzMiddleware auth.AuthorizationMiddleware)
type RouteRegistrationFunc func(
apiV1Router *mux.Router,
services ServicesInterface,
authMiddleware auth.JWTMiddleware,
authzMiddleware auth.AuthorizationMiddleware,
)

var routeRegistry = make(map[string]RouteRegistrationFunc)

func RegisterRoutes(name string, registrationFunc RouteRegistrationFunc) {
routeRegistry[name] = registrationFunc
}

func LoadDiscoveredRoutes(apiV1Router *mux.Router, services ServicesInterface, authMiddleware auth.JWTMiddleware, authzMiddleware auth.AuthorizationMiddleware) {
func LoadDiscoveredRoutes(
apiV1Router *mux.Router,
services ServicesInterface,
authMiddleware auth.JWTMiddleware,
authzMiddleware auth.AuthorizationMiddleware,
) {
for name, registrationFunc := range routeRegistry {
registrationFunc(apiV1Router, services, authMiddleware, authzMiddleware)
_ = name // prevent unused variable warning
Expand Down
10 changes: 7 additions & 3 deletions pkg/api/presenters/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import (
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/util"
)

const (
testConditionReady = "Ready"
)

// Helper function to create test ClusterCreateRequest
func createTestClusterRequest() *openapi.ClusterCreateRequest {
labels := map[string]string{"env": "test"}
Expand Down Expand Up @@ -157,7 +161,7 @@ func TestPresentCluster_Complete(t *testing.T) {
RegisterTestingT(t)

now := time.Now()
reason := "Ready"
reason := testConditionReady
message := "Cluster is ready"

// Create domain ResourceCondition
Expand Down Expand Up @@ -218,7 +222,7 @@ func TestPresentCluster_Complete(t *testing.T) {
Expect(len(result.Status.Conditions)).To(Equal(1))
Expect(result.Status.Conditions[0].Type).To(Equal("Available"))
Expect(result.Status.Conditions[0].Status).To(Equal(openapi.ResourceConditionStatusTrue))
Expect(*result.Status.Conditions[0].Reason).To(Equal("Ready"))
Expect(*result.Status.Conditions[0].Reason).To(Equal(testConditionReady))

// Verify timestamps
Expect(result.CreatedTime.Unix()).To(Equal(now.Unix()))
Expand Down Expand Up @@ -300,7 +304,7 @@ func TestPresentCluster_StatusConditionsConversion(t *testing.T) {
// First condition
Expect(result.Status.Conditions[0].Type).To(Equal("Available"))
Expect(result.Status.Conditions[0].Status).To(Equal(openapi.ResourceConditionStatusTrue))
Expect(*result.Status.Conditions[0].Reason).To(Equal("Ready"))
Expect(*result.Status.Conditions[0].Reason).To(Equal(testConditionReady))
Expect(*result.Status.Conditions[0].Message).To(Equal("All systems operational"))

// Second condition
Expand Down
16 changes: 9 additions & 7 deletions pkg/api/presenters/slice_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ func validate(model interface{}, in map[string]bool, prefix string) *errors.Serv
}
field := reflectValue.Field(i).Interface()
name := strings.Split(tag, ",")[0]
if kind == reflect.Struct {
switch kind { //nolint:exhaustive // Only Struct and Slice need special handling, rest handled by default
case reflect.Struct:
if t.Type == reflect.TypeOf(&time.Time{}) {
delete(in, name)
} else {
Expand All @@ -116,12 +117,12 @@ func validate(model interface{}, in map[string]bool, prefix string) *errors.Serv
}
}
}
} else if t.Type.Kind() == reflect.Slice {
case reflect.Slice:
// TODO: We don't support Slices' validation :(
in = removeStar(in, name)
continue
//_ = validate(slice, in, name)
} else {
// _ = validate(slice, in, name)
default:
prefixedName := name
if prefix != "" {
prefixedName = fmt.Sprintf("%s.%s", prefix, name)
Expand Down Expand Up @@ -188,7 +189,8 @@ func structToMap(item interface{}, in map[string]bool, prefix string) map[string
}
field := reflectValue.Field(i).Interface()
name := strings.Split(tag, ",")[0]
if kind == reflect.Struct {
switch kind { //nolint:exhaustive // Only Struct and Slice need special handling, rest handled by default
case reflect.Struct:
if t.Type == reflect.TypeOf(&time.Time{}) {
if _, ok := in[name]; ok {
if timePtr, ok := field.(*time.Time); ok && timePtr != nil {
Expand All @@ -205,7 +207,7 @@ func structToMap(item interface{}, in map[string]bool, prefix string) map[string
res[name] = subStruct
}
}
} else if kind == reflect.Slice {
case reflect.Slice:
s := reflect.ValueOf(field)
if s.Len() > 0 {
result := make([]interface{}, 0, s.Len())
Expand All @@ -220,7 +222,7 @@ func structToMap(item interface{}, in map[string]bool, prefix string) map[string
res[name] = result
}
}
} else {
default:
prefixedName := name
if prefix != "" {
prefixedName = fmt.Sprintf("%s.%s", prefix, name)
Expand Down
5 changes: 4 additions & 1 deletion pkg/auth/auth_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ func (a *Middleware) AuthenticateAccountJWT(next http.Handler) http.Handler {
ctx := r.Context()
payload, err := GetAuthPayload(r)
if err != nil {
handleError(ctx, w, r, errors.CodeAuthNoCredentials, fmt.Sprintf("Unable to get payload details from JWT token: %s", err))
handleError(
ctx, w, r, errors.CodeAuthNoCredentials,
fmt.Sprintf("Unable to get payload details from JWT token: %s", err),
)
return
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/auth/authz_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ func (a authzMiddleware) AuthorizeApi(next http.Handler) http.Handler {
if username == "" {
_ = fmt.Errorf("authenticated username not present in request context")
// TODO
//body := api.E500.Format(r, "Authentication details not present in context")
//api.SendError(w, r, &body)
// body := api.E500.Format(r, "Authentication details not present in context")
// api.SendError(w, r, &body)
Comment on lines +49 to +50
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Return explicit HTTP errors instead of silent early exits.
These branches return without writing a response, which yields misleading 200/empty bodies for auth failures or backend errors. Send 401/403/500 (or your standard error body) and remove the no-op fmt.Errorf calls.

🛠️ Suggested fix
-import (
-	"fmt"
-	"net/http"
-
-	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/client/ocm"
-)
+import (
+	"net/http"
+
+	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/client/ocm"
+)
@@
 		username := GetUsernameFromContext(ctx)
 		if username == "" {
-			_ = fmt.Errorf("authenticated username not present in request context")
-			// TODO
-			// body := api.E500.Format(r, "Authentication details not present in context")
-			// api.SendError(w, r, &body)
+			http.Error(w, "authentication details not present in context", http.StatusUnauthorized)
 			return
 		}
@@
 		if err != nil {
-			_ = fmt.Errorf("unable to make authorization request: %s", err)
-			// TODO
-			// body := api.E500.Format(r, "Unable to make authorization request")
-			// api.SendError(w, r, &body)
+			http.Error(w, "unable to make authorization request", http.StatusInternalServerError)
 			return
 		}
 
 		if allowed {
 			next.ServeHTTP(w, r)
+			return
 		}
 
-		// TODO
-		// body := api.E403.Format(r, "")
-		// api.SendError(w, r, &body)
+		http.Error(w, "forbidden", http.StatusForbidden)
 	})
 }

Also applies to: 59-60, 69-70

🤖 Prompt for AI Agents
In `@pkg/auth/authz_middleware.go` around lines 49 - 50, The middleware
(authz_middleware.go) currently returns early on missing auth/context errors
without writing a response; update the error branches in the AuthzMiddleware
handler to create and send proper HTTP error responses (use api.E401 or api.E403
for auth failures and api.E500 for backend errors) via api.SendError(w, r,
&body) instead of returning silently, and remove the no-op fmt.Errorf calls;
apply the same change to the other early-return branches referenced (the ones
around lines 59-60 and 69-70) so every failure path writes an appropriate error
body and status.

return
}

Expand All @@ -56,8 +56,8 @@ func (a authzMiddleware) AuthorizeApi(next http.Handler) http.Handler {
if err != nil {
_ = fmt.Errorf("unable to make authorization request: %s", err)
// TODO
//body := api.E500.Format(r, "Unable to make authorization request")
//api.SendError(w, r, &body)
// body := api.E500.Format(r, "Unable to make authorization request")
// api.SendError(w, r, &body)
return
}

Expand All @@ -66,7 +66,7 @@ func (a authzMiddleware) AuthorizeApi(next http.Handler) http.Handler {
}

// TODO
//body := api.E403.Format(r, "")
//api.SendError(w, r, &body)
// body := api.E403.Format(r, "")
// api.SendError(w, r, &body)
})
}
3 changes: 2 additions & 1 deletion pkg/auth/authz_middleware_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ func NewAuthzMiddlewareMock() AuthorizationMiddleware {

func (a authzMiddlewareMock) AuthorizeApi(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
logger.With(r.Context(), logger.HTTPMethod(r.Method), logger.HTTPPath(r.URL.Path)).Info("Mock authz allows <any>/<any> for method/path")
logger.With(r.Context(), logger.HTTPMethod(r.Method), logger.HTTPPath(r.URL.Path)).
Info("Mock authz allows <any>/<any> for method/path")
next.ServeHTTP(w, r)
})
}
12 changes: 6 additions & 6 deletions pkg/auth/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ func GetAuthPayloadFromContext(ctx context.Context) (*Payload, error) {
// TODO figure out how to unmarshal jwt.mapclaims into the struct to avoid all the
// type assertions
//
//var accountAuth api.AuthPayload
//err := json.Unmarshal([]byte(claims), &accountAuth)
//if err != nil {
// err := fmt.Errorf("Unable to parse JWT token claims")
// return nil, err
//}
// var accountAuth api.AuthPayload
// err := json.Unmarshal([]byte(claims), &accountAuth)
// if err != nil {
// err := fmt.Errorf("Unable to parse JWT token claims")
// return nil, err
// }

payload := &Payload{}
// default to the values we expect from RHSSO
Expand Down
Loading