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

Use new helm support for lookups in helm dry-run #449

Closed
MichaelMorrisEst opened this issue Apr 11, 2023 · 5 comments
Closed

Use new helm support for lookups in helm dry-run #449

MichaelMorrisEst opened this issue Apr 11, 2023 · 5 comments

Comments

@MichaelMorrisEst
Copy link
Contributor

PR 9426 in helm will allow setting the value for the --dry-run flag to enable the execution of lookups towards the cluster.
I would like to submit a PR that will use this new option in order to solve #263

I am hopeful the PR in helm will be merged soon and would like to start working on the implementation in helm-diff and would appreciate any thoughts. In particular, is it ok to change the implementation to always use "--dry-run=server" (i.e. execute lookups) or do we need the existing behaviour by default and a new flag in helm-diff to enable using the new helm flag?
@MichaelMorrisEst MichaelMorrisEst mentioned this issue Apr 11, 2023
Closed
@MichaelMorrisEst
Copy link
Contributor Author

@databus23 any thoughts on how you would like to see this implemented?
i.e. is it ok to change the implementation to always use "--dry-run=server" (i.e. execute lookups) or do we need the existing behaviour by default and a new flag in helm-diff to enable using the new helm flag?

@databus23
Copy link
Owner

databus23 commented Apr 24, 2023 •
edited

If the outcome as identical to the current output with the added benefit that the lookup function is evaluated as it will be during upgrade I'm all for always using --dry-run=server. But we need to maintain backward compatibility with older helm versions that don't support the new flag yet and fallback to the currently used command invocation if older versions are being used.

MichaelMorrisEst added a commit to Nordix/helm-diff that referenced this issue May 5, 2023
@MichaelMorrisEst
Use dry-run=server to enable lookups
3be20a8 
Helm PR helm/helm#9426 enables support for executing lookups during dry run. This PR is to make use of this new support in helm-diff.
Backwards compatibility for older versions of helm is maintained by checking the helm version before setting the flag

Addresses issue: databus23#449

Signed-off-by: MichaelMorris <michael.morris@est.tech>
@MichaelMorrisEst MichaelMorrisEst mentioned this issue May 9, 2023
Merged
bdwyertech pushed a commit to bdwyertech/helm-diff that referenced this issue May 16, 2023
@MichaelMorrisEst @bdwyertech
Use dry-run=server to enable lookups
be3cf0e 
Helm PR helm/helm#9426 enables support for executing lookups during dry run. This PR is to make use of this new support in helm-diff.
Backwards compatibility for older versions of helm is maintained by checking the helm version before setting the flag

Addresses issue: databus23#449

Signed-off-by: MichaelMorris <michael.morris@est.tech>
(cherry picked from commit 3be20a8)
bdwyertech pushed a commit to bdwyertech/helm-diff that referenced this issue May 16, 2023
@MichaelMorrisEst @bdwyertech
Use dry-run=server to enable lookups
081c9b1 
Helm PR helm/helm#9426 enables support for executing lookups during dry run. This PR is to make use of this new support in helm-diff.
Backwards compatibility for older versions of helm is maintained by checking the helm version before setting the flag

Addresses issue: databus23#449

Signed-off-by: MichaelMorris <michael.morris@est.tech>
(cherry picked from commit 3be20a8)
MichaelMorrisEst added a commit to Nordix/helm-diff that referenced this issue Jul 25, 2023
@MichaelMorrisEst
Use dry-run=server to enable lookups
d6631aa 
Helm PR helm/helm#9426 enables support for executing lookups during dry run. This PR is to make use of this new support in helm-diff.
Backwards compatibility for older versions of helm is maintained by checking the helm version before setting the flag

Addresses issue: databus23#449

Signed-off-by: MichaelMorris <michael.morris@est.tech>
@dudicoco
Copy link

I'm all for the new flag as I've been waiting for it as well, however it should be added explicitly by the user due to the fact that this will make helm contact the k8s api server, which could be security concern for some users, especially when using a third party chart.

mumoshu pushed a commit that referenced this issue Aug 21, 2023
@MichaelMorrisEst
Use dry-run=server to enable lookups (#458)
c54f085 
Helm PR helm/helm#9426 enables support for executing lookups during dry run. This PR is to make use of this new support in helm-diff.
Backwards compatibility for older versions of helm is maintained by checking the helm version before setting the flag

Addresses issue: #449

Signed-off-by: MichaelMorris <michael.morris@est.tech>
@mumoshu mumoshu mentioned this issue Aug 21, 2023
Merged
@antoineozenne-at-leocare
Copy link

Any news on this?

@gdubicki gdubicki mentioned this issue Dec 22, 2023
Merged
@yxxhero yxxhero closed this as completed Jan 27, 2024
@mumoshu
Copy link
Collaborator

mumoshu commented Jan 27, 2024

The support in helm-diff is available since 3.9.0 and considered complete since 3.9.4 released today! Enjoy!

@jv4n5e jv4n5e mentioned this issue Feb 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@mumoshu @databus23 @yxxhero @dudicoco @MichaelMorrisEst @antoineozenne-at-leocare