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.

Sign up for GitHub

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

Jump to bottom

Support helm-secrets v4.0.0 #360

Merged
merged 1 commit into from Sep 17, 2022
Merged

Support helm-secrets v4.0.0 #360

merged 1 commit into from Sep 17, 2022

Conversation

KqLLL
Copy link
Contributor

@KqLLL KqLLL commented Sep 13, 2022 •
edited

Helm-secrets has been released to v4.0.0 and the secret view command has been deprecated.
Helm-secrets change log, Helm secret view command. The helm secret decrypt command is a drop-in replacement.

@yxxhero
Copy link
Member

yxxhero commented Sep 13, 2022

@KqLLL plesae fix DCO.
@mumoshu how many helm-secret versions we should support?

@yxxhero
Copy link
Member

yxxhero commented Sep 13, 2022

@KqLLL ping

@yxxhero
Copy link
Member

yxxhero commented Sep 13, 2022

@KqLLL https://github.com/helmfile/helmfile/pull/360/checks?check_run_id=8319754882

@KqLLL
Copy link
Contributor Author

KqLLL commented Sep 13, 2022

@KqLLL plesae fix DCO. @mumoshu how many helm-secret versions we should support?

Fixed

@yxxhero
Copy link
Member

yxxhero commented Sep 13, 2022

@KqLLL I feel that we should be compatible with at least three versions of helm-secret at the same time.

@yxxhero
Copy link
Member

yxxhero commented Sep 13, 2022

@KqLLL example:
4.0.0
3.15.x
3.14.x

@KqLLL
Copy link
Contributor Author

KqLLL commented Sep 13, 2022 •
edited

@KqLLL I feel that we should be compatible with at least three versions of helm-secret at the same time.

I think the parameters are judged according to the major version number？

@yxxhero
Copy link
Member

yxxhero commented Sep 13, 2022

add testcase for helm-secret version 3 and 4 @KqLLL Thanks very much.

@KqLLL
Copy link
Contributor Author

KqLLL commented Sep 14, 2022 •
edited

add testcase for helm-secret version 3 and 4 @KqLLL Thanks very much.

testcase , Is this feasible?

v3

➜  cluster-infra git:(master) ✗ helm plugin list                                                              
NAME    VERSION DESCRIPTION                                                                  
diff    3.1.3   Preview helm upgrade changes as a diff                                       
secrets 3.15.0  This plugin provides secrets values encryption for Helm charts secure storing
➜  cluster-infra git:(master) ✗ ~/go/bin/helmfile-test -e minor template > output 
Adding repo vmware-tanzu https://vmware-tanzu.github.io/helm-charts
"vmware-tanzu" has been added to your repositories

Decrypting secret /Users/lllkq/Projects/kube-infra/cluster-infra/values/velero/secrets.yaml
Templating release=velero, chart=vmware-tanzu/velero

v4

➜  cluster-infra git:(master) ✗ helm plugin list                    
NAME    VERSION DESCRIPTION                                                                  
diff    3.1.3   Preview helm upgrade changes as a diff                                       
secrets 4.0.0   This plugin provides secrets values encryption for Helm charts secure storing
➜  cluster-infra git:(master) ✗ ~/go/bin/helmfile-test -e minor template > output 
Adding repo vmware-tanzu https://vmware-tanzu.github.io/helm-charts
"vmware-tanzu" has been added to your repositories

Decrypting secret /Users/lllkq/Projects/kube-infra/cluster-infra/values/velero/secrets.yaml
Templating release=velero, chart=vmware-tanzu/velero

@KqLLL KqLLL requested a review from yxxhero September 14, 2022 02:24
@yxxhero yxxhero linked an issue Sep 14, 2022 that may be closed by this pull request
Secrets fails to decrypt #365
Closed
@yxxhero yxxhero mentioned this pull request Sep 14, 2022
Closed
@yxxhero
Copy link
Member

yxxhero commented Sep 15, 2022

@KqLLL Please fix ci issue.

@KqLLL
Copy link
Contributor Author

KqLLL commented Sep 15, 2022

@KqLLL Please fix ci issue.

ok

@KqLLL
Copy link
Contributor Author

KqLLL commented Sep 15, 2022

@KqLLL Please fix ci issue.

Fixed

@yxxhero
Copy link
Member

yxxhero commented Sep 15, 2022

Good works. I have a idea. We can use github matrix to test helm-secrets version contains 3.x 4.x. WDYT? @KqLLL

@KqLLL
Copy link
Contributor Author

KqLLL commented Sep 15, 2022

Good works. I have a idea. We can use github matrix to test helm-secrets version contains 3.x 4.x. WDYT? @KqLLL

Sounds good, I will try it.

yxxhero
yxxhero reviewed Sep 15, 2022
View reviewed changes
pkg/helmexec/exec_test.go Outdated Show resolved Hide resolved
@KqLLL KqLLL requested a review from yxxhero September 15, 2022 08:57
yxxhero
yxxhero reviewed Sep 15, 2022
View reviewed changes
test/integration/run.sh Outdated Show resolved Hide resolved
@KqLLL KqLLL changed the title Replace secret-view command with secret-decrypt command Support helm-secrets 4.0.0 Sep 15, 2022
yxxhero
yxxhero reviewed Sep 15, 2022
View reviewed changes
test/integration/run.sh Outdated Show resolved Hide resolved
@KqLLL KqLLL changed the title Support helm-secrets 4.0.0 Support helm-secrets v4.0.0 Sep 15, 2022
@KqLLL KqLLL requested a review from yxxhero September 15, 2022 10:16
yxxhero
yxxhero reviewed Sep 15, 2022
View reviewed changes
pkg/helmexec/exec.go Outdated Show resolved Hide resolved
yxxhero
yxxhero reviewed Sep 16, 2022
View reviewed changes
pkg/helmexec/exec.go Outdated Show resolved Hide resolved
@KqLLL
Support helm-secret v4.0.0
572902e 
Signed-off-by: KqLLL <lllkq546449541@gmail.com>
@yxxhero
Copy link
Member

yxxhero commented Sep 16, 2022

@KqLLL great works. LGTM
@mumoshu WDYT?

yxxhero
yxxhero approved these changes Sep 16, 2022
View reviewed changes
Copy link
Member

@yxxhero yxxhero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mumoshu
mumoshu approved these changes Sep 17, 2022
View reviewed changes
Copy link
Contributor

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Thanks a lot for making it backward-compatible. I love the idea of supporting two major versions of the plugin- it would allow users to gradually migrate to a newer plugin version. I also love how you wrote comprehensive test cases for this change. Definitely LGTM!

@mumoshu mumoshu merged commit 0fbcb07 into helmfile:main Sep 17, 2022
@KqLLL KqLLL deleted the fix-unknown-view-command-error-for-helm-secret branch September 17, 2022 11:12
@mumoshu mumoshu mentioned this pull request Sep 17, 2022
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mumoshu mumoshu mumoshu approved these changes

@yxxhero yxxhero yxxhero approved these changes

@itscaro itscaro Awaiting requested review from itscaro itscaro is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Secrets fails to decrypt
3 participants
@KqLLL @yxxhero @mumoshu