-
Notifications
You must be signed in to change notification settings - Fork 12
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
Improve cobra cli and logrus logs #51
Open
Nicolas-Peiffer
wants to merge
3
commits into
master
Choose a base branch
from
improve-cobra-cli-and-logs
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1072105
to
dfb9af5
Compare
Squashing all commits into one. Indeed I removed the KMSv2 features from this branch to focus only on the cobra CLI improvements and logrus improvements. Begin replacement of github.com/ThalesGroup/k8s-kms-plugin/apis/k8s/v1beta1 with k8s.io/kms/apis/v2. However, there are other modification to do in order to update the lib with k8s.io/kms/apis/v2. Indeed, there are both changes from v1beta1 and v2, as well as github.com/ThalesGroup/k8s-kms-plugin/apis/k8s/v1beta1 which has a custom forked implementation of the lib. See #40 (comment) Don't forget to fix TODOs Signed-off-by: Nicolas-Peiffer <[email protected]> Squashing all commits into one. Indeed I removed the KMSv2 features from this branch to focus only on the cobra CLI improvements and logrus improvements. Separate cobra CLI var from root CLI and serve CLI Signed-off-by: Nicolas-Peiffer <[email protected]> Adding a Status method for P11 to match the KeyManagementServiceServer interface from k8s.io/kms/apis/v2 Signed-off-by: Nicolas-Peiffer <[email protected]> add back kekKeyId and sort lines Signed-off-by: Nicolas-Peiffer <[email protected]> add StatusResponse from k8s.io/kms/apis/v2 and remove KeyId and KeyringId In k8s.io/kms/apis/v2, DecryptRequest has no field KeyringId. EncryptRequest has no KeyId field, nor KeyringId field. Signed-off-by: Nicolas-Peiffer <[email protected]> golang coding style don't use Yoda conditions (ST1017) put the eval in the preffered order Signed-off-by: Nicolas-Peiffer <[email protected]> golang coding style: should merge variable declaration with assignment on next line (S1021) Signed-off-by: Nicolas-Peiffer <[email protected]> err was not used, now its catched Signed-off-by: Nicolas-Peiffer <[email protected]> fix: put err in the logrus.Errorf statement as it was not done Signed-off-by: Nicolas-Peiffer <[email protected]> TODO: fix unused variables Signed-off-by: Nicolas-Peiffer <[email protected]> fix: adding require.NoError to handle error variables that were not used before Signed-off-by: Nicolas-Peiffer <[email protected]> sort CLI flags var Signed-off-by: Nicolas-Peiffer <[email protected]> TODO: test1 and test2 are defined above, but never used. Use a blank identifier to explicitly ignore them while we fix this. Signed-off-by: Nicolas-Peiffer <[email protected]> updating k8s.io/kms and protobuf libs Signed-off-by: Nicolas-Peiffer <[email protected]> set debug flag to a PersistentFlags Signed-off-by: Nicolas-Peiffer <[email protected]> configure logrus log-level in PersistentPreRunE of root CLI Signed-off-by: Nicolas-Peiffer <[email protected]> add a logrus.Debugf message after setting the log level Signed-off-by: Nicolas-Peiffer <[email protected]> replace ioutils with os Signed-off-by: Nicolas-Peiffer <[email protected]> fix cobra CLI completion script by adding name of binary command of k8s-kms-plugin Signed-off-by: Nicolas-Peiffer <[email protected]> add logrus logs to improve logs from the gRPC API Signed-off-by: Nicolas-Peiffer <[email protected]> add keyId to P11 Signed-off-by: Nicolas-Peiffer <[email protected]> renaming kek key label variable Signed-off-by: Nicolas-Peiffer <[email protected]> Rename an attribute from P11 Signed-off-by: Nicolas-Peiffer <[email protected]> Use label in Findkey instead of keyId for P11 Encrypt Signed-off-by: Nicolas-Peiffer <[email protected]> set --socket flag as a persistent flag in cobra root CLI This adresses this issue #49 Signed-off-by: Nicolas-Peiffer <[email protected]> move variables that store CLI flags of the root CLI from serve to root Signed-off-by: Nicolas-Peiffer <[email protected]> change priority of env var SOCKET over CLI flags Related to #45 Signed-off-by: Nicolas-Peiffer <[email protected]> change name of env var to make it more consistant with test package Signed-off-by: Nicolas-Peiffer <[email protected]> change priority of env var P11_LIBRARY over CLI flags Related to #45 Signed-off-by: Nicolas-Peiffer <[email protected]> Use an helper method getValueFromCliFlagOrEnv to streamline the priority between envVar and CLI flag. change priority of env var P11_TOKEN P11_SLOT and P11_PIN over CLI flags Related to #45 Signed-off-by: Nicolas-Peiffer <[email protected]> move provider var init closer to where it is used Signed-off-by: Nicolas-Peiffer <[email protected]> consider removing this P11_PIN_FILE feature or refactor Signed-off-by: Nicolas-Peiffer <[email protected]> Removing LookupEnvOrString because it is replaced by getValueFromCliFlagOrEnv adding various comments to improve CLI understanding and raises TODO to consider Signed-off-by: Nicolas-Peiffer <[email protected]> add debug message to see EncryptRequest plaintext or debugging Signed-off-by: Nicolas-Peiffer <[email protected]> base 64 decode Signed-off-by: Nicolas-Peiffer <[email protected]> rename kms manifest to show this is v1 kubernetes KMS API Signed-off-by: Nicolas-Peiffer <[email protected]> add kms v2 kubernetes manifest Signed-off-by: Nicolas-Peiffer <[email protected]> improve host CLI comments and description Signed-off-by: Nicolas-Peiffer <[email protected]> Remove a logrus log because it is not helpful Signed-off-by: Nicolas-Peiffer <[email protected]> add a log-level CLI flags for logrus to help support other level of logs like trace See #47 Signed-off-by: Nicolas-Peiffer <[email protected]> improve loging over the parse config method Signed-off-by: Nicolas-Peiffer <[email protected]> sort imports Signed-off-by: Nicolas-Peiffer <[email protected]> add trace logrus for p11 method Signed-off-by: Nicolas-Peiffer <[email protected]> Adding the keyId in the EncryptResponse Signed-off-by: Nicolas-Peiffer <[email protected]> update goenv file to go1.23.0 Signed-off-by: Nicolas-Peiffer <[email protected]> upgrade dependencies Signed-off-by: Nicolas-Peiffer <[email protected]> Remove binary this was a commit mistake Signed-off-by: Nicolas-Peiffer <[email protected]> update cobra gose & crypto11 and other deps which might be useless Signed-off-by: Nicolas-Peiffer <[email protected]> revert from k8s.io/kms/apis/v2 to custom v1beta1 add RSA var Signed-off-by: Nicolas-Peiffer <[email protected]> revert from k8s.io/kms/apis/v2 to custom v1beta1 Signed-off-by: Nicolas-Peiffer <[email protected]> Update gose to v0.9.1 Signed-off-by: Nicolas-Peiffer <[email protected]> move k8s manifests sample to a deployments folder Signed-off-by: Nicolas-Peiffer <[email protected]> update path to manifest file Signed-off-by: Nicolas-Peiffer <[email protected]> upgrade go direct dependencies Signed-off-by: Nicolas-Peiffer <[email protected]>
dfb9af5
to
5ada26e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses the following issues:
--debug
flag is not a persistent flags and not working outside of root CLI #46--debug
flag is now a root persistent flag--log-level
instead of--debug
#47 Add the flag--log-level
and keep--debug
--socket
persistent flag is defined in multiple subcommands #49 Move--socket
to the root CLI persistentflags.