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

fix bool,deprecated flags regression #293

Merged
merged 2 commits into from
May 7, 2024

Conversation

ibihim
Copy link
Collaborator

@ibihim ibihim commented May 7, 2024

What

Fix a regression for the removed flags.

Why

If the bool value flag was specified like --logtostderr, without the =true, it was treated like --logtostderr=true in v0.15.0.
In v0.16.0 and v0.17.0, --logtostderr without any value would treat the follow up flag as its value, which leads to a regression.

@ibihim ibihim force-pushed the deprecated-flags-regression-fix branch from 03e0825 to 247af26 Compare May 7, 2024 09:32
@ibihim ibihim changed the title c/k/a/opts: fix bool,deprecated flags regression fix bool,deprecated flags regression May 7, 2024
Copy link

@liouk liouk left a comment

Choose a reason for hiding this comment

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

I would split the changes of cmd/kube-rbac-proxy/app/kube-rbac-proxy.go to a separate commit as this fixes a different issue.

@@ -492,6 +492,10 @@ func Run(cfg *completedProxyRunOptions) error {
})
}

if cfg.secureListenAddress == "" && cfg.insecureListenAddress == "" {
Copy link

Choose a reason for hiding this comment

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

I believe we're most widely using the convention len(somestr) == 0 to check for empty strings.

@ibihim ibihim force-pushed the deprecated-flags-regression-fix branch from 247af26 to b2e9035 Compare May 7, 2024 12:06
@liouk
Copy link

liouk commented May 7, 2024

/lgtm

@ibihim ibihim merged commit 8ef7564 into brancz:master May 7, 2024
7 checks passed
@ibihim ibihim deleted the deprecated-flags-regression-fix branch May 7, 2024 13:25
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.

2 participants