UI: Upgrade Ember data 5.3.2 (and upgrade minor versions of `ember-source` and `ember-cli`) #28798

hellobontempo · 2024-10-29T17:10:38Z

Original PR #28705 (opened a new PR due to yarn.lock conflicts on main)

Description

This PR upgrades to ember-data 5.3.2 and tackles resulting test failures. In summary, most of our failures are because Vault is not a RESTful API so we don't consistently receive ids for records after creation.

Some of the failures we saw looked like:

router.js:1129 Error while processing route: vault.cluster.secrets.backend.kmip.configure 'kmip/config' 
was saved to the server, but the response returned the new id 'kmip', which has already been used 
with another record.' Error: 'kmip/config' was saved to the server, but the response returned the new 
id 'kmip', which has already been used with another record.'

This error occurred when navigating to create routes where record schemas were built from openApi. I could not figure out where exactly this error was coming from because this error occurred when navigating to the create form, before a record was even saved. My suspicion is something is happening when open api generates the schema that results in creating a record in the store, but the ember data inspector is broken after upgrading so it's hard to tell what's happening in the store exactly.

To resolve the issue we create the record with a mutableId which is used to create the record, generate the URL and returned by createRecord (which is necessary when creating results in a 204 which ember data has problems with). See comment here.

In addition, we replaced instances of findAll with query because docs

findAll asks the adapter's findAll method to find the records for the given type, and returns a promise which will resolve with all records of this type present in the store, even if the adapter only returns a subset of them.

This means that findAll makes a network requests and adds any records to the existing store and then returns all of those. This is bad. The store could contain an outdated or "ghost" record (something we saw more of in this upgrade - unfortunately tracking these down is not straightforward). Either way, two sources of truth is unreliable and we should consistently only rely on the server as the source of truth - not the ember data store.

By using query we consistently refresh the store with what the API returns which is what we want.

TODO only if you're a HashiCorp employee

Backport Labels: If this PR is in the ENT repo and needs to be backported, backport
to N, N-1, and N-2, using the backport/ent/x.x.x+ent labels. If this PR is in the CE repo, you should only backport to N, using the backport/x.x.x label, not the enterprise labels.
- If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
Jira: If this change has an associated Jira, it's referenced either
in the PR description, commit message, or branch name.
RFC: If this change has an associated RFC, please link it in the description.
ENT PR: If this change has an associated ENT PR, please link it in the
description. Also, make sure the changelog is in this PR, not in your ENT PR.

… ember-source

github-actions · 2024-10-29T17:13:09Z

CI Results:
All Go tests succeeded! ✅

hellobontempo · 2024-10-29T17:13:11Z

ui/README.md

@@ -24,20 +24,19 @@

 This README outlines the details of collaborating on this Ember application.

-## Ember CLI Version Upgrade Matrix


"Ember version" wasn't very clear since we have 3 different ember dependencies: ember-source, ember-data and ember-cli so I added separate columns and only included Vault versions where those versions changed. i looked into doing this dynamically but couldn't find a good solution since since we want to include historical versions, not just the current Vault ember versions

https://github.com/hashicorp/vault/blob/release/1.17.x/ui/package.json
https://github.com/hashicorp/vault/blob/release/1.15.x/ui/package.json
https://github.com/hashicorp/vault/blob/release/1.13.x/ui/package.json
https://github.com/hashicorp/vault/blob/release/1.11.x/ui/package.json
https://github.com/hashicorp/vault/blob/release/1.10.x/ui/package.json
https://github.com/hashicorp/vault/blob/release/1.9.x/ui/package.json

github-actions · 2024-10-29T17:13:39Z

Build Results:
All builds succeeded! ✅

hellobontempo · 2024-10-29T17:13:48Z

ui/app/adapters/auth-method.js

@@ -23,26 +23,21 @@ export default ApplicationAdapter.extend({
    const isUnauthenticated = snapshotRecordArray?.adapterOptions?.unauthenticated;
    // sys/internal/ui/mounts returns the actual value of the system TTL
    // instead of '0' which just indicates the mount is using system defaults
-    const useMountsEndpoint = snapshotRecordArray?.adapterOptions?.useMountsEndpoint;


reverted changes made by https://github.com/hashicorp/vault/pull/26663/files#diff-50667c363b9be9067473f2acbfbf219e7e13c9c8c6d2232ce971e4205e7932cf now that we're using a separate method

hellobontempo · 2024-10-29T17:14:27Z

ui/app/adapters/kmip/config.js

  },
  urlForUpdateRecord() {
    return this._url(...arguments);
  },
+
+  createRecord(store, type, snapshot) {


resolves

index.js:696 Uncaught Error: Your kmip/config record was saved to the server, but the response does not have an id and no id has been set client side. Records must have ids. Please update the server response to provide an id in the response or generate the id on the client side either before saving the record or while normalizing the response.

hellobontempo · 2024-10-29T17:15:28Z

ui/app/routes/vault/cluster/settings/auth/configure/section.js

@@ -86,7 +86,7 @@ export default Route.extend(UnloadModelRoute, {
        // if you haven't saved a config, the API 404s, so create one here to edit and return it
        if (e.httpStatus === 404) {
          config = this.store.createRecord(modelType, {
-            id: backend.id,


I can't find where, but somewhere a record is being created by ember-data (that doesn't persist to the server) which caused this error

router.js:1129 Error while processing route: vault.cluster.settings.auth.configure.section The id oidcNEW has already been used with another 'auth-config/oidc' record. Error: The id oidcNEW has already been used with another 'auth-config/oidc' record.

hellobontempo · 2024-10-29T17:15:48Z

ui/app/services/path-help.js

-
-    // bust cache in schema service
-    const schemas = store.getSchemaDefinitionService?.();
-    if (schemas) {


these properties no longer exist when calling store.getSchemaDefinitionService()

hellobontempo · 2024-10-29T17:16:02Z

ui/lib/kmip/addon/routes/configure.js

@@ -17,7 +17,7 @@ export default Route.extend({
    return this.store.findRecord('kmip/config', this.secretMountPath.currentPath).catch((err) => {
      if (err.httpStatus === 404) {
        const model = this.store.createRecord('kmip/config');
-        model.set('id', this.secretMountPath.currentPath);


resolves this error. i cannot for the life of me figure out where this record is created and/or sent to the server. I tried skipping .findRecord but nope, that's not it. My suspicion is that it's something to do with how openApi is generating schemas but I couldn't pin it down

router.js:1129 Error while processing route: vault.cluster.secrets.backend.kmip.configure 'kmip/config' was saved to the server, but the response returned the new id 'kmip', which has already been used with another record.' Error: 'kmip/config' was saved to the server, but the response returned the new id 'kmip', which has already been used with another record.'

hellobontempo · 2024-10-29T17:16:40Z

ui/lib/pki/addon/components/pki-key-form.hbs

+{{! private_key is only available after initial save }}
+{{#if this.generatedKey.privateKey}}
+  <Page::PkiKeyDetails
+    @key={{this.generatedKey}}
+    @canDelete={{this.generatedKey.canDelete}}
+    @canEdit={{this.generatedKey.canEdit}}
+  />
+{{else}}


just added this part of the conditional, recommend viewing diff with ✅ hide whitespace

hellobontempo · 2024-10-29T17:17:05Z

ui/lib/pki/addon/routes/roles/create.js

@@ -36,4 +36,10 @@ export default class PkiRolesCreateRoute extends Route {
      { label: 'create' },
    ];
  }
+
+  willTransition() {


Resolves this test which caught duplicate roles (with the same id) in the store

yeah, a test caught something. good example of a helpful test.

hellobontempo · 2024-10-29T17:17:54Z

ui/tests/acceptance/pki/pki-engine-workflow-test.js

      await fillIn(GENERAL.inputByAttr('keyType'), 'rsa');
      await click(GENERAL.saveButton);
      keyId = find(GENERAL.infoRowValue('Key ID')).textContent?.trim();
-      assert.strictEqual(currentURL(), `/vault/secrets/${this.mountPath}/pki/keys/${keyId}/details`);
+      assert.strictEqual(


see code comment above here

After upgrading to 5.3.2 Ember data no longer "stores" data returned from a save response to the record. Sometimes performing a save operation returns generated data (like private_key info here)

Previously, any data returned after saving a record was stored in that model in the ember-data store until the page was hard refreshed (or the user navigated away from the route). It did not matter that we re-requested the record fro the server, any server data was just added to the record.

Now, this data is no longer cached so when we transition to details, data like generated private_key is lost because re-requesting the record from the server clears out the record and only data returned by the server is available.

It's important that we return generated data from saving a record, so the user can download or copy it. So we no longer transition and instead show the details on the same page.

hellobontempo · 2024-10-29T17:41:46Z

✅ enterprise tests

hellobontempo · 2024-10-29T18:49:18Z

ui/app/components/pgp-file.js

@@ -64,6 +64,9 @@ export default Component.extend({
  ),

  actions: {
+    handleToggle(e) {


I opted to add this action instead of glimmerize this file to keep scope small and because this file should eventually just be replaced with the Hds::Form::FileInput::Field, instead of continuing to maintain inflexible components

hellobontempo · 2024-10-29T18:50:16Z

ui/app/templates/components/pgp-file.hbs

@@ -23,7 +23,7 @@
        name={{concat "useText-" this.elementId}}
        class="toggle is-success is-small"
        checked={{this.key.enterAsText}}
-        onchange={{action (toggle "enterAsText" this.key)}}
+        onchange={{action "handleToggle"}}


This change and

vault/ui/app/components/pgp-file.js

Lines 67 to 69 in fdf0365

handleToggle(e) {

set(this.key, 'enterAsText', e.target.checked);

},

are the two additional file changes for this PR compared to the original. When digging into the failure, it seemed like the toggle helper no longer updated this.key. I didn't see the template helper used anywhere else in the code base

Integration | Component | pgp file: toggling back and forth: Chrome 130.0 renders the textarea on toggle Expected: Element [data-test-pgp-file-textarea] exists once Result: Element [data-test-pgp-file-textarea] does not exist

I did come across this toggle-action helper

vault/ui/lib/kubernetes/addon/components/page/credentials.hbs

Line 77 in 52c02ae

@onChange={{(toggle-action "clusterRoleBinding" this)}}

and confirmed it still updates this.clusterRoleBinding

Monkeychip · 2024-10-30T15:23:48Z

ui/tests/acceptance/access/methods-test.js

 import { v4 as uuidv4 } from 'uuid';
+import { GENERAL } from 'vault/tests/helpers/general-selectors';
+import { mountAuthCmd, runCmd } from 'vault/tests/helpers/commands';
+import { login } from 'vault/tests/helpers/auth/auth-helpers';


gosh do I sure love to see this happening. that authLogin is used in a lot of places.

Monkeychip

Awesome job on the write up and documentation 🥳🚢 I've done some smoke testing as well and all went smooth.

hellobontempo added 6 commits October 29, 2024 09:52

upgrade ember-data 5.3.2, uninstall legacy compat, upgrade ember-cli,…

7e6b86e

… ember-source

use query instead of findAll for auth methods, update tests

0249cd2

set mutableId for kmip

da6546a

show generated private key data before transitioning to details

160b668

update kv metadata test

7480217

remove deprecated methods from path help service

26be332

hellobontempo added the ui label Oct 29, 2024

hellobontempo added this to the 1.19.0-rc milestone Oct 29, 2024

hellobontempo requested a review from a team as a code owner October 29, 2024 17:10

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Oct 29, 2024

add changelog, update readme version matrix

18b527f

hellobontempo commented Oct 29, 2024

View reviewed changes

hellobontempo mentioned this pull request Oct 29, 2024

UI: Upgrade Ember Data to 5.3.2 #28705

Closed

6 tasks

remove toggle template helper

fdf0365

hellobontempo changed the title ~~UI: Upgrade Ember data 5.3.2, upgrade minor versions of ember-source and ember-cli~~ UI: Upgrade Ember data 5.3.2 (and upgrade minor versions of ember-source and ember-cli) Oct 29, 2024

hellobontempo commented Oct 29, 2024

View reviewed changes

Monkeychip reviewed Oct 30, 2024

View reviewed changes

Monkeychip approved these changes Oct 30, 2024

View reviewed changes

hellobontempo merged commit 17d29f9 into main Oct 30, 2024
32 checks passed

hellobontempo deleted the ui/upgrade-ember-data-5.3.2 branch October 30, 2024 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI: Upgrade Ember data 5.3.2 (and upgrade minor versions of `ember-source` and `ember-cli`) #28798

UI: Upgrade Ember data 5.3.2 (and upgrade minor versions of `ember-source` and `ember-cli`) #28798

hellobontempo commented Oct 29, 2024 •

edited

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading

hellobontempo Oct 29, 2024

github-actions bot commented Oct 29, 2024 •

edited

Loading

hellobontempo Oct 29, 2024

hellobontempo Oct 29, 2024

hellobontempo Oct 29, 2024

hellobontempo Oct 29, 2024

hellobontempo Oct 29, 2024

hellobontempo Oct 29, 2024

hellobontempo Oct 29, 2024

Monkeychip Oct 30, 2024

hellobontempo Oct 29, 2024 •

edited

Loading

hellobontempo commented Oct 29, 2024

hellobontempo Oct 29, 2024

hellobontempo Oct 29, 2024 •

edited

Loading

hellobontempo Oct 29, 2024 •

edited

Loading

Monkeychip Oct 30, 2024

Monkeychip left a comment

		@@ -24,20 +24,19 @@

		This README outlines the details of collaborating on this Ember application.

		## Ember CLI Version Upgrade Matrix

	handleToggle(e) {
	set(this.key, 'enterAsText', e.target.checked);
	},

UI: Upgrade Ember data 5.3.2 (and upgrade minor versions of ember-source and ember-cli) #28798

UI: Upgrade Ember data 5.3.2 (and upgrade minor versions of ember-source and ember-cli) #28798

Conversation

hellobontempo commented Oct 29, 2024 • edited Loading

Description

TODO only if you're a HashiCorp employee

github-actions bot commented Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Oct 29, 2024 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hellobontempo Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

hellobontempo commented Oct 29, 2024

Choose a reason for hiding this comment

hellobontempo Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

hellobontempo Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeychip left a comment

Choose a reason for hiding this comment

UI: Upgrade Ember data 5.3.2 (and upgrade minor versions of `ember-source` and `ember-cli`) #28798

UI: Upgrade Ember data 5.3.2 (and upgrade minor versions of `ember-source` and `ember-cli`) #28798

hellobontempo commented Oct 29, 2024 •

edited

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading

hellobontempo Oct 29, 2024 •

edited

Loading

hellobontempo Oct 29, 2024 •

edited

Loading

hellobontempo Oct 29, 2024 •

edited

Loading