Skip to content

Commit

Permalink
fix: readonly view (#1640)
Browse files Browse the repository at this point in the history
Signed-off-by: veds-g <[email protected]>
  • Loading branch information
veds-g authored Apr 5, 2024
1 parent 85e374d commit a629703
Show file tree
Hide file tree
Showing 16 changed files with 79 additions and 187 deletions.
2 changes: 1 addition & 1 deletion cmd/commands/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func NewServerCommand() *cobra.Command {
command.Flags().BoolVar(&namespaced, "namespaced", sharedutil.LookupEnvBoolOr("NUMAFLOW_SERVER_NAMESPACED", false), "Whether to run in namespaced scope, defaults to false.")
command.Flags().StringVar(&managedNamespace, "managed-namespace", sharedutil.LookupEnvStringOr("NUMAFLOW_SERVER_MANAGED_NAMESPACE", sharedutil.LookupEnvStringOr("NAMESPACE", "numaflow-system")), "The namespace that the server watches when \"--namespaced\" is \"true\".")
command.Flags().StringVar(&baseHref, "base-href", sharedutil.LookupEnvStringOr("NUMAFLOW_SERVER_BASE_HREF", "/"), "Base href for Numaflow server, defaults to '/'.")
command.Flags().BoolVar(&readOnly, "readonly", sharedutil.LookupEnvBoolOr("NUMAFLOW_SERVER_READONLY", true), "Whether to enable read only view for the UX server, defaults to false.")
command.Flags().BoolVar(&readOnly, "readonly", sharedutil.LookupEnvBoolOr("NUMAFLOW_SERVER_READONLY", false), "Whether to enable read only view for the UX server, defaults to false.")
command.Flags().BoolVar(&disableAuth, "disable-auth", sharedutil.LookupEnvBoolOr("NUMAFLOW_SERVER_DISABLE_AUTH", false), "Whether to disable authentication and authorization, defaults to false.")
command.Flags().StringVar(&serverAddr, "server-addr", sharedutil.LookupEnvStringOr("NUMAFLOW_SERVER_ADDRESS", "https://localhost:8443"), "The external address of the Numaflow server.")
return command
Expand Down
1 change: 0 additions & 1 deletion config/advanced-install/namespaced-controller-wo-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ apiVersion: v1
data:
namespaced: "true"
server.disable.auth: "true"
server.readonly: "false"
kind: ConfigMap
metadata:
name: numaflow-cmd-params-config
Expand Down
1 change: 0 additions & 1 deletion config/advanced-install/namespaced-numaflow-server.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ apiVersion: v1
data:
namespaced: "true"
server.disable.auth: "true"
server.readonly: "false"
kind: ConfigMap
metadata:
name: numaflow-cmd-params-config
Expand Down
1 change: 0 additions & 1 deletion config/advanced-install/numaflow-server.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ subjects:
apiVersion: v1
data:
server.disable.auth: "true"
server.readonly: "false"
kind: ConfigMap
metadata:
name: numaflow-cmd-params-config
Expand Down
2 changes: 1 addition & 1 deletion config/base/shared-config/numaflow-cmd-params-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ data:
# server.base.href: "/"
#
### Whether to enable read only view for the UX server, defaults to false.
server.readonly: "false"
# server.readonly: "false"
#
### Whether to disable authentication and authorization for the UX server, defaults to false.
server.disable.auth: "true"
Expand Down
1 change: 0 additions & 1 deletion config/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17083,7 +17083,6 @@ subjects:
apiVersion: v1
data:
server.disable.auth: "true"
server.readonly: "false"
kind: ConfigMap
metadata:
name: numaflow-cmd-params-config
Expand Down
1 change: 0 additions & 1 deletion config/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16987,7 +16987,6 @@ apiVersion: v1
data:
namespaced: "true"
server.disable.auth: "true"
server.readonly: "false"
kind: ConfigMap
metadata:
name: numaflow-cmd-params-config
Expand Down
52 changes: 51 additions & 1 deletion server/apis/v1/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,12 @@ type handler struct {
daemonClientsCache *lru.Cache[string, *daemonclient.DaemonClient]
dexObj *DexObject
localUsersAuthObject *LocalUsersAuthObject
isReadOnly bool
healthChecker *HealthChecker
}

// NewHandler is used to provide a new instance of the handler type
func NewHandler(ctx context.Context, dexObj *DexObject, localUsersAuthObject *LocalUsersAuthObject) (*handler, error) {
func NewHandler(ctx context.Context, dexObj *DexObject, localUsersAuthObject *LocalUsersAuthObject, isReadOnly bool) (*handler, error) {
var (
k8sRestConfig *rest.Config
err error
Expand All @@ -94,6 +95,7 @@ func NewHandler(ctx context.Context, dexObj *DexObject, localUsersAuthObject *Lo
daemonClientsCache: daemonClientsCache,
dexObj: dexObj,
localUsersAuthObject: localUsersAuthObject,
isReadOnly: isReadOnly,
healthChecker: NewHealthChecker(ctx),
}, nil
}
Expand Down Expand Up @@ -282,6 +284,12 @@ func (h *handler) GetClusterSummary(c *gin.Context) {

// CreatePipeline is used to create a given pipeline
func (h *handler) CreatePipeline(c *gin.Context) {
if h.isReadOnly {
errMsg := "Failed to perform this operation in read only mode"
c.JSON(http.StatusForbidden, NewNumaflowAPIResponse(&errMsg, nil))
return
}

ns := c.Param("namespace")
// dryRun is used to check if the operation is just a validation or an actual creation
dryRun := strings.EqualFold("true", c.DefaultQuery("dry-run", "false"))
Expand Down Expand Up @@ -410,6 +418,12 @@ func (h *handler) GetPipeline(c *gin.Context) {

// UpdatePipeline is used to update a given pipeline
func (h *handler) UpdatePipeline(c *gin.Context) {
if h.isReadOnly {
errMsg := "Failed to perform this operation in read only mode"
c.JSON(http.StatusForbidden, NewNumaflowAPIResponse(&errMsg, nil))
return
}

ns, pipeline := c.Param("namespace"), c.Param("pipeline")
// dryRun is used to check if the operation is just a validation or an actual update
dryRun := strings.EqualFold("true", c.DefaultQuery("dry-run", "false"))
Expand Down Expand Up @@ -460,6 +474,12 @@ func (h *handler) UpdatePipeline(c *gin.Context) {

// DeletePipeline is used to delete a given pipeline
func (h *handler) DeletePipeline(c *gin.Context) {
if h.isReadOnly {
errMsg := "Failed to perform this operation in read only mode"
c.JSON(http.StatusForbidden, NewNumaflowAPIResponse(&errMsg, nil))
return
}

ns, pipeline := c.Param("namespace"), c.Param("pipeline")

if err := h.numaflowClient.Pipelines(ns).Delete(context.Background(), pipeline, metav1.DeleteOptions{}); err != nil {
Expand All @@ -475,6 +495,12 @@ func (h *handler) DeletePipeline(c *gin.Context) {

// PatchPipeline is used to patch the pipeline spec to achieve operations such as "pause" and "resume"
func (h *handler) PatchPipeline(c *gin.Context) {
if h.isReadOnly {
errMsg := "Failed to perform this operation in read only mode"
c.JSON(http.StatusForbidden, NewNumaflowAPIResponse(&errMsg, nil))
return
}

ns, pipeline := c.Param("namespace"), c.Param("pipeline")

patchSpec, err := io.ReadAll(c.Request.Body)
Expand All @@ -498,6 +524,12 @@ func (h *handler) PatchPipeline(c *gin.Context) {
}

func (h *handler) CreateInterStepBufferService(c *gin.Context) {
if h.isReadOnly {
errMsg := "Failed to perform this operation in read only mode"
c.JSON(http.StatusForbidden, NewNumaflowAPIResponse(&errMsg, nil))
return
}

ns := c.Param("namespace")
// dryRun is used to check if the operation is just a validation or an actual update
dryRun := strings.EqualFold("true", c.DefaultQuery("dry-run", "false"))
Expand Down Expand Up @@ -567,6 +599,12 @@ func (h *handler) GetInterStepBufferService(c *gin.Context) {
}

func (h *handler) UpdateInterStepBufferService(c *gin.Context) {
if h.isReadOnly {
errMsg := "Failed to perform this operation in read only mode"
c.JSON(http.StatusForbidden, NewNumaflowAPIResponse(&errMsg, nil))
return
}

ns, isbsvcName := c.Param("namespace"), c.Param("isb-service")
// dryRun is used to check if the operation is just a validation or an actual update
dryRun := strings.EqualFold("true", c.DefaultQuery("dry-run", "false"))
Expand Down Expand Up @@ -603,6 +641,12 @@ func (h *handler) UpdateInterStepBufferService(c *gin.Context) {
}

func (h *handler) DeleteInterStepBufferService(c *gin.Context) {
if h.isReadOnly {
errMsg := "Failed to perform this operation in read only mode"
c.JSON(http.StatusForbidden, NewNumaflowAPIResponse(&errMsg, nil))
return
}

ns, isbsvcName := c.Param("namespace"), c.Param("isb-service")

pipelines, err := h.numaflowClient.Pipelines(ns).List(context.Background(), metav1.ListOptions{})
Expand Down Expand Up @@ -672,6 +716,12 @@ func (h *handler) respondWithError(c *gin.Context, message string) {

// UpdateVertex is used to update the vertex spec
func (h *handler) UpdateVertex(c *gin.Context) {
if h.isReadOnly {
errMsg := "Failed to perform this operation in read only mode"
c.JSON(http.StatusForbidden, NewNumaflowAPIResponse(&errMsg, nil))
return
}

var (
requestBody dfv1.AbstractVertex
inputVertexName = c.Param("vertex")
Expand Down
5 changes: 1 addition & 4 deletions server/cmd/server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,13 @@ func (s *server) Start(ctx context.Context) {
routes.SystemInfo{
ManagedNamespace: s.options.ManagedNamespace,
Namespaced: s.options.Namespaced,
IsReadOnly: s.options.ReadOnly,
Version: numaflow.GetVersion().String()},
routes.AuthInfo{
DisableAuth: s.options.DisableAuth,
DexServerAddr: s.options.DexServerAddr,
ServerAddr: s.options.ServerAddr,
},
routes.ReadOnlyInfo{
IsReadOnly: s.options.ReadOnly,
},
s.options.BaseHref,
authRouteMap,
)
Expand Down Expand Up @@ -156,7 +154,6 @@ func UrlRewrite(r *gin.Engine) gin.HandlerFunc {
func CreateAuthRouteMap(baseHref string) authz.RouteMap {
return authz.RouteMap{
"GET:" + baseHref + "api/v1/sysinfo": authz.NewRouteInfo(authz.ObjectPipeline, false),
"GET:" + baseHref + "api/v1/readonlyinfo": authz.NewRouteInfo(authz.ObjectEvents, false),
"GET:" + baseHref + "api/v1/authinfo": authz.NewRouteInfo(authz.ObjectEvents, false),
"GET:" + baseHref + "api/v1/namespaces": authz.NewRouteInfo(authz.ObjectEvents, false),
"GET:" + baseHref + "api/v1/cluster-summary": authz.NewRouteInfo(authz.ObjectPipeline, false),
Expand Down
18 changes: 6 additions & 12 deletions server/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
type SystemInfo struct {
ManagedNamespace string `json:"managedNamespace"`
Namespaced bool `json:"namespaced"`
IsReadOnly bool `json:"isReadOnly"`
Version string `json:"version"`
}

Expand All @@ -42,11 +43,7 @@ type AuthInfo struct {
ServerAddr string `json:"serverAddr"`
}

type ReadOnlyInfo struct {
IsReadOnly bool `json:"isReadOnly"`
}

func Routes(ctx context.Context, r *gin.Engine, sysInfo SystemInfo, authInfo AuthInfo, readOnlyInfo ReadOnlyInfo, baseHref string, authRouteMap authz.RouteMap) {
func Routes(ctx context.Context, r *gin.Engine, sysInfo SystemInfo, authInfo AuthInfo, baseHref string, authRouteMap authz.RouteMap) {
r.GET("/livez", func(c *gin.Context) {
c.Status(http.StatusOK)
})
Expand Down Expand Up @@ -74,13 +71,10 @@ func Routes(ctx context.Context, r *gin.Engine, sysInfo SystemInfo, authInfo Aut
}
// Add the AuthN/AuthZ middleware to the group.
r1Group.Use(authMiddleware(ctx, authorizer, dexObj, localUsersAuthObj, authRouteMap))
v1Routes(ctx, r1Group, dexObj, localUsersAuthObj)
v1Routes(ctx, r1Group, dexObj, localUsersAuthObj, sysInfo.IsReadOnly)
} else {
v1Routes(ctx, r1Group, nil, nil)
v1Routes(ctx, r1Group, nil, nil, sysInfo.IsReadOnly)
}
r1Group.GET("/readonlyinfo", func(c *gin.Context) {
c.JSON(http.StatusOK, v1.NewNumaflowAPIResponse(nil, readOnlyInfo))
})
r1Group.GET("/sysinfo", func(c *gin.Context) {
c.JSON(http.StatusOK, v1.NewNumaflowAPIResponse(nil, sysInfo))
})
Expand All @@ -103,8 +97,8 @@ func v1RoutesNoAuth(r gin.IRouter, dexObj *v1.DexObject, localUsersAuthObject *v

// v1Routes defines the routes for the v1 API. For adding a new route, add a new handler function
// for the route along with an entry in the RouteMap in auth/route_map.go.
func v1Routes(ctx context.Context, r gin.IRouter, dexObj *v1.DexObject, localUsersAuthObject *v1.LocalUsersAuthObject) {
handler, err := v1.NewHandler(ctx, dexObj, localUsersAuthObject)
func v1Routes(ctx context.Context, r gin.IRouter, dexObj *v1.DexObject, localUsersAuthObject *v1.LocalUsersAuthObject, isReadOnly bool) {
handler, err := v1.NewHandler(ctx, dexObj, localUsersAuthObject, isReadOnly)
if err != nil {
panic(err)
}
Expand Down
6 changes: 2 additions & 4 deletions server/routes/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,16 @@ func TestRoutes(t *testing.T) {
sysInfo := SystemInfo{
ManagedNamespace: managedNamespace,
Namespaced: namespaced,
IsReadOnly: false,
}

authInfo := AuthInfo{
DisableAuth: false,
DexServerAddr: "test-dex-server-addr",
}

readOnlyInfo := ReadOnlyInfo{
IsReadOnly: false,
}
authRouteMap := authz.RouteMap{}
Routes(logging.WithLogger(signals.SetupSignalHandler(), log), router, sysInfo, authInfo, readOnlyInfo, "/", authRouteMap)
Routes(logging.WithLogger(signals.SetupSignalHandler(), log), router, sysInfo, authInfo, "/", authRouteMap)
t.Run("/404", func(t *testing.T) {
w := httptest.NewRecorder()
req, err := http.NewRequest(http.MethodGet, "/404", nil)
Expand Down
53 changes: 4 additions & 49 deletions ui/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { Breadcrumbs } from "./components/common/Breadcrumbs";
import { Routes } from "./components/common/Routes";
import { Login } from "./components/pages/Login";
import { useSystemInfoFetch } from "./utils/fetchWrappers/systemInfoFetch";
import { useReadOnlyInfoFetch } from "./utils/fetchWrappers/readOnlyInfoFetch";
import { notifyError } from "./utils/error";
import {
SlidingSidebar,
Expand Down Expand Up @@ -90,12 +89,6 @@ function App(props: AppProps) {
loading,
} = useSystemInfoFetch({ host: hostUrl });

const {
readOnlyInfo,
error: readOnlyInfoError,
loading: readOnlyInfoLoading,
} = useReadOnlyInfoFetch({ host: hostUrl });

const location = useLocation();

useEffect(() => {
Expand Down Expand Up @@ -154,18 +147,6 @@ function App(props: AppProps) {
}
}, [systemInfoError]);

// Notify if error loading read only info
useEffect(() => {
if (readOnlyInfoError) {
notifyError([
{
error: readOnlyInfoError,
options: { toastId: "read-only-info-fetch", autoClose: 5000 },
},
]);
}
}, [readOnlyInfoError]);

const handleSideBarClose = useCallback(() => {
setSidebarCloseIndicator("id" + Math.random().toString(16).slice(2));
}, []);
Expand All @@ -185,8 +166,8 @@ function App(props: AppProps) {
}, []);

const routes = useMemo(() => {
if (loading || readOnlyInfoLoading) {
// System info or Read Only info loading
if (loading) {
// System info loading
return (
<Box
sx={{
Expand Down Expand Up @@ -222,26 +203,6 @@ function App(props: AppProps) {
</Box>
);
}
if (readOnlyInfoError) {
// Read Only info load error
return (
<Box
sx={{
display: "flex",
flexDirection: "column",
width: "100%",
height: "100%",
justifyContent: "center",
alignItems: "center",
}}
>
<ErrorDisplay
title="Error loading read only info"
message={readOnlyInfoError}
/>
</Box>
);
}
if (systemInfo && systemInfo?.namespaced) {
// Namespaced installation routing
return (
Expand Down Expand Up @@ -276,13 +237,7 @@ function App(props: AppProps) {
</Route>
</Switch>
);
}, [
systemInfo,
systemInfoError,
loading,
readOnlyInfoError,
readOnlyInfoLoading,
]);
}, [systemInfo, systemInfoError, loading]);

return (
<div ref={pageRef} className="app-container">
Expand All @@ -293,7 +248,7 @@ function App(props: AppProps) {
host: hostUrl,
namespace,
isPlugin: false,
isReadOnly: readOnlyInfo?.isReadOnly || false,
isReadOnly: systemInfo?.isReadOnly || false,
sidebarProps,
setSidebarProps,
errors,
Expand Down
Loading

0 comments on commit a629703

Please sign in to comment.