Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support alt. namespace resource uuid as tenant id to API gatway service #1076

Merged
merged 11 commits into from
Oct 24, 2019
Merged

Support alt. namespace resource uuid as tenant id to API gatway service #1076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
48703d7
Support alt. namespace resource uuid as tenant id to API gatway service
mrutkows Oct 17, 2019
79b38f1
gofmt
mrutkows Oct 17, 2019
6e20393
gofmt
mrutkows Oct 17, 2019
921103c
wskclient update and remove some trace output
mrutkows Oct 18, 2019
6de5cb9
Attempt to figure out Travis failure
mrutkows Oct 18, 2019
0b5f385
Attempt to figure out Travis failure
mrutkows Oct 18, 2019
39c0596
Attempt to figure out Travis failure
mrutkows Oct 18, 2019
16fcda0
Attempt to figure out Travis failure
mrutkows Oct 18, 2019
6c011fc
Update GoDeps to use the latest openwhisk-client-go commit level
mrutkows Oct 18, 2019
338a3e5
Attempt to figure out Travis failure
mrutkows Oct 18, 2019
e4dff0c
Update GoDeps to use the latest openwhisk-client-go commit level
mrutkows Oct 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@

sudo: required
language: go
go:
- 1.9
services:
- docker

matrix:
include:
- os: linux
sudo: required
go: "1.9.3"
services: docker
dist: xenial

git:
depth: 3

Expand All @@ -41,8 +46,10 @@ install:
before_script:
- GO_FILES=$(find . -iname '*.go' -type f -not -path "./wski18n/i18n_resources.go")
- export BAD_GO=$(gofmt -s -l $(echo $GO_FILES))
- echo $BAD_GO
- test -z "$BAD_GO"
- echo "["$BAD_GO"]"
#- test -z "$BAD_GO"
#- test -z "$(gofmt -s -l $(echo $GO_FILES))"

script:
- echo $TRAVIS
- echo $TRAVIS_PULL_REQUEST
Expand Down
4 changes: 2 additions & 2 deletions Godeps/Godeps.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ dependencies {
build(['name':'github.com/nicksnyder/go-i18n/i18n/bundle', 'version':'991e81cc94f6c54209edb3192cb98e3995ad71c1', 'transitive':false])
build(['name':'github.com/nicksnyder/go-i18n/i18n/language', 'version':'991e81cc94f6c54209edb3192cb98e3995ad71c1', 'transitive':false])
build(['name':'github.com/nicksnyder/go-i18n/i18n/translation', 'version':'991e81cc94f6c54209edb3192cb98e3995ad71c1', 'transitive':false])
build(['name':'github.com/apache/openwhisk-client-go/whisk', 'version':'d8ccb1442651beee6a9245913e3ca0cb182888b1', 'transitive':false])
build(['name':'github.com/apache/openwhisk-client-go/wski18n', 'version':'d8ccb1442651beee6a9245913e3ca0cb182888b1', 'transitive':false])
build(['name':'github.com/apache/openwhisk-client-go/whisk', 'version':'ee5b8709787cd37201c42e38040e9709f6d1e9c8', 'transitive':false])
build(['name':'github.com/apache/openwhisk-client-go/wski18n', 'version':'ee5b8709787cd37201c42e38040e9709f6d1e9c8', 'transitive':false])
build(['name':'github.com/pelletier/go-buffruneio', 'version':'df1e16fde7fc330a0ca68167c23bf7ed6ac31d6d', 'transitive':false])
build(['name':'github.com/pelletier/go-toml', 'version':'45932ad32dfdd20826f5671da37a5f3ce9f26a8d', 'transitive':false])
build(['name':'github.com/spf13/afero', 'version':'06b7e5f50606ecd49148a01a6008942d9b669217', 'transitive':false])
Expand Down
4 changes: 2 additions & 2 deletions deployers/manifestreader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ func TestManifestReader_SetDependencies(t *testing.T) {
err = manifestReader.HandleYaml(manifestParser, manifest, whisk.KeyValue{})
assert.Nil(t, err, fmt.Sprintf(TEST_ERROR_MANIFEST_PARSE_FAILURE, manifestFile))

expectedLocationHelloWorlds := "https://github.com/apache/incubator-openwhisk-test/packages/helloworlds"
expectedLocationHelloWhisk := "https://github.com/apache/incubator-openwhisk-test/packages/hellowhisk"
expectedLocationHelloWorlds := "https://github.com/apache/openwhisk-test/packages/helloworlds"
expectedLocationHelloWhisk := "https://github.com/apache/openwhisk-test/packages/hellowhisk"
expectedLocationUtils := "/whisk.system/utils"

for pkgName, pkg := range deployer.Deployment.Packages {
Expand Down
10 changes: 9 additions & 1 deletion deployers/servicedeployer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,15 @@ func (deployer *ServiceDeployer) createApi(api *whisk.ApiCreateRequest) error {
var response *http.Response

apiCreateReqOptions := deployer.Deployment.ApiOptions[apiPath]
apiCreateReqOptions.SpaceGuid = strings.Split(deployer.Client.Config.AuthToken, ":")[0]

if len(deployer.Client.Config.ApigwTenantId) > 0 {
// Use it to identify the IAM namespace
apiCreateReqOptions.SpaceGuid = deployer.Client.Config.ApigwTenantId
} else {
// assume a CF namespaces (SpaceGuid) which is part of the authtoken
apiCreateReqOptions.SpaceGuid = strings.Split(deployer.Client.Config.AuthToken, ":")[0]
}

apiCreateReqOptions.AccessToken = deployer.Client.Config.ApigwAccessToken

err = retry(DEFAULT_ATTEMPTS, DEFAULT_INTERVAL, func() error {
Expand Down
39 changes: 31 additions & 8 deletions deployers/whiskclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import (
const (
SOURCE_WSKPROPS = ".wskprops"
SOURCE_WHISK_PROPERTIES = "whisk.properties"
SOURCE_DEFAULT_VALUE = "wskdeploy default" // TODO() i18n?
SOURCE_DEFAULT_VALUE = "wskdeploy default"
)

var (
Expand All @@ -46,6 +46,7 @@ var (
key = PropertyValue{}
cert = PropertyValue{}
apigwAccessToken = PropertyValue{}
apigwTenantId = PropertyValue{}
additionalHeaders = make(http.Header)
)

Expand All @@ -70,6 +71,7 @@ var GetWskPropFromWhiskProperty = func(pi whisk.Properties) (*whisk.Wskprops, er
return whisk.GetWskPropFromWhiskProperty(pi)
}

// TODO implement a command line flag for APIGW_TENANT_ID
var GetCommandLineFlags = func() (string, string, string, string, string, string) {
return utils.Flags.ApiHost, utils.Flags.Auth, utils.Flags.Namespace, utils.Flags.Key, utils.Flags.Cert, utils.Flags.ApigwAccessToken
}
Expand All @@ -92,6 +94,7 @@ func resetWhiskConfig() {
key = PropertyValue{}
cert = PropertyValue{}
apigwAccessToken = PropertyValue{}
apigwTenantId = PropertyValue{}
}

func readFromCLI() {
Expand All @@ -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.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

//apigwTenantId = GetPropertyValue(apigwTenantId, tenantId, wski18n.COMMAND_LINE)
}

func setWhiskConfig(cred string, ns string, host string, token string, source string) {
credential = GetPropertyValue(credential, cred, source)
namespace = GetPropertyValue(namespace, ns, source)
apiHost = GetPropertyValue(apiHost, host, source)
apigwAccessToken = GetPropertyValue(apigwAccessToken, token, source)
// TODO decide if we should allow APIGW_TENANT_ID in manifest
}

func readFromDeploymentFile(deploymentPath string) {
Expand All @@ -129,6 +135,7 @@ func readFromManifestFile(manifestPath string) {
mm := parsers.NewYAMLParser()
manifest, _ := mm.ParseManifest(manifestPath)
p := manifest.GetProject()
// TODO look to deprecate reading Namespace, APIGW values from manifest or depl. YAML files
setWhiskConfig(p.Credential, p.Namespace, p.ApiHost, p.ApigwAccessToken, path.Base(manifestPath))
}
}
Expand All @@ -143,6 +150,7 @@ func readFromWskprops(pi whisk.PropertiesImp, proppath string) {
key = GetPropertyValue(key, wskprops.Key, SOURCE_WSKPROPS)
cert = GetPropertyValue(cert, wskprops.Cert, SOURCE_WSKPROPS)
apigwAccessToken = GetPropertyValue(apigwAccessToken, wskprops.AuthAPIGWKey, SOURCE_WSKPROPS)
apigwTenantId = GetPropertyValue(apigwTenantId, wskprops.APIGWTenantId, SOURCE_WSKPROPS)
}

func readFromWhiskProperty(pi whisk.PropertiesImp) {
Expand Down Expand Up @@ -174,6 +182,12 @@ func readFromWhiskProperty(pi whisk.PropertiesImp) {
map[string]interface{}{wski18n.KEY_KEY: wski18n.APIGW_ACCESS_TOKEN})
wskprint.PrintlnOpenWhiskWarning(warnMsg)
}
apigwTenantId = GetPropertyValue(apigwTenantId, whiskproperty.APIGWTenantId, SOURCE_WHISK_PROPERTIES)
if apigwTenantId.Source == SOURCE_WHISK_PROPERTIES {
warnMsg = wski18n.T(wski18n.ID_WARN_WHISK_PROPS_DEPRECATED,
map[string]interface{}{wski18n.KEY_KEY: wski18n.APIGW_TENANT_ID})
wskprint.PrintlnOpenWhiskWarning(warnMsg)
}
}

// we are reading openwhisk credentials (apihost, namespace, and auth) in the following precedence order:
Expand All @@ -198,7 +212,7 @@ func NewWhiskConfig(proppath string, deploymentPath string, manifestPath string)

// TODO() i18n
// Print all flags / values if verbose
wskprint.PrintlnOpenWhiskVerbose(utils.Flags.Verbose, wski18n.CONFIGURATION+":\n"+utils.Flags.Format())
//wskprint.PrintlnOpenWhiskVerbose(utils.Flags.Verbose, wski18n.CONFIGURATION+":\n"+utils.Flags.Format())

// now, read them from deployment file if not found on command line
readFromDeploymentFile(deploymentPath)
Expand Down Expand Up @@ -228,17 +242,22 @@ func NewWhiskConfig(proppath string, deploymentPath string, manifestPath string)
}

clientConfig = &whisk.Config{
AuthToken: credential.Value, //Authtoken
Namespace: namespace.Value, //Namespace
Host: apiHost.Value,
Version: "v1", // TODO() should not be hardcoded, should warn user of default
AuthToken: credential.Value, //Authtoken
Namespace: namespace.Value, //Namespace
Host: apiHost.Value,
Version: "v1", // TODO() should not be hardcoded, should warn user of default
//Version: Apiversion
Cert: cert.Value,
Key: key.Value,
Insecure: mode, // true if you want to ignore certificate signing
ApigwAccessToken: apigwAccessToken.Value,
ApigwTenantId: apigwTenantId.Value,
AdditionalHeaders: additionalHeaders,
}

// Print all flags / values if verbose
wskprint.PrintlnOpenWhiskVerbose(utils.Flags.Verbose, wski18n.CLI_FLAGS+":\n"+utils.Flags.Format())

// validate we have credential, apihost and namespace
err := validateClientConfig(credential, apiHost, namespace)
return clientConfig, err
Expand All @@ -258,13 +277,11 @@ func validateClientConfig(credential PropertyValue, apiHost PropertyValue, names
if len(apiHost.Value) == 0 {
errorMsg = wskderrors.AppendDetailToErrorMessage(
errorMsg, wski18n.T(wski18n.ID_MSG_CONFIG_MISSING_APIHOST), 1)

}

if len(namespace.Value) == 0 {
errorMsg = wskderrors.AppendDetailToErrorMessage(
errorMsg, wski18n.T(wski18n.ID_MSG_CONFIG_MISSING_NAMESPACE), 1)

}

if len(errorMsg) > 0 {
Expand All @@ -291,5 +308,11 @@ func validateClientConfig(credential PropertyValue, apiHost PropertyValue, names
wskprint.PrintOpenWhiskVerbose(utils.Flags.Verbose, stdout)
}

if len(apigwTenantId.Value) != 0 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok good to know.

stdout = wski18n.T(wski18n.ID_MSG_CONFIG_INFO_APIGW_TENANT_ID_X_source_X,
map[string]interface{}{wski18n.KEY_UUID: apigwTenantId, wski18n.KEY_SOURCE: apigwTenantId.Source})
wskprint.PrintOpenWhiskVerbose(utils.Flags.Verbose, stdout)
}

return nil
}
6 changes: 3 additions & 3 deletions docs/sync_projects_between_client_and_server.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ project:
Extension1:
dependencies:
helloworlds:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
```

After deploying this project with `wskdeploy sync -m manifest.yaml`, package `Extension2` has following annotation:
Expand Down Expand Up @@ -277,9 +277,9 @@ project:
Extension2:
dependencies:
helloworlds:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
custom-hellowhisk:
location: github.com/apache/incubator-openwhisk-test/packages/hellowhisk
location: github.com/apache/openwhisk-test/packages/hellowhisk
...
```

Expand Down
2 changes: 1 addition & 1 deletion docs/wskdeploy_interpolation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ project:
hello3:
function: ${OPENWHISK_FUNCTION_PYTHON}.py
hello4:
function: https://${OPENWHISK_FUNCTION_GITHUB_DIR}.js function: github.com/apache/incubator-openwhisk-test/packages/helloworlds
function: https://${OPENWHISK_FUNCTION_GITHUB_DIR}.js function: github.com/apache/openwhisk-test/packages/helloworlds
```
#### Trigger Feed
Expand Down
38 changes: 19 additions & 19 deletions parsers/manifest_parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,38 +190,38 @@ func testUnmarshalTemporaryFile(data []byte, filename string) (p *YAMLParser, m
func TestUnmarshalForHelloNodeJS(t *testing.T) {
testUnmarshalManifestPackageAndActionBasic(t,
"../tests/dat/manifest_hello_nodejs.yaml", // Manifest path
"helloworld", // Package name
1, // # of Actions
"helloNodejs", // Action name
"actions/hello.js", // Function path
"nodejs:6", // "Runtime
"") // "Main" function name
"helloworld", // Package name
1, // # of Actions
"helloNodejs", // Action name
"actions/hello.js", // Function path
"nodejs:6", // "Runtime
"") // "Main" function name
}

// Test 2: validate manifest_parser:Unmarshal() method with a sample manifest in Java
// validate that manifest_parser is able to read and parse the manifest data
func TestUnmarshalForHelloJava(t *testing.T) {
testUnmarshalManifestPackageAndActionBasic(t,
"../tests/dat/manifest_hello_java_jar.yaml", // Manifest path
"helloworld", // Package name
1, // # of Actions
"helloJava", // Action name
"actions/hello.jar", // Function path
"java", // "Runtime
"Hello") // "Main" function name
"helloworld", // Package name
1, // # of Actions
"helloJava", // Action name
"actions/hello.jar", // Function path
"java", // "Runtime
"Hello") // "Main" function name
}

// Test 3: validate manifest_parser:Unmarshal() method with a sample manifest in Python
// validate that manifest_parser is able to read and parse the manifest data
func TestUnmarshalForHelloPython(t *testing.T) {
testUnmarshalManifestPackageAndActionBasic(t,
"../tests/dat/manifest_hello_python.yaml", // Manifest path
"helloworld", // Package name
1, // # of Actions
"helloPython", // Action name
"actions/hello.py", // Function path
"python", // "Runtime
"") // "Main" function name
"helloworld", // Package name
1, // # of Actions
"helloPython", // Action name
"actions/hello.py", // Function path
"python", // "Runtime
"") // "Main" function name
}

// Test 4: validate manifest_parser:Unmarshal() method with a sample manifest in Swift
Expand Down Expand Up @@ -1910,7 +1910,7 @@ func TestParseYAML_param(t *testing.T) {
case "inline6":
assert.Equal(t, -531, param.Value, "Get param value failed.")
case "inline7":
assert.Equal(t, 432.432E-43, param.Value, "Get param value failed.")
assert.Equal(t, 432.432e-43, param.Value, "Get param value failed.")
case "inline8":
assert.Equal(t, "[ true, null, \"boo\", { \"key\": 0 }]", param.Value, "Get param value failed.")
case "inline9":
Expand Down
2 changes: 1 addition & 1 deletion parsers/yamlparser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Version string `yaml:"version"`
Packages map[string]Package `yaml:"packages"`
Inputs map[string]Parameter `yaml: parameters`
Expand Down
14 changes: 7 additions & 7 deletions tests/dat/manifest_validate_dependencies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,22 @@ packages:
helloworld1:
dependencies:
helloworlds:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
dependency1:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
dependency2:
location: github.com/apache/incubator-openwhisk-test/packages/hellowhisk
location: github.com/apache/openwhisk-test/packages/hellowhisk
dependency3:
location: /whisk.system/utils
helloworld2:
dependencies:
helloworlds:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
dependency1:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
dependency4:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
dependency5:
location: github.com/apache/incubator-openwhisk-test/packages/hellowhisk
location: github.com/apache/openwhisk-test/packages/hellowhisk
dependency6:
location: /whisk.system/utils
14 changes: 7 additions & 7 deletions tests/dat/manifest_validate_dependencies_bogus.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,23 @@ packages:
helloworld1:
dependencies:
helloworlds:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
dependency1:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
dependency2:
location: github.com/apache/incubator-openwhisk-test/packages/hellowhisk
location: github.com/apache/openwhisk-test/packages/hellowhisk
dependency3:
location: /whisk.system/utils
helloworld2:
dependencies:
helloworlds:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
dependency1:
location: github.com/apache/incubator-openwhisk-test/packages/hellowhisk
location: github.com/apache/openwhisk-test/packages/hellowhisk
dependency4:
location: github.com/apache/incubator-openwhisk-test/packages/helloworlds
location: github.com/apache/openwhisk-test/packages/helloworlds
dependency5:
location: github.com/apache/incubator-openwhisk-test/packages/hellowhisk
location: github.com/apache/openwhisk-test/packages/hellowhisk
dependency6:
location: /whisk.system/utils

Expand Down
Loading