change the default action context to omit api key

[rabbah]

Actions receives the API key in the environment even if it is not necessary. This should not be the default behavior. With this issue I'm proposing that we flip the default and provide an annotation on the action to enable the key forwarding to preserve existing behavior.

[csantanapr]

Security by default, is my kind of default 👍

[rabbah]

We currently created the following context:

{
       "api_host": process.env['__OW_API_HOST'],
       "api_key": process.env['__OW_API_KEY'],
       "namespace": process.env['__OW_NAMESPACE'],
       "action_name": process.env['__OW_ACTION_NAME'],
       "activation_id": process.env['__OW_ACTIVATION_ID'],
       "deadline": process.env['__OW_DEADLINE']
}

https://github.com/apache/incubator-openwhisk/blob/da21c9fe49b2ae72c95b6866b30d984c65253724/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala#L565-L571

Should we hide the namespace, action name and activation id as well?

[rabbah]

Some thoughts on the dev list to my question above: https://lists.apache.org/thread.html/5e68f68f0879c3a2d5fe78ee74eae53ea1ce2caf1bcbe23baceaaec0@%3Cdev.openwhisk.apache.org%3E

Summary so far: exclude the key only.

[falkzoll]

Forwarding the API key only when it is really needed, means more security 👍 .

One additional thought:
From maintenance point of view, it would be comfortable to have the activation of backward compatibility breaking changes configurable :).

This would allow the devops teams to deploy the actual code to their infrastructure and to communicate and activate the feature according to their individual schedules.
The configurable activation would also make it easier to develop and test migration procedures (e.g. database updates) for an improved user experience when making the backward compatibility breaking change active.

And in the worst case, it would allow a more easy roll back when something happens after it was activated, that was not foreseen. Changing the configuration and maybe a restart is simpler and faster than jumping back to a previous version which may need to be redeployed, first.

I know the related PR is almost complete. But maybe this is something to be considered for future changes that also break backward compatibility :). What do you think?

[rabbah]

@falkzoll you're welcome to submit a pull request separately or send me a pull request to make the change configuration at deployment time.

[rabbah]

I requested additional feedback on the dev list [1] to see if there's a wider desire for a deployment-time toggle specific to this change. In general, I agree and I think we've generally done well keeping backward compatibility. This also goes toward pushing for more frequent releases that can mitigate the risks of riding the master branch in production settings.

[1] https://lists.apache.org/thread.html/d18d0603abb894e8b7919b38a3199d76134058b6d200b85a943a3bdb@%3Cdev.openwhisk.apache.org%3E

[falkzoll]

many thanks for your response... yes, actively contributing would be the best 👍 ...I actually started, getting familiar with the openwhisk code :) ... but I am not deep enough, yet, to really contribute modified code for this change already... but I continue working on it to make this happen for future changes :)

[chetanmeh]

@falkzoll there is now PR #4334 to add feature flag support for this feature.