Feat/Add hangSecondsBeforeUpsertingSwagger option

[MarcioMeier]

🚨 Please review the guidelines for contributing to this repository.

Adds the hangSecondsBeforeUpsertingSwagger option to hang before upserting WSO2 swagger

In some complex/distributed WSO2 setups it may take a while to synchronize the API definitions. When upserting the swagger right after the API def update, it will override the common data between them (e.g. CORS Headers) with the outdated API definitions.
When that happens, adding a hang time in between helps updating both the API definitions & the swagger successfully.

It fixes #99

• Updated unit tests (*.spec.js)
• Updated e2e tests (here)
• Updated documentation (here)

🙌 Thank you!

[flaviostutz]

I think we should have a better way of making sure the API deployment works (e.g. by checking if the service is in the correct state before updating swagger), but as we don't know how to do this, being able to do this delay is better than having the failures we are facing randomly, so +1 for this one!

[ramgrandhi]

First of all, Thanks for identifying and also coming up with a fix for this @MarcioMeier! I have one question.

What is the reason to expose hangSecondsBeforeUpsertingSwagger as a configurable parameter? Is this something a serverless developer has to worry about?

P.S. You've been added as contributors to the main repo; so, feel free to rebase your PR to this repo.

[MarcioMeier]

I believe this can change based on the WSO2 setup. In my scenario, we have multiple WSO2 clusters, which take a few seconds to sync in all instances. I did some tests, and I'll add a 30-second delay, but if we had more clusters, maybe we would need more delay. On the other hand, if we had only 1 cluster, we would need no delay at all.

The reason I added the parameter is to not delay every request by 30 seconds, and if someone needs more, they can delay even more.

[ramgrandhi]

I believe this can change based on the WSO2 setup. In my scenario, we have multiple WSO2 clusters, which take a few seconds to sync in all instances. I did some tests, and I'll add a 30-second delay, but if we had more clusters, maybe we would need more delay. On the other hand, if we had only 1 cluster, we would need no delay at all.

The reason I added the parameter is to not delay every request by 30 seconds, and if someone needs more, they can delay even more.

Thanks for the clarification @MarcioMeier!

Help me out here .. what's bothering me is the already very long serverless.yml config that this plugin requires to actually do the job. Given the nature of your parameter is more about internal workings of the plugin & wso2 server (and not the API deployments, per say), would you suggest other approaches to pass this parameter/value to the plugin apart from extending serverless.yml? It is our collective benefit to keep serverless.yml config as simple as possible.

[MarcioMeier]

I see your concern, in the last service I worked on, we had 300+ lines just for this plugin.
On the other hand, the approaches I know to provide parameters to the plugin are via serverless config or environment variables. If I had to choose, I'd always go with serverless config.

We could fetch the data after upserting the swagger to check if the changes were applied, but then it will be really hard to check against every API def config.

Unless we could do something like

const expectedApiDef = wso2apim.constructAPIDef(wso2APIM.user, wso2APIM.gatewayEnv, apiDefs[i]);
let retries = 0;
while {
  await wso2apim.createAPIDef(...);
  
  // hang 15 seconds on every retry
  await utils.goToSleep(retries * 15000);
 
  await wso2apim.upsertSwaggerSpec(...)
  
  // P.S: I don't know if we can retrieve the API defs from WSO2 server
  const result = await wso2apim.getApiDefs(...)

  // This is the tricky part, if we can compare the whole config it is nice
  // if we need to go over the fields individually, it will be hard
  if (JSON.stringify(result.apiDefs) === JSON.stringify(expectedApiDef)) {
    break;
  }
  
  if (retries > 2) {
    throw new Error('max retries reached');
  }

  retries++
}

[flaviostutz]

I see your concern, in the last service I worked on, we had 300+ lines just for this plugin. On the other hand, the approaches I know to provide parameters to the plugin are via serverless config or environment variables. If I had to choose, I'd always go with serverless config.

We could fetch the data after upserting the swagger to check if the changes were applied, but then it will be really hard to check against every API def config.

Unless we could do something like

const expectedApiDef = wso2apim.constructAPIDef(wso2APIM.user, wso2APIM.gatewayEnv, apiDefs[i]);
let retries = 0;
while {
  await wso2apim.createAPIDef(...);
  
  // hang 15 seconds on every retry
  await utils.goToSleep(retries * 15000);
 
  await wso2apim.upsertSwaggerSpec(...)
  
  // P.S: I don't know if we can retrieve the API defs from WSO2 server
  const result = await wso2apim.getApiDefs(...)

  // This is the tricky part, if we can compare the whole config it is nice
  // if we need to go over the fields individually, it will be hard
  if (JSON.stringify(result.apiDefs) === JSON.stringify(expectedApiDef)) {
    break;
  }
  
  if (retries > 2) {
    throw new Error('max retries reached');
  }

  retries++
}

I like this approach. Actually I came here to give this idea of retries also 😬.

In order to couple with best and worst case scenarios automatically, we could use a retry with a multiplier x2 (like we can se in StepFunctions, for example) as a backoff mechanism.

So it starts with sleeping 1s, then multiplies it by 2, so that it will sleep-retry in 2s, then 4s, 8s, 16s, 32s, 64s... with max retries of 6. I don't think it would create a DDoS situation on WSO2 because we have almost no concurrence with the usage of this plugin in production.

We can also go simple by using the linear approach @MarcioMeier suggested, but I would use a lower number (3000ms instead of 15000) so simpler environments doesn't get too long to be deployed.

This would adapt well to small and large clusters.

What do you think?

[ramgrandhi]

I see your concern, in the last service I worked on, we had 300+ lines just for this plugin. On the other hand, the approaches I know to provide parameters to the plugin are via serverless config or environment variables. If I had to choose, I'd always go with serverless config.

We could fetch the data after upserting the swagger to check if the changes were applied, but then it will be really hard to check against every API def config.

Unless we could do something like

const expectedApiDef = wso2apim.constructAPIDef(wso2APIM.user, wso2APIM.gatewayEnv, apiDefs[i]);
let retries = 0;
while {
  await wso2apim.createAPIDef(...);
  
  // hang 15 seconds on every retry
  await utils.goToSleep(retries * 15000);
 
  await wso2apim.upsertSwaggerSpec(...)
  
  // P.S: I don't know if we can retrieve the API defs from WSO2 server
  const result = await wso2apim.getApiDefs(...)

  // This is the tricky part, if we can compare the whole config it is nice
  // if we need to go over the fields individually, it will be hard
  if (JSON.stringify(result.apiDefs) === JSON.stringify(expectedApiDef)) {
    break;
  }
  
  if (retries > 2) {
    throw new Error('max retries reached');
  }

  retries++
}

Thanks for giving it a thought! I like the exponential backoff you suggested due to its subtlety.

It is worth trying this, you may use these endpoints of WSO2 appropriately to retrieve API definition JSON from the server (for local comparison).

• For 2.1.0, go here and search for /apis/{apiId}.
• For 3.2.0, go here.

Let me know if you need something. Thanks for picking this up! 🙌