Skip to content

Commit

Permalink
UI: Replace getNewModel with hydrateModel when model exists (#27978)
Browse files Browse the repository at this point in the history
* Replace getNewModel with hydrateModel when model exists

* Update getNewModel to only handle nonexistant model types

* Update test

* clarify test

* Fix auth-config models which need hydration not generation

* rename file to match service name

* cleanup + tests

* Add comment about helpUrl method
  • Loading branch information
hashishaw authored and Monkeychip committed Aug 9, 2024
1 parent 7899f99 commit 328fc99
Show file tree
Hide file tree
Showing 14 changed files with 181 additions and 119 deletions.
2 changes: 1 addition & 1 deletion ui/app/routes/vault/cluster/secrets/backend/credentials.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default Route.extend({
}
// hydrate model if backend type is ssh
if (backendType === 'ssh') {
this.pathHelp.getNewModel('ssh-otp-credential', backendPath);
this.pathHelp.hydrateModel('ssh-otp-credential', backendPath);
}
},

Expand Down
2 changes: 1 addition & 1 deletion ui/app/routes/vault/cluster/secrets/backend/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export default Route.extend({
return this.router.transitionTo('vault.cluster.secrets.backend.kv.list', backend);
}
const modelType = this.getModelType(backend, tab);
return this.pathHelp.getNewModel(modelType, backend).then(() => {
return this.pathHelp.hydrateModel(modelType, backend).then(() => {
this.store.unloadAll('capabilities');
});
},
Expand Down
2 changes: 1 addition & 1 deletion ui/app/routes/vault/cluster/secrets/backend/secret-edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export default Route.extend({
if (modelType === 'secret') {
return resolve();
}
return this.pathHelp.getNewModel(modelType, backend);
return this.pathHelp.hydrateModel(modelType, backend);
},

modelType(backend, secret, options = {}) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { set } from '@ember/object';
import Route from '@ember/routing/route';
import RSVP from 'rsvp';
import UnloadModelRoute from 'vault/mixins/unload-model-route';
import { getHelpUrlForModel } from 'vault/utils/openapi-helpers';

export default Route.extend(UnloadModelRoute, {
modelPath: 'model.model',
Expand Down Expand Up @@ -41,6 +42,11 @@ export default Route.extend(UnloadModelRoute, {
const { method } = this.paramsFor('vault.cluster.settings.auth.configure');
const backend = this.modelFor('vault.cluster.settings.auth.configure');
const modelType = this.modelType(backend.type, section_name);
// If this method returns a string it means we expect to hydrate it with OpenAPI
if (getHelpUrlForModel(modelType)) {
return this.pathHelp.hydrateModel(modelType, method);
}
// if no helpUrl is defined, this is a fully generated model
return this.pathHelp.getNewModel(modelType, method, backend.apiPath);
},

Expand Down
62 changes: 39 additions & 23 deletions ui/app/services/path-help.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import Model from '@ember-data/model';
import Service from '@ember/service';
import { encodePath } from 'vault/utils/path-encoding-helpers';
import { getOwner } from '@ember/application';
import { getOwner } from '@ember/owner';
import { expandOpenApiProps, combineAttributes } from 'vault/utils/openapi-to-attrs';
import fieldToAttrs from 'vault/utils/field-to-attrs';
import { resolve, reject } from 'rsvp';
Expand Down Expand Up @@ -42,10 +42,35 @@ export default Service.extend({
},

/**
* getNewModel instantiates models which use OpenAPI fully or partially
* hydrateModel instantiates models which use OpenAPI partially
* @param {string} modelType path for model, eg pki/role
* @param {string} backend path, which will be used for the generated helpUrl
* @returns void - as side effect, registers model via registerNewModelWithProps
*/
hydrateModel(modelType, backend) {
const owner = getOwner(this);
const modelName = `model:${modelType}`;

const modelFactory = owner.factoryFor(modelName);
const helpUrl = getHelpUrlForModel(modelType, backend);

if (!modelFactory) {
throw new Error(`modelFactory for ${modelType} not found -- use getNewModel instead.`);
}

debug(`Model factory found for ${modelType}`);
const newModel = modelFactory.class;
if (newModel.merged || !helpUrl) {
return resolve();
}
return this.registerNewModelWithProps(helpUrl, backend, newModel, modelName);
},

/**
* getNewModel instantiates models which use OpenAPI to generate the model fully
* @param {string} modelType
* @param {string} backend
* @param {string} apiPath (optional) if passed, this method will call getPaths and build submodels for item types
* @param {string} apiPath this method will call getPaths and build submodels for item types
* @param {*} itemType (optional) used in getPaths for additional models
* @returns void - as side effect, registers model via registerNewModelWithProps
*/
Expand All @@ -54,28 +79,19 @@ export default Service.extend({
const modelName = `model:${modelType}`;

const modelFactory = owner.factoryFor(modelName);
let helpUrl = getHelpUrlForModel(modelType, backend);

let newModel;
// if we have a factory, we need to take the existing model into account
if (modelFactory) {
debug(`Model factory found for ${modelType}`);
newModel = modelFactory.class;
if (newModel.merged || !helpUrl) {
return resolve();
}

return this.registerNewModelWithProps(helpUrl, backend, newModel, modelName);
} else {
debug(`Creating new Model for ${modelType}`);
newModel = Model.extend({});
}
// if the modelFactory already exists, it means either this model was already
// generated or the model exists in the code already. In either case resolve

// we don't have an apiPath for dynamic secrets
// and we don't need paths for them yet
if (!apiPath) {
return this.registerNewModelWithProps(helpUrl, backend, newModel, modelName);
if (!modelFactory.class.merged) {
// no merged flag means this model was not previously generated
debug(`Model exists for ${modelType} -- use hydrateModel instead`);
}
return resolve();
}
debug(`Creating new Model for ${modelType}`);
let newModel = Model.extend({});

// use paths to dynamically create our openapi help url
// if we have a brand new model
Expand All @@ -98,7 +114,7 @@ export default Service.extend({
return reject();
}

helpUrl = `/v1/${apiPath}${path.slice(1)}?help=true`;
const helpUrl = `/v1/${apiPath}${path.slice(1)}?help=true`;
pathInfo.paths = paths;
newModel = newModel.extend({ paths: pathInfo });
return this.registerNewModelWithProps(helpUrl, backend, newModel, modelName);
Expand Down Expand Up @@ -307,7 +323,7 @@ export default Service.extend({
},
}),
});
newModel.reopenClass({ merged: true });
newModel.merged = true;
owner.unregister(modelName);
owner.register(modelName, newModel);
});
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/kmip/addon/routes/configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export default Route.extend(UnloadModel, {
secretMountPath: service(),
pathHelp: service(),
beforeModel() {
return this.pathHelp.getNewModel('kmip/config', this.secretMountPath.currentPath);
return this.pathHelp.hydrateModel('kmip/config', this.secretMountPath.currentPath);
},
model() {
return this.store.findRecord('kmip/config', this.secretMountPath.currentPath).catch((err) => {
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/kmip/addon/routes/configure.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default Route.extend({
secretMountPath: service(),
pathHelp: service(),
beforeModel() {
return this.pathHelp.getNewModel('kmip/config', this.secretMountPath.currentPath);
return this.pathHelp.hydrateModel('kmip/config', this.secretMountPath.currentPath);
},
model() {
return this.store.findRecord('kmip/config', this.secretMountPath.currentPath).catch((err) => {
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/kmip/addon/routes/role.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export default class KmipRoleRoute extends Route {
@service pathHelp;

beforeModel() {
return this.pathHelp.getNewModel('kmip/role', this.secretMountPath.currentPath);
return this.pathHelp.hydrateModel('kmip/role', this.secretMountPath.currentPath);
}

model(params) {
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/kmip/addon/routes/role/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default Route.extend({
secretMountPath: service(),
pathHelp: service(),
beforeModel() {
return this.pathHelp.getNewModel('kmip/role', this.secretMountPath.currentPath);
return this.pathHelp.hydrateModel('kmip/role', this.secretMountPath.currentPath);
},
model() {
const params = this.paramsFor(this.routeName);
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/kmip/addon/routes/scope/roles.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default Route.extend(ListRoute, {
return this.paramsFor('scope').scope_name;
},
beforeModel() {
return this.pathHelp.getNewModel('kmip/role', this.secretMountPath.currentPath);
return this.pathHelp.hydrateModel('kmip/role', this.secretMountPath.currentPath);
},
model(params) {
return this.store
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/kmip/addon/routes/scope/roles/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default Route.extend({
},
beforeModel() {
this.store.unloadAll('kmip/role');
return this.pathHelp.getNewModel('kmip/role', this.secretMountPath.currentPath);
return this.pathHelp.hydrateModel('kmip/role', this.secretMountPath.currentPath);
},
model() {
const model = this.store.createRecord('kmip/role', {
Expand Down
18 changes: 9 additions & 9 deletions ui/lib/pki/addon/routes/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ export default class PkiRoute extends Route {
// the openAPI attributes to the model prototype
const mountPath = this.secretMountPath.currentPath;
return hash({
acme: this.pathHelp.getNewModel('pki/config/acme', mountPath),
certGenerate: this.pathHelp.getNewModel('pki/certificate/generate', mountPath),
certSign: this.pathHelp.getNewModel('pki/certificate/sign', mountPath),
cluster: this.pathHelp.getNewModel('pki/config/cluster', mountPath),
key: this.pathHelp.getNewModel('pki/key', mountPath),
role: this.pathHelp.getNewModel('pki/role', mountPath),
signCsr: this.pathHelp.getNewModel('pki/sign-intermediate', mountPath),
tidy: this.pathHelp.getNewModel('pki/tidy', mountPath),
urls: this.pathHelp.getNewModel('pki/config/urls', mountPath),
acme: this.pathHelp.hydrateModel('pki/config/acme', mountPath),
certGenerate: this.pathHelp.hydrateModel('pki/certificate/generate', mountPath),
certSign: this.pathHelp.hydrateModel('pki/certificate/sign', mountPath),
cluster: this.pathHelp.hydrateModel('pki/config/cluster', mountPath),
key: this.pathHelp.hydrateModel('pki/key', mountPath),
role: this.pathHelp.hydrateModel('pki/role', mountPath),
signCsr: this.pathHelp.hydrateModel('pki/sign-intermediate', mountPath),
tidy: this.pathHelp.hydrateModel('pki/tidy', mountPath),
urls: this.pathHelp.hydrateModel('pki/config/urls', mountPath),
});
}
}
118 changes: 118 additions & 0 deletions ui/tests/unit/services/path-help-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/**
* Copyright (c) HashiCorp, Inc.
* SPDX-License-Identifier: BUSL-1.1
*/

import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { setupMirage } from 'ember-cli-mirage/test-support';
import Sinon from 'sinon';
import { reject } from 'rsvp';

const openapiStub = {
openapi: {
components: {
schemas: {
UsersRequest: {
type: 'object',
properties: {
password: {
description: 'Password for the user',
type: 'string',
'x-vault-displayAttrs': { sensitive: true },
},
},
},
},
},
paths: {
'/users/{username}': {
post: {
requestBody: {
content: {
'application/json': {
schema: { $ref: '#/components/schemas/UsersRequest' },
},
},
},
},
parameters: [
{
description: 'Username for this user.',
in: 'path',
name: 'username',
required: true,
schema: { type: 'string' },
},
],
'x-vault-displayAttrs': { itemType: 'User', action: 'Create' },
},
},
},
};

module('Unit | Service | path-help', function (hooks) {
setupTest(hooks);
setupMirage(hooks);

hooks.beforeEach(function () {
this.pathHelp = this.owner.lookup('service:path-help');
this.store = this.owner.lookup('service:store');
});

module('getNewModel', function (hooks) {
hooks.beforeEach(function () {
this.server.get('/auth/userpass/', () => openapiStub);
this.server.get('/auth/userpass/users/example', () => openapiStub);
});
test('it generates a model with mutableId', async function (assert) {
assert.expect(2);
this.server.post('/auth/userpass/users/test', () => {
assert.true(true, 'POST request made to correct endpoint');
return;
});

const modelType = 'generated-user-userpass';
await this.pathHelp.getNewModel(modelType, 'userpass', 'auth/userpass/', 'user');
const model = this.store.createRecord(modelType);
model.set('mutableId', 'test');
await model.save();
assert.strictEqual(model.get('id'), 'test', 'model id is set to mutableId value on save success');
});

test('it only generates the model once', async function (assert) {
assert.expect(2);
Sinon.spy(this.pathHelp, 'getPaths');

const modelType = 'generated-user-userpass';
await this.pathHelp.getNewModel(modelType, 'userpass', 'auth/userpass/', 'user');
assert.true(this.pathHelp.getPaths.calledOnce, 'getPaths is called for new generated model');

await this.pathHelp.getNewModel(modelType, 'userpass2', 'auth/userpass/', 'user');
assert.true(this.pathHelp.getPaths.calledOnce, 'not called again even with different backend path');
});

test('it resolves without error if model already exists', async function (assert) {
Sinon.stub(this.pathHelp, 'getPaths').callsFake(() => {
assert.notOk(true, 'this method should not be called');
return reject();
});
const modelType = 'kv/data';
await this.pathHelp.getNewModel(modelType, 'my-kv').then(() => {
assert.true(true, 'getNewModel resolves');
});
});
});

module('hydrateModel', function () {
test('it should hydrate an existing model', async function (assert) {
this.server.get(`/pki2/roles/example`, () => openapiStub);

const modelType = 'pki/role';
await this.pathHelp.hydrateModel(modelType, 'pki2');
const model = this.store.createRecord(modelType);
model.set('username', 'foobar');
assert.strictEqual(model.username, 'foobar', 'sets value of key that only exists in openAPI response');
});
});
});
Loading

0 comments on commit 328fc99

Please sign in to comment.