-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add ruleSet and KMS setting for CLI CSFLE during init #2942
base: main
Are you sure you want to change the base?
Conversation
🎉 All Contributor License Agreements have been signed. Ready to merge. |
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.
Thanks @channingdong , left some comments
currDir, err := os.Getwd() | ||
if err != nil { | ||
return nil, fmt.Errorf("Error getting current working directory: %v\n", err) | ||
} | ||
log.CliLogger.Debugf("Current working directory is: %s\n", currDir) | ||
|
||
// Copy all the built-in proto schemas needed for CSFLE to <importPath> where the main schema is stored | ||
// Note: folder path should be set correctly based on current working directory | ||
// Expected working directory in CLI test is: /Users/github.com/confluentinc/cli/pkg/serdes | ||
// Expected working directory in CLI shell execution is: /Users/github.com/confluentinc/cli | ||
for _, folder := range builtInSchemaFoldersToCopy { | ||
dst := importPath + "/" + folder | ||
|
||
// Skip copying the built-in schema folders if they are present already in the temp folder | ||
if _, err = os.Stat(dst); err == nil { | ||
log.CliLogger.Debugf("Built-in schema folder already exists %s, skipping copy again:\n", dst) | ||
continue | ||
} | ||
|
||
// Locate the source of built-in schema folders | ||
if strings.HasSuffix(currDir, "confluentinc/cli") { | ||
folder = "pkg/serdes/" + folder | ||
} | ||
if err = copy.Copy(folder, dst); err != nil { | ||
return nil, fmt.Errorf("Error copying built-in schemas folder %s: %w\n", folder, err) | ||
} |
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.
@sgagniere This part is something relatively new to CLI, your review will be appreciated!
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.
Thanks @channingdong , LGTM
…ate test cases accordingly
27bb169
to
5cbc67b
Compare
This reverts commit 5cbc67b.
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.
After syncing w/ Channing: we need to make the proto files available even when users don't have any copy on their machine
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.
lgtm
Release Notes
This PR serves as a following up PR to finalize the CSFLE work on CLI, including the following changes:
Breaking Changes
New Features
ruleSet
and Key Management Service (KMS) driver support to finalize the Client Side Field Level Encryption (CSFLE) feature forconfluent kafka topic [produce | consume]
Bug Fixes
Checklist
What
ruleSet
can be extracted correctly from the native Go object.ruleSet
correctly.confluent-kafka-go
library to latest version.References
Test & Review
The manual verification passing result can be found here:
https://docs.google.com/document/d/1GwXz9hNOkub_Br-2nssoYWCf6elZBvwo7TMhCNYinwE/edit?tab=t.0#heading=h.1xcbdvagwov4