Support alt. namespace resource uuid as tenant id to API gatway service #1076
Conversation
| @@ -103,13 +106,16 @@ func readFromCLI() { | |||
| key = GetPropertyValue(key, keyfile, wski18n.COMMAND_LINE) | |||
| cert = GetPropertyValue(cert, certfile, wski18n.COMMAND_LINE) | |||
| apigwAccessToken = GetPropertyValue(apigwAccessToken, accessToken, wski18n.COMMAND_LINE) | |||
| // TODO optionally allow this value to be set from command line arg. | |||
There was a problem hiding this comment.
Not sure whether this is necessary. I don't think CRN # is intended for users to manipulate directly. I'll double check with whisk-team.
There was a problem hiding this comment.
with wskdeploy, we had taken the approach of allowing users to provide per-deployment settings via command line or a deployment file that accounted for different values in wskprops as it is often the case that diff. namespaces, tokens, certs, etc. are used for different stages of lifecycle (e.g., dev/test/prod). If we do not provide a way to set these values per-deployment, then we have to ask the user/developer to always go back and manually edit the .wskprops or provide multiple whisk.properties files they change between. We tried to take a friendlier approach... otherwise, I see it very challenging to manage namespaces dynamically as it is part of the programming model and can change very dynamically (not ideal for changing a supposedly hidden .wskprops file).
| @@ -291,5 +308,11 @@ func validateClientConfig(credential PropertyValue, apiHost PropertyValue, names | |||
| wskprint.PrintOpenWhiskVerbose(utils.Flags.Verbose, stdout) | |||
| } | |||
|
|
|||
| if len(apigwTenantId.Value) != 0 { | |||
There was a problem hiding this comment.
Since this is verbose, should the user know that their APIGW_TENANT_ID is empty if they are targeting CF namespace? I think we should print out the apigwAccessTenantId regardless the type of namespace being targeted.
There was a problem hiding this comment.
It is my belief that as the code is not authored that all values are either printed (verbose) or not (default/non-verbose); there should be no distinction between these 2 values or any correlation between them that assumes one or the other (i.e., should always print all known properties without control logic).
| @@ -229,7 +229,7 @@ type Project struct { | |||
| Namespace string `yaml:"namespace"` | |||
| Credential string `yaml:"credential"` | |||
| ApiHost string `yaml:"apiHost"` | |||
| ApigwAccessToken string `yaml:"apigwAccessToken"` | |||
| ApigwAccessToken string `yaml:"apigwAccessToken"` // TODO: support apigwTenantId? deprecate? | |||
There was a problem hiding this comment.
Users potentially could use the wrong CRN# in yml file, and if that happens, the value would've been overwritten by the values in wskprops file. The api would be deployed in a namespace unexpected by the users.
There was a problem hiding this comment.
That is the intent, that is the values provided in the manifest/deployment files are per-deployment overrides... if they fail that is OK. Also, it seems that if you have more than 1 IAM namespace, there needs to be a way to toggle between them more dynamically. Looking at my account, i have 2 "default" IAM namespaces now... there is no UI element (or command line) to allow me to say which is truly my "Default" now; therefore, without a way to set this manually, the one auto-set in .wskprops seems to pick one of the 2 arbitrarily... These options allow user to decide and explicitly provide the namespace they intended (via this mechanism, while treating the .wskprops value as the default)
Dependent on apache/openwhisk-client-go#130