support Openbao as an encryption provider #5048
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR adds support for OpenBao as a state encryption provider by removing the custom encryption provider abstraction layer and simplifying configuration validation. It adds Docker and testcontainers support for integration tests, introduces an OpenBao integration test, and updates documentation to include OpenBao as a supported encryption provider option. Changes
Sequence DiagramsequenceDiagram
participant Config as Config<br/>Validation
participant OldProvider as Old: Provider<br/>Instantiation
participant OldUnmarshal as Old: UnmarshalConfig
participant OldToMap as Old: ToMap
participant NewDirect as New: Direct<br/>Validation
participant CodeGen as Code<br/>Generator
Note over Config,CodeGen: Previous Flow (Removed)
Config->>OldProvider: NewRemoteEncryptionKeyProvider(type)
OldProvider-->>Config: provider instance
Config->>OldUnmarshal: UnmarshalConfig(raw map)
OldUnmarshal->>OldToMap: ToMap()
OldToMap-->>Config: processed map
Config->>CodeGen: pass processed map
Note over NewDirect,CodeGen: New Flow (Simplified)
NewDirect->>NewDirect: check key_provider exists
NewDirect->>CodeGen: pass raw cfg.Encryption
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ThisGuyCodes
force-pushed
the
thisguycodes/openbao-key-provider
branch
from
1e490dd to
d6c1344
Compare
ThisGuyCodes
force-pushed
the
thisguycodes/openbao-key-provider
branch
from
5af991c to
0cc09d8
Compare
ThisGuyCodes
changed the title
ThisGuyCodes
force-pushed
the
thisguycodes/openbao-key-provider
branch
from
54bddcb to
78b3223
Compare
ThisGuyCodes
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
.github/workflows/integration-test.yml(3 hunks)docs-starlight/src/content/docs/04-reference/01-hcl/02-blocks.mdx(1 hunks)go.mod(8 hunks)internal/remotestate/config.go(2 hunks)internal/remotestate/remote_encryption.go(0 hunks)internal/remotestate/remote_encryption_test.go(0 hunks)test/fixtures/tofu-state-encryption/openbao/terragrunt.hcl(1 hunks)test/helpers/testcontainer_helpers.go(1 hunks)test/integration_encryption_shared_test.go(1 hunks)test/integration_tofu_aws_state_encryption_test.go(0 hunks)test/integration_tofu_openbao_test.go(1 hunks)
💤 Files with no reviewable changes (3)
- internal/remotestate/remote_encryption_test.go
- test/integration_tofu_aws_state_encryption_test.go
- internal/remotestate/remote_encryption.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
⚙️ CodeRabbit configuration file
Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
Files:
internal/remotestate/config.gotest/integration_encryption_shared_test.gotest/integration_tofu_openbao_test.gotest/helpers/testcontainer_helpers.go
docs-starlight/**/*.md*
⚙️ CodeRabbit configuration file
Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow. There is currently a migration underway from the Jekyll based documentation in
docsto the Starlight + Astro based documentation indocs-starlight. Make sure that thedocs-starlightdocumentation is accurate and up-to-date with thedocsdocumentation, and that any difference between them results in an improvement in thedocs-starlightdocumentation.
Files:
docs-starlight/src/content/docs/04-reference/01-hcl/02-blocks.mdx
🧠 Learnings (1)
📚 Learning: 2025-08-19T16:05:54.723Z
Learnt from: Resonance1584
Repo: gruntwork-io/terragrunt PR: 4683
File: go.mod:86-90
Timestamp: 2025-08-19T16:05:54.723Z
Learning: When analyzing Go module dependencies for removal, always check for both direct imports and API usage across all Go files in the repository, not just a quick search. The github.com/mattn/go-zglob library is used for filesystem walking and glob expansion in multiple Terragrunt files including util/file.go, format commands, and AWS provider patch functionality.
Applied to files:
go.mod
🧬 Code graph analysis (2)
internal/remotestate/config.go (1)
codegen/generate.go (2)
EncryptionKeyProviderKey(69-69)RemoteStateConfigToTerraformCode(276-532)
test/integration_tofu_openbao_test.go (3)
test/helpers/testcontainer_helpers.go (2)
RunContainer(42-80)ContExecNoOutput(19-31)test/helpers/package.go (4)
CopyEnvironment(89-105)CopyAndFillMapPlaceholders(144-157)RunTerragrunt(979-983)FileIsInFolder(355-372)util/file.go (1)
JoinPath(626-628)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build / Build (linux/amd64)
- GitHub Check: base_tests / Test (ubuntu)
- GitHub Check: base_tests / Test (macos)
- GitHub Check: Pull Request has non-contributor approval
🔇 Additional comments (10)
docs-starlight/src/content/docs/04-reference/01-hcl/02-blocks.mdx (1)
602-602: LGTM! Documentation updated to reflect Openbao support.The addition of
openbaoto the list of supported key providers is clear and follows the existing documentation pattern..github/workflows/integration-test.yml (2)
156-159: LGTM! Docker setup appropriately configured for Linux runners.The setup-docker-action is correctly limited to Linux environments where testcontainers will be used.
230-233: LGTM! Tag appending logic correctly handles both cases.The bash parameter expansion properly appends the
dockertag whenHAS_DOCKERis true, handling both empty and non-emptyTAGSvalues correctly.internal/remotestate/config.go (1)
70-74: LGTM! Simplified validation enables pass-through encryption config.The change from validating the full encryption provider configuration to only checking for
key_providerexistence correctly implements the PR objective of passing encryption config through unchanged. This enables support for Openbao and future encryption providers without requiring code changes in Terragrunt.go.mod (1)
92-96: LGTM! Dependencies correctly added for testcontainers support.The addition of
docker/go-connectionsandtestcontainers-gois necessary for the new Docker-based integration tests and is used appropriately in the test helper code.test/helpers/testcontainer_helpers.go (2)
19-31: LGTM! Well-implemented container exec helper.The function properly parses shell commands, executes them in the container, and asserts successful execution with no output. Good use of
shellwords.Parsefor safe command parsing.
42-80: LGTM! Comprehensive container setup helper.The function properly handles test skipping conditions, configures the container with appropriate wait strategies, and returns both the container handle and mapped address. Good use of
testcontainers.CleanupContainerfor automatic cleanup.test/fixtures/tofu-state-encryption/openbao/terragrunt.hcl (1)
2-20: LGTM! Test fixture correctly configured for Openbao.The fixture properly defines the encryption block with Openbao provider and uses clear placeholder values that are replaced by the test code.
test/integration_encryption_shared_test.go (1)
17-43: LGTM! Well-designed shared encryption validation helper.The function properly validates that state files are encrypted by checking for the
encrypted_datafield and verifying it's base64-encoded. Good use of descriptive assertions and proper error handling.test/integration_tofu_openbao_test.go (1)
45-71: LGTM! Comprehensive integration test for Openbao encryption.The test properly sets up an Openbao container, configures a transit key, applies the Terragrunt configuration, and validates that the state file is encrypted. Good use of parallel testing and shared helpers.
| @@ -0,0 +1,20 @@ | |||
| # Test GCP KMS encryption with local state | |||
There was a problem hiding this comment.
Fix misleading comment.
The comment states "Test GCP KMS encryption" but this fixture is actually for testing Openbao encryption (as indicated by key_provider = "openbao" on line 15).
Apply this diff:
-# Test GCP KMS encryption with local state
+# Test Openbao encryption with local state📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Test GCP KMS encryption with local state | |
| # Test Openbao encryption with local state |
🤖 Prompt for AI Agents
In test/fixtures/tofu-state-encryption/openbao/terragrunt.hcl around line 1, the
top-line comment incorrectly says "Test GCP KMS encryption" while this fixture
uses Openbao (key_provider = "openbao"); update the comment to accurately
describe that this is a test for Openbao state encryption (e.g., "Test Openbao
encryption with local state" or similar), ensuring it reflects the key_provider
and purpose of the fixture.
| func setupOpenbao(t *testing.T) (bao *testcontainers.DockerContainer, addr string, rootToken string) { | ||
| t.Helper() | ||
|
|
||
| baoToken := rand.Text() | ||
|
|
||
| baoC, baoAddress := helpers.RunContainer(t, "openbao/openbao:2.4.1", 8200, | ||
| testcontainers.WithWaitStrategy( | ||
| wait.ForLog("core: vault is unsealed"), | ||
| ), | ||
| testcontainers.WithEnv(map[string]string{ | ||
| "BAO_DEV_ROOT_TOKEN_ID": baoToken, | ||
| }), | ||
| ) | ||
|
|
||
| execOptions := []tcexec.ProcessOption{ | ||
| tcexec.WithEnv([]string{"BAO_ADDR=http://localhost:8200", "VAULT_TOKEN=" + baoToken}), | ||
| } | ||
|
|
||
| helpers.ContExecNoOutput(t, baoC, "bao secrets enable transit", execOptions...) | ||
|
|
||
| return baoC, baoToken, baoAddress | ||
| } |
There was a problem hiding this comment.
Fix parameter order inconsistency.
The function signature at line 22 declares (bao *testcontainers.DockerContainer, addr string, rootToken string) but the return statement at line 42 returns baoC, baoToken, baoAddress. The order of addr and rootToken appears to be swapped.
Looking at the test usage on line 48, it calls baoC, baoToken, baoAddr := setupOpenbao(t), which suggests the current return order is (container, token, address), not matching the signature (container, address, token).
Apply this diff to fix the signature:
-func setupOpenbao(t *testing.T) (bao *testcontainers.DockerContainer, addr string, rootToken string) {
+func setupOpenbao(t *testing.T) (bao *testcontainers.DockerContainer, rootToken string, addr string) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func setupOpenbao(t *testing.T) (bao *testcontainers.DockerContainer, addr string, rootToken string) { | |
| t.Helper() | |
| baoToken := rand.Text() | |
| baoC, baoAddress := helpers.RunContainer(t, "openbao/openbao:2.4.1", 8200, | |
| testcontainers.WithWaitStrategy( | |
| wait.ForLog("core: vault is unsealed"), | |
| ), | |
| testcontainers.WithEnv(map[string]string{ | |
| "BAO_DEV_ROOT_TOKEN_ID": baoToken, | |
| }), | |
| ) | |
| execOptions := []tcexec.ProcessOption{ | |
| tcexec.WithEnv([]string{"BAO_ADDR=http://localhost:8200", "VAULT_TOKEN=" + baoToken}), | |
| } | |
| helpers.ContExecNoOutput(t, baoC, "bao secrets enable transit", execOptions...) | |
| return baoC, baoToken, baoAddress | |
| } | |
| func setupOpenbao(t *testing.T) (bao *testcontainers.DockerContainer, rootToken string, addr string) { | |
| t.Helper() | |
| baoToken := rand.Text() | |
| baoC, baoAddress := helpers.RunContainer(t, "openbao/openbao:2.4.1", 8200, | |
| testcontainers.WithWaitStrategy( | |
| wait.ForLog("core: vault is unsealed"), | |
| ), | |
| testcontainers.WithEnv(map[string]string{ | |
| "BAO_DEV_ROOT_TOKEN_ID": baoToken, | |
| }), | |
| ) | |
| execOptions := []tcexec.ProcessOption{ | |
| tcexec.WithEnv([]string{"BAO_ADDR=http://localhost:8200", "VAULT_TOKEN=" + baoToken}), | |
| } | |
| helpers.ContExecNoOutput(t, baoC, "bao secrets enable transit", execOptions...) | |
| return baoC, baoToken, baoAddress | |
| } |
🤖 Prompt for AI Agents
In test/integration_tofu_openbao_test.go around lines 22 to 43, the function
signature declares (bao *testcontainers.DockerContainer, addr string, rootToken
string) but the function actually returns (baoC, baoToken, baoAddress) and tests
call baoC, baoToken, baoAddr := setupOpenbao(t); fix by reordering the signature
parameters to match the returned values and call sites — change the signature to
(bao *testcontainers.DockerContainer, rootToken string, addr string) so the
second return is the root token and the third is the address.
Description
Fixes #5036
We currently parse the encryption config, duplicating some of the schema in this repo. I believe we actively do not want to be in the business of validating the Terraform / Tofu configuration (the the extent possible). So this PR changes to just blindly pass through the config.
This also introduces Testcontainers to test the Openbao integration. GHA is updated to run https://github.com/docker/setup-docker-action as part of all integration tests, and the
dockerbuild tag is injected into all test runs regardless of matrix content. This was to avoid a permutation explosion in the combinations of build tags; plus I will be advocating for using testcontainers to solve more integration test needs moving forward.TODOs
Release Notes (draft)
Transparently support current and future encryption providers.
Summary by CodeRabbit
New Features
Tests