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

Improve cobra cli and logrus logs #51

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Nicolas-Peiffer
Copy link
Contributor

This PR addresses the following issues:

@Nicolas-Peiffer Nicolas-Peiffer force-pushed the improve-cobra-cli-and-logs branch 4 times, most recently from 1072105 to dfb9af5 Compare February 19, 2025 16:06
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]>
@Nicolas-Peiffer Nicolas-Peiffer force-pushed the improve-cobra-cli-and-logs branch from dfb9af5 to 5ada26e Compare February 19, 2025 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant