Skip to content

Conversation

@mlsorensen
Copy link
Contributor

Similar to #49 - however we push the customizations to value encoding into EncodeValues function, and add tests for this function.

Adding Ginkgo/Gomega for table support in test suites, this adds a lot of dependencies but also provides for a richer test framework. We can omit this and I can refactor the tests to be simpler if necessary.

@mlsorensen
Copy link
Contributor Author

Tested manually as well, first ensured spaces still get encoded to %20 instead of + by setting custom.cs.identifier to my cloudstack

	params := client.Configuration.NewUpdateConfigurationParams("custom.cs.identifier")
	params.SetValue("my cloudstack")
	response, err := client.Configuration.UpdateConfiguration(params)

API Log:

GET apiKey=xxxx&command=updateConfiguration&name=custom.cs.identifier&response=json&value=my%20cloudstack&signature=xxxx 200 {"updateconfigurationresponse":{"configuration":{"category":"Advanced","name":"custom.cs.identifier","value":"my cloudstack","description":"Custom identifier for the cloudstack installation","isdynamic":true}}}

Tested using asterisk in consoleproxy.url.domain by setting to *.turboio.com:

	params := client.Configuration.NewUpdateConfigurationParams("consoleproxy.url.domain")
	params.SetValue("*.turboio.com")
	response, err := client.Configuration.UpdateConfiguration(params)

API Log:

GET apiKey=xxxx&command=updateConfiguration&name=consoleproxy.url.domain&response=json&value=*.turboio.com&signature=xxxx 200 {"updateconfigurationresponse":{"configuration":{"category":"Console Proxy","name":"consoleproxy.url.domain","value":"*.turboio.com","description":"Console proxy url domain","isdynamic":false}}}

@mlsorensen
Copy link
Contributor Author

While two thumbs up is good enough for Siskel and Ebert, I'll give some more time for committers to review/reply before merging.

Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

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

LGTM based on code changes and test results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants