Skip to content

feat: remove libexpat dependency #1

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

Merged
merged 12 commits into from
Aug 9, 2024
Merged

feat: remove libexpat dependency #1

merged 12 commits into from
Aug 9, 2024

Conversation

bzp2010
Copy link
Collaborator

@bzp2010 bzp2010 commented Aug 9, 2024

Some Lua files are using the XML-based AWS STS API, and they require bindings based on the libexpat c library to parse the XML, which requires that the expat library be installed for OpenResty; this library has a specific requirement for a version of the expat codebase, which is not available in the repositories of the major Linux distributions, and can only be installed from source. This creates a barrier to building and distributing the software.

However that the AWS STS API doesn't have to use XML as a response format, it also supports JSON, allowing us to parse it directly in cjson.

This PR makes the change to remove the expat dependency and change the two Credentials methods that currently rely on XML to JSON. This will modify the request headers on all service API calls, but is not expected to have an impact since those APIs originally used JSON.

Copy link

github-actions bot commented Aug 9, 2024

Luacheck Report

5 tests   5 ✅  0s ⏱️
1 suites  0 💤
1 files    0 ❌

Results for commit 3b8fc8f.

♻️ This comment has been updated with latest results.

@bzp2010 bzp2010 merged commit 1138d1c into main Aug 9, 2024
9 checks passed
@bzp2010 bzp2010 deleted the bzp/feat-remove-xml branch August 9, 2024 03:59
@@ -314,6 +312,7 @@ describe("operations protocol", function()

assert.same({
headers = {
["Accept"] = 'application/json',

Choose a reason for hiding this comment

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

Clever and this would be much better!

Copy link

@juzhiyuan juzhiyuan left a comment

Choose a reason for hiding this comment

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

Good changes!

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.

2 participants