Skip to content

feat: support aws secret manager#11417

Merged
bzp2010 merged 29 commits into
apache:masterfrom
HuanXin-Chen:feat-aws-secret
Aug 28, 2024
Merged

feat: support aws secret manager#11417
bzp2010 merged 29 commits into
apache:masterfrom
HuanXin-Chen:feat-aws-secret

Conversation

@HuanXin-Chen

Copy link
Copy Markdown
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
Comment thread ci/common.sh Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread apisix/secret/aws.lua Outdated
Comment thread t/secret/aws.t
@bzp2010
bzp2010 self-requested a review August 4, 2024 11:54
Comment thread apisix/secret/aws.lua Outdated
@HuanXin-Chen
HuanXin-Chen dismissed stale reviews from membphis, bzp2010, and moonming via cb26bb8 August 25, 2024 03:45
@HuanXin-Chen
HuanXin-Chen requested a review from bzp2010 August 25, 2024 03:47
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

@membphis membphis left a comment

Copy link
Copy Markdown
Member

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

Comment thread t/secret/aws.t Outdated

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

@membphis membphis Aug 27, 2024

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment thread apisix/secret/aws.lua Outdated
return data[sub_key]
end

_M.get = get

Copy link
Copy Markdown
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

@HuanXin-Chen
HuanXin-Chen dismissed stale reviews from membphis, moonming, bzp2010, and nic-6443 via b9d317c August 27, 2024 10:01
@HuanXin-Chen
HuanXin-Chen requested a review from membphis August 27, 2024 10:03
@bzp2010
bzp2010 requested review from moonming and nic-6443 August 27, 2024 12:42

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@kayx23

kayx23 commented Oct 12, 2024

Copy link
Copy Markdown
Member

Hi, following the doc to configure this plugin (configured the region, access_key_id, and secret_access_key), I am running into failed to retrtive data from aws secret manager unable to get local issuer certificate error:

[lua] secret.lua: fetch(): failed to fetch secret value: failed to retrtive data from aws secret manager: SecretsManager:getSecretValue() failed to connect to 'https://secretsmanager.ap-southeast-2.amazonaws.com:443': 20: unable to get local issuer certificate, client: 192.168.65.1, server: _, request: "GET /anything HTTP/1.1", host: "127.0.0.1:9080"

Any suggestion?

@HuanXin-Chen

Copy link
Copy Markdown
Contributor Author

Hi, following the doc to configure this plugin (configured the region, access_key_id, and secret_access_key), I am running into failed to retrtive data from aws secret manager unable to get local issuer certificate error:

[lua] secret.lua: fetch(): failed to fetch secret value: failed to retrtive data from aws secret manager: SecretsManager:getSecretValue() failed to connect to 'https://secretsmanager.ap-southeast-2.amazonaws.com:443': 20: unable to get local issuer certificate, client: 192.168.65.1, server: _, request: "GET /anything HTTP/1.1", host: "127.0.0.1:9080"

Any suggestion?

You need to install the CA certificate using the command: sudo apt-get install ca-certificates.

And then, make sure that APISIX has configured it with the following setting: ssl_trusted_certificate: /etc/ssl/certs/ca-certificates.crt.

@kayx23

kayx23 commented Oct 12, 2024

Copy link
Copy Markdown
Member

When I used Vault previously I didn't remember having to go through this setup (but again the vault instance is running locally in Docker). What attributed to this differences? 🤔

@HuanXin-Chen

Copy link
Copy Markdown
Contributor Author

When I used Vault previously I didn't remember having to go through this setup (but again the vault instance is running locally in Docker). What attributed to this differences? 🤔

This is because HTTPS is different from HTTP.

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