OpenAPI CRUD views #6702

madalynrose · 2019-05-08T21:24:27Z

This adds functionality to our path help service to grab relevant paths from OpenAPI to be displayed as tabs for auth methods.

…th-help if none exist

…t exist

…ucts the model once

…h new structure of pathHelp functions

…ration

…i-crud

meirish

This is looking good! I haven't pulled it down yet to try it out, but wanted to get these published - I know you're still planning on a cleanup pass - I think some of the comments I left might help with that.

Re: documentation - I think the comments are good, but could we also include a high level "this is how you use this" maybe at the top of the service?

What do you think about dropping in some https://api.emberjs.com/ember/3.10/functions/@ember%2Fdebug/debug calls in the service function s "Generating ____ model/adapter from ____ Open API endpoint" or something like that? Just thinking it'd be nice to have along with the route transition logs that we do as a check to make sure things are running properly.

ui/app/router.js

ui/app/routes/vault/cluster/access/method/item.js

meirish · 2019-05-13T16:06:16Z

ui/app/routes/vault/cluster/access/method/item.js

+    this._super(...arguments);
+    const { item_type: itemType } = this.paramsFor(this.routeName);
+    const { path } = this.paramsFor('vault.cluster.access.method');
+    const { apiPath } = this.modelFor('vault.cluster.access.method');


We could wrap these into a helper method to use in both places.

ui/app/routes/vault/cluster/access/method/item/list.js

ui/app/services/path-help.js

meirish · 2019-05-13T21:24:06Z

ui/app/services/path-help.js

+      //we always want to keep list endpoints for menus
+      //but only use scoped post/delete endpoints
+      if (itemType) {
+        paths = paths.filter(path => path.includes(itemType));


Didn't we come across a backend that had sibling paths instead of nested ones? Will we just handle that when we come across it?

meirish · 2019-05-13T21:27:06Z

ui/app/services/path-help.js

+          }
+        })
+        .filter(path => path != undefined);
+


I think you could do all of this in one pass with a reduce, but that's more of a cleanup thing than a must have.

meirish · 2019-05-13T21:30:31Z

ui/app/templates/components/form-field-groups.hbs

-            {{form-field data-test-field attr=attr model=model onChange=onChange}}
+          {{#unless (and (not-eq mode "create") (eq attr.name "name"))}}
+            {{form-field data-test-field attr=attr model=model onChange=onChange
+            }}


Did this just insert a line break?

ui/app/templates/components/section-tabs.hbs

meirish · 2019-05-13T21:35:03Z

ui/app/templates/vault/cluster/access/method/item/list.hbs

+    </ToolbarLink>
+  </ToolbarActions>
+</Toolbar>
+{{#if model.meta.total}}


There's a ListView and ListItem component that might be helpful here (not sure - it's been a while since I looked at them).

ui/app/router.js

ui/app/templates/vault/cluster/access/method/item/list.hbs

meirish · 2019-05-17T16:05:42Z

One other thing I didn't fully track down - if you go directly to the the method via the URL since there's no way to navigate there yet (we should probably link up ldap in the auth method list) - then you end up on the configuration page - can we change that so that we go to the first tab instead?

…ied in the Model

ui/app/services/path-help.js

meirish · 2019-06-20T20:31:30Z

I don't think we want the delete button in the toolbar and at the bottom of the form - I think just in the toolbar - but @joshuaogle would know for sure!

meirish · 2019-06-20T20:38:28Z

Functionally this looks great - there's a couple of things I'd like to see changed (and I can help track them down too) -

we should link the ldap auth list to the new routes
navigating there should take you to the groups tab since it's the first tab (that's what I was referring to in OpenAPI CRUD views #6702 (comment))

Going to look through the code now.

meirish · 2019-06-20T20:42:06Z

ui/app/components/section-tabs.js

  model: null,
  tabType: 'authSettings',
 });

 SectionTabs.reopenClass({
-  positionalParams: ['model', 'tabType'],
+  positionalParams: ['model', 'tabType', 'paths'],


Just a note here that positionParams are going away in Glimmer components, so we'll eventually want to move away from this.

meirish · 2019-06-20T20:43:40Z

ui/app/models/auth-config/ldap.js

-    helpText:
-      'Use the Active Directory tokenGroups constructed attribute to find the group memberships. This returns all security groups for the user, including nested groups. In an Active Directory environment with a large number of groups this method offers increased performance. Selecting this will cause Group DN, Attribute, and Filter to be ignored.',
-  }),
-


meirish · 2019-06-20T20:48:55Z

ui/app/routes/vault/cluster/access/method.js

  model(params) {
+    // eslint-disable-line rule


I think this can be removed.

meirish · 2019-06-20T20:48:58Z

ui/app/routes/vault/cluster/access/method.js

@@ -12,7 +15,10 @@ export default Route.extend({
        set(error, 'httpStatus', 404);
        throw error;
      }
-      return model;
+      return this.pathHelp.getPaths(model.apiPath, path).then(paths => {
+        model.set('paths', paths);


(for later) we should document what attributes OpenAPI models have on them - paths, newFields, etc.

meirish · 2019-06-20T20:52:00Z

ui/app/routes/vault/cluster/access/method/item/list.js

+    controller.set('method', path);
+    const { apiPath } = this.modelFor('vault.cluster.access.method');
+    this.pathHelp.getPaths(apiPath, path, itemType).then(paths => {
+      controller.set('paths', Array.from(paths.list, pathInfo => pathInfo.path));


what's the type of paths.list here?

meirish · 2019-06-20T20:53:29Z

ui/app/routes/vault/cluster/access/method/section.js

@@ -1,3 +1,4 @@
+/* eslint-disable prettier/prettier */


Should this be here?

meirish · 2019-06-20T20:54:50Z

ui/app/routes/vault/cluster/secrets/backend/secret-edit.js

@@ -63,8 +62,7 @@ export default Route.extend(UnloadModelRoute, {
    if (['secret', 'secret-v2'].includes(modelType)) {
      return resolve();
    }
-    let owner = getOwner(this);
-    return this.pathHelp.getNewModel(modelType, owner, backend);
+    return this.pathHelp.getNewModel(modelType, backend);


Ah cool - I'll have to update these in the KMIP PR after this is merged too

meirish · 2019-06-20T20:56:25Z

ui/app/services/path-help.js

  //Makes a call to grab the OpenAPI document.
  //Returns relevant information from OpenAPI
  //as determined by the expandOpenApiProps util
  getProps(helpUrl, backend) {
+    debug(`Fetching schema properties for ${backend} from ${helpUrl}`);


meirish · 2019-06-20T20:59:21Z

ui/app/services/path-help.js

@@ -24,37 +30,252 @@ export default Service.extend({
    });
  },

+  getNewModel(modelType, backend, apiPath, itemType) {


what is itemType here? I think a comment with example usage of the method would help too

itemType is the type of nested item, i.e. a LDAP group or userpass user. here's an example of where the itemType is set in the go code: https://github.com/hashicorp/vault/pull/7513/files#diff-cb2f72de1a3a7afc5227ef80f58f99f9R24

(also lol hi delayed answer here 🙈 -- just marking up this PR with notes for the rest of the team)

meirish · 2019-06-20T21:03:32Z

ui/app/templates/components/form-field.hbs

@@ -225,4 +179,4 @@
    value=(if (get model valuePath) (stringify (get model valuePath)) emptyData)
    valueUpdated=(action "codemirrorUpdated" attr.name false)
  }}
-{{/if}}
+{{/if}}


Are there any changes in this file other than whitespace? I think the formatting before was easier to read fwiw - if it's just whitespace changes, could we back these out?

meirish · 2019-06-20T21:05:07Z

ui/app/templates/components/generated-item.hbs

+          {{/link-to}}
+        {{/if}}
+      </div>
+      <div class="control">


This is the div and contents we'd remove if we don't need the Delete button on the bottom.

meirish · 2019-06-20T21:06:36Z

ui/app/templates/components/masked-input.hbs

-	</button>
-</div>
+  </button>
+</div>


I see the old one was broken with some indentation, but I think the whitespace was better before even with the bad indentation.

meirish · 2019-06-20T21:08:51Z

ui/app/templates/vault/cluster/access/method/item/list.hbs

+  </p.top>
+  <p.levelLeft>
+    <h1 class="title is-3">
+      {{capitalize method}}


Do we want to add an Icon above this? - should this be method or the path for the auth method?

So I think this should be id of the method

meirish

I think we should merge this and iterate on it after merge, but this should be good for the beta 🙌

andaley · 2020-07-21T21:23:26Z

ui/app/models/auth-config/ldap.js

 export default AuthConfig.extend({
  useOpenAPI: true,
-  binddn: attr('string', {


notice here that when we set useOpenAPI to true, we can get rid of the duplicate attrs. OpenAPI gives us the attrs and sets them on the model here: https://github.com/hashicorp/vault/pull/6702/files#diff-cd3b672bded0741ca8ec360a4deb124fR37

andaley · 2020-07-21T21:26:04Z

ui/app/routes/vault/cluster/access/method/item.js

+    let methodModel = this.modelFor('vault.cluster.access.method');
+    let { apiPath, type } = methodModel;
+    let modelType = `generated-${singularize(itemType)}-${type}`;
+    return this.pathHelp.getNewModel(modelType, method, apiPath, itemType);


this is an example of how we call OpenAPI and generate a model on the auth method items pages.

Madalyn Parker and others added 22 commits April 16, 2019 11:55

add list endpoint tab links to auth section page

f032067

add generic auth method list route and controller, create model in pa…

82d072e

…th-help if none exist

successfully make generic list call and render template if items don'…

ae8bb90

…t exist

add tabs menu to list page

34e2c1d

add show route and show properties for item

4f68001

restructure generic routes to include a parent item route that constr…

5e4a4af

…ucts the model once

add create page (WIP) to generic pages

8603ab3

add beginning of edit view

83e905d

refactor adapter to pull urls from path info, adapt pages to work wit…

4bbf98a

…h new structure of pathHelp functions

box off ldap, make sure path links are generated properly for configu…

97b0112

…ration

fix routing for models with factories

36b7ff4

add comments, try to debug pki role weirdness

190640c

Merge branch 'master' of github.com:hashicorp/vault into openapi-crud

73f7865

commentssssss

b684338

add delete functionality for generated items

8172468

Merge branch 'master' into openapi-crud

61589f6

adjust linting errors

79ecf47

Merge branch 'openapi-crud' of github.com:hashicorp/vault into openap…

676ee89

…i-crud

Merge branch 'master' into openapi-crud

0682a89

merge master

51c1489

update pages to use new toolbar components'

9cc9c1e

Merge branch 'openapi-crud' of github.com:hashicorp/vault into openap…

5e50388

…i-crud

meirish reviewed May 14, 2019

View reviewed changes

meirish reviewed May 15, 2019

View reviewed changes

ui/app/router.js Outdated Show resolved Hide resolved

meirish reviewed May 17, 2019

View reviewed changes

ui/app/templates/vault/cluster/access/method/item/list.hbs Outdated Show resolved Hide resolved

update Model fieldGroups to be build from openAPIif they aren't suppl…

ce6ecdf

…ied in the Model

meirish reviewed Jun 3, 2019

View reviewed changes

ui/app/services/path-help.js Outdated Show resolved Hide resolved

Madalyn Parker added 2 commits June 10, 2019 15:55

use .compact instead of .filter, return paths promise

9ee259f

small fixes from PR comments

325559b

Madalyn Parker added 3 commits June 20, 2019 12:26

remove old definition of deprecated

c3f1aaa

update fieldGroups for github to put things in correct order

5fcb5bd

add debug calls back in (they disappeared somewhere)

b40623c

meirish reviewed Jun 20, 2019

View reviewed changes

ui/app/routes/vault/cluster/access/method.js

model(params) {

// eslint-disable-line rule

Copy link

Contributor

meirish Jun 20, 2019

Choose a reason for hiding this comment

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

I think this can be removed.

meirish reviewed Jun 20, 2019

View reviewed changes

ui/app/routes/vault/cluster/access/method/section.js

@@ -1,3 +1,4 @@

/* eslint-disable prettier/prettier */

Copy link

Contributor

meirish Jun 20, 2019

Choose a reason for hiding this comment

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

Should this be here?

meirish reviewed Jun 20, 2019

View reviewed changes

meirish added this to the 1.2 milestone Jun 20, 2019

link ldap list item to the new access.method route

56f0704

meirish previously approved these changes Jun 21, 2019

View reviewed changes

remove duplicate delete button on item edit page

2d7795f

madalynrose dismissed meirish’s stale review via 2d7795f June 21, 2019 15:01

meirish approved these changes Jun 21, 2019

View reviewed changes

madalynrose merged commit 3b84ef0 into master Jun 21, 2019

madalynrose deleted the openapi-crud branch June 21, 2019 15:18

madalynrose mentioned this pull request Jun 26, 2019

UI: Clean up Dynamic UI for CRUD #6994

Merged

Monkeychip mentioned this pull request Mar 10, 2020

Bug Fix: remove duplicate form inputs on GitHub auth config page #8519

Merged

andaley reviewed Jul 21, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenAPI CRUD views #6702

OpenAPI CRUD views #6702

madalynrose commented May 8, 2019 •

edited

Loading

meirish left a comment

meirish May 13, 2019

meirish May 13, 2019

meirish May 13, 2019

meirish May 13, 2019 •

edited

Loading

meirish May 13, 2019

meirish commented May 17, 2019

meirish commented Jun 20, 2019

meirish commented Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

andaley Jul 21, 2020 •

edited

Loading

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish Jun 20, 2019

meirish left a comment

andaley Jul 21, 2020

andaley Jul 21, 2020

OpenAPI CRUD views #6702

OpenAPI CRUD views #6702

Conversation

madalynrose commented May 8, 2019 • edited Loading

meirish 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

meirish May 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meirish commented May 17, 2019

meirish commented Jun 20, 2019

meirish commented Jun 20, 2019

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

andaley Jul 21, 2020 • edited Loading

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

meirish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madalynrose commented May 8, 2019 •

edited

Loading

meirish May 13, 2019 •

edited

Loading

andaley Jul 21, 2020 •

edited

Loading