search

apache / openwhisk-package-alarms

Apache OpenWhisk package that can be used to create periodic, time-based alarms.
https://openwhisk.apache.org/
Apache License 2.0
24 stars 49 forks source link

Use authKey from action parameters instead of __OW_API_KEY #176

Closed jasonpet closed 5 years ago

jasonpet commented 5 years ago

This is needed to keep current behavior in preparation for the following PR: https://github.com/apache/incubator-openwhisk/pull/4284

rabbah commented 5 years ago

I'm trying to understand why the annotation is actually needed on feed actions which receive the user's api key as an explicit parameter. The __OW_API_KEY is the action owner's key which for the providers is "whisk.system" or similar system key.

I understand there is some overriding of the key that happens from outside vendors who need to inject their own IAM keys but it's still not clear to me why the annotation is necessary or why the provider's administrator key is necessary.

rabbah commented 5 years ago

I've gone through the code with some help from @jasonpet who's helped me understand the IAM connections a little bit - Thanks Jason. But I can't connect all the dots yet.

rabbah commented 5 years ago

I think the entry point for the feed creation is the main in alarm.js, which calls out to the method createWebParams https://github.com/apache/incubator-openwhisk-package-alarms/blob/d59ee2c00730c46c7f391ad954346ed807f2cc07/action/alarm.js#L21.

If I'm following the code, this method then uses the context properties to create the API call to the broker: https://github.com/apache/incubator-openwhisk-package-alarms/blob/2193ee38c211dbafcd9fadc20cff0329e95f98c6/action/lib/common.js#L44-L57

The method ignores the event's api key and instead picks it up from the environment, which I understood the IAM connection, is being used to override the key in some instances from the environment instead of picking up the value from the event payload.

jasonpet commented 5 years ago

yeah, that is correct. line 53 in the createWebParams could be removed and it would then simply use the authKey parameter passed in.

jasonpet commented 5 years ago

@rabbah - thanks for pointing this out. I am thinking the right solution is to remove the line that uses the env var to set the authKey (the one passed in will then be used) and by default do not add the annotation in the install script but make it configurable for vendors that may need it.

rabbah commented 5 years ago

Thanks @jasonpet - if you think that works then it would indeed be cleaner.