Skip to content

fix: repair crash-looping CLI flags, close values.yaml coverage gaps - #6

Merged
lakhansamani merged 2 commits into
mainfrom
fix/cli-flag-coverage
Aug 8, 2026
Merged

fix: repair crash-looping CLI flags, close values.yaml coverage gaps#6
lakhansamani merged 2 commits into
mainfrom
fix/cli-flag-coverage

Conversation

@lakhansamani

Copy link
Copy Markdown
Contributor

Summary

  • --enable-totp-login/--enable-mfa/--enable-email-otp/--enable-sms-otp don't exist in the authorizer binary; cobra rejects unknown flags and exits 1, so every release from this chart currently crash-loops on boot. Renamed to the current --disable-totp-login/--disable-webauthn-mfa/--disable-email-otp/--disable-sms-otp/--disable-mfa flags.
  • Fixed COUCHBASE_RAM_QUOTA: the env var was defined as COUCHBASE_BUCKET_RAM_QUOTA while the args template read $COUCHBASE_RAM_QUOTA, so authorizer.couchbase_bucket_ram_quota in values.yaml silently never reached the flag.
  • Wired --url (CWE-640 host-header-injection mitigation): AUTHORIZER_URL was set as a pod env var but never passed as a CLI flag, so authorizer.authorizer_url in values.yaml was a no-op. Added --oauth2-1-strict and --enable-org-discovery, both previously missing entirely.
  • Exposed ~25 toggles in values.yaml that were previously hardcoded into the args template with no values.yaml key at all — enable_signup, enforce_mfa, app_cookie_secure, enable_basic_authentication, disable_admin_header_auth, and others. Operators had no way to override them; defaults are unchanged.
  • Added host, env, log_level as values.yaml keys (previously only settable by editing the template).
  • Re-pinned appVersion to 2.4.0-rc.7 and bumped chart version to 2.3.0: 2.3.0 predates the flag rename above, so it doesn't have the --disable-* flags either. Move to the stable 2.4.0 tag once it ships.

Test plan

  • helm lint . passes
  • helm template renders valid YAML with all values set; verified all 130 flags from cmd/root.go appear in the rendered args
  • Extracted the exact rendered Deployment args/env and ran them against the real quay.io/authorizer/authorizer:2.4.0-rc.7 image — container boots and serves /healthz (previously exited 1 on the phantom flags)
  • Deploy to a real cluster and confirm the pod reaches Ready

--enable-totp-login/--enable-mfa/--enable-email-otp/--enable-sms-otp
don't exist in the authorizer binary; cobra rejects unknown flags so
every release from this chart crash-loops on boot. Renamed to the
current --disable-totp-login/--disable-webauthn-mfa/--disable-email-otp/
--disable-sms-otp/--disable-mfa flags.

Fixed COUCHBASE_RAM_QUOTA: the env var was defined as
COUCHBASE_BUCKET_RAM_QUOTA while args read $COUCHBASE_RAM_QUOTA, so
couchbase_bucket_ram_quota silently never reached the flag.

Wired --url (CWE-640 host-header-injection mitigation): AUTHORIZER_URL
was set as an env var but never passed as a flag, so authorizer_url
in values.yaml was a no-op. Added --oauth2-1-strict and
--enable-org-discovery.

Exposed ~25 toggles in values.yaml that were previously hardcoded
into the args template with no values.yaml key at all (enable_signup,
enforce_mfa, app_cookie_secure, enable_basic_authentication,
disable_admin_header_auth, and friends) — operators had no way to
override them.

Re-pinned appVersion to 2.4.0-rc.7: 2.3.0 predates the flag rename
above, so it doesn't have the --disable-* flags either. Move to the
stable 2.4.0 tag once it ships.
@netlify

netlify Bot commented Jul 23, 2026

Copy link
Copy Markdown

Deploy Preview for authorizer-helm-chart ready!

Name Link
🔨 Latest commit 093663b
🔍 Latest deploy log https://app.netlify.com/projects/authorizer-helm-chart/deploys/6a7710d887165900086010c9
😎 Deploy Preview https://deploy-preview-6--authorizer-helm-chart.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

The flag-coverage work on this branch was rebased into PR #7 and merged there.
Main is the superset: it additionally replaced the `| default true` pattern
with the authorizer.bool helper (so an explicit false is honoured) and set the
enforce_mfa default to false to match 2.4.0.

Conflicts resolved entirely in main's favour so merging this PR cannot regress
either fix.

# Conflicts:
#	Chart.yaml
#	templates/deployment.yaml
#	values.yaml
@lakhansamani

Copy link
Copy Markdown
Contributor Author

Merging this as a no-op, deliberately, rather than as a content change.

This branch's flag-coverage work was rebased into #7 and merged there, so main already has it. Main is also the superset#7 additionally:

  • replaced the | default true | toString pattern with the authorizer.bool helper, so an explicitly-set false is honoured instead of silently becoming true
  • set the enforce_mfa default to false, matching the 2.4.0 server default

Merging this branch as it stood would have regressed both. A dry-run merge conflicted in Chart.yaml, values.yaml and templates/deployment.yaml, and the 12 lines it would have introduced that main lacks were exactly the superseded | default true forms plus enforce_mfa: true.

So I merged main into this branch and resolved every conflict in main's favour. The branch tree is now byte-identical to main:

$ git diff origin/main --stat
(empty)

Verified after resolution: 14 authorizer.bool uses, 0 remaining | default true | toString, enforce_mfa default false, helm lint clean, chart renders 2.4.0-rc.18.

Net effect: this PR closes out as merged and main is unchanged. If you'd rather have clean history, closing it as superseded is equivalent — say so and I'll do that instead.

@lakhansamani
lakhansamani merged commit 08a9731 into main Aug 8, 2026
4 checks passed
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.

1 participant