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

fix(vault): fix several issues in vault and refactor the vault code base #11402

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

bungle
Copy link
Member

@bungle bungle commented Aug 14, 2023

Summary

  • Make DAOs to fallback to empty string when resolving Vault references fail
  • Use node level mutex when rotation references
  • Refresh references on config changes
  • Update plugin referenced values only once per request
  • Pass only the valid config options to vault implementations
  • Resolve multi-value secrets only once when rotating them
  • Do not start vault secrets rotation timer on control planes
  • Re-enable negative caching
  • Reimplement the kong.vault.try function
  • Remove references from rotation in case their configuration has changed

Commits before squashing them (it turned out difficult to split last refactoring commit)

  • tests(vault): should be able to detect new references when plugin config changes
  • fix(schema): process auto fields to default to empty string on resolve failures
  • fix(schema): use pairs to adjust_field_for_context with arrays and sets
  • perf(plugin-iterator): vault.update only once per request on global iterator
  • docs(vault): mark local functions with local-attribute
  • fix(vault): use node level mutex instead of a thread level semaphore
  • fix(vault): add validation back as the yielding is fixed
  • refactor(vault): move parse_reference close to is_reference
  • fix(vault): refresh secrets on flush in timer
  • refactor(vault): refactor the vault local update function
  • chore(vault): add cooperative yielding on secret rotation
  • fix(vault): no event handlers or rotation timer on control planes
  • refactor(vault): cache key generation and parsing
  • chore(vault): rename get_subfield to extract_key_from_json_string
  • chore(vault): rename flush_and_refresh to handle_vault_crud_event
  • refactor(vault): refactor vault code base

@bungle bungle force-pushed the fix/vaults-cleanup-pass branch from 54b9625 to 719be65 Compare August 17, 2023 15:40
@bungle bungle force-pushed the fix/vaults-cleanup-pass branch 6 times, most recently from 92d6bf5 to fb255df Compare August 17, 2023 20:12
@bungle bungle changed the title fix(vaults): cleanup pass with several fixes and cleanups fix(vaults): cleanup pass with several fixes Aug 17, 2023
@bungle bungle force-pushed the fix/vaults-cleanup-pass branch 10 times, most recently from 4bf867d to 8821c30 Compare August 24, 2023 15:33
@vm-001 vm-001 force-pushed the fix/vaults-cleanup-pass branch from 1b40601 to 0d3f629 Compare August 29, 2023 10:21
@bungle bungle force-pushed the fix/vaults-cleanup-pass branch 5 times, most recently from 027b146 to 30893f6 Compare August 29, 2023 15:49
@bungle bungle force-pushed the fix/vaults-cleanup-pass branch 6 times, most recently from 0a54f31 to 58fb69a Compare August 30, 2023 14:38
@bungle bungle marked this pull request as ready for review August 30, 2023 15:11
@bungle
Copy link
Member Author

bungle commented Aug 30, 2023

TODO: add changelog.

@hanshuebner hanshuebner self-requested a review August 31, 2023 12:55
Copy link
Contributor

@hanshuebner hanshuebner left a comment

Choose a reason for hiding this comment

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

To me, the code reads much easier now. Two small comments remain from my end, otherwise this is good to go from my end. Thank you!

kong/pdk/vault.lua Outdated Show resolved Hide resolved
kong/pdk/vault.lua Outdated Show resolved Hide resolved
@hanshuebner
Copy link
Contributor

A changelog file must be added, and I think it would be good to have a little more information in the pull request description. I believe that squashing the commits together is best at this point.

@bungle bungle force-pushed the fix/vaults-cleanup-pass branch from 58fb69a to 43ad19f Compare September 7, 2023 16:00
@bungle bungle changed the title fix(vaults): cleanup pass with several fixes fix(vault): fix several issues in vault and refactor the vault code base Sep 7, 2023
@bungle bungle force-pushed the fix/vaults-cleanup-pass branch from 43ad19f to 5d451d9 Compare September 7, 2023 16:11
### Summary

- Make DAOs to fallback to empty string when resolving Vault references fail
- Use node level mutex when rotation references
- Refresh references on config changes
- Update plugin referenced values only once per request
- Pass only the valid config options to vault implementations
- Resolve multi-value secrets only once when rotating them
- Do not start vault secrets rotation timer on control planes
- Re-enable negative caching
- Reimplement the kong.vault.try function
- Remove references from rotation in case their configuration has changed

#### Commits before squashing them (it turned out difficult to split last refactoring commit)

- tests(vault): should be able to detect new references when plugin config changes
- fix(schema): process auto fields to default to empty string on resolve failures
- fix(schema): use pairs to adjust_field_for_context with arrays and sets
- perf(plugin-iterator): vault.update only once per request on global iterator
- docs(vault): mark local functions with local-attribute
- fix(vault): use node level mutex instead of a thread level semaphore
- fix(vault): add validation back as the yielding is fixed
- refactor(vault): move parse_reference close to is_reference
- fix(vault): refresh secrets on flush in timer
- refactor(vault): refactor the vault local update function
- chore(vault): add cooperative yielding on secret rotation
- fix(vault): no event handlers or rotation timer on control planes
- refactor(vault): cache key generation and parsing
- chore(vault): rename get_subfield to extract_key_from_json_string
- chore(vault): rename flush_and_refresh to handle_vault_crud_event
- refactor(vault): refactor vault code base

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
@bungle bungle force-pushed the fix/vaults-cleanup-pass branch from 5d451d9 to d8aee9f Compare September 7, 2023 16:14
@hanshuebner hanshuebner merged commit fd64029 into master Sep 7, 2023
@hanshuebner hanshuebner deleted the fix/vaults-cleanup-pass branch September 7, 2023 17:11
team-gateway-bot pushed a commit that referenced this pull request Sep 7, 2023
…ase (#11402)

### Summary

- Make DAOs to fallback to empty string when resolving Vault references fail
- Use node level mutex when rotation references
- Refresh references on config changes
- Update plugin referenced values only once per request
- Pass only the valid config options to vault implementations
- Resolve multi-value secrets only once when rotating them
- Do not start vault secrets rotation timer on control planes
- Re-enable negative caching
- Reimplement the kong.vault.try function
- Remove references from rotation in case their configuration has changed

#### Commits before squashing them (it turned out difficult to split last refactoring commit)

- tests(vault): should be able to detect new references when plugin config changes
- fix(schema): process auto fields to default to empty string on resolve failures
- fix(schema): use pairs to adjust_field_for_context with arrays and sets
- perf(plugin-iterator): vault.update only once per request on global iterator
- docs(vault): mark local functions with local-attribute
- fix(vault): use node level mutex instead of a thread level semaphore
- fix(vault): add validation back as the yielding is fixed
- refactor(vault): move parse_reference close to is_reference
- fix(vault): refresh secrets on flush in timer
- refactor(vault): refactor the vault local update function
- chore(vault): add cooperative yielding on secret rotation
- fix(vault): no event handlers or rotation timer on control planes
- refactor(vault): cache key generation and parsing
- chore(vault): rename get_subfield to extract_key_from_json_string
- chore(vault): rename flush_and_refresh to handle_vault_crud_event
- refactor(vault): refactor vault code base

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
(cherry picked from commit fd64029)
hanshuebner pushed a commit that referenced this pull request Sep 20, 2023
…ase (#11402) (#11521)

### Summary

- Make DAOs to fallback to empty string when resolving Vault references fail
- Use node level mutex when rotation references
- Refresh references on config changes
- Update plugin referenced values only once per request
- Pass only the valid config options to vault implementations
- Resolve multi-value secrets only once when rotating them
- Do not start vault secrets rotation timer on control planes
- Re-enable negative caching
- Reimplement the kong.vault.try function
- Remove references from rotation in case their configuration has changed

#### Commits before squashing them (it turned out difficult to split last refactoring commit)

- tests(vault): should be able to detect new references when plugin config changes
- fix(schema): process auto fields to default to empty string on resolve failures
- fix(schema): use pairs to adjust_field_for_context with arrays and sets
- perf(plugin-iterator): vault.update only once per request on global iterator
- docs(vault): mark local functions with local-attribute
- fix(vault): use node level mutex instead of a thread level semaphore
- fix(vault): add validation back as the yielding is fixed
- refactor(vault): move parse_reference close to is_reference
- fix(vault): refresh secrets on flush in timer
- refactor(vault): refactor the vault local update function
- chore(vault): add cooperative yielding on secret rotation
- fix(vault): no event handlers or rotation timer on control planes
- refactor(vault): cache key generation and parsing
- chore(vault): rename get_subfield to extract_key_from_json_string
- chore(vault): rename flush_and_refresh to handle_vault_crud_event
- refactor(vault): refactor vault code base

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
(cherry picked from commit fd64029)

Co-authored-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
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