RSDK-9651 - Add cloud metadata and api key to module env vars #4736
Conversation
viambot
added
the
safe to test
viambot
added
safe to test
| @@ -596,6 +627,13 @@ func processConfig(unprocessedConfig *Config, fromCloud bool, logger logging.Log | |||
| } | |||
| } | |||
|
|
|||
| // add additional environment vars to modules | |||
| // adding them here ensures that if the parsed API key changes, the module will be restarted with the updated environment. | |||
There was a problem hiding this comment.
the module will be restarted
Where is that happening?
There was a problem hiding this comment.
this happens when we diff the config (the env being different on the module config will cause a reconfigure on the module), so more of just utilizing the existing mechanisms to make sure the module is running in the most up to date environment
There was a problem hiding this comment.
Cool that makes sense; thanks.
| continue | ||
| } | ||
| // the keys come in unsorted, so sort the keys so we'll always get the same API key | ||
| // if there are no changes |
There was a problem hiding this comment.
How do we know that the first API key is the one the module wants?
There was a problem hiding this comment.
we don't - it was mentioned during 6 week check-in that we should just make it easy to access, so opted for this instead of returning all of them.
| env = append(env, v) | ||
| } | ||
| for key, val := range envVars { | ||
| // mask the secret |
There was a problem hiding this comment.
Will the module still get access to this?
There was a problem hiding this comment.
ya, the map passed in is not updated
Co-authored-by: Benjamin Rewis <[email protected]>
viambot
added
safe to test
| @@ -30,6 +31,31 @@ const ( | |||
|
|
|||
| // AndroidFilesDir is hardcoded because golang inits before our android code can override HOME var. | |||
| AndroidFilesDir = "/data/user/0/com.viam.rdk.fgservice/cache" | |||
|
|
|||
| // ViamEnvVarPrefix is the prefix for all Viam-related environment variables. | |||
| ViamEnvVarPrefix = "VIAM_" | |||
There was a problem hiding this comment.
Yeah new env vars should definitely start with VIAM_ now; I feel like that was already an expectation, but thanks for encoding more formally.
added them in the processConfig step so that changes to API keys can cause a module reconfigure. otherwise API keys may change without changing the actual environment the module is running in.
In theory, we can add the extra env vars later in the process (around when we create the NewModuleManager), but opted not to because of then env vars and the actual api key might get out of sync
example log for a cloud robot
example log for a local