fix: after Pierre's review

CleverCloud · Jan 30, 2025 · f0ea215 · f0ea215
1 parent 54013b5
commit f0ea215
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 67 deletions.
diff --git a/docs/ng.md b/docs/ng.md
@@ -106,15 +106,19 @@ clever ng get resourceIdOrName
 You can also search for network groups, members or peers:
 
 ```
-clever ng search query
+clever ng search text_to_search -F json
 ```
 
+> [!NOTE]
+> The search command is case-insensitive and will return all resources containing the search string
+> The get command look for an exact match and will return an error if multiple resources are found
+
 ## Get the Wireguard configuration of a Peer
 
 To get the Wireguard configuration of a peer (a `json` formatted output is available):
 
 ```
-clever ng get-config peerIdOrLabel
+clever ng get-config peerIdOrLabel myNG
 ```
 
 ## Demos & examples

diff --git a/src/commands/ng-print.js b/src/commands/ng-print.js
@@ -7,7 +7,7 @@ import * as NG from '../models/ng.js';
  * @param {string} format Output format
  * @param {boolean} full If true, get more details about the Network Group (default: false)
  */
-export function ng (ng, format, full = false) {
+function printNg (ng, format, full = false) {
 
   switch (format) {
     case 'json': {
@@ -53,7 +53,7 @@ export function ng (ng, format, full = false) {
  * @param {Object} member The Network Group member to print
  * @param {string} format Output format
  */
-export function member (member, format) {
+function printMember (member, format) {
 
   switch (format) {
     case 'json': {
@@ -75,7 +75,7 @@ export function member (member, format) {
  * @param {string} format Output format
  * @param {boolean} full If true, get more details about the peer (default: false)
  */
-export function peer (peer, format, full = false) {
+function printPeer (peer, format, full = false) {
   switch (format) {
     case 'json': {
       Logger.println(JSON.stringify(peer, null, 2));
@@ -92,7 +92,7 @@ export function peer (peer, format, full = false) {
  * @param {Object} peer
  * @param {boolean} full If true, get more details about the peer (default: false)
  */
-export function formatPeer (peer, full = false) {
+function formatPeer (peer, full = false) {
   let peerToPrint = {
     'Parent Member': peer.parentMember,
     ID: peer.id,
@@ -122,7 +122,7 @@ export function formatPeer (peer, full = false) {
  * @param {string} action Action to perform (search or get)
  * @param {string} type Type of item to search (NetworkGroup, Member, Peer)
  */
-export async function results (idOrLabel, org, format, action, type) {
+export async function printResults (idOrLabel, org, format, action, type) {
 
   const exactMatch = action === 'get';
   type = type ?? (action === 'search' ? 'all' : 'single');
@@ -140,12 +140,12 @@ export async function results (idOrLabel, org, format, action, type) {
   if (found.length === 1) {
     switch (found[0].type) {
       case 'NetworkGroup':
-        return ng(found[0], format, true);
+        return printNg(found[0], format, true);
       case 'Member':
-        return member(found[0], format);
+        return printMember(found[0], format);
       case 'CleverPeer':
       case 'ExternalPeer':
-        return peer(found[0], format, true);
+        return printPeer(found[0], format, true);
       default:
         throw new Error(`Unknown item type: ${found[0].type}`);
     }
@@ -170,18 +170,18 @@ export async function results (idOrLabel, org, format, action, type) {
       default: {
         if (grouped.NetworkGroup) {
           Logger.println(`${colors.bold(` • Found ${grouped.NetworkGroup.length} Network Group(s):`)}`);
-          grouped.NetworkGroup?.forEach((item) => ng(item, format));
+          grouped.NetworkGroup?.forEach((item) => printNg(item, format));
         }
 
         if (grouped.Member) {
           Logger.println(`${colors.bold(` • Found ${grouped.Member.length} Member(s):`)}`);
-          grouped.Member?.forEach((item) => member(item, format));
+          grouped.Member?.forEach((item) => printMember(item, format));
         }
 
         if (grouped.ExternalPeer || grouped.CleverPeer) {
           Logger.println(`${colors.bold(` • Found ${grouped.ExternalPeer.length + grouped.CleverPeer.length} Peer(s):`)}`);
-          grouped.CleverPeer?.forEach((item) => peer(item, format));
-          grouped.ExternalPeer?.forEach((item) => peer(item, format));
+          grouped.CleverPeer?.forEach((item) => printPeer(item, format));
+          grouped.ExternalPeer?.forEach((item) => printPeer(item, format));
         }
       }
     }

diff --git a/src/commands/ng.js b/src/commands/ng.js
@@ -1,7 +1,7 @@
 import colors from 'colors/safe.js';
 import { Logger } from '../logger.js';
 import * as NG from '../models/ng.js';
-import * as NGPrint from './ng-print.js';
+import { printResults } from './ng-print.js';
 import * as NGResources from '../models/ng-resources.js';
 
 /** Create a Network Group
@@ -168,11 +168,9 @@ export async function listNg (params) {
 export async function get (params) {
   const [idOrLabel] = params.args;
   const { org, format } = params.options;
-  let type = params.options.type;
+  const type = params.options.type ?? 'single';
 
-  if (!type) type = 'single';
-
-  NGPrint.results(idOrLabel, org, format, 'get', type);
+  printResults(idOrLabel, org, format, 'get', type);
 }
 
 /** Show information about a Network Group, a member or a peer
@@ -186,5 +184,5 @@ export async function search (params) {
   const { org, format } = params.options;
   const type = params.options.type;
 
-  NGPrint.results(idOrLabel, org, format, 'search', type);
+  printResults(idOrLabel, org, format, 'search', type);
 }
diff --git a/src/experimental-features.js b/src/experimental-features.js
@@ -29,7 +29,7 @@ Learn more about Materia KV: https://www.clever-cloud.com/developers/doc/addons/
     clever ng link app_xxx myNG
     clever ng unlink addon_xxx myNG
 - Get the Wireguard configuration of a peer:
-    clever ng get-config peerIdOrLabel
+    clever ng get-config peerIdOrLabel myNG
 - Get details about a Network Group, a member or a peer:
     clever ng get myNg
     clever ng get app_xxx

diff --git a/src/models/ng-resources.js b/src/models/ng-resources.js
@@ -27,7 +27,7 @@ export async function createExternalPeerWithParent (ngIdOrLabel, peerLabel, publ
   const [ng] = await NG.searchNgOrResource(ngIdOrLabel, org, 'NetworkGroup');
 
   if (!ng) {
-    throw new Error(`Network Group ${colors.red(ngIdOrLabel.ngId || ngIdOrLabel.ngLabel)} not found`);
+    throw new Error(`Network Group ${colors.red(ngIdOrLabel.ngId || ngIdOrLabel.ngResourceLabel)} not found`);
   }
 
   // We define a parent member for the external peer
@@ -84,7 +84,7 @@ export async function deleteExternalPeerWithParent (ngIdOrLabel, peerIdOrLabel,
   const [ng] = await NG.searchNgOrResource(ngIdOrLabel, org, 'NetworkGroup');
 
   if (!ng) {
-    throw new Error(`Network Group ${colors.red(ngIdOrLabel.ngId || ngIdOrLabel.ngLabel)} not found`);
+    throw new Error(`Network Group ${colors.red(ngIdOrLabel.ngId || ngIdOrLabel.ngResourceLabel)} not found`);
   }
 
   const externalPeer = peerIdOrLabel.startsWith('external_')
@@ -125,17 +125,17 @@ export async function deleteExternalPeerWithParent (ngIdOrLabel, peerIdOrLabel,
  */
 export async function linkMember (ngIdOrLabel, memberId, org, label) {
   if (!memberId) {
-    throw new Error('A valid member ID is required');
+    throw new Error('A valid member ID is required (addon_xxx, app_xxx, external_xxx)');
   }
 
-  checkMembersToLink([memberId]);
-
   const [ng] = await NG.searchNgOrResource(ngIdOrLabel, org, 'NetworkGroup');
 
   if (!ng) {
     throw new Error(`Network Group ${colors.red(ngIdOrLabel.ngId || ngIdOrLabel.ngResourceLabel)} not found`);
   }
 
+  await checkMembersToLink([memberId]);
+
   const alreadyMember = ng.members.find((m) => m.id === memberId);
   if (alreadyMember) {
     throw new Error(`Member ${colors.red(memberId)} is already linked to Network Group ${colors.red(ng.id)}`);
@@ -174,7 +174,7 @@ export async function linkMember (ngIdOrLabel, memberId, org, label) {
  */
 export async function unlinkMember (ngIdOrLabel, memberId, org) {
   if (!memberId) {
-    throw new Error('A valid member ID is required');
+    throw new Error('A valid member ID is required (addon_xxx, app_xxx, external_xxx)');
   }
 
   const [ng] = await NG.searchNgOrResource(ngIdOrLabel, org, 'NetworkGroup');
@@ -228,7 +228,7 @@ export async function checkMembersToLink (members) {
     else if (!foundRessource && !memberId.startsWith('external_')) {
       membersNotOK.push(memberId);
     }
-  };
+  }
 
   if (membersNotOK.length > 0) {
     Logger.error(`Member(s) ${colors.red(membersNotOK.join(', '))} can't be linked to a Network Group`);
@@ -242,8 +242,8 @@ export async function checkMembersToLink (members) {
  * @param {object} org Organisation ID or name
  * @param {string} resource Resource ID or label
  * @param {boolean} shouldBePresent Expected presence of the resource
- * @param {string} resourceType Resource type (member or peer), default is member
- * @param {string} searchBy Search by 'id' or 'label', default is 'id'
+ * @param {string} [resourceType] Resource type (member or peer), default is member
+ * @param {string} [searchBy] Search by 'id' or 'label', default is 'id'
  * @returns {Promise<boolean>} True if the resource is present, false otherwise
  */
 async function checkResource (ngId, org, resource, shouldBePresent, resourceType = 'member', searchBy = 'id') {

diff --git a/src/models/ng.js b/src/models/ng.js
@@ -32,7 +32,7 @@ export async function create (label, description, tags, membersIds, orgaIdOrName
     throw new Error('A valid Network Group label is required');
   }
 
-  if (membersIds && membersIds.length > 0) {
+  if (membersIds?.length > 0) {
     await checkMembersToLink(membersIds);
   }
 
@@ -81,18 +81,18 @@ export async function destroy (ngIdOrLabel, orgaIdOrName) {
  * @throws {Error} If the Peer is not in the Network Group
  */
 export async function getPeerConfig (peerIdOrLabel, ngIdOrLabel, orgaIdOrName) {
-  const [peer] = await searchNgOrResource(peerIdOrLabel, orgaIdOrName, 'Peer');
-
-  if (!peer || (peerIdOrLabel.ngResourceLabel && peer.label !== peerIdOrLabel.ngResourceLabel)) {
-    throw new Error(`Peer ${colors.red(peerIdOrLabel.ngResourceLabel || peerIdOrLabel.member)} not found`);
-  }
-
   const [parentNg] = await searchNgOrResource(ngIdOrLabel, orgaIdOrName, 'NetworkGroup');
 
   if (!parentNg) {
     throw new Error(`Network Group ${colors.red(ngIdOrLabel.ngId || ngIdOrLabel.ngResourceLabel)} not found`);
   }
 
+  const [peer] = await searchNgOrResource(peerIdOrLabel, orgaIdOrName, 'Peer');
+
+  if (!peer || (peerIdOrLabel.ngResourceLabel && peer.label !== peerIdOrLabel.ngResourceLabel)) {
+    throw new Error(`Peer ${colors.red(peerIdOrLabel.ngResourceLabel || peerIdOrLabel.member)} not found`);
+  }
+
   if (!parentNg.peers.find((p) => p.id === peer.id)) {
     throw new Error(`Peer ${colors.red(peer.id)} is not in Network Group ${colors.red(parentNg.id)}`);
   }
@@ -110,7 +110,7 @@ export async function getPeerConfig (peerIdOrLabel, ngIdOrLabel, orgaIdOrName) {
 
 /**
  * Get a Network group from an owner with members and peers
- * @param {object} ngIdOrLabel The Network Group ID or Label
+ * @param {string} networkGroupId The Network Group ID
  * @param {string} orgaIdOrName The owner ID or name
  * @returns {Promise<Array<Object>>} The Network Groups
  */
@@ -142,7 +142,7 @@ export async function getAllNGs (orgaIdOrName) {
  * Search a Network Group or a resource (member/peer)
  * @param {string|Object} idOrLabel The ID or label to look for
  * @param {Object} orgaIdOrName The owner ID or name
- * @param {boolean} type Look only for a specific type (NetworkGroup, Member, CleverPeer, ExternalPeer, Peer), can be 'single', default to 'all'
+ * @param {string} [type] Look only for a specific type (NetworkGroup, Member, CleverPeer, ExternalPeer, Peer), can be 'single', default to 'all'
  * @param {boolean} exactMatch Look for exact match, default to true
  * @throws {Error} If multiple Network Groups or member/peer are found in single_result mode
  * @returns {Promise<Object>} Found results
@@ -183,9 +183,9 @@ export async function searchNgOrResource (idOrLabel, orgaIdOrName, type = 'all',
     filtered = filtered.filter((f) => f.id === query || f.label === query);
   }
 
-  if (filtered.length > 1 && !type === 'all') {
+  if (filtered.length > 1 && type !== 'all') {
     throw new Error(`Multiple resources found for ${colors.red(query)}, use ID instead:
-${filtered.map((f) => ` - ${f.id} ${colors.grey(`(${f.label} - ${f.type})`)}`).join('\n')}`);
+${filtered.map((f) => ` • ${f.id} ${colors.grey(`(${f.label} - ${f.type})`)}`).join('\n')}`);
   }
 
   // Deduplicate results
@@ -195,7 +195,7 @@ ${filtered.map((f) => ` - ${f.id} ${colors.grey(`(${f.label} - ${f.type})`)}`).j
 /**
  * Construct members from members_ids
  * @param {string} ngId The Network Group ID
- * @param {Array<string>} members_ids The members IDs
+ * @param {Array<string>} membersIds The members IDs
  * @returns {Array<Object>} Array of members with id, domainName and kind
  */
 export function constructMembers (ngId, membersIds) {
@@ -225,39 +225,45 @@ export function constructMembers (ngId, membersIds) {
 async function pollNetworkGroup (ownerId, ngId, { waitForMembers = null, waitForDeletion = false } = {}) {
   return new Promise((resolve, reject) => {
     Logger.info(`Polling Network Groups from owner ${ownerId}`);
+    const timeoutTime = Date.now() + (TIMEOUT * 1000);
 
-    const poll = setInterval(async () => {
-      // We don't use ngApi.getNetworkGroup(), it will lead to an error before creation
-      const ngs = await ngApi.listNetworkGroups({ ownerId }).then(sendToApi);
-      const ng = ngs.find((ng) => ng.id === ngId);
-
-      if (waitForDeletion && !ng) {
-        cleanup(true);
+    async function pollOnce () {
+      if (Date.now() > timeoutTime) {
+        const action = waitForDeletion ? 'deletion of' : 'creation of';
+        reject(new Error(`Timeout while checking ${action} Network Group ${ngId}`));
         return;
       }
 
-      if (!waitForDeletion && ng) {
-        if (waitForMembers?.length) {
-          const members = ng.members.filter((member) => waitForMembers.includes(member.id));
-          if (members.length !== waitForMembers.length) {
-            Logger.debug(`Waiting for members: ${waitForMembers.join(', ')}`);
-            return;
-          }
+      try {
+        const ngs = await ngApi.listNetworkGroups({ ownerId }).then(sendToApi);
+        const ng = ngs.find((ng) => ng.id === ngId);
+
+        if (waitForDeletion && !ng) {
+          resolve();
+          return;
         }
-        cleanup(true);
-      }
-    }, INTERVAL);
 
-    const timer = setTimeout(() => {
-      const action = waitForDeletion ? 'deletion of' : 'creation of';
-      cleanup(false, new Error(`Timeout while checking ${action} Network Group ${ngId}`));
-    }, TIMEOUT * 1000);
+        if (!waitForDeletion && ng) {
+          if (waitForMembers?.length) {
+            const members = ng.members.filter((member) => waitForMembers.includes(member.id));
+            if (members.length !== waitForMembers.length) {
+              Logger.debug(`Waiting for members: ${waitForMembers.join(', ')}`);
+              setTimeout(pollOnce, INTERVAL);
+              return;
+            }
+          }
+          resolve();
+          return;
+        }
 
-    function cleanup (success, error = null) {
-      clearInterval(poll);
-      clearTimeout(timer);
-      success ? resolve() : reject(error);
+        setTimeout(pollOnce, INTERVAL);
+      }
+      catch (error) {
+        reject(error);
+      }
     }
+
+    pollOnce();
   });
 }