Warn when setting up a system with local or mem block adapters #2002

arielshaqed · 2021-05-26T06:58:39Z

These are great for quick setup, but should not be used in production. Warn about it 3 ways:

UI

On the setup page:

Terminal

During startup:


 ██       ██                           ██                 
░██      ░██                          ░░                  ░██
░██   █  ░██  ██████   ██████ ███████  ██ ███████   █████ ░██
░██  ███ ░██ ░░░░░░██ ░░██░░█░░██░░░██░██░░██░░░██ ██░░░██░██
░██ ██░██░██  ███████  ░██ ░  ░██  ░██░██ ░██  ░██░██  ░██░██
░████ ░░████ ██░░░░██  ░██    ░██  ░██░██ ░██  ░██░░██████░░ 
░██░   ░░░██░░████████░███    ███  ░██░██ ███  ░██ ░░░░░██ ██
░░       ░░  ░░░░░░░░ ░░░    ░░░   ░░ ░░ ░░░   ░░   █████ ░░ 
                                                   ░░░░░  
Using the "local" block adapter.  This is suitable only for testing, but not
for production.

Logs

Also write an error on startup:

ERROR  [2021-05-26T09:54:55+03:00]cmd/lakefs/cmd/run.go:112 cmd/lakefs/cmd.glob..func9 Block adapter NOT SUPPORTED for production use  adapter_type=local

ozkatz · 2021-06-03T14:40:54Z

cmd/lakefs/cmd/run.go

+░██ ██░██░██  ███████  ░██ ░  ░██  ░██░██ ░██  ░██░██  ░██░██
+░████ ░░████ ██░░░░██  ░██    ░██  ░██░██ ░██  ░██░░██████░░ 
+░██░   ░░░██░░████████░███    ███  ░██░██ ███  ░██ ░░░░░██ ██
+░░       ░░  ░░░░░░░░ ░░░    ░░░   ░░ ░░ ░░░   ░░   █████ ░░ 


A bit much, given our quickstart will also show this. I'd do without the big banner..

Done. Let me know if you want a small figlet banner...

ozkatz · 2021-06-03T14:43:10Z

docs/assets/js/swagger.yml

@@ -2821,3 +2844,20 @@ paths:
                $ref: "#/components/schemas/Config"
        401:
          $ref: "#/components/responses/Unauthorized"
+
+  /warnings:


We already have /config: Since this is currently only used in the UI, why not have the warning be a client side feature? The startup message already doesn't depend on this API...

Moved it to be part of the config. But still generating on the server side: not all warnings can be generated on the client. In any case the GUI client is pretty well tied to the server.

Thing is, now it requires auth so we cannot place it on /setup. Will consult f2f where to place it.

ozkatz · 2021-06-03T14:44:52Z

webui/src/lib/components/controls.jsx

+
+export const Warning = (props) =>
+<div className="warning">


Suggested change

<div className="warning">

<Alert variant="warning">

ozkatz · 2021-06-03T14:45:37Z

webui/src/lib/components/navbar.jsx

-                <TopNavLink href="/repositories">Repositories</TopNavLink>
-                <TopNavLink href="/auth">Administration</TopNavLink>
-            </Nav>
+	<>


Hysterical raisins. Removed...

ozkatz · 2021-06-03T14:45:56Z

webui/src/pages/index.jsx

-                </Route>
-            </Switch>
-        </Router>
+	<>


Same leftovers.

ozkatz · 2021-06-03T14:46:50Z

webui/src/pages/setup/index.jsx

@@ -108,4 +126,9 @@ server:
    );
 };

+const SetupPage = () => <>
+			    <SetupWarnings/>


I think it looks a little out of place. At a minimum it should be on the same grid as the setup form (above it, below it, idc)

As discussed F2F, anyway moved to:

"Create repo" dialog

"Objects" page

On the "create repo" modal on the repositories page:

On the "upload object" modal on the objects page:

ozkatz · 2021-06-03T14:47:14Z

webui/src/styles/globals.css

@@ -300,6 +300,16 @@
  font-size: 0.85rem;
 }

+.warning {


let's use the react-bootstrap component (<Alert variant="warning"/>)

arielshaqed

Thanks!

PTAL (commits mostly organized and non-overlapping, you might want to read them one at a time).

arielshaqed · 2021-06-06T07:24:12Z

cmd/lakefs/cmd/run.go

+░██ ██░██░██  ███████  ░██ ░  ░██  ░██░██ ░██  ░██░██  ░██░██
+░████ ░░████ ██░░░░██  ░██    ░██  ░██░██ ░██  ░██░░██████░░ 
+░██░   ░░░██░░████████░███    ███  ░██░██ ███  ░██ ░░░░░██ ██
+░░       ░░  ░░░░░░░░ ░░░    ░░░   ░░ ░░ ░░░   ░░   █████ ░░ 


Done. Let me know if you want a small figlet banner...

arielshaqed · 2021-06-06T07:26:22Z

docs/assets/js/swagger.yml

@@ -2821,3 +2844,20 @@ paths:
                $ref: "#/components/schemas/Config"
        401:
          $ref: "#/components/responses/Unauthorized"
+
+  /warnings:


Moved it to be part of the config. But still generating on the server side: not all warnings can be generated on the client. In any case the GUI client is pretty well tied to the server.

Thing is, now it requires auth so we cannot place it on /setup. Will consult f2f where to place it.

arielshaqed · 2021-06-06T07:57:44Z

webui/src/lib/components/navbar.jsx

-                <TopNavLink href="/repositories">Repositories</TopNavLink>
-                <TopNavLink href="/auth">Administration</TopNavLink>
-            </Nav>
+	<>


Hysterical raisins. Removed...

arielshaqed · 2021-06-06T07:58:03Z

webui/src/pages/index.jsx

-                </Route>
-            </Switch>
-        </Router>
+	<>


Same leftovers.

arielshaqed · 2021-06-06T09:39:03Z

webui/src/styles/globals.css

@@ -300,6 +300,16 @@
  font-size: 0.85rem;
 }

+.warning {


arielshaqed · 2021-06-06T09:39:36Z

webui/src/pages/setup/index.jsx

@@ -108,4 +126,9 @@ server:
    );
 };

+const SetupPage = () => <>
+			    <SetupWarnings/>


As discussed F2F, anyway moved to:

"Create repo" dialog

"Objects" page

arielshaqed · 2021-06-06T10:29:47Z

webui/src/pages/setup/index.jsx

@@ -108,4 +126,9 @@ server:
    );
 };

+const SetupPage = () => <>
+			    <SetupWarnings/>


On the "create repo" modal on the repositories page:

arielshaqed · 2021-06-06T11:00:53Z

webui/src/pages/setup/index.jsx

@@ -108,4 +126,9 @@ server:
    );
 };

+const SetupPage = () => <>
+			    <SetupWarnings/>


On the "upload object" modal on the objects page:

lgtm-com · 2021-06-06T11:50:01Z

This pull request introduces 3 alerts when merging 2e843eda4740abcb15d66bb59d72d1eedbc186b3 into 18b9afc - view on LGTM.com

new alerts:

3 for Unused variable, import, function or class

arielshaqed · 2021-06-07T07:47:15Z

This pull request introduces 3 alerts when merging 2e843ed into 18b9afc - view on LGTM.com

new alerts:
* 3 for Unused variable, import, function or class

These are fixed, see new report on LGTM.com.

ozkatz

I'm worried about incompatible changes with #2056

ozkatz · 2021-06-07T08:20:03Z

api/swagger.yml

@@ -681,6 +681,11 @@ components:
          type: string
        blockstore_namespace_ValidityRegex:
          type: string
+        warnings:


This kind of conflicts with #2056 - I think we should fold both into a /config/storage endpoint and forgo the server-side warnings API (for now).

ozkatz · 2021-06-07T08:24:29Z

webui/src/styles/globals.css

@@ -300,6 +300,11 @@
  font-size: 0.85rem;
 }

+.unmarked-list {


for classes with no semantic meaning that simply want to apply a simple style, you can use Bootstrap's spacing classes (see https://getbootstrap.com/docs/4.0/utilities/spacing/#examples)

Cool. Done!

arielshaqed · 2021-06-07T15:13:21Z

@ozkatz , I cannot pull this because:

I cannot pull make authorization errors readable in the UI, change /config to require re fs permissions since it returns storage config #2056 because that fails validation. I did rebase on top of fix/2028-uninformative-auth-errors.
You did not approve this one.

Let's (finally...) close this tomorrow!

arielshaqed

Thanks!

Rebased on top of master. @ozkatz , PT1FL ("please take one final look")...

ozkatz

Looks great! thanks!

Currently only shows warning for using local or mem block storages.

Currently this warns when setting up with local or mem block adapter.

Not a huge banner.

Adding during branch development to add components but never removed.

Also use bootstrap alert warnings

Thanks, LGTM.com!

CSS properties in JSX should be whackyCapped, of course, not thread-cased.

ASI FTL :-(

arielshaqed · 2021-06-09T13:54:38Z

Thanks!

arielshaqed requested a review from ozkatz May 26, 2021 06:58

arielshaqed linked an issue May 26, 2021 that may be closed by this pull request

Warn about using "local" adapter in production #1998

Closed

arielshaqed force-pushed the chore/1998-warn-about-local branch 2 times, most recently from d029112 to c8ff5d9 Compare May 26, 2021 07:38

arielshaqed force-pushed the chore/1998-warn-about-local branch from c8ff5d9 to 54c338b Compare June 1, 2021 14:03

ozkatz requested changes Jun 3, 2021

View reviewed changes

arielshaqed force-pushed the chore/1998-warn-about-local branch from 5982712 to 8d6f34b Compare June 6, 2021 11:04

arielshaqed commented Jun 6, 2021

View reviewed changes

arielshaqed requested a review from ozkatz June 6, 2021 11:05

ozkatz requested changes Jun 7, 2021

View reviewed changes

arielshaqed force-pushed the chore/1998-warn-about-local branch from e717b32 to 6164ae1 Compare June 7, 2021 15:02

arielshaqed mentioned this pull request Jun 7, 2021

make authorization errors readable in the UI, change /config to require re fs permissions since it returns storage config #2056

Merged

arielshaqed changed the base branch from master to fix/2028-uninformative-auth-errors June 7, 2021 15:12

arielshaqed force-pushed the chore/1998-warn-about-local branch from 6164ae1 to fe4dee5 Compare June 8, 2021 10:44

arielshaqed changed the base branch from fix/2028-uninformative-auth-errors to master June 8, 2021 11:04

arielshaqed commented Jun 8, 2021

View reviewed changes

arielshaqed requested a review from ozkatz June 8, 2021 11:08

ozkatz approved these changes Jun 8, 2021

View reviewed changes

ozkatz and others added 7 commits June 9, 2021 16:27

update swagger clients (breaking!) and fix tests

da0f951

rename /config to /config/storage, align permission name

f93be83

Warn on stderr when starting up with local or mem block adapter

52efb5a

Add "warnings endpoint" for UI

1d4526e

Currently only shows warning for using local or mem block storages.

Warn on UI when setting up a poorly configured system

8479ae2

Currently this warns when setting up with local or mem block adapter.

[CR] Small "Warning"

af509ab

Not a huge banner.

[CR] Remove JSX null containers

6b2fc16

Adding during branch development to add components but never removed.

arielshaqed added 10 commits June 9, 2021 16:27

[CR] Move warnings to (authed) upload object / create repo modals

f40f7c3

Also use bootstrap alert warnings

[CR] Report warnings in "get config" API

d4f717a

[CR] Move warnings to *top* of "create repo" popup

6ec0763

[CR] Report get config error in upload object; remove unused import

8fe43c2

Thanks, LGTM.com!

[CR] Use Bootstrap classes to remove left margin and padding

ec22831

Silence console errors on thread-cased CSS properties in JSX

e039a1d

CSS properties in JSX should be whackyCapped, of course, not thread-cased.

Include block storage adapter type in UI storageConfig service

e5e40b4

Add and display warnings for non-prod storage adapters on client

4f9eb3a

Whitespace and semicolon fixes

ba6afb1

ASI FTL :-(

make gen

9701ccc

arielshaqed force-pushed the chore/1998-warn-about-local branch from 6da3c48 to 9701ccc Compare June 9, 2021 13:30

gofmt

bea5cc4

arielshaqed merged commit 3fa9391 into master Jun 9, 2021

arielshaqed deleted the chore/1998-warn-about-local branch June 9, 2021 13:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warn when setting up a system with local or mem block adapters #2002

Warn when setting up a system with local or mem block adapters #2002

arielshaqed commented May 26, 2021

ozkatz Jun 3, 2021

arielshaqed Jun 6, 2021

ozkatz Jun 3, 2021

arielshaqed Jun 6, 2021

ozkatz Jun 3, 2021

ozkatz Jun 3, 2021

arielshaqed Jun 6, 2021

ozkatz Jun 3, 2021

arielshaqed Jun 6, 2021

ozkatz Jun 3, 2021

arielshaqed Jun 6, 2021

arielshaqed Jun 6, 2021

arielshaqed Jun 6, 2021

ozkatz Jun 3, 2021

arielshaqed Jun 6, 2021

arielshaqed left a comment

arielshaqed Jun 6, 2021

arielshaqed Jun 6, 2021

arielshaqed Jun 6, 2021

arielshaqed Jun 6, 2021

arielshaqed Jun 6, 2021

arielshaqed Jun 6, 2021

arielshaqed Jun 6, 2021

arielshaqed Jun 6, 2021

lgtm-com bot commented Jun 6, 2021

arielshaqed commented Jun 7, 2021

ozkatz left a comment

ozkatz Jun 7, 2021

ozkatz Jun 7, 2021

arielshaqed Jun 7, 2021

arielshaqed commented Jun 7, 2021

arielshaqed left a comment

ozkatz left a comment

arielshaqed commented Jun 9, 2021

Warn when setting up a system with local or mem block adapters #2002

Warn when setting up a system with local or mem block adapters #2002

Conversation

arielshaqed commented May 26, 2021

UI

Terminal

Logs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielshaqed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Jun 6, 2021

arielshaqed commented Jun 7, 2021

ozkatz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielshaqed commented Jun 7, 2021

arielshaqed left a comment

Choose a reason for hiding this comment

ozkatz left a comment

Choose a reason for hiding this comment

arielshaqed commented Jun 9, 2021