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

feat: allow to set default ingressclass #992

Merged
merged 1 commit into from
Feb 24, 2024
Merged

feat: allow to set default ingressclass #992

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions cmd/apps/nginx_app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ flag and the ingress-nginx docs for more info`,
nginx.Flags().StringP("namespace", "n", "default", "The namespace used for installation")
nginx.Flags().Bool("update-repo", true, "Update the helm repo")
nginx.Flags().Bool("host-mode", false, "If we should install ingress-nginx in host mode.")
nginx.Flags().Bool("default-ingress", true, "Is this the default ingressClass for the cluster?")
Copy link
Contributor

Choose a reason for hiding this comment

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

If we set this as default isn't this a backward incompatible change?

I think it would be better with default false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but what's the point for it being false? i thought that idea is to preconfigure application to be up&fully functional for development/testing purposes.

Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear, your approach makes more sense, if we have no promise for backward compatibility (which afaik we doesn't) you are right, it is better this way then.

nginx.Flags().StringArray("set", []string{}, "Use custom flags or override existing flags \n(example --set image=org/repo:tag)")

nginx.RunE = func(command *cobra.Command, args []string) error {
Expand Down Expand Up @@ -57,6 +58,14 @@ flag and the ingress-nginx docs for more info`,
overrides["controller.kind"] = "DaemonSet"
}

defaultIngress, flagErr := command.Flags().GetBool("default-ingress")
if flagErr != nil {
return flagErr
}
if defaultIngress {
overrides["controller.ingressClassResource.default"] = "true"
}

customFlags, _ := command.Flags().GetStringArray("set")

if err := config.MergeFlags(overrides, customFlags); err != nil {
Expand Down