cloudstack
cloudstack copied to clipboard
Increase size of column 'value' at table 'account_details'
Description
This PR increases the column value at table account_details from 255 chars to 4096, matching with the value allowed in the API command for updating the configuration of accounts.
When the value length is bigger than 255, the following log is presented right after the updateConfiguration API call:
2022-03-09 17:50:24,627 ERROR [c.c.a.ApiServer] (qtp30578394-234766:ctx-cad18b45 ctx-32e954dd) (logid:0948e203) unhandled exception executing api command: [Ljava.lang.String;@117c6ba7
com.cloud.utils.exception.CloudRuntimeException: DB Exception on: com.mysql.cj.jdbc.ClientPreparedStatement: INSERT INTO account_details (account_details.account_id, account_details.name, account_details.value) VALUES (123, _binary'api.allowed.source.cidr.list', _binary'<huge binary>')
at com.cloud.utils.db.GenericDaoBase.persist(GenericDaoBase.java:1450)
at jdk.internal.reflect.GeneratedMethodAccessor168.invoke(Unknown Source)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
....
....
....
Caused by: com.mysql.cj.jdbc.exceptions.MysqlDataTruncation: Data truncation: Data too long for column 'value' at row 1
at com.mysql.cj.jdbc.exceptions.SQLExceptionsMapping.translateException(SQLExceptionsMapping.java:104)
at com.mysql.cj.jdbc.ClientPreparedStatement.executeInternal(ClientPreparedStatement.java:953)
at com.mysql.cj.jdbc.ClientPreparedStatement.executeUpdateInternal(ClientPreparedStatement.java:1092)
... 83 more
Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [X] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [X] Minor
- [ ] Trivial
Screenshots (if appropriate):
How Has This Been Tested?
- Update configurations for an account
- Assert that configuration is properly updated (which was not the case before the change)
@blueorangutan package
@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2817
@blueorangutan test
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
Trillian test result (tid-3552) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 31419 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6080-t3552-kvm-centos7.zip Smoke tests completed. 92 look OK, 0 have errors Only failed tests results shown below:
| Test | Result | Time (s) | Test File |
|---|
Before merging, I am afraid that we might need to also increase the size of the value in the response object, otherwise, the API response will not contain the whole value stored in DB. Placing it as a draft in the meantime to avoid it getting merged.
Thanks @GabrielBrascher I was spinning an env for testing it - will post my review after that
Thanks, @nvazquez! I will do some changes and run some manual tests.
@GabrielBrascher I've tested through the API and UI:
- API works as expected, passed
valueparameters with length > 255 and the response was properly retrieved. - UI seems to still be truncating the value at 255 characters, can you check if this can be extended?
Example: input length = 650 characters:
(localcloud) 🐱 > update configuration accountid=deb22344-0869-49a2-ac1f-116cb858ee03 name='api.allowed.source.cidr.list' value=12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
{
"configuration": {
"category": "Advanced",
"description": "Comma separated list of IPv4/IPv6 CIDRs from which API calls can be performed. Can be set on Global and Account levels.",
"isdynamic": true,
"name": "api.allowed.source.cidr.list",
"scope": "account",
"value": "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"
}
}
@nvazquez I am seeing the following:
- Update command and response looks OK:
(poc) 🐱 > update configuration accountid=064a02be-caf8-46c7-8264-92c5a9d782b2 name=api.allowed.source.cidr.list value=12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
{
"configuration": {
"category": "Advanced",
"description": "Comma separated list of IPv4/IPv6 CIDRs from which API calls can be performed. Can be set on Global and Account levels.",
"isdynamic": true,
"name": "api.allowed.source.cidr.list",
"scope": "account",
"value": "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"
}
}
- But list command returns a fraction:
(poc) 🐱 > list configurations name=api.allowed.source.cidr.list accountid=064a02be-caf8-46c7-8264-92c5a9d782b2 pagesize=600 page=1
{
"configuration": [
{
"category": "Advanced",
"description": "Comma separated list of IPv4/IPv6 CIDRs from which API calls can be performed. Can be set on Global and Account levels.",
"isdynamic": true,
"name": "api.allowed.source.cidr.list",
"scope": "account",
"value": "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345"
}
],
"count": 1
}
Updated value with length of 651
"value": "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"
listed value with length of 256
"value": "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345"
Yes @GabrielBrascher seeing the same - the only remark from your comment is the lengths are 650 and 255. Extending the response field should solve it, worth checking if the UI does truncate or simply displays the received data for configurations
Hi @GabrielBrascher please advise when this one is ready for testing again
@GabrielBrascher can you address the comments and fix the issue in your testing ?
Hi @${author}, your pull request has merge conflicts. Can you fix the conflicts and sync your branch with the base branch?
Sorry for the silence on my part here @weizhouapache @nvazquez. I will try to have it sorted this week, but I am afraid that I will need to keep it on hold/Draft due to more pressing demands on another task.
Found UI changes, kicking a new UI QA build @blueorangutan ui
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
UI build: :heavy_multiplication_x: (SL-JID-2085)
@blueorangutan package
@shwstppr a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 3921
@blueorangutan package
@shwstppr a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
@BartJM I asked you to move the schema change to 4.17.1 upgrade path as milestone is set to 4.17.1 but the PR is targeted for main, do you want it to go only into main? If you want it in 4.17.1 can you please rebase and target it to 4.17 branch. Else you will need to again move schema changes into 41710to41800 upgrade path. Sorry for not mentioning it earlier. cc @GabrielBrascher
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3942
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
Found UI changes, kicking a new UI QA build @blueorangutan ui
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
UI build: :heavy_check_mark: Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6080 (SL-JID-2113)