Adding create keys to tenant creation wizard #697

adfost · 2021-04-16T20:08:54Z

No description provided.

dvaldivia

Can you also change the title and description of the PR to better reflect what's being introduced and the task at hand?

kubectl.sha256

portal-ui/src/screens/Console/Tenants/AddTenant/AddTenant.tsx

portal-ui/src/screens/Console/Tenants/AddTenant/Steps/IdentityProvider.tsx

restapi/admin_tenants.go

swagger.yml

test.html

dvaldivia

Read comments above

bexsoft · 2021-04-16T23:53:21Z

portal-ui/src/screens/Console/Tenants/AddTenant/AddTenant.tsx

    const ADGroupBaseDN = fields.identityProvider.ADGroupBaseDN;
    const ADGroupSearchFilter = fields.identityProvider.ADGroupSearchFilter;
    const ADNameAttribute = fields.identityProvider.ADNameAttribute;
+    const accesskeys = fields.identityProvider.accesskeys;


Try to always use camel case variables:

Suggested change

const accesskeys = fields.identityProvider.accesskeys;

const accessKeys = fields.identityProvider.accesskeys;

bexsoft · 2021-04-16T23:53:34Z

portal-ui/src/screens/Console/Tenants/AddTenant/AddTenant.tsx

    const ADGroupSearchFilter = fields.identityProvider.ADGroupSearchFilter;
    const ADNameAttribute = fields.identityProvider.ADNameAttribute;
+    const accesskeys = fields.identityProvider.accesskeys;
+    const secretkeys = fields.identityProvider.secretkeys;


Same case:

Suggested change

const secretkeys = fields.identityProvider.secretkeys;

const secretKeys = fields.identityProvider.secretkeys;

bexsoft · 2021-04-16T23:54:45Z

portal-ui/src/screens/Console/Tenants/AddTenant/Steps/IdentityProvider.tsx

 interface IIdentityProviderProps {
  classes: any;
  idpSelection: string;
+  accesskeys: string[];


Suggested change

accesskeys: string[];

accessKeys: string[];

bexsoft · 2021-04-16T23:54:55Z

portal-ui/src/screens/Console/Tenants/AddTenant/Steps/IdentityProvider.tsx

  classes: any;
  idpSelection: string;
+  accesskeys: string[];
+  secretkeys: string[];


Suggested change

secretkeys: string[];

secretKeys: string[];

bexsoft · 2021-04-17T00:00:52Z

portal-ui/src/screens/Console/Tenants/AddTenant/Steps/IdentityProvider.tsx

 const IdentityProvider = ({
  classes,
  idpSelection,
+  accesskeys,


Suggested change

accesskeys,

accessKeys,

bexsoft · 2021-04-17T00:01:01Z

portal-ui/src/screens/Console/Tenants/AddTenant/Steps/IdentityProvider.tsx

  classes,
  idpSelection,
+  accesskeys,
+  secretkeys,


Suggested change

secretkeys,

secretKeys,

bexsoft · 2021-04-17T00:02:22Z

portal-ui/src/screens/Console/Tenants/AddTenant/Steps/IdentityProvider.tsx

+              key={`csv-secretkey-${index.toString()}`}
+            />
+          </div>
+          <button


Please try to use the component from material-ui, you can see examples in the create tenants files

bexsoft · 2021-04-17T00:02:35Z

portal-ui/src/screens/Console/Tenants/AddTenant/Steps/IdentityProvider.tsx

        MinIO supports both OpenID and Active Directory
-      </Grid>
-
+        <button


Same case with button

bexsoft

Also please remember to do make assets

Alevsk · 2021-04-20T01:34:54Z

restapi/admin_tenants.go

 		if err != nil {
 			return nil, prepareError(errorGeneric, nil, err)
 		}
 		minInst.Spec.Users = []*corev1.LocalObjectReference{{Name: consoleUserSecretName}}


Existing code here will create an additional user in MinIO, we need to discuss whats the desired behavior because if the user decided to add, lets say, 5 users via the create tenant wizard he will ned having 6 users on his system and I'm sure he will ask why

Alevsk · 2021-04-20T01:36:39Z

restapi/admin_tenants.go

+	if len(tenantReq.Idp.Keys) > 0 {
+		minInst.Spec.Users = users
+	}


What is this variable assignation happening here? I see below we are iterating over tenantReq.Idp.Keys and assigning more objects to minInst.Spec.Users

Alevsk · 2021-04-20T01:40:16Z

restapi/admin_tenants.go

+		if len(tenantReq.Idp.Keys) > 0 {
+			for i := 0; i < len(tenantReq.Idp.Keys); i++ {
+				userSecretName := fmt.Sprintf("%s-%d-user-secret", tenantName, i)
+				userSecretData := map[string][]byte{
+					"CONSOLE_ACCESS_KEY": []byte(*tenantReq.Idp.Keys[i].AccessKey),
+					"CONSOLE_SECRET_KEY": []byte(*tenantReq.Idp.Keys[i].SecretKey),
+				}
+				_, err := createOrReplaceSecrets(ctx, &k8sClient, ns, []tenantSecret{{Name: userSecretName, Content: userSecretData}}, tenantName)
+				if err != nil {
+					return nil, prepareError(errorGeneric, nil, err)
+				}
+				minInst.Spec.Users = append(minInst.Spec.Users, &corev1.LocalObjectReference{Name: userSecretName})
+			}
+
+		}


Is this repeated code? I think we already iterate over all tenantReq.Idp.Keys, created k8s and populated the users array in line 577

Alevsk · 2021-04-20T01:41:40Z

restapi/admin_tenants.go

+					"CONSOLE_SECRET_KEY": []byte(*tenantReq.Idp.Keys[i].SecretKey),
+				},
+			}
+			_, err := clientSet.CoreV1().Secrets(ns).Create(ctx, &instanceSecret, metav1.CreateOptions{})


You can use the createOrReplaceSecrets function here

dvaldivia · 2021-04-20T20:31:53Z

can you also do make assets and git add anything in portal-ui/build ?

make assets
git add portal-ui/build

dvaldivia · 2021-04-20T20:49:31Z

I tested this PR creating 3 users with short names like test1 but the operator fails with the following error

E0420 20:45:24.243118       1 main-controller.go:742] error syncing 'ns-1/adam': secret key must be minimum 8 or more characters long
E0420 20:46:24.302097       1 main-controller.go:742] error syncing 'ns-1/adam': secret key must be minimum 8 or more characters long
E0420 20:47:24.800068       1 main-controller.go:742] error syncing 'ns-1/adam': secret key must be minimum 8 or more characters long

Please introduce a restriction on the UI to ensure the user and password are at least 8 characters long

bexsoft

Please fix conflicts

test.html

kubectl.sha256

bexsoft · 2021-04-21T18:52:26Z

portal-ui/src/screens/Console/Tenants/AddTenant/Steps/IdentityProvider.tsx

+              error={validationErrors[`secretkey-${index.toString()}`] || ""}
+            />
+          </div>
+          <Button


Please try to place this button as an icon in the secret key, please look for examples of overlayIcon in InputBoxWrapper component

Signed-off-by: Adam Stafford <adam@minio.io>

concerns addressed

Alevsk · 2021-04-22T22:31:22Z

This is being rendered like this on my screen, also how can i remove an access/secret keypair once added? we should have a delete button, you can take a look at how we implemented the keypair certificates on the encryption section of the add tenant wizard

Alevsk · 2021-04-22T22:34:14Z

I think length policy only applies to secret key, access key can be less than 8 characters (via mc admin user add I can create users with access key alevsk), need to confirm this with @harshavardhana

adfost requested review from bexsoft, Alevsk and dvaldivia and removed request for bexsoft and Alevsk April 16, 2021 20:40

dvaldivia approved these changes Apr 16, 2021

View reviewed changes

dvaldivia requested changes Apr 16, 2021

View reviewed changes

dvaldivia requested a review from Alevsk April 16, 2021 21:59

dvaldivia assigned adfost Apr 16, 2021

bexsoft requested changes Apr 17, 2021

View reviewed changes

adfost force-pushed the api-bucket-policy branch 2 times, most recently from 17224a8 to ed8a562 Compare April 19, 2021 22:26

adfost changed the title ~~Api bucket policy~~ Adding create keys to tenant creation wizard Apr 19, 2021

adfost force-pushed the api-bucket-policy branch from e5d093a to b0939be Compare April 20, 2021 01:23

Alevsk previously requested changes Apr 20, 2021

View reviewed changes

bexsoft requested changes Apr 21, 2021

View reviewed changes

adfost force-pushed the api-bucket-policy branch from 57c9dcd to d004f7d Compare April 21, 2021 06:01

bexsoft requested changes Apr 21, 2021

View reviewed changes

adfost force-pushed the api-bucket-policy branch 3 times, most recently from c7196d1 to 300a737 Compare April 22, 2021 20:14

Adding key creation in tenant wizard.

5c42455

Signed-off-by: Adam Stafford <adam@minio.io>

adfost force-pushed the api-bucket-policy branch from 42b1971 to 5c42455 Compare April 22, 2021 20:41

dvaldivia approved these changes Apr 22, 2021

View reviewed changes

bexsoft approved these changes Apr 22, 2021

View reviewed changes

bexsoft requested a review from Alevsk April 22, 2021 20:46

dvaldivia merged commit ca742b7 into minio:master Apr 22, 2021

	const accesskeys = fields.identityProvider.accesskeys;
	const accessKeys = fields.identityProvider.accesskeys;

	const secretkeys = fields.identityProvider.secretkeys;
	const secretKeys = fields.identityProvider.secretkeys;

Adding create keys to tenant creation wizard #697

Adding create keys to tenant creation wizard #697

Uh oh!

Conversation

adfost commented Apr 16, 2021

Uh oh!

dvaldivia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dvaldivia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bexsoft left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dvaldivia commented Apr 20, 2021

Uh oh!

dvaldivia commented Apr 20, 2021

Uh oh!

bexsoft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Alevsk commented Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alevsk commented Apr 22, 2021

Uh oh!

Uh oh!

Alevsk commented Apr 22, 2021 •

edited

Loading