refactor: 💡sb dynamic credential type secret fields and how we serialize (#1907)

* refactor: 💡 secret fields and how we serialize

addons/api/addon/generated/models/storage-bucket.js

@@ -73,6 +73,13 @@ export default class GeneratedStorageBucketModel extends BaseModel {
   })
   region;
 
+  //this field is not used in the UI, but this is kept here to replicate the API
+  @attr('string', {
+    readOnly: true,
+    isNestedAttribute: true,
+  })
+  secrets_hmac;
+
   @attr('boolean', {
     isNestedAttribute: true,
     description: 'If set to `true`, credential rotation is not performed.',

addons/api/addon/models/storage-bucket.js

@@ -22,6 +22,9 @@ export const TYPES_CREDENTIALS = Object.freeze([
 ]);
 
 export default class StorageBucketModel extends GeneratedStorageBucketModel {
+  // attributes
+
+  #credentialType;
   /**
    * True if the storage bucket is a plugin.
    * @type {boolean}
@@ -31,18 +34,21 @@ export default class StorageBucketModel extends GeneratedStorageBucketModel {
   }
 
   /**
-   * If the storage bucket has role_arn, return dynamic
-   * otherwise return the default static type.
+   * Gets a value from credentialType
    * @type {string}
    */
   get credentialType() {
-    if (this.role_arn && !this.access_key_id) {
-      return TYPE_CREDENTIAL_DYNAMIC;
-    } else {
-      return TYPE_CREDENTIAL_STATIC;
-    }
+    if (!this.#credentialType) return 'static';
+    return this.#credentialType;
   }
 
+  /**
+   * Sets a value to credentialType
+   * @type {string}
+   */
+  set credentialType(type) {
+    this.#credentialType = type;
+  }
   /**
    * True if the storage bucket is an unknown type.
    * @type {boolean}

addons/api/addon/serializers/storage-bucket.js

@@ -49,17 +49,22 @@ export default class StorageBucketSerializer extends ApplicationSerializer {
         json.attributes[key] = null;
     }
 
+    //json.secrets is only present, if there are any updates to the secret fields
+    if (options.isNestedSecret && json.secrets) {
+      if (!fieldsByCredentialType[credentialType].includes(key)) {
+        delete json.secrets;
+      }
+    }
     return value;
   }
 
   normalize(typeClass, hash, ...rest) {
     const normalizedHash = structuredClone(hash);
+    const normalized = super.normalize(typeClass, normalizedHash, ...rest);
 
     // Remove secret fields so we don't track them after creating/updating
-    if (normalizedHash?.secrets?.access_key_id) {
-      delete normalizedHash.secrets.access_key_id;
-      delete normalizedHash.secrets.secret_access_key;
-    }
-    return super.normalize(typeClass, normalizedHash, ...rest);
+    normalized.data.attributes.access_key_id = null;
+    normalized.data.attributes.secret_access_key = null;
+    return normalized;
   }
 }

addons/api/tests/unit/models/storage-bucket-test.js

@@ -51,10 +51,12 @@ module('Unit | Model | storage bucket', function (hooks) {
     const modelA = store.createRecord('storage-bucket', {
       role_arn: 'test-role-arn',
       access_key_id: null,
+      credentialType: TYPE_CREDENTIAL_DYNAMIC,
     });
     const modelB = store.createRecord('storage-bucket', {
       role_arn: null,
       access_key_id: 'test-access-key-id',
+      credentialType: TYPE_CREDENTIAL_STATIC,
     });
     assert.strictEqual(modelA.credentialType, TYPE_CREDENTIAL_DYNAMIC);
     assert.strictEqual(modelB.credentialType, TYPE_CREDENTIAL_STATIC);

addons/api/tests/unit/serializers/storage-bucket-test.js

@@ -8,6 +8,8 @@ import { setupTest } from 'ember-qunit';
 import {
   TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3,
   TYPE_STORAGE_BUCKET_PLUGIN,
+  TYPE_CREDENTIAL_STATIC,
+  TYPE_CREDENTIAL_DYNAMIC,
 } from 'api/models/storage-bucket';
 
 module('Unit | Serializer | storage bucket', function (hooks) {
@@ -27,6 +29,7 @@ module('Unit | Serializer | storage bucket', function (hooks) {
     const store = this.owner.lookup('service:store');
     const record = store.createRecord('storage-bucket', {
       compositeType: TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3,
+      credentialType: TYPE_CREDENTIAL_STATIC,
       name: 'AWS',
       description: 'this has an aws plugin',
       bucket_name: 'bucketname',
@@ -36,10 +39,13 @@ module('Unit | Serializer | storage bucket', function (hooks) {
       access_key_id: 'foobars',
       secret_access_key: 'testing',
       disable_credential_rotation: true,
-      role_arn: null,
-      role_external_id: null,
-      role_session_name: null,
-      role_tags: null,
+      role_arn: 'test',
+      role_external_id: 'test',
+      role_session_name: 'test',
+      role_tags: [
+        { key: 'Project', value: 'Automation' },
+        { key: 'foo', value: 'bar' },
+      ],
     });
     const expectedResult = {
       name: 'AWS',
@@ -55,6 +61,7 @@ module('Unit | Serializer | storage bucket', function (hooks) {
         role_external_id: null,
         role_session_name: null,
         role_tags: null,
+        secrets_hmac: null,
       },
       secrets: {
         access_key_id: 'foobars',
@@ -64,11 +71,12 @@ module('Unit | Serializer | storage bucket', function (hooks) {
     assert.deepEqual(record.serialize(), expectedResult);
   });
 
-  test('it serializes a new dynamic aws plugin as expected', async function (assert) {
+  test('it serializes a new aws plugin with dynamic credentials as expected', async function (assert) {
     assert.expect(1);
     const store = this.owner.lookup('service:store');
     const record = store.createRecord('storage-bucket', {
       compositeType: TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3,
+      credentialType: TYPE_CREDENTIAL_DYNAMIC,
       name: 'AWS',
       description: 'this has an aws plugin',
       bucket_name: 'bucketname',
@@ -100,21 +108,22 @@ module('Unit | Serializer | storage bucket', function (hooks) {
         role_external_id: 'Example987',
         role_session_name: 'my-session',
         role_tags: { Project: 'Automation', foo: 'bar' },
+        secrets_hmac: null,
       },
     };
     assert.deepEqual(record.serialize(), expectedResult);
   });
-  test('it serializes when updating a storage bucket with static credential correctly', async function (assert) {
+
+  test('it serializes when updating a dynamic type credential storage bucket with static credentials correctly', async function (assert) {
     assert.expect(1);
     const store = this.owner.lookup('service:store');
     const serializer = store.serializerFor('storage-bucket');
     store.push({
       data: {
-        id: '1',
+        id: '100',
         type: 'storage-bucket',
         attributes: {
           type: TYPE_STORAGE_BUCKET_PLUGIN,
-          compositeType: TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3,
           name: 'AWS',
           description: 'this has an aws plugin',
           bucket_name: 'bucketname',
@@ -125,10 +134,18 @@ module('Unit | Serializer | storage bucket', function (hooks) {
           secret_access_key: 'testing',
           disable_credential_rotation: true,
           version: 1,
+          role_arn: 'role_arn_test',
+          role_external_id: 'role_external_id_test',
+          role_session_name: 'role_session_test',
+          role_tags: [
+            { key: 'Project', value: 'Automation' },
+            { key: 'foo', value: 'bar' },
+          ],
         },
       },
     });
-    const record = store.peekRecord('storage-bucket', '1');
+    const record = store.peekRecord('storage-bucket', '100');
+    record.credentialType = TYPE_CREDENTIAL_STATIC;
     const snapshot = record._createSnapshot();
     const serializedRecord = serializer.serialize(snapshot);
 
@@ -144,6 +161,7 @@ module('Unit | Serializer | storage bucket', function (hooks) {
         role_external_id: null,
         role_session_name: null,
         role_tags: null,
+        secrets_hmac: null,
       },
       secrets: {
         access_key_id: 'foobars',
@@ -154,7 +172,7 @@ module('Unit | Serializer | storage bucket', function (hooks) {
     assert.deepEqual(serializedRecord, expectedResult);
   });
 
-  test('it serializes when updating a storage bucket with dynamic credential correctly', async function (assert) {
+  test('it serializes when updating a static type credential storage bucket with dynamic credentials correctly', async function (assert) {
     assert.expect(1);
     const store = this.owner.lookup('service:store');
     const serializer = store.serializerFor('storage-bucket');
@@ -164,7 +182,6 @@ module('Unit | Serializer | storage bucket', function (hooks) {
         type: 'storage-bucket',
         attributes: {
           type: TYPE_STORAGE_BUCKET_PLUGIN,
-          compositeType: TYPE_STORAGE_BUCKET_PLUGIN_AWS_S3,
           name: 'AWS',
           description: 'this has an aws plugin',
           bucket_name: 'bucketname',
@@ -172,6 +189,8 @@ module('Unit | Serializer | storage bucket', function (hooks) {
           worker_filter: 'workerfilter',
           region: 'eu-west-1',
           disable_credential_rotation: true,
+          access_key_id: 'test',
+          secret_access_key: 'test',
           version: 1,
           role_arn: 'arn',
           role_external_id: 'Example987',
@@ -184,6 +203,7 @@ module('Unit | Serializer | storage bucket', function (hooks) {
       },
     });
     const record = store.peekRecord('storage-bucket', '1');
+    record.credentialType = TYPE_CREDENTIAL_DYNAMIC;
     const snapshot = record._createSnapshot();
     const serializedRecord = serializer.serialize(snapshot);
 
@@ -199,6 +219,7 @@ module('Unit | Serializer | storage bucket', function (hooks) {
         role_external_id: 'Example987',
         role_session_name: 'my-session',
         role_tags: { Project: 'Automation', foo: 'bar' },
+        secrets_hmac: null,
       },
       version: 1,
     };

ui/admin/app/controllers/scopes/scope/storage-buckets.js

@@ -10,7 +10,6 @@ export default class ScopesScopeStorageBucketsController extends Controller {
   // =services
 
   @service intl;
-  queryParams = ['credentialType'];
 
   // = attributes
   /**

ui/admin/app/routes/scopes/scope/storage-buckets.js

@@ -20,11 +20,6 @@ export default class ScopesScopeStorageBucketsRoute extends Route {
 
   // =attributes
 
-  queryParams = {
-    credentialType: {
-      refreshModel: true,
-    },
-  };
   // =methods
 
   /**
@@ -101,7 +96,7 @@ export default class ScopesScopeStorageBucketsRoute extends Route {
    * @param {string} credentialType
    */
   @action
-  changeType(credentialType) {
-    this.router.replaceWith({ queryParams: { credentialType } });
+  changeType(storageBucket, credentialType) {
+    storageBucket.credentialType = credentialType;
   }
 }

ui/admin/app/routes/scopes/scope/storage-buckets/new.js

@@ -25,10 +25,23 @@ export default class ScopesScopeStorageBucketsNewRoute extends Route {
    */
   model() {
     const scopeModel = this.store.peekRecord('scope', 'global');
-    let name, description, scope, bucket_name, bucket_prefix, region;
+    let name,
+      description,
+      scope,
+      bucket_name,
+      bucket_prefix,
+      region,
+      worker_filter;
     if (this.currentModel?.isNew) {
-      ({ name, description, scope, bucket_name, bucket_prefix, region } =
-        this.currentModel);
+      ({
+        name,
+        description,
+        scope,
+        bucket_name,
+        bucket_prefix,
+        region,
+        worker_filter,
+      } = this.currentModel);
       this.currentModel.rollbackAttributes();
     }
     const record = this.store.createRecord('storage-bucket', {
@@ -40,6 +53,7 @@ export default class ScopesScopeStorageBucketsNewRoute extends Route {
       bucket_name,
       bucket_prefix,
       region,
+      worker_filter,
     });
     record.scopeModel = scopeModel;
     return record;

ui/admin/app/templates/scopes/scope/storage-buckets/new.hbs

@@ -23,7 +23,7 @@
       @submit={{route-action 'save' @model}}
       @cancel={{route-action 'cancel' @model}}
       @scopes={{this.scopes}}
-      @changeType={{route-action 'changeType'}}
+      @changeType={{route-action 'changeType' @model}}
     />
   </page.body>
 

ui/admin/app/templates/scopes/scope/storage-buckets/storage-bucket/index.hbs

@@ -36,7 +36,7 @@
       @submit={{route-action 'save' @model}}
       @cancel={{route-action 'cancel' @model}}
       @scopes={{this.scopes}}
-      @changeType={{route-action 'changeType'}}
+      @changeType={{route-action 'changeType' @model}}
     />
   </page.body>
 </Rose::Layout::Page>