-
Notifications
You must be signed in to change notification settings - Fork 59
feat: Add JSON Log Format Support to Vault CSI Provider #398
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
|
||
// set log level | ||
logger = setupLogger(flags) | ||
logger := setupLogger(flags) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we are not passing logger during func realmain()
so logger needs to be created after parsing the Flagconfig template.
} | ||
|
||
func setupLogger(flags config.FlagsConfig) hclog.Logger { | ||
logger := hclog.Default() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is removed on purpose so that we can manipulate hclog logger Default values futher.
} | ||
|
||
// Create logger with options | ||
logger := hclog.New(&hclog.LoggerOptions{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created new logger format replacing default to pass Jsonformat flag. Following RFC3339Nano Timeformat.
args: | ||
- -endpoint=/provider/vault.sock | ||
- -log-level=info | ||
- -log-format=LOG_FORMAT_PLACEHOLDER # text or json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This placeholder coming from root dir Makefile to test log format
Makefile
Outdated
-rm -rf $(BUILD_DIR) | ||
|
||
# Test log format: JSON and TEXT | ||
test-log-format: e2e-image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels a bit odd to have a separate command for testing the log format like this. The unit test already covers the logic.
We can either choose to remove this from here and the doc or add it to the existing e2e.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed it, can be tested manually later on after cluster spin up. thanks
Summary
This PR adds support for JSON-formatted logging to the vault-csi-provider, enabling better integration with log aggregation systems like Datadog, ELK, and Splunk.
Changes
Core Implementation
New CLI Flag:
-log-format
flag with support forjson
andtext
formatstext
to maintain backward compatibilityJSON
,TEXT
, etc.)Files Modified:
main.go
: ImplementedsetupLogger()
function with format selection logicinternal/config/config.go
: AddedLogFormat
field toFlagsConfig
Testing
Unit Tests (
main_test.go
):TestSetupLoggerFormat
: Validates format configuration (JSON, TEXT, default)TestSetupLoggerFormatValidation
: Tests case-insensitive validationTestSetupLoggerIntegration
: End-to-end JSON output verificationTest Configuration:
test/bats/configs/vault-csi-provider-test.yaml
: Kubernetes test manifesttest/bats/configs/test-app-with-vault-secrets.yaml
: Sample app for mount testingUsage
Command Line
Kubernetes Deployment
Example Output
JSON Format
Manual Testing
Benefits
Migration Guide
For existing deployments wanting to switch to JSON logging:
-log-format=json
in container argsRelated Issues
#177
PCI review checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.