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.

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

add integration test CI dimension for MariaDB #1046

Merged
merged 6 commits into from
Jun 23, 2023

Conversation

samuel40791765
Copy link
Contributor

Issues:

No Issue cut

Description of changes:

We've recently resolved missing symbols for MariaDB and are looking to add a CI dimension to ensure we don't break the build. Patches and flags necessary were taken from internal work on MariaDB.

Call-outs:

N/A

Testing:

New CI dimension

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@samuel40791765 samuel40791765 marked this pull request as ready for review June 6, 2023 20:36
@samuel40791765 samuel40791765 requested a review from andrewhop June 7, 2023 19:03
@samuel40791765 samuel40791765 force-pushed the mariadb-ci branch 11 times, most recently from 2261da8 to f9728e8 Compare June 9, 2023 17:24
@justsmth justsmth self-requested a review June 13, 2023 17:43
Comment on lines +65 to +72
- ivec);
- DES_set_key_unchecked(&keyblock.key1,&(des_keyschedule[(int)offset].ks1));
- DES_set_key_unchecked(&keyblock.key2,&(des_keyschedule[(int)offset].ks2));
- DES_set_key_unchecked(&keyblock.key3,&(des_keyschedule[(int)offset].ks3));
+ ivec.bytes);
+ DES_set_key(&keyblock.key1,&(des_keyschedule[(int)offset].ks1));
+ DES_set_key(&keyblock.key2,&(des_keyschedule[(int)offset].ks2));
+ DES_set_key(&keyblock.key3,&(des_keyschedule[(int)offset].ks3));
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you consider adding a simple function that maps DES_set_key_unchecked -> DES_set_key? This isn't the only project to hit this issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took this patch directly from the work ottoke@ had been doing. But I can add it in a subsequent PR and remove this patch once we do so

Comment on lines +7 to +8
Disable the WolfSSL specific bug fix as it breaks AWS-LC builds with
errors like:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the AWS-LC build enabling WolfSSL bug fixes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't have a good answer to what MariaDB/WolfSSL was doing under the hood here :( If it helps, I looked into the files and it seems like the WolfSSL bug fix was being performed, regardless of what backend crypto was being used.

tests/ci/integration/run_mariadb_integration.sh Outdated Show resolved Hide resolved
tests/ci/integration/run_mariadb_integration.sh Outdated Show resolved Hide resolved
@samuel40791765 samuel40791765 force-pushed the mariadb-ci branch 2 times, most recently from 7b0cfb8 to 728fa17 Compare June 21, 2023 19:14
justsmth
justsmth previously approved these changes Jun 22, 2023
andrewhop
andrewhop previously approved these changes Jun 23, 2023
@samuel40791765 samuel40791765 merged commit 341a240 into aws:main Jun 23, 2023
@samuel40791765 samuel40791765 deleted the mariadb-ci branch June 23, 2023 18:56
@skmcgrail skmcgrail mentioned this pull request Jul 7, 2023
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