Skip to content

fix: HELM_DIFF_IGNORE_UNKNOWN_FLAGS ignored issue #538

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

Merged
merged 3 commits into from
Jan 27, 2024
Merged

fix: HELM_DIFF_IGNORE_UNKNOWN_FLAGS ignored issue #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a798023
fix: HELM_DIFF_IGNORE_UNKNOWN_FLAGS ignored issue
yxxhero Jan 19, 2024
c7f1b0d
remove unused file
yxxhero Jan 19, 2024
afd375b
Merge branch 'databus23:master' into fix-HELM_DIFF_IGNORE_UNKNOWN_FLA…
yxxhero Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ release/
.envrc
.idea
docker-run-release-cache/
.vscode/
14 changes: 10 additions & 4 deletions cmd/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func newChartCommand() *cobra.Command {
diff := diffCmd{
namespace: os.Getenv("HELM_NAMESPACE"),
}
unknownFlags := os.Getenv("HELM_DIFF_IGNORE_UNKNOWN_FLAGS") == "true"

cmd := &cobra.Command{
Use: "upgrade [flags] [RELEASE] [CHART]",
Expand Down Expand Up @@ -164,13 +165,18 @@ func newChartCommand() *cobra.Command {
" --dry-run=server enables the cluster access with helm-get and the lookup template function."
)

cmdFlags := cmd.Flags()

// see: https://github.com/databus23/helm-diff/issues/537
cmdFlags.ParseErrorsWhitelist.UnknownFlags = unknownFlags
Copy link
Collaborator

@mumoshu mumoshu Jan 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Wow, good catch! Thanks.

So does this basically mean DisableFlagParsing: true, results in `UnknownFlags NOT set by cobra? (Just confirming for learning purpose

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this will disrupt a lot of functionality. I think we only support -- dry run=something. @mumoshu WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@yxxhero Thanks! Re dry-run, it won't be a good idea to start dropping the backward-compatibility without any prenotices. We are still in the same major ver of helm-diff (3.x). Dropping the compatibility may break downstream user CI/CD pipelines and/or tools.

I think this justifies having more test coverage in future though 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mumoshu yeah. make sense.


legacyDryRunFlagSet := pflag.NewFlagSet("upgrade", pflag.ContinueOnError)
legacyDryRun := legacyDryRunFlagSet.Bool("dry-run", false, dryRunUsage)
if err := legacyDryRunFlagSet.Parse(args); err == nil && *legacyDryRun {
diff.dryRunModeSpecified = true
args = legacyDryRunFlagSet.Args()
} else {
cmd.Flags().StringVar(&diff.dryRunMode, "dry-run", "", dryRunUsage)
cmdFlags.StringVar(&diff.dryRunMode, "dry-run", "", dryRunUsage)
}

// Here we parse the flags ourselves so that we can support
Expand All @@ -181,11 +187,11 @@ func newChartCommand() *cobra.Command {
//
// This works becase we have `DisableFlagParsing: true`` above.
// Never turn that off, or you'll get the error again.
if err := cmd.Flags().Parse(args); err != nil {
if err := cmdFlags.Parse(args); err != nil {
return err
}

args = cmd.Flags().Args()
args = cmdFlags.Args()

if !diff.dryRunModeSpecified {
dryRunModeSpecified := cmd.Flags().Changed("dry-run")
Expand Down Expand Up @@ -254,7 +260,7 @@ func newChartCommand() *cobra.Command {
return diff.runHelm3()
},
FParseErrWhitelist: cobra.FParseErrWhitelist{
UnknownFlags: os.Getenv("HELM_DIFF_IGNORE_UNKNOWN_FLAGS") == "true",
UnknownFlags: unknownFlags,
},
}

Expand Down