Skip to content

fix(vault): allow arrays in conf loader to be referenced #12672

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

Closed
wants to merge 1 commit into from

Conversation

bungle
Copy link
Member

@bungle bungle commented Feb 29, 2024

Summary

Some properties, like KONG_SSL_CERT and KONG_SSL_CERT_KEY are arrays and users can specify many. Vaults didn't work in this scenario:

For example below didn't work before:

CERT_1=$(<cert1.crt) \
KEY_1=$(<key1.key) \
CERT_2=$(<cert2.crt) \
KEY_2=$(<key2.key) \
KONG_SSL_CERT_KEY="{vault://env/key-1},{vault://env/key-2}" \
KONG_SSL_CERT="{vault://env/cert-1},{vault://env/cert-2}" \
kong prepare --vv

There were also erroneous warning in logs like because of bad array handling:

[warn] 680#0: [kong] vault.lua:1475 error caching secret reference {vault://env/cert-1}: bad value type

This fixes those.

The other commit is just a small refactor on schema:

  • move some repetitive vault resolving code to local functions

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issues Resolved

KAG-3869
FTI-6163

@bungle bungle force-pushed the fix/ssl-cert-vault branch from 2db7703 to 78faecc Compare March 1, 2024 00:27
@bungle bungle force-pushed the fix/ssl-cert-vault branch from 78faecc to 004f175 Compare March 1, 2024 02:05
@bungle bungle force-pushed the fix/ssl-cert-vault branch from fff37df to b1ab67e Compare April 22, 2024 16:07
@github-actions github-actions bot added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Apr 22, 2024
@bungle bungle force-pushed the fix/ssl-cert-vault branch 5 times, most recently from 21a9e28 to 5bfd1e7 Compare April 29, 2024 19:01
@bungle bungle force-pushed the fix/ssl-cert-vault branch from 5bfd1e7 to ae30a71 Compare July 5, 2024 09:40
### Summary

Some properties, like `KONG_SSL_CERT` and `KONG_SSL_CERT_KEY` are
arrays and users can specify many. Vaults didn't work in this scenario:

For example below didn't work before:
```
CERT_1=$(<cert1.crt) \
KEY_1=$(<key1.key) \
CERT_2=$(<cert2.crt) \
KEY_2=$(<key2.key) \
KONG_SSL_CERT_KEY="{vault://env/key-1},{vault://env/key-2}" \
KONG_SSL_CERT="{vault://env/cert-1},{vault://env/cert-2}" \
kong prepare --vv
```

There were also erroneous warning in logs like because of bad array handling:

```
[warn] 680#0: [kong] vault.lua:1475 error caching secret reference {vault://env/cert-1}: bad value type
```

This fixes those.

Signed-off-by: Aapo Talvensaari <[email protected]>
@bungle bungle force-pushed the fix/ssl-cert-vault branch from ae30a71 to bb3190b Compare July 5, 2024 10:22
return nil, err
end

-- TODO: remember!
Copy link
Member

Choose a reason for hiding this comment

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

@bungle Do you still remember what is the exact todo item here? I'm working to finish this PR

windmgc added a commit that referenced this pull request Nov 29, 2024
…bsystem (#13855)

* fix(vault): allow arrays in conf loader to be referenced

### Summary

The commit includes the following fix for vault:

Fix the issue that array-type configuration's element cannot be referenced. conf_loader now supports iterate through array-like configuration field and deref the secrets one by one.

Enable the vault to dereference secrets using partial SSL configurations, for example, lua_ssl_trusted_certificate=system, {vault://abc/def}. This is implemented especially for resty environments that execute the actual kong command, with the opts pre_cmd = true. Vault related functionalities can work normally by using valid partial configuration generated in the pre_cmd environment. This change does not have a separate changelog entry because it is part of the previous "array-type config vault ref" fix and is more inline with user's intuition.

Fix the issue that vault reference cannot work well when multiple subsystems(http/stream) are enabled. The kong_processed_secrets environment variable/file are now generated by subsystems, so enabling multiple subsystem will generates multiple env var/secret file for storing vault deref result.

* fix(vault): allow arrays in conf loader to be referenced (#12672)

* fix(*): special treatment for pre_cmd

* fix(cmd): fix vault refs when both http and stream are enabled

* tests(*): fix prepare cmd test

* tests(*): add vault related cmd tests

* docs(changelog): add changelog

* test(*): try to fix test

* fix(*): unsetenv unconditionally clean http/stream env

---------

Signed-off-by: Aapo Talvensaari <[email protected]>
Co-authored-by: Aapo Talvensaari <[email protected]>
github-actions bot pushed a commit that referenced this pull request Nov 29, 2024
…bsystem (#13855)

* fix(vault): allow arrays in conf loader to be referenced

### Summary

The commit includes the following fix for vault:

Fix the issue that array-type configuration's element cannot be referenced. conf_loader now supports iterate through array-like configuration field and deref the secrets one by one.

Enable the vault to dereference secrets using partial SSL configurations, for example, lua_ssl_trusted_certificate=system, {vault://abc/def}. This is implemented especially for resty environments that execute the actual kong command, with the opts pre_cmd = true. Vault related functionalities can work normally by using valid partial configuration generated in the pre_cmd environment. This change does not have a separate changelog entry because it is part of the previous "array-type config vault ref" fix and is more inline with user's intuition.

Fix the issue that vault reference cannot work well when multiple subsystems(http/stream) are enabled. The kong_processed_secrets environment variable/file are now generated by subsystems, so enabling multiple subsystem will generates multiple env var/secret file for storing vault deref result.

* fix(vault): allow arrays in conf loader to be referenced (#12672)

* fix(*): special treatment for pre_cmd

* fix(cmd): fix vault refs when both http and stream are enabled

* tests(*): fix prepare cmd test

* tests(*): add vault related cmd tests

* docs(changelog): add changelog

* test(*): try to fix test

* fix(*): unsetenv unconditionally clean http/stream env

---------

Signed-off-by: Aapo Talvensaari <[email protected]>
Co-authored-by: Aapo Talvensaari <[email protected]>
(cherry picked from commit bc0acea)
windmgc added a commit that referenced this pull request Nov 29, 2024
…bsystem (#13855)

* fix(vault): allow arrays in conf loader to be referenced

### Summary

The commit includes the following fix for vault:

Fix the issue that array-type configuration's element cannot be referenced. conf_loader now supports iterate through array-like configuration field and deref the secrets one by one.

Enable the vault to dereference secrets using partial SSL configurations, for example, lua_ssl_trusted_certificate=system, {vault://abc/def}. This is implemented especially for resty environments that execute the actual kong command, with the opts pre_cmd = true. Vault related functionalities can work normally by using valid partial configuration generated in the pre_cmd environment. This change does not have a separate changelog entry because it is part of the previous "array-type config vault ref" fix and is more inline with user's intuition.

Fix the issue that vault reference cannot work well when multiple subsystems(http/stream) are enabled. The kong_processed_secrets environment variable/file are now generated by subsystems, so enabling multiple subsystem will generates multiple env var/secret file for storing vault deref result.

* fix(vault): allow arrays in conf loader to be referenced (#12672)

* fix(*): special treatment for pre_cmd

* fix(cmd): fix vault refs when both http and stream are enabled

* tests(*): fix prepare cmd test

* tests(*): add vault related cmd tests

* docs(changelog): add changelog

* test(*): try to fix test

* fix(*): unsetenv unconditionally clean http/stream env

---------

Signed-off-by: Aapo Talvensaari <[email protected]>
Co-authored-by: Aapo Talvensaari <[email protected]>
(cherry picked from commit bc0acea)
@bungle bungle closed this Feb 4, 2025
@bungle bungle deleted the fix/ssl-cert-vault branch February 4, 2025 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants