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

feat: support aws secret manager #11417

Merged
merged 29 commits into from
Aug 28, 2024
Merged

Conversation

HuanXin-Chen
Copy link
Contributor

Description

Many enterprises are utilizing cloud services from AWS and GCP, relying on the secret manager provided by these platforms to handle sensitive information. Integrating Apache APISIX with these secret managers can streamline the process of using sensitive information within APISIX, enabling users to manage and utilize cloud-stored sensitive data more conveniently, thus enhancing the overall security and usability of the system.

This PR has completed the support for AWS. It added the aws.lua file to the original secret module, allowing users to store their secret information on AWS using the same reference method as before.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@HuanXin-Chen HuanXin-Chen changed the title feat:support aws secret manager feat: support aws secret manager Jul 18, 2024
@bzp2010 bzp2010 self-requested a review July 20, 2024 03:03
ci/common.sh Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
t/secret/aws.t Show resolved Hide resolved
@bzp2010 bzp2010 self-requested a review August 4, 2024 11:54
apisix/secret/aws.lua Outdated Show resolved Hide resolved
apisix/secret/aws.lua Outdated Show resolved Hide resolved
@bzp2010 bzp2010 requested a review from nic-6443 August 23, 2024 06:10
nic-6443
nic-6443 previously approved these changes Aug 25, 2024
bzp2010
bzp2010 previously approved these changes Aug 25, 2024
moonming
moonming previously approved these changes Aug 27, 2024
membphis
membphis previously approved these changes Aug 27, 2024
Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

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

LGTM, except minor code style issues

t/secret/aws.t Outdated

local secret = require("apisix.secret")
local value = secret.fetch_by_uri("$secret://aws/mysecret/jack/key")
ngx.say(value)
Copy link
Member

@membphis membphis Aug 27, 2024

Choose a reason for hiding this comment

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

Just confirm, it will print a nil value in this test case?
And it will be considered as expected output, it make me little confused

I prefer this way:

if value then
    ngx.say("secret value: ", value)
end

ngx.say("all done")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

return data[sub_key]
end

_M.get = get
Copy link
Member

Choose a reason for hiding this comment

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

if we need to reuse the function get, the current style is good.

In this code, we do not need to reuse function get, so _M.get is better to read and understand

Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

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

LGTM

@bzp2010 bzp2010 merged commit 9c81c93 into apache:master Aug 28, 2024
36 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants