Skip to content

Commit

Permalink
Code cleanup (#5312)
Browse files Browse the repository at this point in the history
# Description

Most code inside ucp/frontend/controller.go is not needed and is
replaced by armrpc/frontend/controller.go. Removing unwanted code

#5175 

<!--
We strive to have all PR being opened based on an issue, where the
problem or feature have been discussed prior to implementation.
-->

Fixes: #5175 

## Checklist

Please make sure you've completed the relevant tasks for this PR, out of
the following list:

* [ ] Code compiles correctly
* [ ] Adds necessary unit tests for change
* [ ] Adds necessary E2E tests for change
* [ ] Unit tests passing
* [ ] Extended the documentation / Created issue for it
  • Loading branch information
vinayada1 authored Mar 24, 2023
1 parent 9b48ca1 commit 24608f4
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 202 deletions.
3 changes: 1 addition & 2 deletions pkg/ucp/frontend/api/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,9 @@ func Register(ctx context.Context, router *mux.Router, ctrlOpts ctrl.Options) er
HandlerFactory: kubernetes_ctrl.NewDiscoveryDoc,
},
}...)

}

ctrl.ConfigureDefaultHandlers(router, ctrlOpts)
ctrl.ConfigureDefaultHandlers(router, ctrlOpts.Options)

logger := ucplog.FromContextOrDiscard(ctx)
logger.Info(fmt.Sprintf("Registering routes with base path: %s", baseURL))
Expand Down
169 changes: 7 additions & 162 deletions pkg/ucp/frontend/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,15 @@ package controller

import (
"context"
"fmt"
"io"
"net/http"
"strings"

"github.com/gorilla/mux"
v1 "github.com/project-radius/radius/pkg/armrpc/api/v1"
armrpc_controller "github.com/project-radius/radius/pkg/armrpc/frontend/controller"
armrpc_rest "github.com/project-radius/radius/pkg/armrpc/rest"
"github.com/project-radius/radius/pkg/armrpc/frontend/server"
ucp_aws "github.com/project-radius/radius/pkg/ucp/aws"
"github.com/project-radius/radius/pkg/ucp/resources"
"github.com/project-radius/radius/pkg/ucp/secret"
"github.com/project-radius/radius/pkg/ucp/store"
"github.com/project-radius/radius/pkg/ucp/ucplog"
"github.com/project-radius/radius/pkg/validator"
)

// Options represents controller options.
Expand Down Expand Up @@ -59,18 +54,6 @@ type HandlerOptions struct {
HandlerFactory ControllerFunc
}

// BaseController is the base operation controller.
type BaseController struct {
Options Options
}

// NewBaseController creates BaseController instance.
func NewBaseController(options Options) BaseController {
return BaseController{
options,
}
}

func RegisterHandler(ctx context.Context, opts HandlerOptions, ctrlOpts Options) error {
storageClient, err := ctrlOpts.DataProvider.GetStorageClient(ctx, opts.ResourceType)
if err != nil {
Expand All @@ -89,13 +72,13 @@ func RegisterHandler(ctx context.Context, opts HandlerOptions, ctrlOpts Options)

response, err := ctrl.Run(ctx, w, req)
if err != nil {
HandleError(ctx, w, req, err)
server.HandleError(ctx, w, req, err)
return
}
if response != nil {
err = response.Apply(ctx, w, req)
if err != nil {
HandleError(ctx, w, req, err)
server.HandleError(ctx, w, req, err)
return
}
}
Expand All @@ -111,145 +94,7 @@ func RegisterHandler(ctx context.Context, opts HandlerOptions, ctrlOpts Options)
return nil
}

// StorageClient gets storage client for this controller.
func (b *BaseController) StorageClient() store.StorageClient {
return b.Options.StorageClient
}

// GetResource is the helper to get the resource via storage client.
func (c *BaseController) GetResource(ctx context.Context, id string, out any) (etag string, err error) {
etag = ""
var res *store.Object
if res, err = c.StorageClient().Get(ctx, id); err == nil && res != nil {
if err = res.As(out); err == nil {
etag = res.ETag
return
}
}
return
}

// SaveResource is the helper to save the resource via storage client.
func (c *BaseController) SaveResource(ctx context.Context, id string, in any, etag string) (*store.Object, error) {
nr := &store.Object{
Metadata: store.Metadata{
ID: id,
},
Data: in,
}
err := c.StorageClient().Save(ctx, nr, store.WithETag(etag))
if err != nil {
return nil, err
}
return nr, nil
}

// DeleteResource is the helper to delete the resource via storage client.
func (c *BaseController) DeleteResource(ctx context.Context, id string, etag string) error {
err := c.StorageClient().Delete(ctx, id, store.WithETag(etag))
if err != nil {
return err
}
return nil
}

// Responds with an HTTP 500
func HandleError(ctx context.Context, w http.ResponseWriter, req *http.Request, err error) {
logger := ucplog.FromContextOrDiscard(ctx)

var response armrpc_rest.Response
// Try to use the ARM format to send back the error info
// if the error is due to api conversion failure return bad request
switch v := err.(type) {
case *v1.ErrModelConversion:
response = armrpc_rest.NewBadRequestARMResponse(v1.ErrorResponse{
Error: v1.ErrorDetails{
Code: v1.CodeHTTPRequestPayloadAPISpecValidationFailed,
Message: err.Error(),
},
})
case *v1.ErrClientRP:
response = armrpc_rest.NewBadRequestARMResponse(v1.ErrorResponse{
Error: v1.ErrorDetails{
Code: v.Code,
Message: v.Message,
},
})
default:
if err.Error() == v1.ErrInvalidModelConversion.Error() {
response = armrpc_rest.NewBadRequestARMResponse(v1.ErrorResponse{
Error: v1.ErrorDetails{
Code: v1.CodeHTTPRequestPayloadAPISpecValidationFailed,
Message: err.Error(),
},
})
} else {
logger.V(ucplog.Debug).Error(err, "unhandled error")
response = armrpc_rest.NewInternalServerErrorARMResponse(v1.ErrorResponse{
Error: v1.ErrorDetails{
Code: v1.CodeInternal,
Message: err.Error(),
},
})
}
}
err = response.Apply(ctx, w, req)
if err != nil {
body := v1.ErrorResponse{
Error: v1.ErrorDetails{
Code: v1.CodeInternal,
Message: err.Error(),
},
}
// There's no way to recover if we fail writing here, we likly partially wrote to the response stream.
w.WriteHeader(http.StatusInternalServerError)
logger.Error(err, fmt.Sprintf("error writing marshaled %T bytes to output", body))
}
}

func (b *BaseController) GetRelativePath(path string) string {
trimmedPath := strings.TrimPrefix(path, b.Options.BasePath)
return trimmedPath
}

func (b *BaseController) NotFoundHandler(w http.ResponseWriter, r *http.Request) {
path := b.GetRelativePath(r.URL.Path)
restResponse := armrpc_rest.NewNoResourceMatchResponse(path)
err := restResponse.Apply(r.Context(), w, r)
if err != nil {
HandleError(r.Context(), w, r, err)
return
}
}

func (b *BaseController) MethodNotAllowedHandler(w http.ResponseWriter, r *http.Request) {
path := b.GetRelativePath(r.URL.Path)
target := ""
if rID, err := resources.Parse(path); err == nil {
target = rID.Type() + "/" + rID.Name()
}
restResponse := armrpc_rest.NewMethodNotAllowedResponse(target, fmt.Sprintf("The request method '%s' is invalid.", r.Method))
if err := restResponse.Apply(r.Context(), w, r); err != nil {
HandleError(r.Context(), w, r, err)
}
}

func ReadRequestBody(req *http.Request) ([]byte, error) {
defer req.Body.Close()
data, err := io.ReadAll(req.Body)
if err != nil {
return nil, fmt.Errorf("error reading request body: %w", err)
}
return data, nil
}

func ConfigureDefaultHandlers(router *mux.Router, opts Options) {
b := NewBaseController(opts)
router.NotFoundHandler = http.HandlerFunc(b.NotFoundHandler)
router.MethodNotAllowedHandler = http.HandlerFunc(b.MethodNotAllowedHandler)
}

// GetAPIVersion extracts the API version from the request
func GetAPIVersion(req *http.Request) string {
return req.URL.Query().Get("api-version")
func ConfigureDefaultHandlers(router *mux.Router, opts armrpc_controller.Options) {
router.NotFoundHandler = validator.APINotFoundHandler()
router.MethodNotAllowedHandler = validator.APIMethodNotAllowedHandler()
}
10 changes: 5 additions & 5 deletions pkg/ucp/frontend/controller/kubernetes/discoverydoc.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,21 @@ import (
http "net/http"

armrpc_controller "github.com/project-radius/radius/pkg/armrpc/frontend/controller"
"github.com/project-radius/radius/pkg/armrpc/frontend/server"
armrpc_rest "github.com/project-radius/radius/pkg/armrpc/rest"
"github.com/project-radius/radius/pkg/ucp/frontend/controller"
ctrl "github.com/project-radius/radius/pkg/ucp/frontend/controller"
)

var _ armrpc_controller.Controller = (*DiscoveryDoc)(nil)

// DiscoveryDoc is the controller implementation to handle the discovery document.
type DiscoveryDoc struct {
ctrl.BaseController
armrpc_controller.BaseController
}

// NewDiscoveryDoc creates a new DiscoveryDoc.
func NewDiscoveryDoc(opts ctrl.Options) (armrpc_controller.Controller, error) {
return &DiscoveryDoc{ctrl.NewBaseController(opts)}, nil
return &DiscoveryDoc{armrpc_controller.NewBaseController(opts.Options)}, nil
}

func (e *DiscoveryDoc) Run(ctx context.Context, w http.ResponseWriter, req *http.Request) (armrpc_rest.Response, error) {
Expand All @@ -43,7 +43,7 @@ func (e *DiscoveryDoc) Run(ctx context.Context, w http.ResponseWriter, req *http
"resources": []any{},
})
if err != nil {
controller.HandleError(ctx, w, req, err)
server.HandleError(ctx, w, req, err)
return nil, nil
}

Expand All @@ -52,7 +52,7 @@ func (e *DiscoveryDoc) Run(ctx context.Context, w http.ResponseWriter, req *http

_, err = w.Write(b)
if err != nil {
controller.HandleError(ctx, w, req, err)
server.HandleError(ctx, w, req, err)
return nil, nil
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/ucp/frontend/controller/kubernetes/openapiv2doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,21 @@ import (
http "net/http"

armrpc_controller "github.com/project-radius/radius/pkg/armrpc/frontend/controller"
"github.com/project-radius/radius/pkg/armrpc/frontend/server"
armrpc_rest "github.com/project-radius/radius/pkg/armrpc/rest"
"github.com/project-radius/radius/pkg/ucp/frontend/controller"
ctrl "github.com/project-radius/radius/pkg/ucp/frontend/controller"
)

var _ armrpc_controller.Controller = (*OpenAPIv2Doc)(nil)

// OpenAPIv2Doc is the controller implementation to handle the OpenAPIv2Doc endpoint.
type OpenAPIv2Doc struct {
ctrl.BaseController
armrpc_controller.BaseController
}

// NewOpenAPIv2Doc creates a new OpenAPIv2Doc.
func NewOpenAPIv2Doc(opts ctrl.Options) (armrpc_controller.Controller, error) {
return &OpenAPIv2Doc{ctrl.NewBaseController(opts)}, nil
return &OpenAPIv2Doc{armrpc_controller.NewBaseController(opts.Options)}, nil
}

func (e *OpenAPIv2Doc) Run(ctx context.Context, w http.ResponseWriter, req *http.Request) (armrpc_rest.Response, error) {
Expand All @@ -45,7 +45,7 @@ func (e *OpenAPIv2Doc) Run(ctx context.Context, w http.ResponseWriter, req *http
"paths": map[string]any{},
})
if err != nil {
controller.HandleError(ctx, w, req, err)
server.HandleError(ctx, w, req, err)
return nil, nil
}

Expand All @@ -54,7 +54,7 @@ func (e *OpenAPIv2Doc) Run(ctx context.Context, w http.ResponseWriter, req *http

_, err = w.Write(b)
if err != nil {
controller.HandleError(ctx, w, req, err)
server.HandleError(ctx, w, req, err)
return nil, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/ucp/frontend/controller/planes/listplanes.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (e *ListPlanes) Run(ctx context.Context, w http.ResponseWriter, req *http.R
}

func (p *ListPlanes) createResponse(ctx context.Context, req *http.Request, result *store.ObjectQueryResult) ([]any, error) {
apiVersion := ctrl.GetAPIVersion(req)
serviceCtx := v1.ARMRequestContextFromContext(ctx)
listOfPlanes := []any{}
if len(result.Items) > 0 {
for _, item := range result.Items {
Expand All @@ -72,7 +72,7 @@ func (p *ListPlanes) createResponse(ctx context.Context, req *http.Request, resu
return nil, err
}

versioned, err := converter.PlaneDataModelToVersioned(&plane, apiVersion)
versioned, err := converter.PlaneDataModelToVersioned(&plane, serviceCtx.APIVersion)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/ucp/frontend/controller/planes/listplanesbytype.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (e *ListPlanesByType) Run(ctx context.Context, w http.ResponseWriter, req *
}

func (p *ListPlanesByType) createResponse(ctx context.Context, req *http.Request, result *store.ObjectQueryResult) ([]any, error) {
apiVersion := ctrl.GetAPIVersion(req)
serviceCtx := v1.ARMRequestContextFromContext(ctx)
listOfPlanes := []any{}
if len(result.Items) > 0 {
for _, item := range result.Items {
Expand All @@ -80,7 +80,7 @@ func (p *ListPlanesByType) createResponse(ctx context.Context, req *http.Request
return nil, err
}

versioned, err := converter.PlaneDataModelToVersioned(&plane, apiVersion)
versioned, err := converter.PlaneDataModelToVersioned(&plane, serviceCtx.APIVersion)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit 24608f4

Please sign in to comment.