Admin APIs and tools for node params (without refreshing) #1051

kriii · 2022-05-27T05:50:48Z

Changes

Generalize exist APIs
Fix tools due to API changes
Fix some typos

TODO

Make the collection of callbacks for some parameters that need a refresh.

Related Issue

Generalize APIs and tools for node prams #956

liayoo · 2022-05-27T13:57:44Z

tools/api-access/removeFromDevClientApiIpWhitelist.js

-  console.log('node tools/api-access/getDevClientApiIpWhitelist.js http://localhost:8081 0 private_key 127.0.0.1');
-  console.log('node tools/api-access/getDevClientApiIpWhitelist.js http://localhost:8081 0 mnemonic 127.0.0.1');
-  console.log('node tools/api-access/getDevClientApiIpWhitelist.js http://localhost:8081 0 keystore /path/to/keystore/file 127.0.0.1');
+  console.log('node tools/api-access/removeFromDevClientApiIpWhitelist.js http://localhost:8081 0 private_key 127.0.0.1');


thanks for fixing these~ 👍

liayoo · 2022-05-27T13:58:36Z

common/result-code.js

  // ain_validateAppName
-  INVALID_APP_NAME_FOR_STATE_LABEL: 30701,


Could you make sure to run the unit & integration tests? I think they would need some updates from this change.

Thanks for denoting this! I couldn't think these codes were directly used in the tests. Fixed now!

liayoo · 2022-05-27T13:59:33Z

json_rpc/constants.js

  AIN_GET_EVENT_HANDLER_CHANNEL_INFO: 'ain_getEventHandlerChannelInfo',
  AIN_GET_EVENT_HANDLER_FILTER_INFO: 'ain_getEventHandlerFilterInfo',
  AIN_GET_LAST_BLOCK: 'ain_getLastBlock',
  AIN_GET_LAST_BLOCK_NUMBER: 'ain_getLastBlockNumber',
+  AIN_GET_NODE_PARAM: 'ain_getNodeParam',


Could you add tool scripts for the new apis?

New tools added!

platfowner · 2022-05-29T18:14:24Z

json_rpc/admin.js

-          verified) {
-        done(null,
-            JsonRpcUtil.addProtocolVersion({ result: NodeConfigs.DEV_CLIENT_API_IP_WHITELIST }));
+      trafficStatsManager.addEvent(TrafficEventTypes.ACCESS_CONTROL_SET, latency);


s/ACCESS_CONTROL_SET/ACCESS_CONTROL_GET/?

Didn't notice it! Fixed now, thanks!

platfowner · 2022-05-29T18:23:34Z

json_rpc/admin.js

+      if (_.get(args.message, 'method') !== JSON_RPC_METHODS.AIN_GET_NODE_PARAM || !verified) {
+        const latency = Date.now() - beginTime;
+        trafficStatsManager.addEvent(TrafficEventTypes.ACCESS_CONTROL_SET, latency);
+        done({ code: 403, message: 'Forbidden' });


it looks a little bit out of the scope but could you define a result code in JsonRpcApiResultCode for this case?

platfowner · 2022-05-29T18:24:22Z

json_rpc/admin.js

      const beginTime = Date.now();
      const verified = node.verifyNodeAccountSignature(args.message, args.signature);
-      if (_.get(args.message, 'method') !== JSON_RPC_METHODS.AIN_ADD_TO_DEV_CLIENT_API_IP_WHITELIST ||
-          !verified) {
+      if (_.get(args.message, 'method') !== JSON_RPC_METHODS.AIN_SET_NODE_PARAM || !verified) {
        const latency = Date.now() - beginTime;
        trafficStatsManager.addEvent(TrafficEventTypes.ACCESS_CONTROL_SET, latency);
        done({ code: 403, message: 'Forbidden' });


platfowner · 2022-05-29T18:24:36Z

json_rpc/admin.js

+      if (_.get(args.message, 'method') !== JSON_RPC_METHODS.AIN_ADD_TO_WHITELIST_NODE_PARAM || !verified) {
+        const latency = Date.now() - beginTime;
+        trafficStatsManager.addEvent(TrafficEventTypes.ACCESS_CONTROL_SET, latency);
+        done({ code: 403, message: 'Forbidden' });


platfowner · 2022-05-29T18:25:54Z

json_rpc/admin.js

+        done(null, JsonRpcUtil.addProtocolVersion({
+          result: {
+            code: JsonRpcApiResultCode.PARAM_INVALID,
+            message: `Param [${param}] is not exist.`


s/is/does/?

platfowner · 2022-05-29T18:26:56Z

json_rpc/admin.js

-            code: JsonRpcApiResultCode.SUCCESS,
-            message: `Added IP (${args.message.ip}) to whitelist: ${JSON.stringify(NodeConfigs.DEV_CLIENT_API_IP_WHITELIST)}`
+            code: JsonRpcApiResultCode.PARAM_INVALID,
+            message: `Param [${param}] is not exists.`


s/is not exists/does not exist/?

platfowner · 2022-05-29T18:33:22Z

json_rpc/admin.js

-            code: JsonRpcApiResultCode.IP_ALREADY_IN_WHITELIST,
-            message: `IP (${args.message.ip}) already in whitelist: ${JSON.stringify(NodeConfigs.DEV_CLIENT_API_IP_WHITELIST)}`
+            code: JsonRpcApiResultCode.PARAM_INVALID,
+            message: `Param [${param}] is not exists.`


platfowner · 2022-05-29T18:34:08Z

json_rpc/admin.js

-            code: JsonRpcApiResultCode.SUCCESS,
-            message: `Removed IP (${args.message.ip}) from whitelist: ${JSON.stringify(NodeConfigs.DEV_CLIENT_API_IP_WHITELIST)}`
+            code: JsonRpcApiResultCode.PARAM_INVALID,
+            message: `Param [${param}] is not exists.`


platfowner · 2022-05-29T18:36:00Z

json_rpc/admin.js

+        done(null, JsonRpcUtil.addProtocolVersion({
+          result: {
+            code: JsonRpcApiResultCode.PARAM_INVALID,
+            message: `Param [${param}] is not whitelist`


s/not whitelist/not a whitelist/?

platfowner · 2022-05-29T18:37:39Z

common/result-code.js

-  // ain_removeFromDevClientApiIpWhitelist
-  IP_NOT_IN_WHITELIST: 30601,
+  // Admin APIs
+  PARAM_INVALID: 30501,


suggestion) what about adding prefix 'ADMIN_' for these admin API code?

platfowner · 2022-05-29T18:39:58Z

json_rpc/admin.js

-            code: JsonRpcApiResultCode.IP_NOT_IN_WHITELIST,
-            message: `IP (${args.message.ip}) not in whitelist: ${JSON.stringify(NodeConfigs.DEV_CLIENT_API_IP_WHITELIST)}`
+            code: JsonRpcApiResultCode.PARAM_INVALID,
+            message: `Param [${param}] is not whitelist`


platfowner · 2022-05-29T18:42:25Z

tools/api-access/addToDevClientApiIpWhitelist.js

-  console.log('node tools/api-access/getDevClientApiIpWhitelist.js http://localhost:8081 0 private_key 127.0.0.1');
-  console.log('node tools/api-access/getDevClientApiIpWhitelist.js http://localhost:8081 0 mnemonic 127.0.0.1');
-  console.log('node tools/api-access/getDevClientApiIpWhitelist.js http://localhost:8081 0 keystore /path/to/keystore/file 127.0.0.1');
+  console.log('node tools/api-access/addToDevClientApiIpWhitelist.js http://localhost:8081 0 private_key 127.0.0.1');


thanks for fixing this!

platfowner · 2022-05-29T18:44:02Z

tools/api-access/removeFromDevClientApiIpWhitelist.js

-  console.log('node tools/api-access/getDevClientApiIpWhitelist.js http://localhost:8081 0 keystore /path/to/keystore/file 127.0.0.1');
+  console.log('node tools/api-access/removeFromDevClientApiIpWhitelist.js http://localhost:8081 0 private_key 127.0.0.1');
+  console.log('node tools/api-access/removeFromDevClientApiIpWhitelist.js http://localhost:8081 0 mnemonic 127.0.0.1');
+  console.log('node tools/api-access/removeFromDevClientApiIpWhitelist.js http://localhost:8081 0 keystore /path/to/keystore/file 127.0.0.1');


minor) what about adding an example with wildcard ('*')?

platfowner · 2022-05-29T18:44:39Z

tools/api-access/addToDevClientApiIpWhitelist.js

-  console.log('node tools/api-access/getDevClientApiIpWhitelist.js http://localhost:8081 0 keystore /path/to/keystore/file 127.0.0.1');
+  console.log('node tools/api-access/addToDevClientApiIpWhitelist.js http://localhost:8081 0 private_key 127.0.0.1');
+  console.log('node tools/api-access/addToDevClientApiIpWhitelist.js http://localhost:8081 0 mnemonic 127.0.0.1');
+  console.log('node tools/api-access/addToDevClientApiIpWhitelist.js http://localhost:8081 0 keystore /path/to/keystore/file 127.0.0.1');


minor) what about adding an example with wildcard('*')?

kriii · 2022-05-30T17:10:50Z

I addressed all reviews and made some changes in API, please take a look!

platfowner · 2022-05-31T02:35:18Z

json_rpc/admin.js

-            code: JsonRpcApiResultCode.SUCCESS,
-            message: `Added IP (${args.message.ip}) to whitelist: ${JSON.stringify(NodeConfigs.DEV_CLIENT_API_IP_WHITELIST)}`
+            code: JsonRpcApiResultCode.ADMIN_PARAM_INVALID,
+            message: `Param [${param}] does not exists.`


minor) s/exists/exist/?

Sorry, fixed!

platfowner · 2022-05-31T02:35:31Z

json_rpc/admin.js

-            code: JsonRpcApiResultCode.IP_ALREADY_IN_WHITELIST,
-            message: `IP (${args.message.ip}) already in whitelist: ${JSON.stringify(NodeConfigs.DEV_CLIENT_API_IP_WHITELIST)}`
+            code: JsonRpcApiResultCode.ADMIN_PARAM_INVALID,
+            message: `Param [${param}] does not exists.`


platfowner · 2022-05-31T02:35:41Z

json_rpc/admin.js

+        done(null, JsonRpcUtil.addProtocolVersion({
+          result: {
+            code: JsonRpcApiResultCode.ADMIN_PARAM_INVALID,
+            message: `Param [${param}] does not exists.`


platfowner

LGTM with some minor comments.
Thanks!

liayoo

LGTM, thanks!!

kriii · 2022-06-03T01:16:58Z

Now merged! Thanks for the reviews!

kriii added 2 commits May 27, 2022 11:51

Admin node param APIs

5af151a

Fix tools

59c70bd

kriii requested review from liayoo, platfowner, shyun-comcom and minsulee2 May 27, 2022 05:50

kriii changed the base branch from master to develop May 27, 2022 05:51

liayoo reviewed May 27, 2022

View reviewed changes

kriii added 2 commits May 27, 2022 23:19

Fix integration tests

da41135

Fix json prc set method set

ce33486

platfowner reviewed May 29, 2022

View reviewed changes

kriii added 5 commits May 30, 2022 10:19

Fix results

9cea149

Add wildcard example for tools

a1611e6

Change set methods value handling as like evironment variable-ish

bb9dad0

Fix tool usages

5d140ef

Add tools for new APIs

b02b08d

platfowner reviewed May 31, 2022

View reviewed changes

Fix typo

43434cd

platfowner approved these changes May 31, 2022

View reviewed changes

kriii added the enhancement New feature or request label May 31, 2022

kriii changed the title ~~Admin APIs for node params (without refreshing)~~ Admin APIs and tools for node params (without refreshing) May 31, 2022

Add TODO

cea30fd

liayoo approved these changes May 31, 2022

View reviewed changes

kriii merged commit 6b63787 into develop Jun 3, 2022

kriii deleted the feature/kriii/node_param_api branch June 3, 2022 01:16

		// ain_validateAppName
		INVALID_APP_NAME_FOR_STATE_LABEL: 30701,

Admin APIs and tools for node params (without refreshing) #1051

Admin APIs and tools for node params (without refreshing) #1051

Conversation

kriii commented May 27, 2022

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

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

platfowner May 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

platfowner May 29, 2022 • 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

platfowner May 29, 2022 • 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

platfowner May 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

platfowner May 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kriii commented May 30, 2022

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

platfowner left a comment

Choose a reason for hiding this comment

liayoo left a comment

Choose a reason for hiding this comment

kriii commented Jun 3, 2022

platfowner May 29, 2022 •

edited

Loading

platfowner May 29, 2022 •

edited

Loading

platfowner May 29, 2022 •

edited

Loading

platfowner May 29, 2022 •

edited

Loading

platfowner May 29, 2022 •

edited

Loading